Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 1/2] core: don't reset MAKEOVERRIDES when re-entering make
@ 2016-11-03  1:55 Arnout Vandecappelle
  2016-11-03  1:55 ` [Buildroot] [PATCH 2/2] core: remove redundant "override O := $(O)" Arnout Vandecappelle
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Arnout Vandecappelle @ 2016-11-03  1:55 UTC (permalink / raw)
  To: buildroot

We reset MAKEOVERRIDES to avoid passing down variables that are
overridden on the command line to the package build systems. Indeed,
the variables overridden on the command line will be Buildroot
variables and not relevant to the package build system. In particular
the O option is used by some packages and the value passed in on the
command line is plain wrong for the individual package.

However, in commit 916e614b, MAKEOVERRIDES was moved earlier and it
was reset _before_ re-entering make in the cases when something has
to be fixed up (incorrect umask, non-absolute paths in O or CURDIR).
Therefore, if make is re-entered, any command line overrides are lost.

This particularly bites the autobuilders, because they use
O=<relative path> to specify the output directory, and they add
BR2_JLEVEL=... to avoid starting too many jobs in parallel. The
BR2_JLEVEL override is lost.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Cc: Samuel Martin <s.martin49@gmail.com>
Cc: Yann E. MORIN <yann.morin.1998@free.fr>
---
I have a little worry, because there are two other places where we re-
enter top-level make and those are not handled: silentoldconfig and
external-deps. BR2_JLEVEL is not relevant for those, but other command-
line overridden options may be. However, this hasn't changed from before
commit 916e614b so I guess it's OK.
---
 Makefile | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index c00e200..4b494a3 100644
--- a/Makefile
+++ b/Makefile
@@ -35,11 +35,6 @@ SHELL := $(shell if [ -x "$$BASH" ]; then echo $$BASH; \
 ifneq ("$(origin O)", "command line")
 O := $(CURDIR)/output
 else
-# Other packages might also support Linux-style out of tree builds
-# with the O=<dir> syntax (E.G. BusyBox does). As make automatically
-# forwards command line variable definitions those packages get very
-# confused. Fix this by telling make to not do so.
-MAKEOVERRIDES :=
 # Strangely enough O is still passed to submakes with MAKEOVERRIDES
 # (with make 3.81 atleast), the only thing that changes is the output
 # of the origin function (command line -> environment).
@@ -155,6 +150,13 @@ else ifneq ($(filter-out $(nobuild_targets),$(MAKECMDGOALS)),)
 BR_BUILDING = y
 endif
 
+# We call make recursively to build packages. The command-line overrides that
+# are passed to Buildroot don't apply to those package build systems. In
+# particular, we don't want to pass down the O=<dir> option for out-of-tree
+# builds, because the value specified on the command line will not be correct
+# for packages.
+MAKEOVERRIDES :=
+
 # Include some helper macros and variables
 include support/misc/utils.mk
 
-- 
2.9.3

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

* [Buildroot] [PATCH 2/2] core: remove redundant "override O := $(O)"
  2016-11-03  1:55 [Buildroot] [PATCH 1/2] core: don't reset MAKEOVERRIDES when re-entering make Arnout Vandecappelle
@ 2016-11-03  1:55 ` Arnout Vandecappelle
  2016-11-03  7:45   ` Samuel Martin
  2016-11-03  7:48 ` [Buildroot] [PATCH 1/2] core: don't reset MAKEOVERRIDES when re-entering make Samuel Martin
  2016-11-03 20:49 ` Thomas Petazzoni
  2 siblings, 1 reply; 6+ messages in thread
From: Arnout Vandecappelle @ 2016-11-03  1:55 UTC (permalink / raw)
  To: buildroot

The top-level Makefile contains an "override O := $(O)" statement that
is purportedly required to make sure the O flag doesn't leak into the
environment of sub-makes. However, since commit 173135d, there is
already an "override O := ..." a few lines down. Therefore, the first
override is redundant.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
I haven't been able to reproduce the issue mentioned in the comment
(tested by printing the value of $(O) in a dummy package) with make 4.1.
I therefore removed the comment completely.
---
 Makefile | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/Makefile b/Makefile
index 4b494a3..cb030d4 100644
--- a/Makefile
+++ b/Makefile
@@ -34,13 +34,6 @@ SHELL := $(shell if [ -x "$$BASH" ]; then echo $$BASH; \
 # build by preventing it from being forwarded to sub-make calls.
 ifneq ("$(origin O)", "command line")
 O := $(CURDIR)/output
-else
-# Strangely enough O is still passed to submakes with MAKEOVERRIDES
-# (with make 3.81 atleast), the only thing that changes is the output
-# of the origin function (command line -> environment).
-# Unfortunately some packages don't look at origin (E.G. uClibc 0.9.31+)
-# To really make O go away, we have to override it.
-override O := $(O)
 endif
 
 # Check if the current Buildroot execution meets all the pre-requisites.
-- 
2.9.3

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

* [Buildroot] [PATCH 2/2] core: remove redundant "override O := $(O)"
  2016-11-03  1:55 ` [Buildroot] [PATCH 2/2] core: remove redundant "override O := $(O)" Arnout Vandecappelle
