public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Ximin Luo <infinity0@pwned.gg>
To: Yury Gribov <tetra2005.patches@gmail.com>
Cc: Jeff Law <law@redhat.com>, GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PING^4][PATCH v2] Generate reproducible output independently of the build-path
Date: Thu, 10 Aug 2017 21:29:00 -0000	[thread overview]
Message-ID: <03d07491-59ab-4a71-616c-662a22531eb8@pwned.gg> (raw)
In-Reply-To: <CAKuS3csWCaAdQTzEhSG4VsFRdHhU+uO7C8hAdzW4D-J-_a9u5w@mail.gmail.com>

Yury Gribov:
> On Thu, Aug 3, 2017 at 12:45 PM, Ximin Luo <infinity0@pwned.gg> wrote:
>> Yury Gribov:
>>> [..]
>>>
>>> Shouldn't -fdebug-prefix-map be updated to use the same syntax as BUILD_PATH_PREFIX_MAP?
>>>
>>
>> -fdebug-prefix-map is a CLI option and can be given multiple times, each flag given is in the form of $from=$to where $from can't contain a '='.
>>
>> BUILD_PATH_PREFIX_MAP is a single envvar that encodes a list-of-pairs of the form $to=$from:$to=$from with some escaping for flexibility and to support things like windows paths. Since it's a new envvar, Ian Jackson suggested $to=$from to emphasise the reproducible ($to) part. I liked the idea so I implemented it like that. (We did a lot of bikeshedding over on the rb-general mailing list about the exact format and this is what we settled on, I'd like to avoid getting into that again but would nevertheless do it, if it's necessary to get this patch accepted.)
>>
>> Because -fdebug-prefix-map currently only encodes one $from=$to pair, it would be a very disruptive and highly backward-incompatible change to make it use the same syntax as B_P_P_M. A slightly less disruptive but still backward-incompatible change would be to make it encode a single $to=$from pair, but I don't really see the advantage to doing so - what were your thoughts on this?
> 
> I believe it would much easier to reason about environment variable
> behavior when it boils down to "prepend some standard flag to
> command-line flags".  It would also simplify maintenance of local
> compiler patch as core functionality can be merged to mainline GCC
> whereas debatable environment variable part stays in the distro.
> 

[answered in another email together with other related points]

>> If by "first class option" you meant a command-line flag, GCC *already has* that (-fdebug-prefix-map) and it wasn't enough to achieve reproducibility in many cases we tested.
>> dpkg-buildflags actually already adds these flags to CFLAGS CXXFLAGS etc on Debian. However, with this patch using the environment variable, we are able to reproduce 1800 more packages out of 26000.
> 
> Just curious, why -fdebug-prefix-map (maybe modified to support
> multiple renames) was not enough for these packages (and why they
> can't be fixed instead)?
> 

One important reason is that some packages embed CFLAGS/CXXFLAGS in build
output such as pkg-config files or Makefiles to be installed as examples. To
fix this, we'd have to add buildsystem-specific logic to strip out
-fdebug-prefix-map when it was writing such output. This does not affect all of
these 1800 packages, but I saw enough cases that I was convinced that the use
of a new envvar was a better approach - I don't think buildsystems should be
burdened with having to know "which flag values are reproducible vs not", this
is not the case with other CFLAGS.

X

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

  reply	other threads:[~2017-08-10 20:26 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-21 16:16 Ximin Luo
2017-07-21 16:16 ` [PATCH 3/3] When remapping paths, only match whole path components Ximin Luo
2017-07-21 16:16 ` [PATCH 1/3] Use BUILD_PATH_PREFIX_MAP envvar for debug-prefix-map Ximin Luo
2017-08-02 19:19   ` Jeff Law
2017-07-21 16:16 ` [PATCH 2/3] Use BUILD_PATH_PREFIX_MAP envvar to transform __FILE__ Ximin Luo
2017-08-02 19:09 ` [PING^4][PATCH v2] Generate reproducible output independently of the build-path Jeff Law
2017-08-03  2:06   ` Ximin Luo
2017-08-03  4:49     ` Yury Gribov
2017-08-03 11:46       ` Ximin Luo
2017-08-04  8:40         ` Yury Gribov
2017-08-10 21:29           ` Ximin Luo [this message]
2017-08-03 15:57       ` Jeff Law
2017-08-03 16:05     ` Jeff Law
2017-08-03 17:02       ` Ximin Luo
2017-08-04 12:32       ` Matthias Klose
2017-08-04 13:05         ` Jakub Jelinek
2017-08-10 21:15           ` Ximin Luo
2017-08-04 16:05         ` Yury Gribov
2017-08-10 20:55           ` Ximin Luo
2017-08-11  2:53             ` Joseph Myers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=03d07491-59ab-4a71-616c-662a22531eb8@pwned.gg \
    --to=infinity0@pwned.gg \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=law@redhat.com \
    --cc=tetra2005.patches@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).