[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#783210: glibc: please make the package build reproducibly



On 2016-12-09 18:29, Ximin Luo wrote:
> - the upstream folks
> 
> Aurelien Jarno:
> > On 2016-06-06 02:48, Ximin Luo wrote:
> >> On Mon, 19 Oct 2015 10:11:25 +0200 Aurelien Jarno <aurelien@aurel32.net> wrote:
> >>> It means that the point 3 (usage of __DATE__ and __TIME__) is the only
> >>> one left to address in the version currently in experimental.
> >>>
> >>
> >> Hi, GCC have now added support for SOURCE_DATE_EPOCH so we can use this instead. Debhelper now sets this environment variable automatically, and GCC should automatically read it, so perhaps we can get rid of the whole (3)-related patch, but I haven't tested this yet.
> > 
> > In the current code __DATE__ and __TIME__ are used to generate a unique
> > serial number shared between two processes. Therefore using
> > SOURCE_DATE_EPOCH for that is not correct here.
> > 
> > I have submitted a patch upstream to fix that another way [1].
> > Unfortunately there is still work to do and I am currently lacking time
> > to work on that. Help would be appreciated.
> > 
> 
> Hi Aurelian, I noticed that due to GCC supporting SOURCE_DATE_EPOCH and dpkg-buildpackage automatically setting this now, glibc is actually already reproducible [1] if you apply this patch. I've attached an updated version of it in this email, for convenience. (It's the same as what I attached to [1].)
>
> The issue you mentioned, of this "unique id", is *already* a problem in the currently-released version (in Debian testing and unstable) because of the GCC+dpkg-buildpackage changes. So I think it doesn't make much sense to keep holding this patch off now.

I don't get it. On one side you says that dpkg-buildpackage is already
exporting SOURCE_DATE_EPOCH so that the bug is already fixed. Then you
said that's the reason why the patch should be apply to fix the bug.

Why do we need to export SOURCE_DATE_EPOCH if it is already done by
dpkg-buildpackage?

> The concrete benefit of applying this patch, is that tests.r-b.org would be able to produce sensible diffoscope output for glibc, so we could track both [1] and any future cases of unreproducibility. Currently it times out for glibc, because there are too many differences, due to the patch not being applied.

I fail to understand why it prevents to have sensible diffoscope. The
current issue only concerns one binary in a single package. The other
parts of the provided patch have been applied more than one year ago.

> What do you think?

It would be nice to have an explanation why the changes from your patch
are needed. See my comments below.


> diff -Nru glibc-2.24/debian/rules glibc-2.24/debian/rules
> --- glibc-2.24/debian/rules	2016-11-25 21:59:04.000000000 +0000
> +++ glibc-2.24/debian/rules	2016-11-15 18:03:37.000000000 +0000
> @@ -45,6 +45,7 @@
>  
>  DEB_SOURCE_PACKAGE := $(strip $(shell egrep '^Source: ' debian/control | cut -f 2 -d ':'))
>  
> +SOURCE_DATE_EPOCH ?= $(shell dpkg-parsechangelog -STimestamp)
>  DEB_VERSION := $(shell dpkg-parsechangelog | egrep '^Version:' | cut -f 2 -d ' ')
>  GLIBC_VERSION = $(shell echo $(DEB_VERSION) | sed -e 's/.*://' -e 's/[+-].*//')

You told above that SOURCE_DATE_EPOCH is already set by
dpkg-buildpackage, so why do we need to do that?


> diff -Nru glibc-2.24/debian/rules.d/build.mk glibc-2.24/debian/rules.d/build.mk
> --- glibc-2.24/debian/rules.d/build.mk	2016-11-25 12:02:24.000000000 +0000
> +++ glibc-2.24/debian/rules.d/build.mk	2016-11-15 18:03:37.000000000 +0000
> @@ -316,18 +316,16 @@
>  $(stamp)source: $(stamp)patch
>  	mkdir -p $(build-tree)
>  	cd .. && \
> -	       find $(GLIBC_SOURCES) -depth -newermt '$(DEB_BUILD_DATE)' \
> -			-print0 | \
> -               xargs -0r touch --no-dereference --date='$(DEB_BUILD_DATE)'
> -	cd .. && \
>  		find $(GLIBC_SOURCES) -print0 | \
>  		LC_ALL=C sort -z | \
>  		tar -c -J --null -T - --no-recursion \
>  			--mode=go=rX,u+rw,a-s \
> +			--clamp-mtime --mtime "@$(SOURCE_DATE_EPOCH)" \
>  			--owner=root --group=root --numeric-owner \
>  			-f $(CURDIR)/$(build-tree)/glibc-$(GLIBC_VERSION).tar.xz
>  	mkdir -p debian/glibc-source/usr/src/glibc
>  	tar cf - --files-from debian/glibc-source.filelist \
> +		--clamp-mtime --mtime "@$(SOURCE_DATE_EPOCH)" \
>  	  | tar -x -C debian/glibc-source/usr/src/glibc -f -

Why do we need to change this, what doesn't work in the current code? The current code
has been provided by Jérémy Bobbio in the current bug.

>  	touch $@
> diff -Nru glibc-2.24/debian/rules.d/debhelper.mk glibc-2.24/debian/rules.d/debhelper.mk
> --- glibc-2.24/debian/rules.d/debhelper.mk	2016-11-25 22:08:30.000000000 +0000
> +++ glibc-2.24/debian/rules.d/debhelper.mk	2016-11-15 18:03:37.000000000 +0000
> @@ -77,8 +77,7 @@
>  		-exec chmod a+x '{}' ';'
>  	dh_makeshlibs -Xgconv/ -p$(curpass) -V "$(call xx,shlib_dep)"
>  	# Add relevant udeb: lines in shlibs files
> -	chmod a+x debian/shlibs-add-udebs
> -	./debian/shlibs-add-udebs $(curpass)
> +	sh ./debian/shlibs-add-udebs $(curpass)
>  
>  	dh_installdeb -p$(curpass)
>  	dh_shlibdeps -p$(curpass)

Why do we need this change?

Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

Attachment: signature.asc
Description: PGP signature


Reply to: