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-builds@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
Attachment:
signature.asc
Description: This is a digitally signed message part.