| Message ID | 1479460224-6119-10-git-send-email-jezz@sysmic.org |
|---|---|
| State | Superseded |
| Headers | show |
Hello, On Fri, 18 Nov 2016 10:10:18 +0100, Jérôme Pouiller wrote: > `date' is widely used by packages to include build information in their > binaries. Unfortunately, this is incompatible with BR2_REPRODUCIBLE. > > Instead to find all `date' invocation in build process, we add small tool > allowing to alway return same date. Instead of having to identify all `date' invocations in the different packages, this commit adds a small tool that allows to always return the same date. > +PATH=/bin:/usr/bin It is not really nice to override the PATH. I guess you want to remove $(HOST_DIR)/usr/bin from the PATH to not call yourself recursively, but I think we should do better than assuming /bin:/usr/bin is OK. > +LOG=/dev/null This variable is used by? > +if [ -n "$SOURCE_DATE_EPOCH" ]; then > + INHIBIT=0 > + for i in "$@"; do > + case $i in > + -d|-[!-]*d|--date=*|-f|-[!-]*f|--file=*) > + INHIBIT=1 > + ;; > + esac > + done > + if [ $INHIBIT -eq 0 ]; then > + echo "date: Warning: using \$SOURCE_DATE_EPOCH instead of true time" >&2 > + echo "Catch call to date from `pwd` with parameters: '$@'" >> $LOG > + exec date -d "@$SOURCE_DATE_EPOCH" "$@" > + fi > +fi > + > +exec date "$@" Could you explain a bit the logic here? Thanks, Thomas
On 18-11-16 10:10, Jérôme Pouiller wrote: > `date' is widely used by packages to include build information in their > binaries. Unfortunately, this is incompatible with BR2_REPRODUCIBLE. > > Instead to find all `date' invocation in build process, we add small tool > allowing to alway return same date. > > This work was sponsored by `BA Robotic Systems'. > > Signed-off-by: Jérôme Pouiller <jezz@sysmic.org> > --- > package/fakedate/fakedate | 28 ++++++++++++++++++++++++++++ > package/fakedate/fakedate.mk | 14 ++++++++++++++ > 2 files changed, 42 insertions(+) > create mode 100755 package/fakedate/fakedate > create mode 100644 package/fakedate/fakedate.mk > > diff --git a/package/fakedate/fakedate b/package/fakedate/fakedate > new file mode 100755 > index 0000000..2eded22 > --- /dev/null > +++ b/package/fakedate/fakedate > @@ -0,0 +1,28 @@ > +#!/bin/sh > +# vim: set sw=4 expandtab: > +# > +# Licence: GPL Please use a proper copyright blurb. Yes, it's long, but it's also more accurate. You seem to be saying here that it is GPLv1 only, which is most likely not what you want. > +# Created: 2016-11-04 16:31:18+01:00 > +# Main authors: > +# - Jérôme Pouiller <jezz@sysmic.org> > +# > + > +PATH=/bin:/usr/bin > +LOG=/dev/null > +if [ -n "$SOURCE_DATE_EPOCH" ]; then > + INHIBIT=0 INHIBIT is a bit vague. How about DATE_IS_FORCED? > + for i in "$@"; do > + case $i in > + -d|-[!-]*d|--date=*|-f|-[!-]*f|--file=*) We use [^-] everywhere else. Note that this pattern will also match something like -rfrood, i.e. --reference=frood. Fixing that becomes tricky without regexp. Anyway, the -d option doesn't really need to be checked. 'date -d foo -d bar' will ignore the first -d, so things work OK. It's just that you get the spurious warning. So we could limit to checking -f, and limit to -f|--file=*). In that case, if someone passes something like -uf we'll get an error and the build will most likely terminate, so that particular error can be fixed. > + INHIBIT=1 You could add a break here. > + ;; > + esac > + done > + if [ $INHIBIT -eq 0 ]; then > + echo "date: Warning: using \$SOURCE_DATE_EPOCH instead of true time" >&2 Is it really needed to print this warning? > + echo "Catch call to date from `pwd` with parameters: '$@'" >> $LOG > + exec date -d "@$SOURCE_DATE_EPOCH" "$@" > + fi > +fi > + > +exec date "$@" > diff --git a/package/fakedate/fakedate.mk b/package/fakedate/fakedate.mk > new file mode 100644 > index 0000000..e81ce5d > --- /dev/null > +++ b/package/fakedate/fakedate.mk > @@ -0,0 +1,14 @@ > +################################################################################ > +# > +# fakedate > +# > +################################################################################ > + > +# source included in buildroot > +HOST_FAKEDATE_LICENSE = GPLv2+ This is inconsistent with the script itself, which specifies GPLv1 only :-P Regards, Arnout > + > +define HOST_FAKEDATE_INSTALL_CMDS > + $(INSTALL) -D -m 755 package/fakedate/fakedate $(HOST_DIR)/usr/bin/date > +endef > + > +$(eval $(host-generic-package)) >
On Saturday 19 November 2016 11:21:39 Arnout Vandecappelle wrote: > On 18-11-16 10:10, Jérôme Pouiller wrote: [...] > > + for i in "$@"; do > > + case $i in > > + -d|-[!-]*d|--date=*|-f|-[!-]*f|--file=*) > > We use [^-] everywhere else. It seems this syntax is a bashism. From glob(7): "POSIX has declared the effect of a wildcard pattern "[^...]" to be undefined" (and I confirm it does not work with dash) > Note that this pattern will also match something > like -rfrood, i.e. --reference=frood. Fixing that becomes tricky without regexp. hmmm... yes, it matches -rfrood (and it is what we want), but it does not match --reference=frood, isn't? > Anyway, the -d option doesn't really need to be checked. 'date -d foo -d bar' > will ignore the first -d, so things work OK. It's just that you get the spurious > warning. So we could limit to checking -f, and limit to -f|--file=*). In that > case, if someone passes something like -uf we'll get an error and the build will > most likely terminate, so that particular error can be fixed. You are right. However, since it may produce unexpected situation, I prefer to identify precisely the cases where fakedate is used. [...] > > + ;; > > + esac > > + done > > + if [ $INHIBIT -eq 0 ]; then > > + echo "date: Warning: using \$SOURCE_DATE_EPOCH instead of true time" >&2 > > Is it really needed to print this warning? From user point of view, result of `date' when fakedate is installed is unexpected. I prefer to warn.
Hello, On Friday 18 November 2016 12:48:06 Thomas Petazzoni wrote: > On Fri, 18 Nov 2016 10:10:18 +0100, Jérôme Pouiller wrote: [...] > > +PATH=/bin:/usr/bin > > It is not really nice to override the PATH. I guess you want to remove > $(HOST_DIR)/usr/bin from the PATH to not call yourself recursively, but > I think we should do better than assuming /bin:/usr/bin is OK. My initial idea was something based on: sed "s/@@DATE_CMD@@/`which date`/" However, I worried about people who add $HOST_DIR in their $PATH and call host-fakedate-reinstall. However, stripping $(dirname $0) from PATH during runtime seems a good idea. I will try that. > > +LOG=/dev/null > > This variable is used by? In my initial version, I logged calls to fakedate in $BUILD_DIR. I removed the log file, but it may be convenient to easily restore it if necessary. > > +if [ -n "$SOURCE_DATE_EPOCH" ]; then > > + INHIBIT=0 > > + for i in "$@"; do > > + case $i in > > + -d|-[!-]*d|--date=*|-f|-[!-]*f|--file=*) > > + INHIBIT=1 > > + ;; > > + esac > > + done > > + if [ $INHIBIT -eq 0 ]; then > > + echo "date: Warning: using \$SOURCE_DATE_EPOCH instead of true time" >&2 > > + echo "Catch call to date from `pwd` with parameters: '$@'" >> $LOG > > + exec date -d "@$SOURCE_DATE_EPOCH" "$@" > > + fi > > +fi > > + > > +exec date "$@" > > Could you explain a bit the logic here? If this script is called with '--date', '--file' or any aliases of these option, just call `date' as usual. Else, this script force returned date. BR,
On 19-11-16 14:06, Jérôme Pouiller wrote: > On Saturday 19 November 2016 11:21:39 Arnout Vandecappelle wrote: >> On 18-11-16 10:10, Jérôme Pouiller wrote: > [...] >>> + for i in "$@"; do >>> + case $i in >>> + -d|-[!-]*d|--date=*|-f|-[!-]*f|--file=*) >> >> We use [^-] everywhere else. > > It seems this syntax is a bashism. From glob(7): "POSIX has declared > the effect of a wildcard pattern "[^...]" to be undefined" (and I > confirm it does not work with dash) > >> Note that this pattern will also match something >> like -rfrood, i.e. --reference=frood. Fixing that becomes tricky without regexp. > > hmmm... yes, it matches -rfrood (and it is what we want), but it does not > match --reference=frood, isn't? -rfrood and --reference=frood are the same thing, so no, we don't want it to match -rfrood. >> Anyway, the -d option doesn't really need to be checked. 'date -d foo -d bar' >> will ignore the first -d, so things work OK. It's just that you get the spurious >> warning. So we could limit to checking -f, and limit to -f|--file=*). In that >> case, if someone passes something like -uf we'll get an error and the build will >> most likely terminate, so that particular error can be fixed. > > You are right. However, since it may produce unexpected situation, I > prefer to identify precisely the cases where fakedate is used. I would also prefer that, but I don't think it's possible without relying on regex. This could work: if echo "$i" | grep -qE '^-([urI]*d|-date|[urI]*f|-file)'; then I notice now that you forgot a pattern for 'date --date yesterday' - that one is handled as well by the regex above. > > [...] >>> + ;; >>> + esac >>> + done >>> + if [ $INHIBIT -eq 0 ]; then >>> + echo "date: Warning: using \$SOURCE_DATE_EPOCH instead of true time" >&2 >> >> Is it really needed to print this warning? > > From user point of view, result of `date' when fakedate is installed > is unexpected. I prefer to warn. I'm just worried that it might confuse some script that captures stderr and tries to do something with it. Regards, Arnout
On Saturday 19 November 2016 14:26:40 Arnout Vandecappelle wrote: > > On 19-11-16 14:06, Jérôme Pouiller wrote: > > On Saturday 19 November 2016 11:21:39 Arnout Vandecappelle wrote: > >> On 18-11-16 10:10, Jérôme Pouiller wrote: [...] > >> Note that this pattern will also match something > >> like -rfrood, i.e. --reference=frood. Fixing that becomes tricky without regexp. > > > > hmmm... yes, it matches -rfrood (and it is what we want), but it does not > > match --reference=frood, isn't? > > -rfrood and --reference=frood are the same thing, so no, we don't want it to > match -rfrood. Ok, I get the point. > >> Anyway, the -d option doesn't really need to be checked. 'date -d foo -d bar' > >> will ignore the first -d, so things work OK. It's just that you get the spurious > >> warning. So we could limit to checking -f, and limit to -f|--file=*). In that > >> case, if someone passes something like -uf we'll get an error and the build will > >> most likely terminate, so that particular error can be fixed. > > > > You are right. However, since it may produce unexpected situation, I > > prefer to identify precisely the cases where fakedate is used. > > I would also prefer that, but I don't think it's possible without relying on > regex. This could work: > > if echo "$i" | grep -qE '^-([urI]*d|-date|[urI]*f|-file)'; then It begins to be complex, but I do not see better ways. > I notice now that you forgot a pattern for 'date --date yesterday' - that one is > handled as well by the regex above. > > > > > [...] > >>> + ;; > >>> + esac > >>> + done > >>> + if [ $INHIBIT -eq 0 ]; then > >>> + echo "date: Warning: using \$SOURCE_DATE_EPOCH instead of true time" >&2 > >> > >> Is it really needed to print this warning? > > > > From user point of view, result of `date' when fakedate is installed > > is unexpected. I prefer to warn. > > I'm just worried that it might confuse some script that captures stderr and > tries to do something with it. I suggest to keep it until we find a script that captures stderr.
Hello Arnoult, On Saturday 19 November 2016 14:26:40 Arnout Vandecappelle wrote: > > On 19-11-16 14:06, Jérôme Pouiller wrote: > > On Saturday 19 November 2016 11:21:39 Arnout Vandecappelle wrote: > >> On 18-11-16 10:10, Jérôme Pouiller wrote: > > [...] > >>> + for i in "$@"; do > >>> + case $i in > >>> + -d|-[!-]*d|--date=*|-f|-[!-]*f|--file=*) > >> > >> We use [^-] everywhere else. > > > > It seems this syntax is a bashism. From glob(7): "POSIX has declared > > the effect of a wildcard pattern "[^...]" to be undefined" (and I > > confirm it does not work with dash) > > > >> Note that this pattern will also match something > >> like -rfrood, i.e. --reference=frood. Fixing that becomes tricky without regexp. > > > > hmmm... yes, it matches -rfrood (and it is what we want), but it does not > > match --reference=frood, isn't? > > -rfrood and --reference=frood are the same thing, so no, we don't want it to > match -rfrood. > > > >> Anyway, the -d option doesn't really need to be checked. 'date -d foo -d bar' > >> will ignore the first -d, so things work OK. It's just that you get the spurious > >> warning. So we could limit to checking -f, and limit to -f|--file=*). In that > >> case, if someone passes something like -uf we'll get an error and the build will > >> most likely terminate, so that particular error can be fixed. > > > > You are right. However, since it may produce unexpected situation, I > > prefer to identify precisely the cases where fakedate is used. > > I would also prefer that, but I don't think it's possible without relying on > regex. This could work: > > if echo "$i" | grep -qE '^-([urI]*d|-date|[urI]*f|-file)'; then From manual page, only option -u and -R do not take arguments. In add, we also have to inhibit fakedate is --reference (or -r) is detected. So, I think that the expression should be: '^-([uR]*d|-date|[uR]*f|-file|[uR]*r|--reference)'
diff --git a/package/fakedate/fakedate b/package/fakedate/fakedate new file mode 100755 index 0000000..2eded22 --- /dev/null +++ b/package/fakedate/fakedate @@ -0,0 +1,28 @@ +#!/bin/sh +# vim: set sw=4 expandtab: +# +# Licence: GPL +# Created: 2016-11-04 16:31:18+01:00 +# Main authors: +# - Jérôme Pouiller <jezz@sysmic.org> +# + +PATH=/bin:/usr/bin +LOG=/dev/null +if [ -n "$SOURCE_DATE_EPOCH" ]; then + INHIBIT=0 + for i in "$@"; do + case $i in + -d|-[!-]*d|--date=*|-f|-[!-]*f|--file=*) + INHIBIT=1 + ;; + esac + done + if [ $INHIBIT -eq 0 ]; then + echo "date: Warning: using \$SOURCE_DATE_EPOCH instead of true time" >&2 + echo "Catch call to date from `pwd` with parameters: '$@'" >> $LOG + exec date -d "@$SOURCE_DATE_EPOCH" "$@" + fi +fi + +exec date "$@" diff --git a/package/fakedate/fakedate.mk b/package/fakedate/fakedate.mk new file mode 100644 index 0000000..e81ce5d --- /dev/null +++ b/package/fakedate/fakedate.mk @@ -0,0 +1,14 @@ +################################################################################ +# +# fakedate +# +################################################################################ + +# source included in buildroot +HOST_FAKEDATE_LICENSE = GPLv2+ + +define HOST_FAKEDATE_INSTALL_CMDS + $(INSTALL) -D -m 755 package/fakedate/fakedate $(HOST_DIR)/usr/bin/date +endef + +$(eval $(host-generic-package))
`date' is widely used by packages to include build information in their binaries. Unfortunately, this is incompatible with BR2_REPRODUCIBLE. Instead to find all `date' invocation in build process, we add small tool allowing to alway return same date. This work was sponsored by `BA Robotic Systems'. Signed-off-by: Jérôme Pouiller <jezz@sysmic.org> --- package/fakedate/fakedate | 28 ++++++++++++++++++++++++++++ package/fakedate/fakedate.mk | 14 ++++++++++++++ 2 files changed, 42 insertions(+) create mode 100755 package/fakedate/fakedate create mode 100644 package/fakedate/fakedate.mk