build, qt: Make Qt rcc output always deterministic#21654
Merged
fanquake merged 1 commit intobitcoin:masterfrom Jun 3, 2021
Merged
build, qt: Make Qt rcc output always deterministic#21654fanquake merged 1 commit intobitcoin:masterfrom
fanquake merged 1 commit intobitcoin:masterfrom
Conversation
Member
Author
Guix builds: |
Contributor
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
Member
Author
Gitian builds:
|
jarolrod
reviewed
Apr 11, 2021
Contributor
jarolrod
left a comment
jarolrod
left a comment
There was a problem hiding this comment.
Contributing GUIX hashes, mine match @hebasto
find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
5d0c5016f27b1e3122a38dbe32ece19543a7fb01c94a6da5146f017cea81efa2 guix-build-bbd54508e186/output/aarch64-linux-gnu/bitcoin-bbd54508e186-aarch64-linux-gnu-debug.tar.gz
1694738123d5bd4af625f38eaa17b7ba1df223aa4e3471f53a5d081d3a8cc7ed guix-build-bbd54508e186/output/aarch64-linux-gnu/bitcoin-bbd54508e186-aarch64-linux-gnu.tar.gz
d62e913e95a6843b6cb84d607dca6db832cb55a3e286a9a739dd1b17d58a8bb1 guix-build-bbd54508e186/output/arm-linux-gnueabihf/bitcoin-bbd54508e186-arm-linux-gnueabihf-debug.tar.gz
37af054550f202f307f6c031f88f3ef23b39bbba4a4b6cda850f0a04048ceb8e guix-build-bbd54508e186/output/arm-linux-gnueabihf/bitcoin-bbd54508e186-arm-linux-gnueabihf.tar.gz
9f7fec2708b006635646ca5f259abbd96a99cb5a2b2b6ee7fbc15de07499d2ee guix-build-bbd54508e186/output/dist-archive/bitcoin-bbd54508e186.tar.gz
de142857b2ea16b4a6b6c9cb67c169dc79020e07bf90fc4dd48c5eceba34cbce guix-build-bbd54508e186/output/powerpc64-linux-gnu/bitcoin-bbd54508e186-powerpc64-linux-gnu-debug.tar.gz
3f3e6e1dac5d35cd70310fd966edd257bce4d6e58a7c0f5481bf41bfe2a32d09 guix-build-bbd54508e186/output/powerpc64-linux-gnu/bitcoin-bbd54508e186-powerpc64-linux-gnu.tar.gz
c7111777890122068de13006c13048a7fad2c3761f367949639b30401d2573fe guix-build-bbd54508e186/output/powerpc64le-linux-gnu/bitcoin-bbd54508e186-powerpc64le-linux-gnu-debug.tar.gz
967bf59cc18478b50c6563d32b1f34bac4d22e9b21f3af580d0704cdf1479ae7 guix-build-bbd54508e186/output/powerpc64le-linux-gnu/bitcoin-bbd54508e186-powerpc64le-linux-gnu.tar.gz
4126161aadf7c2eef8070e2a8a9c6064f9d66fe220261db4cf2f6c668e0f1ec2 guix-build-bbd54508e186/output/riscv64-linux-gnu/bitcoin-bbd54508e186-riscv64-linux-gnu-debug.tar.gz
b9a17f4bb45c5d19ea2085da2703057bc4cbdf9297004712382f593aa6150201 guix-build-bbd54508e186/output/riscv64-linux-gnu/bitcoin-bbd54508e186-riscv64-linux-gnu.tar.gz
c589a046701b6eb27fef10ebb5f1c637f3419483cfffdc2cb0a30396777f03c5 guix-build-bbd54508e186/output/x86_64-apple-darwin18/bitcoin-bbd54508e186-osx-unsigned.dmg
dd1cf0e945fc47a59d760688aa11db877f5267a3bd5df45393edc3992b5a2912 guix-build-bbd54508e186/output/x86_64-apple-darwin18/bitcoin-bbd54508e186-osx-unsigned.tar.gz
a799066051cc52bcee54bd8e4f513a97c4fad3ecd237c20711b50e8b02384e95 guix-build-bbd54508e186/output/x86_64-apple-darwin18/bitcoin-bbd54508e186-osx64.tar.gz
446ce5a65a7671d61d5f7a81dd908413059531967ec6245c5847fc99cedcdca9 guix-build-bbd54508e186/output/x86_64-linux-gnu/bitcoin-bbd54508e186-x86_64-linux-gnu-debug.tar.gz
f7c8c35a4cbd0fbf8e4ed2cef34c31d817f17dfda5114ec9536d1aee4cc9f774 guix-build-bbd54508e186/output/x86_64-linux-gnu/bitcoin-bbd54508e186-x86_64-linux-gnu.tar.gz
35be25ad31ebf3dc90b1f513a269974b650ba242e4dd2ac5ea41de549988513e guix-build-bbd54508e186/output/x86_64-w64-mingw32/bitcoin-bbd54508e186-win-unsigned.tar.gz
47a0e86ed18af195c051e2e67f4f5b55d6ef8d0d6fc4240ef95483975414342b guix-build-bbd54508e186/output/x86_64-w64-mingw32/bitcoin-bbd54508e186-win64-debug.zip
ccf64b3eeac2725da079121da4df07830e94002af1e56dcc8a68c91ce94b2992 guix-build-bbd54508e186/output/x86_64-w64-mingw32/bitcoin-bbd54508e186-win64-setup-unsigned.exe
c752ae831b88c758989ae8c93ffadfc06572520105ebb364a33e1d3d8e19b206 guix-build-bbd54508e186/output/x86_64-w64-mingw32/bitcoin-bbd54508e186-win64.zip
Member
|
Does this still need setting in the qt build env if it's now being done in the makefile? If so you should probably note why that's the case: bitcoin/depends/packages/qt.mk Line 177 in e7af2f3 I assume we could also skip setting |
hebasto
changed the title
Member
Author
No. The determinism is preserved.
Thanks!. I checked the Qt source code, and implemented your suggestion. |
hebasto
changed the title
Member
Author
Guix builds:Gitian builds:
|
hebasto
changed the title
fanquake
reviewed
Apr 20, 2021
Member
Author
|
Updated 6c05fa9 -> 712d9b4 (pr21654.02 -> pr21654.03, diff): |
Member
Author
Guix builds:Gitian builds:
|
Member
Author
|
Rebased 712d9b4 -> f17d340 (pr21654.03 -> pr21654.04) due to the conflict with #21655. |
Member
Author
Guix builds:Gitian builds:
|
Contributor
|
🕵️ @achow101 @sipa @practicalswift have been requested to review this pull request as specified in the REVIEWERS file. |
The Qt Resource Compiler (rcc) has a command-line option `--format-version` which has the default value 2. The only difference from `--format-version 1` is adding a last modified timestamp to the output file. That, in turn, forces us to use `QT_RCC_SOURCE_DATE_OVERRIDE=1` to get deterministic builds. This change makes rcc output always deterministic by using `--format-version 1` option that makes usage of the `QT_RCC_SOURCE_DATE_OVERRIDE` needless. Also it improves interaction with ccache. Co-authored-by: fanquake <[email protected]>
Member
Author
|
Rebased f17d340 -> a58868d (pr21654.04 -> pr21654.05) due to the conflict with #21708. |
Member
Author
Guix builds:Gitian builds:
|
Member
|
@laanwj do you have any opinion on passing |
Member
Not really. If it improves determinism that's a good thing. I think the only thing to be careful of is that if a new version of Qt adds a new format that is better somehow and differs in more than just this comment, we need to remember to reevaluate this. |
Member
Author
Do you think the usage of |
Member
I don't have a strong opinion on it. I think it's fine to change this. |
fanquake
approved these changes
Jun 3, 2021
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Jun 9, 2021
kwvg
added a commit
to kwvg/dash
that referenced
this pull request
Apr 26, 2022
gwillen
pushed a commit
to ElementsProject/elements
that referenced
this pull request
Jun 1, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Qt Resource Compiler (rcc) has a command-line option
--format-versionwhich has the default value 2.The only difference from
--format-version 1is adding a last modified timestamp to the output file (credits to fanquake). That, in turn, forces us to useQT_RCC_SOURCE_DATE_OVERRIDE=1to get deterministic builds (#13732).This change makes rcc output always deterministic by using
--format-version 1option that makes usage of theQT_RCC_SOURCE_DATE_OVERRIDEneedless.Also it improves interaction with ccache:
On master (f6c44e9):
The missed file is always
qt/libbitcoinqt_a-qrc_bitcoin_locale.o.With this PR: