Bug#876901: QFINDTESTDATA uses __FILE__
On jueves, 16 de noviembre de 2017 13:22:00 -03 Ximin Luo wrote: [snip] > I pointed to the various C standards documents, as well as documentation > from multiple compilers, stating that __FILE__ is the "name of the source > file" and in no way guarantees that the expansion can later be re-used as > the path to an actual file. GCC documentation even explicitly states the > expansion is arbitrarily chosen by the implementation of the preprocessor, > and is explicitly "not [..] the input file name argument". OK, let's agree we do not agree here. > So I do consider this a bug in the QT test suite. > > The ideal solution would be to not use __FILE__ - that has numerous other > benefits as well. But if this is too complex to change, I also suggested a > 1-line addition to d/rules - which I agree would be "papering over" the > issue. However, it's a simple 1-line change so I don't understand why there > is so much resistance to it. Because it means diverging from upstream and that causes us *lots* of headaches when trying to solve unit tests issues with upstream's help. > There are several other possible solutions, all of which are low-cost and > unintrusive, and could be done in a QT build helper in one single place: > > - define a custom macro, QT_TEST_SOURCE_BASE, and set that in the test build > scripts, instead of using __FILE__ - export > BUILD_PATH_PREFIX_MAP="$BUILD_PATH_PREFIX_MAP:tests=$BASEDIR/tests" - > symlink "$srcpkg-$version" -> "." > > I would be very happy to send you the patches myself if you don't want to do > the work, since writing 1-line patches to a few QT projects, costs far less > time than patching 1800 packages across Debian. Let me propose you something: create a suitable patch for upstream that makes stuff work no matter the distro not OS. As I do think your approach is not correct you should push it yourself, see http://wiki.qt.io/Qt_Contribution_Guidelines for knowing how to contribute it. -- May the source be with you. Lisandro Damián Nicanor Pérez Meyer http://perezmeyer.com.ar/ http://perezmeyer.blogspot.com/ signature.asc Description: This is a digitally signed message part.
Bug#876901: QFINDTESTDATA uses __FILE__
Lisandro Damián Nicanor Pérez Meyer: > By the way: is there a macro or combination of macros which *default* value > can be replaced in the use of __FILE__ without caussing regressions? > > Because if that's the case it's easier to convince upstream people that > changing the usage goes in favor of reproducibility without using strange up- > to-the-builder definitions. > Unfortunately, not that I know of. However, let me try to explain why BUILD_PATH_PREFIX_MAP is not as strange as you might think. GCC already supports a flag -fdebug-prefix-map whose purpose is to do the same thing for debugging symbols. You can give multiple maps, to map different parts of your source tree to different other source directories. So maybe one could give a map like this: 1. "PWD/" -> "/usr/src/debug/$mypackage" 2. "PWD/tests" -> "./tests" if one wanted to generate debugging information for your main source code, to point to installed-source-code at /usr/src/debug/$mypackage; but since one doesn't install the tests anywhere, it would be convenient for developers to have the debuginfo of tests point to ./tests instead, so they can just run "gdb -d." if stuff fails. What I'm suggesting for QT tests would be exactly analogous to this already-supported use case for debuginfo. dpkg gives a default map corresponding to (1), and since QT tests use __FILE__ for other purposes, you give a second map corresponding to (2) above. X -- GPG: ed25519/56034877E1F87C35 GPG: rsa4096/1318EFAC5FBBDBCE https://github.com/infinity0/pubkeys.git
Bug#876901: QFINDTESTDATA uses __FILE__
Lisandro Damián Nicanor Pérez Meyer: > [..] > > Yes, we do understand that your workaround solves the issue, but we do also > understand that we should not be using this workaround in the first place. > > Guillem: the thread is long, but be sure that we Qt/KDE maintainers consider > that this change will be insta-RC I'm afraid. > Guillem: the TL;DR is that some QT tests fail (with our reproducibility GCC and dpkg patch) because they depend on __FILE__ to point to a real path, but the patched dpkg rewrites it in all packages so that a prefix $PWD changes to $srcpkg-$version, which doesn't exist. A simple fix on the dpkg side would be to instead rewrite $PWD to ".", much like -fdebug-prefix-map is done currently. However this loses some information and makes it hard to distinguish different packages that might share filetree structure. But that is the best fallback option IMO, if we decide that we don't want to auto-break QT tests for using __FILE__ (IMO, inappropriately). > Xi: you have found a *wonderful* way to find where bugs are, please try to > fix > the relevant code and not paper over it, because in the Qt case it is not a > bug on our side. > I pointed to the various C standards documents, as well as documentation from multiple compilers, stating that __FILE__ is the "name of the source file" and in no way guarantees that the expansion can later be re-used as the path to an actual file. GCC documentation even explicitly states the expansion is arbitrarily chosen by the implementation of the preprocessor, and is explicitly "not [..] the input file name argument". So I do consider this a bug in the QT test suite. The ideal solution would be to not use __FILE__ - that has numerous other benefits as well. But if this is too complex to change, I also suggested a 1-line addition to d/rules - which I agree would be "papering over" the issue. However, it's a simple 1-line change so I don't understand why there is so much resistance to it. There are several other possible solutions, all of which are low-cost and unintrusive, and could be done in a QT build helper in one single place: - define a custom macro, QT_TEST_SOURCE_BASE, and set that in the test build scripts, instead of using __FILE__ - export BUILD_PATH_PREFIX_MAP="$BUILD_PATH_PREFIX_MAP:tests=$BASEDIR/tests" - symlink "$srcpkg-$version" -> "." I would be very happy to send you the patches myself if you don't want to do the work, since writing 1-line patches to a few QT projects, costs far less time than patching 1800 packages across Debian. X -- GPG: ed25519/56034877E1F87C35 GPG: rsa4096/1318EFAC5FBBDBCE https://github.com/infinity0/pubkeys.git
Bug#876901: QFINDTESTDATA uses __FILE__
By the way: is there a macro or combination of macros which *default* value can be replaced in the use of __FILE__ without caussing regressions? Because if that's the case it's easier to convince upstream people that changing the usage goes in favor of reproducibility without using strange up- to-the-builder definitions. -- "In college, I cooked some hot dogs by putting metal forks in each end of the hot dog and running 120 volts through it. Hot dogs have just enough conductivity so that this works well" greenroom(576281) - on a truly geeky way to cook hot dogs. Posted in Slashdot, also found in The Open Source Cookbook for Geeks. Lisandro Damián Nicanor Pérez Meyer http://perezmeyer.com.ar/ http://perezmeyer.blogspot.com/ signature.asc Description: This is a digitally signed message part.
Bug#876901: QFINDTESTDATA uses __FILE__
Explicitely CCing Guillem for this one. On miércoles, 15 de noviembre de 2017 23:01:00 -03 Ximin Luo wrote: [sip] > 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. So it basically does not changes the default __FILE__ behavior, so neither GCC nor other projects upstream developers will have anything to complain... > > There is a separate patch to dpkg that enables this ability for all > packages, in the same way that SOURCE_DATE_EPOCH is enabled. Guillem the > dpkg maintainer has previously indicated that he's happy to take the patch, > once GCC accepts their end of it. ...except for a Debian self-inflicted change that will *only* happen while building Debian packages, but not when using it for normal developing processes. > It's a combination of these two patches that would cause these QT tests to > fail. The reason it fails, is because we specifically map $PWD to a > non-existent path. I suggested various ways around this. One of the > suggestions, was to add an extra mapping to map $PWD/tests back to > $PWD/tests (or just ./tests), overriding the earlier non-existent mapping. > I think this is the cleanest suggestion - assuming that tests reside in the > same directory, and away from the main source code. Kai Pastor over on bug > #876934 indicated that this would probably work for > openorienteering-mapper. Yes, we do understand that your workaround solves the issue, but we do also understand that we should not be using this workaround in the first place. Guillem: the thread is long, but be sure that we Qt/KDE maintainers consider that this change will be insta-RC I'm afraid. Xi: you have found a *wonderful* way to find where bugs are, please try to fix the relevant code and not paper over it, because in the Qt case it is not a bug on our side. -- “I don’t think security can solve problems. We need to teach greater respect.” Oslo Mayor Stang when asked whether Oslo needs greater security after the attacks in Norway, 07/2011. Lisandro Damián Nicanor Pérez Meyer http://perezmeyer.com.ar/ http://perezmeyer.blogspot.com/ signature.asc Description: This is a digitally signed message part.
Bug#876901: QFINDTESTDATA uses __FILE__
Lisandro Damián Nicanor Pérez Meyer: > On miércoles, 15 de noviembre de 2017 20:35:00 -03 Ximin Luo wrote: >> Lisandro Damián Nicanor Pérez Meyer: >>> I was wanting to write this on a keyboard, but let me try on the phone and >>> see if we can all agree in one point. >>> >>> There is one *excellent* method to settle this once and forever: submit >>> the >>> __FILE__ patch to GCC devs. If it gets accepted then Qt and whatever >>> project is affected should change it. >>> >>> If it does not gets accepted then it's a Debian regression. >>> >>> Until that is done I will consider this a non-Qt bug but a GCC regression. >>> Show me that GCC upstream devs agree with you Xi and I'll happily forward >>> the bug to Qt upstream. >> >> That sounds perfectly fair to me, and in line with my existing plans. Thanks >> for your patience, and I will ping back hopefully with good news in a few >> months. > > By the way: I will consider this as a valid patch if it changes the __FILE__ > **default** behavior, not something that can make it change at the will of > the > builder (like an environmet variable or such). > > Having the chance of changing the meaning does not means it can be a > regression. > 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. There is a separate patch to dpkg that enables this ability for all packages, in the same way that SOURCE_DATE_EPOCH is enabled. Guillem the dpkg maintainer has previously indicated that he's happy to take the patch, once GCC accepts their end of it. It's a combination of these two patches that would cause these QT tests to fail. The reason it fails, is because we specifically map $PWD to a non-existent path. I suggested various ways around this. One of the suggestions, was to add an extra mapping to map $PWD/tests back to $PWD/tests (or just ./tests), overriding the earlier non-existent mapping. I think this is the cleanest suggestion - assuming that tests reside in the same directory, and away from the main source code. Kai Pastor over on bug #876934 indicated that this would probably work for openorienteering-mapper. X -- GPG: ed25519/56034877E1F87C35 GPG: rsa4096/1318EFAC5FBBDBCE https://github.com/infinity0/pubkeys.git
Bug#876901: QFINDTESTDATA uses __FILE__
On miércoles, 15 de noviembre de 2017 20:35:00 -03 Ximin Luo wrote: > Lisandro Damián Nicanor Pérez Meyer: > > I was wanting to write this on a keyboard, but let me try on the phone and > > see if we can all agree in one point. > > > > There is one *excellent* method to settle this once and forever: submit > > the > > __FILE__ patch to GCC devs. If it gets accepted then Qt and whatever > > project is affected should change it. > > > > If it does not gets accepted then it's a Debian regression. > > > > Until that is done I will consider this a non-Qt bug but a GCC regression. > > Show me that GCC upstream devs agree with you Xi and I'll happily forward > > the bug to Qt upstream. > > That sounds perfectly fair to me, and in line with my existing plans. Thanks > for your patience, and I will ping back hopefully with good news in a few > months. By the way: I will consider this as a valid patch if it changes the __FILE__ **default** behavior, not something that can make it change at the will of the builder (like an environmet variable or such). Having the chance of changing the meaning does not means it can be a regression. -- SlackDeb: velo como un entrenamiento shaolin para geeks, en vez de meditación y tortura física, abstinencia de internet y sexo Horacio Francisco Sebastián "Perrito" Durán Barrionuevo, sobre un viaje que Federico "SlackDeb" Peretti estaba planeando con su novia. Lisandro Damián Nicanor Pérez Meyer http://perezmeyer.com.ar/ http://perezmeyer.blogspot.com/ signature.asc Description: This is a digitally signed message part.
Bug#876901: QFINDTESTDATA uses __FILE__
El 15 nov. 2017 5:35 p.m., "Mattia Rizzolo"escribió: On Wed, Nov 15, 2017 at 05:28:06PM -0300, Lisandro Damián Nicanor Pérez Meyer wrote: > There is one *excellent* method to settle this once and forever: submit the > __FILE__ patch to GCC devs. If it gets accepted then Qt and whatever > project is affected should change it. It's forwarded, and it was rejected with some concerns, none of which where those rised in this thread. We are working on settling those concerns and will hopefully soon to be able to send another batch for review. That's wonderful! Can I please have the upstream bug link?
Bug#876901: QFINDTESTDATA uses __FILE__
On Wed, Nov 15, 2017 at 05:28:06PM -0300, Lisandro Damián Nicanor Pérez Meyer wrote: > There is one *excellent* method to settle this once and forever: submit the > __FILE__ patch to GCC devs. If it gets accepted then Qt and whatever > project is affected should change it. It's forwarded, and it was rejected with some concerns, none of which where those rised in this thread. We are working on settling those concerns and will hopefully soon to be able to send another batch for review. > If it does not gets accepted then it's a Debian regression. Remember that for now this patch it is *not* in the Debian's GCC, only in our patched variant that is used only for the unstable tests in tests.reproducible-builds.org > Until that is done I will consider this a non-Qt bug but a GCC regression. > Show me that GCC upstream devs agree with you Xi and I'll happily forward > the bug to Qt upstream. Yes, that's also what I was recommending to my team mates on IRC yesterday. In the meantime, please feel free to mark the bug as wontfix as it indeed clearly is. -- regards, Mattia Rizzolo GPG Key: 66AE 2B4A FCCF 3F52 DA18 4D18 4B04 3FCD B944 4540 .''`. more about me: https://mapreri.org : :' : Launchpad user: https://launchpad.net/~mapreri `. `'` Debian QA page: https://qa.debian.org/developer.php?login=mattia `- signature.asc Description: PGP signature
Bug#876901: QFINDTESTDATA uses __FILE__
Lisandro Damián Nicanor Pérez Meyer: > I was wanting to write this on a keyboard, but let me try on the phone and > see if we can all agree in one point. > > There is one *excellent* method to settle this once and forever: submit the > __FILE__ patch to GCC devs. If it gets accepted then Qt and whatever > project is affected should change it. > > If it does not gets accepted then it's a Debian regression. > > Until that is done I will consider this a non-Qt bug but a GCC regression. > Show me that GCC upstream devs agree with you Xi and I'll happily forward > the bug to Qt upstream. > That sounds perfectly fair to me, and in line with my existing plans. Thanks for your patience, and I will ping back hopefully with good news in a few months. X -- GPG: ed25519/56034877E1F87C35 GPG: rsa4096/1318EFAC5FBBDBCE https://github.com/infinity0/pubkeys.git
Bug#876901: QFINDTESTDATA uses __FILE__
I was wanting to write this on a keyboard, but let me try on the phone and see if we can all agree in one point. There is one *excellent* method to settle this once and forever: submit the __FILE__ patch to GCC devs. If it gets accepted then Qt and whatever project is affected should change it. If it does not gets accepted then it's a Debian regression. Until that is done I will consider this a non-Qt bug but a GCC regression. Show me that GCC upstream devs agree with you Xi and I'll happily forward the bug to Qt upstream. Lisandro
Bug#876901: QFINDTESTDATA uses __FILE__
In data mercoledì 15 novembre 2017 20:14:00 CET, Ximin Luo ha scritto: > Pino Toscano: > > Loose meanings do not imply neither the other way around, that you are > > free to break because people cannot do anything with it. Also, > > considering the very same behaviour (short of the different wording in > > documentations) that is established for decades, this is a "de facto" > > standard. > > > > It's not, Microsoft doesn't do it. Last time I checked, QT is supposed > to work on Windows. Yup, and the fact that they used __FILE__ means the approach works also on Windows. > > [..] > > > >> https://gcc.gnu.org/onlinedocs/cpp/Standard-Predefined-Macros.html > >> "__FILE__ and __LINE__ are useful in generating an error message [..]" > >> > >> https://msdn.microsoft.com/en-us/library/027c4t2s.aspx > >> "Without /FC, the diagnostic text would look similar to this diagnostic > >> text:" > >> > >> The only examples I can find in compiler documentation of __FILE__ > >> usage, is for error messages. > >> > >> [..] > >> > >> http://c0x.coding-guidelines.com/6.10.8.html The presumed name of the > >> current source file [..] > >> https://software.intel.com/en-us/node/524489 Defined as a character > >> string literal containing the name of the source file. > >> https://www.ibm.com/support/knowledgecenter/SSAE4W_9.1.1/com.ibm.etools.iseries.langref.doc/ilcrefer13.htm > >> A string literal representing the name of the file being compiled. > >> > >> You can even find threads online confirming __FILE__ is > >> implementation-specific and not required by any standard to be a full > >> path. You can even find threads of people complaining about the fact > >> that __FILE__ sometimes has a full path, sometimes not - e.g. > >> depending on what options you pass to the Microsoft compiler. > > > > The texts above clearly said that __FILE__ contains the path passed to > > the compiler, which is very different from saying it is purely > > "implementation-defined". [..] > > > > That's false, you're misleading casual readers when you say this. > *None* of them define it using the word "path", they all use the word > "name" - except the GCC docs which say: Now it is you that plays with words. Really, you see "file name" used as both path and without path. > "path by which the preprocessor opened the file, not the short name > specified in ‘#include’ or as the input file name argument." > > Let me highlight that for you: "not [..] the input file name argument". Let me highlight this for you: "path by which the preprocessor opened the file" which implies it is an *existing* *path*. -- Pino Toscano signature.asc Description: This is a digitally signed message part.
Bug#876901: QFINDTESTDATA uses __FILE__
Pino Toscano: > Loose meanings do not imply neither the other way around, that you are > free to break because people cannot do anything with it. Also, > considering the very same behaviour (short of the different wording in > documentations) that is established for decades, this is a "de facto" > standard. > It's not, Microsoft doesn't do it. Last time I checked, QT is supposed to work on Windows. > [..] > >> https://gcc.gnu.org/onlinedocs/cpp/Standard-Predefined-Macros.html >> "__FILE__ and __LINE__ are useful in generating an error message [..]" >> >> https://msdn.microsoft.com/en-us/library/027c4t2s.aspx >> "Without /FC, the diagnostic text would look similar to this diagnostic >> text:" >> >> The only examples I can find in compiler documentation of __FILE__ >> usage, is for error messages. >> >> [..] >> >> http://c0x.coding-guidelines.com/6.10.8.html The presumed name of the >> current source file [..] >> https://software.intel.com/en-us/node/524489 Defined as a character >> string literal containing the name of the source file. >> https://www.ibm.com/support/knowledgecenter/SSAE4W_9.1.1/com.ibm.etools.iseries.langref.doc/ilcrefer13.htm >> A string literal representing the name of the file being compiled. >> >> You can even find threads online confirming __FILE__ is >> implementation-specific and not required by any standard to be a full >> path. You can even find threads of people complaining about the fact >> that __FILE__ sometimes has a full path, sometimes not - e.g. >> depending on what options you pass to the Microsoft compiler. > > The texts above clearly said that __FILE__ contains the path passed to > the compiler, which is very different from saying it is purely > "implementation-defined". [..] > That's false, you're misleading casual readers when you say this. *None* of them define it using the word "path", they all use the word "name" - except the GCC docs which say: "path by which the preprocessor opened the file, not the short name specified in ‘#include’ or as the input file name argument." Let me highlight that for you: "not [..] the input file name argument". X -- GPG: ed25519/56034877E1F87C35 GPG: rsa4096/1318EFAC5FBBDBCE https://github.com/infinity0/pubkeys.git
Bug#876901: QFINDTESTDATA uses __FILE__
In data mercoledì 15 novembre 2017 19:29:00 CET, Ximin Luo ha scritto: > Pino Toscano: > > [..] > > > >> In summary: in no document or standard, does it guarantee or imply > >> that __FILE__ can be taken to represent a real filesystem path. > >> Applications relying on this behaviour are broken and should not be > >> upset when things don't work. As documented in multiple places, > >> __FILE__ only has a very loose meaning, my patch fits within this > >> loose meaning, and it is intended mostly for error messages. > > > > ... this is your own interpretation. Even if __FILE__ has a loose > > meaning, you are still changing that small bit of meaning it has, > > without even thinking twice. Also, where it is written that __FILE__ > > is "only for error messages"? > > > > The loose meaning, indicates that programs should not rely on it for > other meanings, and should be open to fixing themselves if > implementations change their specifics. This is a general principle > that applies to all text in all standards, it is not "my own > interpretation". Loose meanings do not imply neither the other way around, that you are free to break because people cannot do anything with it. Also, considering the very same behaviour (short of the different wording in documentations) that is established for decades, this is a "de facto" standard. > And stop twisting my words. You literally just misquoted me to > advance your own point. I said "intended mostly for error messages": Point taken, but my argument remain: where it written it is "mostly for error messages"? > https://gcc.gnu.org/onlinedocs/cpp/Standard-Predefined-Macros.html > "__FILE__ and __LINE__ are useful in generating an error message [..]" > > https://msdn.microsoft.com/en-us/library/027c4t2s.aspx > "Without /FC, the diagnostic text would look similar to this diagnostic text:" > > The only examples I can find in compiler documentation of __FILE__ > usage, is for error messages. These are just very short documentation snippets; these documentations surely cannot cover all the cases, so the most prominent one is shown. > There are no examples of any other usage. That strongly suggests it > was originally intended for error messages and never intended to > support file lookups. That's not the same thing as saying "it's > intended only for error messages", but it does strongly imply that > you shouldn't be surprised if your file lookups break with minor > changes like my patch, Err no: the fact that something is not documented does not make it automatically bad. By this logic, __FILE__ would be the very last problems in sources of modern software. > and it certainly gives you no right to go off on a rant about me > "breaking" stuff. My rant replies to another rant about __FILE__... done by you. > http://c0x.coding-guidelines.com/6.10.8.html The presumed name of the > current source file [..] > https://software.intel.com/en-us/node/524489 Defined as a character > string literal containing the name of the source file. > https://www.ibm.com/support/knowledgecenter/SSAE4W_9.1.1/com.ibm.etools.iseries.langref.doc/ilcrefer13.htm > A string literal representing the name of the file being compiled. > > You can even find threads online confirming __FILE__ is > implementation-specific and not required by any standard to be a full > path. You can even find threads of people complaining about the fact > that __FILE__ sometimes has a full path, sometimes not - e.g. > depending on what options you pass to the Microsoft compiler. The texts above clearly said that __FILE__ contains the path passed to the compiler, which is very different from saying it is purely "implementation-defined". I never claimed that __FILE__ is a full path in all the cases. Here you are misquoting what I said in previous emails. > You need to get your facts straight ... > [...] Please stop ad-hominem attacks. Please stop ignoring other people's opinion. And yes, please get out of your own echo chamber. > And make some concrete proposals. I already did, multiple times; alas, you discarded them a priori, since they don't fit your own solution. -- Pino Toscano signature.asc Description: This is a digitally signed message part.
Bug#876901: QFINDTESTDATA uses __FILE__
Pino Toscano: > [..] > >> In summary: in no document or standard, does it guarantee or imply >> that __FILE__ can be taken to represent a real filesystem path. >> Applications relying on this behaviour are broken and should not be >> upset when things don't work. As documented in multiple places, >> __FILE__ only has a very loose meaning, my patch fits within this >> loose meaning, and it is intended mostly for error messages. > > ... this is your own interpretation. Even if __FILE__ has a loose > meaning, you are still changing that small bit of meaning it has, > without even thinking twice. Also, where it is written that __FILE__ > is "only for error messages"? > The loose meaning, indicates that programs should not rely on it for other meanings, and should be open to fixing themselves if implementations change their specifics. This is a general principle that applies to all text in all standards, it is not "my own interpretation". And stop twisting my words. You literally just misquoted me to advance your own point. I said "intended mostly for error messages": https://gcc.gnu.org/onlinedocs/cpp/Standard-Predefined-Macros.html "__FILE__ and __LINE__ are useful in generating an error message [..]" https://msdn.microsoft.com/en-us/library/027c4t2s.aspx "Without /FC, the diagnostic text would look similar to this diagnostic text:" The only examples I can find in compiler documentation of __FILE__ usage, is for error messages. There are no examples of any other usage. That strongly suggests it was originally intended for error messages and never intended to support file lookups. That's not the same thing as saying "it's intended only for error messages", but it does strongly imply that you shouldn't be surprised if your file lookups break with minor changes like my patch, and it certainly gives you no right to go off on a rant about me "breaking" stuff. http://c0x.coding-guidelines.com/6.10.8.html The presumed name of the current source file [..] https://software.intel.com/en-us/node/524489 Defined as a character string literal containing the name of the source file. https://www.ibm.com/support/knowledgecenter/SSAE4W_9.1.1/com.ibm.etools.iseries.langref.doc/ilcrefer13.htm A string literal representing the name of the file being compiled. You can even find threads online confirming __FILE__ is implementation-specific and not required by any standard to be a full path. You can even find threads of people complaining about the fact that __FILE__ sometimes has a full path, sometimes not - e.g. depending on what options you pass to the Microsoft compiler. Go do your own research about it. You need to get your facts straight before going on an ill-tempered rampage accusing me in bad faith of "not having a discussion" and being in "my own echo chamber". And stop pretending that my arguments are "my personal beliefs", they are not. And stop accusing me of blindly changing stuff against the standards, without knowing what the standards actually are. And stop accusing me of making my mind up before the discussion, when it is you that seems to have made up your mind before the discussion that I was "blindly" making changes and not thinking through consequences, I did think those through perfectly well thank you very much. And stop diverting the thread to making meta-comments about my word usage. It makes it seem like you're trying to make me look bad in front of an audience. I don't like playing those sorts of games, which is why I'm ignoring various other things you've been saying. And make some concrete proposals. X -- GPG: ed25519/56034877E1F87C35 GPG: rsa4096/1318EFAC5FBBDBCE https://github.com/infinity0/pubkeys.git
Bug#876901: QFINDTESTDATA uses __FILE__
In data mercoledì 15 novembre 2017 12:09:00 CET, Ximin Luo ha scritto: > Lisandro Damián Nicanor Pérez Meyer: > > [..] > > > > Xi: you also mentioned that having to file hundreds of patchs seems > > impossible. Well, it seems so, but it is actually not that necessary. > > Please > > allow me to explain the idea. > > > > Thanks for being less inflammatory than Pino. Flame that you started, mind you. > I agree that eventually all projects should move away from using > __FILE__. Again, why? Just because __FILE__ gives results that break reproducibility does not mean that it is wrong/bad -- the topic of this bug is exactly one of the cases where it *does* make sense to use __FILE__. > This BUILD_PATH_PREFIX_MAP variable is only a stepping stone to that, > just like SOURCE_DATE_EPOCH was a stepping stone to less projects > using __DATE__ and __TIME__. It allows people to see the obvious > benefit of a reproducible build, by actually achieving a large amount > of reproductions, today. We did not need to file mass bug reports for > __DATE__ and __TIME__ with SOURCE_DATE_EPOCH. Again: comparing with __DATE__ and __TIME__ makes no sense, since they were used in 99+% of the cases only for display purposes. OTOH, this thread shows __FILE__ it is not used (only) for that. > You're implying that QT's use of __FILE__ for tests, as being > discussed in this thread, is a "good use case" Exactly, it *is*. > That's not how I see it. As I pointed out various times already, the > use of __FILE__ here is *also not appropriate*. You can consider > these emails from me now, as "documenting/blogging" about "bad use > cases". OK, sure, we agree to disagree. But ... > In summary: in no document or standard, does it guarantee or imply > that __FILE__ can be taken to represent a real filesystem path. > Applications relying on this behaviour are broken and should not be > upset when things don't work. As documented in multiple places, > __FILE__ only has a very loose meaning, my patch fits within this > loose meaning, and it is intended mostly for error messages. ... this is your own interpretation. Even if __FILE__ has a loose meaning, you are still changing that small bit of meaning it has, without even thinking twice. Also, where it is written that __FILE__ is "only for error messages"? > The BUILD_PATH_PREFIX_MAP variable works around a lot of "bad use > cases", but it doesn't cover this particular case. Some minor tweaks > are needed to cover it, and I suggested them already. So this is not a solution, but low quality workaround. > But you guys continue to push the position "we're doing nothing > wrong, it's other people doing stuff wrong, go talk to them instead". Let's flip this: you continue to push the position "everybody using __FILE__ is wrong, I won't listen to anybody". > I'm sorry but it's not a convincing position for me to agree with. Neither is yours, sorry. > At the end of the day, all of these cases, including yours, ought to > be fixed to not use __FILE__ at all. Again, we agree to disagree. -- Pino Toscano signature.asc Description: This is a digitally signed message part.
Bug#876901: QFINDTESTDATA uses __FILE__
In data mercoledì 15 novembre 2017 10:57:00 CET, Ximin Luo ha scritto: > Pino Toscano: > > In data martedì 14 novembre 2017 11:14:00 CET, Ximin Luo ha scritto: > >> You're using __FILE__ inappropriately, none of the documentation > >> guarantees or implies that you can access __FILE__ as a real > >> filesystem path. "Surely for relative paths" is your justification > >> for your own broken behaviour. > > > > Again, this is your own echo chamber: "__FILE__ is broken, everybody > > using it is broken no matter what". > > > > It's not my "own echo chamber", I pointed you to lots of docs that > confirm your usage of __FILE__ is not appropriate here. I don't see any of these documentations. All you did was taking GCC and MSVC documentations and declare they mean nothing. > Adrian Bunk also mentioned the C89 to me on IRC yesterday. Again, the > wording gives no guarantee that __FILE__ should represent a real > filesystem path. The default is to be what is passed to the compiler as argument, which *is* a filesystem path. If the source uses #line, then that is invalidated, but this is another situation. > If my previous words were a bit terse I am sorry for that, but I don't > appreciate comments like "Any of your solutions get a big, fat, and > giant nope" after trying to explain the problem and present you with > no less than 4 alternative simple unintrusive solutions. My comment comes after you keep ignoring whatever else is said to you. I explained your solution changes a standard behaviour (that, like it or not, it *is* that, and the fact that so far it was used for decades like that means it is a portable and standard behaviour). You (generic "you") cannot pretend to have discussion, when you basically decided a priori (although in a very arguable way) what is "right" and what is "wrong", and thus discarding any other opinion which does not fit into your criterias. Just like Lisandro, I'm all for reproducibility, but this does not mean standard things must be broken "just because". What you want to change is: a) standard (standard C, and standard compiler behaviour) b) so widely used so trying to find a solution a) without looking around at least a good majority of the use cases b) flagging all the usages as "invalid" it does not work. And no, it simply does not. It hardly is a discussion at all, rather an echo chamber for your ideas. -- Pino Toscano signature.asc Description: This is a digitally signed message part.
Bug#876901: QFINDTESTDATA uses __FILE__
El 15 nov. 2017 8:00 a.m., "Ximin Luo"escribió: Pino Toscano: > In data martedì 14 novembre 2017 11:14:00 CET, Ximin Luo ha scritto: >> You're using __FILE__ inappropriately, none of the documentation >> guarantees or implies that you can access __FILE__ as a real >> filesystem path. "Surely for relative paths" is your justification >> for your own broken behaviour. > > Again, this is your own echo chamber: "__FILE__ is broken, everybody > using it is broken no matter what". > It's not my "own echo chamber", I pointed you to lots of docs that confirm your usage of __FILE__ is not appropriate here. Adrian Bunk also mentioned the C89 to me on IRC yesterday. Again, the wording gives no guarantee that __FILE__ should represent a real filesystem path. If my previous words were a bit terse I am sorry for that, but I don't appreciate comments like "Any of your solutions get a big, fat, and giant nope" after trying to explain the problem and present you with no less than 4 alternative simple unintrusive solutions. >> You can either accept my one line patch suggestion, or fix it some >> other way. > > I am not interested in working around broken changes introduced blindly > for very doubtful reasons. > >> I'm not going to change the GCC patch, it does nothing wrong. > > Let me add also another POV to this approach: do you really expect > Debian to carry this important diversion for GCC upstream? I really > doubt GCC will accept this. > I'm in the process of getting the patch into GCC. We certainly don't intend to keep this as a Debian-specific thing. [1] If they don't accept it, you don't need to patch your package - but I'd say the use of __FILE__ is still not the best, since no documentation implies it can be used to find files, and all the examples only mention error messages. And to be clear, in case Holger's earlier messages were missed, the FTBFS is currently only on tests.r-b.org and not on the official Debian archive. I see your point here. I'm currently on the phone, please allow me some time to get into a proper keyboard and make a. Proposals that I think will work for both sides.
Bug#876901: QFINDTESTDATA uses __FILE__
Lisandro Damián Nicanor Pérez Meyer: > [..] > > Xi: you also mentioned that having to file hundreds of patchs seems > impossible. Well, it seems so, but it is actually not that necessary. Please > allow me to explain the idea. > Thanks for being less inflammatory than Pino. I agree that eventually all projects should move away from using __FILE__. This BUILD_PATH_PREFIX_MAP variable is only a stepping stone to that, just like SOURCE_DATE_EPOCH was a stepping stone to less projects using __DATE__ and __TIME__. It allows people to see the obvious benefit of a reproducible build, by actually achieving a large amount of reproductions, today. We did not need to file mass bug reports for __DATE__ and __TIME__ with SOURCE_DATE_EPOCH. > What you can do here is starting by documenting/blogging about bad use cases > so people have something to read when bugs arrive. > > [..] You're implying that QT's use of __FILE__ for tests, as being discussed in this thread, is a "good use case" - and that it is *other people* with "bad use cases" that we should be spending a lot of time and effort to reach out to. That's not how I see it. As I pointed out various times already, the use of __FILE__ here is *also not appropriate*. You can consider these emails from me now, as "documenting/blogging" about "bad use cases". In summary: in no document or standard, does it guarantee or imply that __FILE__ can be taken to represent a real filesystem path. Applications relying on this behaviour are broken and should not be upset when things don't work. As documented in multiple places, __FILE__ only has a very loose meaning, my patch fits within this loose meaning, and it is intended mostly for error messages. The BUILD_PATH_PREFIX_MAP variable works around a lot of "bad use cases", but it doesn't cover this particular case. Some minor tweaks are needed to cover it, and I suggested them already. But you guys continue to push the position "we're doing nothing wrong, it's other people doing stuff wrong, go talk to them instead". I'm sorry but it's not a convincing position for me to agree with. At the end of the day, all of these cases, including yours, ought to be fixed to not use __FILE__ at all. BUILD_PATH_PREFIX_MAP doesn't prevent the fix, it just means we can achieve many many real reproductions today without having to wait. That's a good thing. X -- GPG: ed25519/56034877E1F87C35 GPG: rsa4096/1318EFAC5FBBDBCE https://github.com/infinity0/pubkeys.git
Bug#876901: QFINDTESTDATA uses __FILE__
Pino Toscano: > In data martedì 14 novembre 2017 11:14:00 CET, Ximin Luo ha scritto: >> You're using __FILE__ inappropriately, none of the documentation >> guarantees or implies that you can access __FILE__ as a real >> filesystem path. "Surely for relative paths" is your justification >> for your own broken behaviour. > > Again, this is your own echo chamber: "__FILE__ is broken, everybody > using it is broken no matter what". > It's not my "own echo chamber", I pointed you to lots of docs that confirm your usage of __FILE__ is not appropriate here. Adrian Bunk also mentioned the C89 to me on IRC yesterday. Again, the wording gives no guarantee that __FILE__ should represent a real filesystem path. If my previous words were a bit terse I am sorry for that, but I don't appreciate comments like "Any of your solutions get a big, fat, and giant nope" after trying to explain the problem and present you with no less than 4 alternative simple unintrusive solutions. >> You can either accept my one line patch suggestion, or fix it some >> other way. > > I am not interested in working around broken changes introduced blindly > for very doubtful reasons. > >> I'm not going to change the GCC patch, it does nothing wrong. > > Let me add also another POV to this approach: do you really expect > Debian to carry this important diversion for GCC upstream? I really > doubt GCC will accept this. > I'm in the process of getting the patch into GCC. We certainly don't intend to keep this as a Debian-specific thing. [1] If they don't accept it, you don't need to patch your package - but I'd say the use of __FILE__ is still not the best, since no documentation implies it can be used to find files, and all the examples only mention error messages. And to be clear, in case Holger's earlier messages were missed, the FTBFS is currently only on tests.r-b.org and not on the official Debian archive. X [1] Although interestingly, NetBSD have a GCC patch that was written by us (dkg specifically) for a related purpose, that was not accepted into GCC. But they use it for reproducibility purposes. -- GPG: ed25519/56034877E1F87C35 GPG: rsa4096/1318EFAC5FBBDBCE https://github.com/infinity0/pubkeys.git
Bug#876901: QFINDTESTDATA uses __FILE__
In data martedì 14 novembre 2017 13:47:56 CET, Holger Levsen ha scritto: > On Tue, Nov 14, 2017 at 07:22:07AM +0100, Pino Toscano wrote: > > > There are quite a lot of packages that use __FILE__ so forgive me for > > > not checking every single use-case of it. > > > > This. When something is used so widely, then changing its behaviour > > blindly is simply a no-no. > > I tend to agree. > > > > This will require us to patch hundreds of packages, and isn't > > > realistic. > > > > If "hundreds of packages" misuse __FILE__, then simply *fix them*. > > Sure, it will require time, energy, etc, but I do not see other ways > > around that without breaking standard behaviours. > > also agreed. > > > > Please, you try sending hundreds of patches, then I will take your > > > "solution" more seriously. > > My *solution* (without quotes) is more realistic than your blind > > breakage. > > Pino, could you be so kind and repeat your proposed solution here again, > for the sake of clarity?! It is basically what I wrote in the quoted part above (not this one, the last where you replied "also agreed". I will repeat it again, as asked: | No, the solution is: | a) *not* break what __FILE__ means | b) remove the misuses of __FILE__ in packages (not the case of | QFINDTESTDATA) As also Lisandro explained a bit more in another email, this solution takes a bit more time and effort, but it pays off (especially if this is done directly upstream) as all the improper usages will be removed, with little to no effort specific for Debian. -- Pino Toscano signature.asc Description: This is a digitally signed message part.
Bug#876901: QFINDTESTDATA uses __FILE__
In data martedì 14 novembre 2017 11:14:00 CET, Ximin Luo ha scritto: > You're using __FILE__ inappropriately, none of the documentation > guarantees or implies that you can access __FILE__ as a real > filesystem path. "Surely for relative paths" is your justification > for your own broken behaviour. Again, this is your own echo chamber: "__FILE__ is broken, everybody using it is broken no matter what". > You can either accept my one line patch suggestion, or fix it some > other way. I am not interested in working around broken changes introduced blindly for very doubtful reasons. > I'm not going to change the GCC patch, it does nothing wrong. Let me add also another POV to this approach: do you really expect Debian to carry this important diversion for GCC upstream? I really doubt GCC will accept this. -- Pino Toscano signature.asc Description: This is a digitally signed message part.
Bug#876901: QFINDTESTDATA uses __FILE__
On martes, 14 de noviembre de 2017 13:45:37 -03 Holger Levsen wrote: > On Tue, Nov 14, 2017 at 11:34:00AM +, Ximin Luo wrote: > > > Sorry, it is not realistic to force us to have a delta from upstream for > > > no > > > direct gain. > > > > None of the solutions I suggested involve patching upstream, they would be > > done in d/rules. > "we do not only care about Debian but free software in general" - so how > to do you propose this should be handled for non Debian, eg rpms or > plain building from source? > > A Debian only solution for this problem is pretty bad or rather, not a > real solution. Please allow me to try to analyze this situation from another perspective, much more related to what Holger wrote above. First of all, I want to emphasize that we Qt/KDE maintainers do see a real value in both reproducibilty and (because it was mentioned before) cross building efforts. For the cross part Helmut Grone and Dmitry Schanev have been doing an *amazing* work to avoid having to create multiple Qt builds to be able to cross build stuff, which would have been much easier but a worse solution for everyone. In the same vein Sune Vuorela has been creating patches *directly* to Qt upstream in order to improve reproducibility within Qt. It took time, but upstream did undertood the reasons behind them and are now part of our code. I do understand that wanting to change what __FILE__ means is an interesting approach, specially because it solves many issues at once (IIRC ~1800?). But it has some drawbacks too: - It changes the meaning of a well predefined macro (and as I say before I consider this a delta), which can have real valid usages like in our case. - It does not expands outside Debian. - It does not reinforce the idea of reproducibility to programmers. Other solutions to related issues made upstreams know and understand the issue, so everybody got a better experience. Xi: you also mentioned that having to file hundreds of patchs seems impossible. Well, it seems so, but it is actually not that necessary. Please allow me to explain the idea. What you can do here is starting by documenting/blogging about bad use cases so people have something to read when bugs arrive. Then go ahead with mass bug filing: announce it in debian-devel first, let people get involved, then file whatever amount of bugs is necessary, being sure to link the aforementioned doc of the first step. At this point many bugs will get an upstream bug, patches upstream, developers understanding the issue... and many bugs being fixed by other people. Projects will get a better source code, all distros can benefit from it an most probably eve cooperate by patching their preferred sources upstream. Drawbacks: it takes much more time. But I think the final result justifies it. You might wonder if there has been any experience on this before. At least from my POV I can say yes. Removing Qt3 in favour of Qt4 did not meant simply asking for the Qt3 sources removal but pushing people to do their best to port the code. It took us (if I remember correctly) one and a half release. Removing Qt4 is an effort currently undergoing, which has already taken at least three Debian releases. Is it worth the effort? Yes! Because we pushed many sources to be ported, thus giving projects a new lifespan, better usability, better code... So yes, it might not sound pretty, but the long road, in my opinion at least, is the way to go here. Even if it means filing 1800 bugs. -- Una vez que hemos eliminado lo imposible, lo que queda, por improbable que parezca, es la verdad. Sherlock Holmes Lisandro Damián Nicanor Pérez Meyer http://perezmeyer.com.ar/ http://perezmeyer.blogspot.com/ signature.asc Description: This is a digitally signed message part.
Bug#876901: QFINDTESTDATA uses __FILE__
On martes, 14 de noviembre de 2017 11:34:00 -03 Ximin Luo wrote: > Lisandro Damián Nicanor Pérez Meyer: > > [..] > > > > Sorry, it is not realistic to force us to have a delta from upstream for > > no > > direct gain. > > None of the solutions I suggested involve patching upstream, they would be > done in d/rules. The proposed change changes a macro. That means the resulting code is different, no matter if the change comes from a patch or debian/rules. Consider this "a patch in debian/rules". Allow me to try another way to see this issue, please see my next mail (to keep the threading better). -- Without us [Free Software developers], people would study computer science and programming without ever having seen a real program in its entirety. That's like becoming writers without ever having read a complete book. Matthias Ettrich, founder of the KDE project. http://www.efytimes.com/efytimes/25412/news.htm Lisandro Damián Nicanor Pérez Meyer http://perezmeyer.com.ar/ http://perezmeyer.blogspot.com/ signature.asc Description: This is a digitally signed message part.
Bug#876901: QFINDTESTDATA uses __FILE__
On Tue, Nov 14, 2017 at 07:22:07AM +0100, Pino Toscano wrote: > > There are quite a lot of packages that use __FILE__ so forgive me for > > not checking every single use-case of it. > > This. When something is used so widely, then changing its behaviour > blindly is simply a no-no. I tend to agree. > > This will require us to patch hundreds of packages, and isn't > > realistic. > > If "hundreds of packages" misuse __FILE__, then simply *fix them*. > Sure, it will require time, energy, etc, but I do not see other ways > around that without breaking standard behaviours. also agreed. > > Please, you try sending hundreds of patches, then I will take your > > "solution" more seriously. > My *solution* (without quotes) is more realistic than your blind > breakage. Pino, could you be so kind and repeat your proposed solution here again, for the sake of clarity?! -- cheers, Holger signature.asc Description: PGP signature
Bug#876901: QFINDTESTDATA uses __FILE__
On Tue, Nov 14, 2017 at 11:34:00AM +, Ximin Luo wrote: > > Sorry, it is not realistic to force us to have a delta from upstream for no > > direct gain. > None of the solutions I suggested involve patching upstream, they would be > done in d/rules. "we do not only care about Debian but free software in general" - so how to do you propose this should be handled for non Debian, eg rpms or plain building from source? A Debian only solution for this problem is pretty bad or rather, not a real solution. -- cheers, Holger signature.asc Description: PGP signature
Bug#876901: QFINDTESTDATA uses __FILE__
Lisandro Damián Nicanor Pérez Meyer: > [..] > > Sorry, it is not realistic to force us to have a delta from upstream for no > direct gain. > None of the solutions I suggested involve patching upstream, they would be done in d/rules. X -- GPG: ed25519/56034877E1F87C35 GPG: rsa4096/1318EFAC5FBBDBCE https://github.com/infinity0/pubkeys.git
Bug#876901: QFINDTESTDATA uses __FILE__
Pino Toscano: > [..] > >> Well let's take a look at the standards: >> [...] > > Even with different wordings, both describe basically the same > behaviour. And yes, none of them says that __FILE__ is a full path, > but surely for relative paths the combination of $srcdir + __FILE__ > will give me the path to the source. > Whatever, I don't care about having such a pointless long discussion. You're using __FILE__ inappropriately, none of the documentation guarantees or implies that you can access __FILE__ as a real filesystem path. "Surely for relative paths" is your justification for your own broken behaviour. And that's just for GCC, IBM and Microsoft both use the term "name of the file" which don't even suggest any sort of path. You can either accept my one line patch suggestion, or fix it some other way. I'm not going to change the GCC patch, it does nothing wrong. Not going to spend any more time on this. X -- GPG: ed25519/56034877E1F87C35 GPG: rsa4096/1318EFAC5FBBDBCE https://github.com/infinity0/pubkeys.git
Bug#876901: QFINDTESTDATA uses __FILE__
In data lunedì 13 novembre 2017 23:44:00 CET, Ximin Luo ha scritto: > Pino Toscano: > > [..] > > > > This is not an annoyance, it is the crux of the problem! __FILE__ is > > something standard, with a very well defined behaviour, upon which > > people rely on: you cannot trash it from one day to another like this, > > saying "too bad" to all its users, even those using it appropriately. > > It does not simply work. > > > >> but the point is that the previous use does not generalise well. > > > > To be honest: I already said multiple times that abuses of __FILE__ > > *must* be fixed. However, there are valid use cases, and in this case > > the only generalization is done by you, who declared __FILE__ as > > "absolutely BAD". > > > > It's an objective fact that tests using __FILE__ are unable to be > generalised to be run outside of the build. Yes, and this is already acknoledged. > As Lisandro said in the other comment, "We do *not* want to run the > tests outside the build." This refers to Qt tests, and those using QFINDTESTDATA in particular, for reasons I already explained in another reply. > >> It's not a sledgehammer solution, it's tweakable and we can try to > >> use its flexibility to un-break your tests. > > > > No, it *is* a sledgehammer solution, since you are not even checking > > what __FILE__ is used for, and thus blindly changing it without any > > pre-research on its effect. Again, this is not the correct approach > > to fix the issue at hand. > > > > There are quite a lot of packages that use __FILE__ so forgive me for > not checking every single use-case of it. This. When something is used so widely, then changing its behaviour blindly is simply a no-no. > My patch fixes 1800 packages, and the amount of extra FTBFS it caused > (such as this) is a blip in the stats, and a very minor one at that - > failing tests, because a file was not found. Using statistics to justify a bad behaviour change like this is not a good way to go. This "blip is the stats" ought to show you the behaviour change broke *valid* use cases; also, this does not account how many packages build, but will fail to work at runtime because of this. > (Yes it is RC, but technically it is minor and very easy to fix, but > you're denying us the fix because you seem to feel that the standards > are on your side. To be honest, the only denial I see is your refusal to acknoledge your behaviour change is a bad regression, and a no-go. > Well let's take a look at the standards: > [...] Even with different wordings, both describe basically the same behaviour. And yes, none of them says that __FILE__ is a full path, but surely for relative paths the combination of $srcdir + __FILE__ will give me the path to the source. > You're forcing us to choose between fixing 1800 packages minus your > QT packages, or fixing nothing. And again, your position here is "black or white": either "we change __FILE__ to our liking", or "we fix nothing". There is the middle ground of fixing misuses, which are *already* bugs floating in packages, regardless of reproducible builds. > I ask you to be slightly less absolutist. I am already. That is why I suggested to fix only what is broken in packages. > The solution I proposed is 1-line and should cause basically no > disruption to anything else. By your own admission above, there are so many usages of __FILE__ that you did not check all of them. OK, but in this case, how do you claim your solution will not break anything else? Just because something else builds, then it does not imply it will run correctly at runtime. Even beside this, this case is one *valid* use case of __FILE__ that breaks your assumptions. > >> If this isn't suitable to you, how else do you suppose we should try > >> to make builds independent of (i.e. not embed) the build path? > >> Suggestions are welcome. > > > > I already wrote it in my email, and I repeat it again: > > > > | No, the solution is: > > | a) *not* break what __FILE__ means > > | b) remove the misuses of __FILE__ in packages (not the case of > > |QFINDTESTDATA) > > > > This will require us to patch hundreds of packages, and isn't > realistic. If "hundreds of packages" misuse __FILE__, then simply *fix them*. Sure, it will require time, energy, etc, but I do not see other ways around that without breaking standard behaviours. It is a bit like gets(): it is old, buggy, and obsolete, so sources using it are buggy already. Then, the proper way to fix them is to switch them to use something else, not to make gets() a no-op in libc. > Please, you try sending hundreds of patches, then I will take your > "solution" more seriously. My *solution* (without quotes) is more realistic than your blind breakage. -- Pino Toscano signature.asc Description: This is a digitally signed message part.
Bug#876901: QFINDTESTDATA uses __FILE__
On lunes, 13 de noviembre de 2017 23:44:00 -03 Ximin Luo wrote: > Pino Toscano: > > [..] > > > > This is not an annoyance, it is the crux of the problem! __FILE__ is > > something standard, with a very well defined behaviour, upon which > > people rely on: you cannot trash it from one day to another like this, > > saying "too bad" to all its users, even those using it appropriately. > > It does not simply work. > > > >> but the point is that the previous use does not generalise well. > > > > To be honest: I already said multiple times that abuses of __FILE__ > > *must* be fixed. However, there are valid use cases, and in this case > > the only generalization is done by you, who declared __FILE__ as > > "absolutely BAD". > > It's an objective fact that tests using __FILE__ are unable to be > generalised to be run outside of the build. No, it's by design. > As Lisandro said in the other comment, "We do *not* want to run the tests > outside the build." > > As I said in my previous comment, "For example if you try a program that > works fine when compiled natively but doesn't work when cross-compiled, > that is a failure to generalise and one can either say "not supported, stop > breaking us" or one can try to generalise one's program." Sorry, but I do not buy that comparison. I do understan the cross compilation effort and the reproducibility one, but this test are designed and ment to be run at build time, they are unti tests for a purpose. > It's also an objective fact that we're encouraging post-install tests such > as autopkgtest. Pre-install tests (dh_auto_test) are good because of a > tighter feedback loop, post-install tests are good because they are more > accurate, they test directly what users install. For example, that > installation paths were actually correct. And the test this change are breaking are not ment to be run as post-install tests. > >> It's not a sledgehammer solution, it's tweakable and we can try to > >> use its flexibility to un-break your tests. > > > > No, it *is* a sledgehammer solution, since you are not even checking > > what __FILE__ is used for, and thus blindly changing it without any > > pre-research on its effect. Again, this is not the correct approach > > to fix the issue at hand. > > There are quite a lot of packages that use __FILE__ so forgive me for not > checking every single use-case of it. > > My patch fixes 1800 packages, and the amount of extra FTBFS it caused (such > as this) is a blip in the stats, and a very minor one at that - failing > tests, because a file was not found. Give me a break. But then if we accept a work around as you porpose it becomes a delta from upstream. There is too much in our plates to be forced to keep a delta. > (Yes it is RC, but technically it is minor and very easy to fix, but you're > denying us the fix because you seem to feel that the standards are on your > side. Well let's take a look at the standards: > > - https://gcc.gnu.org/onlinedocs/cpp/Standard-Predefined-Macros.html > "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. " Nope, no > guarantee of absolute path here. The preprocesser could have cd'd to a > random directory and then opened the file by a relative path. Nobody asked for full paths. > - https://msdn.microsoft.com/en-us/library/027c4t2s.aspx > "__FILE__ The name of the current source file. __FILE__ expands to a > character string literal. To ensure that the full path to the file is > displayed, use /FC (Full Path of Source Code File in Diagnostics). This > macro is always defined." No guarantee of absolute path here either. It > even says to guarantee full path you need to pass a specific compiler > switch. > > So, get off your high horse about "standards" and "well-defined". I really > didn't want to pull this card because I think it is missing the point, but > you seem to hold onto it with particular religious fervour.) > >> So, what do you think of this alternate solution, that I suggested: > > Any of your solutions get a big, fat, and giant nope. > > You're forcing us to choose between fixing 1800 packages minus your QT > packages, or fixing nothing. I ask you to be slightly less absolutist. The > solution I proposed is 1-line and should cause basically no disruption to > anything else. And you are forcing us to keep a delta from upstream. If a unit test now fails upstream will look at it and the first thing they will say is "Fix your macros". They have done so in the past and they where right. > >> If this isn't suitable to you, how else do you suppose we should try > >> to make builds independent of (i.e. not embed) the build path? > >> Suggestions are welcome. > > > > I already wrote it in my email, and I repeat it again: > > | No, the solution is: > > | a) *not* break what __FILE__ means > > | b) remove the misuses of __FILE__ in packages (not the case of > > | > > |
Bug#876901: QFINDTESTDATA uses __FILE__
Pino Toscano: > [..] > > This is not an annoyance, it is the crux of the problem! __FILE__ is > something standard, with a very well defined behaviour, upon which > people rely on: you cannot trash it from one day to another like this, > saying "too bad" to all its users, even those using it appropriately. > It does not simply work. > >> but the point is that the previous use does not generalise well. > > To be honest: I already said multiple times that abuses of __FILE__ > *must* be fixed. However, there are valid use cases, and in this case > the only generalization is done by you, who declared __FILE__ as > "absolutely BAD". > It's an objective fact that tests using __FILE__ are unable to be generalised to be run outside of the build. As Lisandro said in the other comment, "We do *not* want to run the tests outside the build." As I said in my previous comment, "For example if you try a program that works fine when compiled natively but doesn't work when cross-compiled, that is a failure to generalise and one can either say "not supported, stop breaking us" or one can try to generalise one's program." It's also an objective fact that we're encouraging post-install tests such as autopkgtest. Pre-install tests (dh_auto_test) are good because of a tighter feedback loop, post-install tests are good because they are more accurate, they test directly what users install. For example, that installation paths were actually correct. >> It's not a sledgehammer solution, it's tweakable and we can try to >> use its flexibility to un-break your tests. > > No, it *is* a sledgehammer solution, since you are not even checking > what __FILE__ is used for, and thus blindly changing it without any > pre-research on its effect. Again, this is not the correct approach > to fix the issue at hand. > There are quite a lot of packages that use __FILE__ so forgive me for not checking every single use-case of it. My patch fixes 1800 packages, and the amount of extra FTBFS it caused (such as this) is a blip in the stats, and a very minor one at that - failing tests, because a file was not found. Give me a break. (Yes it is RC, but technically it is minor and very easy to fix, but you're denying us the fix because you seem to feel that the standards are on your side. Well let's take a look at the standards: - https://gcc.gnu.org/onlinedocs/cpp/Standard-Predefined-Macros.html "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. " Nope, no guarantee of absolute path here. The preprocesser could have cd'd to a random directory and then opened the file by a relative path. - https://msdn.microsoft.com/en-us/library/027c4t2s.aspx "__FILE__ The name of the current source file. __FILE__ expands to a character string literal. To ensure that the full path to the file is displayed, use /FC (Full Path of Source Code File in Diagnostics). This macro is always defined." No guarantee of absolute path here either. It even says to guarantee full path you need to pass a specific compiler switch. So, get off your high horse about "standards" and "well-defined". I really didn't want to pull this card because I think it is missing the point, but you seem to hold onto it with particular religious fervour.) >> So, what do you think of this alternate solution, that I suggested: > > Any of your solutions get a big, fat, and giant nope. > You're forcing us to choose between fixing 1800 packages minus your QT packages, or fixing nothing. I ask you to be slightly less absolutist. The solution I proposed is 1-line and should cause basically no disruption to anything else. >> If this isn't suitable to you, how else do you suppose we should try >> to make builds independent of (i.e. not embed) the build path? >> Suggestions are welcome. > > I already wrote it in my email, and I repeat it again: > > | No, the solution is: > | a) *not* break what __FILE__ means > | b) remove the misuses of __FILE__ in packages (not the case of > |QFINDTESTDATA) > This will require us to patch hundreds of packages, and isn't realistic. Please, you try sending hundreds of patches, then I will take your "solution" more seriously. X -- GPG: ed25519/56034877E1F87C35 GPG: rsa4096/1318EFAC5FBBDBCE https://github.com/infinity0/pubkeys.git
Bug#876901: QFINDTESTDATA uses __FILE__
On lunes, 13 de noviembre de 2017 20:03:00 -03 Ximin Luo wrote: [snip] > > This is clearly a misuse, and thus it must be fixed. OTOH, the > > comparison with __FILE__ is not appropriate. > > Why's it not appropriate? Because it changes well defined macro. > If you ever want to write tests to be runnable > outside the build, e.g. with autopkgtest, then you're going to have to not > use __FILE__ anyway. (Assuming you install the tests somewhere, rather than > running the whole build again.) That's the point actually. We do *not* want to run the tests outside the build. They are designed to do exactly that: run during the build, not later. > I can understand that breaking something that used to work is annoying, but > the point is that the previous use does not generalise well. No, the problem is that __FILE__ is well defined and in this particular case it's use is just correct. > I don't really > want to get into holy wars about the definition of "validity", I just ask > you to consider generality. And I want you to respect well defined macros. > For example if you try a program that works > fine when compiled natively but doesn't work when cross-compiled, that is a > failure to generalise and one can either say "not supported, stop breaking > us" or one can try to generalise one's program. And that's different from changing the meaning of a macro. > It's not a sledgehammer solution, it's tweakable and we can try to use its > flexibility to un-break your tests. So, what do you think of this alternate > solution, that I suggested: Again: no. And I still consider this a bug in *your* side. Please reassign as appropriate. -- Un viejo proverbio de El.Machi dice que la memoria es como las papas fritas... ¡nunca sobran! Lisandro Damián Nicanor Pérez Meyer http://perezmeyer.com.ar/ http://perezmeyer.blogspot.com/ signature.asc Description: This is a digitally signed message part.
Bug#876901: QFINDTESTDATA uses __FILE__
In data lunedì 13 novembre 2017 20:20:00 CET, Ximin Luo ha scritto: > Pino Toscano: > > [..] > > > > A better approach here is to work on removing the invalid & abusing > > usages of __FILE__ from packages, just like it was done for __DATE__. > > > > We in fact did not do the latter because it was easier to implement > SOURCE_DATE_EPOCH to fix the expansion of __DATE__, rather than patch > 400 packages. To be fair, it should be noted that various project stopped using it altogether, as result of "reproducible builds" patches. Also, __DATE__ expnds to a C string like "Nov 13 2017": https://gcc.gnu.org/onlinedocs/cpp/Standard-Predefined-Macros.html this means that you cannot do something like "__DATE__ + 3" you mentioned earlier, but that requires a bit more work to parse it... and at that point using gmtime() & friends is then way easier. Indeed, all the usages I have seen so far are just simple display of the build time, with messages like "Foo App vX.Y built at " __DATE__ " " __TIME__ Hence, changing what __DATE__ returns is almost a no-op for the 99+% of sources using it, since there is basically almost no other usage for it. OTOH, as already mentioned in other replies, __FILE__ is actually *used* for stuff, it is not a mere display string. -- Pino Toscano signature.asc Description: This is a digitally signed message part.
Bug#876901: QFINDTESTDATA uses __FILE__
In data lunedì 13 novembre 2017 20:03:00 CET, Ximin Luo ha scritto: > Pino Toscano: > > [..] > > > > No, the solution is: > > a) *not* break what __FILE__ means > > b) remove the misuses of __FILE__ in packages (not the case of > >QFINDTESTDATA) > > > >> I am not "blaming the user", but pointing to the fact that __FILE__ > >> is being used in a surprising way here, which is not good for > >> reproducible builds. > > > > What I see it is happening here is: you (= people working on > > reproducible builds) see __FILE__, and the problems that arise from its > > abuse; to overcome this issue, you use the sledgehammer solution, > > basically changing what __FILE__ means, and thus breaking even valid > > use cases. Sorry, but I do not see how this is useful. > > > > A better approach here is to work on removing the invalid & abusing > > usages of __FILE__ from packages, just like it was done for __DATE__. > > > >> An analogy would be to write your program to execute something at > >> time "__DATE__ + 30 seconds". This is obviously ridiculous, but works > >> "by accident" if done inside a test case. > > > > This is clearly a misuse, and thus it must be fixed. OTOH, the > > comparison with __FILE__ is not appropriate. > > > Why's it not appropriate? Because using __DATE__ as timing for a test is extremely fragile, and it has nothing to do with reproducible builds. It is just wrong, and as such __DATE__ is used wrongly. OTOH, as I already explained, at least what QFINDTESTDATA does with __FILE__ is perfectly valid and acceptable, since it is about stuff run during test suites at build time only. Again, see: http://doc.qt.io/qt-5/qtest.html#QFINDTESTDATA > If you ever want to write tests to be runnable outside the build, > e.g. with autopkgtest, then you're going to have to not use __FILE__ > anyway. Sure, and it is exactly what I said in point (b) of the solution I proposed above. > (Assuming you install the tests somewhere, rather than > running the whole build again.) This will not work with tests using QFINDTESTDATA anyway, as it works with either the source or build directory of the source using it, or with the Qt installation prefix. (Hint: none of them is useful for an installed test.) > I can understand that breaking something that used to work is > annoying, This is not an annoyance, it is the crux of the problem! __FILE__ is something standard, with a very well defined behaviour, upon which people rely on: you cannot trash it from one day to another like this, saying "too bad" to all its users, even those using it appropriately. It does not simply work. > but the point is that the previous use does not generalise well. To be honest: I already said multiple times that abuses of __FILE__ *must* be fixed. However, there are valid use cases, and in this case the only generalization is done by you, who declared __FILE__ as "absolutely BAD". > It's not a sledgehammer solution, it's tweakable and we can try to > use its flexibility to un-break your tests. No, it *is* a sledgehammer solution, since you are not even checking what __FILE__ is used for, and thus blindly changing it without any pre-research on its effect. Again, this is not the correct approach to fix the issue at hand. > So, what do you think of this alternate solution, that I suggested: Any of your solutions get a big, fat, and giant nope. > If this isn't suitable to you, how else do you suppose we should try > to make builds independent of (i.e. not embed) the build path? > Suggestions are welcome. I already wrote it in my email, and I repeat it again: | No, the solution is: | a) *not* break what __FILE__ means | b) remove the misuses of __FILE__ in packages (not the case of |QFINDTESTDATA) -- Pino Toscano signature.asc Description: This is a digitally signed message part.
Bug#876901: QFINDTESTDATA uses __FILE__
Pino Toscano: > [..] > > A better approach here is to work on removing the invalid & abusing > usages of __FILE__ from packages, just like it was done for __DATE__. > We in fact did not do the latter because it was easier to implement SOURCE_DATE_EPOCH to fix the expansion of __DATE__, rather than patch 400 packages. With __FILE__ I believe the number is similar, but I didn't yet count exactly how many. (Our BUILD_PATH_PREFIX_MAP patch fixes 1800 packages, but only some of these are due to __FILE__.) X -- GPG: ed25519/56034877E1F87C35 GPG: rsa4096/1318EFAC5FBBDBCE https://github.com/infinity0/pubkeys.git
Bug#876901: QFINDTESTDATA uses __FILE__
Pino Toscano: > [..] > > No, the solution is: > a) *not* break what __FILE__ means > b) remove the misuses of __FILE__ in packages (not the case of >QFINDTESTDATA) > >> I am not "blaming the user", but pointing to the fact that __FILE__ >> is being used in a surprising way here, which is not good for >> reproducible builds. > > What I see it is happening here is: you (= people working on > reproducible builds) see __FILE__, and the problems that arise from its > abuse; to overcome this issue, you use the sledgehammer solution, > basically changing what __FILE__ means, and thus breaking even valid > use cases. Sorry, but I do not see how this is useful. > > A better approach here is to work on removing the invalid & abusing > usages of __FILE__ from packages, just like it was done for __DATE__. > >> An analogy would be to write your program to execute something at >> time "__DATE__ + 30 seconds". This is obviously ridiculous, but works >> "by accident" if done inside a test case. > > This is clearly a misuse, and thus it must be fixed. OTOH, the > comparison with __FILE__ is not appropriate. > Why's it not appropriate? If you ever want to write tests to be runnable outside the build, e.g. with autopkgtest, then you're going to have to not use __FILE__ anyway. (Assuming you install the tests somewhere, rather than running the whole build again.) I can understand that breaking something that used to work is annoying, but the point is that the previous use does not generalise well. I don't really want to get into holy wars about the definition of "validity", I just ask you to consider generality. For example if you try a program that works fine when compiled natively but doesn't work when cross-compiled, that is a failure to generalise and one can either say "not supported, stop breaking us" or one can try to generalise one's program. It's not a sledgehammer solution, it's tweakable and we can try to use its flexibility to un-break your tests. So, what do you think of this alternate solution, that I suggested: Myself: > If all the test files reside underneath the same directory, like tests/, you > could: > > 4. export BUILD_PATH_PREFIX_MAP="$BUILD_PATH_PREFIX_MAP:tests=$BASEDIR/tests". > > This should make the tests pass, whilst keeping our "$srcpkg-$version" > mapping for the non-test files. Explanation: the map is parsed right-to-left so the later map will be activated first for files underneath tests/ and this mapping will be applied, rather than the default mapping set by our patched dpkg. This can be done generically in a build helper or something, rather than per-package. If this isn't suitable to you, how else do you suppose we should try to make builds independent of (i.e. not embed) the build path? Suggestions are welcome. X -- GPG: ed25519/56034877E1F87C35 GPG: rsa4096/1318EFAC5FBBDBCE https://github.com/infinity0/pubkeys.git
Bug#876901: QFINDTESTDATA uses __FILE__
On lunes, 13 de noviembre de 2017 19:15:44 -03 Pino Toscano wrote: [snip] > What I see it is happening here is: you (= people working on > reproducible builds) see __FILE__, and the problems that arise from its > abuse; to overcome this issue, you use the sledgehammer solution, > basically changing what __FILE__ means, and thus breaking even valid > use cases. Sorry, but I do not see how this is useful. > > A better approach here is to work on removing the invalid & abusing > usages of __FILE__ from packages, just like it was done for __DATE__. I can't but agree with Pino here. Moreover I consider this a gcc patch: it breaks a well defined macro. Please reassign as appropiate (or I'll do it later on). -- Lisandro Damián Nicanor Pérez Meyer http://perezmeyer.com.ar/ http://perezmeyer.blogspot.com/ signature.asc Description: This is a digitally signed message part.
Bug#876901: QFINDTESTDATA uses __FILE__
control: severity -1 wishlist On Mon, Nov 13, 2017 at 07:15:44PM +0100, Pino Toscano wrote: > Exactly, this is the source of the problem. OTOH, the problem was > created by changing __FILE__: it has a well-defined meaning: > https://gcc.gnu.org/onlinedocs/cpp/Standard-Predefined-Macros.html > changing this behaviour is only going to create problems, because > people rely on such well-defined (and standard) behaviours. [...] > No, the solution is: > a) *not* break what __FILE__ means > b) remove the misuses of __FILE__ in packages (not the case of >QFINDTESTDATA) [...] > What I see it is happening here is: you (= people working on > reproducible builds) see __FILE__, and the problems that arise from its > abuse; to overcome this issue, you use the sledgehammer solution, > basically changing what __FILE__ means, and thus breaking even valid > use cases. Sorry, but I do not see how this is useful. > > A better approach here is to work on removing the invalid & abusing > usages of __FILE__ from packages, just like it was done for __DATE__. downgrading the severity based on this. -- cheers, Holger signature.asc Description: PGP signature
Bug#876901: QFINDTESTDATA uses __FILE__
In data lunedì 13 novembre 2017 17:35:00 CET, Ximin Luo ha scritto: > Adrian Bunk: > > Control: reassign -1 qtbase5-dev > > Control: reassign 876917 qtbase5-dev > > Control: reassign 876933 qtbase5-dev > > Control: forcemerge -1 876917 876933 > > Control: retitle -1 QFINDTESTDATA uses __FILE__ > > Control: severity -1 normal > > Control: affects -1 src:karchive src:ki18n src:kcodecs src:kparts > > thanks > > > > The problem is the following implementation in > > /usr/include/x86_64-linux-gnu/qt5/QtTest/qtestcase.h: > > # define QFINDTESTDATA(basepath)\ > > QTest::qFindTestData(basepath, __FILE__, __LINE__) > > > > With the patched gcc in the unstable reproducible builds setting > > __FILE__ to something other value, this does no longer find the > > test data. > > > > I do not really see any reason for blaming the users here for using > > a documented public Qt API for accesssing test data in the source > > directory: > > http://doc.qt.io/qt-5/qtest.html#QFINDTESTDATA > > > > I've added reproducible-bui...@lists.alioth.debian.org to Cc for giving > > input what a reproducible and portable implementation might be for Qt. > > > > Our patched dpkg tells GCC to map $PWD to "$srcpkg-$version" when > expanding __FILE__. That is the source of the problem, because this > path doesn't exist at test-time. Exactly, this is the source of the problem. OTOH, the problem was created by changing __FILE__: it has a well-defined meaning: https://gcc.gnu.org/onlinedocs/cpp/Standard-Predefined-Macros.html changing this behaviour is only going to create problems, because people rely on such well-defined (and standard) behaviours. QFINDTESTDATA is a macro used mostly (if not only) in unit tests, so the binaries built with it are usually not installed. QFINDTESTDATA uses __FILE__ as one of the methods to locate files in the source directory of the file being built. AFAICS, this usage is *valid*, so the reproducible changes done to __FILE__ are only a regression. > The problem stems from the fact that we assume __FILE__ represents a > build-time path and not a run-time path, so we rewrite it > indiscriminately with BUILD_PATH_PREFIX_MAP. Exactly -- but in the case of QFINDTESTDATA this is wanted. > This assumption is broken in the specific case where you have some > source code that uses __FILE__, that are specifically only meant to > be run at build-time, so that they are in fact run-time paths (that > are also build-time paths). The assumption is fine in all other cases. So this is fine for QFINDTESTDATA. > Therefore, the only solution to fix this problem, that also preserves > reproducible builds, is to make those tests *not use __FILE__*. Or > option (2), which makes the non-existent rewritten paths, into an > actually-existing build-time path. No, the solution is: a) *not* break what __FILE__ means b) remove the misuses of __FILE__ in packages (not the case of QFINDTESTDATA) > I am not "blaming the user", but pointing to the fact that __FILE__ > is being used in a surprising way here, which is not good for > reproducible builds. What I see it is happening here is: you (= people working on reproducible builds) see __FILE__, and the problems that arise from its abuse; to overcome this issue, you use the sledgehammer solution, basically changing what __FILE__ means, and thus breaking even valid use cases. Sorry, but I do not see how this is useful. A better approach here is to work on removing the invalid & abusing usages of __FILE__ from packages, just like it was done for __DATE__. > An analogy would be to write your program to execute something at > time "__DATE__ + 30 seconds". This is obviously ridiculous, but works > "by accident" if done inside a test case. This is clearly a misuse, and thus it must be fixed. OTOH, the comparison with __FILE__ is not appropriate. -- Pino Toscano signature.asc Description: This is a digitally signed message part.
Bug#876901: QFINDTESTDATA uses __FILE__
On Mon, Nov 13, 2017 at 05:35:00PM +, Ximin Luo wrote: > Our patched dpkg tells GCC to map $PWD to "$srcpkg-$version" when expanding > __FILE__. That is the source of the problem, because this path doesn't exist > at test-time. You have the following options: > > 1. Unset BUILD_PATH_PREFIX_MAP. This is not great because things that use > __FILE__ will become unreproducible. actually I tend to think this is the best option currently. Our modification of gcc currently only exists in a test/development environment and it seems totally unclear whether this implementation will be used or another. so basing other changes in unstable (which will become buster sooner or later) seems less than ideal to me. > 2. Symlink "$srcpkg-$version" -> ".", so that the files can be accessed under > the paths that __FILE__ was expanded to. > > 3. Set BUILD_PATH_PREFIX_MAP to map $PWD to . instead. You do this by doing > `export BUILD_PATH_PREFIX_MAP=$BUILD_PATH_PREFIX_MAP:.=$PWD`, then the tests > should work. This makes any other uses of __FILE__ in this package, > inconsistent with other Debian packages (built with our patched dpkg). > > (maybe there are other options) 4. just ignore the ftbfs in unstable in tests.r-b.o setup. -- cheers, Holger signature.asc Description: PGP signature
Bug#876901: QFINDTESTDATA uses __FILE__
Ximin Luo: > Ximin Luo: >> [..] >> >> (maybe there are other options) >> > > If all the test files reside underneath the same directory, like tests/, you > could: > > 4. export BUILD_PATH_PREFIX_MAP="$BUILD_PATH_PREFIX_MAP=tests=$BASEDIR/tests". > Argh, I mean of course: 4. export BUILD_PATH_PREFIX_MAP="$BUILD_PATH_PREFIX_MAP:tests=$BASEDIR/tests". ^ colon, not equals > This should make the tests pass, whilst keeping our "$srcpkg-$version" > mapping for the non-test files. > > X > -- GPG: ed25519/56034877E1F87C35 GPG: rsa4096/1318EFAC5FBBDBCE https://github.com/infinity0/pubkeys.git
Bug#876901: QFINDTESTDATA uses __FILE__
Ximin Luo: > [..] > > (maybe there are other options) > If all the test files reside underneath the same directory, like tests/, you could: 4. export BUILD_PATH_PREFIX_MAP="$BUILD_PATH_PREFIX_MAP=tests=$BASEDIR/tests". This should make the tests pass, whilst keeping our "$srcpkg-$version" mapping for the non-test files. X -- GPG: ed25519/56034877E1F87C35 GPG: rsa4096/1318EFAC5FBBDBCE https://github.com/infinity0/pubkeys.git
Bug#876901: QFINDTESTDATA uses __FILE__
Adrian Bunk: > Control: reassign -1 qtbase5-dev > Control: reassign 876917 qtbase5-dev > Control: reassign 876933 qtbase5-dev > Control: forcemerge -1 876917 876933 > Control: retitle -1 QFINDTESTDATA uses __FILE__ > Control: severity -1 normal > Control: affects -1 src:karchive src:ki18n src:kcodecs src:kparts > thanks > > The problem is the following implementation in > /usr/include/x86_64-linux-gnu/qt5/QtTest/qtestcase.h: > # define QFINDTESTDATA(basepath)\ > QTest::qFindTestData(basepath, __FILE__, __LINE__) > > With the patched gcc in the unstable reproducible builds setting > __FILE__ to something other value, this does no longer find the > test data. > > I do not really see any reason for blaming the users here for using > a documented public Qt API for accesssing test data in the source > directory: > http://doc.qt.io/qt-5/qtest.html#QFINDTESTDATA > > I've added reproducible-bui...@lists.alioth.debian.org to Cc for giving > input what a reproducible and portable implementation might be for Qt. > Our patched dpkg tells GCC to map $PWD to "$srcpkg-$version" when expanding __FILE__. That is the source of the problem, because this path doesn't exist at test-time. You have the following options: 1. Unset BUILD_PATH_PREFIX_MAP. This is not great because things that use __FILE__ will become unreproducible. 2. Symlink "$srcpkg-$version" -> ".", so that the files can be accessed under the paths that __FILE__ was expanded to. 3. Set BUILD_PATH_PREFIX_MAP to map $PWD to . instead. You do this by doing `export BUILD_PATH_PREFIX_MAP=$BUILD_PATH_PREFIX_MAP:.=$PWD`, then the tests should work. This makes any other uses of __FILE__ in this package, inconsistent with other Debian packages (built with our patched dpkg). (maybe there are other options) I chose "$srcpkg-$version" because it provides some extra information, and allows one to distinguish files in different packages. Currently, dpkg-buildflags(1) sets -fdebug-prefix-map= to ".", so propsal (3) would actually be consistent with that. However I do think "$srcpkg-$version" is better because it's more informative. More generally, we don't want __FILE__-using tests to dictate how we should remap build paths *in general* in Debian. The problem stems from the fact that we assume __FILE__ represents a build-time path and not a run-time path, so we rewrite it indiscriminately with BUILD_PATH_PREFIX_MAP. This assumption is broken in the specific case where you have some source code that uses __FILE__, that are specifically only meant to be run at build-time, so that they are in fact run-time paths (that are also build-time paths). The assumption is fine in all other cases. Therefore, the only solution to fix this problem, that also preserves reproducible builds, is to make those tests *not use __FILE__*. Or option (2), which makes the non-existent rewritten paths, into an actually-existing build-time path. I am not "blaming the user", but pointing to the fact that __FILE__ is being used in a surprising way here, which is not good for reproducible builds. An analogy would be to write your program to execute something at time "__DATE__ + 30 seconds". This is obviously ridiculous, but works "by accident" if done inside a test case. X -- GPG: ed25519/56034877E1F87C35 GPG: rsa4096/1318EFAC5FBBDBCE https://github.com/infinity0/pubkeys.git
Bug#876901: QFINDTESTDATA uses __FILE__
Control: reassign -1 qtbase5-dev Control: reassign 876917 qtbase5-dev Control: reassign 876933 qtbase5-dev Control: forcemerge -1 876917 876933 Control: retitle -1 QFINDTESTDATA uses __FILE__ Control: severity -1 normal Control: affects -1 src:karchive src:ki18n src:kcodecs src:kparts thanks The problem is the following implementation in /usr/include/x86_64-linux-gnu/qt5/QtTest/qtestcase.h: # define QFINDTESTDATA(basepath)\ QTest::qFindTestData(basepath, __FILE__, __LINE__) With the patched gcc in the unstable reproducible builds setting __FILE__ to something other value, this does no longer find the test data. I do not really see any reason for blaming the users here for using a documented public Qt API for accesssing test data in the source directory: http://doc.qt.io/qt-5/qtest.html#QFINDTESTDATA I've added reproducible-bui...@lists.alioth.debian.org to Cc for giving input what a reproducible and portable implementation might be for Qt. cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed