All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] $(SED) not defined if 'make menuconfig savedefconfig'
@ 2015-06-15 10:26 Alvaro Gamez
  2015-06-15 13:37 ` Thomas Petazzoni
  0 siblings, 1 reply; 14+ messages in thread
From: Alvaro Gamez @ 2015-06-15 10:26 UTC (permalink / raw)
  To: buildroot

Hi!

While using buildroot-submodule (
https://github.com/Openwide-Ingenierie/buildroot-submodule) I've noticed
this happens since commit f71a621d91ec27f175fc84012962f88b1107305f was
introduced into buildroot.

$ make menuconfig savedefconfig
[Do whatever, or nothing at all and exit, there's no need to save anything]
*** End of the configuration.
*** Execute 'make' to start the build or try 'make help'.

make: /BR2_DEFCONFIG=/d: Command not found
Makefile:777: recipe for target 'savedefconfig' failed
make: *** [savedefconfig] Error 127

However,
$ make menuconfig
$ make savedefconfig

does not end with that error. I've just tracked it down to the fact that
$(SED) is not defined when menuconfig savedefconfig are used in a single
make call, whereas it equals to '/bin/sed -i -e'

Simply replacing that @$(SED) by /bin/sed -i -e on line 777 of Makefile
makes it work again, but I don't think that solution is desirable. I'm
sorry I can't provide a full patch, the motive of this bug is beyond my
knowledge.

Best regards

-- 
?lvaro G?mez Machado
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20150615/e5bb3d96/attachment.html>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] $(SED) not defined if 'make menuconfig savedefconfig'
  2015-06-15 10:26 [Buildroot] $(SED) not defined if 'make menuconfig savedefconfig' Alvaro Gamez
@ 2015-06-15 13:37 ` Thomas Petazzoni
  2015-06-15 14:20   ` rdkehn at yahoo.com
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Petazzoni @ 2015-06-15 13:37 UTC (permalink / raw)
  To: buildroot

Dear Alvaro Gamez,

On Mon, 15 Jun 2015 12:26:02 +0200, Alvaro Gamez wrote:

> While using buildroot-submodule (
> https://github.com/Openwide-Ingenierie/buildroot-submodule) I've noticed
> this happens since commit f71a621d91ec27f175fc84012962f88b1107305f was
> introduced into buildroot.
> 
> $ make menuconfig savedefconfig
> [Do whatever, or nothing at all and exit, there's no need to save anything]
> *** End of the configuration.
> *** Execute 'make' to start the build or try 'make help'.
> 
> make: /BR2_DEFCONFIG=/d: Command not found
> Makefile:777: recipe for target 'savedefconfig' failed
> make: *** [savedefconfig] Error 127
> 
> However,
> $ make menuconfig
> $ make savedefconfig
> 
> does not end with that error. I've just tracked it down to the fact that
> $(SED) is not defined when menuconfig savedefconfig are used in a single
> make call, whereas it equals to '/bin/sed -i -e'
> 
> Simply replacing that @$(SED) by /bin/sed -i -e on line 777 of Makefile
> makes it work again, but I don't think that solution is desirable. I'm
> sorry I can't provide a full patch, the motive of this bug is beyond my
> knowledge.

Hum, right. Can you try the below patch:

diff --git a/Makefile b/Makefile
index d3f80c4..67eb50c 100644
--- a/Makefile
+++ b/Makefile
@@ -272,6 +272,7 @@ HOSTLN := $(shell which $(HOSTLN) || type -p $(HOSTLN) || echo ln)
 HOSTNM := $(shell which $(HOSTNM) || type -p $(HOSTNM) || echo nm)
 HOSTOBJCOPY := $(shell which $(HOSTOBJCOPY) || type -p $(HOSTOBJCOPY) || echo objcopy)
 HOSTRANLIB := $(shell which $(HOSTRANLIB) || type -p $(HOSTRANLIB) || echo ranlib)
+SED := $(shell which sed || type -p sed) -i -e
 
 export HOSTAR HOSTAS HOSTCC HOSTCXX HOSTLD
 export HOSTCC_NOCCACHE HOSTCXX_NOCCACHE
diff --git a/package/Makefile.in b/package/Makefile.in
index c02d31f..f256b05 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -217,7 +217,6 @@ endif
 INSTALL := $(shell which install || type -p install)
 FLEX := $(shell which flex || type -p flex)
 BISON := $(shell which bison || type -p bison)
-SED := $(shell which sed || type -p sed) -i -e
 UNZIP := $(shell which unzip || type -p unzip) -q
 
 APPLY_PATCHES = support/scripts/apply-patches.sh $(if $(QUIET),-s)


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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Buildroot] $(SED) not defined if 'make menuconfig savedefconfig'
  2015-06-15 13:37 ` Thomas Petazzoni
@ 2015-06-15 14:20   ` rdkehn at yahoo.com
  2015-06-16  7:58     ` Alvaro Gamez
  0 siblings, 1 reply; 14+ messages in thread
From: rdkehn at yahoo.com @ 2015-06-15 14:20 UTC (permalink / raw)
  To: buildroot

Hi Thomas, Alvaro,

On Mon, Jun 15, 2015 at 03:37:42PM +0200, Thomas Petazzoni wrote:
> Dear Alvaro Gamez,
> 
> On Mon, 15 Jun 2015 12:26:02 +0200, Alvaro Gamez wrote:
> 
> > While using buildroot-submodule (
> > https://github.com/Openwide-Ingenierie/buildroot-submodule) I've noticed
> > this happens since commit f71a621d91ec27f175fc84012962f88b1107305f was
> > introduced into buildroot.
> > 
> > $ make menuconfig savedefconfig
> > [Do whatever, or nothing at all and exit, there's no need to save anything]
> > *** End of the configuration.
> > *** Execute 'make' to start the build or try 'make help'.
> > 
> > make: /BR2_DEFCONFIG=/d: Command not found
> > Makefile:777: recipe for target 'savedefconfig' failed
> > make: *** [savedefconfig] Error 127
> > 
> > However,
> > $ make menuconfig
> > $ make savedefconfig
> > 
> > does not end with that error. I've just tracked it down to the fact that
> > $(SED) is not defined when menuconfig savedefconfig are used in a single
> > make call, whereas it equals to '/bin/sed -i -e'
> > 
> > Simply replacing that @$(SED) by /bin/sed -i -e on line 777 of Makefile
> > makes it work again, but I don't think that solution is desirable. I'm
> > sorry I can't provide a full patch, the motive of this bug is beyond my
> > knowledge.
> 
> Hum, right. Can you try the below patch:
> 
> diff --git a/Makefile b/Makefile
> index d3f80c4..67eb50c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -272,6 +272,7 @@ HOSTLN := $(shell which $(HOSTLN) || type -p $(HOSTLN) || echo ln)
>  HOSTNM := $(shell which $(HOSTNM) || type -p $(HOSTNM) || echo nm)
>  HOSTOBJCOPY := $(shell which $(HOSTOBJCOPY) || type -p $(HOSTOBJCOPY) || echo objcopy)
>  HOSTRANLIB := $(shell which $(HOSTRANLIB) || type -p $(HOSTRANLIB) || echo ranlib)
> +SED := $(shell which sed || type -p sed) -i -e
>  
>  export HOSTAR HOSTAS HOSTCC HOSTCXX HOSTLD
>  export HOSTCC_NOCCACHE HOSTCXX_NOCCACHE
> diff --git a/package/Makefile.in b/package/Makefile.in
> index c02d31f..f256b05 100644
> --- a/package/Makefile.in
> +++ b/package/Makefile.in
> @@ -217,7 +217,6 @@ endif
>  INSTALL := $(shell which install || type -p install)
>  FLEX := $(shell which flex || type -p flex)
>  BISON := $(shell which bison || type -p bison)
> -SED := $(shell which sed || type -p sed) -i -e
>  UNZIP := $(shell which unzip || type -p unzip) -q
>  
>  APPLY_PATCHES = support/scripts/apply-patches.sh $(if $(QUIET),-s)
> 
> 

I reproduced Alvaro's observation and your patch does resolve the
issue.

Tested-by: Doug Kehn <rdkehn@yahoo.com>

Regards,
...doug

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] $(SED) not defined if 'make menuconfig savedefconfig'
  2015-06-15 14:20   ` rdkehn at yahoo.com
@ 2015-06-16  7:58     ` Alvaro Gamez
  2016-02-16 11:46       ` Alvaro Gamez
  0 siblings, 1 reply; 14+ messages in thread
From: Alvaro Gamez @ 2015-06-16  7:58 UTC (permalink / raw)
  To: buildroot

Hi, Thomas, Doug,

2015-06-15 16:20 GMT+02:00 <rdkehn@yahoo.com>:

> > diff --git a/Makefile b/Makefile
> > index d3f80c4..67eb50c 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -272,6 +272,7 @@ HOSTLN := $(shell which $(HOSTLN) || type -p
> $(HOSTLN) || echo ln)
> >  HOSTNM := $(shell which $(HOSTNM) || type -p $(HOSTNM) || echo nm)
> >  HOSTOBJCOPY := $(shell which $(HOSTOBJCOPY) || type -p $(HOSTOBJCOPY)
> || echo objcopy)
> >  HOSTRANLIB := $(shell which $(HOSTRANLIB) || type -p $(HOSTRANLIB) ||
> echo ranlib)
> > +SED := $(shell which sed || type -p sed) -i -e
> >
> >  export HOSTAR HOSTAS HOSTCC HOSTCXX HOSTLD
> >  export HOSTCC_NOCCACHE HOSTCXX_NOCCACHE
> > diff --git a/package/Makefile.in b/package/Makefile.in
> > index c02d31f..f256b05 100644
> > --- a/package/Makefile.in
> > +++ b/package/Makefile.in
> > @@ -217,7 +217,6 @@ endif
> >  INSTALL := $(shell which install || type -p install)
> >  FLEX := $(shell which flex || type -p flex)
> >  BISON := $(shell which bison || type -p bison)
> > -SED := $(shell which sed || type -p sed) -i -e
> >  UNZIP := $(shell which unzip || type -p unzip) -q
> >
> >  APPLY_PATCHES = support/scripts/apply-patches.sh $(if $(QUIET),-s)
> >
> >
>
> I reproduced Alvaro's observation and your patch does resolve the
> issue.
>
> Tested-by: Doug Kehn <rdkehn@yahoo.com>
>
> Regards,
> ...doug
>
>
I've just tested it too and can confirm that this fixes the problem and my
particular use case with buildroot-submodule.

Tested-by: Alvaro G. M. <alvaro.gamez@hazent.com>

Thanks a lot


-- 
?lvaro G?mez Machado
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20150616/193bdcfa/attachment.html>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] $(SED) not defined if 'make menuconfig savedefconfig'
  2015-06-16  7:58     ` Alvaro Gamez
@ 2016-02-16 11:46       ` Alvaro Gamez
  2016-02-16 15:10         ` Thomas Petazzoni
  0 siblings, 1 reply; 14+ messages in thread
From: Alvaro Gamez @ 2016-02-16 11:46 UTC (permalink / raw)
  To: buildroot

Hi, all

2015-06-16 9:58 GMT+02:00 Alvaro Gamez <alvaro.gamez@hazent.com>:

> 2015-06-15 16:20 GMT+02:00 <rdkehn@yahoo.com>:
>
>> > diff --git a/Makefile b/Makefile
>> > index d3f80c4..67eb50c 100644
>> > --- a/Makefile
>> > +++ b/Makefile
>> > @@ -272,6 +272,7 @@ HOSTLN := $(shell which $(HOSTLN) || type -p
>> $(HOSTLN) || echo ln)
>> >  HOSTNM := $(shell which $(HOSTNM) || type -p $(HOSTNM) || echo nm)
>> >  HOSTOBJCOPY := $(shell which $(HOSTOBJCOPY) || type -p $(HOSTOBJCOPY)
>> || echo objcopy)
>> >  HOSTRANLIB := $(shell which $(HOSTRANLIB) || type -p $(HOSTRANLIB) ||
>> echo ranlib)
>> > +SED := $(shell which sed || type -p sed) -i -e
>> >
>> >  export HOSTAR HOSTAS HOSTCC HOSTCXX HOSTLD
>> >  export HOSTCC_NOCCACHE HOSTCXX_NOCCACHE
>> > diff --git a/package/Makefile.in b/package/Makefile.in
>> > index c02d31f..f256b05 100644
>> > --- a/package/Makefile.in
>> > +++ b/package/Makefile.in
>> > @@ -217,7 +217,6 @@ endif
>> >  INSTALL := $(shell which install || type -p install)
>> >  FLEX := $(shell which flex || type -p flex)
>> >  BISON := $(shell which bison || type -p bison)
>> > -SED := $(shell which sed || type -p sed) -i -e
>> >  UNZIP := $(shell which unzip || type -p unzip) -q
>> >
>> >  APPLY_PATCHES = support/scripts/apply-patches.sh $(if $(QUIET),-s)
>> >
>> >
>>
>> I reproduced Alvaro's observation and your patch does resolve the
>> issue.
>>
>> Tested-by: Doug Kehn <rdkehn@yahoo.com>
>>
>> Regards,
>> ...doug
>>
>>
> I've just tested it too and can confirm that this fixes the problem and my
> particular use case with buildroot-submodule.
>
> Tested-by: Alvaro G. M. <alvaro.gamez@hazent.com>
>
>
I just wanted to query about the status of this patch. I haven't found it
on patchwork, probably because it wasn't sent as a PATCH email on itself,
but it was tested by Doug and me.

I can confirm it still applies and works as intended. If you need me to, I
can git-email it.

Best regards


-- 
?lvaro G?mez Machado
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20160216/b528d529/attachment.html>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] $(SED) not defined if 'make menuconfig savedefconfig'
  2016-02-16 11:46       ` Alvaro Gamez
@ 2016-02-16 15:10         ` Thomas Petazzoni
  2016-02-16 15:15           ` [Buildroot] [PATCH 1/1] Declare SED Makefile instead of package/Makefile.in so it exists globally Alvaro G. M
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Petazzoni @ 2016-02-16 15:10 UTC (permalink / raw)
  To: buildroot

Hello Alvaro,

On Tue, 16 Feb 2016 12:46:55 +0100, Alvaro Gamez wrote:

> > I've just tested it too and can confirm that this fixes the problem and my
> > particular use case with buildroot-submodule.
> >
> > Tested-by: Alvaro G. M. <alvaro.gamez@hazent.com>
> >
> >
> I just wanted to query about the status of this patch. I haven't found it
> on patchwork, probably because it wasn't sent as a PATCH email on itself,
> but it was tested by Doug and me.
> 
> I can confirm it still applies and works as intended. If you need me to, I
> can git-email it.

Indeed this patch is not in patchwork. And if a patch is not in
patchwork, we will definitely forget about it. We have several hundreds
of patches already in patchwork, so we can hardly keep track of patches
that were not submitted in a form that allows them to be tracked by
patchwork.

So, it would be really good if you could submit this patch with 'git
send-email'.

Thanks a lot!

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/1] Declare SED Makefile instead of package/Makefile.in so it exists globally.
  2016-02-16 15:10         ` Thomas Petazzoni
@ 2016-02-16 15:15           ` Alvaro G. M
  2016-02-16 20:35             ` Thomas Petazzoni
  0 siblings, 1 reply; 14+ messages in thread
From: Alvaro G. M @ 2016-02-16 15:15 UTC (permalink / raw)
  To: buildroot

In particular, this allows the use of buildroot-submodule and in general,
any use of buildroot from out of tree.

Tested-by: Doug Kehn <rdkehn@yahoo.com>
Tested-by: Alvaro G. M. <alvaro.gamez@hazent.com>
Signed-off-by: Alvaro G. M <alvaro.gamez@hazent.com>
---
 Makefile            | 1 +
 package/Makefile.in | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 71735b5..d6069dd 100644
--- a/Makefile
+++ b/Makefile
@@ -279,6 +279,7 @@ HOSTLN := $(shell which $(HOSTLN) || type -p $(HOSTLN) || echo ln)
 HOSTNM := $(shell which $(HOSTNM) || type -p $(HOSTNM) || echo nm)
 HOSTOBJCOPY := $(shell which $(HOSTOBJCOPY) || type -p $(HOSTOBJCOPY) || echo objcopy)
 HOSTRANLIB := $(shell which $(HOSTRANLIB) || type -p $(HOSTRANLIB) || echo ranlib)
+SED := $(shell which sed || type -p sed) -i -e
 
 export HOSTAR HOSTAS HOSTCC HOSTCXX HOSTLD
 export HOSTCC_NOCCACHE HOSTCXX_NOCCACHE
diff --git a/package/Makefile.in b/package/Makefile.in
index dd595e2..273a594 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -214,7 +214,6 @@ endif
 INSTALL := $(shell which install || type -p install)
 FLEX := $(shell which flex || type -p flex)
 BISON := $(shell which bison || type -p bison)
-SED := $(shell which sed || type -p sed) -i -e
 UNZIP := $(shell which unzip || type -p unzip) -q
 
 APPLY_PATCHES = support/scripts/apply-patches.sh $(if $(QUIET),-s)
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/1] Declare SED Makefile instead of package/Makefile.in so it exists globally.
  2016-02-16 15:15           ` [Buildroot] [PATCH 1/1] Declare SED Makefile instead of package/Makefile.in so it exists globally Alvaro G. M
@ 2016-02-16 20:35             ` Thomas Petazzoni
  2016-02-16 22:35               ` Arnout Vandecappelle
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Petazzoni @ 2016-02-16 20:35 UTC (permalink / raw)
  To: buildroot

Hello,

The title is too long, it should be:

	Makefile: declare SED globally

or something like that.

On Tue, 16 Feb 2016 16:15:26 +0100, Alvaro G. M wrote:
> In particular, this allows the use of buildroot-submodule and in general,
> any use of buildroot from out of tree.

This commit log should be improved, because:

 1/ Why should we care about buildroot-submodule, and if we care, what
    is the actual problem ?

 2/ What do you mean by "use Buildroot from out of tree" ? I do out of
    tree builds every day and it works just fine. So this aspect needs
    to be detailed.

I have nothing against the change, which is fairly simple, but it needs
to be justified properly.

Thanks a lot!

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/1] Declare SED Makefile instead of package/Makefile.in so it exists globally.
  2016-02-16 20:35             ` Thomas Petazzoni
