[PATCH 0/5] tools: various changes

[PATCH 0/5] tools: various changes

[Olaf Hering]

The following series is a resend of changes I already sent earlier
this year:

* Not sure if the last discussion about mkrpm was a yes or no for
  inclusion. I find rpm -U --force dist/xen.rpm cumbersome...

* The XENSTORED_ARGS= should be fine according to IanJ

* The changes for reproducible builds are really just interesting for
  distro packages. Passing all the variables via a rpm spec file for
  example can be automated in such build environment. The current
  behaviour is preserved.

Please review once more and apply.

Thanks,
Olaf

Olaf Hering (5):
  tools/mkrpm: improve version.release handling
  tools/hotplug: introduce XENSTORED_ARGS= in sysconfig file.
  INSTALL: mention variables for reproducible builds
  hvmloader: add knob for fixed SMBIOS date string
  hvmloader: add knob for fixed VGABIOS date string

 INSTALL                                            |  8 ++++++++
 tools/firmware/hvmloader/Makefile                  |  5 ++++-
 tools/firmware/vgabios/Makefile                    |  4 ++--
 tools/hotplug/Linux/init.d/sysconfig.xencommons.in |  8 ++++++++
 tools/misc/mkrpm                                   | 12 +++++-------
 5 files changed, 27 insertions(+), 10 deletions(-)

[PATCH 1/5] tools/mkrpm: improve version.release handling

[Olaf Hering]

An increasing version and/or release number helps to update existing
packages without --force as in "rpm Uvh --force xen.rpm". Instead its
possible to do "rpm -Fvh *.rpm" to update only already installed
packages.

The usage of --force disables essentials checks such as file conflict
detection. As a result the new xen.rpm may overwrite files owned by
other packages.

With the current way of calculating version-release it is difficult to
get an increasing release number into the spec file. The release is
always zero unless "make make XEN_VENDORVERSION=`date +.%s`" is used,
which has the bad side effect that xen.gz always gets a different
filename every time.

Since the value of release has no meaning its fine to have an ever
increasing number. This could be either the number of seconds (+%s), or
some representation which could mean something to a human. The change
uses a representation of date+time.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>
---
 tools/misc/mkrpm | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/tools/misc/mkrpm b/tools/misc/mkrpm
index 9b8c6d9..b54de24 100644
--- a/tools/misc/mkrpm
+++ b/tools/misc/mkrpm
@@ -13,13 +13,11 @@ fi
 
 xenroot="$1"
 
-# rpmbuild doesn't like dashes in the version; break it down into
-# version and release.  Default to "0" if there isn't a release.
-v=(${2/-/ })
-version=${v[0]}
-release=${v[1]}
-
-[[ -n "$release" ]] || release="0"
+# rpmbuild doesn't support dashes in the version;
+version=${2//-/_}
+# Use an ever increasing release number for this devel pkg.
+# This makes sure rpm -Fvh xen$PKG_SUFFIX.rpm can be updated wihtout --force.
+release="`date +%Y%m%d%H%M%S`"
 
 cd $xenroot

Re: [PATCH 1/5] tools/mkrpm: improve version.release handling

[George Dunlap]

On 03/20/2015 05:24 PM, Olaf Hering wrote:
> An increasing version and/or release number helps to update existing
> packages without --force as in "rpm Uvh --force xen.rpm". Instead its
> possible to do "rpm -Fvh *.rpm" to update only already installed
> packages.
> 
> The usage of --force disables essentials checks such as file conflict
> detection. As a result the new xen.rpm may overwrite files owned by
> other packages.
> 
> With the current way of calculating version-release it is difficult to
> get an increasing release number into the spec file. The release is
> always zero unless "make make XEN_VENDORVERSION=`date +.%s`" is used,
> which has the bad side effect that xen.gz always gets a different
> filename every time.
> 
> Since the value of release has no meaning its fine to have an ever
> increasing number. This could be either the number of seconds (+%s), or
> some representation which could mean something to a human. The change
> uses a representation of date+time.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: George Dunlap <george.dunlap@eu.citrix.com>

Hey Olaf,

Sorry for not getting back to you with this before.

I applied this patch, and the name came out looking like this:

xen-4.6_unstable-20150323160547.x86_64.rpm

I'm not really happy with having the massive number show up
automatically when anyone makes an rpm.  What would you think about
adding a variable where you could set this as part of the build process?
 e.g.,
  XEN_RPM_RELEASE="$(date +%Y%m%d%H%M%S)" make rpmball

 -George

> ---
>  tools/misc/mkrpm | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/misc/mkrpm b/tools/misc/mkrpm
> index 9b8c6d9..b54de24 100644
> --- a/tools/misc/mkrpm
> +++ b/tools/misc/mkrpm
> @@ -13,13 +13,11 @@ fi
>  
>  xenroot="$1"
>  
> -# rpmbuild doesn't like dashes in the version; break it down into
> -# version and release.  Default to "0" if there isn't a release.
> -v=(${2/-/ })
> -version=${v[0]}
> -release=${v[1]}
> -
> -[[ -n "$release" ]] || release="0"
> +# rpmbuild doesn't support dashes in the version;
> +version=${2//-/_}
> +# Use an ever increasing release number for this devel pkg.
> +# This makes sure rpm -Fvh xen$PKG_SUFFIX.rpm can be updated wihtout --force.
> +release="`date +%Y%m%d%H%M%S`"
>  
>  cd $xenroot
>  
> 

Re: [PATCH 1/5] tools/mkrpm: improve version.release handling

[Olaf Hering]

On Mon, Mar 23, George Dunlap wrote:

> I'm not really happy with having the massive number show up
> automatically when anyone makes an rpm.  What would you think about
> adding a variable where you could set this as part of the build process?
>  e.g.,
>   XEN_RPM_RELEASE="$(date +%Y%m%d%H%M%S)" make rpmball

This may work as well for me. mkrpm handles PKG_SUFFIX already. So
PKG_RELEASE or RPM_RELEASE would be a suitable variable name.
I will try this suggestion.

Olaf

Re: [PATCH 1/5] tools/mkrpm: improve version.release handling

[Olaf Hering]

On Mon, Mar 23, George Dunlap wrote:

> xen-4.6_unstable-20150323160547.x86_64.rpm

Do you want to continue to get "unstable" as release? 
My patch uses everything passed as "$2" as Version string. I think thats
ok. Release will be either 0 or $PKG_DATE.

Olaf

Re: [PATCH 1/5] tools/mkrpm: improve version.release handling

[George Dunlap]

On Mon, Mar 23, 2015 at 5:52 PM, Olaf Hering <olaf@aepfle.de> wrote:
> On Mon, Mar 23, George Dunlap wrote:
>
>> xen-4.6_unstable-20150323160547.x86_64.rpm
>
> Do you want to continue to get "unstable" as release?
> My patch uses everything passed as "$2" as Version string. I think thats
> ok.

I think "unstable" (or "pre" for the stable branches) should
definitely be in there to distinguish it from an actual release.  If I
were writing the patch I might consider taking out the _ (i.e., s/-//;
instead of s/-/_/;) but that's not a big deal.

> Release will be either 0 or $PKG_DATE.

You mean, "0 or $PKG_RELEASE"?  I had in mind that someone might want
to either set their own package release manually (PKG_RELEASE="9"), or
do PKG_RELEASE="$(git log RELEASE-4.5.0.. | wc -l)"

Thanks,
 -George

Re: [PATCH 1/5] tools/mkrpm: improve version.release handling

[Olaf Hering]

On Mon, Mar 23, George Dunlap wrote:

> On Mon, Mar 23, 2015 at 5:52 PM, Olaf Hering <olaf@aepfle.de> wrote:
> > On Mon, Mar 23, George Dunlap wrote:
> >> xen-4.6_unstable-20150323160547.x86_64.rpm
> > Do you want to continue to get "unstable" as release?
> > My patch uses everything passed as "$2" as Version string. I think thats
> > ok.
> I think "unstable" (or "pre" for the stable branches) should
> definitely be in there to distinguish it from an actual release.  If I
> were writing the patch I might consider taking out the _ (i.e., s/-//;
> instead of s/-/_/;) but that's not a big deal.
> 
> > Release will be either 0 or $PKG_DATE.
> You mean, "0 or $PKG_RELEASE"?  I had in mind that someone might want
> to either set their own package release manually (PKG_RELEASE="9"), or
> do PKG_RELEASE="$(git log RELEASE-4.5.0.. | wc -l)"

Ok. Then the essential part of the patch could look like this:

--- a/tools/misc/mkrpm
+++ b/tools/misc/mkrpm
@@ -17,9 +17,7 @@ xenroot="$1"
 # version and release.  Default to "0" if there isn't a release.
 v=(${2/-/ })
 version=${v[0]}
-release=${v[1]}
-
-[[ -n "$release" ]] || release="0"
+release="${v[1]:-0}${PKG_RELEASE:+.$PKG_RELEASE}"

 cd $xenroot

release will be anything after a dash, or 0. And an optional ".$PKG_RELEASE"
will be appended.

Olaf

Re: [PATCH 1/5] tools/mkrpm: improve version.release handling

[George Dunlap]

On 03/23/2015 06:49 PM, Olaf Hering wrote:
> On Mon, Mar 23, George Dunlap wrote:
> 
>> On Mon, Mar 23, 2015 at 5:52 PM, Olaf Hering <olaf@aepfle.de> wrote:
>>> On Mon, Mar 23, George Dunlap wrote:
>>>> xen-4.6_unstable-20150323160547.x86_64.rpm
>>> Do you want to continue to get "unstable" as release?
>>> My patch uses everything passed as "$2" as Version string. I think thats
>>> ok.
>> I think "unstable" (or "pre" for the stable branches) should
>> definitely be in there to distinguish it from an actual release.  If I
>> were writing the patch I might consider taking out the _ (i.e., s/-//;
>> instead of s/-/_/;) but that's not a big deal.
>>
>>> Release will be either 0 or $PKG_DATE.
>> You mean, "0 or $PKG_RELEASE"?  I had in mind that someone might want
>> to either set their own package release manually (PKG_RELEASE="9"), or
>> do PKG_RELEASE="$(git log RELEASE-4.5.0.. | wc -l)"
> 
> Ok. Then the essential part of the patch could look like this:
> 
> --- a/tools/misc/mkrpm
> +++ b/tools/misc/mkrpm
> @@ -17,9 +17,7 @@ xenroot="$1"
>  # version and release.  Default to "0" if there isn't a release.
>  v=(${2/-/ })
>  version=${v[0]}
> -release=${v[1]}
> -
> -[[ -n "$release" ]] || release="0"
> +release="${v[1]:-0}${PKG_RELEASE:+.$PKG_RELEASE}"
> 
>  cd $xenroot
> 
> release will be anything after a dash, or 0. And an optional ".$PKG_RELEASE"
> will be appended.

Just trying to decode this: So if I were using PKG_RELEASE=$(date ...),
then I'd get something like

xen-4.6-unstable.20150323160547.x86_64.rpm
xen-4.4.2-0.20150323160547.x86_64.rpm

For non-release states and release states, respectively; and if I hadn't
set PKG_RELEASE, those would come out like this:

xen-4.6-unstable.x86_64.rpm
xen-4.4.2-0.x86_64.rpm

Is that about right?  That all looks good to me.

 -George

Re: [PATCH 1/5] tools/mkrpm: improve version.release handling

[Olaf Hering]

On Mon, Mar 23, George Dunlap wrote:

> On 03/23/2015 06:49 PM, Olaf Hering wrote:
> > release will be anything after a dash, or 0. And an optional ".$PKG_RELEASE"
> > will be appended.
> Just trying to decode this: So if I were using PKG_RELEASE=$(date ...),
> then I'd get something like
> 
> xen-4.6-unstable.20150323160547.x86_64.rpm
> xen-4.4.2-0.20150323160547.x86_64.rpm
> 
> For non-release states and release states, respectively; and if I hadn't
> set PKG_RELEASE, those would come out like this:
> 
> xen-4.6-unstable.x86_64.rpm
> xen-4.4.2-0.x86_64.rpm
> 
> Is that about right?  That all looks good to me.

Ok, so I will send an updated patch.

Olaf

[PATCH 2/5] tools/hotplug: introduce XENSTORED_ARGS= in sysconfig file.

[Olaf Hering]

It is already used in the runlevel script and the service file.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 tools/hotplug/Linux/init.d/sysconfig.xencommons.in | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
index c12fc8a..f0fa98d 100644
--- a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
+++ b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
@@ -21,6 +21,14 @@
 #XENSTORED=@XENSTORED@
 
 ## Type: string
+## Default: ""
+#
+# Additional commandline arguments to start xenstored,
+# like "--trace-file /var/log/xen/xenstored-trace.log"
+# See "@sbindir@/xenstored --help" for possible options.
+XENSTORED_ARGS=
+
+## Type: string
 ## Default: Not defined, tracing off
 #
 # Log xenstored messages

Re: [PATCH 2/5] tools/hotplug: introduce XENSTORED_ARGS= in sysconfig file.

[Ian Campbell]

On Fri, 2015-03-20 at 18:24 +0100, Olaf Hering wrote:
> It is already used in the runlevel script and the service file.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> ---
>  tools/hotplug/Linux/init.d/sysconfig.xencommons.in | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
> index c12fc8a..f0fa98d 100644
> --- a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
> +++ b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
> @@ -21,6 +21,14 @@
>  #XENSTORED=@XENSTORED@
>  
>  ## Type: string
> +## Default: ""
> +#
> +# Additional commandline arguments to start xenstored,
> +# like "--trace-file /var/log/xen/xenstored-trace.log"
> +# See "@sbindir@/xenstored --help" for possible options.
> +XENSTORED_ARGS=
> +
> +## Type: string
>  ## Default: Not defined, tracing off
>  #
>  # Log xenstored messages

[PATCH 3/5] INSTALL: mention variables for reproducible builds

[Olaf Hering]

Mention two variables introduced by commit ac977f5 ("use more fixed
strings to build the hypervisor").

Signed-off-by: Olaf Hering <olaf@aepfle.de>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Tim Deegan <tim@xen.org>
---
 INSTALL | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/INSTALL b/INSTALL
index 33f65ba..eac34b4 100644
--- a/INSTALL
+++ b/INSTALL
@@ -209,6 +209,12 @@ changed with these variables.
 XEN_WHOAMI=
 XEN_DOMAIN=
 
+Some components of xen and tools will include an unpredictable timestamp
+into the binaries. To allow reproducible builds the following variables
+can be used to provide fixed timestamps in the expected format.
+XEN_BUILD_DATE=any
+XEN_BUILD_TIME=hh:mm:ss
+
 The following variables can be used to tweak some aspects of the
 hypervisor build.
 verbose=y

Re: [PATCH 3/5] INSTALL: mention variables for reproducible builds

[Ian Campbell]

On Fri, 2015-03-20 at 18:24 +0100, Olaf Hering wrote:
> Mention two variables introduced by commit ac977f5 ("use more fixed
> strings to build the hypervisor").
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Keir Fraser <keir@xen.org>
> Cc: Tim Deegan <tim@xen.org>
> ---
>  INSTALL | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/INSTALL b/INSTALL
> index 33f65ba..eac34b4 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -209,6 +209,12 @@ changed with these variables.
>  XEN_WHOAMI=
>  XEN_DOMAIN=
>  
> +Some components of xen and tools will include an unpredictable timestamp
> +into the binaries. To allow reproducible builds the following variables
> +can be used to provide fixed timestamps in the expected format.
> +XEN_BUILD_DATE=any

I'm not sure what this implies I'm allowed to pass here...

Perhaps both cases would benefit from adding "(e.g. something like hte
output from <...>)" where <...> is the date invocation which is used if
the variable isn't set?

> +XEN_BUILD_TIME=hh:mm:ss
> +
>  The following variables can be used to tweak some aspects of the
>  hypervisor build.
>  verbose=y

Re: [PATCH 3/5] INSTALL: mention variables for reproducible builds

[Olaf Hering]

On Tue, Mar 24, Ian Campbell wrote:

> On Fri, 2015-03-20 at 18:24 +0100, Olaf Hering wrote:
> > +Some components of xen and tools will include an unpredictable timestamp
> > +into the binaries. To allow reproducible builds the following variables
> > +can be used to provide fixed timestamps in the expected format.
> > +XEN_BUILD_DATE=any
> I'm not sure what this implies I'm allowed to pass here...

Any string would do, if I read the code correctly.
Would this be more obvious?

XEN_BUILD_DATE=<output of date(1)>

Olaf

Re: [PATCH 3/5] INSTALL: mention variables for reproducible builds

[Ian Campbell]

On Wed, 2015-03-25 at 09:49 +0100, Olaf Hering wrote:
> On Tue, Mar 24, Ian Campbell wrote:
> 
> > On Fri, 2015-03-20 at 18:24 +0100, Olaf Hering wrote:
> > > +Some components of xen and tools will include an unpredictable timestamp
> > > +into the binaries. To allow reproducible builds the following variables
> > > +can be used to provide fixed timestamps in the expected format.
> > > +XEN_BUILD_DATE=any
> > I'm not sure what this implies I'm allowed to pass here...
> 
> Any string would do, if I read the code correctly.
> Would this be more obvious?
> 
> XEN_BUILD_DATE=<output of date(1)>

Yes.

[PATCH 4/5] hvmloader: add knob for fixed SMBIOS date string

[Olaf Hering]

To allow reproducible builds of hvmloader introduce a make variable
SMBIOS_REL_DATE=mm/dd/yyyy to provide a fixed date string. Without this
change the hvmloader binary changes with every rebuild.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
Cc: Keir Fraser <keir@xen.org>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 INSTALL                           | 1 +
 tools/firmware/hvmloader/Makefile | 5 ++++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/INSTALL b/INSTALL
index eac34b4..198cb9d 100644
--- a/INSTALL
+++ b/INSTALL
@@ -214,6 +214,7 @@ into the binaries. To allow reproducible builds the following variables
 can be used to provide fixed timestamps in the expected format.
 XEN_BUILD_DATE=any
 XEN_BUILD_TIME=hh:mm:ss
+SMBIOS_REL_DATE=mm/dd/yyyy
 
 The following variables can be used to tweak some aspects of the
 hypervisor build.
diff --git a/tools/firmware/hvmloader/Makefile b/tools/firmware/hvmloader/Makefile
index cf967fd..34008d2 100644
--- a/tools/firmware/hvmloader/Makefile
+++ b/tools/firmware/hvmloader/Makefile
@@ -26,6 +26,9 @@ SUBDIRS := acpi
 # The HVM loader is started in 32-bit mode at the address below:
 LOADADDR = 0x100000
 
+# SMBIOS spec requires format mm/dd/yyyy
+SMBIOS_REL_DATE ?= $(shell date +%m/%d/%Y)
+
 CFLAGS += $(CFLAGS_xeninclude)
 
 OBJS  = hvmloader.o mp_tables.o util.o smbios.o 
@@ -88,7 +91,7 @@ all: subdirs-all
 	$(MAKE) hvmloader
 
 ovmf.o rombios.o seabios.o hvmloader.o: roms.inc
-smbios.o: CFLAGS += -D__SMBIOS_DATE__="\"$(shell date +%m/%d/%Y)\""
+smbios.o: CFLAGS += -D__SMBIOS_DATE__="$(SMBIOS_REL_DATE)"
 
 hvmloader: $(OBJS) acpi/acpi.a
 	$(LD) $(LDFLAGS_DIRECT) -N -Ttext $(LOADADDR) -o hvmloader.tmp $^

Re: [PATCH 4/5] hvmloader: add knob for fixed SMBIOS date string

[Jan Beulich]

>>> On 20.03.15 at 18:24, <olaf@aepfle.de> wrote:
> To allow reproducible builds of hvmloader introduce a make variable
> SMBIOS_REL_DATE=mm/dd/yyyy to provide a fixed date string. Without this
> change the hvmloader binary changes with every rebuild.

I don't really object to this change, but didn't we discuss using
smbios.c's time stamp instead, which then would properly reflect
eventual changes during incremental builds along with producing
unchanged output when there hasn't been any change?

Jan

Re: [PATCH 4/5] hvmloader: add knob for fixed SMBIOS date string

[Olaf Hering]

On Mon, Mar 23, Jan Beulich wrote:

> >>> On 20.03.15 at 18:24, <olaf@aepfle.de> wrote:
> > To allow reproducible builds of hvmloader introduce a make variable
> > SMBIOS_REL_DATE=mm/dd/yyyy to provide a fixed date string. Without this
> > change the hvmloader binary changes with every rebuild.
> 
> I don't really object to this change, but didn't we discuss using
> smbios.c's time stamp instead, which then would properly reflect
> eventual changes during incremental builds along with producing
> unchanged output when there hasn't been any change?

Using the file timestamp will actually break the attempt to get a
reproducible build if the file is patched during package build.
With each rebuild patch(1) will alter the timestamp.

Olaf

Re: [PATCH 4/5] hvmloader: add knob for fixed SMBIOS date string

[Jan Beulich]

>>> On 23.03.15 at 10:15, <olaf@aepfle.de> wrote:
> On Mon, Mar 23, Jan Beulich wrote:
> 
>> >>> On 20.03.15 at 18:24, <olaf@aepfle.de> wrote:
>> > To allow reproducible builds of hvmloader introduce a make variable
>> > SMBIOS_REL_DATE=mm/dd/yyyy to provide a fixed date string. Without this
>> > change the hvmloader binary changes with every rebuild.
>> 
>> I don't really object to this change, but didn't we discuss using
>> smbios.c's time stamp instead, which then would properly reflect
>> eventual changes during incremental builds along with producing
>> unchanged output when there hasn't been any change?
> 
> Using the file timestamp will actually break the attempt to get a
> reproducible build if the file is patched during package build.
> With each rebuild patch(1) will alter the timestamp.

Oh, right, so it's really the default we may want to alter. But that's
not subject of this patch then, so I'll go ack it.

Jan

Re: [PATCH 4/5] hvmloader: add knob for fixed SMBIOS date string

[Jan Beulich]

>>> On 20.03.15 at 18:24, <olaf@aepfle.de> wrote:
> To allow reproducible builds of hvmloader introduce a make variable
> SMBIOS_REL_DATE=mm/dd/yyyy to provide a fixed date string. Without this
> change the hvmloader binary changes with every rebuild.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>

Acked-by: Jan Beulich <jbeulich@suse.com>

[PATCH 5/5] hvmloader: add knob for fixed VGABIOS date string

[Olaf Hering]

To allow reproducible builds of hvmloader introduce a make variable
VGABIOS_REL_DATE="dd Mon yyyy" to provide a fixed date string. Without
this change the hvmloader binary changes with every rebuild.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 INSTALL                         | 1 +
 tools/firmware/vgabios/Makefile | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/INSTALL b/INSTALL
index 198cb9d..1b67c36 100644
--- a/INSTALL
+++ b/INSTALL
@@ -215,6 +215,7 @@ can be used to provide fixed timestamps in the expected format.
 XEN_BUILD_DATE=any
 XEN_BUILD_TIME=hh:mm:ss
 SMBIOS_REL_DATE=mm/dd/yyyy
+VGABIOS_REL_DATE="dd Mon yyyy"
 
 The following variables can be used to tweak some aspects of the
 hypervisor build.
diff --git a/tools/firmware/vgabios/Makefile b/tools/firmware/vgabios/Makefile
index 51d9e6e..3284812 100644
--- a/tools/firmware/vgabios/Makefile
+++ b/tools/firmware/vgabios/Makefile
@@ -5,10 +5,10 @@ BCC = bcc
 AS86 = as86
 
 RELEASE = `pwd | sed "s-.*/--"`
-RELDATE = `date '+%d %b %Y'`
+VGABIOS_REL_DATE ?= `date '+%d %b %Y'`
 RELVERS = `pwd | sed "s-.*/--" | sed "s/vgabios//" | sed "s/-//"`
 
-VGABIOS_DATE = "-DVGABIOS_DATE=\"$(RELDATE)\""
+VGABIOS_DATE = "-DVGABIOS_DATE=\"$(VGABIOS_REL_DATE)\""
 
 .PHONY: all
 all: bios cirrus-bios

Re: [PATCH 5/5] hvmloader: add knob for fixed VGABIOS date string

[Ian Campbell]

On Fri, 2015-03-20 at 18:24 +0100, Olaf Hering wrote:
> To allow reproducible builds of hvmloader introduce a make variable
> VGABIOS_REL_DATE="dd Mon yyyy" to provide a fixed date string. Without
> this change the hvmloader binary changes with every rebuild.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> ---
>  INSTALL                         | 1 +
>  tools/firmware/vgabios/Makefile | 4 ++--
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/INSTALL b/INSTALL
> index 198cb9d..1b67c36 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -215,6 +215,7 @@ can be used to provide fixed timestamps in the expected format.
>  XEN_BUILD_DATE=any
>  XEN_BUILD_TIME=hh:mm:ss
>  SMBIOS_REL_DATE=mm/dd/yyyy
> +VGABIOS_REL_DATE="dd Mon yyyy"

This is getting to be a rather long list of overrides.

Is it expected that they should all be settable independently? Or would
one global setting be sufficient?

If so then a single variable which can be set to something accepted by
date -d which could be formatted in each location would seem preferable.

Ian.

Re: [PATCH 5/5] hvmloader: add knob for fixed VGABIOS date string

[Olaf Hering]

On Tue, Mar 24, Ian Campbell wrote:

> Is it expected that they should all be settable independently? Or would
> one global setting be sufficient?

A single timestamp would be nice, sure.

> If so then a single variable which can be set to something accepted by
> date -d which could be formatted in each location would seem preferable.

I did consider that. But that adds a dependency on 'date -d'. Not sure
if every build environment supports that. On Linux everyone will most
likely run date(1) from coreutils, which understands -d. Not sure about
BSD. I dont have access to BSD right now to check if their date
understands it. Their online manuals state that -d means something
else...

http://netbsd.gw.com/cgi-bin/man-cgi?date++NetBSD-current
http://www.freebsd.org/cgi/man.cgi?query=date
http://www.openbsd.org/cgi-bin/man.cgi/OpenBSD-current/man1/date.1?query=date

In the end I think its acceptable to have a number of variables from
environment or make cmdline. Its only used during automated package
build.

Olaf

Re: [PATCH 5/5] hvmloader: add knob for fixed VGABIOS date string

[Ian Campbell]

On Wed, 2015-03-25 at 09:46 +0100, Olaf Hering wrote:
> On Tue, Mar 24, Ian Campbell wrote:
> 
> > Is it expected that they should all be settable independently? Or would
> > one global setting be sufficient?
> 
> A single timestamp would be nice, sure.
> 
> > If so then a single variable which can be set to something accepted by
> > date -d which could be formatted in each location would seem preferable.
> 
> I did consider that. But that adds a dependency on 'date -d'. Not sure
> if every build environment supports that. On Linux everyone will most
> likely run date(1) from coreutils, which understands -d. Not sure about
> BSD. I dont have access to BSD right now to check if their date
> understands it. Their online manuals state that -d means something
> else...
> 
> http://netbsd.gw.com/cgi-bin/man-cgi?date++NetBSD-current
> http://www.freebsd.org/cgi/man.cgi?query=date
> http://www.openbsd.org/cgi-bin/man.cgi/OpenBSD-current/man1/date.1?query=date

Plus http://pubs.opengroup.org/onlinepubs/9699919799/utilities/date.html
doesn't mention -d at all, which si a shame.

> In the end I think its acceptable to have a number of variables from
> environment or make cmdline. Its only used during automated package
> build.

OK, I guess we can live with it.

Ian.