bug#27593: groff build is not reproducible

bug#27593: groff build is not reproducible

[Danny Milosavljevic]

on core-updates

bug#27593: [PATCH] gnu: groff: Remove timestamps.

[Danny Milosavljevic]

Fixes <https://bugs.gnu.org/27593>.

* gnu/packages/groff.scm (groff)[source]: Remove timestamps.
---
 gnu/packages/groff.scm | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/groff.scm b/gnu/packages/groff.scm
index 46e1ccf23..e5dd732ab 100644
--- a/gnu/packages/groff.scm
+++ b/gnu/packages/groff.scm
@@ -40,7 +40,20 @@
             (uri (string-append "mirror://gnu/groff/groff-" version
                                 ".tar.gz"))
             (sha256 (base32
-                     "1998v2kcs288d3y7kfxpvl369nqi06zbbvjzafyvyl3pr7bajj1s"))))
+                     "1998v2kcs288d3y7kfxpvl369nqi06zbbvjzafyvyl3pr7bajj1s"))
+            (modules '((guix build utils)))
+            (snippet
+             '(begin
+                (substitute* "mdate.sh"
+                 (("^LANGUAGE=") "
+
+echo \"1 Jan 1970\"
+exit 0
+
+"))
+                (substitute* '("src/devices/grops/ps.cpp"
+                               "src/devices/grohtml/post-html.cpp")
+                  (("time\\(0\\)") "0"))))))
    (build-system gnu-build-system)
    (outputs '("out"
               "doc"))                    ;12MiB of PS, PDF, HTML, and examples

bug#27593: [PATCH v2] gnu: groff: Remove timestamps.

[Danny Milosavljevic]

Fixes <https://bugs.gnu.org/27593>.

* gnu/packages/groff.scm (groff)[source]: Remove timestamps.
---
 gnu/packages/groff.scm | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/groff.scm b/gnu/packages/groff.scm
index 46e1ccf23..1a74194a6 100644
--- a/gnu/packages/groff.scm
+++ b/gnu/packages/groff.scm
@@ -40,7 +40,23 @@
             (uri (string-append "mirror://gnu/groff/groff-" version
                                 ".tar.gz"))
             (sha256 (base32
-                     "1998v2kcs288d3y7kfxpvl369nqi06zbbvjzafyvyl3pr7bajj1s"))))
+                     "1998v2kcs288d3y7kfxpvl369nqi06zbbvjzafyvyl3pr7bajj1s"))
+            (modules '((guix build utils)))
+            (snippet
+             '(begin
+                (substitute* "mdate.sh"
+                 (("^LANGUAGE=") "
+
+echo \"1 Jan 1970\"
+exit 0
+
+"))
+                (substitute* '("src/devices/grops/ps.cpp"
+                               "src/devices/grohtml/post-html.cpp"
+                               "src/roff/troff/input.cpp")
+                  (("time\\(0\\)") "0"))
+                (substitute* "src/devices/gropdf/gropdf.pl"
+                  (("\\(time\\)") "(0)"))))))
    (build-system gnu-build-system)
    (outputs '("out"
               "doc"))                    ;12MiB of PS, PDF, HTML, and examples

bug#27593: [PATCH v2] gnu: groff: Remove timestamps.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 1880 bytes --]

Danny Milosavljevic <dannym@scratchpost.org> writes:

> Fixes <https://bugs.gnu.org/27593>.
>
> * gnu/packages/groff.scm (groff)[source]: Remove timestamps.
> ---
>  gnu/packages/groff.scm | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/gnu/packages/groff.scm b/gnu/packages/groff.scm
> index 46e1ccf23..1a74194a6 100644
> --- a/gnu/packages/groff.scm
> +++ b/gnu/packages/groff.scm
> @@ -40,7 +40,23 @@
>              (uri (string-append "mirror://gnu/groff/groff-" version
>                                  ".tar.gz"))
>              (sha256 (base32
> -                     "1998v2kcs288d3y7kfxpvl369nqi06zbbvjzafyvyl3pr7bajj1s"))))
> +                     "1998v2kcs288d3y7kfxpvl369nqi06zbbvjzafyvyl3pr7bajj1s"))
> +            (modules '((guix build utils)))
> +            (snippet
> +             '(begin
> +                (substitute* "mdate.sh"
> +                 (("^LANGUAGE=") "
> +
> +echo \"1 Jan 1970\"
> +exit 0
> +
> +"))

It would be more obvious what's going on if we substitute the "mdate.sh"
invocation from the Makefile instead. OTOH, this is more
future-proof. No strong opinion, but feels a bit "hacky".

Note that you can use \n here.

> +                (substitute* '("src/devices/grops/ps.cpp"
> +                               "src/devices/grohtml/post-html.cpp"
> +                               "src/roff/troff/input.cpp")
> +                  (("time\\(0\\)") "0"))
> +                (substitute* "src/devices/gropdf/gropdf.pl"
> +                  (("\\(time\\)") "(0)"))))))

Are all of these really a cause of indeterminism in the build process?
It looks like this could make "legitimate" uses of groff return epoch 0
at runtime, I'm not sure if that is okay.

Which output files are fixed by these substitutions? Are there other
ways to deal with the indeterminism, e.g. substitute them in place?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#27593: [PATCH v2] gnu: groff: Remove timestamps.

[Ludovic Courtès]

Marius Bakke <mbakke@fastmail.com> skribis:

> Danny Milosavljevic <dannym@scratchpost.org> writes:
>
>> Fixes <https://bugs.gnu.org/27593>.
>>
>> * gnu/packages/groff.scm (groff)[source]: Remove timestamps.
>> ---
>>  gnu/packages/groff.scm | 18 +++++++++++++++++-
>>  1 file changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/gnu/packages/groff.scm b/gnu/packages/groff.scm
>> index 46e1ccf23..1a74194a6 100644
>> --- a/gnu/packages/groff.scm
>> +++ b/gnu/packages/groff.scm
>> @@ -40,7 +40,23 @@
>>              (uri (string-append "mirror://gnu/groff/groff-" version
>>                                  ".tar.gz"))
>>              (sha256 (base32
>> -                     "1998v2kcs288d3y7kfxpvl369nqi06zbbvjzafyvyl3pr7bajj1s"))))
>> +                     "1998v2kcs288d3y7kfxpvl369nqi06zbbvjzafyvyl3pr7bajj1s"))
>> +            (modules '((guix build utils)))
>> +            (snippet
>> +             '(begin
>> +                (substitute* "mdate.sh"
>> +                 (("^LANGUAGE=") "
>> +
>> +echo \"1 Jan 1970\"
>> +exit 0
>> +
>> +"))
>
> It would be more obvious what's going on if we substitute the "mdate.sh"
> invocation from the Makefile instead. OTOH, this is more
> future-proof. No strong opinion, but feels a bit "hacky".
>
> Note that you can use \n here.
>
>> +                (substitute* '("src/devices/grops/ps.cpp"
>> +                               "src/devices/grohtml/post-html.cpp"
>> +                               "src/roff/troff/input.cpp")
>> +                  (("time\\(0\\)") "0"))
>> +                (substitute* "src/devices/gropdf/gropdf.pl"
>> +                  (("\\(time\\)") "(0)"))))))
>
> Are all of these really a cause of indeterminism in the build process?
> It looks like this could make "legitimate" uses of groff return epoch 0
> at runtime, I'm not sure if that is okay.
>
> Which output files are fixed by these substitutions? Are there other
> ways to deal with the indeterminism, e.g. substitute them in place?

Sometimes it’s better to make such changes available as a .patch file so
that we can more clearly see from the context that we’re patching the
right thing.  Perhaps that’s one such case here.

BTW, looking at issues.yml in
git://git.debian.org/git/reproducible/notes.git leads me to:

  https://wiki.debian.org/ReproducibleBuilds/TimestampsInDocumentationGeneratedByGroff

Debian has a SOURCE_DATE_EPOCH patch:

  http://sources.debian.net/patches/groff/1.22.3-9/source-date-epoch.patch/

Perhaps we could just use that?  That way, the default behavior of groff
would remained unchanged.

Apparently upstream wasn’t convinced by the idea of SOURCE_DATE_EPOCH,
so we could also restart the discussion with upstream in parallel.

However, it would be nice to commit this groff change quickly so we can
go on with core-updates builds.  :-)

Thoughts?

Ludo’.

