Debian Bug report logs - #979570
dpkg: Breaks perfectly valid usage of FILE

Package: src:dpkg; Maintainer for src:dpkg is Dpkg Developers <debian-dpkg@lists.debian.org>;

Reported by: Lisandro Damián Nicanor Pérez Meyer <lisandro@debian.org>

Date: Fri, 8 Jan 2021 13:51:02 UTC

Severity: serious

Found in version dpkg/1.20.6

Done: Lisandro Damián Nicanor Pérez Meyer <perezmeyer@gmail.com>

Bug is archived. No further changes may be made.

Toggle useless messages

View this report as an mbox folder, status mbox, maintainer mbox

Report forwarded to debian-bugs-dist@lists.debian.org, lisandro@debian.org, Dpkg Developers <debian-dpkg@lists.debian.org>:
Bug#979570; Package src:dpkg. (Fri, 08 Jan 2021 13:51:04 GMT) (full text, mbox, link).

Acknowledgement sent to Lisandro Damián Nicanor Pérez Meyer <lisandro@debian.org>:
New Bug report received and forwarded. Copy sent to lisandro@debian.org, Dpkg Developers <debian-dpkg@lists.debian.org>. (Fri, 08 Jan 2021 13:51:04 GMT) (full text, mbox, link).

Message #5 received at submit@bugs.debian.org (full text, mbox, reply):

Source: dpkg
Version: 1.20.6
Severity: serious
Justification: Breaks well defined functionality
X-Debbugs-Cc: pkg-kde-talk@alioth-lists.debian.net, reproducible-builds@lists.alioth.debian.org

Hi! As discussed in https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=876901
mangling __FILE__, even if for reproducible builds, breaks a perfectly valid
usage of __FILE__.

Please revert #974087 until someone provides a valid patch for QFINDTESTDATA
and it's well tested.

-- Package-specific info:

