diff mbox

[1/2] help: add a way to document targets declared in local.mk/external.mk

Message ID 1447857421-9380-1-git-send-email-jezz@sysmic.org
State Superseded
Headers show

Commit Message

Jérôme Pouiller Nov. 18, 2015, 2:37 p.m. UTC
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 LOCAL_HELP variable. This variable is aimed to be assigned
from any .mk files. Its content is displayed with 'make help'.

For exemple:
  LOCAL_HELP += "flash                  - Flash target"
  LOCAL_HELP += "chroot                 - Chroot into target/"
  LOCAL_HELP += "qemu                   - Run image with qemu"
  LOCAL_HELP += "install-nfs            - Extract rootfs in \$$NFSROOT (=$(NFSROOT))"
  LOCAL_HELP += "`printf '%-22s%s' '$(var)-feature' ' - Call $(var) feature'`"
  LOCAL_HELP += "Please contact support@company.com in case of problem."

Signed-off-by: Jérôme Pouiller <jezz@sysmic.org>
---
 Makefile | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Yann E. MORIN Nov. 29, 2015, 8:26 p.m. UTC | #1
Jérôme, All,

On 2015-11-18 15:37 +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.
> This patch add LOCAL_HELP variable. This variable is aimed to be assigned
> from any .mk files. Its content is displayed with 'make help'.
> 
> For exemple:
>   LOCAL_HELP += "flash                  - Flash target"
>   LOCAL_HELP += "chroot                 - Chroot into target/"
>   LOCAL_HELP += "qemu                   - Run image with qemu"
>   LOCAL_HELP += "install-nfs            - Extract rootfs in \$$NFSROOT (=$(NFSROOT))"
>   LOCAL_HELP += "`printf '%-22s%s' '$(var)-feature' ' - Call $(var) feature'`"
>   LOCAL_HELP += "Please contact support@company.com in case of problem."
> 
> Signed-off-by: Jérôme Pouiller <jezz@sysmic.org>
> ---
>  Makefile | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 80c264f..4322da9 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -901,6 +901,11 @@ ifeq ($(BR2_TARGET_BAREBOX),y)
>  	@echo '  barebox-menuconfig     - Run barebox menuconfig'
>  	@echo '  barebox-savedefconfig  - Run barebox savedefconfig'
>  endif
> +ifneq ($(LOCAL_HELP),)
> +	@echo
> +	@echo 'Local targets:'
> +	@for i in $(LOCAL_HELP); do echo "  $$i"; done
> +endif

Well, for help from extenal.mk (or local.mk, but I'd arue that would be
a bad idea, given that local.mk is supposedly short-lived), there is in
my opinion a much better solution.

Change the 'help' rule to a double-colon make rule, like so:

    help::
        echo Current Buildroot help

Then you can add as many such rules in as many places you want,
especially in external.mk, and the will be appended one after the
others.

We just have to ensure that our help comes before the external ones, so
maybe you'll have to move our help block before inclusion of
external.mk...

Regards,
Yann E. MORIN.

>  	@echo
>  	@echo 'Documentation:'
>  	@echo '  manual                 - build manual in all formats'
> -- 
> 2.1.4
> 
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
Jérôme Pouiller Nov. 30, 2015, 12:04 p.m. UTC | #2
Hello Yann,

On Sunday 29 November 2015 21:26:50 Yann E. MORIN wrote:
> Jérôme, All,
> 
> On 2015-11-18 15:37 +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.
> > This patch add LOCAL_HELP variable. This variable is aimed to be
> > assigned from any .mk files. Its content is displayed with 'make
> > help'.> 
> > For exemple:
> >   LOCAL_HELP += "flash                  - Flash target"
> >   LOCAL_HELP += "chroot                 - Chroot into target/"
> >   LOCAL_HELP += "qemu                   - Run image with qemu"
> >   LOCAL_HELP += "install-nfs            - Extract rootfs in
> >   \$$NFSROOT (=$(NFSROOT))" LOCAL_HELP += "`printf '%-22s%s'
> >   '$(var)-feature' ' - Call $(var) feature'`" LOCAL_HELP += "Please
> >   contact support@company.com in case of problem."> 
> > Signed-off-by: Jérôme Pouiller <jezz@sysmic.org>
> > ---
> > 
> >  Makefile | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/Makefile b/Makefile
> > index 80c264f..4322da9 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -901,6 +901,11 @@ ifeq ($(BR2_TARGET_BAREBOX),y)
> > 
> >  	@echo '  barebox-menuconfig     - Run barebox menuconfig'
> >  	@echo '  barebox-savedefconfig  - Run barebox savedefconfig'
> >  
> >  endif
> > 
> > +ifneq ($(LOCAL_HELP),)
> > +	@echo
> > +	@echo 'Local targets:'
> > +	@for i in $(LOCAL_HELP); do echo "  $$i"; done
> > +endif
> 
> Well, for help from extenal.mk (or local.mk, but I'd arue that would
> be a bad idea, given that local.mk is supposedly short-lived), there
> is in my opinion a much better solution.
> 
> Change the 'help' rule to a double-colon make rule, like so:
> 
>     help::
>         echo Current Buildroot help
> 
> Then you can add as many such rules in as many places you want,
> especially in external.mk, and the will be appended one after the
> others.
Nice trick.


