Re: bash: Please make bash build reproducibly

[Ximin Luo]

Control: tags -1 + patch

I've attached the full Debian patch to make bash 4.3-14 reproducible, that 
includes Reiner's from above.

It also includes *fixing a bug in upstream bash*, which currently 
does-not-appear-in-the-wild *only because* distros already happen to be working 
around it. But upstream really should fix it - i.e. to fix the PGRP_PIPE check 
in configure/configure.ac to account for Linux 4+.

I decided to just set PGRP_PIPE unconditionally in configure.ac and configure, 
because I figure nobody will ever again use linux 0, 1 or 2 so it's not worth 
the extra complexity (which only gives a minor performance gain anyway, as 
opposed to having *incorrect behaviour*). Upstream is free to choose whichever 
behaviour he wants - either this simpler version, or the more complex version 
from my previous email quoted below. Both are correct, if I understand right, 
and when either is applied to upstream, all distros (including Debian) can drop 
our specific patches for PGRP_PIPE.

To re-iterate again, this does not solve the longer-term issue of "installing 
config.h is bad".

Ximin

Ximin Luo:
> (Chet, your specific attention is required for this email, please)
> 
> Ximin Luo:
>> On Sat, 28 May 2016 13:38:35 +0200 Reiner Herrmann  
>> wrote:
>>> After that, the only remaining issue is that the included header file
>>> /usr/include/bash/config.h varies depending on the kernel version used
>>> during build [1] (with kernel <4, PGRP_PIPE is defined).
>>
>> For this particular example, we can just patch this out, i.e. remove it from 
>> the installed config.h. Debian already forces PGRP_PIPE 1 in config-bot.h, 
>> which config.h includes at the end.
>>
> 
> I dug into this a bit more and it looks like the cause of the difference is 
> this snippet from configure.ac:
> 
> linux*) LOCAL_LDFLAGS=-rdynamic  # allow dynamic loading
> case "`uname -r`" in
> 2.[[456789]]*|3*)   AC_DEFINE(PGRP_PIPE) ;;
> esac ;;
> 
> 
> This was added between bash-3.0.16 and bash-3.1, way before Linux 4 came out. 
> So I wonder if this snippet should instead be:
> 
> linux*) LOCAL_LDFLAGS=-rdynamic  # allow dynamic loading
> case "`uname -r`" in
> 1.*|2.[[0123]]*) true ;;
> *) AC_DEFINE(PGRP_PIPE) ;;
> esac ;;
> 
> to set this for all future kernels? Then Debian (and probably other distros) 
> could get rid of our patch, too.
> 
> However, the question still remains why config.h is installed into the 
> end-user system, and if bash-built-with-linux-5 required PGRP_PIPE to be 
> *undefined*, we would still have a reproducibility problem.
> 
> Ximin
> 


-- 
GPG: ed25519/56034877E1F87C35
GPG: rsa4096/1318EFAC5FBBDBCE
git://github.com/infinity0/pubkeys.git
diff -Nru bash-4.3/debian/changelog bash-4.3/debian/changelog
--- bash-4.3/debian/changelog   2015-09-01 01:04:48.0 +0200
+++ bash-4.3/debian/changelog   2016-06-07 11:56:09.0 +0200
@@ -1,3 +1,15 @@
+bash (4.3-14.0~reproducible1) UNRELEASED; urgency=medium
+
+  [ Ximin Luo ]
+  * Non-maintainer upload.
+  * Set PGRP_PIPE unconditionally on Linux, reproducibly.
+
+  [ Reiner Herrmann ]
+  * Use the system man2html instead of the embedded one, for better build
+reproducibility.
+
+ -- Ximin Luo   Tue, 07 Jun 2016 11:56:07 +0200
+
 bash (4.3-14) unstable; urgency=medium
 
   * Apply upstream patches 040 - 042.
diff -Nru bash-4.3/debian/control bash-4.3/debian/control
--- bash-4.3/debian/control 2015-01-28 17:13:32.0 +0100
+++ bash-4.3/debian/control 2016-06-06 03:00:38.0 +0200
@@ -5,7 +5,7 @@
 Standards-Version: 3.9.6
 Build-Depends: autoconf, autotools-dev, bison, libncurses5-dev,
  texinfo, texi2html, debhelper (>= 5), locales, gettext, sharutils, time,
- xz-utils, dpkg-dev (>= 1.16.1)
+ xz-utils, dpkg-dev (>= 1.16.1), man2html
 Build-Depends-Indep: texlive-latex-base, ghostscript, texlive-fonts-recommended
 Homepage: http://tiswww.case.edu/php/chet/bash/bashtop.html
 Vcs-Browser: https://code.launchpad.net/~doko/+junk/pkg-bash-debian
