[Reproducible-builds] Minimising work needed for this build-path issue

[Daniel Kahn Gillmor]

Hi Ximin--

On Tue 2016-11-01 09:52:00 -0400, Ximin Luo wrote:
> Updated patches here:
>
> https://gist.github.com/infinity0/72aba22411a2e8baaae4649329f96643
>
> Everyone please feel free to comment / review before I soon send them off to GCC.

This looks great, thank you!  I have not tried running it myself yet,
but the logic and the structure of the changes seem sensible.  A few
nit-picky comments below:

---

Looking at what we're doing in debian, i see:

    https://sources.debian.net/src/gcc-6/6.2.0-9/debian/patches/gcc-SOURCE_DATE_EPOCH.diff/
    https://sources.debian.net/src/gcc-6/6.2.0-9/debian/patches/gcc-SOURCE_DATE_EPOCH-doc.diff/
    https://sources.debian.net/src/gcc-6/6.2.0-9/debian/patches/gcc-SOURCE_DATE_EPOCH-2.diff/
    https://sources.debian.net/src/gcc-6/6.2.0-9/debian/patches/gcc-SOURCE_DATE_EPOCH-2-doc.diff/

Is there a reason that these changes differ from
SOURCE_DATE_EPOCH_-_for_TIMESTAMP_macro.patch ?  Or is it just
divergence between gcc 6 and the master dev branch?

---

Also, i note that the -doc.diff patches above are really helpful (though
i see no reason to break them out separately from the code changes
unless there's an upstream convention to do so; i think coupling
documentation changes with code changes is a Good Thing).  Maybe this
SOURCE_PREFIX_MAP changeset can also include changes to gcc
documentation?

---

Your accompanying SRD_debug.txt describes the reasoning behind these
changes well, thanks!  But it says:

    I will also be submitting a follow-up patch to perform this
    behaviour for the __FILE__ macro as well,

Unless i'm misunderstanding, you have already included a patch for the
__FILE__ macro as well, so unless you plan on submitting these with some
sort of deliberately delayed timing, i'd change the text in
SRD_debug.txt to reflect the fact that this is a complete changeset that
we expect to Do the Right Thing.

---

Finally, SOURCE_PREFIX_MAP_-_rsplit.patch changes from splitting on the
first "=" to splitting on the last "=".  I think you've justified this
change well in previous e-mails on this thread, but the justification
doesn't appear in this patchset.  If the gcc folks are going to see this
series fresh, it'd be good to explain the change and justify it, as well
as marking it clearly as optional.  That is, if this change makes
upstream balk at accepting the patchset, we want them to drop it and
accept the rest, right?  



Thanks for your work on this, Ximin!

       --dkg
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 930 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/reproducible-builds/attachments/20161101/8f20729d/attachment.sig>