> We just have to ensure that our help comes before the external ones,
> so maybe you'll have to move our help block before inclusion of
> external.mk...

I have a slight preference for my proposal since I think it allows a 
finer control of the way extra help is handled. However, I have no 
problem to send a new patch using your trick.

Regards,
Arnout Vandecappelle Nov. 30, 2015, 10:22 p.m. UTC | #3
On 30-11-15 13:04, Jérôme Pouiller wrote:
> Hello Yann,
> 
> On Sunday 29 November 2015 21:26:50 Yann E. MORIN wrote:
>> Jérôme, All,
>>
>> On 2015-11-18 15:37 +0100, Jérôme Pouiller spake thusly:
[snip]
>> Well, for help from extenal.mk (or local.mk, but I'd arue that would
>> be a bad idea, given that local.mk is supposedly short-lived), there
>> is in my opinion a much better solution.
>>
>> Change the 'help' rule to a double-colon make rule, like so:
>>
>>     help::
>>         echo Current Buildroot help
>>
>> Then you can add as many such rules in as many places you want,
>> especially in external.mk, and the will be appended one after the
>> others.
>
> Nice trick.

 IMHO tricks (nice or not) are no good because they make things more difficult
to understand. Also, we don't currently use the :: construct so I'd prefer to
avoid introducing it.


>> We just have to ensure that our help comes before the external ones,
>> so maybe you'll have to move our help block before inclusion of
>> external.mk...
> 
> I have a slight preference for my proposal since I think it allows a 
> finer control of the way extra help is handled. However, I have no 
> problem to send a new patch using your trick.

 Because of what I wrote above, I also prefer your proposal.


 Regards,
 Arnout

> 
> Regards,
>
Arnout Vandecappelle March 8, 2016, 8:54 p.m. UTC | #4
On 11/18/15 15:37, Jérôme Pouiller wrote:
> 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 LOCAL_HELP variable. This variable is aimed to be assigned
> from any .mk files. Its content is displayed with 'make help'.
>
> For exemple:
>    LOCAL_HELP += "flash                  - Flash target"
>    LOCAL_HELP += "chroot                 - Chroot into target/"
>    LOCAL_HELP += "qemu                   - Run image with qemu"
>    LOCAL_HELP += "install-nfs            - Extract rootfs in \$$NFSROOT (=$(NFSROOT))"
>    LOCAL_HELP += "`printf '%-22s%s' '$(var)-feature' ' - Call $(var) feature'`"
>    LOCAL_HELP += "Please contact support@company.com in case of problem."
>
> Signed-off-by: Jérôme Pouiller <jezz@sysmic.org>

  Looks good to me, except that I don't like the name 'LOCAL'. I'd prefer e.g. 
ADDITIONAL.

> ---
>   Makefile | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index 80c264f..4322da9 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -901,6 +901,11 @@ ifeq ($(BR2_TARGET_BAREBOX),y)
>   	@echo '  barebox-menuconfig     - Run barebox menuconfig'
>   	@echo '  barebox-savedefconfig  - Run barebox savedefconfig'
>   endif
> +ifneq ($(LOCAL_HELP),)
> +	@echo
> +	@echo 'Local targets:'

  And this bit is not needed for me (neither the empty line).

  But with that:

  Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>


  Regards,
  Arnout

> +	@for i in $(LOCAL_HELP); do echo "  $$i"; done
> +endif
>   	@echo
>   	@echo 'Documentation:'
>   	@echo '  manual                 - build manual in all formats'
>
Thomas Petazzoni March 8, 2016, 9:02 p.m. UTC | #5
Hello,

On Tue, 8 Mar 2016 21:54:55 +0100, Arnout Vandecappelle wrote:
> On 11/18/15 15:37, Jérôme Pouiller wrote:
> > 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 LOCAL_HELP variable. This variable is aimed to be assigned
> > from any .mk files. Its content is displayed with 'make help'.
> >
> > For exemple:
> >    LOCAL_HELP += "flash                  - Flash target"
> >    LOCAL_HELP += "chroot                 - Chroot into target/"
> >    LOCAL_HELP += "qemu                   - Run image with qemu"
> >    LOCAL_HELP += "install-nfs            - Extract rootfs in \$$NFSROOT (=$(NFSROOT))"
> >    LOCAL_HELP += "`printf '%-22s%s' '$(var)-feature' ' - Call $(var) feature'`"
> >    LOCAL_HELP += "Please contact support@company.com in case of problem."
> >
> > Signed-off-by: Jérôme Pouiller <jezz@sysmic.org>
> 
>   Looks good to me, except that I don't like the name 'LOCAL'. I'd prefer e.g. 
> ADDITIONAL.

Wouldn't EXTERNAL_HELP be even better, suggesting that its main
use-case is for BR2_EXTERNAL ?

Thomas
Thomas Petazzoni March 8, 2016, 9:19 p.m. UTC | #6
Hello,

On Tue, 8 Mar 2016 22:02:21 +0100, Thomas Petazzoni wrote:

> >   Looks good to me, except that I don't like the name 'LOCAL'. I'd prefer e.g. 
> > ADDITIONAL.
> 
> Wouldn't EXTERNAL_HELP be even better, suggesting that its main
> use-case is for BR2_EXTERNAL ?

My suggestion obviously doesn't work if we want to use this for
internal packages, as PATCH 2/2 is doing.

Best regards,

Thomas
Arnout Vandecappelle March 8, 2016, 9:21 p.m. UTC | #7
On 03/08/16 22:02, Thomas Petazzoni wrote:
> Hello,
>
> On Tue, 8 Mar 2016 21:54:55 +0100, Arnout Vandecappelle wrote:
>> On 11/18/15 15:37, Jérôme Pouiller wrote:
>>> 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 LOCAL_HELP variable. This variable is aimed to be assigned
>>> from any .mk files. Its content is displayed with 'make help'.
>>>
>>> For exemple:
>>>     LOCAL_HELP += "flash                  - Flash target"
>>>     LOCAL_HELP += "chroot                 - Chroot into target/"
>>>     LOCAL_HELP += "qemu                   - Run image with qemu"
>>>     LOCAL_HELP += "install-nfs            - Extract rootfs in \$$NFSROOT (=$(NFSROOT))"
>>>     LOCAL_HELP += "`printf '%-22s%s' '$(var)-feature' ' - Call $(var) feature'`"
>>>     LOCAL_HELP += "Please contact support@company.com in case of problem."
>>>
>>> Signed-off-by: Jérôme Pouiller <jezz@sysmic.org>
>>
>>    Looks good to me, except that I don't like the name 'LOCAL'. I'd prefer e.g.
>> ADDITIONAL.
>
> Wouldn't EXTERNAL_HELP be even better, suggesting that its main
> use-case is for BR2_EXTERNAL ?

  Works for me as well. Additional is a word we're currently not using at all, 
so perhaps reusing external is better.


  Regards,
  Arnout
diff mbox

Patch

diff --git a/Makefile b/Makefile
index 80c264f..4322da9 100644
--- a/Makefile
+++ b/Makefile
@@ -901,6 +901,11 @@  ifeq ($(BR2_TARGET_BAREBOX),y)
 	@echo '  barebox-menuconfig     - Run barebox menuconfig'
 	@echo '  barebox-savedefconfig  - Run barebox savedefconfig'
 endif
+ifneq ($(LOCAL_HELP),)
+	@echo
+	@echo 'Local targets:'
+	@for i in $(LOCAL_HELP); do echo "  $$i"; done
+endif
 	@echo
 	@echo 'Documentation:'
 	@echo '  manual                 - build manual in all formats'