Make build reproducible

[guimard]

Hi,

cyrus-imapd is fully reproducible (at least Debian package from 3.4.3-2 😉) except Perl/XS part which embeds some debug info (same for managesieve.xs)

│ │ │ ├── ./usr/lib/x86_64-linux-gnu/perl5/5.32/auto/Cyrus/IMAP/IMAP.so
│ │ │ │ │[...]
│ │ │ │ │ -/build/1st/cyrus-imapd-3.4.2/perl/imap/IMAP.c:1329
│ │ │ │ │ +/build/2/cyrus-imapd-3.4.2/2nd/perl/imap/IMAP.c:1329
│ │ │ │ │  	jmp    ea50 <Perl_xs_boot_epilog@plt>
│ │ │ │ │  	nopl   0x0(%rax)
│ │ │ │ │  
│ │ │ │ │  00000000000135e0 <MailboxToURL>:
│ │ │ │ │  MailboxToURL():
│ │ │ │ │  ./lib/imapurl.c:80
│ │ │ │ │  	push   %r15
│ │ │ │ ├── readelf --wide --decompress --hex-dump=.gnu_debuglink {}
│ │ │ │ │┄ error from `readelf --wide --decompress --hex-dump=.gnu_debuglink {}`:
│ │ │ │ │┄ readelf: Error: no .dynamic section in the dynamic segment
│ │ │ │ │┄ readelf: Warning: Separate debug info file /srv/reproducible-results/rbuild-debian/tmp.KkHPq3woU4/dbd-tmp-RsZD9QC/diffoscope_ayuykb6s_b2/tmpm5q43lfl_DebTarContainer/0/.debug/2eec2f61dc56b86b6c6432905049fee9df6d21.debug found, but CRC does not match - ignoring
│ │ │ │ │ @@ -1,7 +1,7 @@
│ │ │ │ │  
│ │ │ │ │  Hex dump of section '.gnu_debuglink':
│ │ │ │ │ -  0x00000000 32656563 32663631 64633536 62383662 2eec2f61dc56b86b
│ │ │ │ │ -  0x00000010 36633634 33323930 35303439 66656539 6c6432905049fee9
│ │ │ │ │ -  0x00000020 64663664 32312e64 65627567 00000000 df6d21.debug....
│ │ │ │ │ -  0x00000030 ea962d76                            ..-v
│ │ │ │ │ +  0x00000000 36383739 61383432 66623964 33373637 6879a842fb9d3767
│ │ │ │ │ +  0x00000010 34333038 30323134 62383738 63636430 43080214b878ccd0
│ │ │ │ │ +  0x00000020 32353666 36302e64 65627567 00000000 256f60.debug....
│ │ │ │ │ +  0x00000030 d270b308                            .p..

This is probably linked to missing CFLAGS/CPPFLAGS/LDFLAGS propagation when building Perl/XS files (see Debian BHLC job)

My reprotest artifacts are here (temporary artifacts link). To see difference:

• download artifacts and unzip it
• extract debs:

dpkg -x debian/output/reprotest/control/source-root/libcyrus-imap-perl_3.4.3-1+salsaci_amd64.deb build1
dpkg -x debian/output/reprotest/experiment-1/source-root/libcyrus-imap-perl_3.4.3-1+salsaci_amd64.deb build2

• compare build1/usr/lib/x86_64-linux-gnu/perl5/5.32/auto/Cyrus/IMAP/IMAP.so and build2/usr/lib/x86_64-linux-gnu/perl5/5.32/auto/Cyrus/IMAP/IMAP.so

Thanks a lot for providing such amazing software !

[lamby]

Yes, this is almost certainly an issue with CFLAGS/CPPFLAGS/LDFLAGS not being correctly propagated to the 'sub-compilation' of IMAP.c. You can actually see this occurring in the build log:

