Message ID | qHq-f-ApWJCp4wLr7bTekukXH3hkLWqooUuEQeJIcXKG1vDbOCsBUYpeedzxf1qeM1CWiGKy6pfeME24YXbJDqD3RA6zUt3fv324jK-n09g=@carldong.me |
---|---|
State | Accepted |
Headers | show |
Series | [bug#34165] gnu: bitcoin-core: Make bitcoin-qt deterministic. | expand |
Context | Check | Description |
---|---|---|
cbaines/applying patch | success | Successfully applied |
Thanks! Pushed this as commit 99629e5a110a9a91151bd0e63a1805446996a3c8 to guix master.
Hi! Carl Dong <contact@carldong.me> skribis: >>From 2b3162dde22a5d44eb5910b0fcfa07318f935aaf Mon Sep 17 00:00:00 2001 > From: Carl Dong <accounts@carldong.me> > Date: Mon, 21 Jan 2019 14:51:57 -0500 > Subject: [PATCH] gnu: bitcoin-core: Make bitcoin-qt deterministic. [...] > + (add-before 'configure 'qt-time > + (lambda _ > + (setenv "QT_RCC_SOURCE_DATE_OVERRIDE" "1"))) ; Make QT deterministic Looking at this and the upstream commit¹, I’m thinking maybe we should do the same in other Qt applications. Any idea how we can identify applications where it’s needed? Thanks, Ludo’. ¹ https://github.com/qt/qtbase/commit/38271e9298dcf48652a6e2e08414a940a97867fa
Ludovic Courtès <ludo@gnu.org> writes: > Hi! > > Carl Dong <contact@carldong.me> skribis: > >>>From 2b3162dde22a5d44eb5910b0fcfa07318f935aaf Mon Sep 17 00:00:00 2001 >> From: Carl Dong <accounts@carldong.me> >> Date: Mon, 21 Jan 2019 14:51:57 -0500 >> Subject: [PATCH] gnu: bitcoin-core: Make bitcoin-qt deterministic. > > [...] > >> + (add-before 'configure 'qt-time >> + (lambda _ >> + (setenv "QT_RCC_SOURCE_DATE_OVERRIDE" "1"))) ; Make QT deterministic > > Looking at this and the upstream commit¹, I’m thinking maybe we should > do the same in other Qt applications. Any idea how we can identify > applications where it’s needed? > > ¹ > https://github.com/qt/qtbase/commit/38271e9298dcf48652a6e2e08414a940a97867fa This is a bit unfortunate. It seems to me that the original intent of the patch was to support SOURCE_DATE_EPOCH as the commit message mentions that variable’s specification. Pity that it is now a custom Qt variable. This probably should be used for all packages that generate resource files, which I assume are most non-trivial applications. I think it’s fine to add it to all packages using qtbase. -- Ricardo
On Wed, Jan 23, 2019 at 03:41:43PM +0100, Ricardo Wurmus wrote: > > > This is a bit unfortunate. It seems to me that the original intent of > the patch was to support SOURCE_DATE_EPOCH as the commit message > mentions that variable’s specification. Pity that it is now a custom Qt > variable. > We could just add it in to the gnu-build-system and set the variable in all events. Or patch qt/qtbase to accept SOURCE_DATE_EPOCH in place of their own variable.
Efraim Flashner <efraim@flashner.co.il> skribis: > On Wed, Jan 23, 2019 at 03:41:43PM +0100, Ricardo Wurmus wrote: >> >> >> This is a bit unfortunate. It seems to me that the original intent of >> the patch was to support SOURCE_DATE_EPOCH as the commit message >> mentions that variable’s specification. Pity that it is now a custom Qt >> variable. >> > > We could just add it in to the gnu-build-system and set the variable in > all events. Or patch qt/qtbase to accept SOURCE_DATE_EPOCH in place of > their own variable. Either way is fine with me. Adding it to the ‘set-SOURCE-DATE-EPOCH’ phase in ‘core-updates’ would be reasonable IMO. Ludo’.
Hi, > > https://github.com/qt/qtbase/commit/38271e9298dcf48652a6e2e08414a940a97867fa > > This is a bit unfortunate. It seems to me that the original intent of > the patch was to support SOURCE_DATE_EPOCH as the commit message > mentions that variable’s specification. Pity that it is now a custom Qt > variable. I asked them and they said [1]: >There was a policy to prefix variables with QT_ and I did not want to discuss more with the reviewer. >But later, this was merged: >https://codereview.qt-project.org/#/c/243636/4/src/tools/rcc/rcc.cpp,unified [1] https://github.com/qt/qtbase/commit/38271e9298dcf48652a6e2e08414a940a97867fa#commitcomment-32085478 https://codereview.qt-project.org/#/c/243636/4/src/tools/rcc/rcc.cpp,unified contains the following patch: ... 228 228 const QDateTime lastModified = m_fileInfo.lastModified(); 229 229 quint64 lastmod = quint64(lastModified.isValid() ? lastModified.toMSecsSinceEpoch() : 0); 230 230 static const quint64 sourceDate = 1000 * qgetenv("QT_RCC_SOURCE_DATE_OVERRIDE").toULongLong( ); 231 231 if (sourceDate != 0) 232 232 lastmod = sourceDate; 233 + static const quint64 sourceDate2 = 1000 * qgetenv("SOURCE_DATE_EPOCH").toULongLong(); 234 + if (sourceDate2 != 0) 235 + lastmod = sourceDate2;
Hi Danny, Danny Milosavljevic <dannym@scratchpost.org> skribis: > [1] https://github.com/qt/qtbase/commit/38271e9298dcf48652a6e2e08414a940a97867fa#commitcomment-32085478 > > https://codereview.qt-project.org/#/c/243636/4/src/tools/rcc/rcc.cpp,unified contains the following patch: > > ... > 228 228 const QDateTime lastModified = m_fileInfo.lastModified(); > 229 229 quint64 lastmod = quint64(lastModified.isValid() ? lastModified.toMSecsSinceEpoch() : 0); > 230 230 static const quint64 sourceDate = 1000 * qgetenv("QT_RCC_SOURCE_DATE_OVERRIDE").toULongLong( > ); > 231 231 if (sourceDate != 0) > 232 232 lastmod = sourceDate; > 233 + static const quint64 sourceDate2 = 1000 * qgetenv("SOURCE_DATE_EPOCH").toULongLong(); > 234 + if (sourceDate2 != 0) > 235 + lastmod = sourceDate2; Nice, so maybe we don’t need to change anything and just wait for the next Qt version? Ludo’.
diff --git a/gnu/packages/finance.scm b/gnu/packages/finance.scm index 468388797..96df919c4 100644 --- a/gnu/packages/finance.scm +++ b/gnu/packages/finance.scm @@ -115,6 +115,9 @@ "/bin/lupdate")) #:phases (modify-phases %standard-phases + (add-before 'configure 'qt-time + (lambda _ + (setenv "QT_RCC_SOURCE_DATE_OVERRIDE" "1"))) ; Make QT deterministic (add-before 'check 'set-home (lambda _ (setenv "HOME" (getenv "TMPDIR"))))))) ; Tests write to $HOME.
From 2b3162dde22a5d44eb5910b0fcfa07318f935aaf Mon Sep 17 00:00:00 2001 From: Carl Dong <accounts@carldong.me> Date: Mon, 21 Jan 2019 14:51:57 -0500 Subject: [PATCH] gnu: bitcoin-core: Make bitcoin-qt deterministic. * gnu/packages/finance.scm: Make bitcoin-qt deterministic. --- gnu/packages/finance.scm | 3 +++ 1 file changed, 3 insertions(+) -- 2.20.1