| Message ID | 8b0e8b8be9c77476ecc702a7c6216ba50659deec.1604156731.git.frederic.pierret@qubes-os.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Reproducibility: use of SOURCE_DATE_EPOCH | expand |
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
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
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
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/
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
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
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,
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/
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),)