Skip to contentSkip to conversation

help: add a way to document targets declared in local.mk/external.mk

PATCH New Jérôme Pouiller 4 participants 12 messages Mar 8, 2016 – Mar 20, 2016 Kbuild

lore
Share & tools
lore

Jérôme Pouiller

author
#0 lore raw

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

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 <[email protected]>

---
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(+)
Makefile +1 -0
index f2822a2..1c9f63c 100644
@@ -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'
Signature · 1 line
2.7.0
author
#1 lore raw

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

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 <[email protected]>

---
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(-)
Makefile +0 -16
index 1c9f63c..eb92052 100644
@@ -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:'
boot/barebox/barebox.mk +5 -0
index 7715daf..24d0162 100644
@@ -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))
linux/linux.mk +7 -0
index 7e20255..6cfc8e7 100644
@@ -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
package/busybox/busybox.mk +4 -0
index 7c904c8..d4ee149 100644
@@ -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))
package/uclibc/uclibc.mk +4 -0
index 200de04..9448364 100644
@@ -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))
Signature · 1 line
2.7.0
#2 lore raw

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

J?r?me, All,

On 2016-03-08 22:48 +0100, J?r?me Pouiller spake thusly:

Quoted 34 lines

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 <[email protected]>

---
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(-)
Makefile +0 -7
index 1c9f63c..eb92052 100644
@@ -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'

Makefile help: · line 935

Quoted 10 lines
Makefile +0 -9
-	@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...

Quoted 7 lines
Makefile +0 -0
 	@echo
 	@echo 'Documentation:'
boot/barebox/barebox.mk +0 -0
index 7715daf..24d0162 100644

boot/barebox/barebox.mk $(error No Barebox config. Check your BR2_TARGET_BAREBOX_BOARD_DEFCONFIG or BR2_ · line 123

Quoted 7 lines
boot/barebox/barebox.mk +4 -0
@@ -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.

Quoted 53 lines
boot/barebox/barebox.mk +1 -0
+
 $(eval $(kconfig-package))
linux/linux.mk +7 -0
index 7e20255..6cfc8e7 100644
@@ -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
package/busybox/busybox.mk +4 -0
index 7c904c8..d4ee149 100644
@@ -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))
package/uclibc/uclibc.mk +4 -0
index 200de04..9448364 100644
@@ -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))
Signature · 6 lines
2.7.0

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
#3 lore raw

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

Quoted 52 lines · Yann E. MORIN

J?r?me, All,

On 2016-03-08 22:48 +0100, J?r?me Pouiller spake thusly:

Quoted 34 lines

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 <[email protected]>

---
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(-)
Makefile +0 -7
index 1c9f63c..eb92052 100644
@@ -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'

Makefile help: · line 935

Quoted 10 lines
Makefile +0 -9
-	@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):

Quoted 3 lines
     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%% : *}.

Quoted 13 lines

and see below how it is set...

Quoted 10 lines
Makefile +0 -0
  	@echo
  	@echo 'Documentation:'
boot/barebox/barebox.mk +0 -0
index 7715daf..24d0162 100644
@@ -117,4 +117,9 @@ $(error No Barebox config. Check your BR2_TARGET_BAREBOX_BOARD_DEFCONFIG or BR2_
  endif
  endif
Quoted 7 lines

boot/barebox/barebox.mk $(error No Barebox config. Check your BR2_TARGET_BAREBOX_BOARD_DEFCONFIG or BR2_ · line 123

Quoted 4 lines
boot/barebox/barebox.mk +4 -0
+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

Quoted 63 lines

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.

Quoted 53 lines
boot/barebox/barebox.mk +1 -0
+
  $(eval $(kconfig-package))
linux/linux.mk +7 -0
index 7e20255..6cfc8e7 100644
@@ -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
package/busybox/busybox.mk +4 -0
index 7c904c8..d4ee149 100644
@@ -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))
package/uclibc/uclibc.mk +4 -1
index 200de04..9448364 100644
@@ -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

Signature · 6 lines
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
#4 lore raw

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

Arnout, J?r?me, All,

On 2016-03-09 00:16 +0100, Arnout Vandecappelle spake thusly:

Quoted 53 lines · Yann E. MORIN
Quoted 52 lines

J?r?me, All,

On 2016-03-08 22:48 +0100, J?r?me Pouiller spake thusly:

Quoted 34 lines

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 <[email protected]>

