[XEN PATCH v3 0/2] build: reduce number of $(shell) execution on make 4.4

[XEN PATCH v3 0/2] build: reduce number of $(shell) execution on make 4.4

[Anthony PERARD]

Patch series available in this git branch:
https://xenbits.xen.org/git-http/people/aperard/xen-unstable.git br.build-exported-shell-command-value-v3

v3:
- replace evaluation on first use construct by immediate expansion.

v2:
- new patches removing TARGET_SUBARCH and TARGET_ARCH.
- style change in first patch

With GNU make 4.4, the number of execution of the command present in $(shell )
in exported variables increased greatly. This is probably because as of make
4.4, exported variable are also added to the environment of $(shell )
construct.

From the annoncement:

    https://lists.gnu.org/archive/html/info-gnu/2022-10/msg00008.html
    > * WARNING: Backward-incompatibility!
    >   Previously makefile variables marked as export were not exported to commands
    >   started by the $(shell ...) function.  Now, all exported variables are
    >   exported to $(shell ...).  If this leads to recursion during expansion, then
    >   for backward-compatibility the value from the original environment is used.
    >   To detect this change search for 'shell-export' in the .FEATURES variable.

Also, there's a new paragraph in the GNU make manual, but it's a warning about
exporting all variable, still it might be relevant to the new observed
behavior:

    https://www.gnu.org/software/make/manual/make.html#Variables_002fRecursion
    > Adding a variable’s value to the environment requires it to be expanded. If
    > expanding a variable has side-effects (such as the info or eval or similar
    > functions) then these side-effects will be seen every time a command is
    > invoked.

The issue was reported on IRC by jandryuk.

So, I've locate a few obvious candidate to fix, maybe there's more $(shell) to
change?

Anthony PERARD (2):
  build: evaluate XEN_BUILD_* and XEN_DOMAIN immediately
  Config.mk: evaluate XEN_COMPILE_ARCH and XEN_OS immediately

 Config.mk    |  8 ++++++--
 xen/Makefile | 16 ++++++++++++----
 2 files changed, 18 insertions(+), 6 deletions(-)

-- 
Anthony PERARD

[XEN PATCH v3 1/2] build: evaluate XEN_BUILD_* and XEN_DOMAIN immediately

[Anthony PERARD]

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:15: not recursively expanding XEN_BUILD_DATE to export to shell function
    Makefile:16: not recursively expanding XEN_BUILD_TIME to export to shell function
    Makefile:17: not recursively expanding XEN_BUILD_HOST to export to shell function
    Makefile:14: not recursively expanding XEN_DOMAIN to export to shell function

So to avoid having these command been run more than necessary, we
will replace ?= by an equivalent but with immediate expansion.

Reported-by: Jason Andryuk <jandryuk@gmail.com>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---

Notes:
    v3:
    - replace evaluation on first use construct by immediate expansion which
      isn't likely to break and is clearer.

 xen/Makefile | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/xen/Makefile b/xen/Makefile
index e8aa663781..c1738dbbde 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -11,10 +11,18 @@ export XEN_FULLVERSION   = $(XEN_VERSION).$(XEN_SUBVERSION)$(XEN_EXTRAVERSION)
 -include xen-version
 
 export XEN_WHOAMI	?= $(USER)
-export XEN_DOMAIN	?= $(shell ([ -x /bin/dnsdomainname ] && /bin/dnsdomainname) || ([ -x /bin/domainname ] && /bin/domainname || echo [unknown]))
-export XEN_BUILD_DATE	?= $(shell LC_ALL=C date)
-export XEN_BUILD_TIME	?= $(shell LC_ALL=C date +%T)
-export XEN_BUILD_HOST	?= $(shell hostname)
+ifeq ($(origin XEN_DOMAIN), undefined)
+export XEN_DOMAIN	:= $(shell ([ -x /bin/dnsdomainname ] && /bin/dnsdomainname) || ([ -x /bin/domainname ] && /bin/domainname || echo [unknown]))
+endif
+ifeq ($(origin XEN_BUILD_DATE), undefined)
+export XEN_BUILD_DATE	:= $(shell LC_ALL=C date)
+endif
+ifeq ($(origin XEN_BUILD_TIME), undefined)
+export XEN_BUILD_TIME	:= $(shell LC_ALL=C date +%T)
+endif
+ifeq ($(origin XEN_BUILD_HOST), undefined)
+export XEN_BUILD_HOST	:= $(shell hostname)
+endif
 
 # Best effort attempt to find a python interpreter, defaulting to Python 3 if
 # available.  Fall back to just `python` if `which` is nowhere to be found.
-- 
Anthony PERARD

[XEN PATCH v3 2/2] Config.mk: evaluate XEN_COMPILE_ARCH and XEN_OS immediately

[Anthony PERARD]

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.

So to avoid having these command been run more than necessary, we
will replace ?= by an equivalent but with immediate expansion.

Reported-by: Jason Andryuk <jandryuk@gmail.com>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---

Notes:
    v3:
    - replace evaluation on first use construct by immediate expansion which
      isn't likely to break and is clearer.

 Config.mk | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/Config.mk b/Config.mk
index c529b1ba19..dc3afaa47e 100644
--- a/Config.mk
+++ b/Config.mk
@@ -19,13 +19,17 @@ or       = $(if $(strip $(1)),$(1),$(if $(strip $(2)),$(2),$(if $(strip $(3)),$(
 
 -include $(XEN_ROOT)/.config
 
-XEN_COMPILE_ARCH    ?= $(shell uname -m | sed -e s/i.86/x86_32/ \
+ifeq ($(origin XEN_COMPILE_ARCH), undefined)
+XEN_COMPILE_ARCH    := $(shell uname -m | sed -e s/i.86/x86_32/ \
                          -e s/i86pc/x86_32/ -e s/amd64/x86_64/ \
                          -e s/armv7.*/arm32/ -e s/armv8.*/arm64/ \
                          -e s/aarch64/arm64/)
+endif
 
 XEN_TARGET_ARCH     ?= $(XEN_COMPILE_ARCH)
-XEN_OS              ?= $(shell uname -s)
+ifeq ($(origin XEN_OS), undefined)
+XEN_OS              := $(shell uname -s)
+endif
 
 CONFIG_$(XEN_OS) := y
 
-- 
Anthony PERARD

Re: [XEN PATCH v3 0/2] build: reduce number of $(shell) execution on make 4.4

[Jason Andryuk]

On Thu, Jul 27, 2023 at 10:55 AM Anthony PERARD
<anthony.perard@citrix.com> wrote:
> Anthony PERARD (2):
>   build: evaluate XEN_BUILD_* and XEN_DOMAIN immediately
>   Config.mk: evaluate XEN_COMPILE_ARCH and XEN_OS immediately

For the series:
Tested-by: Jason Andryuk <jandryuk@gmail.com>

Thanks,
Jason

Re: [XEN PATCH v3 1/2] build: evaluate XEN_BUILD_* and XEN_DOMAIN immediately

[Jan Beulich]

On 27.07.2023 16:54, Anthony PERARD wrote:
> 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:15: not recursively expanding XEN_BUILD_DATE to export to shell function
>     Makefile:16: not recursively expanding XEN_BUILD_TIME to export to shell function
>     Makefile:17: not recursively expanding XEN_BUILD_HOST to export to shell function
>     Makefile:14: not recursively expanding XEN_DOMAIN to export to shell function
> 
> So to avoid having these command been run more than necessary, we
> will replace ?= by an equivalent but with immediate expansion.
> 
> Reported-by: Jason Andryuk <jandryuk@gmail.com>
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

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

Re: [XEN PATCH v3 2/2] Config.mk: evaluate XEN_COMPILE_ARCH and XEN_OS immediately

[Jan Beulich]

On 27.07.2023 16:54, Anthony PERARD wrote:
> 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.
> 
> So to avoid having these command been run more than necessary, we
> will replace ?= by an equivalent but with immediate expansion.
> 
> Reported-by: Jason Andryuk <jandryuk@gmail.com>
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

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