Bug#1059631: qhelpgenerator-qt5: nearly-reproducible LastRegisterTime value in .qch files is not timezone-normalized
Followup-For: Bug #1059631 X-Debbugs-Cc: mity...@debian.org On Wed, 10 Jan 2024 00:01:40 +0300, Dmitry wrote: > On Tue, Jan 09, 2024 at 06:40:35PM +, James Addison wrote: > [ ... snip ... ] > > My sense is that with the patch here and also the patch from #1059592 > > applied, > > we would see at least eight qt-related packages in Debian building with more > > reliable reproducibility. Not a huge number, but it's becoming rarer to > > find > > fixups like this that benefit multiple dependent packages (a good thing). > I was going to include these patches together with Qt 5.15.12 transition, > which I am currently preparing. > > But if you want it in unstable sooner, I can do a new upload for 5.15.10 and > then merge into my 5.15.12 branch. Just let me know if you need that. Thanks Dmitry - there's no need for that, and no hurry. Also: the reproducible builds test coverage does include experimental (where, now that I check, I see the 5.15.12 changes staged) so that'll provide relevant build results when ready.
Bug#1059631: qhelpgenerator-qt5: nearly-reproducible LastRegisterTime value in .qch files is not timezone-normalized
Hi James! On Tue, Jan 09, 2024 at 06:40:35PM +, James Addison wrote: > Followup-For: Bug #1059631 > X-Debbugs-Cc: mity...@debian.org > > Hi Dmitry - could you recommend whether there's anything I should do next for > this bug? > > As context: the patch was accepted upstream, but with modifications that make > it cleaner for Qt6.6 albeit in a non-5.15.x compatible way. I realize that > might not be ideal maintainence-wise; sorry about that. I'm a bit unclear on > the licensing status of 5.15.x and that's making me uncertain about whether to > offer another patch for that lineage upstream. > > My sense is that with the patch here and also the patch from #1059592 applied, > we would see at least eight qt-related packages in Debian building with more > reliable reproducibility. Not a huge number, but it's becoming rarer to find > fixups like this that benefit multiple dependent packages (a good thing). I was going to include these patches together with Qt 5.15.12 transition, which I am currently preparing. But if you want it in unstable sooner, I can do a new upload for 5.15.10 and then merge into my 5.15.12 branch. Just let me know if you need that. -- Dmitry Shachnev signature.asc Description: PGP signature
Bug#1059631: qhelpgenerator-qt5: nearly-reproducible LastRegisterTime value in .qch files is not timezone-normalized
Followup-For: Bug #1059631 X-Debbugs-Cc: mity...@debian.org Hi Dmitry - could you recommend whether there's anything I should do next for this bug? As context: the patch was accepted upstream, but with modifications that make it cleaner for Qt6.6 albeit in a non-5.15.x compatible way. I realize that might not be ideal maintainence-wise; sorry about that. I'm a bit unclear on the licensing status of 5.15.x and that's making me uncertain about whether to offer another patch for that lineage upstream. My sense is that with the patch here and also the patch from #1059592 applied, we would see at least eight qt-related packages in Debian building with more reliable reproducibility. Not a huge number, but it's becoming rarer to find fixups like this that benefit multiple dependent packages (a good thing). Thanks, James
Bug#1059631: qhelpgenerator-qt5: nearly-reproducible LastRegisterTime value in .qch files is not timezone-normalized
Followup-For: Bug #1059631 Control: tags -1 fixed-upstream > Control: forwarded -1 https://codereview.qt-project.org/c/qt/qttools/+/527972 This fix has been merged upstream; I've also offered what I think is a further cleanup[1], but it does not affect the behaviour of the code (only readability and performance properties). Please note: based on code review feedback, I updated the patch to use some features of Qt that are not available in Qt5, notably a QTimeZone::UTC enum value[2]. So it seems that the approach taken to develop a fix for Debian and/or qt5 would necessarily diverge. [1] - https://codereview.qt-project.org/c/qt/qttools/+/527983 [2] - https://doc.qt.io/qt-6.5/qt.html#TimeSpec-enum
Bug#1059631: qhelpgenerator-qt5: nearly-reproducible LastRegisterTime value in .qch files is not timezone-normalized
On Sat, Dec 30, 2023 at 10:02:00PM +, James Addison wrote: > Package: qhelpgenerator-qt5 > Followup-For: Bug #1059631 > X-Debbugs-Cc: mity...@debian.org > Control: forwarded -1 https://codereview.qt-project.org/c/qt/qttools/+/527972 > > Hi Dmitry, > > On Sat, 30 Dec 2023 22:50:47, Dmitry wrote: > > Thank you for the patch! > > > > Any chance you can forward it to upstream Qt? See [1] for the details. > > Yep, certainly - done. Thanks! Thank you! +1 from me and added a couple of reviewers. -- Dmitry Shachnev signature.asc Description: PGP signature
Bug#1059631: qhelpgenerator-qt5: nearly-reproducible LastRegisterTime value in .qch files is not timezone-normalized
Package: qhelpgenerator-qt5 Followup-For: Bug #1059631 X-Debbugs-Cc: mity...@debian.org Control: forwarded -1 https://codereview.qt-project.org/c/qt/qttools/+/527972 Hi Dmitry, On Sat, 30 Dec 2023 22:50:47, Dmitry wrote: > Thank you for the patch! > > Any chance you can forward it to upstream Qt? See [1] for the details. Yep, certainly - done. Thanks!
Bug#1059631: qhelpgenerator-qt5: nearly-reproducible LastRegisterTime value in .qch files is not timezone-normalized
Hi James! On Sat, Dec 30, 2023 at 05:12:52PM +, James Addison wrote: > Package: qhelpgenerator-qt5 > Followup-For: Bug #1059631 > X-Debbugs-Cc: reproducible-b...@lists.alioth.debian.org > > My apologies: I had indeed misdiagnosed the problem here. > > The code that inserts into the SettingsTable, where the problem reported here > manifests, is unrelated to the patch from bug #875847 - there is a separate > check for SOURCE_CODE_EPOCH in the src/assistant/qhelpgenerator/main.cpp file. > > To test a fix, I used the following commands to replicate the problem: > > $ SOURCE_DATE_EPOCH=1503951538 qhelpgenerator > examples/assistant/simpletextviewer/documentation/simpletextviewer.qhcp -o > foo.qch > $ TZ=GMT+8 SOURCE_DATE_EPOCH=1503951538 qhelpgenerator > examples/assistant/simpletextviewer/documentation/simpletextviewer.qhcp -o > bar.qch > > Please find attached a patch to address the problem. Note that I decided to > patch both SOURCE_CODE_EPOCH locations for consistency, despite the fact that > only the main.cpp code site was confirmed affected. Thank you for the patch! Any chance you can forward it to upstream Qt? See [1] for the details. I could forward it myself, but upstream requires signing the CLA so they will not always accept a patch which is forwarded not by its author. [1]: https://wiki.qt.io/Gerrit_Introduction -- Dmitry Shachnev signature.asc Description: PGP signature
Bug#1059631: qhelpgenerator-qt5: nearly-reproducible LastRegisterTime value in .qch files is not timezone-normalized
Package: qhelpgenerator-qt5
Followup-For: Bug #1059631
X-Debbugs-Cc: reproducible-b...@lists.alioth.debian.org
My apologies: I had indeed misdiagnosed the problem here.
The code that inserts into the SettingsTable, where the problem reported here
manifests, is unrelated to the patch from bug #875847 - there is a separate
check for SOURCE_CODE_EPOCH in the src/assistant/qhelpgenerator/main.cpp file.
To test a fix, I used the following commands to replicate the problem:
$ SOURCE_DATE_EPOCH=1503951538 qhelpgenerator
examples/assistant/simpletextviewer/documentation/simpletextviewer.qhcp -o
foo.qch
$ TZ=GMT+8 SOURCE_DATE_EPOCH=1503951538 qhelpgenerator
examples/assistant/simpletextviewer/documentation/simpletextviewer.qhcp -o
bar.qch
Please find attached a patch to address the problem. Note that I decided to
patch both SOURCE_CODE_EPOCH locations for consistency, despite the fact that
only the main.cpp code site was confirmed affected.
Description: helpgenerator: clear UTC offset to zero when reading
SOURCE_DATE_EPOCH value
Author: James Addison
Bug-Debian: https://bugs.debian.org/1059631
---
qttools-opensource-src-5.15.10.orig/src/assistant/help/qhelpcollectionhandler.cpp
+++ qttools-opensource-src-5.15.10/src/assistant/help/qhelpcollectionhandler.cpp
@@ -2202,8 +2202,10 @@ bool QHelpCollectionHandler::registerInd
const QString sourceDateEpochStr =
qEnvironmentVariable("SOURCE_DATE_EPOCH");
bool ok;
const qlonglong sourceDateEpoch = sourceDateEpochStr.toLongLong();
-if (ok && sourceDateEpoch < lastModified.toSecsSinceEpoch())
+if (ok && sourceDateEpoch < lastModified.toSecsSinceEpoch()) {
+lastModified.setOffsetFromUtc(0);
lastModified.setSecsSinceEpoch(sourceDateEpoch);
+}
}
m_query->addBindValue(lastModified.toString(Qt::ISODate));
if (!m_query->exec())
--- qttools-opensource-src-5.15.10.orig/src/assistant/qhelpgenerator/main.cpp
+++ qttools-opensource-src-5.15.10/src/assistant/qhelpgenerator/main.cpp
@@ -116,6 +116,7 @@ int generateCollectionFile(const QByteAr
if (!config.filesToRegister().isEmpty()) {
if (Q_UNLIKELY(qEnvironmentVariableIsSet("SOURCE_DATE_EPOCH"))) {
QDateTime dt;
+dt.setOffsetFromUtc(0);
dt.setSecsSinceEpoch(qEnvironmentVariableIntValue("SOURCE_DATE_EPOCH"));
CollectionConfiguration::updateLastRegisterTime(helpEngine, dt);
} else {
Bug#1059631: qhelpgenerator-qt5: nearly-reproducible LastRegisterTime value in .qch files is not timezone-normalized
Package: qhelpgenerator-qt5 Followup-For: Bug #1059631 X-Debbugs-Cc: reproducible-b...@lists.alioth.debian.org On Fri, 29 Dec 2023 15:30:58, I wrote: > Inspecting the patch from #875847 and the values that appear in the diffoscope > output from the build logs: the SOURCE_DATE_EPOCH value of the build is used, > as expected, to improve the reproducibility of the build. It takes the value > of the most recent Debian changelog entry. > > However: the patch mutates an existing QT QDateTime instance (last_modified) > to > store the seconds-since-epoch value -- without specifying a timezone for the > value. ... > My sense is that the LastRegisterTime column value is probably intended to be > stored in UTC; it may be sufficient to set the timezone of the last_modified > instance to UTC -- making careful to ensure that it is indeed a _set_ timezone > operation and not a _translate_ timezone operation. In hindsight: I think I've misunderstood some of the details here. If the root cause is indeed a non-UTC timezone on the last_modified object, then maybe it does in fact make sense to translate that object's value into UTC -- because that would mean that we the date-string we bind is always UTC-based, regardless of whether SOURCE_DATE_EPOCH is set. Related, though: I also don't yet understand why the date string that appears in the INSERT statement does not include a timezone offset (+1200, for example). My reading of the QT documentation[1] is that a timezone offset will be included in the formatted string for non-UTC date+time objects.. I'll do some more investigation. [1] - https://doc.qt.io/qt-5/qt.html#DateFormat-enum
Bug#1059631: qhelpgenerator-qt5: nearly-reproducible LastRegisterTime value in .qch files is not timezone-normalized
Package: qhelpgenerator-qt5
Version: 5.15.2-3
Severity: wishlist
User: reproducible-bui...@lists.alioth.debian.org
Usertags: timezone
X-Debbugs-Cc: reproducible-b...@lists.alioth.debian.org
Dear Maintainer,
Looking at some recent Reproducible Build[1] test results[2] for the Debian
openorienteering-mapper package, the LastRegisterTime value placed into the
SettingsTable table in the .qch (sqlite3 db) file format can vary based on
the build host's configuration.
Inspecting the patch from #875847 and the values that appear in the diffoscope
output from the build logs: the SOURCE_DATE_EPOCH value of the build is used,
as expected, to improve the reproducibility of the build. It takes the value
of the most recent Debian changelog entry.
However: the patch mutates an existing QT QDateTime instance (last_modified) to
store the seconds-since-epoch value -- without specifying a timezone for the
value.
I'm not 100% certain, but I think it's likely (given that the duration
between the two timestamps that appear in the diffoscope output is 26 hours,
equal to the local-time-difference between GMT-14 and GMT+12, the two build
timezones) that the last_modified object remains timezone-relative in each
build, and therefore emits differing LastRegisterTime values in ISO format.
-INSERT INTO SettingsTable
VALUES('LastRegisterTime','2021-12-27T21:45:41.000');
+INSERT INTO SettingsTable
VALUES('LastRegisterTime','2021-12-28T23:45:41.000');
My sense is that the LastRegisterTime column value is probably intended to be
stored in UTC; it may be sufficient to set the timezone of the last_modified
instance to UTC -- making careful to ensure that it is indeed a _set_ timezone
operation and not a _translate_ timezone operation.
Regards,
James
[1] - https://reproducible-builds.org/
[2] -
https://tests.reproducible-builds.org/debian/rb-pkg/bookworm/arm64/diffoscope-results/openorienteering-mapper.html
[3] - https://doc.qt.io/qt-5/qdatetime.html