bug#27593: [PATCH v3] gnu: groff: Remove timestamps.

[Danny Milosavljevic]

[-- Attachment #1: Type: text/plain, Size: 457 bytes --]

Hi Ludo,

On Sun, 09 Jul 2017 21:51:08 +0200
ludo@gnu.org (Ludovic Courtès) wrote:
> Debian has a SOURCE_DATE_EPOCH patch:
> 
>   http://sources.debian.net/patches/groff/1.22.3-9/source-date-epoch.patch/
> 
> Perhaps we could just use that?  That way, the default behavior of groff
> would remained unchanged.

Yeah, it seems to work fine.

Apparently git send-email doesn't work for me right now.

So I attached the patch manually here...

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-groff-Remove-timestamps.patch --]
[-- Type: text/x-patch, Size: 12428 bytes --]

From 2545f9001d674a8bd216ecc13fa939ed31ccfc91 Mon Sep 17 00:00:00 2001
From: Danny Milosavljevic <dannym@scratchpost.org>
Date: Thu, 6 Jul 2017 03:47:08 +0200
Subject: [PATCH v3] gnu: groff: Remove timestamps.
Tags: patch

Fixes <https://bugs.gnu.org/27593>.

* gnu/packages/patches/groff-source-date-epoch.patch: New file.
* gnu/local.mk: Add it.
* gnu/packages/groff.scm (groff)[source]: Add it.
---
 gnu/local.mk                                       |   1 +
 gnu/packages/groff.scm                             |   4 +-
 gnu/packages/patches/groff-source-date-epoch.patch | 299 +++++++++++++++++++++
 3 files changed, 303 insertions(+), 1 deletion(-)
 create mode 100644 gnu/packages/patches/groff-source-date-epoch.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index 23b8f449a..0fe6cdc39 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -645,6 +645,7 @@ dist_patch_DATA =						\
   %D%/packages/patches/graphite2-ffloat-store.patch		\
   %D%/packages/patches/grep-gnulib-lock.patch                   \
   %D%/packages/patches/grep-timing-sensitive-test.patch		\
+  %D%/packages/patches/groff-source-date-epoch.patch		\
   %D%/packages/patches/gsl-test-i686.patch			\
   %D%/packages/patches/gspell-dash-test.patch			\
   %D%/packages/patches/guile-1.8-cpp-4.5.patch			\
diff --git a/gnu/packages/groff.scm b/gnu/packages/groff.scm
index d7f45d4fc..c2f33736f 100644
--- a/gnu/packages/groff.scm
+++ b/gnu/packages/groff.scm
@@ -40,7 +40,9 @@
             (uri (string-append "mirror://gnu/groff/groff-" version
                                 ".tar.gz"))
             (sha256 (base32
-                     "1998v2kcs288d3y7kfxpvl369nqi06zbbvjzafyvyl3pr7bajj1s"))))
+                     "1998v2kcs288d3y7kfxpvl369nqi06zbbvjzafyvyl3pr7bajj1s"))
+            (patches (search-patches "groff-source-date-epoch.patch"))
+            (modules '((guix build utils)))))
    (build-system gnu-build-system)
    (outputs '("out"
               "doc"))                    ;12MiB of PS, PDF, HTML, and examples