@ 2016-02-16 22:35               ` Arnout Vandecappelle
  2016-02-17  8:22                 ` Alvaro Gamez
  0 siblings, 1 reply; 14+ messages in thread
From: Arnout Vandecappelle @ 2016-02-16 22:35 UTC (permalink / raw)
  To: buildroot

 Hi Alvaro,

 In addition to Thomas's comments, please use your full name in the
Signed-off-by line.

On 16-02-16 21:35, Thomas Petazzoni wrote:
> Hello,
> 
> The title is too long, it should be:
> 
> 	Makefile: declare SED globally
> 
> or something like that.
> 
> On Tue, 16 Feb 2016 16:15:26 +0100, Alvaro G. M wrote:
>> In particular, this allows the use of buildroot-submodule and in general,
>> any use of buildroot from out of tree.
> 
> This commit log should be improved, because:
> 
>  1/ Why should we care about buildroot-submodule, and if we care, what
>     is the actual problem ?
> 
>  2/ What do you mean by "use Buildroot from out of tree" ? I do out of
>     tree builds every day and it works just fine. So this aspect needs
>     to be detailed.
> 
> I have nothing against the change, which is fairly simple, but it needs
> to be justified properly.

 I suspect we're not going to like the justification :-)


 Regards,
 Arnout


-- 
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] 14+ messages in thread

* [Buildroot] [PATCH 1/1] Declare SED Makefile instead of package/Makefile.in so it exists globally.
  2016-02-16 22:35               ` Arnout Vandecappelle
@ 2016-02-17  8:22                 ` Alvaro Gamez
  2016-02-17  8:47                   ` Thomas Petazzoni
  0 siblings, 1 reply; 14+ messages in thread
From: Alvaro Gamez @ 2016-02-17  8:22 UTC (permalink / raw)
  To: buildroot

Hi, guys

2016-02-16 23:35 GMT+01:00 Arnout Vandecappelle <arnout@mind.be>:

>  Hi Alvaro,
>
>  In addition to Thomas's comments, please use your full name in the
> Signed-off-by line.
>
>
Well, I've been reviewing the original thread (
http://lists.busybox.net/pipermail/buildroot/2015-June/130347.html) in
order to write a good commit and I've just realized that this patch wasn't
in fact written by me, it was written by Thomas in respones to my
inquiries, so I think the Signed-off-by line should in fact belong to him,
not me. Sorry I took over that, it wasn't my intention.



> On 16-02-16 21:35, Thomas Petazzoni wrote:
> >  1/ Why should we care about buildroot-submodule, and if we care, what
> >     is the actual problem ?
>

Because it provides a nice workflow, which even though not perfect, is very
useful at least to me and to Jeremy Rosen, its original author :) Not that
it should matter much to anyone who doesn't use it, I guess.

But, anyhow, reviewing the original thread, the bug this patch solves does
not only affect buildroot-submodule, although I discovered it by using that
piece of software, so the commit message was in fact very wrong, sorry.

There's one case of make invocation that won't work without this patch,
which is this:

make menuconfig savedefconfig

After exiting menuconfig, the following error is reported:

make[1]: /BR2_DEFCONFIG=/d: Command not found
Makefile:849: recipe for target 'savedefconfig' failed
make[1]: *** [savedefconfig] Error 127
common.mk:65: recipe for target 'menuconfig' failed
make: *** [menuconfig] Error 2

>
> >  2/ What do you mean by "use Buildroot from out of tree" ? I do out of
> >     tree builds every day and it works just fine. So this aspect needs
> >     to be detailed.
>
>
As explained above, that was simply false. Oops. I had forgotten about the
details of these and I thought it only affected make -C invocations, but
it's not a matter of current path but of those two make targets invoked at
the same time.



>  I suspect we're not going to like the justification :-)
>

Why not?


Best regards, sorry about all the trouble!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20160217/76c9f91a/attachment-0001.html>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/1] Declare SED Makefile instead of package/Makefile.in so it exists globally.
  2016-02-17  8:22                 ` Alvaro Gamez
@ 2016-02-17  8:47                   ` Thomas Petazzoni
  2016-02-27 13:17                     ` Arnout Vandecappelle
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Petazzoni @ 2016-02-17  8:47 UTC (permalink / raw)
  To: buildroot

Hello,

On Wed, 17 Feb 2016 09:22:48 +0100, Alvaro Gamez wrote:

> Well, I've been reviewing the original thread (
> http://lists.busybox.net/pipermail/buildroot/2015-June/130347.html) in
> order to write a good commit and I've just realized that this patch wasn't
> in fact written by me, it was written by Thomas in respones to my
> inquiries, so I think the Signed-off-by line should in fact belong to him,
> not me. Sorry I took over that, it wasn't my intention.

Aaah, yes, now I remember.

The savedefconfig target looks like:

savedefconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
        @$(COMMON_CONFIG_ENV) $< \
                --savedefconfig=$(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig) \
                $(CONFIG_CONFIG_IN)
        @$(SED) '/BR2_DEFCONFIG=/d' $(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig)

So it uses the $(SED) variable.

However, this savedefconfig targets is implemented *outside* of the
ifeq ($(BR2_HAVE_DOT_CONFIG),y) condition, but the inclusion of
package/Makefile.in (which defines the SED variable) is only done
inside the ifeq ($(BR2_HAVE_DOT_CONFIG),y).

So when you start from scratch and do "make menuconfig savedefconfig",
BR2_HAVE_DOT_CONFIG is not defined, so package/Makefile.in is not
included.

Define SED in the main Makefile is a quick work-around.

However, it will only fix this specific problem, and I believe if you do:

	make menuconfig savedefconfig all

With the aim of starting the build right after the menuconfig, then it
will still not work for other reasons.

I see two possible directions here:

 1. We decide to really separate targets that can "create" the
    configuration from the other targets, and in this case, savedefconfig
    should be defined inside the BR2_HAVE_DOT_CONFIG condition. Which
    means that "make menuconfig savedefconfig" would no longer work,
    and you would have to do "make menuconfig" and then "make
    savedefconfig".

 2. We decide that all targets should be available at all times, and
    get rid of this BR2_HAVE_DOT_CONFIG. However, I don't see how this
    can work since if you do "make menuconfig all", then@the time of
    "menuconfig", the .config doesn't exist, so it cannot be included
    by the main Makefile, and therefore when the "all" target will be
    processed, we won't have all the BR2_* make variables defined.

