[PATCH v1 0/2] Reproducibility: use of SOURCE_DATE_EPOCH

[PATCH v1 0/2] Reproducibility: use of SOURCE_DATE_EPOCH

[Frédéric Pierret (fepitre)]

It intends to take into account feedback on previous patch removing timestamp
from xen efi binary. If defined, we can set build dates and time variables with
respect to SOURCE_DATE_EPOCH. A default value is provided if sources
come from git repository else, that's up to the builder to handle it.

Frédéric Pierret (fepitre) (2):
  Define build dates/time based on SOURCE_DATE_EPOCH
  Define SOURCE_DATE_EPOCH based on git log

 tools/firmware/hvmloader/Makefile | 4 ++++
 tools/firmware/vgabios/Makefile   | 4 ++++
 xen/Makefile                      | 7 +++++++
 3 files changed, 15 insertions(+)

-- 
2.26.2

[PATCH v1 1/2] Define build dates/time based on SOURCE_DATE_EPOCH

[Frédéric Pierret (fepitre)]

It improves reproducibility if SOURCE_DATE_EPOCH is
defined while building xen binary
---
 tools/firmware/hvmloader/Makefile | 4 ++++
 tools/firmware/vgabios/Makefile   | 4 ++++
 xen/Makefile                      | 5 +++++
 3 files changed, 13 insertions(+)

diff --git a/tools/firmware/hvmloader/Makefile b/tools/firmware/hvmloader/Makefile
index e980ce7c5f..923e3c8b9a 100644
--- a/tools/firmware/hvmloader/Makefile
+++ b/tools/firmware/hvmloader/Makefile
@@ -21,7 +21,11 @@ XEN_ROOT = $(CURDIR)/../../..
 include $(XEN_ROOT)/tools/firmware/Rules.mk
 
 # SMBIOS spec requires format mm/dd/yyyy
+ifneq ($(SOURCE_DATE_EPOCH),)
+SMBIOS_REL_DATE ?= $(shell date -u -d "@$(SOURCE_DATE_EPOCH)" "+%m/%d/%Y" 2>/dev/null)
+else
 SMBIOS_REL_DATE ?= $(shell date +%m/%d/%Y)
+endif
 
 CFLAGS += $(CFLAGS_xeninclude)
 
diff --git a/tools/firmware/vgabios/Makefile b/tools/firmware/vgabios/Makefile
index 3284812fde..9b8b687a73 100644
--- a/tools/firmware/vgabios/Makefile
+++ b/tools/firmware/vgabios/Makefile
@@ -5,7 +5,11 @@ BCC = bcc
 AS86 = as86
 
 RELEASE = `pwd | sed "s-.*/--"`
+ifneq ($(SOURCE_DATE_EPOCH),)
+VGABIOS_REL_DATE ?= $(shell date -u -d "@$(SOURCE_DATE_EPOCH)" "+%d %b %Y" 2>/dev/null)
+else
 VGABIOS_REL_DATE ?= `date '+%d %b %Y'`
+endif
 RELVERS = `pwd | sed "s-.*/--" | sed "s/vgabios//" | sed "s/-//"`
 
 VGABIOS_DATE = "-DVGABIOS_DATE=\"$(VGABIOS_REL_DATE)\""
diff --git a/xen/Makefile b/xen/Makefile
index bf0c804d43..30b1847515 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -8,8 +8,13 @@ export XEN_FULLVERSION   = $(XEN_VERSION).$(XEN_SUBVERSION)$(XEN_EXTRAVERSION)
 
 export XEN_WHOAMI	?= $(USER)
 export XEN_DOMAIN	?= $(shell ([ -x /bin/dnsdomainname ] && /bin/dnsdomainname) || ([ -x /bin/domainname ] && /bin/domainname || echo [unknown]))
+ifneq ($(SOURCE_DATE_EPOCH),)
+export XEN_BUILD_DATE	?= $(shell date -u -d "@$(SOURCE_DATE_EPOCH)" 2>/dev/null)
+export XEN_BUILD_TIME	?= $(shell date -u -d "@$(SOURCE_DATE_EPOCH)" +%T 2>/dev/null)
+else
 export XEN_BUILD_DATE	?= $(shell LC_ALL=C date)
 export XEN_BUILD_TIME	?= $(shell LC_ALL=C date +%T)