x86_64-linux-gnu-gcc -c  -I../.. -I../../com_err/et    -I../../perl/imap -D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fwrapv -fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -O2 -g   -DVERSION=\"1.00\" -DXS_VERSION=\"1.00\" -fPIC "-I/usr/lib/x86_64-linux-gnu/perl/5.32/CORE"  -DPERL_POLLUTE IMAP.c

... but previous compilations in the log look like this:

gcc -DHAVE_CONFIG_H -I.   -I. -I./lib -I. -I./lib -DLIBEXEC_DIR=\"/usr/lib/cyrus/bin\" -DSBIN_DIR=\"/usr/lib/cyrus/bin\" -DSYSCONF_DIR=\"/etc\" -DHAVE_CONFIG_H      -I/usr/include/libxml2             -I/usr/include/postgresql -Wdate-time -D_FORTIFY_SOURCE=2 -I/usr/include -fPIC  -g -O2 **-ffile-prefix-map**=/home/lamby/temp/cdt.20220202081919.AtqO782Fyq.repro.cyrus-imapd/build-a/cyrus-imapd-3.4.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wall -Wextra -g -fno-strict-aliasing -pipe -O2 -c -o master/service-thread.o master/service-thread.c

Note, in particular, the presence of -ffile-prefix-map in this second snippet.

[guimard]

This fixes half of the problem (both bhlc and reprotest) :

--- a/perl/imap/Makefile.PL.in
+++ b/perl/imap/Makefile.PL.in
@@ -88,12 +88,12 @@
                    'IMCLIENT_LIBS' => '',      # hack
                },
     'clean'    => {'FILES' => 'libcyrperl.a cyradm'},
-    'LD'       => $Config{ld} . ' @GCOV_LDFLAGS@',
+    'LD'       => $Config{ld} . ' @GCOV_LDFLAGS@' . ' @LDFLAGS@',
     'OBJECT'    => 'IMAP.o',
     'MYEXTLIB'  => '@top_builddir@/perl/.libs/libcyrus.a @top_builddir@/perl/.libs/libcyrus_min.a',
     'LIBS'     => [ "$LIB_SASL @SSL_LIBS@ @LIB_UUID@ @ZLIB@ @GCOV_LIBS@ @LIBCAP_LIBS@"],
     'DEFINE'   => '-DPERL_POLLUTE',    # e.g., '-DHAVE_SOMETHING'
-    'INC'      => "-I@top_srcdir@ -I@top_srcdir@/com_err/et @SASLFLAGS@ @SSL_CPPFLAGS@ @GCOV_CFLAGS@ -I@top_srcdir@/perl/imap",
+    'INC'      => "-I@top_srcdir@ -I@top_srcdir@/com_err/et @SASLFLAGS@ @SSL_CPPFLAGS@ @CFLAGS@ @CPPFLAGS@ @GCOV_CFLAGS@ -I@top_srcdir@/perl/imap",
     'EXE_FILES' => [cyradm],
     # This is a disgusting hack to effectively disable the stupid
     # behaviour of the generated Makefile which moves itself aside

Now, I'm trying this:

--- a/perl/sieve/managesieve/Makefile.PL.in
+++ b/perl/sieve/managesieve/Makefile.PL.in
@@ -72,9 +72,9 @@
     'LIBS'     => ["$LIB_SASL @SSL_LIBS@ @LIB_UUID@ @LIB_REGEX@ @ZLIB@ @SQLITE_LIBADD@ @MYSQL_LIBADD@ @PGSQL_LIBADD@"],
     'CCFLAGS'  => '@GCOV_CFLAGS@',
     'DEFINE'   => '-DPERL_POLLUTE',     # e.g., '-DHAVE_SOMETHING' 
-    'INC'      => "-I@top_srcdir@/lib -I@top_srcdir@/perl/sieve -I@top_srcdir@/perl/sieve/lib @SASLFLAGS@ @SSL_CPPFLAGS@",
+    'INC'      => "-I@top_srcdir@/lib -I@top_srcdir@/perl/sieve -I@top_srcdir@/perl/sieve/lib @SASLFLAGS@ @SSL_CPPFLAGS@ @CFLAGS@ @CPPFLAGS@",
     'OBJECT'    => 'managesieve.o',
-    'LD'       => $Config{ld} . ' @GCOV_LDFLAGS@',
+    'LD'       => $Config{ld} . ' @GCOV_LDFLAGS@ @LDFLAGS@',
     # This is a disgusting hack to effectively disable the stupid
     # behaviour of the generated Makefile which moves itself aside
     # on a 'make clean' instead of on 'make distclean'.

[guimard]

Confirmed, this patch fixes both bhlc and reprotest: https://salsa.debian.org/debian/cyrus-imapd/-/pipelines/344141

--- a/perl/imap/Makefile.PL.in
+++ b/perl/imap/Makefile.PL.in
@@ -88,12 +88,12 @@
                    'IMCLIENT_LIBS' => '',      # hack
                },
     'clean'    => {'FILES' => 'libcyrperl.a cyradm'},
-    'LD'       => $Config{ld} . ' @GCOV_LDFLAGS@',
+    'LD'       => $Config{ld} . ' @GCOV_LDFLAGS@ @LDFLAGS@',
     'OBJECT'    => 'IMAP.o',
     'MYEXTLIB'  => '@top_builddir@/perl/.libs/libcyrus.a @top_builddir@/perl/.libs/libcyrus_min.a',
     'LIBS'     => [ "$LIB_SASL @SSL_LIBS@ @LIB_UUID@ @ZLIB@ @GCOV_LIBS@ @LIBCAP_LIBS@"],
     'DEFINE'   => '-DPERL_POLLUTE',    # e.g., '-DHAVE_SOMETHING'
-    'INC'      => "-I@top_srcdir@ -I@top_srcdir@/com_err/et @SASLFLAGS@ @SSL_CPPFLAGS@ @GCOV_CFLAGS@ -I@top_srcdir@/perl/imap",
+    'INC'      => "-I@top_srcdir@ -I@top_srcdir@/com_err/et @SASLFLAGS@ @SSL_CPPFLAGS@ @CFLAGS@ @CPPFLAGS@ @GCOV_CFLAGS@ -I@top_srcdir@/perl/imap",
     'EXE_FILES' => [cyradm],
     # This is a disgusting hack to effectively disable the stupid
     # behaviour of the generated Makefile which moves itself aside
--- a/perl/sieve/managesieve/Makefile.PL.in
+++ b/perl/sieve/managesieve/Makefile.PL.in
@@ -72,9 +72,9 @@
     'LIBS'     => ["$LIB_SASL @SSL_LIBS@ @LIB_UUID@ @LIB_REGEX@ @ZLIB@ @SQLITE_LIBADD@ @MYSQL_LIBADD@ @PGSQL_LIBADD@"],
     'CCFLAGS'  => '@GCOV_CFLAGS@',
     'DEFINE'   => '-DPERL_POLLUTE',     # e.g., '-DHAVE_SOMETHING' 
-    'INC'      => "-I@top_srcdir@/lib -I@top_srcdir@/perl/sieve -I@top_srcdir@/perl/sieve/lib @SASLFLAGS@ @SSL_CPPFLAGS@",
+    'INC'      => "-I@top_srcdir@/lib -I@top_srcdir@/perl/sieve -I@top_srcdir@/perl/sieve/lib @SASLFLAGS@ @SSL_CPPFLAGS@ @CFLAGS@ @CPPFLAGS@",
     'OBJECT'    => 'managesieve.o',
-    'LD'       => $Config{ld} . ' @GCOV_LDFLAGS@',
+    'LD'       => $Config{ld} . ' @GCOV_LDFLAGS@ @LDFLAGS@',
     # This is a disgusting hack to effectively disable the stupid
     # behaviour of the generated Makefile which moves itself aside
     # on a 'make clean' instead of on 'make distclean'.

