build: Make dependency package archive timestamps deterministic #21995
Conversation
This file contains 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
fanquake
reviewed
May 20, 2021
|
Updated 0dbda79 -> 9046de8 (pr21995.01 -> pr21995.02, diff): |
|
Urgh BSD |
|
Updated 9046de8 -> acc7bf6 (pr21995.02 -> pr21995.03, diff): |
Guix builds: |
|
Code Review ACK acc7bf6 |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsNo conflicts as of last run. |
jarolrod
reviewed
Jul 17, 2021
There was a problem hiding this comment.
ACK acc7bf6
The example provided in the PR description is a good example of a use-case for this. A question is: What is the cost of maintaining this in comparison to how often it will be used?
Tested on Ubuntu 20.04. Checked that the PR does what it says it will do by comparing hashes of qt and with the example provided in the PR description:
This PR Branch
cat ./built/x86_64-w64-mingw32/qt/qt-5.12.10-006a988092a.tar.gz.hash
66b9de9a51be36fe6cce13b9a9c1c26c4789a6efad869d8f10ae74e35e3c4c22 qt-5.12.10-006a988092a.tar.gz
This PR Branch + Revert 21593
cat ./built/x86_64-w64-mingw32/qt/qt-5.12.10-976a9eea696.tar.gz.hash
66b9de9a51be36fe6cce13b9a9c1c26c4789a6efad869d8f10ae74e35e3c4c22 qt-5.12.10-976a9eea696.tar.gz
It should be noted that trying to reproduce from different directories seems to affect the hash:
PR
$ cat ./comparison-a/bitcoin/depends/built/x86_64-w64-mingw32/qt/qt-5.12.10-006a988092a.tar.gz.hash
41847652ec2e05a3fb990111bbb29ccb5a94f83d09032b65d54d213496d1e116 qt-5.12.10-006a988092a.tar.gz
PR + Revert in another directory
$ cat ./comparison-b/bitcoin/depends/built/x86_64-w64-mingw32/qt/qt-5.12.10-976a9eea696.tar.gz.hash
9d4e30d3838aead074caf2cdb1d0da5d1306bef5ed8758902fa65fb12241ecf0 qt-5.12.10-976a9eea696.tar.gz
fanquake
added a commit
that referenced
this pull request
Jul 18, 2021
1155978 build, qt: Do not install *.prl files (Hennadii Stepanov) 763793b build, qt: Fix wrong cross-compiling detection on macOS (Hennadii Stepanov) 3098272 build, qt: Force bootstrap while building linguist tools (Hennadii Stepanov) 689320e build, qt: Drop translations.pro hack (Hennadii Stepanov) 6a1f98f build, qt: Drop lrelease dependency patch (Hennadii Stepanov) 39e561e build, qt: Add linguist_tools list (Hennadii Stepanov) 27d3def build: Use Qt top-level build facilities (Hennadii Stepanov) Pull request description: This PR: - uses Qt top-level build facilities without the need to download all-in-one archive - is based on **BlockMechanic**'s [idea](#20600), and is an alternative to #20600 - makes it easy to integrate [new modules](#16883) into static builds - has the minimal diff - makes the qt package build process streamlined by dropping some patches and hacks (an alternative to #21420 and #20642) Fixes #18536 (a non-intrusive alternative to #21589 and #19785). Fixes #14648. Fixes #21588 (a non-intrusive alternative to #21591). Required for adding [Wayland support](#19950) on Linux. --- **Note for reviewers**: With 9046de8 from #21995 it is easy to verify that there are no changes in the resulted `qt` package archive on the per commit basis. For example, for `HOST=i686-pc-linux-gnu` no commit in this PR introduces any changes. ACKs for top commit: fanquake: ACK 1155978 Tree-SHA512: 667b06b72cb7ff26d68b9b88e8dddb51084783ca9e3d80b3392710794c1dc7fd77bbcc3ccf4ccece9919d33b9bf8fce13c5059502bd228021dc7c93fdb87ca7a
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Jul 23, 2021
1155978 build, qt: Do not install *.prl files (Hennadii Stepanov) 763793b build, qt: Fix wrong cross-compiling detection on macOS (Hennadii Stepanov) 3098272 build, qt: Force bootstrap while building linguist tools (Hennadii Stepanov) 689320e build, qt: Drop translations.pro hack (Hennadii Stepanov) 6a1f98f build, qt: Drop lrelease dependency patch (Hennadii Stepanov) 39e561e build, qt: Add linguist_tools list (Hennadii Stepanov) 27d3def build: Use Qt top-level build facilities (Hennadii Stepanov) Pull request description: This PR: - uses Qt top-level build facilities without the need to download all-in-one archive - is based on **BlockMechanic**'s [idea](bitcoin#20600), and is an alternative to bitcoin#20600 - makes it easy to integrate [new modules](bitcoin#16883) into static builds - has the minimal diff - makes the qt package build process streamlined by dropping some patches and hacks (an alternative to bitcoin#21420 and bitcoin#20642) Fixes bitcoin#18536 (a non-intrusive alternative to bitcoin#21589 and bitcoin#19785). Fixes bitcoin#14648. Fixes bitcoin#21588 (a non-intrusive alternative to bitcoin#21591). Required for adding [Wayland support](bitcoin#19950) on Linux. --- **Note for reviewers**: With 9046de8 from bitcoin#21995 it is easy to verify that there are no changes in the resulted `qt` package archive on the per commit basis. For example, for `HOST=i686-pc-linux-gnu` no commit in this PR introduces any changes. ACKs for top commit: fanquake: ACK 1155978 Tree-SHA512: 667b06b72cb7ff26d68b9b88e8dddb51084783ca9e3d80b3392710794c1dc7fd77bbcc3ccf4ccece9919d33b9bf8fce13c5059502bd228021dc7c93fdb87ca7a
|
GUIX hashes, mine do not match @hebasto, i have collisions on all platforms except macOS: |
|
Rebased acc7bf6 -> a88dbc7 (pr21995.03 -> pr21995.04) on top of the recent Guix code (see ee88320 etc). |
Guix builds: |
|
GUIX hashes, mine match @hebasto: |
hebasto
force-pushed
the
210519-repro
branch
from
a88dbc7
to
061b6ef
Compare
|
Rebased a88dbc7 -> 061b6ef (pr21995.04 -> pr21995.05) due to the conflict with #22283. |
Guix builds: |
|
Also this approach won't work, at least for the Nevertheless, I believe the benefit of this PR is still valuable. |
laanwj
added a commit
that referenced
this pull request
Apr 13, 2022
e644591 build, refactor: Drop useless `call` Make function (Hennadii Stepanov) Pull request description: Using the [`call`](https://www.gnu.org/software/make/manual/html_node/Call-Function.html) function with `$(package)_*_cmds` is effectively noop because the latter, which could be found in `<package>.mk` files, do not use temporary `$(1)` variable at all. This PR removes useless calls of the `call` function, and makes code more readable and easier to reason about. No change in resulted dependency binaries could be easy verified with bitcoin/bitcoin/#21995. ACKs for top commit: laanwj: Code review ACK e644591 shaavan: Code review ACK e644591 Tree-SHA512: 8481fa0dc5bbf7dd6a180f7fae5a2ccc07f85b50c7a966bceb2d7e010e07e5f211ee3f74f8ac79bc5acfde5f0764264d599d959ff3ebb8511b1b4a33f79509bd
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Apr 14, 2022
e644591 build, refactor: Drop useless `call` Make function (Hennadii Stepanov) Pull request description: Using the [`call`](https://www.gnu.org/software/make/manual/html_node/Call-Function.html) function with `$(package)_*_cmds` is effectively noop because the latter, which could be found in `<package>.mk` files, do not use temporary `$(1)` variable at all. This PR removes useless calls of the `call` function, and makes code more readable and easier to reason about. No change in resulted dependency binaries could be easy verified with bitcoin/bitcoin/bitcoin#21995. ACKs for top commit: laanwj: Code review ACK e644591 shaavan: Code review ACK e644591 Tree-SHA512: 8481fa0dc5bbf7dd6a180f7fae5a2ccc07f85b50c7a966bceb2d7e010e07e5f211ee3f74f8ac79bc5acfde5f0764264d599d959ff3ebb8511b1b4a33f79509bd
|
guix hashes x86: |
fanquake
reviewed
Dec 2, 2022
There was a problem hiding this comment.
This is broken when using BusyBox tar (i.e building on Alpine):
Caching libevent...
cd /bitcoin/depends/work/staging/aarch64-unknown-linux-musl/libevent/2.1.12-stable-714086d0d4d//bitcoin/depends/aarch64-unknown-linux-musl; find . ! -name '.stamp_postprocessed' -print0 | TZ=UTC xargs -0r touch -h -m -t 200001011200; find . ! -name '.stamp_postprocessed' -print0 | LC_ALL=C sort -z | tar --create --numeric-owner --no-recursion --null -T - | gzip -9n > /bitcoin/depends/work/staging/aarch64-unknown-linux-musl/libevent/2.1.12-stable-714086d0d4d/libevent-2.1.12-stable-714086d0d4d.tar.gz
tar: unrecognized option: null
BusyBox v1.35.0 (2022-11-19 10:13:10 UTC) multi-call binary.
Usage: tar c|x|t [-ZzJjahmvokO] [-f TARFILE] [-C DIR] [-T FILE] [-X FILE] [LONGOPT]... [FILE]...
Create, extract, or list files from a tar file
c Create
x Extract
t List
-f FILE Name of TARFILE ('-' for stdin/out)
-C DIR Change to DIR before operation
-v Verbose
-O Extract to stdout
-m Don't restore mtime
-o Don't restore user:group
-k Don't replace existing files
-Z (De)compress using compress
-z (De)compress using gzip
-J (De)compress using xz
-j (De)compress using bzip2
--lzma (De)compress using lzma
-a (De)compress based on extension
-h Follow symlinks
-T FILE File with names to include
-X FILE File with glob patterns to exclude
--exclude PATTERN Glob pattern to exclude
--overwrite Replace existing files
--strip-components NUM NUM of leading components to strip
--no-recursion Don't descend in directories
--numeric-owner Use numeric user:group
--no-same-permissions Don't restore access permissions
Calling |
|
The PR description has been updated with a contemporary example. |
TheCharlatan
approved these changes
Feb 6, 2023
There was a problem hiding this comment.
Code review ACK 6ebe576
I can confirm the findings of others that this indeed removes major factors of nondeterminism. I varied some of the introduced commands and parameters and can confirm that each plays a part in eliminating the nondeterminism.
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Feb 7, 2023
…s deterministic 6ebe576 build: Make dependency package archive timestamps deterministic (Hennadii Stepanov) Pull request description: This PR makes testing changes like bitcoin#20641, bitcoin#21593, bitcoin#22142, bitcoin#24279, bitcoin#24285 as easy as comparing hashes. With this PR: ``` $ make -C depends clean $ make -C depends HOST=x86_64-w64-mingw32 $ find depends/built/x86_64-w64-mingw32 -name '*.hash' | sort | xargs cat 1f685a61cbf205f81977ecf88cba91fa1ccdfbe77ab4ec3405dcd33ceb778af4 bdb-4.8.30-ca950bd6d13.tar.gz 08a9acde276e6e5e5c8913e3ad07eeecda184a996882ae226b3ed056c7ec1b01 boost-1.80.0-b537c466dcb.tar.gz 144c6d92e4108fcc90740bee27007db58a88336a97be6367f9c8ba4cc208af27 libevent-2.1.12-stable-e13b2bdd8b8.tar.gz e3c9c9609bf32bfd460432c6ab99a64e9f8750ed775a193925ff4f5aed363e4c libnatpmp-07004b97cf691774efebe70404cf22201e4d330d-82255b84667.tar.gz 62c6a089a4b24a413eccd2f389bf4c8b0716423b0ace5e87e984069635da9f83 miniupnpc-2.2.2-c43fc4cf2f6.tar.gz 78762700066273e597698a78479a506b33532ea565d18ef561614b9fc3820cf5 qrencode-3.4.4-663de0dc628.tar.gz 5e2183faf91838510a48e6dbb4b65ae74a7d48ba1abc070b82767c4076582360 qt-5.15.5-986926343e2.tar.gz 9f8459f8d27fc3af9146712be6ba6577f15741429936504a950cc51c17da1ba8 sqlite-3380500-bec6a4d3299.tar.gz 0eca5d01d427de50be4bd57c8bb100ab69b017792c32b8733e2b20443f4c9c28 zeromq-4.3.4-8ae81bab6f4.tar.gz ``` As an example, here is an evidence that bitcoin#24279 is a strict refactoring change: ``` $ git fetch origin pull/24279/head $ git cherry-pick 7060268 $ git cherry-pick 3f90dde $ make -C depends clean $ make -C depends HOST=x86_64-w64-mingw32 $ find depends/built/x86_64-w64-mingw32 -name '*.hash' | sort | xargs cat 1f685a61cbf205f81977ecf88cba91fa1ccdfbe77ab4ec3405dcd33ceb778af4 bdb-4.8.30-c7faf31d5ca.tar.gz 08a9acde276e6e5e5c8913e3ad07eeecda184a996882ae226b3ed056c7ec1b01 boost-1.80.0-1af3dd1d99e.tar.gz 144c6d92e4108fcc90740bee27007db58a88336a97be6367f9c8ba4cc208af27 libevent-2.1.12-stable-6228a9f8534.tar.gz e3c9c9609bf32bfd460432c6ab99a64e9f8750ed775a193925ff4f5aed363e4c libnatpmp-07004b97cf691774efebe70404cf22201e4d330d-41aa6194ecc.tar.gz 62c6a089a4b24a413eccd2f389bf4c8b0716423b0ace5e87e984069635da9f83 miniupnpc-2.2.2-6a93027769c.tar.gz 78762700066273e597698a78479a506b33532ea565d18ef561614b9fc3820cf5 qrencode-3.4.4-d40cb2d45c9.tar.gz 5e2183faf91838510a48e6dbb4b65ae74a7d48ba1abc070b82767c4076582360 qt-5.15.5-120c3cb745d.tar.gz 9f8459f8d27fc3af9146712be6ba6577f15741429936504a950cc51c17da1ba8 sqlite-3380500-bbd4d813c69.tar.gz 0eca5d01d427de50be4bd57c8bb100ab69b017792c32b8733e2b20443f4c9c28 zeromq-4.3.4-df0858a19d2.tar.gz ``` ACKs for top commit: TheCharlatan: Code review ACK 6ebe576 Tree-SHA512: 20e0222781f5dcb50126c11677d0671bcdd7be144b2e528c75a02983acc494206552fb35039697ccd094de27a21b3fb439e9965c34feb8a6d74627fa20a9a5e7
This was referenced Mar 30, 2023
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jan 2, 2024
e644591 build, refactor: Drop useless `call` Make function (Hennadii Stepanov) Pull request description: Using the [`call`](https://www.gnu.org/software/make/manual/html_node/Call-Function.html) function with `$(package)_*_cmds` is effectively noop because the latter, which could be found in `<package>.mk` files, do not use temporary `$(1)` variable at all. This PR removes useless calls of the `call` function, and makes code more readable and easier to reason about. No change in resulted dependency binaries could be easy verified with bitcoin/bitcoin/bitcoin#21995. ACKs for top commit: laanwj: Code review ACK e644591 shaavan: Code review ACK e644591 Tree-SHA512: 8481fa0dc5bbf7dd6a180f7fae5a2ccc07f85b50c7a966bceb2d7e010e07e5f211ee3f74f8ac79bc5acfde5f0764264d599d959ff3ebb8511b1b4a33f79509bd
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jan 2, 2024
e644591 build, refactor: Drop useless `call` Make function (Hennadii Stepanov) Pull request description: Using the [`call`](https://www.gnu.org/software/make/manual/html_node/Call-Function.html) function with `$(package)_*_cmds` is effectively noop because the latter, which could be found in `<package>.mk` files, do not use temporary `$(1)` variable at all. This PR removes useless calls of the `call` function, and makes code more readable and easier to reason about. No change in resulted dependency binaries could be easy verified with bitcoin/bitcoin/bitcoin#21995. ACKs for top commit: laanwj: Code review ACK e644591 shaavan: Code review ACK e644591 Tree-SHA512: 8481fa0dc5bbf7dd6a180f7fae5a2ccc07f85b50c7a966bceb2d7e010e07e5f211ee3f74f8ac79bc5acfde5f0764264d599d959ff3ebb8511b1b4a33f79509bd
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jan 2, 2024
e644591 build, refactor: Drop useless `call` Make function (Hennadii Stepanov) Pull request description: Using the [`call`](https://www.gnu.org/software/make/manual/html_node/Call-Function.html) function with `$(package)_*_cmds` is effectively noop because the latter, which could be found in `<package>.mk` files, do not use temporary `$(1)` variable at all. This PR removes useless calls of the `call` function, and makes code more readable and easier to reason about. No change in resulted dependency binaries could be easy verified with bitcoin/bitcoin/bitcoin#21995. ACKs for top commit: laanwj: Code review ACK e644591 shaavan: Code review ACK e644591 Tree-SHA512: 8481fa0dc5bbf7dd6a180f7fae5a2ccc07f85b50c7a966bceb2d7e010e07e5f211ee3f74f8ac79bc5acfde5f0764264d599d959ff3ebb8511b1b4a33f79509bd
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jan 2, 2024
e644591 build, refactor: Drop useless `call` Make function (Hennadii Stepanov) Pull request description: Using the [`call`](https://www.gnu.org/software/make/manual/html_node/Call-Function.html) function with `$(package)_*_cmds` is effectively noop because the latter, which could be found in `<package>.mk` files, do not use temporary `$(1)` variable at all. This PR removes useless calls of the `call` function, and makes code more readable and easier to reason about. No change in resulted dependency binaries could be easy verified with bitcoin/bitcoin/bitcoin#21995. ACKs for top commit: laanwj: Code review ACK e644591 shaavan: Code review ACK e644591 Tree-SHA512: 8481fa0dc5bbf7dd6a180f7fae5a2ccc07f85b50c7a966bceb2d7e010e07e5f211ee3f74f8ac79bc5acfde5f0764264d599d959ff3ebb8511b1b4a33f79509bd
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.
This PR makes testing changes like #20641, #21593, #22142, #24279, #24285 as easy as comparing hashes.
With this PR:
As an example, here is an evidence that #24279 is a strict refactoring change: