[Buildroot] [PATCH] core/help: fix custom help without a .config

[Buildroot] [PATCH] core/help: fix custom help without a .config

[Yann E. MORIN]

When there is no .config, we do not source $(BR2_EXTERNAL)/external.mk.
So we can not expose the custom help in that situation.

It is now known whether sourcing $(BR2_EXTERNAL)/external.mk outside the
HAVE_DOT_CONFIG conditional block is entirely safe. Even if it would
work today, we would have a hard time not breaking it in the future,
because we do not have automatic checks for that and would need to rely
on users reporting issues after the fact.

Instead, we require the custom help to be defined in its own file in the
br2-external tree. This way, we can safely include it unconditionally.

Reported-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 Makefile                             | 11 +++++++----
 docs/manual/customize-outside-br.txt |  7 ++++---
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index ea8b1e4..03657d5 100644
--- a/Makefile
+++ b/Makefile
@@ -976,10 +976,13 @@ endif
 	@echo 'it on-line at http://buildroot.org/docs.html'
 	@echo
 
-# This rule does nothing, it is expected to be overloaded by
-# a br2-external tree or a local.mk . However, it must exist,
-# as we reference it in the main help, above. Making the rule
-# .PHONY does not work.
+ifneq ($(wildcard $(BR2_EXTERNAL)/help.mk),)
+include $(BR2_EXTERNAL)/help.mk
+endif
+
+# This rule does nothing, it is expected to be overloaded by a
+# br2-external tree. However, it must exist, as we reference it
+# as a dependency of the main help, above.
 help-custom:
 
 list-defconfigs:
diff --git a/docs/manual/customize-outside-br.txt b/docs/manual/customize-outside-br.txt
index be1827e..4f1c752 100644
--- a/docs/manual/customize-outside-br.txt
+++ b/docs/manual/customize-outside-br.txt
@@ -108,9 +108,10 @@ And then in +$(BR2_EXTERNAL)/package/package1+ and
    normal +make <name>_defconfig+ command. They will be visible under the
    +User-provided configs+' label in the 'make list-defconfigs' output.
 
-Additionally, an +external.mk+ file may define the +help-custom+ make
-rule, to document custom make targets specific to this +BR2_EXTERNAL+
-tree. The help is completely free-form.
+Additionally, a +BR2_EXTERNAL+ tree may provide a file named +help.mk+
+that defines the +help-custom+ make rule, to document custom make
+targets specific to this +BR2_EXTERNAL+ tree. The help is completely
+free-form. See below for a sample +$(BR2_EXTERNAL)/help.mk+ file:
 
 ------
 help-custom:
-- 
1.9.1

[Buildroot] [PATCH] core/help: fix custom help without a .config

[Arnout Vandecappelle]

On 03/20/16 22:24, Yann E. MORIN wrote:
> When there is no .config, we do not source $(BR2_EXTERNAL)/external.mk.
> So we can not expose the custom help in that situation.
>
> It is now known whether sourcing $(BR2_EXTERNAL)/external.mk outside the
> HAVE_DOT_CONFIG conditional block is entirely safe. Even if it would
> work today, we would have a hard time not breaking it in the future,
> because we do not have automatic checks for that and would need to rely
> on users reporting issues after the fact.
>
> Instead, we require the custom help to be defined in its own file in the
> br2-external tree. This way, we can safely include it unconditionally.

  IMHO that custom help was a bad idea. It's adding complexity, an extra file in 
BR2_EXTERNAL, and really not that useful...

>
> Reported-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>   Makefile                             | 11 +++++++----
>   docs/manual/customize-outside-br.txt |  7 ++++---
>   2 files changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index ea8b1e4..03657d5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -976,10 +976,13 @@ endif
>   	@echo 'it on-line at http://buildroot.org/docs.html'
>   	@echo
>
> -# This rule does nothing, it is expected to be overloaded by
> -# a br2-external tree or a local.mk . However, it must exist,
> -# as we reference it in the main help, above. Making the rule
> -# .PHONY does not work.
> +ifneq ($(wildcard $(BR2_EXTERNAL)/help.mk),)
> +include $(BR2_EXTERNAL)/help.mk

  Why not just -include?

  Regards,
  Arnout

> +endif
> +
> +# This rule does nothing, it is expected to be overloaded by a
> +# br2-external tree. However, it must exist, as we reference it
> +# as a dependency of the main help, above.
>   help-custom:
>
>   list-defconfigs:
> diff --git a/docs/manual/customize-outside-br.txt b/docs/manual/customize-outside-br.txt
> index be1827e..4f1c752 100644
> --- a/docs/manual/customize-outside-br.txt
> +++ b/docs/manual/customize-outside-br.txt
> @@ -108,9 +108,10 @@ And then in +$(BR2_EXTERNAL)/package/package1+ and
>      normal +make <name>_defconfig+ command. They will be visible under the
>      +User-provided configs+' label in the 'make list-defconfigs' output.
>
> -Additionally, an +external.mk+ file may define the +help-custom+ make
> -rule, to document custom make targets specific to this +BR2_EXTERNAL+
> -tree. The help is completely free-form.
> +Additionally, a +BR2_EXTERNAL+ tree may provide a file named +help.mk+
> +that defines the +help-custom+ make rule, to document custom make
> +targets specific to this +BR2_EXTERNAL+ tree. The help is completely
> +free-form. See below for a sample +$(BR2_EXTERNAL)/help.mk+ file:
>
>   ------
>   help-custom:
>


