[PATCH] build: tweak variable exporting for make 3.82

[PATCH] build: tweak variable exporting for make 3.82

[Jan Beulich]

While I've been running into an issue here only because of an additional
local change I'm carrying, to be able to override just the compiler in
$(XEN_ROOT)/.config (rather than the whole tool chain), in
config/StdGNU.mk:

ifeq ($(filter-out default undefined,$(origin CC)),)

I'd like to propose to nevertheless correct the underlying issue:
Exporting an unset variable changes its origin from "undefined" to
"file". This comes into effect because of our adding of -rR to
MAKEFLAGS, which make 3.82 wrongly applies also upon re-invoking itself
after having updated auto.conf{,.cmd}.

Move the export statement past $(XEN_ROOT)/config/$(XEN_OS).mk inclusion
such that the variables already have their designated values at that
point, while retaining their initial origin up to the point they get
defined.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/Makefile
+++ b/xen/Makefile
@@ -17,8 +17,6 @@ export XEN_BUILD_HOST	?= $(shell hostnam
 PYTHON_INTERPRETER	:= $(word 1,$(shell which python3 python python2 2>/dev/null) python)
 export PYTHON		?= $(PYTHON_INTERPRETER)
 
-export CC CXX LD
-
 export BASEDIR := $(CURDIR)
 export XEN_ROOT := $(BASEDIR)/..
 
@@ -42,6 +40,8 @@ export TARGET_ARCH     := $(shell echo $
 # Allow someone to change their config file
 export KCONFIG_CONFIG ?= .config
 
+export CC CXX LD
+
 .PHONY: default
 default: build
 

Re: [PATCH] build: tweak variable exporting for make 3.82

[Bertrand Marquis]

Hi Jan,

> On 26 Jun 2020, at 16:02, Jan Beulich <jbeulich@suse.com> wrote:
> 
> While I've been running into an issue here only because of an additional
> local change I'm carrying, to be able to override just the compiler in
> $(XEN_ROOT)/.config (rather than the whole tool chain), in
> config/StdGNU.mk:
> 
> ifeq ($(filter-out default undefined,$(origin CC)),)
> 
> I'd like to propose to nevertheless correct the underlying issue:
> Exporting an unset variable changes its origin from "undefined" to
> "file". This comes into effect because of our adding of -rR to
> MAKEFLAGS, which make 3.82 wrongly applies also upon re-invoking itself
> after having updated auto.conf{,.cmd}.
> 
> Move the export statement past $(XEN_ROOT)/config/$(XEN_OS).mk inclusion
> such that the variables already have their designated values at that
> point, while retaining their initial origin up to the point they get
> defined.

If I understand correctly you actually need this to be after 
include $(XEN_ROOT)/Config.mk

Which actually includes the .config and the StdGNU.mk
Maybe you could say this as $(XEN_ROOT)/config/$(XEN_OS).mk is not actually included directly in the Makefile itself ?

I tested the patch and it works on arm and x86 on my side.

> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Bertrand Marquis <bertrand.marquis@arm.com>

> 
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -17,8 +17,6 @@ export XEN_BUILD_HOST	?= $(shell hostnam
> PYTHON_INTERPRETER	:= $(word 1,$(shell which python3 python python2 2>/dev/null) python)
> export PYTHON		?= $(PYTHON_INTERPRETER)
> 
> -export CC CXX LD
> -
> export BASEDIR := $(CURDIR)
> export XEN_ROOT := $(BASEDIR)/..
> 
> @@ -42,6 +40,8 @@ export TARGET_ARCH     := $(shell echo $
> # Allow someone to change their config file
> export KCONFIG_CONFIG ?= .config
> 
> +export CC CXX LD
> +
> .PHONY: default
> default: build
> 
> 

Re: [PATCH] build: tweak variable exporting for make 3.82

[Jan Beulich]

On 26.06.2020 17:32, Bertrand Marquis wrote:
> Hi Jan,
> 
>> On 26 Jun 2020, at 16:02, Jan Beulich <jbeulich@suse.com> wrote:
>>
>> While I've been running into an issue here only because of an additional
>> local change I'm carrying, to be able to override just the compiler in
>> $(XEN_ROOT)/.config (rather than the whole tool chain), in
>> config/StdGNU.mk:
>>
>> ifeq ($(filter-out default undefined,$(origin CC)),)
>>
>> I'd like to propose to nevertheless correct the underlying issue:
>> Exporting an unset variable changes its origin from "undefined" to
>> "file". This comes into effect because of our adding of -rR to
>> MAKEFLAGS, which make 3.82 wrongly applies also upon re-invoking itself
>> after having updated auto.conf{,.cmd}.
>>
>> Move the export statement past $(XEN_ROOT)/config/$(XEN_OS).mk inclusion
>> such that the variables already have their designated values at that
>> point, while retaining their initial origin up to the point they get
>> defined.
> 
> If I understand correctly you actually need this to be after 
> include $(XEN_ROOT)/Config.mk
> 
> Which actually includes the .config and the StdGNU.mk
> Maybe you could say this as $(XEN_ROOT)/config/$(XEN_OS).mk is not
> actually included directly in the Makefile itself ?

I thought it would be obvious enough, but since you ask, I've added
half a sentence.

> I tested the patch and it works on arm and x86 on my side.
> 
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Tested-by: Bertrand Marquis <bertrand.marquis@arm.com>

Thanks much.

Jan

Re: [PATCH] build: tweak variable exporting for make 3.82

[Anthony PERARD]

On Fri, Jun 26, 2020 at 05:02:30PM +0200, Jan Beulich wrote:
> While I've been running into an issue here only because of an additional
> local change I'm carrying, to be able to override just the compiler in
> $(XEN_ROOT)/.config (rather than the whole tool chain), in
> config/StdGNU.mk:
> 
> ifeq ($(filter-out default undefined,$(origin CC)),)
> 
> I'd like to propose to nevertheless correct the underlying issue:
> Exporting an unset variable changes its origin from "undefined" to
> "file". This comes into effect because of our adding of -rR to
> MAKEFLAGS, which make 3.82 wrongly applies also upon re-invoking itself
> after having updated auto.conf{,.cmd}.
> 
> Move the export statement past $(XEN_ROOT)/config/$(XEN_OS).mk inclusion
> such that the variables already have their designated values at that
> point, while retaining their initial origin up to the point they get
> defined.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -17,8 +17,6 @@ export XEN_BUILD_HOST	?= $(shell hostnam
>  PYTHON_INTERPRETER	:= $(word 1,$(shell which python3 python python2 2>/dev/null) python)
>  export PYTHON		?= $(PYTHON_INTERPRETER)
>  
> -export CC CXX LD
> -
>  export BASEDIR := $(CURDIR)
>  export XEN_ROOT := $(BASEDIR)/..
>  
> @@ -42,6 +40,8 @@ export TARGET_ARCH     := $(shell echo $
>  # Allow someone to change their config file
>  export KCONFIG_CONFIG ?= .config
>  
> +export CC CXX LD
> +
>  .PHONY: default
>  default: build

That patch is fine and it is probably better to export a variable that
has a value rather than before the variable is set.

Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks,

-- 
Anthony PERARD

Ping: [PATCH] build: tweak variable exporting for make 3.82

[Jan Beulich]

On 29.06.2020 18:30, Anthony PERARD wrote:
> On Fri, Jun 26, 2020 at 05:02:30PM +0200, Jan Beulich wrote:
>> While I've been running into an issue here only because of an additional
>> local change I'm carrying, to be able to override just the compiler in
>> $(XEN_ROOT)/.config (rather than the whole tool chain), in
>> config/StdGNU.mk:
>>
>> ifeq ($(filter-out default undefined,$(origin CC)),)
>>
>> I'd like to propose to nevertheless correct the underlying issue:
>> Exporting an unset variable changes its origin from "undefined" to
>> "file". This comes into effect because of our adding of -rR to
>> MAKEFLAGS, which make 3.82 wrongly applies also upon re-invoking itself
>> after having updated auto.conf{,.cmd}.
>>
>> Move the export statement past $(XEN_ROOT)/config/$(XEN_OS).mk inclusion
>> such that the variables already have their designated values at that
>> point, while retaining their initial origin up to the point they get
>> defined.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> --- a/xen/Makefile
>> +++ b/xen/Makefile
>> @@ -17,8 +17,6 @@ export XEN_BUILD_HOST	?= $(shell hostnam
>>  PYTHON_INTERPRETER	:= $(word 1,$(shell which python3 python python2 2>/dev/null) python)
>>  export PYTHON		?= $(PYTHON_INTERPRETER)
>>  
>> -export CC CXX LD
>> -
>>  export BASEDIR := $(CURDIR)
>>  export XEN_ROOT := $(BASEDIR)/..
>>  
>> @@ -42,6 +40,8 @@ export TARGET_ARCH     := $(shell echo $
>>  # Allow someone to change their config file
>>  export KCONFIG_CONFIG ?= .config
>>  
>> +export CC CXX LD
>> +
>>  .PHONY: default
>>  default: build
> 
> That patch is fine and it is probably better to export a variable that
> has a value rather than before the variable is set.
> 
> Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>

Paul - thoughts either way as to 4.14? If not to go in now, I
definitely intend to backport it. (And in fact I'm meanwhile
considering to enter a make bug for the behavior, unless its
behavior has changed in later versions.)

Thanks, Jan

RE: Ping: [PATCH] build: tweak variable exporting for make 3.82

[Paul Durrant]

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 02 July 2020 08:45
> To: Paul Durrant <paul@xen.org>
> Cc: Anthony PERARD <anthony.perard@citrix.com>; xen-devel@lists.xenproject.org; Andrew Cooper
> <andrew.cooper3@citrix.com>; George Dunlap <George.Dunlap@eu.citrix.com>; Ian Jackson
> <ian.jackson@citrix.com>; Julien Grall <julien@xen.org>; Wei Liu <wl@xen.org>; Stefano Stabellini
> <sstabellini@kernel.org>
> Subject: Ping: [PATCH] build: tweak variable exporting for make 3.82
> 
> On 29.06.2020 18:30, Anthony PERARD wrote:
> > On Fri, Jun 26, 2020 at 05:02:30PM +0200, Jan Beulich wrote:
> >> While I've been running into an issue here only because of an additional
> >> local change I'm carrying, to be able to override just the compiler in
> >> $(XEN_ROOT)/.config (rather than the whole tool chain), in
> >> config/StdGNU.mk:
> >>
> >> ifeq ($(filter-out default undefined,$(origin CC)),)
> >>
> >> I'd like to propose to nevertheless correct the underlying issue:
> >> Exporting an unset variable changes its origin from "undefined" to
> >> "file". This comes into effect because of our adding of -rR to
> >> MAKEFLAGS, which make 3.82 wrongly applies also upon re-invoking itself
> >> after having updated auto.conf{,.cmd}.
> >>
> >> Move the export statement past $(XEN_ROOT)/config/$(XEN_OS).mk inclusion
> >> such that the variables already have their designated values at that
> >> point, while retaining their initial origin up to the point they get
> >> defined.
> >>
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >>
> >> --- a/xen/Makefile
> >> +++ b/xen/Makefile
> >> @@ -17,8 +17,6 @@ export XEN_BUILD_HOST	?= $(shell hostnam
> >>  PYTHON_INTERPRETER	:= $(word 1,$(shell which python3 python python2 2>/dev/null) python)
> >>  export PYTHON		?= $(PYTHON_INTERPRETER)
> >>
> >> -export CC CXX LD
> >> -
> >>  export BASEDIR := $(CURDIR)
> >>  export XEN_ROOT := $(BASEDIR)/..
> >>
> >> @@ -42,6 +40,8 @@ export TARGET_ARCH     := $(shell echo $
> >>  # Allow someone to change their config file
> >>  export KCONFIG_CONFIG ?= .config
> >>
> >> +export CC CXX LD
> >> +
> >>  .PHONY: default
> >>  default: build
> >
> > That patch is fine and it is probably better to export a variable that
> > has a value rather than before the variable is set.
> >
> > Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
> 
> Paul - thoughts either way as to 4.14? If not to go in now, I
> definitely intend to backport it. (And in fact I'm meanwhile
> considering to enter a make bug for the behavior, unless its
> behavior has changed in later versions.)
> 

I agree with Anthony's statement so I'm happy for this to go in 4.14.

Release-acked-by: Paul Durrant <paul@xen.org>