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

Re: RFC: dpkg patch to using -ffile-prefix-map



Hi!

On Tue, 2018-07-31 at 08:29:33 +0800, Vagrant Cascadian wrote:
> With gcc 8 being the default compiler in debian now, we should be able
> to use -ffile-prefix-map, which should handle *some* of the cases that
> BUILD_PATH_PREFIX_MAP is intended to solve. It definitely won't help
> with things that embed the gcc commandline into arguments.
> 
> But since we've been unable to convince gcc on the merits of
> BUILD_PATH_PREFIX_MAP using an environment variable, rebasing the gcc
> patches to support it takes a lot of effort, perhaps we should explore
> other options.
> 
> I've got a dpkg patch which makes use of -ffile-prefix-map. I haven't
> found a good test case yet... package that fails to build reproducibly
> due to using __FILE__, __BASE_FILE__, or __builtin_FILE() and builds
> fast enough that it's easy to test (holger suggested trying "dtkwm", but
> I haven't had a chance to try yet). Figured I'd at least publish my
> patch to get some review and/or testers.

Hah! Just several hours ago I was talking with Mattia over IRC about
the status of this, and ended up cooking the attached patch, which at
his request didn't merge, because he wanted to give it a try first.

And yeah, I also kind of understand gcc's upstream position, that if
you unconditionally embed all build flags into your resulting objects,
then they are really bound to be unreproducible, and as such that's
arguably a bug to fix there probably.

Also AIUI this divergence and the lack of forward porting is the
reason the repro buildds are currently stopped? If so I think getting
something going, even if it might produce some new (or old :)
reproducible problems is probably better than the current situation.

> The patch largely just copy-and-pastes the -fdebug-prefix-map code,
> though arguably could obsolete it entirely, since -ffile-prefix-map
> effectivly sets both -fdebug-prefix-map and -fmacro-prefix-map. But
> having the two features independently allows enabling or disabling one
> or the other easily for now.

I think the semantics in mine are slightly better? :)

> If nothing else, carrying a patch on dpkg builds *much* faster than gcc,
> and rebasing it periodically will be a lot less effort. Though if it
> works, hopefully we can get it into dpkg directly.

From my side, I see no problem with merging something like the
attached patch if it works (I've not even run the test suite on it I
think :).

Thanks,
Guillem
From a9ab53a1ea8e1e249b2dba6625a5aeb2c69a91d4 Mon Sep 17 00:00:00 2001
From: Guillem Jover <guillem@debian.org>
Date: Mon, 30 Jul 2018 16:42:21 +0200
Subject: [PATCH] Dpkg::Vendor::Debian: Add fixfilepath to reproducible feature

This is a superset of the fixdebugpath feature supported by gcc-8, but
covering in addition mappings for macros such as __FILE__ and similar.
---
 man/dpkg-buildflags.man       | 11 +++++++++++
 scripts/Dpkg/Vendor/Debian.pm | 21 +++++++++++++++++----
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/man/dpkg-buildflags.man b/man/dpkg-buildflags.man
index 7712c5576..ba18dbbb7 100644
--- a/man/dpkg-buildflags.man
+++ b/man/dpkg-buildflags.man
@@ -476,6 +476,17 @@ This will cause warnings when the \fB__TIME__\fP, \fB__DATE__\fP and
 \fB__TIMESTAMP__\fP macros are used.
 .
 .TP
+.B fixfilepath
+This setting (disabled by default) adds
+.BI \-ffile\-prefix\-map= BUILDPATH =.
+to \fBCFLAGS\fP, \fBCXXFLAGS\fP, \fBOBJCFLAGS\fP, \fBOBJCXXFLAGS\fP,
+\fBGCJFLAGS\fP, \fBFFLAGS\fP and \fBFCFLAGS\fP where \fBBUILDPATH\fP is
+set to the top-level directory of the package being built.
+This has the effect of removing the build path from any generated file.
+
+If both \fBfixdebugpath\fP and \fBfixfilepath\fP are set, this option
+takes precedence, because it is a superset of the former.
+.TP
 .B fixdebugpath
 This setting (enabled by default) adds
 .BI \-fdebug\-prefix\-map= BUILDPATH =.
diff --git a/scripts/Dpkg/Vendor/Debian.pm b/scripts/Dpkg/Vendor/Debian.pm
index 1e8f24397..4bcf120e0 100644
--- a/scripts/Dpkg/Vendor/Debian.pm
+++ b/scripts/Dpkg/Vendor/Debian.pm
@@ -100,6 +100,7 @@ sub _add_build_flags {
         },
         reproducible => {
             timeless => 1,
+            fixfilepath => 0,
             fixdebugpath => 1,
         },
         sanitize => {
@@ -205,7 +206,8 @@ sub _add_build_flags {
     my $build_path;
 
     # Mask features that might have an unsafe usage.
-    if ($use_feature{reproducible}{fixdebugpath}) {
+    if ($use_feature{reproducible}{fixfilepath} or
+        $use_feature{reproducible}{fixdebugpath}) {
         require Cwd;
 
         $build_path = $ENV{DEB_BUILD_PATH} || Cwd::cwd();
@@ -214,6 +216,7 @@ sub _add_build_flags {
         # so that we do not need to worry about escaping the characters
         # on output.
         if ($build_path =~ m/[^-+:.0-9a-zA-Z~\/_]/) {
+            $use_feature{reproducible}{fixfilepath} = 0;
             $use_feature{reproducible}{fixdebugpath} = 0;
         }
     }
@@ -223,9 +226,19 @@ sub _add_build_flags {
        $flags->append('CPPFLAGS', '-Wdate-time');
     }
 
-    # Avoid storing the build path in the debug symbols.
-    if ($use_feature{reproducible}{fixdebugpath}) {
-        my $map = '-fdebug-prefix-map=' . $build_path . '=.';
+    # Avoid storing the build path in the binaries.
+    if ($use_feature{reproducible}{fixfilepath} or
+        $use_feature{reproducible}{fixdebugpath}) {
+        my $map;
+
+        # -ffile-prefix-map is a superset of -fdebug-prefix-map, prefer it
+        # if both are set.
+        if ($use_feature{reproducible}{fixfilepath}) {
+            $map = '-ffile-prefix-map=' . $build_path . '=.';
+        } else {
+            $map = '-fdebug-prefix-map=' . $build_path . '=.';
+        }
+
         $flags->append('CFLAGS', $map);
         $flags->append('CXXFLAGS', $map);
         $flags->append('OBJCFLAGS', $map);
-- 
2.18.0


Reply to: