Re: [Xen-devel] [PATCH v2 2/8] xen: introduce XEN_COMPILE_POSIX_TIME

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 19.06.18 at 16:35, <daniel.kiper@xxxxxxxxxx> wrote:
> We need the POSIX time to properly fill the TimeDateStamp field in the PE 
> header.
> 
> Additionally, realign the variables assignment in xen/Makefile to increase 
> readability.
> 
> Signed-off-by: Daniel Kiper <daniel.kiper@xxxxxxxxxx>
> ---
> v2 - suggestions/fixes:
>    - derive XEN_COMPILE_POSIX_TIME from XEN_BUILD_DATE
>      (suggested by Jan Beulich),
>    - echo 0 if date command does not work
>      (suggested by Konrad Rzeszutek Wilk),

Why would the date command produce an error, other than for not
supporting -d? But yes, I'm fine with falling back to zero in that case.
If anyone runs into it and cares, they can submit a patch making it
work on their platform.

> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -6,12 +6,13 @@ export XEN_EXTRAVERSION ?= -rc$(XEN_VENDORVERSION)
>  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 -d '$(XEN_BUILD_DATE)' 
> +%T)
> -export XEN_BUILD_HOST        ?= $(shell hostname)
> -export XEN_CONFIG_EXPERT ?= n
> +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 -d 
> '$(XEN_BUILD_DATE)' +%T)
> +export XEN_BUILD_POSIX_TIME  ?= $(shell LC_ALL=C date -d '$(XEN_BUILD_DATE)' 
> +%s || echo 0)
> +export XEN_BUILD_HOST                ?= $(shell hostname)
> +export XEN_CONFIG_EXPERT     ?= n

To be honest I'd prefer if you avoided the re-indentation. Especially the
XEN_DOMAIN line is already pretty long, so it would seem better to me to
accept the mis-alignment the new setting will have. I continue to not be
overly happy anyway with this being put here when it's needed only in a
single place (and the transformation could presumably be easily done
there, without the need for any new global environment variable).

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel