* [Buildroot] [PATCH v2 1/2] help: add a way to document targets declared in local.mk/external.mk
@ 2016-03-08 21:48 Jérôme Pouiller
2016-03-08 21:48 ` [Buildroot] [PATCH v2 2/2] help: relocate help messages specific to one package Jérôme Pouiller
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Jérôme Pouiller @ 2016-03-08 21:48 UTC (permalink / raw)
To: buildroot
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Buildroot] [PATCH v2 2/2] help: relocate help messages specific to one package
2016-03-08 21:48 [Buildroot] [PATCH v2 1/2] help: add a way to document targets declared in local.mk/external.mk Jérôme Pouiller
@ 2016-03-08 21:48 ` Jérôme Pouiller
2016-03-08 22:59 ` Yann E. MORIN
2016-03-08 22:10 ` [Buildroot] [PATCH v2 1/2] help: add a way to document targets declared in local.mk/external.mk Yann E. MORIN
2016-03-20 18:28 ` Yann E. MORIN
2 siblings, 1 reply; 12+ messages in thread
From: Jérôme Pouiller @ 2016-03-08 21:48 UTC (permalink / raw)
To: buildroot
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Buildroot] [PATCH v2 1/2] help: add a way to document targets declared in local.mk/external.mk
2016-03-08 21:48 [Buildroot] [PATCH v2 1/2] help: add a way to document targets declared in local.mk/external.mk Jérôme Pouiller
2016-03-08 21:48 ` [Buildroot] [PATCH v2 2/2] help: relocate help messages specific to one package Jérôme Pouiller
@ 2016-03-08 22:10 ` Yann E. MORIN
2016-03-08 22:13 ` Thomas Petazzoni
2016-03-08 23:19 ` Arnout Vandecappelle
2016-03-20 18:28 ` Yann E. MORIN
2 siblings, 2 replies; 12+ messages in thread
From: Yann E. MORIN @ 2016-03-08 22:10 UTC (permalink / raw)
To: buildroot
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. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Buildroot] [PATCH v2 1/2] help: add a way to document targets declared in local.mk/external.mk
2016-03-08 22:10 ` [Buildroot] [PATCH v2 1/2] help: add a way to document targets declared in local.mk/external.mk Yann E. MORIN
@ 2016-03-08 22:13 ` Thomas Petazzoni
2016-03-08 22:31 ` Yann E. MORIN
2016-03-08 23:19 ` Arnout Vandecappelle
1 sibling, 1 reply; 12+ messages in thread
From: Thomas Petazzoni @ 2016-03-08 22:13 UTC (permalink / raw)
To: 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 Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Buildroot] [PATCH v2 1/2] help: add a way to document targets declared in local.mk/external.mk
2016-03-08 22:13 ` Thomas Petazzoni
@ 2016-03-08 22:31 ` Yann E. MORIN
0 siblings, 0 replies; 12+ messages in thread
From: Yann E. MORIN @ 2016-03-08 22:31 UTC (permalink / raw)
To: buildroot
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. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Buildroot] [PATCH v2 2/2] help: relocate help messages specific to one package
2016-03-08 21:48 ` [Buildroot] [PATCH v2 2/2] help: relocate help messages specific to one package Jérôme Pouiller
@ 2016-03-08 22:59 ` Yann E. MORIN
2016-03-08 23:16 ` Arnout Vandecappelle
0 siblings, 1 reply; 12+ messages in thread
From: Yann E. MORIN @ 2016-03-08 22:59 UTC (permalink / raw)
To: buildroot
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. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Buildroot] [PATCH v2 2/2] help: relocate help messages specific to one package
2016-03-08 22:59 ` Yann E. MORIN
@ 2016-03-08 23:16 ` Arnout Vandecappelle
2016-03-09 18:01 ` Yann E. MORIN
0 siblings, 1 reply; 12+ messages in thread
From: Arnout Vandecappelle @ 2016-03-08 23:16 UTC (permalink / raw)
To: buildroot
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Buildroot] [PATCH v2 1/2] help: add a way to document targets declared in local.mk/external.mk
2016-03-08 22:10 ` [Buildroot] [PATCH v2 1/2] help: add a way to document targets declared in local.mk/external.mk Yann E. MORIN
2016-03-08 22:13 ` Thomas Petazzoni
@ 2016-03-08 23:19 ` Arnout Vandecappelle
2016-03-09 8:27 ` Jérôme Pouiller
2016-03-09 17:57 ` Yann E. MORIN
1 sibling, 2 replies; 12+ messages in thread
From: Arnout Vandecappelle @ 2016-03-08 23:19 UTC (permalink / raw)
To: buildroot
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Buildroot] [PATCH v2 1/2] help: add a way to document targets declared in local.mk/external.mk
2016-03-08 23:19 ` Arnout Vandecappelle
@ 2016-03-09 8:27 ` Jérôme Pouiller
2016-03-09 17:57 ` Yann E. MORIN
1 sibling, 0 replies; 12+ messages in thread
From: Jérôme Pouiller @ 2016-03-09 8:27 UTC (permalink / raw)
To: 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 "..."
--
J?r?me Pouiller, Sysmic
Embedded Linux specialist
http://www.sysmic.fr
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Buildroot] [PATCH v2 1/2] help: add a way to document targets declared in local.mk/external.mk
2016-03-08 23:19 ` Arnout Vandecappelle
2016-03-09 8:27 ` Jérôme Pouiller
@ 2016-03-09 17:57 ` Yann E. MORIN
1 sibling, 0 replies; 12+ messages in thread
From: Yann E. MORIN @ 2016-03-09 17:57 UTC (permalink / raw)
To: buildroot
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. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Buildroot] [PATCH v2 2/2] help: relocate help messages specific to one package
2016-03-08 23:16 ` Arnout Vandecappelle
@ 2016-03-09 18:01 ` Yann E. MORIN
0 siblings, 0 replies; 12+ messages in thread
From: Yann E. MORIN @ 2016-03-09 18:01 UTC (permalink / raw)
To: buildroot
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. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Buildroot] [PATCH v2 1/2] help: add a way to document targets declared in local.mk/external.mk
2016-03-08 21:48 [Buildroot] [PATCH v2 1/2] help: add a way to document targets declared in local.mk/external.mk Jérôme Pouiller
2016-03-08 21:48 ` [Buildroot] [PATCH v2 2/2] help: relocate help messages specific to one package Jérôme Pouiller
2016-03-08 22:10 ` [Buildroot] [PATCH v2 1/2] help: add a way to document targets declared in local.mk/external.mk Yann E. MORIN
@ 2016-03-20 18:28 ` Yann E. MORIN
2 siblings, 0 replies; 12+ messages in thread
From: Yann E. MORIN @ 2016-03-20 18:28 UTC (permalink / raw)
To: buildroot
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. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-03-20 18:28 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-08 21:48 [Buildroot] [PATCH v2 1/2] help: add a way to document targets declared in local.mk/external.mk Jérôme Pouiller
2016-03-08 21:48 ` [Buildroot] [PATCH v2 2/2] help: relocate help messages specific to one package Jérôme Pouiller
2016-03-08 22:59 ` Yann E. MORIN
2016-03-08 23:16 ` Arnout Vandecappelle
2016-03-09 18:01 ` Yann E. MORIN
2016-03-08 22:10 ` [Buildroot] [PATCH v2 1/2] help: add a way to document targets declared in local.mk/external.mk Yann E. MORIN
2016-03-08 22:13 ` Thomas Petazzoni
2016-03-08 22:31 ` Yann E. MORIN
2016-03-08 23:19 ` Arnout Vandecappelle
2016-03-09 8:27 ` Jérôme Pouiller
2016-03-09 17:57 ` Yann E. MORIN
2016-03-20 18:28 ` Yann E. MORIN
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.