-- 
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] core/help: fix custom help without a .config

[Thomas Petazzoni]

Hello,

On Tue, 22 Mar 2016 23:40:17 +0100, Arnout Vandecappelle wrote:
> On 03/20/16 22:24, Yann E. MORIN wrote:
> > When there is no .config, we do not source $(BR2_EXTERNAL)/external.mk.
> > So we can not expose the custom help in that situation.
> >
> > It is now known whether sourcing $(BR2_EXTERNAL)/external.mk outside the
> > HAVE_DOT_CONFIG conditional block is entirely safe. Even if it would
> > work today, we would have a hard time not breaking it in the future,
> > because we do not have automatic checks for that and would need to rely
> > on users reporting issues after the fact.
> >
> > Instead, we require the custom help to be defined in its own file in the
> > br2-external tree. This way, we can safely include it unconditionally.
> 
>   IMHO that custom help was a bad idea. It's adding complexity, an extra file in 
> BR2_EXTERNAL, and really not that useful...

Agreed. I'm not sure it's worth it. It's still time to revert if we
don't think it's a good idea. Peter?

Without this custom help thing, it is already possible to define some
custom make target in external.mk that will display some help. It won't
be available until a configuration is defined, but oh well, who reads
help texts anyway?

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH] core/help: fix custom help without a .config

[Peter Korsgaard]

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

Hi,

 >> > Instead, we require the custom help to be defined in its own file in the
 >> > br2-external tree. This way, we can safely include it unconditionally.
 >> 
 >> IMHO that custom help was a bad idea. It's adding complexity, an extra file in 
 >> BR2_EXTERNAL, and really not that useful...

 > Agreed. I'm not sure it's worth it. It's still time to revert if we
 > don't think it's a good idea. Peter?

 > Without this custom help thing, it is already possible to define some
 > custom make target in external.mk that will display some help. It won't
 > be available until a configuration is defined, but oh well, who reads
 > help texts anyway?

Yeah, I also think this adds too much complexity just for a pointer to
help text. I think we should drop it.

-- 
Venlig hilsen,
Peter Korsgaard 

[Buildroot] [PATCH] core/help: fix custom help without a .config

[Thomas Petazzoni]

Hello,

On Sat, 16 Apr 2016 00:16:06 +0200, Peter Korsgaard wrote:

>  > Without this custom help thing, it is already possible to define some
>  > custom make target in external.mk that will display some help. It won't
>  > be available until a configuration is defined, but oh well, who reads
>  > help texts anyway?
> 
> Yeah, I also think this adds too much complexity just for a pointer to
> help text. I think we should drop it.

So will you revert it?

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH] core/help: fix custom help without a .config

[Yann E. MORIN]

Thomas, Peter, All,

On 2016-04-16 09:28 +0200, Thomas Petazzoni spake thusly:
> On Sat, 16 Apr 2016 00:16:06 +0200, Peter Korsgaard wrote:
> >  > Without this custom help thing, it is already possible to define some
> >  > custom make target in external.mk that will display some help. It won't
> >  > be available until a configuration is defined, but oh well, who reads
> >  > help texts anyway?
> > 
> > Yeah, I also think this adds too much complexity just for a pointer to
> > help text. I think we should drop it.
> 
> So will you revert it?

I'll send the patches to revert it.

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] core/help: fix custom help without a .config

[Arnout Vandecappelle]

On 03/20/16 22:24, Yann E. MORIN wrote:
> When there is no .config, we do not source $(BR2_EXTERNAL)/external.mk.
> So we can not expose the custom help in that situation.
>
> It is now known whether sourcing $(BR2_EXTERNAL)/external.mk outside the
> HAVE_DOT_CONFIG conditional block is entirely safe. Even if it would
> work today, we would have a hard time not breaking it in the future,
> because we do not have automatic checks for that and would need to rely
> on users reporting issues after the fact.
>
> Instead, we require the custom help to be defined in its own file in the
> br2-external tree. This way, we can safely include it unconditionally.

  Custom help has been reverted now, so I marked this patch as rejected.

  Regards,
  Arnout