@ 2016-11-03  7:45   ` Samuel Martin
  0 siblings, 0 replies; 6+ messages in thread
From: Samuel Martin @ 2016-11-03  7:45 UTC (permalink / raw)
  To: buildroot

On Thu, Nov 3, 2016 at 2:55 AM, Arnout Vandecappelle (Essensium/Mind)
<arnout@mind.be> wrote:
> The top-level Makefile contains an "override O := $(O)" statement that
> is purportedly required to make sure the O flag doesn't leak into the
> environment of sub-makes. However, since commit 173135d, there is
> already an "override O := ..." a few lines down. Therefore, the first
> override is redundant.
>
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

Reviewed-by: Samuel Martin <s.martin49@gmail.com>

Regards,

-- 
Samuel

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

* [Buildroot] [PATCH 1/2] core: don't reset MAKEOVERRIDES when re-entering make
  2016-11-03  1:55 [Buildroot] [PATCH 1/2] core: don't reset MAKEOVERRIDES when re-entering make Arnout Vandecappelle
  2016-11-03  1:55 ` [Buildroot] [PATCH 2/2] core: remove redundant "override O := $(O)" Arnout Vandecappelle
@ 2016-11-03  7:48 ` Samuel Martin
  2016-11-03 17:58   ` Maxime Hadjinlian
  2016-11-03 20:49 ` Thomas Petazzoni
  2 siblings, 1 reply; 6+ messages in thread
From: Samuel Martin @ 2016-11-03  7:48 UTC (permalink / raw)
  To: buildroot

On Thu, Nov 3, 2016 at 2:55 AM, Arnout Vandecappelle (Essensium/Mind)
<arnout@mind.be> wrote:
> We reset MAKEOVERRIDES to avoid passing down variables that are
> overridden on the command line to the package build systems. Indeed,
> the variables overridden on the command line will be Buildroot
> variables and not relevant to the package build system. In particular
> the O option is used by some packages and the value passed in on the
> command line is plain wrong for the individual package.
>
> However, in commit 916e614b, MAKEOVERRIDES was moved earlier and it
> was reset _before_ re-entering make in the cases when something has
> to be fixed up (incorrect umask, non-absolute paths in O or CURDIR).
> Therefore, if make is re-entered, any command line overrides are lost.
>
> This particularly bites the autobuilders, because they use
> O=<relative path> to specify the output directory, and they add
> BR2_JLEVEL=... to avoid starting too many jobs in parallel. The
> BR2_JLEVEL override is lost.
>
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> Cc: Samuel Martin <s.martin49@gmail.com>
> Cc: Yann E. MORIN <yann.morin.1998@free.fr>
> ---
> I have a little worry, because there are two other places where we re-
> enter top-level make and those are not handled: silentoldconfig and
> external-deps. BR2_JLEVEL is not relevant for those, but other command-
> line overridden options may be. However, this hasn't changed from before
> commit 916e614b so I guess it's OK.


Reviewed-by: Samuel Martin <s.martin49@gmail.com>

Regards,

-- 
Samuel

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

* [Buildroot] [PATCH 1/2] core: don't reset MAKEOVERRIDES when re-entering make
  2016-11-03  7:48 ` [Buildroot] [PATCH 1/2] core: don't reset MAKEOVERRIDES when re-entering make Samuel Martin