---
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(-)
Makefile +0 -7
index 1c9f63c..eb92052 100644
@@ -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'

Makefile help: · line 935

Quoted 10 lines
Makefile +0 -9
-	@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):

Quoted 5 lines
Quoted 3 lines
    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.

Quoted 6 lines

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. ;-)

Quoted 20 lines
Quoted 12 lines

and see below how it is set...

Quoted 10 lines
Makefile +0 -0
 	@echo
 	@echo 'Documentation:'
boot/barebox/barebox.mk +0 -0
index 7715daf..24d0162 100644
@@ -117,4 +117,9 @@ $(error No Barebox config. Check your BR2_TARGET_BAREBOX_BOARD_DEFCONFIG or BR2_
 endif
 endif
Quoted 7 lines

boot/barebox/barebox.mk $(error No Barebox config. Check your BR2_TARGET_BAREBOX_BOARD_DEFCONFIG or BR2_ · line 123

Quoted 4 lines
boot/barebox/barebox.mk +4 -0
+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"

Quoted 5 lines

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

Quoted 2 lines

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.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
#5 lore raw

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

In reply to Jérôme Pouiller

J?r?me, All,

On 2016-03-08 22:48 +0100, J?r?me Pouiller spake thusly:

Quoted 3 lines

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...
Quoted 10 lines

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.

Quoted 28 lines
Signed-off-by J?r?me Pouiller <[email protected]>

---
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(+)
Makefile +1 -0
index f2822a2..1c9f63c 100644
@@ -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'
Signature · 6 lines
2.7.0

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
#6 lore raw

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

Dear Yann E. MORIN,

Quoted 7 lines · Yann E. MORIN

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

Quoted 7 lines

- 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

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#7 lore raw

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

Thomas, All,

On 2016-03-08 23:13 +0100, Thomas Petazzoni spake thusly:

Quoted 10 lines · Yann E. MORIN
Quoted 9 lines

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:

Quoted 7 lines
Quoted 5 lines

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.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
#8 lore raw

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

In reply to Yann E. MORIN
Quoted 23 lines · Yann E. MORIN

J?r?me, All,

On 2016-03-08 22:48 +0100, J?r?me Pouiller spake thusly:

Quoted 3 lines

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...
Quoted 10 lines

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

Quoted 1 line

This definitely does not look nice to me... :-(

Why not? Extremely nice and simple.

Quoted 6 lines

What we could do, however, would be:

- in external.mk:
BR2_HAS_EXTRA_HELP = YES # Whatever non empty

Quoted 7 lines

- 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

Quoted 33 lines

Regards,
Yann E. MORIN.

Quoted 28 lines
Signed-off-by J?r?me Pouiller <[email protected]>

---
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(+)
Makefile +1 -1
index f2822a2..1c9f63c 100644
@@ -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

Signature · 6 lines
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
author
#9 lore raw

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

Hello Yann, Arnout, All,

Quoted 1 line · On Wednesday 09 March 2016 00:19:23 Arnout Vandecappelle wrote:

On 03/08/16 23:10, Yann E. MORIN wrote:

[...]

Quoted 11 lines
Quoted 10 lines

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

Quoted 3 lines

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

Signature · 3 lines
J?r?me Pouiller, Sysmic
Embedded Linux specialist
http://www.sysmic.fr
#10 lore raw

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

In reply to Arnout Vandecappelle

Arnout, J?r?me, All,

On 2016-03-09 00:19 +0100, Arnout Vandecappelle spake thusly:

Quoted 24 lines · Yann E. MORIN
Quoted 23 lines

J?r?me, All,

On 2016-03-08 22:48 +0100, J?r?me Pouiller spake thusly:

Quoted 3 lines

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...
Quoted 10 lines

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

Quoted 3 lines
Quoted 1 line

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"

)

Quoted 7 lines
Quoted 7 lines

What we could do, however, would be:

- in external.mk:
BR2_HAS_EXTRA_HELP = YES # Whatever non empty

- in the main Makefile:

Quoted 8 lines
Quoted 5 lines

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.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
#11 lore raw

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

In reply to Jérôme Pouiller

J?r?me, All,

On 2016-03-08 22:48 +0100, J?r?me Pouiller spake thusly:

Quoted 3 lines

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.

Quoted 39 lines

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 <[email protected]>

---
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(+)
Makefile +1 -0
index f2822a2..1c9f63c 100644
@@ -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'
Signature · 6 lines
2.7.0

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'