1. Patchew
  2. Xen
  3. [XEN PATCH v2 0/5] build: reduce number of $(shell) execution on make 4.4
  4. View patch

[XEN PATCH v2 4/5] build: evaluate XEN_BUILD_* and XEN_DOMAIN on first use

Anthony PERARD posted 5 patches 2 years, 7 months ago
There is a newer version of this series
[XEN PATCH v2 4/5] build: evaluate XEN_BUILD_* and XEN_DOMAIN on first use
Posted by Anthony PERARD 2 years, 7 months ago 
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 necessery, we
will use a construct to evaluate on first use.

Reported-by: Jason Andryuk <jandryuk@gmail.com>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Tested-by: Jason Andryuk <jandryuk@gmail.com>
---
 xen/Makefile | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/xen/Makefile b/xen/Makefile
index ac2765050b..e3b1468f83 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -11,10 +11,10 @@ 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)
+export XEN_DOMAIN	?= $(eval XEN_DOMAIN := $(shell ([ -x /bin/dnsdomainname ] && /bin/dnsdomainname) || ([ -x /bin/domainname ] && /bin/domainname || echo [unknown])))$(XEN_DOMAIN)
+export XEN_BUILD_DATE	?= $(eval XEN_BUILD_DATE := $(shell LC_ALL=C date))$(XEN_BUILD_DATE)
+export XEN_BUILD_TIME	?= $(eval XEN_BUILD_TIME := $(shell LC_ALL=C date +%T))$(XEN_BUILD_TIME)
+export XEN_BUILD_HOST	?= $(eval XEN_BUILD_HOST := $(shell hostname))$(XEN_BUILD_HOST)
 
 # 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
Re: [XEN PATCH v2 4/5] build: evaluate XEN_BUILD_* and XEN_DOMAIN on first use
Posted by Jan Beulich 2 years, 7 months ago 
On 22.06.2023 17:30, Anthony PERARD wrote:
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -11,10 +11,10 @@ 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)
> +export XEN_DOMAIN	?= $(eval XEN_DOMAIN := $(shell ([ -x /bin/dnsdomainname ] && /bin/dnsdomainname) || ([ -x /bin/domainname ] && /bin/domainname || echo [unknown])))$(XEN_DOMAIN)
> +export XEN_BUILD_DATE	?= $(eval XEN_BUILD_DATE := $(shell LC_ALL=C date))$(XEN_BUILD_DATE)
> +export XEN_BUILD_TIME	?= $(eval XEN_BUILD_TIME := $(shell LC_ALL=C date +%T))$(XEN_BUILD_TIME)
> +export XEN_BUILD_HOST	?= $(eval XEN_BUILD_HOST := $(shell hostname))$(XEN_BUILD_HOST)

Interesting approach. It looks like it should be independent of make's
internal workings, but I still wonder: Is this documented somewhere,
so we won't be caught by surprise of it not working anymore because of
some change to make's internals?

Jan
Re: [XEN PATCH v2 4/5] build: evaluate XEN_BUILD_* and XEN_DOMAIN on first use
Posted by Anthony PERARD 2 years, 6 months ago 
On Fri, Jun 23, 2023 at 10:07:02AM +0200, Jan Beulich wrote:
> On 22.06.2023 17:30, Anthony PERARD wrote:
> > --- a/xen/Makefile
> > +++ b/xen/Makefile
> > @@ -11,10 +11,10 @@ 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)
> > +export XEN_DOMAIN	?= $(eval XEN_DOMAIN := $(shell ([ -x /bin/dnsdomainname ] && /bin/dnsdomainname) || ([ -x /bin/domainname ] && /bin/domainname || echo [unknown])))$(XEN_DOMAIN)
> > +export XEN_BUILD_DATE	?= $(eval XEN_BUILD_DATE := $(shell LC_ALL=C date))$(XEN_BUILD_DATE)
> > +export XEN_BUILD_TIME	?= $(eval XEN_BUILD_TIME := $(shell LC_ALL=C date +%T))$(XEN_BUILD_TIME)
> > +export XEN_BUILD_HOST	?= $(eval XEN_BUILD_HOST := $(shell hostname))$(XEN_BUILD_HOST)
> 
> Interesting approach. It looks like it should be independent of make's
> internal workings, but I still wonder: Is this documented somewhere,
> so we won't be caught by surprise of it not working anymore because of
> some change to make's internals?

So, this is a makefile trick that I've seen on someone's blog post.

But I tried to find evidence in the GNU make manual if variable expansion is
expected to work like that, and I can't. So I can imagine one day make
doing expansion of variable in parallel, or were the result of the eval
would happen only on the next line. So I don't know if this approach is
always going to work.

Maybe we could go for a safer alternative:

Simply replacing ?= by something actually documented in the manual, and
then replacing = by := .

    ifeq ($(origin XEN_BUILD_DATE), undefined)
    export XEN_BUILD_DATE := $(shell LC_ALL=C date)
    endif

An alternative, with a macro could be:

    set-shell-if-undef = $(if $(filter undefined,$(origin $(1))),$(eval $(1) := $(shell $(2))))

    $(call set-shell-if-undef,XEN_BUILD_DATE,LC_ALL=C date)
    export XEN_BUILD_DATE

But this kind of hide that a shell command is been called. But having
$(shell) as parameter of call $(call) mean the shell command is always
called even when unneeded.

But then, with the macro, I'm not sure where to put it, to be able to
use it here and in Config.mk for the next patch, another file in
xen.git/config/*.mk ?

Should I replace all the eval with "ifeq (); endif" ?

Thanks,

-- 
Anthony PERARD
Re: [XEN PATCH v2 4/5] build: evaluate XEN_BUILD_* and XEN_DOMAIN on first use
Posted by Jan Beulich 2 years, 6 months ago 
On 27.07.2023 11:15, Anthony PERARD wrote:
> On Fri, Jun 23, 2023 at 10:07:02AM +0200, Jan Beulich wrote:
>> On 22.06.2023 17:30, Anthony PERARD wrote:
>>> --- a/xen/Makefile
>>> +++ b/xen/Makefile
>>> @@ -11,10 +11,10 @@ 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)
>>> +export XEN_DOMAIN	?= $(eval XEN_DOMAIN := $(shell ([ -x /bin/dnsdomainname ] && /bin/dnsdomainname) || ([ -x /bin/domainname ] && /bin/domainname || echo [unknown])))$(XEN_DOMAIN)
>>> +export XEN_BUILD_DATE	?= $(eval XEN_BUILD_DATE := $(shell LC_ALL=C date))$(XEN_BUILD_DATE)
>>> +export XEN_BUILD_TIME	?= $(eval XEN_BUILD_TIME := $(shell LC_ALL=C date +%T))$(XEN_BUILD_TIME)
>>> +export XEN_BUILD_HOST	?= $(eval XEN_BUILD_HOST := $(shell hostname))$(XEN_BUILD_HOST)
>>
>> Interesting approach. It looks like it should be independent of make's
>> internal workings, but I still wonder: Is this documented somewhere,
>> so we won't be caught by surprise of it not working anymore because of
>> some change to make's internals?
> 
> So, this is a makefile trick that I've seen on someone's blog post.
> 
> But I tried to find evidence in the GNU make manual if variable expansion is
> expected to work like that, and I can't. So I can imagine one day make
> doing expansion of variable in parallel, or were the result of the eval
> would happen only on the next line. So I don't know if this approach is
> always going to work.
> 
> Maybe we could go for a safer alternative:
> 
> Simply replacing ?= by something actually documented in the manual, and
> then replacing = by := .
> 
>     ifeq ($(origin XEN_BUILD_DATE), undefined)
>     export XEN_BUILD_DATE := $(shell LC_ALL=C date)
>     endif
> 
> An alternative, with a macro could be:
> 
>     set-shell-if-undef = $(if $(filter undefined,$(origin $(1))),$(eval $(1) := $(shell $(2))))
> 
>     $(call set-shell-if-undef,XEN_BUILD_DATE,LC_ALL=C date)
>     export XEN_BUILD_DATE
> 
> But this kind of hide that a shell command is been called. But having
> $(shell) as parameter of call $(call) mean the shell command is always
> called even when unneeded.
> 
> But then, with the macro, I'm not sure where to put it, to be able to
> use it here and in Config.mk for the next patch, another file in
> xen.git/config/*.mk ?
> 
> Should I replace all the eval with "ifeq (); endif" ?

I think that would be best (and I prefer that form anyway for being more
clear as to what it does).

Jan

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.