Re: [XEN PATCH 1/3] build: define ARCH and SRCARCH later

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.06.2023 18:25, Andrew Cooper wrote:
> On 21/06/2023 5:19 pm, Anthony PERARD wrote:
>> Defining ARCH and SRCARCH later in xen/Makefile allows to switch to
>> immediate evaluation variable type.
>>
>> ARCH and SRCARCH depends on value defined in Config.mk and aren't used
>> TARGET_SUBARCH or TARGET_ARCH, and not before it's needed in a
>> sub-make or a rule.
>>
>> This will help reduce the number of times the shell rune is been
>> run.
>>
>> With GNU make 4.4, the number of execution of the command present in
>> these $(shell ) increased greatly. This is probably because as of make
>> 4.4, exported variable are also added to the environment of $(shell )
>> construct.
>>
>> Also, `make -d` shows a lot of these:
>>     Makefile:39: not recursively expanding SRCARCH to export to shell 
>> function
>>     Makefile:38: not recursively expanding ARCH to export to shell function
>>
>> Reported-by: Jason Andryuk <jandryuk@xxxxxxxxx>
>> Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
>> ---
>>  xen/Makefile | 13 +++++++------
>>  1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/xen/Makefile b/xen/Makefile
>> index e89fc461fc4b..9631e45cfb9b 100644
>> --- a/xen/Makefile
>> +++ b/xen/Makefile
>> @@ -35,12 +35,6 @@ MAKEFLAGS += -rR
>>  
>>  EFI_MOUNTPOINT ?= $(BOOT_DIR)/efi
>>  
>> -ARCH=$(XEN_TARGET_ARCH)
>> -SRCARCH=$(shell echo $(ARCH) | \
>> -          sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g' \
>> -              -e s'/riscv.*/riscv/g')
>> -export ARCH SRCARCH
>> -
>>  # Allow someone to change their config file
>>  export KCONFIG_CONFIG ?= .config
>>  
>> @@ -241,6 +235,13 @@ include scripts/Kbuild.include
>>  include $(XEN_ROOT)/Config.mk
>>  
>>  # Set ARCH/SUBARCH appropriately.
>> +
>> +ARCH := $(XEN_TARGET_ARCH)
>> +SRCARCH := $(shell echo $(ARCH) | \
>> +          sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g' \
>> +              -e s'/riscv.*/riscv/g')
>> +export ARCH SRCARCH
>> +
>>  export TARGET_SUBARCH  := $(XEN_TARGET_ARCH)
>>  export TARGET_ARCH     := $(shell echo $(XEN_TARGET_ARCH) | \
>>                              sed -e 's/x86.*/x86/' -e 
>> s'/arm\(32\|64\)/arm/g' \
> 
> The change looks plausible to fix this issue, but could we take the
> opportunity to dedup the sed expression into a $(call src-arch ...) or so ?
> 
> Except, given that ARCH := $(XEN_TARGET_ARCH) now, doesn't that mean
> SRCARCH is always TARGET_ARCH ?
> 
> Can't we simplify this to just be plain aliases?

Or, putting it differently, do we actually need both TARGET_* values
when they match other (exported) variables anyway?

Jan