[guimard]

I just filed #3895 to master branch and released Debian package cyrus-imapd-3.4.3-3 which includes above patch (pushed to Debian unstable)

[elliefm]

Do we know that the perl modules still work with these patches? Or only that the build is now reproducible?

I have a very vague and maybe incorrect memory that XS modules need to be built with the same xxFLAGS that perl itself was, and that might be we why don't pass Cyrus's own CFLAGS etc down (except for where it's really needed for linking e.g. gcov, sasl, ssl). Does it make any difference in practice though? I have no idea.

[guimard]

At least test passed...

[guimard]

All Perl module are compiled with these flags in Debian, we never encored any problem with them

[guimard]

We can also wait for Debian users tests, cyrus-imapd will enter in testing (and then I'll push it to backports). Then I'll rebase this MR. Do you agree?

[elliefm]

That sounds reasonable to me. I believe the Cyrus perl modules are a bit of a black hole when it comes to testing, so just because "tests pass" doesn't mean they're not broken, if the tests never try to use them. And I don't really have a useful way to test them directly myself.

Since the patch is already in Debian unstable, let's see what reports shake out from that. And if it turns out it's fine there, then I'll consider it tested, and I'll be happy to merge it here so you don't need to keep maintaining the patch.

Thanks!

[guimard]

Hi @elliefm, nobody in Debian reported anything about this change. I think there is no risk to merge it

[elliefm]

Oh, I had forgotten about this particular change!

Someone popped up on the info-cyrus mailing list a day or two ago complaining about a cyradm problem on Debian Testing (3.4.3-3+b1). It looks like a libpcre2/3 linkage problem, so I directed them to #3861. But maybe it's related to this as well.

Here's the thread:
https://cyrus.topicbox.com/groups/info/T499290c4eacdba28-Md8614925e1a4b13aee055246

[guimard]

I tried to join conversation (code never come). Even if libpcre2 is installed, cyrus-imapd is linked only with old libpcre3

[guimard]

Looking at this thread, this MR breaks Cyrus/Perl. Closing for now

[guimard]

Sorry, closing #3895, not this issue

[guimard]

Hi,

as you can see here, there are some other values in CFLAGS that pollutes the build

[elliefm]

I'm not really sure what I'm seeing there. There seems to be three kinds of differences:

1. things that look like checksums, sha1 sums, or something, sometimes referred to as a "build ID"
2. files that contain the build path in them, and then differ because the paths are different
3. offset changes, which appear to be just because the second build path was two bytes longer than the first build path, and the build path was embedded in the file, so all the offsets within the second version of the file are +2 compared to those in the first file

The two build paths seem to be:

/build/1st/cyrus-imapd-3.6.0~beta3/
/build/2/cyrus-imapd-3.6.0~beta3/2nd/

Is it deliberate that they're structured differently? If the two paths were structured the same, I think most of the reported differences would go away.

[guimard]

Hi,

1. you can ignore checksums differences, they are consequence of other things
2. "files that contain the build path in them, and then differ because the paths are different", "offset changes" : that's the goal of reproducible test, build result shouldn't change. As you can see, only Perl files differ here because global XXFLAGS aren't exported. The whole other Cyrus files build is totally reproducible.

But propagating XXFLAGS to Perl files breaks them (see related thread). I updated this issue for the record, it stays a minor issue.

Cheers,
Yadd

[lamby]

The two build paths seem to be:

/build/1st/cyrus-imapd-3.6.0~beta3/
/build/2/cyrus-imapd-3.6.0~beta3/2nd/

Is it deliberate that they're structured differently?

Oh yes, it is absolutely deliberate that they are structured differently! And there are actually three differences in the paths that are useful when flushing out reproducibility issues. I list them here in some slight detail for future reference:

1. Firstly, the paths quite obviously do not contain the exact same bytes. This replicates the real world in that the absolute build path will vary from machine to machine.
2. They are of different lengths. This means that your statement "If the two paths were structured the same, I think most of the reported differences would go away." maybe indeed be true in a roundabout way: if the build path is embedded in the binary and that build path is different in the second build, then parts of the second binary are likely to be pushed further along and it is very likely offsets will change in some headers as a result. However, all this is merely an effect of embedding the build path in the binary to begin with, rather than something to be fixed in isolation.. somewhat analogous to the "fixing" of the Build ID; it is merely a consequence, not a cause.
3. The basename(1) of the two paths differ, specifically cyrus-imapd-3.6.0~beta3 vs 2nd. This flushes out issues in scripts that look at the basename of the absolute build path and embed them in artifacts. I doubt this is an issue here, but a surprising number of programs parse the last directory component configure scripts, even if they do not go on to embed the full path.

[elliefm]

Oh, I see; and:

Note, in particular, the presence of -ffile-prefix-map in this second snippet.

You already told me about this back in February, but I didn't understand the significance at the time, and then Yadd provided a patch so I didn't think further about it, until the patch turned out to not work...

I think it seems clear from Yadd's experiments that my hunch about perl XS's need for particular xxFLAGS was correct, and so directly passing all of Cyrus's xxFLAGS down to the perl build just breaks things. But I wonder if -ffile-prefix-map is one of the things it cares about or not -- if we just pass that one flag down, maybe that will work?

That said, I don't know where -ffile-prefix-map flag is even coming from -- the string "file-prefix-map" doesn't exist in the repository, and it isn't used in my builds. For comparison, here's my master/service-thread.o build (this file was the example from February):

ellie@debian:master:~/fastmail/cyrus-imapd$make V=1 master/service-thread.o
depbase=`echo master/service-thread.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`;\
gcc -DHAVE_CONFIG_H -I.  -I/home/ellie/fastmail/cyrus-imapd/com_err/et -I. -I./lib -I. -I./lib -DLIBEXEC_DIR=\"/dev/shm/cyrus/main/libexec\" -DSBIN_DIR=\"/dev/shm/cyrus/main/sbin\" -DSYSCONF_DIR=\"/etc\" -DHAVE_CONFIG_H    -I/usr/local/cyruslibs/include  -I/usr/include/libxml2  -I/usr/local/cyruslibs/include    -I/usr/local/cyruslibs/include    -I/usr/local/cyruslibs/include  -I/usr/local/cyruslibs/include -I/usr/local/cyruslibs/include -I/usr/include/uuid  -fPIC  -g -O0 -Wall -Wextra -Werror -Wl,--as-needed -fstack-protector-all  -MT master/service-thread.o -MD -MP -MF $depbase.Tpo -c -o master/service-thread.o master/service-thread.c &&\
mv -f $depbase.Tpo $depbase.Po

So I guess this flag must be added by the Debian package build, not by Cyrus itself. That makes it tricky. If the Cyrus build was adding it, I could simply find what's doing it and do the same in the perl part. But if it's coming from outside, I can't just do-the-same-thing, because I don't know what's been done.

[lamby]

I'm not the Debian maintainer so I might not be 100% correct, but my guess is that the flag is coming from here:

https://sources.debian.org/src/cyrus-imapd/3.6.0~beta3-1/debian/rules/#L25

Look along line 25: the dpkg-buildflags(1) mechanism is a distribution-wide mechanism for injection compiler flags into builds. Packages are meant to respect the flags coming from this tool, and, relevant to this conversation, this is exactly where -ffile-prefix-map comes from. You can see the other feature areas this mechanism supports on the dpkg-buildflags manpage. The $(dpkg-buildflags --get CFLAGS) bit expands to a whole bunch of stuff -- on my system, it returns -g -O2 -ffile-prefix-map=/home/lamby=. -fstack-protector-strong -Wformat -Werror=format-security.

Maybe there is another variable like PERL_MM_OPT that needs to include a call to dpkg-buildflags, I don't exactly know I'm afraid. :)