So I believe (2) doesn't work, and we should instead go with (1).

Arnout, what do you think?

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/1] Declare SED Makefile instead of package/Makefile.in so it exists globally.
  2016-02-17  8:47                   ` Thomas Petazzoni
@ 2016-02-27 13:17                     ` Arnout Vandecappelle
  2016-02-27 22:00                       ` Peter Korsgaard
  0 siblings, 1 reply; 14+ messages in thread
From: Arnout Vandecappelle @ 2016-02-27 13:17 UTC (permalink / raw)
  To: buildroot

On 02/17/16 09:47, Thomas Petazzoni wrote:
> Hello,
> 
> On Wed, 17 Feb 2016 09:22:48 +0100, Alvaro Gamez wrote:
> 
>> Well, I've been reviewing the original thread (
>> http://lists.busybox.net/pipermail/buildroot/2015-June/130347.html) in
>> order to write a good commit and I've just realized that this patch wasn't
>> in fact written by me, it was written by Thomas in respones to my
>> inquiries, so I think the Signed-off-by line should in fact belong to him,
>> not me. Sorry I took over that, it wasn't my intention.
> 
> Aaah, yes, now I remember.
> 
> The savedefconfig target looks like:
> 
> savedefconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
>         @$(COMMON_CONFIG_ENV) $< \
>                 --savedefconfig=$(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig) \
>                 $(CONFIG_CONFIG_IN)
>         @$(SED) '/BR2_DEFCONFIG=/d' $(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig)
> 
> So it uses the $(SED) variable.
> 
> However, this savedefconfig targets is implemented *outside* of the
> ifeq ($(BR2_HAVE_DOT_CONFIG),y) condition, but the inclusion of
> package/Makefile.in (which defines the SED variable) is only done
> inside the ifeq ($(BR2_HAVE_DOT_CONFIG),y).
> 
> So when you start from scratch and do "make menuconfig savedefconfig",
> BR2_HAVE_DOT_CONFIG is not defined, so package/Makefile.in is not
> included.
> 
> Define SED in the main Makefile is a quick work-around.
> 
> However, it will only fix this specific problem, and I believe if you do:
> 
> 	make menuconfig savedefconfig all
> 
> With the aim of starting the build right after the menuconfig, then it
> will still not work for other reasons.
> 
> I see two possible directions here:
> 
>  1. We decide to really separate targets that can "create" the
>     configuration from the other targets, and in this case, savedefconfig
>     should be defined inside the BR2_HAVE_DOT_CONFIG condition. Which
>     means that "make menuconfig savedefconfig" would no longer work,
>     and you would have to do "make menuconfig" and then "make
>     savedefconfig".
> 
>  2. We decide that all targets should be available at all times, and
>     get rid of this BR2_HAVE_DOT_CONFIG. However, I don't see how this
>     can work since if you do "make menuconfig all", then at the time of
>     "menuconfig", the .config doesn't exist, so it cannot be included
>     by the main Makefile, and therefore when the "all" target will be
>     processed, we won't have all the BR2_* make variables defined.
> 
> So I believe (2) doesn't work, and we should instead go with (1).
> 
> Arnout, what do you think? In addition, I think it would be good t In addition, I think it would be good to  In addition, I think it would be good to o 

 I agree we should go for (1).

 However, this doesn't completely stop the 'make menuconfig savedefconfig': once
a .config file exists, you can still do that. But it could have some funky side
effects (not that I can think of any right away, but it feels a bit iffy). So I
think it would be good to enforce that all the noconfig_targets can only be
called on their own on the command line. This would block completely silly
things like 'make menuconfig defconfig' (it would also block potentially valid
things like 'make foo_defconfig menuconfig', but I think adding an extra '&&
make' in the middle of that is not so painful).


 Regarding (2), it is probably not impossible, because make has this second-pass
mechanism. When you include a makefile (like we do for .config), and that file
is the target of some rule that gets triggered, then all the makefiles will be
re-read after that rule has been executed (but this happens only once, to avoid
infinite recursion). So you could add something like:

.config: $(filter $(noconfig_targets),$(MAKECMDGOALS))
	@:

to re-read the .config. However, I think that's very fragile, difficult to
understand, and not so much added value.


 Regards,
 Arnout


-- 
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] 14+ messages in thread

* [Buildroot] [PATCH 1/1] Declare SED Makefile instead of package/Makefile.in so it exists globally.
  2016-02-27 13:17                     ` Arnout Vandecappelle