>
> Reported-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>   Makefile                             | 11 +++++++----
>   docs/manual/customize-outside-br.txt |  7 ++++---
>   2 files changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index ea8b1e4..03657d5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -976,10 +976,13 @@ endif
>   	@echo 'it on-line at http://buildroot.org/docs.html'
>   	@echo
>
> -# This rule does nothing, it is expected to be overloaded by
> -# a br2-external tree or a local.mk . However, it must exist,
> -# as we reference it in the main help, above. Making the rule
> -# .PHONY does not work.
> +ifneq ($(wildcard $(BR2_EXTERNAL)/help.mk),)
> +include $(BR2_EXTERNAL)/help.mk
> +endif
> +
> +# This rule does nothing, it is expected to be overloaded by a
> +# br2-external tree. However, it must exist, as we reference it
> +# as a dependency of the main help, above.
>   help-custom:
>
>   list-defconfigs:
> diff --git a/docs/manual/customize-outside-br.txt b/docs/manual/customize-outside-br.txt
> index be1827e..4f1c752 100644
> --- a/docs/manual/customize-outside-br.txt
> +++ b/docs/manual/customize-outside-br.txt
> @@ -108,9 +108,10 @@ And then in +$(BR2_EXTERNAL)/package/package1+ and
>      normal +make <name>_defconfig+ command. They will be visible under the
>      +User-provided configs+' label in the 'make list-defconfigs' output.
>
> -Additionally, an +external.mk+ file may define the +help-custom+ make
> -rule, to document custom make targets specific to this +BR2_EXTERNAL+
> -tree. The help is completely free-form.
> +Additionally, a +BR2_EXTERNAL+ tree may provide a file named +help.mk+
> +that defines the +help-custom+ make rule, to document custom make
> +targets specific to this +BR2_EXTERNAL+ tree. The help is completely
> +free-form. See below for a sample +$(BR2_EXTERNAL)/help.mk+ file:
>
>   ------
>   help-custom:
>


-- 
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] core/help: fix custom help without a .config

[Yann E. MORIN]

Arnout, All,

On 2016-04-18 00:51 +0200, Arnout Vandecappelle spake thusly:
> On 03/20/16 22:24, Yann E. MORIN wrote:
> >When there is no .config, we do not source $(BR2_EXTERNAL)/external.mk.
> >So we can not expose the custom help in that situation.
> >
> >It is now known whether sourcing $(BR2_EXTERNAL)/external.mk outside the
> >HAVE_DOT_CONFIG conditional block is entirely safe. Even if it would
> >work today, we would have a hard time not breaking it in the future,
> >because we do not have automatic checks for that and would need to rely
> >on users reporting issues after the fact.
> >
> >Instead, we require the custom help to be defined in its own file in the
> >br2-external tree. This way, we can safely include it unconditionally.
> 
>  Custom help has been reverted now, so I marked this patch as rejected.

Yes, thanks! :-)

Regards,
Yann E. MORIN.

>  Regards,
>  Arnout
> 
> >
> >Reported-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> >Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> >Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> >---
> >  Makefile                             | 11 +++++++----
> >  docs/manual/customize-outside-br.txt |  7 ++++---
> >  2 files changed, 11 insertions(+), 7 deletions(-)
> >
> >diff --git a/Makefile b/Makefile
> >index ea8b1e4..03657d5 100644
> >--- a/Makefile
> >+++ b/Makefile
> >@@ -976,10 +976,13 @@ endif
> >  	@echo 'it on-line at http://buildroot.org/docs.html'
> >  	@echo
> >
> >-# This rule does nothing, it is expected to be overloaded by
> >-# a br2-external tree or a local.mk . However, it must exist,
> >-# as we reference it in the main help, above. Making the rule
> >-# .PHONY does not work.
> >+ifneq ($(wildcard $(BR2_EXTERNAL)/help.mk),)
> >+include $(BR2_EXTERNAL)/help.mk
> >+endif
> >+
> >+# This rule does nothing, it is expected to be overloaded by a
> >+# br2-external tree. However, it must exist, as we reference it
> >+# as a dependency of the main help, above.
> >  help-custom:
> >
> >  list-defconfigs:
> >diff --git a/docs/manual/customize-outside-br.txt b/docs/manual/customize-outside-br.txt
> >index be1827e..4f1c752 100644
> >--- a/docs/manual/customize-outside-br.txt
> >+++ b/docs/manual/customize-outside-br.txt
> >@@ -108,9 +108,10 @@ And then in +$(BR2_EXTERNAL)/package/package1+ and
> >     normal +make <name>_defconfig+ command. They will be visible under the
> >     +User-provided configs+' label in the 'make list-defconfigs' output.
> >
> >-Additionally, an +external.mk+ file may define the +help-custom+ make
> >-rule, to document custom make targets specific to this +BR2_EXTERNAL+
> >-tree. The help is completely free-form.
> >+Additionally, a +BR2_EXTERNAL+ tree may provide a file named +help.mk+
> >+that defines the +help-custom+ make rule, to document custom make
> >+targets specific to this +BR2_EXTERNAL+ tree. The help is completely
> >+free-form. See below for a sample +$(BR2_EXTERNAL)/help.mk+ file:
> >
> >  ------
> >  help-custom:
> >
> 
> 
> -- 
> 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

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