Re: [PATCH 4 of 5] xen/makefile: Allow XEN_CHANGESET to be set externally

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 01/08/12 08:49, Jan Beulich wrote:
>>>> On 31.07.12 at 18:55, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:
>> Andrew Cooper writes ("[PATCH 4 of 5] xen/makefile: Allow XEN_CHANGESET to be 
>> set externally"):
>> ..
>>> diff -r 6db5c184a777 -r ae32690d0d74 xen/Makefile
>>> --- a/xen/Makefile
>>> +++ b/xen/Makefile
>>> @@ -13,6 +13,7 @@ export BASEDIR := $(CURDIR)
>>>  export XEN_ROOT := $(BASEDIR)/..
>>>  
>>>  EFI_MOUNTPOINT ?= /boot/efi
>>> +XEN_CHANGESET  ?= $(shell hg root &> /dev/null && hg parents --template 
>> "{date|date} {rev}:{node|short}" || echo "unavailable" )
>>>  
>>>  .PHONY: default
>>>  default: build
>>> @@ -107,7 +108,7 @@ include/xen/compile.h: include/xen/compi
>>>  	    -e 's/@@version@@/$(XEN_VERSION)/g' \
>>>  	    -e 's/@@subversion@@/$(XEN_SUBVERSION)/g' \
>>>  	    -e 's/@@extraversion@@/$(XEN_EXTRAVERSION)/g' \
>>> -	    -e 's!@@changeset@@!$(shell ((hg parents --template "{date|date} 
>> {rev}:{node|short}" >/dev/null && hg parents --template "{date|date} 
>> {rev}:{node|short}") || echo "unavailable") 2>/dev/null)!g' \
>>> +	    -e 's!@@changeset@@!$(XEN_CHANGESET)!g' \
>>>  	    < include/xen/compile.h.in > $@.new
>>>  	@grep \" .banner >> $@.new
>>>  	@grep -v \" .banner
>> We need to check how many times, and at which point, this gets
>> executed, when this patch is applied.  I think it's OK...
> If in doubt, perhaps the better option would be to execute this
> once unconditionally (via := assignment to a helper variable), or
> to use := here but frame the assignment with some if construct
> excluding it to be executed when the changeset was already
> specified (assuming that an empty specification is pointless,
> simply checking for it to be non-empty should suffice).
>
> Jan
>

The rule looks like:

# compile.h contains dynamic build info. Rebuilt on every 'make'
invocation.                                                                                                                                  

include/xen/compile.h: include/xen/compile.h.in .banner
        @sed -e 's/@@date@@/$(shell LC_ALL=C date)/g' \

So it should only be executed once (unless someone is messing around
deleting compile.h)

Having said that, even if it were executed more than once, there is no
reasonable circumstance during which the contents of XEN_CHANGESET
should change.

-- 
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com