@ 2016-11-03 17:58   ` Maxime Hadjinlian
  0 siblings, 0 replies; 6+ messages in thread
From: Maxime Hadjinlian @ 2016-11-03 17:58 UTC (permalink / raw)
  To: buildroot

Tested-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>

On Thu, Nov 3, 2016 at 8:48 AM, Samuel Martin <s.martin49@gmail.com> wrote:
> On Thu, Nov 3, 2016 at 2:55 AM, Arnout Vandecappelle (Essensium/Mind)
> <arnout@mind.be> wrote:
>> We reset MAKEOVERRIDES to avoid passing down variables that are
>> overridden on the command line to the package build systems. Indeed,
>> the variables overridden on the command line will be Buildroot
>> variables and not relevant to the package build system. In particular
>> the O option is used by some packages and the value passed in on the
>> command line is plain wrong for the individual package.
>>
>> However, in commit 916e614b, MAKEOVERRIDES was moved earlier and it
>> was reset _before_ re-entering make in the cases when something has
>> to be fixed up (incorrect umask, non-absolute paths in O or CURDIR).
>> Therefore, if make is re-entered, any command line overrides are lost.
>>
>> This particularly bites the autobuilders, because they use
>> O=<relative path> to specify the output directory, and they add
>> BR2_JLEVEL=... to avoid starting too many jobs in parallel. The
>> BR2_JLEVEL override is lost.
>>
>> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
>> Cc: Samuel Martin <s.martin49@gmail.com>
>> Cc: Yann E. MORIN <yann.morin.1998@free.fr>
>> ---
>> I have a little worry, because there are two other places where we re-
>> enter top-level make and those are not handled: silentoldconfig and
>> external-deps. BR2_JLEVEL is not relevant for those, but other command-
>> line overridden options may be. However, this hasn't changed from before
>> commit 916e614b so I guess it's OK.
>
>
> Reviewed-by: Samuel Martin <s.martin49@gmail.com>
>
> Regards,
>
> --
> Samuel
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

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

* [Buildroot] [PATCH 1/2] core: don't reset MAKEOVERRIDES when re-entering make
  2016-11-03  1:55 [Buildroot] [PATCH 1/2] core: don't reset MAKEOVERRIDES when re-entering make Arnout Vandecappelle
  2016-11-03  1:55 ` [Buildroot] [PATCH 2/2] core: remove redundant "override O := $(O)" Arnout Vandecappelle
  2016-11-03  7:48 ` [Buildroot] [PATCH 1/2] core: don't reset MAKEOVERRIDES when re-entering make Samuel Martin
@ 2016-11-03 20:49 ` Thomas Petazzoni
  2 siblings, 0 replies; 6+ messages in thread
From: Thomas Petazzoni @ 2016-11-03 20:49 UTC (permalink / raw)
  To: buildroot

Hello,

On Thu, 3 Nov 2016 02:55:16 +0100, Arnout Vandecappelle
(Essensium/Mind) wrote:
> We reset MAKEOVERRIDES to avoid passing down variables that are
> overridden on the command line to the package build systems. Indeed,
> the variables overridden on the command line will be Buildroot
> variables and not relevant to the package build system. In particular
> the O option is used by some packages and the value passed in on the
> command line is plain wrong for the individual package.
> 
> However, in commit 916e614b, MAKEOVERRIDES was moved earlier and it
> was reset _before_ re-entering make in the cases when something has
> to be fixed up (incorrect umask, non-absolute paths in O or CURDIR).
> Therefore, if make is re-entered, any command line overrides are lost.
> 
> This particularly bites the autobuilders, because they use
> O=<relative path> to specify the output directory, and they add
> BR2_JLEVEL=... to avoid starting too many jobs in parallel. The
> BR2_JLEVEL override is lost.
> 
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> Cc: Samuel Martin <s.martin49@gmail.com>
> Cc: Yann E. MORIN <yann.morin.1998@free.fr>
> ---
> I have a little worry, because there are two other places where we re-
> enter top-level make and those are not handled: silentoldconfig and
> external-deps. BR2_JLEVEL is not relevant for those, but other command-
> line overridden options may be. However, this hasn't changed from before
> commit 916e614b so I guess it's OK.
> ---
>  Makefile | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)

Both applied, thanks!

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

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

end of thread, other threads:[~2016-11-03 20:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-03  1:55 [Buildroot] [PATCH 1/2] core: don't reset MAKEOVERRIDES when re-entering make Arnout Vandecappelle
2016-11-03  1:55 ` [Buildroot] [PATCH 2/2] core: remove redundant "override O := $(O)" Arnout Vandecappelle
2016-11-03  7:45   ` Samuel Martin
2016-11-03  7:48 ` [Buildroot] [PATCH 1/2] core: don't reset MAKEOVERRIDES when re-entering make Samuel Martin
2016-11-03 17:58   ` Maxime Hadjinlian
2016-11-03 20:49 ` Thomas Petazzoni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox