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

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

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