build, qt: No longer need to set QT_RCC_TEST=1 for determinism #21655
Conversation
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
Since Qt 5.3.1 hash seeding is disabled for rcc. See commit 5283a6c87beac5a43f612786fefd6e43f2c70bf6.
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
Guix builds: |
Gitian builds:
|
jarolrod
reviewed
Apr 11, 2021
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
d81dbbdc6efeb1af17dd57a2d94104203904104989f4c4adda9275b43018eb05 guix-build-c799a19b4bb8/output/aarch64-linux-gnu/bitcoin-c799a19b4bb8-aarch64-linux-gnu-debug.tar.gz
058b48c0b0b3fd899eec3ea87404a9197735bae7c6506bd325a88490de98b52e guix-build-c799a19b4bb8/output/aarch64-linux-gnu/bitcoin-c799a19b4bb8-aarch64-linux-gnu.tar.gz
13157cf85a6b5825b1619a39c7d8652978a2ac1474ec8be53fd6951d9aac1051 guix-build-c799a19b4bb8/output/arm-linux-gnueabihf/bitcoin-c799a19b4bb8-arm-linux-gnueabihf-debug.tar.gz
02e5742eae245f7e3ec379f85f03ad79334de09ace54f3773e751ba74fefcd2f guix-build-c799a19b4bb8/output/arm-linux-gnueabihf/bitcoin-c799a19b4bb8-arm-linux-gnueabihf.tar.gz
448f4b5f7cc7f8446d7360ddfd1c79eea9430d0fb51f6d1a92f1b3841915386e guix-build-c799a19b4bb8/output/dist-archive/bitcoin-c799a19b4bb8.tar.gz
facddb30ba28a9a7f036d0e832f0e43909c8f959448220ac7662c8d4d4b9602a guix-build-c799a19b4bb8/output/powerpc64-linux-gnu/bitcoin-c799a19b4bb8-powerpc64-linux-gnu-debug.tar.gz
f955ab24e01fa27b461b768f394661a6a1fa146711470bdf112276ae63fac316 guix-build-c799a19b4bb8/output/powerpc64-linux-gnu/bitcoin-c799a19b4bb8-powerpc64-linux-gnu.tar.gz
ba6d03b0c981aa6095ebb6318d495a53af1a93c3e89532bcbfb3a5b9c5685187 guix-build-c799a19b4bb8/output/powerpc64le-linux-gnu/bitcoin-c799a19b4bb8-powerpc64le-linux-gnu-debug.tar.gz
6f449774d006bcb5fa5812e1f5c37fadcec1982db8d75862d00e1ca5a57a5d24 guix-build-c799a19b4bb8/output/powerpc64le-linux-gnu/bitcoin-c799a19b4bb8-powerpc64le-linux-gnu.tar.gz
afc87c84c942708f3ac4057e8fb5950e7cfee6db6bcf900603461da2401be528 guix-build-c799a19b4bb8/output/riscv64-linux-gnu/bitcoin-c799a19b4bb8-riscv64-linux-gnu-debug.tar.gz
24627a442c216767cc6ba14fb4974e0ef3587836e265ab40ca3b37d70f2b6696 guix-build-c799a19b4bb8/output/riscv64-linux-gnu/bitcoin-c799a19b4bb8-riscv64-linux-gnu.tar.gz
e852c0e1a8a17956262395b4cdf0dfed0e7dade6a10761986ecbb2c8906a1548 guix-build-c799a19b4bb8/output/x86_64-apple-darwin18/bitcoin-c799a19b4bb8-osx-unsigned.dmg
a537e1f70758aaace3b50858a97d61bc7ebc22a4440eec4b86dbb969223d2125 guix-build-c799a19b4bb8/output/x86_64-apple-darwin18/bitcoin-c799a19b4bb8-osx-unsigned.tar.gz
3f9cbac2c16e445e30f696c8040b6d9c460a5c0523be37a07e048c5702e3510f guix-build-c799a19b4bb8/output/x86_64-apple-darwin18/bitcoin-c799a19b4bb8-osx64.tar.gz
10c536dc0c08febff84f6b6de057703f5242686035a4d612c24028018cdcb77f guix-build-c799a19b4bb8/output/x86_64-linux-gnu/bitcoin-c799a19b4bb8-x86_64-linux-gnu-debug.tar.gz
9ef22b4f0c2ad9a2b7f16441c91d52af2b8c411331aefeb3eac6c0ea633459eb guix-build-c799a19b4bb8/output/x86_64-linux-gnu/bitcoin-c799a19b4bb8-x86_64-linux-gnu.tar.gz
7b71bd1a7ef66b1f8aee545f1747ecee54efffa5c90a48c5042d5ccea6c85ff3 guix-build-c799a19b4bb8/output/x86_64-w64-mingw32/bitcoin-c799a19b4bb8-win-unsigned.tar.gz
164a9a1c7161acc92fb73d7a6c63b3475b90aad0299c1535f5b91f872680a063 guix-build-c799a19b4bb8/output/x86_64-w64-mingw32/bitcoin-c799a19b4bb8-win64-debug.zip
96e1d5a65c90b38ff89629dcb2cea9f6b5edc365132477365b664792adfd6105 guix-build-c799a19b4bb8/output/x86_64-w64-mingw32/bitcoin-c799a19b4bb8-win64-setup-unsigned.exe
6b7040357d54de20f67a01ef32dc94db589d993659964f0a78ead2bd6f3defae guix-build-c799a19b4bb8/output/x86_64-w64-mingw32/bitcoin-c799a19b4bb8-win64.zip
|
Concept ACK - given the builds look deterministic. I think the PR description could do a slightly better job of explaining why this is no longer needed. Looking at the source for the 5.12 branch, |
The commits you mentioned are cleanups after the behavior change commit mentioned in the OP. |
Done. |
I realise. However that isn't immediately obvious to reviewers, especially not from the original PR description or by looking at the linked commit. When reviewing a change like this, the first thing someone might do is go and grep the upstream source for |
I'd suggest to start ab ovo :) When using of bitcoin/contrib/gitian-descriptors/gitian-win.yml Lines 39 to 46 in 65615a3 it was clearly stated the QHash is the root of non-determinism. And Qt's commit fixed it.
@fanquake If so, going to close this PR. |
Yes, stated 7 years ago, in a comment that has since been deleted from our own source code. I'd hardly use that as the source of truth for how Qts codebase or tooling works now, so would need to look upstream to verify comments like that anyways. My point here is that you can be more verbose with details, and provide as much information as possible when opening a PR. Not only does that potentially lower the bar for others to review, but no one is going to complain that your PR descriptions are too verbose, or explain changes too well.
No. Once I've done a build myself I'll review this. |
Agree. Does the current PR description require more info? |
fanquake
approved these changes
Apr 21, 2021
There was a problem hiding this comment.
ACK c799a19
Guix builds:
bash-5.1# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
d81dbbdc6efeb1af17dd57a2d94104203904104989f4c4adda9275b43018eb05 guix-build-c799a19b4bb8/output/aarch64-linux-gnu/bitcoin-c799a19b4bb8-aarch64-linux-gnu-debug.tar.gz
058b48c0b0b3fd899eec3ea87404a9197735bae7c6506bd325a88490de98b52e guix-build-c799a19b4bb8/output/aarch64-linux-gnu/bitcoin-c799a19b4bb8-aarch64-linux-gnu.tar.gz
13157cf85a6b5825b1619a39c7d8652978a2ac1474ec8be53fd6951d9aac1051 guix-build-c799a19b4bb8/output/arm-linux-gnueabihf/bitcoin-c799a19b4bb8-arm-linux-gnueabihf-debug.tar.gz
02e5742eae245f7e3ec379f85f03ad79334de09ace54f3773e751ba74fefcd2f guix-build-c799a19b4bb8/output/arm-linux-gnueabihf/bitcoin-c799a19b4bb8-arm-linux-gnueabihf.tar.gz
448f4b5f7cc7f8446d7360ddfd1c79eea9430d0fb51f6d1a92f1b3841915386e guix-build-c799a19b4bb8/output/dist-archive/bitcoin-c799a19b4bb8.tar.gz
facddb30ba28a9a7f036d0e832f0e43909c8f959448220ac7662c8d4d4b9602a guix-build-c799a19b4bb8/output/powerpc64-linux-gnu/bitcoin-c799a19b4bb8-powerpc64-linux-gnu-debug.tar.gz
f955ab24e01fa27b461b768f394661a6a1fa146711470bdf112276ae63fac316 guix-build-c799a19b4bb8/output/powerpc64-linux-gnu/bitcoin-c799a19b4bb8-powerpc64-linux-gnu.tar.gz
ba6d03b0c981aa6095ebb6318d495a53af1a93c3e89532bcbfb3a5b9c5685187 guix-build-c799a19b4bb8/output/powerpc64le-linux-gnu/bitcoin-c799a19b4bb8-powerpc64le-linux-gnu-debug.tar.gz
6f449774d006bcb5fa5812e1f5c37fadcec1982db8d75862d00e1ca5a57a5d24 guix-build-c799a19b4bb8/output/powerpc64le-linux-gnu/bitcoin-c799a19b4bb8-powerpc64le-linux-gnu.tar.gz
afc87c84c942708f3ac4057e8fb5950e7cfee6db6bcf900603461da2401be528 guix-build-c799a19b4bb8/output/riscv64-linux-gnu/bitcoin-c799a19b4bb8-riscv64-linux-gnu-debug.tar.gz
24627a442c216767cc6ba14fb4974e0ef3587836e265ab40ca3b37d70f2b6696 guix-build-c799a19b4bb8/output/riscv64-linux-gnu/bitcoin-c799a19b4bb8-riscv64-linux-gnu.tar.gz
e852c0e1a8a17956262395b4cdf0dfed0e7dade6a10761986ecbb2c8906a1548 guix-build-c799a19b4bb8/output/x86_64-apple-darwin18/bitcoin-c799a19b4bb8-osx-unsigned.dmg
a537e1f70758aaace3b50858a97d61bc7ebc22a4440eec4b86dbb969223d2125 guix-build-c799a19b4bb8/output/x86_64-apple-darwin18/bitcoin-c799a19b4bb8-osx-unsigned.tar.gz
3f9cbac2c16e445e30f696c8040b6d9c460a5c0523be37a07e048c5702e3510f guix-build-c799a19b4bb8/output/x86_64-apple-darwin18/bitcoin-c799a19b4bb8-osx64.tar.gz
10c536dc0c08febff84f6b6de057703f5242686035a4d612c24028018cdcb77f guix-build-c799a19b4bb8/output/x86_64-linux-gnu/bitcoin-c799a19b4bb8-x86_64-linux-gnu-debug.tar.gz
9ef22b4f0c2ad9a2b7f16441c91d52af2b8c411331aefeb3eac6c0ea633459eb guix-build-c799a19b4bb8/output/x86_64-linux-gnu/bitcoin-c799a19b4bb8-x86_64-linux-gnu.tar.gz
7b71bd1a7ef66b1f8aee545f1747ecee54efffa5c90a48c5042d5ccea6c85ff3 guix-build-c799a19b4bb8/output/x86_64-w64-mingw32/bitcoin-c799a19b4bb8-win-unsigned.tar.gz
164a9a1c7161acc92fb73d7a6c63b3475b90aad0299c1535f5b91f872680a063 guix-build-c799a19b4bb8/output/x86_64-w64-mingw32/bitcoin-c799a19b4bb8-win64-debug.zip
96e1d5a65c90b38ff89629dcb2cea9f6b5edc365132477365b664792adfd6105 guix-build-c799a19b4bb8/output/x86_64-w64-mingw32/bitcoin-c799a19b4bb8-win64-setup-unsigned.exe
6b7040357d54de20f67a01ef32dc94db589d993659964f0a78ead2bd6f3defae guix-build-c799a19b4bb8/output/x86_64-w64-mingw32/bitcoin-c799a19b4bb8-win64.zipGitian builds:
# macOS:
cbf6f6a922d117d0c367d675d66466360d4352a7d804d24e3424f44e73cc63bb bitcoin-c799a19b4bb8-osx-unsigned.dmg
03cd981e7acde7ac870659b252e01512b53e68802ccf36cc48e48cf673abad08 bitcoin-c799a19b4bb8-osx-unsigned.tar.gz
2ab4037086cd1713f772a805859bc6857923876222eabe6c15015b2bc076b78b bitcoin-c799a19b4bb8-osx64.tar.gz
448f4b5f7cc7f8446d7360ddfd1c79eea9430d0fb51f6d1a92f1b3841915386e src/bitcoin-c799a19b4bb8.tar.gz
2d7b007bfca635e10150e3cf5e4bbfe790301a5e1a4957842ef5ce45c821efa6 bitcoin-core-osx-22-res.yml
# Windows:
8b363c7e32d900a739f395fbf7caa3bb2fcb82bfdc129fb769c4373f5dd44430 bitcoin-c799a19b4bb8-win-unsigned.tar.gz
e50ba1de000e86cc9762e10143a75a3b10d3938214bfd35312935c0c4d55953b bitcoin-c799a19b4bb8-win64-debug.zip
32daff7a9de10e7125d6af3019bcce9a028a850be4c562067950bee28d01303e bitcoin-c799a19b4bb8-win64-setup-unsigned.exe
f16f5a51de0119f0dc73bf3c41b7dc68e5f847118c84fca248392186729fa2e6 bitcoin-c799a19b4bb8-win64.zip
448f4b5f7cc7f8446d7360ddfd1c79eea9430d0fb51f6d1a92f1b3841915386e src/bitcoin-c799a19b4bb8.tar.gz
fbe94494f64faa059c3f31224e3498d4ca931957b4e12c3fa144dd0856fcd204 bitcoin-core-win-22-res.yml
# Linux:
a76307460204682fc86b93e1971a4fd64b06725e1bee918b04a50be0efc0332c bitcoin-c799a19b4bb8-aarch64-linux-gnu-debug.tar.gz
ea1387d9a5aa29b1fd2a31fc189a015b34da3fd4668c5d3ae6fc3411c129abcc bitcoin-c799a19b4bb8-aarch64-linux-gnu.tar.gz
57470f5b72fd6e44a32197732abdefcecea660366a9e8f82d3dc4fba134747d2 bitcoin-c799a19b4bb8-arm-linux-gnueabihf-debug.tar.gz
61d501caa7225bcc093b1b6763344bb86a813d4cc7e35d9278d3452e1aa5ce7c bitcoin-c799a19b4bb8-arm-linux-gnueabihf.tar.gz
e84ec7a5e195bf5725d3da2628d57a9a70f6c66b61be84b5623b1bee6830917e bitcoin-c799a19b4bb8-powerpc64-linux-gnu-debug.tar.gz
fc0a8c9b60c8bf9e3f4aae7418ad03641bcd197d415e07cda1584629645e4f08 bitcoin-c799a19b4bb8-powerpc64-linux-gnu.tar.gz
351a20f8ce6f333b1d8cbf0cb43f99bf23baf43a51ba71c313bd8f65491b6cae bitcoin-c799a19b4bb8-powerpc64le-linux-gnu-debug.tar.gz
b5554f369c147b13aab84b6b051fc1509d5d6cd85066a2f814125a575762be11 bitcoin-c799a19b4bb8-powerpc64le-linux-gnu.tar.gz
54f545cac647d2ac6cc952cb7d8b262edacf279c9adbf14ebf3b68a52813d6dd bitcoin-c799a19b4bb8-riscv64-linux-gnu-debug.tar.gz
12fe8bf9a08ca8a7a4f27e3a6284acf43fa31827e319fe30bb2647463f29b1cf bitcoin-c799a19b4bb8-riscv64-linux-gnu.tar.gz
0bed687831a3be14d41b2c98147d5edbba8e4f6508da4b7426e242922d11d894 bitcoin-c799a19b4bb8-x86_64-linux-gnu-debug.tar.gz
01b82d6a4b69441aadddbb5dd97034519b9bea7b0f45e457c1dcee31db882c08 bitcoin-c799a19b4bb8-x86_64-linux-gnu.tar.gz
448f4b5f7cc7f8446d7360ddfd1c79eea9430d0fb51f6d1a92f1b3841915386e src/bitcoin-c799a19b4bb8.tar.gz
b693d6c47592a3321667dade997540d1c84c2be224afee5748301f601bc13318 bitcoin-core-linux-22-res.yml|
Posthumous ACK—glad to see this override gone, and Qt upstream taking build determinism seriously. |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Apr 21, 2021
…or determinism c799a19 build, qt: No longer need to set QT_RCC_TEST=1 for determinism (Hennadii Stepanov) Pull request description: The Qt Resource Compiler (rcc) output order relies on [`QHash`](https://doc.qt.io/qt-5/qhash.html): > This randomization of `QHash` is enabled by default. Even though programs should never depend on a particular `QHash` ordering, there may be situations where you temporarily need deterministic behavior, for example for debugging or regression testing. To disable the randomization, define the environment variable `QT_HASH_SEED` to have the value 0. Since bitcoin#3620 we use `QT_RCC_TEST=1` to achieve a deterministic output. Since Qt 5.3.1 hash seeding is disabled for all of the bootstrapped tools, including rcc. Therefore, `QT_RCC_TEST=1` is no longer needed. See commit [5283a6c87beac5a43f612786fefd6e43f2c70bf6](qt/qtbase@5283a6c). ACKs for top commit: fanquake: ACK c799a19 Tree-SHA512: 9d116ac1e8c605ee3e8ed7f618586f0de85d8b06bbbb70fe8c298939ce203d2a7e97264a9afac037179993ab54c5f69a65ebb9ab27ca7f45acb963011bd45743
kwvg
added a commit
to kwvg/dash
that referenced
this pull request
Feb 17, 2022
kwvg
added a commit
to kwvg/dash
that referenced
this pull request
Feb 18, 2022
kwvg
added a commit
to kwvg/dash
that referenced
this pull request
Apr 25, 2022
kwvg
added a commit
to kwvg/dash
that referenced
this pull request
Apr 25, 2022
kwvg
added a commit
to kwvg/dash
that referenced
this pull request
Apr 26, 2022
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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) output order relies on
QHash:Since #3620 we use
QT_RCC_TEST=1to achieve a deterministic output.Since Qt 5.3.1 hash seeding is disabled for all of the bootstrapped tools, including rcc. Therefore,
QT_RCC_TEST=1is no longer needed.See commit 5283a6c87beac5a43f612786fefd6e43f2c70bf6.