+endif
 export XEN_BUILD_HOST	?= $(shell hostname)
 
 # Best effort attempt to find a python interpreter, defaulting to Python 3 if
-- 
2.26.2

[PATCH v1 2/2] Define SOURCE_DATE_EPOCH based on git log

[Frédéric Pierret (fepitre)]

---
 xen/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/Makefile b/xen/Makefile
index 30b1847515..4cc35556ef 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -6,6 +6,8 @@ export XEN_EXTRAVERSION ?= -unstable$(XEN_VENDORVERSION)
 export XEN_FULLVERSION   = $(XEN_VERSION).$(XEN_SUBVERSION)$(XEN_EXTRAVERSION)
 -include xen-version
 
+export SOURCE_DATE_EPOCH	?= $(shell git log -1 --format=%ct 2>/dev/null)
+
 export XEN_WHOAMI	?= $(USER)
 export XEN_DOMAIN	?= $(shell ([ -x /bin/dnsdomainname ] && /bin/dnsdomainname) || ([ -x /bin/domainname ] && /bin/domainname || echo [unknown]))
 ifneq ($(SOURCE_DATE_EPOCH),)
-- 
2.26.2

Re: [PATCH v1 1/2] Define build dates/time based on SOURCE_DATE_EPOCH

[Jan Beulich]

On 31.10.2020 16:14, Frédéric Pierret (fepitre) wrote:
> --- a/tools/firmware/hvmloader/Makefile
> +++ b/tools/firmware/hvmloader/Makefile
> @@ -21,7 +21,11 @@ XEN_ROOT = $(CURDIR)/../../..
>  include $(XEN_ROOT)/tools/firmware/Rules.mk
>  
>  # SMBIOS spec requires format mm/dd/yyyy
> +ifneq ($(SOURCE_DATE_EPOCH),)
> +SMBIOS_REL_DATE ?= $(shell date -u -d "@$(SOURCE_DATE_EPOCH)" "+%m/%d/%Y" 2>/dev/null)
> +else
>  SMBIOS_REL_DATE ?= $(shell date +%m/%d/%Y)
> +endif

As this pattern recurs, how about abstracting it away via a
definition (perhaps to be placed in ./Config.mk) along the
lines of (variable name subject to improvement)

DATE_EPOCH_OPTS := $(if $(SOURCE_DATE_EPOCH),-u -d "@$(SOURCE_DATE_EPOCH)")

and then here simply

SMBIOS_REL_DATE ?= $(shell date $(DATE_EPOCH_OPTS) +%m/%d/%Y)

(i.e. in particular also without any "ifneq()")?

Jan

Re: [PATCH v1 2/2] Define SOURCE_DATE_EPOCH based on git log

[Jan Beulich]

On 31.10.2020 16:14, Frédéric Pierret (fepitre) wrote:
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -6,6 +6,8 @@ export XEN_EXTRAVERSION ?= -unstable$(XEN_VENDORVERSION)
>  export XEN_FULLVERSION   = $(XEN_VERSION).$(XEN_SUBVERSION)$(XEN_EXTRAVERSION)
>  -include xen-version
>  
> +export SOURCE_DATE_EPOCH	?= $(shell git log -1 --format=%ct 2>/dev/null)

In patch 1 you also use the variable under tools/. Why do you
place this here rather than in the top level Makefile?

This said I'm not convinced anyway that we want this to be the
default. I'd rather see this as something to get set by the
package build process of distros, outside of any of the source
files.

Jan

Re: [PATCH v1 1/2] Define build dates/time based on SOURCE_DATE_EPOCH

[Frédéric Pierret]

