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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 31.07.12 at 18:55, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx> 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


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel