| Message ID | 1457473693-25776-1-git-send-email-jezz@sysmic.org |
|---|---|
| State | Rejected |
| Headers | show |
Jérôme, All, On 2016-03-08 22:48 +0100, Jérôme Pouiller spake thusly: > It is handy to use local.mk or external.mk to add specific targets > for current project. However, until now, it not possible to add help > message these targets. I have: $ make local-help Local blabla help displays here... > This patch add EXTRA_HELP variable. This variable is aimed to be assigned > from any .mk files. Its content is displayed with 'make help'. > > For exemple: > EXTRA_HELP += "flash - Flash target" > EXTRA_HELP += "chroot - Chroot into target/" > EXTRA_HELP += "qemu - Run image with qemu" > EXTRA_HELP += "install-nfs - Extract rootfs in \$$NFSROOT (=$(NFSROOT))" > EXTRA_HELP += "`printf '%-22s%s' '$(var)-feature' ' - Call $(var) feature'`" > EXTRA_HELP += "Please contact support@company.com in case of problem." This definitely does not look nice to me... :-( What we could do, however, would be: - in external.mk: BR2_HAS_EXTRA_HELP = YES # Whatever non empty - in the main Makefile: help: blabla our current help $(if $(BR2_HAS_EXTRA_HELP),echo " - local-help to get local help blabla) which would be IMHO much much nicer and much much simpler... Regards, Yann E. MORIN. > Signed-off-by: Jérôme Pouiller <jezz@sysmic.org> > --- > v2: > - Rename LOCAL_HELP to EXTRA_HELP > - Remove introduction lines (so, 'ifneq ($(LOCAL_HELP),)' is no more needed) > > Makefile | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Makefile b/Makefile > index f2822a2..1c9f63c 100644 > --- a/Makefile > +++ b/Makefile > @@ -948,6 +948,7 @@ ifeq ($(BR2_TARGET_BAREBOX),y) > @echo ' barebox-menuconfig - Run barebox menuconfig' > @echo ' barebox-savedefconfig - Run barebox savedefconfig' > endif > + @for i in $(EXTRA_HELP); do echo " $$i"; done > @echo > @echo 'Documentation:' > @echo ' manual - build manual in all formats' > -- > 2.7.0 > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
Dear Yann E. MORIN, On Tue, 8 Mar 2016 23:10:49 +0100, Yann E. MORIN wrote: > This definitely does not look nice to me... :-( > > What we could do, however, would be: > > - in external.mk: > BR2_HAS_EXTRA_HELP = YES # Whatever non empty > > - in the main Makefile: > > help: > blabla our current help > $(if $(BR2_HAS_EXTRA_HELP),echo " - local-help to get local help blabla) > > which would be IMHO much much nicer and much much simpler... But it doesn't allow PATCH 2/2. Thomas
Thomas, All, On 2016-03-08 23:13 +0100, Thomas Petazzoni spake thusly: > On Tue, 8 Mar 2016 23:10:49 +0100, Yann E. MORIN wrote: > > This definitely does not look nice to me... :-( > > > > What we could do, however, would be: > > > > - in external.mk: > > BR2_HAS_EXTRA_HELP = YES # Whatever non empty > > > > - in the main Makefile: > > > > help: > > blabla our current help > > $(if $(BR2_HAS_EXTRA_HELP),echo " - local-help to get local help blabla) > > > > which would be IMHO much much nicer and much much simpler... > > But it doesn't allow PATCH 2/2. Indeed, it does not, but I'll reply with an alternate proposal... Regards, Yann E. MORIN.
On 03/08/16 23:10, Yann E. MORIN wrote: > Jérôme, All, > > On 2016-03-08 22:48 +0100, Jérôme Pouiller spake thusly: >> It is handy to use local.mk or external.mk to add specific targets >> for current project. However, until now, it not possible to add help >> message these targets. > > I have: > > $ make local-help > Local blabla help displays here... > >> This patch add EXTRA_HELP variable. This variable is aimed to be assigned >> from any .mk files. Its content is displayed with 'make help'. >> >> For exemple: >> EXTRA_HELP += "flash - Flash target" >> EXTRA_HELP += "chroot - Chroot into target/" >> EXTRA_HELP += "qemu - Run image with qemu" >> EXTRA_HELP += "install-nfs - Extract rootfs in \$$NFSROOT (=$(NFSROOT))" >> EXTRA_HELP += "`printf '%-22s%s' '$(var)-feature' ' - Call $(var) feature'`" >> EXTRA_HELP += "Please contact support@company.com in case of problem." > > This definitely does not look nice to me... :-( Why not? Extremely nice and simple. > > What we could do, however, would be: > > - in external.mk: > BR2_HAS_EXTRA_HELP = YES # Whatever non empty > > - in the main Makefile: > > help: > blabla our current help > $(if $(BR2_HAS_EXTRA_HELP),echo " - local-help to get local help blabla) > > which would be IMHO much much nicer and much much simpler... I don't see the advantage of that, to be honest... The point is to append it to the main help. You expect just a few lines of extra help text. Regards, Arnout > > Regards, > Yann E. MORIN. > >> Signed-off-by: Jérôme Pouiller <jezz@sysmic.org> >> --- >> v2: >> - Rename LOCAL_HELP to EXTRA_HELP >> - Remove introduction lines (so, 'ifneq ($(LOCAL_HELP),)' is no more needed) >> >> Makefile | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/Makefile b/Makefile >> index f2822a2..1c9f63c 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -948,6 +948,7 @@ ifeq ($(BR2_TARGET_BAREBOX),y) >> @echo ' barebox-menuconfig - Run barebox menuconfig' >> @echo ' barebox-savedefconfig - Run barebox savedefconfig' >> endif >> + @for i in $(EXTRA_HELP); do echo " $$i"; done >> @echo >> @echo 'Documentation:' >> @echo ' manual - build manual in all formats' >> -- >> 2.7.0 >> >> _______________________________________________ >> buildroot mailing list >> buildroot@busybox.net >> http://lists.busybox.net/mailman/listinfo/buildroot >
Hello Yann, Arnout, All, On Wednesday 09 March 2016 00:19:23 Arnout Vandecappelle wrote: > On 03/08/16 23:10, Yann E. MORIN wrote: [...] > > What we could do, however, would be: > > - in external.mk: > > BR2_HAS_EXTRA_HELP = YES # Whatever non empty > > > > - in the main Makefile: > > help: > > blabla our current help > > $(if $(BR2_HAS_EXTRA_HELP),echo " - local-help to get > > local help blabla)> > > which would be IMHO much much nicer and much much simpler... > > I don't see the advantage of that, to be honest... The point is to > append it to the main help. You expect just a few lines of extra help > text. I agree with Arnout. I add that EXTRA_HELP is generic enough to implement Yann's idea if necessary. Instead of supporting BR2_HAS_EXTRA_HELP, just add in external.mk: EXTRA_HELP = "- local-help to get local help blabla" local-help: @echo "..."
Arnout, Jérôme, All, On 2016-03-09 00:19 +0100, Arnout Vandecappelle spake thusly: > On 03/08/16 23:10, Yann E. MORIN wrote: > >Jérôme, All, > > > >On 2016-03-08 22:48 +0100, Jérôme Pouiller spake thusly: > >>It is handy to use local.mk or external.mk to add specific targets > >>for current project. However, until now, it not possible to add help > >>message these targets. > > > >I have: > > > > $ make local-help > > Local blabla help displays here... > > > >>This patch add EXTRA_HELP variable. This variable is aimed to be assigned > >>from any .mk files. Its content is displayed with 'make help'. > >> > >>For exemple: > >> EXTRA_HELP += "flash - Flash target" > >> EXTRA_HELP += "chroot - Chroot into target/" > >> EXTRA_HELP += "qemu - Run image with qemu" > >> EXTRA_HELP += "install-nfs - Extract rootfs in \$$NFSROOT (=$(NFSROOT))" > >> EXTRA_HELP += "`printf '%-22s%s' '$(var)-feature' ' - Call $(var) feature'`" > >> EXTRA_HELP += "Please contact support@company.com in case of problem." > > > >This definitely does not look nice to me... :-( > > Why not? Extremely nice and simple. It's not nice because: - it assumes a certain layout of the help texts. If/when we re-arrange our own help text, this local help text will no longer be properly indented; - the print stuff above *is* ugly; - the local help is intermixed with our own help text, which I find dubious, as it may lead users to believe those are native buildroot actions when it is not. As for it being simple, it is not simple to write, as the print example clearly demonstrate. With the proposal I made in the second patch, it is far easier to do, as the formatting would be done in a single place. (the printf example would thus be simply: HELP += "$(var)-feature : Call $(var) feature" ) > >What we could do, however, would be: > > > > - in external.mk: > > BR2_HAS_EXTRA_HELP = YES # Whatever non empty > > > > - in the main Makefile: > > > > help: > > blabla our current help > > $(if $(BR2_HAS_EXTRA_HELP),echo " - local-help to get local help blabla) > > > >which would be IMHO much much nicer and much much simpler... > > I don't see the advantage of that, to be honest... The point is to append > it to the main help. You expect just a few lines of extra help text. And the fact that it is mixed with our own help is one thing that I don't like. I would probably agree to the following (accounting for Jérôme's suggestion to reuse the same variable and your own suggestion to change the separator): help: Our complete help goes here ifneq ($(BR2_EXTRA_HELP),) printf "Local, custom help:\n" for h in $(BR2_EXTRA_HELP); do \ printf " %-22.22s-%s\n" "$${h%%* : *}" "${h#* : }"; \ done endif Regards, Yann E. MORIN.
Jérôme, All, On 2016-03-08 22:48 +0100, Jérôme Pouiller spake thusly: > It is handy to use local.mk or external.mk to add specific targets > for current project. However, until now, it not possible to add help > message these targets. Another implementation has been applied: https://git.buildroot.org/buildroot/commit/?id=84c825f8e893bfb56847ab4a880c46066a41744f Consequently, I've marked your patches as rejected in patchwork. Thanks for starting up the discussion! :-) (Note: Thomas applied my implementation by accident, but later decided not to revert it. There are still a few issues, which your proposal also had, and we're fixing them now.) Regards, Yann E. MORIN. > This patch add EXTRA_HELP variable. This variable is aimed to be assigned > from any .mk files. Its content is displayed with 'make help'. > > For exemple: > EXTRA_HELP += "flash - Flash target" > EXTRA_HELP += "chroot - Chroot into target/" > EXTRA_HELP += "qemu - Run image with qemu" > EXTRA_HELP += "install-nfs - Extract rootfs in \$$NFSROOT (=$(NFSROOT))" > EXTRA_HELP += "`printf '%-22s%s' '$(var)-feature' ' - Call $(var) feature'`" > EXTRA_HELP += "Please contact support@company.com in case of problem." > > Signed-off-by: Jérôme Pouiller <jezz@sysmic.org> > --- > v2: > - Rename LOCAL_HELP to EXTRA_HELP > - Remove introduction lines (so, 'ifneq ($(LOCAL_HELP),)' is no more needed) > > Makefile | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Makefile b/Makefile > index f2822a2..1c9f63c 100644 > --- a/Makefile > +++ b/Makefile > @@ -948,6 +948,7 @@ ifeq ($(BR2_TARGET_BAREBOX),y) > @echo ' barebox-menuconfig - Run barebox menuconfig' > @echo ' barebox-savedefconfig - Run barebox savedefconfig' > endif > + @for i in $(EXTRA_HELP); do echo " $$i"; done > @echo > @echo 'Documentation:' > @echo ' manual - build manual in all formats' > -- > 2.7.0 > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
diff --git a/Makefile b/Makefile index f2822a2..1c9f63c 100644 --- a/Makefile +++ b/Makefile @@ -948,6 +948,7 @@ ifeq ($(BR2_TARGET_BAREBOX),y) @echo ' barebox-menuconfig - Run barebox menuconfig' @echo ' barebox-savedefconfig - Run barebox savedefconfig' endif + @for i in $(EXTRA_HELP); do echo " $$i"; done @echo @echo 'Documentation:' @echo ' manual - build manual in all formats'
It is handy to use local.mk or external.mk to add specific targets for current project. However, until now, it not possible to add help message these targets. This patch add EXTRA_HELP variable. This variable is aimed to be assigned from any .mk files. Its content is displayed with 'make help'. For exemple: EXTRA_HELP += "flash - Flash target" EXTRA_HELP += "chroot - Chroot into target/" EXTRA_HELP += "qemu - Run image with qemu" EXTRA_HELP += "install-nfs - Extract rootfs in \$$NFSROOT (=$(NFSROOT))" EXTRA_HELP += "`printf '%-22s%s' '$(var)-feature' ' - Call $(var) feature'`" EXTRA_HELP += "Please contact support@company.com in case of problem." Signed-off-by: Jérôme Pouiller <jezz@sysmic.org> --- v2: - Rename LOCAL_HELP to EXTRA_HELP - Remove introduction lines (so, 'ifneq ($(LOCAL_HELP),)' is no more needed) Makefile | 1 + 1 file changed, 1 insertion(+)