diff -Nru bash-4.3/debian/patches/pgrp-pipe.diff 
bash-4.3/debian/patches/pgrp-pipe.diff
--- bash-4.3/debian/patches/pgrp-pipe.diff  2013-10-23 14:41:22.0 
+0200
+++ bash-4.3/debian/patches/pgrp-pipe.diff  2016-06-07 12:17:05.0 
+0200
@@ -1,11 +1,43 @@
-# DP: Define PGRP_PIPE to avoid race condition.
-
 a/config-bot.h
-+++ b/config-bot.h
-@@ -197,3 +197,6 @@
- 
- /* If you don't want bash to provide a default mail file to check. */
- #undef DEFAULT_MAIL_DIRECTORY
-+
-+/* Bug #224543 */
-+#define PGRP_PIPE 1
+Description: Set PGRP_PIPE unconditionally on Linux, reproducibly
+ The original fix to #224543 involved defining this unconditionally in
+ config-bot.h. Unfortunately, upstream 

Re: bash: Please make bash build reproducibly

[Ximin Luo]

(Chet, your specific attention is required for this email, please)

Ximin Luo:
> On Sat, 28 May 2016 13:38:35 +0200 Reiner Herrmann  wrote:
>> After that, the only remaining issue is that the included header file
>> /usr/include/bash/config.h varies depending on the kernel version used
>> during build [1] (with kernel <4, PGRP_PIPE is defined).
> 
> For this particular example, we can just patch this out, i.e. remove it from 
> the installed config.h. Debian already forces PGRP_PIPE 1 in config-bot.h, 
> which config.h includes at the end.
> 

I dug into this a bit more and it looks like the cause of the difference is 
this snippet from configure.ac:

linux*) LOCAL_LDFLAGS=-rdynamic  # allow dynamic loading
case "`uname -r`" in
2.[[456789]]*|3*)   AC_DEFINE(PGRP_PIPE) ;;
esac ;;


This was added between bash-3.0.16 and bash-3.1, way before Linux 4 came out. 
So I wonder if this snippet should instead be:

linux*) LOCAL_LDFLAGS=-rdynamic  # allow dynamic loading
case "`uname -r`" in
1.*|2.[[0123]]*) true ;;
*) AC_DEFINE(PGRP_PIPE) ;;
esac ;;

to set this for all future kernels? Then Debian (and probably other distros) 
could get rid of our patch, too.

However, the question still remains why config.h is installed into the end-user 
system, and if bash-built-with-linux-5 required PGRP_PIPE to be *undefined*, we 
would still have a reproducibility problem.

Ximin

-- 
GPG: ed25519/56034877E1F87C35
GPG: rsa4096/1318EFAC5FBBDBCE
git://github.com/infinity0/pubkeys.git

Re: bash: Please make bash build reproducibly

[Ximin Luo]

CC'ing everyone that was on the previous conversation; Debian's BTS doesn't do 
this automatically :(

We've made some progress on the Debian side; there's a few more stumbling 
blocks though:

On Sat, 28 May 2016 13:38:35 +0200 Reiner Herrmann  wrote:
> Hi,
> 
> it looks like most of the documentation related issues are now solved
> by fixed toolchain packages.  But it still uses the embedded and
> outdated copy of man2html, which doesn't support SOURCE_DATE_EPOCH yet.
> The attached patch uses the system man2html instead of the embedded one.
> 
> After that, the only remaining issue is that the included header file
> /usr/include/bash/config.h varies depending on the kernel version used
> during build [1] (with kernel <4, PGRP_PIPE is defined).
> 

For this particular example, we can just patch this out, i.e. remove it from 
the installed config.h. Debian already forces PGRP_PIPE 1 in config-bot.h, 
which config.h includes at the end.

In general however, installing config.h is a code smell and an anti-pattern - 
it takes test results from the *build machine*, and then forces my machine to 
assume those. The presence of this anti-pattern potentially can make future 
versions unreproducible again, and we'll have to think of new ways to fix 
those, since this PGRP_PIPE forcing is just a lucky coincidence.

The ideal solution from a software architecture viewpoint, would be (1) make 
the headers platform independent and not require a config.h, or if this is 
truly impossible then (2) instead of installing config.h, install some scripts 
to allow the user to generate their own config.h, with their own values. 
However, I don't know how much effort either of these options are. Only a few 
installed headers actually need config.h; perhaps these could be fixed to *not* 
require it:

/usr/include/bash$ grep -r '#include .config.h.'
builtins.h:#include "config.h"
lib/glob/strmatch.h:#include 
shell.h:#include "config.h"
shmbutil.h:#include 

X

-- 
GPG: ed25519/56034877E1F87C35
GPG: rsa4096/1318EFAC5FBBDBCE
git://github.com/infinity0/pubkeys.git