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

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

[Jérôme Pouiller]

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 at 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] [PATCH v2 2/2] help: relocate help messages specific to one package

[Jérôme Pouiller]

Use $EXTRA_HELP feature in order to integrate help messages specific
to one package (linux-menuconfig, etc...) package they are
related.

It would be possible to do it using kconfig framework, but I was not
sure it is necessary to document kconfig targets systematicaly.

Signed-off-by: J?r?me Pouiller <jezz@sysmic.org>
---
v2:
  - Rename LOCAL_HELP

 Makefile                   | 16 ----------------
 boot/barebox/barebox.mk    |  5 +++++
 linux/linux.mk             |  7 +++++++
 package/busybox/busybox.mk |  4 ++++
 package/uclibc/uclibc.mk   |  4 ++++
 5 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/Makefile b/Makefile
index 1c9f63c..eb92052 100644
--- a/Makefile
+++ b/Makefile
@@ -932,22 +932,6 @@ help:
 	@echo '  <pkg>-dirclean         - Remove <pkg> build directory'
 	@echo '  <pkg>-reconfigure      - Restart the build from the configure step'
 	@echo '  <pkg>-rebuild          - Restart the build from the build step'
-ifeq ($(BR2_PACKAGE_BUSYBOX),y)
-	@echo '  busybox-menuconfig     - Run BusyBox menuconfig'
-endif
-ifeq ($(BR2_LINUX_KERNEL),y)
-	@echo '  linux-menuconfig       - Run Linux kernel menuconfig'
-	@echo '  linux-savedefconfig    - Run Linux kernel savedefconfig'
-	@echo '  linux-update-defconfig - Save the Linux configuration to the path specified'
-	@echo '                             by BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE'
-endif
-ifeq ($(BR2_TOOLCHAIN_BUILDROOT),y)
-	@echo '  uclibc-menuconfig      - Run uClibc menuconfig'
-endif
-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:'
diff --git a/boot/barebox/barebox.mk b/boot/barebox/barebox.mk
index 7715daf..24d0162 100644
--- a/boot/barebox/barebox.mk
+++ b/boot/barebox/barebox.mk
@@ -117,4 +117,9 @@ $(error No Barebox config. Check your BR2_TARGET_BAREBOX_BOARD_DEFCONFIG or BR2_
 endif
 endif
 
+ifeq ($(BR2_TARGET_BAREBOX),y)
+	EXTRA_HELP += 'barebox-menuconfig     - Run barebox menuconfig'
+	EXTRA_HELP += 'barebox-savedefconfig  - Run barebox savedefconfig'
+endif
+
 $(eval $(kconfig-package))
diff --git a/linux/linux.mk b/linux/linux.mk
index 7e20255..6cfc8e7 100644
--- a/linux/linux.mk
+++ b/linux/linux.mk
@@ -435,6 +435,13 @@ endif
 
 endif # BR_BUILDING
 
+ifeq ($(BR2_LINUX_KERNEL),y)
+	EXTRA_HELP += 'linux-menuconfig       - Run Linux kernel menuconfig'
+	EXTRA_HELP += 'linux-savedefconfig    - Run Linux kernel savedefconfig'
+	EXTRA_HELP += 'linux-update-defconfig - Save the Linux configuration to the path specified'
+	EXTRA_HELP += '                             by BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE'
+endif
+
 $(eval $(kconfig-package))
 
 # Support for rebuilding the kernel after the cpio archive has
diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
index 7c904c8..d4ee149 100644
--- a/package/busybox/busybox.mk
+++ b/package/busybox/busybox.mk
@@ -250,4 +250,8 @@ $(error No BusyBox configuration file specified, check your BR2_PACKAGE_BUSYBOX_
 endif
 endif
 
+ifeq ($(BR2_PACKAGE_BUSYBOX),y)
+	EXTRA_HELP += 'busybox-menuconfig     - Run BusyBox menuconfig'
+endif
+
 $(eval $(kconfig-package))
diff --git a/package/uclibc/uclibc.mk b/package/uclibc/uclibc.mk
index 200de04..9448364 100644
--- a/package/uclibc/uclibc.mk
+++ b/package/uclibc/uclibc.mk
@@ -460,4 +460,8 @@ $(error No uClibc configuration file specified, check your BR2_UCLIBC_CONFIG set
 endif
 endif
 
+ifeq ($(BR2_PACKAGE_UCLIBC),y)
+	EXTRA_HELP += 'uclibc-menuconfig      - Run uClibc menuconfig'
+endif
+
 $(eval $(kconfig-package))
-- 
2.7.0

[Buildroot] [PATCH v2 2/2] help: relocate help messages specific to one package

[Yann E. MORIN]

J?r?me, All,

On 2016-03-08 22:48 +0100, J?r?me Pouiller spake thusly:
> Use $EXTRA_HELP feature in order to integrate help messages specific
> to one package (linux-menuconfig, etc...) package they are
> related.
> 
> It would be possible to do it using kconfig framework, but I was not
> sure it is necessary to document kconfig targets systematicaly.
> 
> Signed-off-by: J?r?me Pouiller <jezz@sysmic.org>
> ---
> v2:
>   - Rename LOCAL_HELP
> 
>  Makefile                   | 16 ----------------
>  boot/barebox/barebox.mk    |  5 +++++
>  linux/linux.mk             |  7 +++++++
>  package/busybox/busybox.mk |  4 ++++
>  package/uclibc/uclibc.mk   |  4 ++++
>  5 files changed, 20 insertions(+), 16 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 1c9f63c..eb92052 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -932,22 +932,6 @@ help:
>  	@echo '  <pkg>-dirclean         - Remove <pkg> build directory'
>  	@echo '  <pkg>-reconfigure      - Restart the build from the configure step'
>  	@echo '  <pkg>-rebuild          - Restart the build from the build step'
> -ifeq ($(BR2_PACKAGE_BUSYBOX),y)
> -	@echo '  busybox-menuconfig     - Run BusyBox menuconfig'
> -endif
> -ifeq ($(BR2_LINUX_KERNEL),y)
> -	@echo '  linux-menuconfig       - Run Linux kernel menuconfig'
> -	@echo '  linux-savedefconfig    - Run Linux kernel savedefconfig'
> -	@echo '  linux-update-defconfig - Save the Linux configuration to the path specified'
> -	@echo '                             by BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE'
> -endif
> -ifeq ($(BR2_TOOLCHAIN_BUILDROOT),y)
> -	@echo '  uclibc-menuconfig      - Run uClibc menuconfig'
> -endif
> -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

Following my reply to the previous mail, here's an alternate proposal
(it may need a bit of tweaking, though, I jut wrote it in the mail
without testing):

    for h in $(PACKAGE_HELP_y); do \
        printf "  %24.24s - %s\n" "$${h%% *}" "$${h#* }"; \
    done

and see below how it is set...

>  	@echo
>  	@echo 'Documentation:'
> diff --git a/boot/barebox/barebox.mk b/boot/barebox/barebox.mk
> index 7715daf..24d0162 100644
> --- a/boot/barebox/barebox.mk
> +++ b/boot/barebox/barebox.mk
> @@ -117,4 +117,9 @@ $(error No Barebox config. Check your BR2_TARGET_BAREBOX_BOARD_DEFCONFIG or BR2_
>  endif
>  endif
>  
> +ifeq ($(BR2_TARGET_BAREBOX),y)
> +	EXTRA_HELP += 'barebox-menuconfig     - Run barebox menuconfig'
> +	EXTRA_HELP += 'barebox-savedefconfig  - Run barebox savedefconfig'
> +endif

PACKAGE_HELP_$(BR2_TARGET_BAREBOX) += "barebox-menuconfig Run barebox menuconfig"
PACKAGE_HELP_$(BR2_TARGET_BAREBOX) += "barebox-savedefconfig Run barebox savedefconfig"

and so on...

Which IMHO is much more readable, and allows us to nicely change the
layout of the help texts.

Regards,
Yann E. MORIN.

> +
>  $(eval $(kconfig-package))
> diff --git a/linux/linux.mk b/linux/linux.mk
> index 7e20255..6cfc8e7 100644
> --- a/linux/linux.mk
> +++ b/linux/linux.mk
> @@ -435,6 +435,13 @@ endif
>  
>  endif # BR_BUILDING
>  
> +ifeq ($(BR2_LINUX_KERNEL),y)
> +	EXTRA_HELP += 'linux-menuconfig       - Run Linux kernel menuconfig'
> +	EXTRA_HELP += 'linux-savedefconfig    - Run Linux kernel savedefconfig'
> +	EXTRA_HELP += 'linux-update-defconfig - Save the Linux configuration to the path specified'
> +	EXTRA_HELP += '                             by BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE'
> +endif
> +
>  $(eval $(kconfig-package))
>  
>  # Support for rebuilding the kernel after the cpio archive has
> diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
> index 7c904c8..d4ee149 100644
> --- a/package/busybox/busybox.mk
> +++ b/package/busybox/busybox.mk
> @@ -250,4 +250,8 @@ $(error No BusyBox configuration file specified, check your BR2_PACKAGE_BUSYBOX_
>  endif
>  endif
>  
> +ifeq ($(BR2_PACKAGE_BUSYBOX),y)
> +	EXTRA_HELP += 'busybox-menuconfig     - Run BusyBox menuconfig'
> +endif
> +
>  $(eval $(kconfig-package))
> diff --git a/package/uclibc/uclibc.mk b/package/uclibc/uclibc.mk
> index 200de04..9448364 100644
> --- a/package/uclibc/uclibc.mk
> +++ b/package/uclibc/uclibc.mk
> @@ -460,4 +460,8 @@ $(error No uClibc configuration file specified, check your BR2_UCLIBC_CONFIG set
>  endif
>  endif
>  
> +ifeq ($(BR2_PACKAGE_UCLIBC),y)
> +	EXTRA_HELP += 'uclibc-menuconfig      - Run uClibc menuconfig'
> +endif
> +
>  $(eval $(kconfig-package))
> -- 
> 2.7.0
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v2 2/2] help: relocate help messages specific to one package

[Arnout Vandecappelle]

On 03/08/16 23:59, Yann E. MORIN wrote:
> J?r?me, All,
>
> On 2016-03-08 22:48 +0100, J?r?me Pouiller spake thusly:
>> Use $EXTRA_HELP feature in order to integrate help messages specific
>> to one package (linux-menuconfig, etc...) package they are
>> related.
>>
>> It would be possible to do it using kconfig framework, but I was not
>> sure it is necessary to document kconfig targets systematicaly.
>>
>> Signed-off-by: J?r?me Pouiller <jezz@sysmic.org>
>> ---
>> v2:
>>    - Rename LOCAL_HELP
>>
>>   Makefile                   | 16 ----------------
>>   boot/barebox/barebox.mk    |  5 +++++
>>   linux/linux.mk             |  7 +++++++
>>   package/busybox/busybox.mk |  4 ++++
>>   package/uclibc/uclibc.mk   |  4 ++++
>>   5 files changed, 20 insertions(+), 16 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 1c9f63c..eb92052 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -932,22 +932,6 @@ help:
>>   	@echo '  <pkg>-dirclean         - Remove <pkg> build directory'
>>   	@echo '  <pkg>-reconfigure      - Restart the build from the configure step'
>>   	@echo '  <pkg>-rebuild          - Restart the build from the build step'
>> -ifeq ($(BR2_PACKAGE_BUSYBOX),y)
>> -	@echo '  busybox-menuconfig     - Run BusyBox menuconfig'
>> -endif
>> -ifeq ($(BR2_LINUX_KERNEL),y)
>> -	@echo '  linux-menuconfig       - Run Linux kernel menuconfig'
>> -	@echo '  linux-savedefconfig    - Run Linux kernel savedefconfig'
>> -	@echo '  linux-update-defconfig - Save the Linux configuration to the path specified'
>> -	@echo '                             by BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE'
>> -endif
>> -ifeq ($(BR2_TOOLCHAIN_BUILDROOT),y)
>> -	@echo '  uclibc-menuconfig      - Run uClibc menuconfig'
>> -endif
>> -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
>
> Following my reply to the previous mail, here's an alternate proposal
> (it may need a bit of tweaking, though, I jut wrote it in the mail
> without testing):
>
>      for h in $(PACKAGE_HELP_y); do \
>          printf "  %24.24s - %s\n" "$${h%% *}" "$${h#* }"; \
>      done

  Nice, but actually largely independent of this patch. This improvement could 
be done in a follow-up (third) patch. The patch in the current state is mostly 
moving things around.

  I would tend to prefer splitting on something other than space, though. 
<space><colon><space> for instance. So $${h%% : *}.


>
> and see below how it is set...
>
>>   	@echo
>>   	@echo 'Documentation:'
>> diff --git a/boot/barebox/barebox.mk b/boot/barebox/barebox.mk
>> index 7715daf..24d0162 100644
>> --- a/boot/barebox/barebox.mk
>> +++ b/boot/barebox/barebox.mk
>> @@ -117,4 +117,9 @@ $(error No Barebox config. Check your BR2_TARGET_BAREBOX_BOARD_DEFCONFIG or BR2_
>>   endif
>>   endif
>>
>> +ifeq ($(BR2_TARGET_BAREBOX),y)
>> +	EXTRA_HELP += 'barebox-menuconfig     - Run barebox menuconfig'
>> +	EXTRA_HELP += 'barebox-savedefconfig  - Run barebox savedefconfig'
>> +endif
>
> PACKAGE_HELP_$(BR2_TARGET_BAREBOX) += "barebox-menuconfig Run barebox menuconfig"
> PACKAGE_HELP_$(BR2_TARGET_BAREBOX) += "barebox-savedefconfig Run barebox savedefconfig"

  I'm not particularly fond of the _y trick. We have it in a few places, but 
IMHO it doesn't make things more readable or maintainable at all. Only when you 
have a lot of different symbols to check and something relatively short on the 
right hand side, then it looks OK. Like is often the case in the kernel.

  So I would tend to accept these patches as they are now, and have your 
proposal as a follow-up.

  Regards,
  Arnout

>
> and so on...
>
> Which IMHO is much more readable, and allows us to nicely change the
> layout of the help texts.
>
> Regards,
> Yann E. MORIN.
>
>> +
>>   $(eval $(kconfig-package))
>> diff --git a/linux/linux.mk b/linux/linux.mk
>> index 7e20255..6cfc8e7 100644
>> --- a/linux/linux.mk
>> +++ b/linux/linux.mk
>> @@ -435,6 +435,13 @@ endif
>>
>>   endif # BR_BUILDING
>>
>> +ifeq ($(BR2_LINUX_KERNEL),y)
>> +	EXTRA_HELP += 'linux-menuconfig       - Run Linux kernel menuconfig'
>> +	EXTRA_HELP += 'linux-savedefconfig    - Run Linux kernel savedefconfig'
>> +	EXTRA_HELP += 'linux-update-defconfig - Save the Linux configuration to the path specified'
>> +	EXTRA_HELP += '                             by BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE'
>> +endif
>> +
>>   $(eval $(kconfig-package))
>>
>>   # Support for rebuilding the kernel after the cpio archive has
>> diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
>> index 7c904c8..d4ee149 100644
>> --- a/package/busybox/busybox.mk
>> +++ b/package/busybox/busybox.mk
>> @@ -250,4 +250,8 @@ $(error No BusyBox configuration file specified, check your BR2_PACKAGE_BUSYBOX_
>>   endif
>>   endif
>>
>> +ifeq ($(BR2_PACKAGE_BUSYBOX),y)
>> +	EXTRA_HELP += 'busybox-menuconfig     - Run BusyBox menuconfig'
>> +endif
>> +
>>   $(eval $(kconfig-package))
>> diff --git a/package/uclibc/uclibc.mk b/package/uclibc/uclibc.mk
>> index 200de04..9448364 100644
>> --- a/package/uclibc/uclibc.mk
>> +++ b/package/uclibc/uclibc.mk
>> @@ -460,4 +460,8 @@ $(error No uClibc configuration file specified, check your BR2_UCLIBC_CONFIG set
>>   endif
>>   endif
>>
>> +ifeq ($(BR2_PACKAGE_UCLIBC),y)
>> +	EXTRA_HELP += 'uclibc-menuconfig      - Run uClibc menuconfig'
>> +endif
>> +
>>   $(eval $(kconfig-package))
>> --
>> 2.7.0
>>
>> _______________________________________________
>> buildroot mailing list
>> buildroot at busybox.net
>> http://lists.busybox.net/mailman/listinfo/buildroot
>


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v2 2/2] help: relocate help messages specific to one package

[Yann E. MORIN]

Arnout, J?r?me, All,

On 2016-03-09 00:16 +0100, Arnout Vandecappelle spake thusly:
> On 03/08/16 23:59, Yann E. MORIN wrote:
> >J?r?me, All,
> >
> >On 2016-03-08 22:48 +0100, J?r?me Pouiller spake thusly:
> >>Use $EXTRA_HELP feature in order to integrate help messages specific
> >>to one package (linux-menuconfig, etc...) package they are
> >>related.
> >>
> >>It would be possible to do it using kconfig framework, but I was not
> >>sure it is necessary to document kconfig targets systematicaly.
> >>
> >>Signed-off-by: J?r?me Pouiller <jezz@sysmic.org>
> >>---
> >>v2:
> >>   - Rename LOCAL_HELP
> >>
> >>  Makefile                   | 16 ----------------
> >>  boot/barebox/barebox.mk    |  5 +++++
> >>  linux/linux.mk             |  7 +++++++
> >>  package/busybox/busybox.mk |  4 ++++
> >>  package/uclibc/uclibc.mk   |  4 ++++
> >>  5 files changed, 20 insertions(+), 16 deletions(-)
> >>
> >>diff --git a/Makefile b/Makefile
> >>index 1c9f63c..eb92052 100644
> >>--- a/Makefile
> >>+++ b/Makefile
> >>@@ -932,22 +932,6 @@ help:
> >>  	@echo '  <pkg>-dirclean         - Remove <pkg> build directory'
> >>  	@echo '  <pkg>-reconfigure      - Restart the build from the configure step'
> >>  	@echo '  <pkg>-rebuild          - Restart the build from the build step'
> >>-ifeq ($(BR2_PACKAGE_BUSYBOX),y)
> >>-	@echo '  busybox-menuconfig     - Run BusyBox menuconfig'
> >>-endif
> >>-ifeq ($(BR2_LINUX_KERNEL),y)
> >>-	@echo '  linux-menuconfig       - Run Linux kernel menuconfig'
> >>-	@echo '  linux-savedefconfig    - Run Linux kernel savedefconfig'
> >>-	@echo '  linux-update-defconfig - Save the Linux configuration to the path specified'
> >>-	@echo '                             by BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE'
> >>-endif
> >>-ifeq ($(BR2_TOOLCHAIN_BUILDROOT),y)
> >>-	@echo '  uclibc-menuconfig      - Run uClibc menuconfig'
> >>-endif
> >>-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
> >
> >Following my reply to the previous mail, here's an alternate proposal
> >(it may need a bit of tweaking, though, I jut wrote it in the mail
> >without testing):
> >
> >     for h in $(PACKAGE_HELP_y); do \
> >         printf "  %24.24s - %s\n" "$${h%% *}" "$${h#* }"; \
> >     done
> 
>  Nice, but actually largely independent of this patch.

Absolutely not, if you account for my position as explained in answer to
the first patch in the series.

> This improvement
> could be done in a follow-up (third) patch. The patch in the current state
> is mostly moving things around.
> 
>  I would tend to prefer splitting on something other than space, though.
> <space><colon><space> for instance. So $${h%% : *}.

Yes, that's fine with me. ;-)

> >and see below how it is set...
> >
> >>  	@echo
> >>  	@echo 'Documentation:'
> >>diff --git a/boot/barebox/barebox.mk b/boot/barebox/barebox.mk
> >>index 7715daf..24d0162 100644
> >>--- a/boot/barebox/barebox.mk
> >>+++ b/boot/barebox/barebox.mk
> >>@@ -117,4 +117,9 @@ $(error No Barebox config. Check your BR2_TARGET_BAREBOX_BOARD_DEFCONFIG or BR2_
> >>  endif
> >>  endif
> >>
> >>+ifeq ($(BR2_TARGET_BAREBOX),y)
> >>+	EXTRA_HELP += 'barebox-menuconfig     - Run barebox menuconfig'
> >>+	EXTRA_HELP += 'barebox-savedefconfig  - Run barebox savedefconfig'
> >>+endif
> >
> >PACKAGE_HELP_$(BR2_TARGET_BAREBOX) += "barebox-menuconfig Run barebox menuconfig"
> >PACKAGE_HELP_$(BR2_TARGET_BAREBOX) += "barebox-savedefconfig Run barebox savedefconfig"
> 
>  I'm not particularly fond of the _y trick. We have it in a few places, but
> IMHO it doesn't make things more readable or maintainable at all. Only when
> you have a lot of different symbols to check and something relatively short
> on the right hand side, then it looks OK. Like is often the case in the
> kernel.

I'm OK with using the ifneq conditional too, even though I prefer the _y
"trick" since it is pretty well used in many places (in Buildroot, but
in the kernel too for example).

>  So I would tend to accept these patches as they are now, and have your
> proposal as a follow-up.

Sorry, I really do not like the first patch and what it implies.

But code speaks better than words, so I'll hack something now and submit
so we can compare objectivly, and so the powers-that-be may decide. ;-)

Thanks! :-)

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

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

[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.

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 at 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 at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

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

[Thomas Petazzoni]

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 Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

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

[Yann E. MORIN]

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.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

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

[Arnout Vandecappelle]

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 at 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 at busybox.net
>> http://lists.busybox.net/mailman/listinfo/buildroot
>


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

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

[Jérôme Pouiller]

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 "..."




-- 
J?r?me Pouiller, Sysmic
Embedded Linux specialist
http://www.sysmic.fr

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

[Yann E. MORIN]

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 at 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.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

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

[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 at 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 at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'