| Message ID | 1440663230-4715-1-git-send-email-andreas.devel@googlemail.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
On Thursday, August 27, 2015 at 10:13:49 AM, Andreas Bießmann wrote: > The SOURCE_DATE_EPOCH mechanism for reproducible builds requires the GNU > variant of date. Why does it require the GNU date ? > Respect this and search it, error on missing GNU date. Wouldn't it make more sense to fix the code to NOT depend on GNU extensions ? > Signed-off-by: Andreas Bießmann <andreas.devel@googlemail.com> [...] Best regards, Marek Vasut
Le jeudi 27 août 2015 à 10:13 +0200, Andreas Bießmann a écrit : > The SOURCE_DATE_EPOCH mechanism for reproducible builds requires the GNU > variant of date. Respect this and search it, error on missing GNU date. Well, IMHO we shouldn't check for GNU date but for the extensions it implements. Those could be included in other date implementations. For instance, it looks like busybox's date implements the -u and -d options too. > Signed-off-by: Andreas Bießmann <andreas.devel@googlemail.com> > --- > This commit tries to figure out if we have a GNU date variant > available. It errors on missing GNU date when it is required (for > SOURCE_DATE_EPOCH set). The result is: Also, I don't think it should be up to the Makefile to figure out which date binary to use. It should be up to the user to put the right one as "date", e.g. with an alias. That is, unless the name for GNU date is something really standardized on non-GNU systems. > ---8<--- > abiessmann@punisher % PATH=$ARMv5_PATH:$PATH CROSS_COMPILE=arm-v5te-linux-gnueabi- make SOURCE_DATE_EPOCH="`date -R`" O=/tmp/picosam ARCH=arm include/generated/timestamp_autogenerated.h > make[1]: Entering directory '/tmp/picosam' > /home/abiessmann/src/u-boot/Makefile:1303: *** "No GNU date found". Stop. > make[1]: Leaving directory '/tmp/picosam' > Makefile:146: recipe for target 'sub-make' failed > make: *** [sub-make] Error 2 > abiessmann@punisher % PATH=$ARMv5_PATH:$PATH CROSS_COMPILE=arm-v5te-linux-gnueabi- make O=/tmp/picosam ARCH=arm include/generated/timestamp_autogenerated.h > make[1]: Entering directory '/tmp/picosam' > CHK include/generated/timestamp_autogenerated.h > UPD include/generated/timestamp_autogenerated.h > make[1]: Leaving directory '/tmp/picosam' > --->8--- > > It applies on top of http://patchwork.ozlabs.org/patch/506856/ (Makefile: Use correct timezone for U_BOOT_TZ). > > Makefile | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > diff --git a/Makefile b/Makefile > index b9b2375..95eae64 100644 > --- a/Makefile > +++ b/Makefile > @@ -346,6 +346,9 @@ PERL = perl > PYTHON = python > DTC = dtc > CHECK = sparse > +GNUDATE := $(foreach date,gdate date.gnu date, \ > + $(shell _date=`which $(date)`; \ > + $${_date} --version 2> /dev/null | $(AWK) "/GNU coreutils/ { print \"$${_date}\"; }")) I advise running date with the needed options (-u -d) and checked the error code instead of this. And of course, the variable names shouldn't mention GNU date. > CHECKFLAGS := -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ \ > -Wbitwise -Wno-return-void -D__CHECK_ENDIAN__ $(CF) > @@ -1281,9 +1284,9 @@ endef > define filechk_timestamp.h > (if test -n "$${SOURCE_DATE_EPOCH}"; then \ > SOURCE_DATE="@$${SOURCE_DATE_EPOCH}"; \ > - LC_ALL=C date -u -d "$${SOURCE_DATE}" +'#define U_BOOT_DATE "%b %d %C%y"'; \ > - LC_ALL=C date -u -d "$${SOURCE_DATE}" +'#define U_BOOT_TIME "%T"'; \ > - LC_ALL=C date -u -d "$${SOURCE_DATE}" +'#define U_BOOT_TZ "%z"'; \ > + LC_ALL=C $(GNUDATE) -u -d "$${SOURCE_DATE}" +'#define U_BOOT_DATE "%b %d %C%y"'; \ > + LC_ALL=C $(GNUDATE) -u -d "$${SOURCE_DATE}" +'#define U_BOOT_TIME "%T"'; \ > + LC_ALL=C $(GNUDATE) -u -d "$${SOURCE_DATE}" +'#define U_BOOT_TZ "%z"'; \ > else \ > LC_ALL=C date +'#define U_BOOT_DATE "%b %d %C%y"'; \ > LC_ALL=C date +'#define U_BOOT_TIME "%T"'; \ > @@ -1295,6 +1298,11 @@ $(version_h): include/config/uboot.release FORCE > $(call filechk,version.h) > > $(timestamp_h): $(srctree)/Makefile FORCE > +ifneq ($(strip $(SOURCE_DATE_EPOCH)),) > +ifeq ($(strip $(GNUDATE)),) > + $(error "No GNU date found") > +endif > +endif > $(call filechk,timestamp.h) > > # ---------------------------------------------------------------------------
On 08/27/2015 10:32 AM, Paul Kocialkowski wrote: > Le jeudi 27 août 2015 à 10:13 +0200, Andreas Bießmann a écrit : >> The SOURCE_DATE_EPOCH mechanism for reproducible builds requires the GNU >> variant of date. Respect this and search it, error on missing GNU date. > > Well, IMHO we shouldn't check for GNU date but for the extensions it > implements. Those could be included in other date implementations. For > instance, it looks like busybox's date implements the -u and -d options > too. But BSD date used the -d switch to set the timezone [1] ... ---8<--- -d dst Set the kernel's value for daylight saving time. If dst is non-zero, future calls to gettimeofday(2) will return a non-zero for tz_dsttime. --->8--- And it is the same for OS X [2]. >> Signed-off-by: Andreas Bießmann <andreas.devel@googlemail.com> >> --- >> This commit tries to figure out if we have a GNU date variant >> available. It errors on missing GNU date when it is required (for >> SOURCE_DATE_EPOCH set). The result is: > > Also, I don't think it should be up to the Makefile to figure out which > date binary to use. It should be up to the user to put the right one as > "date", e.g. with an alias. That is, unless the name for GNU date is > something really standardized on non-GNU systems. Most *BSD systems use gdate or date.gnu for the GNU variant of date, if available. The date tool available is the one of the *BSD distributiuon. HAving a shell alias for date covering gdate is for some users not an option. So it has to be some make variable. For me it would be neat to have the make find some possible solution (e.g. search for the GNU variant on systems which have it). >> diff --git a/Makefile b/Makefile >> index b9b2375..95eae64 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -346,6 +346,9 @@ PERL = perl >> PYTHON = python >> DTC = dtc >> CHECK = sparse >> +GNUDATE := $(foreach date,gdate date.gnu date, \ >> + $(shell _date=`which $(date)`; \ >> + $${_date} --version 2> /dev/null | $(AWK) "/GNU coreutils/ { print \"$${_date}\"; }")) > > I advise running date with the needed options (-u -d) and checked the > error code instead of this. And of course, the variable names shouldn't > mention GNU date. Good point, let's see how my OS X date behaves with a set '-d' switch. Andreas [1] http://www.freebsd.org/cgi/man.cgi?query=date [2] https://developer.apple.com/library/mac/documentation/Darwin/Reference/ManPages/man1/date.1.html
Hi Marek, On 08/27/2015 10:28 AM, Marek Vasut wrote: > On Thursday, August 27, 2015 at 10:13:49 AM, Andreas Bießmann wrote: >> The SOURCE_DATE_EPOCH mechanism for reproducible builds requires the GNU >> variant of date. > > Why does it require the GNU date ? Cause of the -d switch. It is used in GNU date variants to set the time to print the string from. On the other hand *BSD variants use it to set the DST flag. >> Respect this and search it, error on missing GNU date. > > Wouldn't it make more sense to fix the code to NOT depend on GNU extensions ? That would be great, but so we need to check for the tool and use the right switches depending on that. I think forcing the user to provide the right tool here is Ok. Best regards Andreas
On Thursday, August 27, 2015 at 11:01:19 AM, Andreas Bießmann wrote: > Hi Marek, Hi! > On 08/27/2015 10:28 AM, Marek Vasut wrote: > > On Thursday, August 27, 2015 at 10:13:49 AM, Andreas Bießmann wrote: > >> The SOURCE_DATE_EPOCH mechanism for reproducible builds requires the GNU > >> variant of date. > > > > Why does it require the GNU date ? > > Cause of the -d switch. It is used in GNU date variants to set the time > to print the string from. On the other hand *BSD variants use it to set > the DST flag. This sort of stuff should be in the commit message :) > >> Respect this and search it, error on missing GNU date. > > > > Wouldn't it make more sense to fix the code to NOT depend on GNU > > extensions ? > > That would be great, but so we need to check for the tool and use the > right switches depending on that. > I think forcing the user to provide the right tool here is Ok. I think we don't have a better option here anyway, right ? Maybe try checking uname ? But you can have linux with bsd userspace ... and probably BSD with GNU userspace, meh. Best regards, Marek Vasut
diff --git a/Makefile b/Makefile index b9b2375..95eae64 100644 --- a/Makefile +++ b/Makefile @@ -346,6 +346,9 @@ PERL = perl PYTHON = python DTC = dtc CHECK = sparse +GNUDATE := $(foreach date,gdate date.gnu date, \ + $(shell _date=`which $(date)`; \ + $${_date} --version 2> /dev/null | $(AWK) "/GNU coreutils/ { print \"$${_date}\"; }")) CHECKFLAGS := -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ \ -Wbitwise -Wno-return-void -D__CHECK_ENDIAN__ $(CF) @@ -1281,9 +1284,9 @@ endef define filechk_timestamp.h (if test -n "$${SOURCE_DATE_EPOCH}"; then \ SOURCE_DATE="@$${SOURCE_DATE_EPOCH}"; \ - LC_ALL=C date -u -d "$${SOURCE_DATE}" +'#define U_BOOT_DATE "%b %d %C%y"'; \ - LC_ALL=C date -u -d "$${SOURCE_DATE}" +'#define U_BOOT_TIME "%T"'; \ - LC_ALL=C date -u -d "$${SOURCE_DATE}" +'#define U_BOOT_TZ "%z"'; \ + LC_ALL=C $(GNUDATE) -u -d "$${SOURCE_DATE}" +'#define U_BOOT_DATE "%b %d %C%y"'; \ + LC_ALL=C $(GNUDATE) -u -d "$${SOURCE_DATE}" +'#define U_BOOT_TIME "%T"'; \ + LC_ALL=C $(GNUDATE) -u -d "$${SOURCE_DATE}" +'#define U_BOOT_TZ "%z"'; \ else \ LC_ALL=C date +'#define U_BOOT_DATE "%b %d %C%y"'; \ LC_ALL=C date +'#define U_BOOT_TIME "%T"'; \ @@ -1295,6 +1298,11 @@ $(version_h): include/config/uboot.release FORCE $(call filechk,version.h) $(timestamp_h): $(srctree)/Makefile FORCE +ifneq ($(strip $(SOURCE_DATE_EPOCH)),) +ifeq ($(strip $(GNUDATE)),) + $(error "No GNU date found") +endif +endif $(call filechk,timestamp.h) # ---------------------------------------------------------------------------
The SOURCE_DATE_EPOCH mechanism for reproducible builds requires the GNU variant of date. Respect this and search it, error on missing GNU date. Signed-off-by: Andreas Bießmann <andreas.devel@googlemail.com> --- This commit tries to figure out if we have a GNU date variant available. It errors on missing GNU date when it is required (for SOURCE_DATE_EPOCH set). The result is: ---8<--- abiessmann@punisher % PATH=$ARMv5_PATH:$PATH CROSS_COMPILE=arm-v5te-linux-gnueabi- make SOURCE_DATE_EPOCH="`date -R`" O=/tmp/picosam ARCH=arm include/generated/timestamp_autogenerated.h make[1]: Entering directory '/tmp/picosam' /home/abiessmann/src/u-boot/Makefile:1303: *** "No GNU date found". Stop. make[1]: Leaving directory '/tmp/picosam' Makefile:146: recipe for target 'sub-make' failed make: *** [sub-make] Error 2 abiessmann@punisher % PATH=$ARMv5_PATH:$PATH CROSS_COMPILE=arm-v5te-linux-gnueabi- make O=/tmp/picosam ARCH=arm include/generated/timestamp_autogenerated.h make[1]: Entering directory '/tmp/picosam' CHK include/generated/timestamp_autogenerated.h UPD include/generated/timestamp_autogenerated.h make[1]: Leaving directory '/tmp/picosam' --->8--- It applies on top of http://patchwork.ozlabs.org/patch/506856/ (Makefile: Use correct timezone for U_BOOT_TZ). Makefile | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)