Hi KiBi, Many thanks for reviewing this.
Cyril Brulebois wrote: > Please make sure not to depend on features which are not found in stable > (I'm not entirely sure about oldstable at this point), which might hinder > our ability to cherry-pick bits and pieces from master to jessie. I think I could make that easier by splitting into smaller commits: * parts only needed for kfreebsd * parts more appropriate for stable, oldstable, etc. * parts only appropriate for sid (or just not use new GNU tar features yet) > > --- a/build/Makefile > > +++ b/build/Makefile > > @@ -56,7 +56,7 @@ > > # Add to PATH so dpkg will always work, and so local programs will be > > found. > > PATH := util:$(PATH):/usr/sbin:/sbin > > EATMYDATA = $(shell which eatmydata 2>/dev/null) > > -GZIP = $(shell which pigz gzip | head -1) > > +GZIP = $(shell which pigz gzip | head -1) -n > > I think I already added -n to a bunch of calls. Not sure whether adding > it here once and for all would be better than adding it where it's > missing though. Anyway, not my biggest question/comment/concern here. Seems reasonable to factor out and put it here. If we don't, someone may add a new $GZIP call later, forget -n and make it unreproducible again. Although it is a macro here, GZIP is also the name of an environment variable used by gzip (but not pigz?), which is likely confusing. And the later tar invocations call gzip (not pigz) in quite a few places regardless of the GZIP macro contents; those do look at the GZIP environment variable though. > I think those 3(.5) occurrences really should be factorized, especially > given the logic is the same: replacing "cd foo && tar bar ." with more > code. Somewhere under build/util would probably be suitable. I agree. It is a very common pattern in other Debian packages too, and it often needs patching for reproducibility. > > --- a/build/config/x86.cfg > > +++ b/build/config/x86.cfg > > @@ -332,6 +332,11 @@ arch_miniiso: x86_syslinux x86_grub_efi > > | todos > $(TEMP_CD_TREE)/win32-loader.ini; \ > > fi > > > > + # Clamp timestamps to be no newer than last changelog entry, see > > + # https://wiki.debian.org/ReproducibleBuilds/TimestampsInTarball > > + find $(TEMP_CD_TREE) -newermt "$(SOURCE_DATE)" -print0 \ > > + | xargs -0r touch --no-dereference --date="$(SOURCE_DATE)" > > + > [...] above is using SOURCE_DATE, which is undefined as > far as I can tell since SOURCE_DATE_EPOCH is what's getting defined. > Maybe it should call the same new util (with different parameters since > we only need touch here, and no tar call)? That's a typo, and was buggy - it would touch all timestamps, not just the ones necessary. I may drop this part as I don't think it's needed any more with the newer makefs, which will clamp the timestamps itself. > Finally, not everything is built using debian/rules targets (with or > without dpkg-buildpackage). One should still be able to just run e.g.: > “make -C build build_netboot-gtk USE_UDEBS_FROM=sid” > > See BUILD_DATE handling, for example. We end up with a default setting > through: > | build/config/common:BUILD_DATE ?= $(shell date -u '+%Y%m%d-%H:%M') That would not be reproducible, then! (it is embedded in the tarballs) > [ Please note that calling $(shell dpkg-parsechangelog -SDate) to set > SOURCE_DATE_EPOCH there would only work when building from the toplevel > directory, and not from the build/ subdirectory for example. ] If it's anyway not going to be reproducible, we could similarly fall back to a SOURCE_DATE_EPOCH ?= now; or the caller could choose to provide them. Regards, -- Steven Chamberlain ste...@pyro.eu.org
Description: Digital signature
_______________________________________________ Reproducible-builds mailing list Reproducibleemail@example.com http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/reproducible-builds