@ 2016-02-27 22:00                       ` Peter Korsgaard
  2016-02-28  7:43                         ` Alvaro Gamez
  0 siblings, 1 reply; 14+ messages in thread
From: Peter Korsgaard @ 2016-02-27 22:00 UTC (permalink / raw)
  To: buildroot

>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:

Hi,

>> Arnout, what do you think? In addition, I think it would be good t In
>> addition, I think it would be good to In addition, I think it would
>> be good to o

 >  I agree we should go for (1).

Yes, I also think this is the sanest / simplest thing to do.

 >  However, this doesn't completely stop the 'make menuconfig savedefconfig': once
 > a .config file exists, you can still do that. But it could have some funky side
 > effects (not that I can think of any right away, but it feels a bit iffy). So I
 > think it would be good to enforce that all the noconfig_targets can only be
 > called on their own on the command line. This would block completely silly
 > things like 'make menuconfig defconfig' (it would also block potentially valid
 > things like 'make foo_defconfig menuconfig', but I think adding an extra '&&
 > make' in the middle of that is not so painful).

No, that seems ok (as long as it is clear from the error message that
you need to do so).

-- 
Bye, Peter Korsgaard

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/1] Declare SED Makefile instead of package/Makefile.in so it exists globally.
  2016-02-27 22:00                       ` Peter Korsgaard
@ 2016-02-28  7:43                         ` Alvaro Gamez
  0 siblings, 0 replies; 14+ messages in thread
From: Alvaro Gamez @ 2016-02-28  7:43 UTC (permalink / raw)
  To: buildroot

Hi,

2016-02-27 23:00 GMT+01:00 Peter Korsgaard <peter@korsgaard.com>:

> >>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:
>   >  However, this doesn't completely stop the 'make menuconfig
> savedefconfig': once
>  > a .config file exists, you can still do that. But it could have some
> funky side
>  > effects (not that I can think of any right away, but it feels a bit
> iffy). So I
>  > think it would be good to enforce that all the noconfig_targets can
> only be
>  > called on their own on the command line. This would block completely
> silly
>  > things like 'make menuconfig defconfig' (it would also block
> potentially valid
>  > things like 'make foo_defconfig menuconfig', but I think adding an
> extra '&&
>  > make' in the middle of that is not so painful).
>
> No, that seems ok (as long as it is clear from the error message that
> you need to do so).
>

I agree. For example from the point of view of buildroot-submodule there's
no problem to invoke make twice, and, as you say, if there's an error
message that explains that two targets cannot be called at once, that's all
right then.

-- 
?lvaro G?mez Machado
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20160228/077824fe/attachment.html>

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2016-02-28  7:43 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-15 10:26 [Buildroot] $(SED) not defined if 'make menuconfig savedefconfig' Alvaro Gamez
2015-06-15 13:37 ` Thomas Petazzoni
2015-06-15 14:20   ` rdkehn at yahoo.com
2015-06-16  7:58     ` Alvaro Gamez
2016-02-16 11:46       ` Alvaro Gamez
2016-02-16 15:10         ` Thomas Petazzoni
2016-02-16 15:15           ` [Buildroot] [PATCH 1/1] Declare SED Makefile instead of package/Makefile.in so it exists globally Alvaro G. M
2016-02-16 20:35             ` Thomas Petazzoni
2016-02-16 22:35               ` Arnout Vandecappelle
2016-02-17  8:22                 ` Alvaro Gamez
2016-02-17  8:47                   ` Thomas Petazzoni
2016-02-27 13:17                     ` Arnout Vandecappelle
2016-02-27 22:00                       ` Peter Korsgaard
2016-02-28  7:43                         ` Alvaro Gamez

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.