[-- Attachment #1.1.1: Type: text/plain, Size: 1092 bytes --]


Le 11/3/20 à 10:10 AM, Jan Beulich a écrit :
> On 31.10.2020 16:14, Frédéric Pierret (fepitre) wrote:
>> --- a/tools/firmware/hvmloader/Makefile
>> +++ b/tools/firmware/hvmloader/Makefile
>> @@ -21,7 +21,11 @@ XEN_ROOT = $(CURDIR)/../../..
>>   include $(XEN_ROOT)/tools/firmware/Rules.mk
>>   
>>   # SMBIOS spec requires format mm/dd/yyyy
>> +ifneq ($(SOURCE_DATE_EPOCH),)
>> +SMBIOS_REL_DATE ?= $(shell date -u -d "@$(SOURCE_DATE_EPOCH)" "+%m/%d/%Y" 2>/dev/null)
>> +else
>>   SMBIOS_REL_DATE ?= $(shell date +%m/%d/%Y)
>> +endif
> 
> As this pattern recurs, how about abstracting it away via a
> definition (perhaps to be placed in ./Config.mk) along the
> lines of (variable name subject to improvement)
> 
> DATE_EPOCH_OPTS := $(if $(SOURCE_DATE_EPOCH),-u -d "@$(SOURCE_DATE_EPOCH)")
> 
> and then here simply
> 
> SMBIOS_REL_DATE ?= $(shell date $(DATE_EPOCH_OPTS) +%m/%d/%Y)
> 
> (i.e. in particular also without any "ifneq()")?
> 
> Jan
> 

Hi Jan,

Yes it makes sense. I'll prepare another version with your comments. Thank you.

Frédéric

[-- Attachment #1.1.2: OpenPGP_0x484010B5CDC576E2.asc --]
[-- Type: application/pgp-keys, Size: 3143 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v1 2/2] Define SOURCE_DATE_EPOCH based on git log

[Frédéric Pierret]

[-- Attachment #1.1.1: Type: text/plain, Size: 1052 bytes --]



Le 11/3/20 à 10:15 AM, Jan Beulich a écrit :
> On 31.10.2020 16:14, Frédéric Pierret (fepitre) wrote:
>> --- a/xen/Makefile
>> +++ b/xen/Makefile
>> @@ -6,6 +6,8 @@ export XEN_EXTRAVERSION ?= -unstable$(XEN_VENDORVERSION)
>>   export XEN_FULLVERSION   = $(XEN_VERSION).$(XEN_SUBVERSION)$(XEN_EXTRAVERSION)
>>   -include xen-version
>>   
>> +export SOURCE_DATE_EPOCH	?= $(shell git log -1 --format=%ct 2>/dev/null)
> 
> In patch 1 you also use the variable under tools/. Why do you
> place this here rather than in the top level Makefile?
> 
> This said I'm not convinced anyway that we want this to be the
> default. I'd rather see this as something to get set by the
> package build process of distros, outside of any of the source
> files.
> 
> Jan
> 

In fact this was intended to provide a default/example value. Indeed, each package manager should
handle this with changelog or such.

This is not mandatory and if not wanted by default, maybe add this example value into the INSTALL documentation?

Frédéric

[-- Attachment #1.1.2: OpenPGP_0x484010B5CDC576E2.asc --]
[-- Type: application/pgp-keys, Size: 3143 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v1 2/2] Define SOURCE_DATE_EPOCH based on git log

[Jan Beulich]

On 03.11.2020 10:21, Frédéric Pierret wrote:
> 
> 
> Le 11/3/20 à 10:15 AM, Jan Beulich a écrit :
>> On 31.10.2020 16:14, Frédéric Pierret (fepitre) wrote:
>>> --- a/xen/Makefile
>>> +++ b/xen/Makefile
>>> @@ -6,6 +6,8 @@ export XEN_EXTRAVERSION ?= -unstable$(XEN_VENDORVERSION)
>>>   export XEN_FULLVERSION   = $(XEN_VERSION).$(XEN_SUBVERSION)$(XEN_EXTRAVERSION)
>>>   -include xen-version
>>>   
>>> +export SOURCE_DATE_EPOCH	?= $(shell git log -1 --format=%ct 2>/dev/null)
>>
>> In patch 1 you also use the variable under tools/. Why do you
>> place this here rather than in the top level Makefile?
>>
>> This said I'm not convinced anyway that we want this to be the
>> default. I'd rather see this as something to get set by the
>> package build process of distros, outside of any of the source
>> files.
> 
> In fact this was intended to provide a default/example value. Indeed, each package manager should
> handle this with changelog or such.
> 
> This is not mandatory and if not wanted by default, maybe add this example value into the INSTALL documentation?

I'm certainly fine with putting this in the docs. (Whether
INSTALL is the right place I can#t tell.)

Jan

Re: [PATCH v1 2/2] Define SOURCE_DATE_EPOCH based on git log

[Julien Grall]

Hi Frédéric,

On 31/10/2020 15:14, Frédéric Pierret (fepitre) wrote:
> ---
>   xen/Makefile | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/xen/Makefile b/xen/Makefile
> index 30b1847515..4cc35556ef 100644
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -6,6 +6,8 @@ export XEN_EXTRAVERSION ?= -unstable$(XEN_VENDORVERSION)
>   export XEN_FULLVERSION   = $(XEN_VERSION).$(XEN_SUBVERSION)$(XEN_EXTRAVERSION)
>   -include xen-version
>   
> +export SOURCE_DATE_EPOCH	?= $(shell git log -1 --format=%ct 2>/dev/null)

It is possible to download a tarball for Xen release (see [1]). They 
don't contain the .git directory and therefore this command would fail.

Should we fallback to "date" in this case?

> +
>   export XEN_WHOAMI	?= $(USER)
>   export XEN_DOMAIN	?= $(shell ([ -x /bin/dnsdomainname ] && /bin/dnsdomainname) || ([ -x /bin/domainname ] && /bin/domainname || echo [unknown]))
>   ifneq ($(SOURCE_DATE_EPOCH),)
> 

Cheers,

[1] https://xenproject.org/downloads/

-- 
Julien Grall

Re: [PATCH v1 2/2] Define SOURCE_DATE_EPOCH based on git log

[Jan Beulich]

On 03.11.2020 11:00, Julien Grall wrote:
> Hi Frédéric,
> 
> On 31/10/2020 15:14, Frédéric Pierret (fepitre) wrote:
>> ---
>>   xen/Makefile | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/xen/Makefile b/xen/Makefile
>> index 30b1847515..4cc35556ef 100644
>> --- a/xen/Makefile
>> +++ b/xen/Makefile
>> @@ -6,6 +6,8 @@ export XEN_EXTRAVERSION ?= -unstable$(XEN_VENDORVERSION)
>>   export XEN_FULLVERSION   = $(XEN_VERSION).$(XEN_SUBVERSION)$(XEN_EXTRAVERSION)
>>   -include xen-version
>>   
>> +export SOURCE_DATE_EPOCH	?= $(shell git log -1 --format=%ct 2>/dev/null)
> 
> It is possible to download a tarball for Xen release (see [1]). They 
> don't contain the .git directory and therefore this command would fail.
> 
> Should we fallback to "date" in this case?

Isn't this what already happens? The variable would be assigned
an empty value in this case, wouldn't it?

Jan

Re: [PATCH v1 2/2] Define SOURCE_DATE_EPOCH based on git log

[Frédéric Pierret]

[-- Attachment #1.1.1: Type: text/plain, Size: 1409 bytes --]


Le 11/3/20 à 11:05 AM, Jan Beulich a écrit :
> On 03.11.2020 11:00, Julien Grall wrote:
>> Hi Frédéric,
>>
Hi Julien,

>> On 31/10/2020 15:14, Frédéric Pierret (fepitre) wrote:
>>> ---
>>>    xen/Makefile | 2 ++
>>>    1 file changed, 2 insertions(+)
>>>
>>> diff --git a/xen/Makefile b/xen/Makefile
>>> index 30b1847515..4cc35556ef 100644
>>> --- a/xen/Makefile
>>> +++ b/xen/Makefile
>>> @@ -6,6 +6,8 @@ export XEN_EXTRAVERSION ?= -unstable$(XEN_VENDORVERSION)
>>>    export XEN_FULLVERSION   = $(XEN_VERSION).$(XEN_SUBVERSION)$(XEN_EXTRAVERSION)
>>>    -include xen-version
>>>    
>>> +export SOURCE_DATE_EPOCH	?= $(shell git log -1 --format=%ct 2>/dev/null)
>>
>> It is possible to download a tarball for Xen release (see [1]). They
>> don't contain the .git directory and therefore this command would fail.
>>
>> Should we fallback to "date" in this case?
> 
> Isn't this what already happens? The variable would be assigned
> an empty value in this case, wouldn't it?

Julien, Jan, yes it already fallback to "date" if the variable is empty (it's the reason of "2>/dev/null") in the other test of check if SOURCE_DATE_EPOCH is defined. Maybe there is more elegant way for this. Depending on the wanted here for providing or not a default value in case of git sources, this could be documented instead as suggested previously.

> Jan
> 

Regards,
Frédéric

[-- Attachment #1.1.2: OpenPGP_0x484010B5CDC576E2.asc --]
[-- Type: application/pgp-keys, Size: 3143 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v1 2/2] Define SOURCE_DATE_EPOCH based on git log

[Julien Grall]

On 03/11/2020 10:05, Jan Beulich wrote:
> On 03.11.2020 11:00, Julien Grall wrote:
>> Hi Frédéric,
>>
>> On 31/10/2020 15:14, Frédéric Pierret (fepitre) wrote:
>>> ---
>>>    xen/Makefile | 2 ++
>>>    1 file changed, 2 insertions(+)
>>>
>>> diff --git a/xen/Makefile b/xen/Makefile
>>> index 30b1847515..4cc35556ef 100644
>>> --- a/xen/Makefile
>>> +++ b/xen/Makefile
>>> @@ -6,6 +6,8 @@ export XEN_EXTRAVERSION ?= -unstable$(XEN_VENDORVERSION)
>>>    export XEN_FULLVERSION   = $(XEN_VERSION).$(XEN_SUBVERSION)$(XEN_EXTRAVERSION)
>>>    -include xen-version
>>>    
>>> +export SOURCE_DATE_EPOCH	?= $(shell git log -1 --format=%ct 2>/dev/null)
>>
>> It is possible to download a tarball for Xen release (see [1]). They
>> don't contain the .git directory and therefore this command would fail.
>>
>> Should we fallback to "date" in this case?
> 
> Isn't this what already happens? The variable would be assigned
> an empty value in this case, wouldn't it?

My question was whether empty SOURCE_DATE_EPOCH is acceptable?

Looking at patch #1, the users of the variable will use "date" if it is 
empty. Why can't this behavior be common?

Cheers,

-- 
Julien Grall

Re: [PATCH v1 2/2] Define SOURCE_DATE_EPOCH based on git log

[Frédéric Pierret]

[-- Attachment #1.1.1: Type: text/plain, Size: 1856 bytes --]



Le 11/3/20 à 11:11 AM, Julien Grall a écrit :
> 
> 
> On 03/11/2020 10:05, Jan Beulich wrote:
>> On 03.11.2020 11:00, Julien Grall wrote:
>>> Hi Frédéric,
>>>
>>> On 31/10/2020 15:14, Frédéric Pierret (fepitre) wrote:
>>>> ---
>>>>    xen/Makefile | 2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/xen/Makefile b/xen/Makefile
>>>> index 30b1847515..4cc35556ef 100644
>>>> --- a/xen/Makefile
>>>> +++ b/xen/Makefile
>>>> @@ -6,6 +6,8 @@ export XEN_EXTRAVERSION ?= -unstable$(XEN_VENDORVERSION)
>>>>    export XEN_FULLVERSION   = $(XEN_VERSION).$(XEN_SUBVERSION)$(XEN_EXTRAVERSION)
>>>>    -include xen-version
>>>> +export SOURCE_DATE_EPOCH    ?= $(shell git log -1 --format=%ct 2>/dev/null)
>>>
>>> It is possible to download a tarball for Xen release (see [1]). They
>>> don't contain the .git directory and therefore this command would fail.
>>>
>>> Should we fallback to "date" in this case?
>>
>> Isn't this what already happens? The variable would be assigned
>> an empty value in this case, wouldn't it?
> 
> My question was whether empty SOURCE_DATE_EPOCH is acceptable?
> 
> Looking at patch #1, the users of the variable will use "date" if it is empty. Why can't this behavior be common?
> 
> Cheers,
> 

In fact, we could fallback to date in SOURCE_DATE_EPOCH definition and in this case this would always be defined. Now, I'm wondering how misleading that could be with respect to its definition (see [1]): "The value MUST be reproducible (deterministic) across different executions of the build, depending only on the source code.". In this case, if someone looks to the code and interpret the build time etc, defined with respect to SOURCE_DATE_EPOCH, that would be odd?

Regards,
Frédéric


[1]: https://reproducible-builds.org/specs/source-date-epoch/

[-- Attachment #1.1.2: OpenPGP_0x484010B5CDC576E2.asc --]
[-- Type: application/pgp-keys, Size: 3143 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]