diff --git a/gnu/packages/patches/groff-source-date-epoch.patch b/gnu/packages/patches/groff-source-date-epoch.patch
new file mode 100644
index 000000000..53c590b04
--- /dev/null
+++ b/gnu/packages/patches/groff-source-date-epoch.patch
@@ -0,0 +1,299 @@
+Adds support for the environment variable SOURCE_DATE_EPOCH.
+
+See: https://sources.debian.net/patches/groff/1.22.3-9/source-date-epoch.patch/
+
+From abc23bc9245e18468817f2838361c3a08f7521e2 Mon Sep 17 00:00:00 2001
+From: Colin Watson <cjwatson@debian.org>
+Date: Thu, 5 Nov 2015 11:47:34 +0000
+Subject: Implement `SOURCE_DATE_EPOCH' for reproducible builds.
+
+Author: Colin Watson <cjwatson@debian.org>
+Forwarded: yes
+Last-Update: 2015-11-05
+
+Patch-Name: source-date-epoch.patch
+---
+ doc/groff.texinfo                 |  6 +++++
+ src/devices/grohtml/grohtml.man   |  7 ++++++
+ src/devices/grohtml/post-html.cpp |  5 ++--
+ src/devices/gropdf/gropdf.man     |  7 ++++++
+ src/devices/gropdf/gropdf.pl      |  3 ++-
+ src/devices/grops/grops.man       |  7 ++++++
+ src/devices/grops/ps.cpp          |  3 ++-
+ src/include/curtime.h             | 23 ++++++++++++++++++
+ src/libs/libgroff/Makefile.sub    |  2 ++
+ src/libs/libgroff/curtime.cpp     | 51 +++++++++++++++++++++++++++++++++++++++
+ src/roff/troff/input.cpp          |  3 ++-
+ 11 files changed, 112 insertions(+), 5 deletions(-)
+ create mode 100644 src/include/curtime.h
+ create mode 100644 src/libs/libgroff/curtime.cpp
+
+diff --git a/doc/groff.texinfo b/doc/groff.texinfo
+index 066b5274..1d3c7a9f 100644
+--- a/doc/groff.texinfo
++++ b/doc/groff.texinfo
+@@ -1453,6 +1453,12 @@ default directory (on Unix and GNU/Linux systems, this is usually
+ @item GROFF_TYPESETTER
+ @tindex GROFF_TYPESETTER@r{, environment variable}
+ The default output device.
++
++@item SOURCE_DATE_EPOCH
++@tindex SOURCE_DATE_EPOCH@r{, environment variable}
++A timestamp (expressed as seconds since the Unix epoch) to use in place of
++the current time when initializing time-based built-in registers such as
++@code{\n[seconds]}.
+ @end table
+ 
+ Note that MS-DOS and MS-Windows ports of @code{groff} use semi-colons,
+diff --git a/src/devices/grohtml/grohtml.man b/src/devices/grohtml/grohtml.man
+index 51eae224..4be4abbc 100644
+--- a/src/devices/grohtml/grohtml.man
++++ b/src/devices/grohtml/grohtml.man
+@@ -419,6 +419,13 @@ and
+ for more details.
+ .
+ .
++.TP
++.SM
++.B SOURCE_DATE_EPOCH
++A timestamp (expressed as seconds since the Unix epoch) to use as the
++creation timestamp in place of the current time.
++.
++.
+ .\" --------------------------------------------------------------------
+ .SH BUGS
+ .\" --------------------------------------------------------------------
+diff --git a/src/devices/grohtml/post-html.cpp b/src/devices/grohtml/post-html.cpp
+index fefbf014..b5fc5167 100644
+--- a/src/devices/grohtml/post-html.cpp
++++ b/src/devices/grohtml/post-html.cpp
+@@ -28,6 +28,7 @@ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+ #include "html.h"
+ #include "html-text.h"
+ #include "html-table.h"
++#include "curtime.h"
+ 
+ #include <time.h>
+ 
+@@ -5013,7 +5014,7 @@ void html_printer::do_file_components (void)
+ 	.put_string(Version_string)
+ 	.end_comment();
+ 
+-      t = time(0);
++      t = current_time();
+       html.begin_comment("CreationDate: ")
+ 	.put_string(ctime(&t), strlen(ctime(&t))-1)
+ 	.end_comment();
+@@ -5126,7 +5127,7 @@ html_printer::~html_printer()
+     .put_string(Version_string)
+     .end_comment();
+ 
+-  t = time(0);
++  t = current_time();
+   html.begin_comment("CreationDate: ")
+     .put_string(ctime(&t), strlen(ctime(&t))-1)
+     .end_comment();
+diff --git a/src/devices/gropdf/gropdf.man b/src/devices/gropdf/gropdf.man
+index 3bbace6a..cc0c82f1 100644
+--- a/src/devices/gropdf/gropdf.man
++++ b/src/devices/gropdf/gropdf.man
+@@ -1029,6 +1029,13 @@ and
+ for more details.
+ .
+ .
++.TP
++.SM
++.B SOURCE_DATE_EPOCH
++A timestamp (expressed as seconds since the Unix epoch) to use as the
++creation timestamp in place of the current time.
++.
++.
+ .\" --------------------------------------------------------------------
+ .SH FILES
+ .\" --------------------------------------------------------------------
+diff --git a/src/devices/gropdf/gropdf.pl b/src/devices/gropdf/gropdf.pl
+index 035d1238..c25c4c67 100644
+--- a/src/devices/gropdf/gropdf.pl
++++ b/src/devices/gropdf/gropdf.pl
+@@ -239,13 +239,14 @@ elsif (exists($ppsz{$papersz}))
+     @defaultmb=@mediabox=(0,0,$ppsz{$papersz}->[0],$ppsz{$papersz}->[1]);
+ }
+ 
+-my (@dt)=localtime(time);
++my (@dt)=localtime($ENV{SOURCE_DATE_EPOCH} || time);
+ my $dt=PDFDate(\@dt);
+ 
+ my %info=('Creator' => "(groff version $cfg{GROFF_VERSION})",
+ 				'Producer' => "(gropdf version $cfg{GROFF_VERSION})",
+ 				'ModDate' => "($dt)",
+ 				'CreationDate' => "($dt)");
++
+ while (<>)
+ {
+     chomp;
+diff --git a/src/devices/grops/grops.man b/src/devices/grops/grops.man
+index 99fb7486..272c2d18 100644
+--- a/src/devices/grops/grops.man
++++ b/src/devices/grops/grops.man
+@@ -1419,6 +1419,13 @@ and
+ for more details.
+ .
+ .
++.TP
++.SM
++.B SOURCE_DATE_EPOCH
++A timestamp (expressed as seconds since the Unix epoch) to use as the
++creation timestamp in place of the current time.
++.
++.
+ .\" --------------------------------------------------------------------
+ .SH FILES
+ .\" --------------------------------------------------------------------
+diff --git a/src/devices/grops/ps.cpp b/src/devices/grops/ps.cpp
+index 745a503b..03e65372 100644
+--- a/src/devices/grops/ps.cpp
++++ b/src/devices/grops/ps.cpp
+@@ -28,6 +28,7 @@ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+ #include "cset.h"
+ #include "nonposix.h"
+ #include "paper.h"
++#include "curtime.h"
+ 
+ #include "ps.h"
+ #include <time.h>
+@@ -1390,7 +1391,7 @@ ps_printer::~ps_printer()
+ #else
+     time_t
+ #endif
+-    t = time(0);
++    t = current_time();
+     fputs(ctime(&t), out.get_file());
+   }
+   for (font_pointer_list *f = font_list; f; f = f->next) {
+diff --git a/src/include/curtime.h b/src/include/curtime.h
+new file mode 100644
+index 00000000..a4105196
+--- /dev/null
++++ b/src/include/curtime.h
+@@ -0,0 +1,23 @@
++/* Copyright (C) 2015  Free Software Foundation, Inc.
++
++This file is part of groff.
++
++groff is free software; you can redistribute it and/or modify it under
++the terms of the GNU General Public License as published by the Free
++Software Foundation, either version 2 of the License, or
++(at your option) any later version.
++
++groff is distributed in the hope that it will be useful, but WITHOUT ANY
++WARRANTY; without even the implied warranty of MERCHANTABILITY or
++FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
++for more details.
++
++The GNU General Public License version 2 (GPL2) is available in the
++internet at <http://www.gnu.org/licenses/gpl-2.0.txt>. */
++
++#ifdef LONG_FOR_TIME_T
++long
++#else
++time_t
++#endif
++current_time();
+diff --git a/src/libs/libgroff/Makefile.sub b/src/libs/libgroff/Makefile.sub
+index 840d9934..4cb4937a 100644
+--- a/src/libs/libgroff/Makefile.sub
++++ b/src/libs/libgroff/Makefile.sub
+@@ -32,6 +32,7 @@ OBJS=\
+   cmap.$(OBJEXT) \
+   color.$(OBJEXT) \
+   cset.$(OBJEXT) \
++  curtime.$(OBJEXT) \
+   device.$(OBJEXT) \
+   errarg.$(OBJEXT) \
+   error.$(OBJEXT) \
+@@ -82,6 +83,7 @@ CCSRCS=\
+   $(srcdir)/cmap.cpp \
+   $(srcdir)/color.cpp \
+   $(srcdir)/cset.cpp \
++  $(srcdir)/curtime.cpp \
+   $(srcdir)/device.cpp \
+   $(srcdir)/errarg.cpp \
+   $(srcdir)/error.cpp \
+diff --git a/src/libs/libgroff/curtime.cpp b/src/libs/libgroff/curtime.cpp
+new file mode 100644
+index 00000000..00821b7f
+--- /dev/null
++++ b/src/libs/libgroff/curtime.cpp
+@@ -0,0 +1,51 @@
++/* Copyright (C) 2015  Free Software Foundation, Inc.
++
++This file is part of groff.
++
++groff is free software; you can redistribute it and/or modify it under
++the terms of the GNU General Public License as published by the Free
++Software Foundation, either version 2 of the License, or
++(at your option) any later version.
++
++groff is distributed in the hope that it will be useful, but WITHOUT ANY
++WARRANTY; without even the implied warranty of MERCHANTABILITY or
++FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
++for more details.
++
++The GNU General Public License version 2 (GPL2) is available in the
++internet at <http://www.gnu.org/licenses/gpl-2.0.txt>. */
++
++#include <errno.h>
++#include <limits.h>
++#include <stdlib.h>
++#include <string.h>
++#include <time.h>
++
++#include "errarg.h"
++#include "error.h"
++
++#ifdef LONG_FOR_TIME_T
++long
++#else
++time_t
++#endif
++current_time()
++{
++  char *source_date_epoch = getenv("SOURCE_DATE_EPOCH");
++
++  if (source_date_epoch) {
++    errno = 0;
++    char *endptr;
++    long epoch = strtol(source_date_epoch, &endptr, 10);
++
++    if ((errno == ERANGE && (epoch == LONG_MAX || epoch == LONG_MIN)) ||
++	(errno != 0 && epoch == 0))
++      fatal("$SOURCE_DATE_EPOCH: strtol: %1", strerror(errno));
++    if (endptr == source_date_epoch)
++      fatal("$SOURCE_DATE_EPOCH: no digits found: %1", endptr);
++    if (*endptr != '\0')
++      fatal("$SOURCE_DATE_EPOCH: trailing garbage: %1", endptr);
++    return epoch;
++  } else
++    return time(0);
++}
+diff --git a/src/roff/troff/input.cpp b/src/roff/troff/input.cpp
+index 9594f074..f7d2c18a 100644
+--- a/src/roff/troff/input.cpp
++++ b/src/roff/troff/input.cpp
+@@ -36,6 +36,7 @@ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+ #include "input.h"
+ #include "defs.h"
+ #include "unicode.h"
++#include "curtime.h"
+ 
+ // Needed for getpid() and isatty()
+ #include "posix.h"
+@@ -8138,7 +8139,7 @@ static void init_registers()
+ #else /* not LONG_FOR_TIME_T */
+   time_t
+ #endif /* not LONG_FOR_TIME_T */
+-    t = time(0);
++    t = current_time();
+   // Use struct here to work around misfeature in old versions of g++.
+   struct tm *tt = localtime(&t);
+   set_number_reg("seconds", int(tt->tm_sec));

bug#27593: [PATCH v3] gnu: groff: Remove timestamps.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 902 bytes --]

Danny Milosavljevic <dannym@scratchpost.org> writes:

> Hi Ludo,
>
> On Sun, 09 Jul 2017 21:51:08 +0200
> ludo@gnu.org (Ludovic Courtès) wrote:
>> Debian has a SOURCE_DATE_EPOCH patch:
>> 
>>   http://sources.debian.net/patches/groff/1.22.3-9/source-date-epoch.patch/
>> 
>> Perhaps we could just use that?  That way, the default behavior of groff
>> would remained unchanged.
>
> Yeah, it seems to work fine.
>
> Apparently git send-email doesn't work for me right now.
>
> So I attached the patch manually here...

[...]

> -                     "1998v2kcs288d3y7kfxpvl369nqi06zbbvjzafyvyl3pr7bajj1s"))))
> +                     "1998v2kcs288d3y7kfxpvl369nqi06zbbvjzafyvyl3pr7bajj1s"))
> +            (patches (search-patches "groff-source-date-epoch.patch"))
> +            (modules '((guix build utils)))))

(guix build utils) is no longer needed. Apart from that LGTM!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#27593: [PATCH v3] gnu: groff: Remove timestamps.

[Ludovic Courtès]

Marius Bakke <mbakke@fastmail.com> skribis:

> Danny Milosavljevic <dannym@scratchpost.org> writes:
>
>> Hi Ludo,
>>
>> On Sun, 09 Jul 2017 21:51:08 +0200
>> ludo@gnu.org (Ludovic Courtès) wrote:
>>> Debian has a SOURCE_DATE_EPOCH patch:
>>> 
>>>   http://sources.debian.net/patches/groff/1.22.3-9/source-date-epoch.patch/
>>> 
>>> Perhaps we could just use that?  That way, the default behavior of groff
>>> would remained unchanged.
>>
>> Yeah, it seems to work fine.
>>
>> Apparently git send-email doesn't work for me right now.
>>
>> So I attached the patch manually here...
>
> [...]
>
>> -                     "1998v2kcs288d3y7kfxpvl369nqi06zbbvjzafyvyl3pr7bajj1s"))))
>> +                     "1998v2kcs288d3y7kfxpvl369nqi06zbbvjzafyvyl3pr7bajj1s"))
>> +            (patches (search-patches "groff-source-date-epoch.patch"))
>> +            (modules '((guix build utils)))))
>
> (guix build utils) is no longer needed. Apart from that LGTM!

+1!

Ludo'.

bug#27593: [PATCH v3] gnu: groff: Remove timestamps.

[Danny Milosavljevic]

Thanks!

Pushed to core-updates as 9146543f8dce997c194aea93bf4c7a7a4144c8c9.  Note that for real reproducibility the ghostscript patches are required too, and a phase in groff that sets the environment variable.

I've sent a new series for ghostscript.

bug#27593: [PATCH] gnu: groff: Make build reproducible.

[Danny Milosavljevic]

* gnu/packages/groff.scm (groff)[arguments]: Add phase "setenv".
---
 gnu/packages/groff.scm | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/groff.scm b/gnu/packages/groff.scm
index 67dd1dbfa..6e1a60e9b 100644
--- a/gnu/packages/groff.scm
+++ b/gnu/packages/groff.scm
@@ -56,7 +56,14 @@
                     ("perl" ,perl)
                     ("psutils" ,psutils)
                     ("texinfo" ,texinfo)))
-   (arguments '(#:parallel-build? #f))  ; parallel build fails
+   (arguments
+    `(#:parallel-build? #f   ; parallel build fails
+      #:phases
+      (modify-phases %standard-phases
+        (add-after 'unpack 'setenv
+          (lambda _
+            (setenv "GS_GENERATE_UUIDS" "0")
+            #t)))))
    (synopsis "Typesetting from plain text mixed with formatting commands")
    (description
     "Groff is a typesetting package that reads plain text and produces

bug#27593: [PATCH] gnu: groff: Make build reproducible.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 224 bytes --]

Danny Milosavljevic <dannym@scratchpost.org> writes:

> * gnu/packages/groff.scm (groff)[arguments]: Add phase "setenv".

What does this add to the previous patch? Can you add a comment in the
code explaining what it's for?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#27593: [PATCH] gnu: groff: Make build reproducible.

[Danny Milosavljevic]

Hi Marius,

On Mon, 10 Jul 2017 21:07:00 +0200
Marius Bakke <mbakke@fastmail.com> wrote:

> Danny Milosavljevic <dannym@scratchpost.org> writes:
> 
> > * gnu/packages/groff.scm (groff)[arguments]: Add phase "setenv".  
> 
> What does this add to the previous patch? Can you add a comment in the
> code explaining what it's for?

It replaces the previous patch by using a new patched-ghostscript feature:
- ghostscript can now be instructed to not print the problematic parts (timestamps in headers, UUIDs in headers) that were the original cause of the "ever-changing PDFs" problem.

The previous approach was fighting the symptoms, where every time a problem appeared in one of the outputs, I'd patch it somewhere else so that, if it did make it to ghostscript, ghostscript would put fake timestamps and would derive UUIDs from them - which upstream recommended we should definitely not do.

That was really not scalable or sane.

Also, the original ghostscript states that they are printing the current system date into PDFs (rather than, say, the input file mtime), against better judgement, for compatibility.  Just no.

So this patch prevents ghostscript from putting the timestamps or the document UUIDs into the PDFs in the first place (I don't mean it puts a fake one there now - I mean it leaves the fields off entirely).  Much simpler.

I'll add a comment.

bug#27593: [PATCH] gnu: groff: Make build reproducible.

[Ludovic Courtès]

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> * gnu/packages/groff.scm (groff)[arguments]: Add phase "setenv".
> ---
>  gnu/packages/groff.scm | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/gnu/packages/groff.scm b/gnu/packages/groff.scm
> index 67dd1dbfa..6e1a60e9b 100644
> --- a/gnu/packages/groff.scm
> +++ b/gnu/packages/groff.scm
> @@ -56,7 +56,14 @@
>                      ("perl" ,perl)
>                      ("psutils" ,psutils)
>                      ("texinfo" ,texinfo)))
> -   (arguments '(#:parallel-build? #f))  ; parallel build fails
> +   (arguments
> +    `(#:parallel-build? #f   ; parallel build fails
> +      #:phases
> +      (modify-phases %standard-phases
> +        (add-after 'unpack 'setenv
> +          (lambda _
> +            (setenv "GS_GENERATE_UUIDS" "0")
> +            #t)))))

With a comment like Marius wrote, it’d be perfect; otherwise LGTM.

However, since ‘core-updates’ is now building and we’d like to make
progress on it, could you push it to ‘core-updates-next’ or similar?

Thanks,
Ludo’.

bug#27593: [PATCH] gnu: groff: Make build reproducible.

[Ludovic Courtès]

Hi Danny,

ludo@gnu.org (Ludovic Courtès) skribis:

> Danny Milosavljevic <dannym@scratchpost.org> skribis:
>
>> * gnu/packages/groff.scm (groff)[arguments]: Add phase "setenv".
>> ---
>>  gnu/packages/groff.scm | 9 ++++++++-
>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/gnu/packages/groff.scm b/gnu/packages/groff.scm
>> index 67dd1dbfa..6e1a60e9b 100644
>> --- a/gnu/packages/groff.scm
>> +++ b/gnu/packages/groff.scm
>> @@ -56,7 +56,14 @@
>>                      ("perl" ,perl)
>>                      ("psutils" ,psutils)
>>                      ("texinfo" ,texinfo)))
>> -   (arguments '(#:parallel-build? #f))  ; parallel build fails
>> +   (arguments
>> +    `(#:parallel-build? #f   ; parallel build fails
>> +      #:phases
>> +      (modify-phases %standard-phases
>> +        (add-after 'unpack 'setenv
>> +          (lambda _
>> +            (setenv "GS_GENERATE_UUIDS" "0")
>> +            #t)))))
>
> With a comment like Marius wrote, it’d be perfect; otherwise LGTM.
>
> However, since ‘core-updates’ is now building and we’d like to make
> progress on it, could you push it to ‘core-updates-next’ or similar?

Maybe now is the time to push this to ‘core-updates’?

(I was just hit by this reproducibility issue for a PDF file made with
groff…)

Cheers,
Ludo’.

bug#27593: [PATCH] gnu: groff: Make build reproducible.

[Danny Milosavljevic]

Hi Ludo,

On Sat, 09 Sep 2017 22:31:36 +0200
ludo@gnu.org (Ludovic Courtès) wrote:

> Maybe now is the time to push this to ‘core-updates’?
> 
> (I was just hit by this reproducibility issue for a PDF file made with
> groff…)

Isn't it pushed already?

If so, bad sign.  Maybe the new environment variable doesn't always work?

Do you have a diff of what is changed in the PDF?  It should have left the creationdate and moddate off, so it shouldn't list those in the diff...

bug#27593: [PATCH] gnu: groff: Make build reproducible.

[Ludovic Courtès]

Hi Danny,

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> Hi Ludo,
>
> On Sat, 09 Sep 2017 22:31:36 +0200
> ludo@gnu.org (Ludovic Courtès) wrote:
>
>> Maybe now is the time to push this to ‘core-updates’?
>> 
>> (I was just hit by this reproducibility issue for a PDF file made with
>> groff…)
>
> Isn't it pushed already?

Oh it is, but in the new ‘core-updates’ (I was looking at ‘master’.).

Perfect, happily closing the bug now.  :-)

Thanks,
Ludo’.