-- System Information:
Debian Release: bullseye/sid
  APT prefers unstable
  APT policy: (990, 'unstable'), (500, 'unstable-debug'), (500, 'testing-debug'), (500, 'buildd-unstable'), (500, 'testing'), (1, 'experimental-debug'), (1, 'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386, arm64, armhf

Kernel: Linux 5.9.0-5-amd64 (SMP w/4 CPU threads)
Kernel taint flags: TAINT_OOT_MODULE, TAINT_UNSIGNED_MODULE
Locale: LANG=es_AR.UTF-8, LC_CTYPE=es_AR.UTF-8 (charmap=UTF-8), LANGUAGE=es_AR:es
Shell: /bin/sh linked to /bin/bash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

-- no debconf information

Information forwarded to debian-bugs-dist@lists.debian.org, Dpkg Developers <debian-dpkg@lists.debian.org>:
Bug#979570; Package src:dpkg. (Fri, 08 Jan 2021 17:54:05 GMT) (full text, mbox, link).

Acknowledgement sent to Vagrant Cascadian <vagrant@reproducible-builds.org>:
Extra info received and forwarded to list. Copy sent to Dpkg Developers <debian-dpkg@lists.debian.org>. (Fri, 08 Jan 2021 17:54:06 GMT) (full text, mbox, link).

Message #10 received at 979570@bugs.debian.org (full text, mbox, reply):

[Message part 1 (text/plain, inline)]

On 2021-01-08, Lisandro Damián Nicanor Pérez Meyer wrote:
> On Fri, 13 Nov 2020 at 17:40, Vagrant Cascadian
> <vagrant@reproducible-builds.org> wrote:
>>
>> On 2020-11-13, Sune Vuorela wrote:
>> > On 2020-10-27, Vagrant Cascadian <vagrant@reproducible-builds.org> wrote:
>> >> Though, of course, identifying the exact reproducibility problem would
>> >> be preferable. One of the common issues is test suites relying on the
>> >> behavior of __FILE__ returning a full path to find fixtures or other
>> >> test data.
>> >
>> > has QFIND_TESTDATA been adapted to work with this, or are we just
>> > "lucky" that most packages don't actually build and run test suites?
>>
>> Yes, QFINDTESTDATA is one of the primary (only?) issues with test suites
>> found in about 20-30 packages in the archive, according to the
>> archive-wide rebuild that was performed. For most of those packages
>> patches have been submitted, and some are already either fixed or marked
>> as pending.
>
> But QFINDTESTDATA is using __FILE__ in a valid way. It might not be
> what you are expecting, but still a valid usage.
>
> See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=876901 We have
> discussed this before.
>
>
>> If it could be fixed at the core for QFINDTESTDATA, that would be nicer
>> than fixing 20-30 packages individually, though we're not there right
>> now.
>
> In that case I would expect a valid patch from the people interested
> in enabling this. In the meantime the dpkg change broke a very valid
> usage. Inconvenient for reproducibility? yes, probably, but still very
> valid.

We did a full archive rebuild testing this change, and I provided
patches to all known affected packages several months ago. It is a
one-line change in debian/rules in most cases:

  https://udd.debian.org/cgi-bin/bts-usertags.cgi?user=reproducible-builds%40lists.alioth.debian.org&tag=fixfilepath

It seems there are less than 10 packages left that have not applied the
patch.

Longer-term, it would be nice to be able to fix QFINDTESTDATA to be
compatible, sure.


live well,
  vagrant

[signature.asc (application/pgp-signature, inline)]

Information forwarded to debian-bugs-dist@lists.debian.org, Dpkg Developers <debian-dpkg@lists.debian.org>:
Bug#979570; Package src:dpkg. (Sat, 09 Jan 2021 00:18:02 GMT) (full text, mbox, link).

Acknowledgement sent to Lisandro Damián Nicanor Pérez Meyer <perezmeyer@gmail.com>:
Extra info received and forwarded to list. Copy sent to Dpkg Developers <debian-dpkg@lists.debian.org>. (Sat, 09 Jan 2021 00:18:02 GMT) (full text, mbox, link).

Message #15 received at 979570@bugs.debian.org (full text, mbox, reply):

Hi! Explicitely CCing my bug, since it seems it missed my previous reply.

On Fri, 8 Jan 2021 at 20:49, Guillem Jover <guillem@debian.org> wrote:
>
> On Fri, 2021-01-08 at 19:23:13 -0300, Lisandro Damián Nicanor Pérez Meyer wrote:
> > [snip]
> > > We did a full archive rebuild testing this change, and I provided
> > > patches to all known affected packages several months ago. It is a
> > > one-line change in debian/rules in most cases:
> > >
> > >   https://udd.debian.org/cgi-bin/bts-usertags.cgi?user=reproducible-builds%40lists.alioth.debian.org&tag=fixfilepath
> > >
> > > It seems there are less than 10 packages left that have not applied the
> > > patch.
> > >
> > > Longer-term, it would be nice to be able to fix QFINDTESTDATA to be
> > > compatible, sure.
> >
> > >From a couple of "fixes":
> >
> > -export DEB_BUILD_MAINT_OPTIONS = hardening=+all
> > +# Disable fixfilepath feature, as it triggers build failures when
> > +# enabled.
> > +export DEB_BUILD_MAINT_OPTIONS = hardening=+all reproducible=-fixfilepath
> >
> > That's not a fix but hiding the dirt under the carpet. You are not
> > fixing the root issue nor the reproducibility one.
>
> I'm not sure I understand this objection. Reverting the patch from
> dpkg would do the same but at a global scale, which would make many
> packages that would benefit from the new default, not reproducible,
> and would still "hide the dirt under the carpet" for the very few
> that would otherwise need the option disabled.

__FILE__ has defined behaviour: the path the preprocessor finds. The
only place where __FILE__ is mangled, to the best of my knowledge, is
Debian. So expecting that any developer out there makes code that runs
with this Debian specific variation is unreasonable. In other words:
breaking perfectly valid code *by default*, even in the name of
reproducibility, is not right.

>
> Disabling this option in these few places gives you room to possibly
> look for a fix, or not, w/o the pressure of the freeze, while not
> affecting the other packages.

But still breaking perfectly valid code.


> As stated in the thread in d-d, I still fail to see the weight of the
> objections, TBH. And given this I'm planning on closing the bug in
> dpkg.

In that case we at least should agree that we disagree, so I would ask
the tech-ctte to review this case.



-- 
Lisandro Damián Nicanor Pérez Meyer
http://perezmeyer.com.ar/
http://perezmeyer.blogspot.com/

Message #20 received at 979570@bugs.debian.org (full text, mbox, reply):

On Fri, 8 Jan 2021 at 21:15, Lisandro Damián Nicanor Pérez Meyer
<perezmeyer@gmail.com> wrote:
>
> Hi! Explicitely CCing my bug, since it seems it missed my previous reply.
>
> On Fri, 8 Jan 2021 at 20:49, Guillem Jover <guillem@debian.org> wrote:
> >
> > On Fri, 2021-01-08 at 19:23:13 -0300, Lisandro Damián Nicanor Pérez Meyer wrote:
> > > [snip]
> > > > We did a full archive rebuild testing this change, and I provided
> > > > patches to all known affected packages several months ago. It is a
> > > > one-line change in debian/rules in most cases:
> > > >
> > > >   https://udd.debian.org/cgi-bin/bts-usertags.cgi?user=reproducible-builds%40lists.alioth.debian.org&tag=fixfilepath
> > > >
> > > > It seems there are less than 10 packages left that have not applied the
> > > > patch.
> > > >
> > > > Longer-term, it would be nice to be able to fix QFINDTESTDATA to be
> > > > compatible, sure.
> > >
> > > >From a couple of "fixes":
> > >
> > > -export DEB_BUILD_MAINT_OPTIONS = hardening=+all
> > > +# Disable fixfilepath feature, as it triggers build failures when
> > > +# enabled.
> > > +export DEB_BUILD_MAINT_OPTIONS = hardening=+all reproducible=-fixfilepath
> > >
> > > That's not a fix but hiding the dirt under the carpet. You are not
> > > fixing the root issue nor the reproducibility one.
> >
> > I'm not sure I understand this objection. Reverting the patch from
> > dpkg would do the same but at a global scale, which would make many
> > packages that would benefit from the new default, not reproducible,
> > and would still "hide the dirt under the carpet" for the very few
> > that would otherwise need the option disabled.

In fact most of those packages would not be unreproducible if the
environment would be the same as the original build. That includes the
build path.

I do understand that it is *desirable* to be able to reproducibly
build a package no matter the build path, but that's just desirable.

The real fix here is to do the right thing, ie, provide the very same
environment as the original build, including the build path.

So again, mangling __FILE__ should not be a default.


-- 
Lisandro Damián Nicanor Pérez Meyer
http://perezmeyer.com.ar/
http://perezmeyer.blogspot.com/

Acknowledgement sent to Vagrant Cascadian <vagrant@reproducible-builds.org>:
Extra info received and forwarded to list. Copy sent to Dpkg Developers <debian-dpkg@lists.debian.org>. (Sat, 09 Jan 2021 08:57:02 GMT) (full text, mbox, link).

Message #25 received at 979570@bugs.debian.org (full text, mbox, reply):

[Message part 1 (text/plain, inline)]

On 2021-01-08, Lisandro Damián Nicanor Pérez Meyer wrote:
> On Fri, 8 Jan 2021 at 21:15, Lisandro Damián Nicanor Pérez Meyer
> <perezmeyer@gmail.com> wrote:
> In fact most of those packages would not be unreproducible if the
> environment would be the same as the original build. That includes the
> build path.

True, that is a fairly simple workaround. Which is why we do not vary
the build path when testing bullseye for tests.reproducible-builds.org.

But we do vary build paths when testing experimental and unstable on
tests.reproducible-builds.org, as it helps identify cases where build
paths are an issue.

It would also help greatly as we move towards verification builds of
packages in the archive to not have to worry about build paths as much.

> I do understand that it is *desirable* to be able to reproducibly
> build a package no matter the build path, but that's just desirable.

According to Debian Policy it is recommended:

  "It is recommended that packages produce bit-for-bit identical
   binaries even if most environment variables and build paths are
   varied.  It is intended for this stricter standard to replace the
   above when it is easier for packages to meet it."

> The real fix here is to do the right thing, ie, provide the very same
> environment as the original build, including the build path.

That does sound like a workaround more than a real fix.

> So again, mangling __FILE__ should not be a default.

I'll agree to disagree.

I will admit that a change of defaults in dpkg this close to freeze does
seem a bit on the late side of things. Still, The process has been going
on for months, announced in accordance with the process for getting
defaults changes into dpkg. Bugs with trivial patches were filed in
October.

Unfortunately, most of the affected packages seem to disproportionately
affect packages maintained by the KDE team. I did what I could to
mitigate that impact by actually building each and every one of the
affected packages to ensure that the opt-out patches worked
correctly. Most of those have been applied already.

live well,
  vagrant

[signature.asc (application/pgp-signature, inline)]

Message #30 received at 979570@bugs.debian.org (full text, mbox, reply):

Note: in case we do not agree on this topic this will be the text I'll
send to the
tech-ctte.

Let me start by noting that I have nothing against reproducibility. In fact
quite the opposite: I love the idea... as long as it's properly implemented.

The problem here is that __FILE__ is a public, well defined API with very valid
use cases (more on this below), even if the current implementation of all the
major compilers go against reproducibility. So if we want to mangle __FILE__
(thus breaking API) this should be done by an opt-in, and **never** by opting
out. Else we risk breaking a valid implementation, be it already on the archive
any new package added afterwards.

Even more: we library maintainers continuously ask our upstreams to keep their
API as stable as possible, and if not possible following some rules
(SONAME change, for example). I will present an option to do the same ourselves
without breaking API/ABI.

# __FILE__ is a public, well defined API

During the course of #876901 many reasons were used to both say that __FILE__
is or not a well defined API. In fact one of the evidences used where the
compiler's documentation. For example
https://gcc.gnu.org/onlinedocs/cpp/Standard-Predefined-Macros.html
(enphasis mine):

This macro expands to the name of the current input file, in the form of a C
string constant. **This is the path by which the preprocessor opened
the file**,
not the short name specified in ‘#include’ or as the input file name argument.
For example, "/usr/local/include/myheader.h" is a possible expansion of this
macro.

This definition says that it's up to the preprocessor to define the path. So,
what has been the behaviour of all major compilers during all these years? Using
the full path. The proof of this is Qt itself. Check
https://sources.debian.org/src/qtbase-opensource-src/5.15.2+dfsg-2/src/testlib/qtestcase.h/#L216

This code has been working on **every** platform Qt works without any change.
Qt is compiled in a myriad of OSs, using a myriad of compilers. They all do
the exact same thing. So developers depend on a very stable definition
of an API.

And we all know that breaking API is bad, except if done carefully (read below).

# Doing the right thing

This is just an idea of "doing the right thing", like bumping SONAME
on a library.
It definitely doesn't have to be the only one.

I understand that the reproducibility people do not want to consider
providing the
same build path. While this is arguable I do understand that
reproducibility without
depending on the build path is a good thing. So I've tried to come up with a
path to achieve this.

## New macro and warning (if they do not exist already)

This would be the first step. The idea is to provide a new macro that,
by definition
and documentation, it can be mangled with the help of the build
system, much as you
are currently doing with __FILE__ now.

The second step in this is to make compilers create a reproducibility warning if
some code uses __FILE__. This will have three effects effects:

- discouraging it's use
- creating awareness on reproducibility.
- making other distros jump into reproducibility in a much easier way.

## Mangling by opting-in, or what to do in the meantime

Of course we want to make things reproducible as fast as possible too. This is
why mangling __FILE__ was considered and implemented. But this is breaking API,
no matter how you look at it. But that doesn't means it can not be used: if a
package can be built mangling __FILE__ in order to get a reproducible way then
the maintainer should really consider opting *in* for the change, but never
ever should the maintainer be forced to opt *out* using a broken API.

In fact I consider all the "fixes" of opting in a bug, and forcing this change
a couple of weeks before freeze even worse. So I propose you to undo the change
in dpkg and, after the release, starting to work on letting maintainers opting
in.

Regards, Lisandro.

--
Lisandro Damián Nicanor Pérez Meyer
http://perezmeyer.com.ar/
http://perezmeyer.blogspot.com/

Message #35 received at 979570@bugs.debian.org (full text, mbox, reply):

Oh, I have sadly forgotten to mention another thing.

On Sat, 9 Jan 2021 at 15:53, Lisandro Damián Nicanor Pérez Meyer
<perezmeyer@gmail.com> wrote:
> # __FILE__ is a public, well defined API

According to:
Adrian Bunks mentions it in
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=876901#10
Holger Levsen in https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=876901#42
Mattia Rizzolo on https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=876901#192

The ability of gcc to change __FILE__ was a patch that, at the time of
those writings, wasn't yet accepted.

Even more, Ximion Luo wrote on
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=876901#212

  The GCC patch (neither the previous nor the planned version) does
not change the
  default behaviour of __FILE__, and was never intended to. Instead,
it gives users
  the ability to rewrite __FILE__, more specifically a prefix of it.

makes it clear that the default behaviour is not changed. So if the
patch got accepted
(did it get accepted?) it was only to allow reproducible people to
break API in order to
get reproducibility. This in itself, if something has not changed in
the meantime, marks
another reference that __FILE__ is a public, well defined API.

Reply sent to Lisandro Damián Nicanor Pérez Meyer <perezmeyer@gmail.com>:
You have taken responsibility. (Sat, 09 Jan 2021 20:51:05 GMT) (full text, mbox, link).

Notification sent to Lisandro Damián Nicanor Pérez Meyer <lisandro@debian.org>:
Bug acknowledged by developer. (Sat, 09 Jan 2021 20:51:05 GMT) (full text, mbox, link).

Message #40 received at 979570-done@bugs.debian.org (full text, mbox, reply):

Hi!

On Sat, 9 Jan 2021 at 16:52, Mattia Rizzolo <mattia@debian.org> wrote:
>
> On Sat, Jan 09, 2021 at 08:37:48PM +0100, Samuel Thibault wrote:
> > Lisandro Damián Nicanor Pérez Meyer, le sam. 09 janv. 2021 15:53:41 -0300, a ecrit:
> > > # __FILE__ is a public, well defined API
> >
> > ? My copy of C11 says
> >
> > “
> > __FILE__ The presumed name of the current source file (a character string literal)
> > ”
> >
> > that's not so well-defined.  I would not expect it to necessarily
> > contain the path to it.
>
> In fact, empirically I've seen that __FILE__ is expanded to whatever
> path gets passed to the compiler.
> You can easily see how stuff build with cmake get a full path in there,
> whereas stuff built with make gets a path relative to the Makefile.

Mattias has just showed me that in fact __FILE__ is too malleable:

mattia@warren /tmp/tmp % cat test.c
#include <stdio.h>
int main() {
    printf("%s\n", __FILE__);
}

mattia@warren /tmp/tmp % gcc test.c ; ./a.out
test.c
mattia@warren /tmp/tmp % gcc ./test.c ; ./a.out
./test.c
mattia@warren /tmp/tmp % gcc /tmp/tmp/./test.c ; ./a.out
/tmp/tmp/./test.c

So yes, the cornerstone of my assumption was totally wrong. My
apologies for all the noise, all I can say is: today I learned
something new.




-- 
Lisandro Damián Nicanor Pérez Meyer
http://perezmeyer.com.ar/
http://perezmeyer.blogspot.com/

Acknowledgement sent to Vagrant Cascadian <vagrant@reproducible-builds.org>:
Extra info received and forwarded to list. Copy sent to Dpkg Developers <debian-dpkg@lists.debian.org>. (Sat, 09 Jan 2021 20:57:02 GMT) (full text, mbox, link).

Message #45 received at 979570@bugs.debian.org (full text, mbox, reply):

[Message part 1 (text/plain, inline)]

On 2021-01-09, Lisandro Damián Nicanor Pérez Meyer wrote:
> Oh, I have sadly forgotten to mention another thing.
>
> On Sat, 9 Jan 2021 at 15:53, Lisandro Damián Nicanor Pérez Meyer
> <perezmeyer@gmail.com> wrote:
>> # __FILE__ is a public, well defined API
>
> According to:
> Adrian Bunks mentions it in
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=876901#10
> Holger Levsen in https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=876901#42
> Mattia Rizzolo on https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=876901#192
>
> The ability of gcc to change __FILE__ was a patch that, at the time of
> those writings, wasn't yet accepted.

That is no longer the case. The fixfilepath feature enabled in dpkg only
uses features (e.g. -ffile-prefix-map) available in both upstream GCC
(>=9? or 8? ~2018) and Clang (>= 10), possibly other compilers as
well. There are no special patches to toolchains needed to enable this
feature.

> Even more, Ximion Luo wrote on
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=876901#212
>
>   The GCC patch (neither the previous nor the planned version) does
>   not change the default behaviour of __FILE__, and was never intended
>   to. Instead, it gives users the ability to rewrite __FILE__, more
>   specifically a prefix of it.
>
> makes it clear that the default behaviour is not changed. So if the
> patch got accepted (did it get accepted?) it was only to allow
> reproducible people to break API in order to get reproducibility.

Since then an alternate implementation was implemented that the
reproducible=+fixfilepath feature in dpkg takes advantage of, in order
to implement this feature in another distribution, if I recall
correctly.

It didn't address all the cases of the old GCC patches that Ximin
submitted, but the Reproducible Builds team decided in 2018 to make use
of upstream supported features only and dropped all of our patches to
GCC.

The notable difference is that it not longer makes use of an environment
variable; it requires passing the -ffile-prefix-map option to the
compiler. The dpkg patch simply adds this to the default relevent *FLAGS
variables.

(For historical completeness, though somewhat an aside to the topic at
hand, the -ffile-prefix-map approach does not address all the cases of
the former patches to GCC as the compiler flags sometimes end up in
various shipped artifacts in *some* cases, though certainly not all.)

> This in itself, if something has not changed in the meantime, marks
> another reference that __FILE__ is a public, well defined API.

I think reading #876901 demonstrates that the case can be made either
way; it not as well defined as you make it out to be.

live well,
  vagrant

[signature.asc (application/pgp-signature, inline)]

Message #50 received at 979570@bugs.debian.org (full text, mbox, reply):

Hi (again)!

On Sat, 9 Jan 2021 at 17:53, Vagrant Cascadian
<vagrant@reproducible-builds.org> wrote:
>
> On 2021-01-09, Lisandro Damián Nicanor Pérez Meyer wrote:
> > Oh, I have sadly forgotten to mention another thing.
> >
> > On Sat, 9 Jan 2021 at 15:53, Lisandro Damián Nicanor Pérez Meyer
> > <perezmeyer@gmail.com> wrote:
> >> # __FILE__ is a public, well defined API
> >
> > According to:
> > Adrian Bunks mentions it in
> > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=876901#10
> > Holger Levsen in https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=876901#42
> > Mattia Rizzolo on https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=876901#192
> >
> > The ability of gcc to change __FILE__ was a patch that, at the time of
> > those writings, wasn't yet accepted.
>
> That is no longer the case. The fixfilepath feature enabled in dpkg only
> uses features (e.g. -ffile-prefix-map) available in both upstream GCC
> (>=9? or 8? ~2018) and Clang (>= 10), possibly other compilers as
> well. There are no special patches to toolchains needed to enable this
> feature.

This is actually good to read!

> > Even more, Ximion Luo wrote on
> > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=876901#212
> >
> >   The GCC patch (neither the previous nor the planned version) does
> >   not change the default behaviour of __FILE__, and was never intended
> >   to. Instead, it gives users the ability to rewrite __FILE__, more
> >   specifically a prefix of it.
> >
> > makes it clear that the default behaviour is not changed. So if the
> > patch got accepted (did it get accepted?) it was only to allow
> > reproducible people to break API in order to get reproducibility.
>
> Since then an alternate implementation was implemented that the
> reproducible=+fixfilepath feature in dpkg takes advantage of, in order
> to implement this feature in another distribution, if I recall
> correctly.
>
> It didn't address all the cases of the old GCC patches that Ximin
> submitted, but the Reproducible Builds team decided in 2018 to make use
> of upstream supported features only and dropped all of our patches to
> GCC.
>
> The notable difference is that it not longer makes use of an environment
> variable; it requires passing the -ffile-prefix-map option to the
> compiler. The dpkg patch simply adds this to the default relevent *FLAGS
> variables.
>
> (For historical completeness, though somewhat an aside to the topic at
> hand, the -ffile-prefix-map approach does not address all the cases of
> the former patches to GCC as the compiler flags sometimes end up in
> various shipped artifacts in *some* cases, though certainly not all.)

Again, good to read.

> > This in itself, if something has not changed in the meantime, marks
> > another reference that __FILE__ is a public, well defined API.
>
> I think reading #876901 demonstrates that the case can be made either
> way; it not as well defined as you make it out to be.

As stated in my previous mail I was wrong here, as demonstrated by
Mattias. I even just updated the base Qt bug accordingly.

Thanks for being so informative too!


-- 
Lisandro Damián Nicanor Pérez Meyer
http://perezmeyer.com.ar/
http://perezmeyer.blogspot.com/

Acknowledgement sent to Vagrant Cascadian <vagrant@reproducible-builds.org>:
Extra info received and forwarded to list. Copy sent to Dpkg Developers <debian-dpkg@lists.debian.org>. (Sat, 09 Jan 2021 21:54:03 GMT) (full text, mbox, link).

Message #55 received at 979570@bugs.debian.org (full text, mbox, reply):

[Message part 1 (text/plain, inline)]

On 2021-01-09, Lisandro Damián Nicanor Pérez Meyer wrote:
> Note: in case we do not agree on this topic this will be the text I'll
> send to the
> tech-ctte.

Thanks for taking the time to draft some text. If we can come closer to
agreement on the proposed text, that would probably take a bit of the
load off of the tech-ctte. Hopefully some of my comments move in that
direction, although I also have added some counterpoint text as well...

> Let me start by noting that I have nothing against reproducibility. In fact
> quite the opposite: I love the idea... as long as it's properly implemented.

It is even "recommended" in Debian Policy policy to build reproducibly
with different build paths.

> The problem here is that __FILE__ is a public, well defined API with very valid
> use cases (more on this below), even if the current implementation of all the
> major compilers go against reproducibility. 

At least two major compilers, GCC and Clang provide the
-ffile-prefix-map feature to do exactly that(which is what dpkg is
enabling), so it seems a bit overstated to say that all major compilers
"go against" reproducibility. They do not enable reproducibility by
default.

> So if we want to mangle __FILE__
> (thus breaking API) this should be done by an opt-in, and **never** by opting
> out. Else we risk breaking a valid implementation, be it already on the archive
> any new package added afterwards.

While I understand that may be how you feel, it would be appreciated if
we could use something a little less loaded than "mangle". perhaps:

  So if we want to enable features using __FILE__ in a way which
  arguably breaks API

> Even more: we library maintainers continuously ask our upstreams to keep their
> API as stable as possible, and if not possible following some rules
> (SONAME change, for example). I will present an option to do the same ourselves
> without breaking API/ABI.
>
> # __FILE__ is a public, well defined API
>
> During the course of #876901 many reasons were used to both say that __FILE__
> is or not a well defined API. In fact one of the evidences used where the
> compiler's documentation. For example
> https://gcc.gnu.org/onlinedocs/cpp/Standard-Predefined-Macros.html
> (enphasis mine):
>
>   This macro expands to the name of the current input file, in the form of a C
>   string constant. **This is the path by which the preprocessor opened
> the file**,
>   not the short name specified in ‘#include’ or as the input file name argument.
>   For example, "/usr/local/include/myheader.h" is a possible expansion of this
>   macro.
>
> This definition says that it's up to the preprocessor to define the path. So,
> what has been the behaviour of all major compilers during all these years? Using
> the full path. The proof of this is Qt itself. Check
> https://sources.debian.org/src/qtbase-opensource-src/5.15.2+dfsg-2/src/testlib/qtestcase.h/#L216
>
> This code has been working on **every** platform Qt works without any change.
> Qt is compiled in a myriad of OSs, using a myriad of compilers. They all do
> the exact same thing. So developers depend on a very stable definition
> of an API.
>
> And we all know that breaking API is bad, except if done carefully (read below).
>
> # Doing the right thing
>
> This is just an idea of "doing the right thing", like bumping SONAME
> on a library.
> It definitely doesn't have to be the only one.

I understand your position is fundamentally about "Doing the right
thing" but I would say that we are all trying to do the right thing.

> I understand that the reproducibility people do not want to consider
> providing the
> same build path. While this is arguable I do understand that
> reproducibility without
> depending on the build path is a good thing. So I've tried to come up with a
> path to achieve this.

> ## New macro and warning (if they do not exist already)
>
> This would be the first step. The idea is to provide a new macro that,
> by definition
> and documentation, it can be mangled with the help of the build
> system, much as you
> are currently doing with __FILE__ now.
>
> The second step in this is to make compilers create a reproducibility warning if
> some code uses __FILE__. This will have three effects effects:

We haven't proposed an alternate macro, which would surely take years at
best, possibly decades. That doesn't seem too realistic.

> - discouraging it's use
> - creating awareness on reproducibility.

We've discouraged the use of __FILE__ for years, have done plenty of
outreach on the subject of reproducibility, and gotten traction with
various upstream projects.

> - making other distros jump into reproducibility in a much easier way.

Arguably some distros (e.g. archlinux) are passing us by when it comes
to real-world reproducibility; I'm not sure Debian is the example by
which all other distros should be measured anymore. Which is good in
some ways, but somewhat disappointing to see Debian start something
great and then lag behind.

> ## Mangling by opting-in, or what to do in the meantime

Maybe drop the phrase "Mangling"? Perhaps

  Opt-in to the fixfilepath feature, ...

Or maybe:

  Keeping the opt-in status quo, ...

> Of course we want to make things reproducible as fast as possible too. This is
> why mangling __FILE__ was considered and implemented. But this is breaking API,
> no matter how you look at it.

Well, that's the crux of the argument, isn't it?

Is making an assumption about correct behavior by reading ambigous
documentation that can be interpreted either way, and then examining
default compiler behavior ... really an API?

> But that doesn't means it can not be used: if a
> package can be built mangling __FILE__ in order to get a reproducible way then
> the maintainer should really consider opting *in* for the change, but never
> ever should the maintainer be forced to opt *out* using a broken API.
>
> In fact I consider all the "fixes" of opting in a bug, and forcing this change
> a couple of weeks before freeze even worse. So I propose you to undo the change
> in dpkg and, after the release, starting to work on letting maintainers opting
> in.

It has been an option to opt-in to this feature for at least two years;
and some packages have done so. Adding boilerplate one-line changes to
hundreds or thousands of packages in Debian seems suboptimal to actually
effect change.

I don't know if size of the issue is relevent to tech-ctte, but it seems
worth noting that the archive-wide rebuild revealed less than 30
packages broken by this change, most of which have been patched to
opt-out of this feature. Most or all of the reamaining issues are due to
QFINDTESTDATA in test suites.

And for completeness, It was speculated that hundreds more packages
which currently disable test suites would need to also opt-out of this
feature in order to enable test suites, or come up with some other way
to remove these flags during tests.

It also seems worth asking what are the negative impacts of enabling
this, other than a handful of packages that need changes to
debian/rules in order to build?

Thanks for taking the time with all this everyone!

live well,
  vagrant

[signature.asc (application/pgp-signature, inline)]

Bug archived. Request was from Debbugs Internal Request <owner@bugs.debian.org> to internal_control@bugs.debian.org. (Sun, 07 Feb 2021 07:30:52 GMT) (full text, mbox, link).

Send a report that this bug log contains spam.

Debian bug tracking system administrator <owner@bugs.debian.org>. Last modified: Wed May 17 09:58:45 2023; Machine Name: buxtehude

Debian Bug tracking system

Debbugs is free software and licensed under the terms of the GNU Public License version 2. The current version can be obtained from https://bugs.debian.org/debbugs-source/.

Debian Bug report logs - #979570 dpkg: Breaks perfectly valid usage of __FILE__

Debian Bug report logs - #979570
dpkg: Breaks perfectly valid usage of FILE