-
Notifications
You must be signed in to change notification settings - Fork 38.7k
guix: Clean up manifest #27811
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
guix: Clean up manifest #27811
Conversation
|
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. 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. |
|
Updated 8673970 -> 404a0e9 (pr27811.02 -> pr27811.03, diff):
|
|
Rebased 404a0e9 -> 77414d5 (pr27811.03 -> pr27811.04) on top of the just merged #27779. |
|
Guix build: |
|
My guix build matches @TheCharlatan |
|
I'll be able to provide my own Guix build hashes tomorrow only. |
|
My Guix build: |
| (name "osslsigncode") | ||
| (version "2.5") | ||
| (source (origin | ||
| (method url-fetch) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a good reason for using git-fetch beyond being more consistent? Seems like this is just less efficient, because we get and hash the entire repository instead of just an archive. Probably all the other cases should be using url-fetch instead, or at least with the exception of the sourceware repository?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably all the other cases should be using
url-fetchinstead, or at least with the exception of the sourceware repository?
It seems the opposite approach is preferable according to that discussion. Anyway, I'm happy to drop that commit if it is indeed controversial.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can it use a --depth 1 fetch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, specifying depth is not supported afaict. I'm ok with moving ahead with this as is. I don't think the reasoning provided in the linked posts really applies here, but would still be good follow best practices.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was reminded of this discussion after reading about the recent XZ backdoor (context here and here, I'm sure you've all seen it by now).
One portion of the backdoor is solely in the distributed tarballs.
Specifically, a small change in the upstream source of m4/build-to-host.m4, which:
injected an obfuscated script from the files committed [in the source tree] to be “executed at the end of configure”.
Many projects that use the autotools build system include autogenerated scripts in their signed release tarballs (so consumers can simply run ./configure). This makes it hard to compare the contents of these tarballs with their git sources (where I believe changes enjoy more scrutiny). For example, try diffing expat's tarball (which Bitcoin uses in depends) with its git repository checked out to the same version. It contains 100k+ lines of unauditable autogenerated code soup.
Critically, in the case of XZ, running autoreconf -fi would not have prevented the backdoor from being inserted.
The discussion on the linked guix-devel list did not specifically mention the supply chain attack angle, so I wanted to bring it up here. Going with git-fetch for the manifest definitely seems to have been the right choice. It may also be worth exploring whether depends should be able to fetch from git repositories as well or automatically clean up autogenerated files in the preprocess stage to somewhat mitigate this attack vector.
|
Concept ACK - have tested this as part of some other changes. |
Guix builds
|
This change improves the maintainability of the manifest:
(1) It allows to remove the module when the specified symbols are no
longer used.
(2) It prevents accidental use of other symbols, such as `bash`
instead of `bash-minimal`.
|
Rebased due to the conflict with #27813. |
|
ACK a51d7ab Guix builds: |
|
Guix builds: |
|
Guix Build: 6edd213a90a043d09da144b99332ab16a320774098d35de28464262b25f260b2 guix-build-a51d7abf1e13/output/aarch64-linux-gnu/SHA256SUMS.part
1ececa8121d51a5c358e25e1a4b529413faadef5721387005db2b928c05cad3d guix-build-a51d7abf1e13/output/aarch64-linux-gnu/bitcoin-a51d7abf1e13-aarch64-linux-gnu-debug.tar.gz
fbc1724995b2d9a43c0bccbb3c20e02d115d9aeaecce0792bde46d9fedc0dbe0 guix-build-a51d7abf1e13/output/aarch64-linux-gnu/bitcoin-a51d7abf1e13-aarch64-linux-gnu.tar.gz
8a9edb553d6e6db1d63e286bb972086f0a775f1b589eb81e108c4d382d0a33da guix-build-a51d7abf1e13/output/arm-linux-gnueabihf/SHA256SUMS.part
b261a02f701635bd17abf75af99b1b9f8a474d3e2e5554218d3ed3e06e35c5cc guix-build-a51d7abf1e13/output/arm-linux-gnueabihf/bitcoin-a51d7abf1e13-arm-linux-gnueabihf-debug.tar.gz
3594d2df18a28642e0e5a4459421179ce8b2826c62f3ad5e087e1fdc9911c9a4 guix-build-a51d7abf1e13/output/arm-linux-gnueabihf/bitcoin-a51d7abf1e13-arm-linux-gnueabihf.tar.gz
b6e9ac4d00fb837e314607cb2618c5ae9e85b209dd8dfa0a7a0be941a58bb8a4 guix-build-a51d7abf1e13/output/arm64-apple-darwin/SHA256SUMS.part
8281d236e2f33bb313441e888bfa0d84c8b0dfa636184c383b2958dc1e883f8c guix-build-a51d7abf1e13/output/arm64-apple-darwin/bitcoin-a51d7abf1e13-arm64-apple-darwin-unsigned.dmg
d2ac774f35f2459a970b9d5536367d90e9c75b62800beb666529e92c417b47d0 guix-build-a51d7abf1e13/output/arm64-apple-darwin/bitcoin-a51d7abf1e13-arm64-apple-darwin-unsigned.tar.gz
11cb4660ba02cae997656cf0bf37cb47882c892dc4134d7d2e26d8f3c7648476 guix-build-a51d7abf1e13/output/arm64-apple-darwin/bitcoin-a51d7abf1e13-arm64-apple-darwin.tar.gz
7679e0643d5868e1fc4feed71f3d4b8e9178a7fc560a153fb4205f0586901c75 guix-build-a51d7abf1e13/output/dist-archive/bitcoin-a51d7abf1e13.tar.gz
ba860e8c30da71409d6281779cd7a2ad98afa2f9e395887d5635ca5a06e02697 guix-build-a51d7abf1e13/output/powerpc64-linux-gnu/SHA256SUMS.part
dc4d10e2bf00d2403bd2f90763d60e51eba4f4f754b934bdbdce8ac57291d0c1 guix-build-a51d7abf1e13/output/powerpc64-linux-gnu/bitcoin-a51d7abf1e13-powerpc64-linux-gnu-debug.tar.gz
aaf849c0db6e047f36bc98bbe4a8b9f9f2abb8144844160bba79c303b7532be4 guix-build-a51d7abf1e13/output/powerpc64-linux-gnu/bitcoin-a51d7abf1e13-powerpc64-linux-gnu.tar.gz
ddab779b72f8201f79945f68b9066fbb3f93d705efd7361ee6c71ed612933e5b guix-build-a51d7abf1e13/output/powerpc64le-linux-gnu/SHA256SUMS.part
d8cbedb7da7f2b45e36da334421d380046fc1b2218417bed0966721d13cd6c7b guix-build-a51d7abf1e13/output/powerpc64le-linux-gnu/bitcoin-a51d7abf1e13-powerpc64le-linux-gnu-debug.tar.gz
29b50d5e4f548b72a3df9e8a4e7d48b5eb5995526a94783d453caa6528eb8796 guix-build-a51d7abf1e13/output/powerpc64le-linux-gnu/bitcoin-a51d7abf1e13-powerpc64le-linux-gnu.tar.gz
ffd40756fdc58ee408adde791b0746b6388f8beef37a25c394d6a2c43534ce8c guix-build-a51d7abf1e13/output/riscv64-linux-gnu/SHA256SUMS.part
86a61895adc86ee45f2e15ff5b1dfd86dba8a7cfefa0d35729c167a91d9735e4 guix-build-a51d7abf1e13/output/riscv64-linux-gnu/bitcoin-a51d7abf1e13-riscv64-linux-gnu-debug.tar.gz
a35b4619b435775bda3507af775a00f48cb54d02b2fdf227ae02e2b13b4b60b7 guix-build-a51d7abf1e13/output/riscv64-linux-gnu/bitcoin-a51d7abf1e13-riscv64-linux-gnu.tar.gz
05b7fedbb46b7199fdbd8726054acfc474574175b32fefd21f96169b070d03aa guix-build-a51d7abf1e13/output/x86_64-apple-darwin/SHA256SUMS.part
1a4215bd089041249f995de413a3627340f27dfa83477bc8a937954ce4bd0d29 guix-build-a51d7abf1e13/output/x86_64-apple-darwin/bitcoin-a51d7abf1e13-x86_64-apple-darwin-unsigned.dmg
b6862e2cfb40eeb214852f9f6b27e59a0fcc12d5fc1e215bb20f595fa74f856c guix-build-a51d7abf1e13/output/x86_64-apple-darwin/bitcoin-a51d7abf1e13-x86_64-apple-darwin-unsigned.tar.gz
87396d48917a62f9851cc044bd6f0535f1ba60ee532feebfb79882ba03248050 guix-build-a51d7abf1e13/output/x86_64-apple-darwin/bitcoin-a51d7abf1e13-x86_64-apple-darwin.tar.gz
9aa6ec4e7e28348cbb013582a278ee6888cba6aac8592299adddb15b5e50b9a7 guix-build-a51d7abf1e13/output/x86_64-linux-gnu/SHA256SUMS.part
d46f0c44863f838f079cf9f3d61877129074cda920a6b52f1e7bc24280ce52ba guix-build-a51d7abf1e13/output/x86_64-linux-gnu/bitcoin-a51d7abf1e13-x86_64-linux-gnu-debug.tar.gz
adb5d445e390db0f738f92e8ac4b55814c816ce374975d129e281e5097c45d79 guix-build-a51d7abf1e13/output/x86_64-linux-gnu/bitcoin-a51d7abf1e13-x86_64-linux-gnu.tar.gz
d2ef49496929675e5ce2fa97490af52a0e205a71f80216fbf82224697abfc936 guix-build-a51d7abf1e13/output/x86_64-w64-mingw32/SHA256SUMS.part
2b3a37788e4d05fbf31ec41c94c68a8069cb4a90e7a4473b5cca970198b899c7 guix-build-a51d7abf1e13/output/x86_64-w64-mingw32/bitcoin-a51d7abf1e13-win64-debug.zip
03b2f27204c0d1616cba0addbda2bd3e820e34dc8a04ddaf244b024b4568acea guix-build-a51d7abf1e13/output/x86_64-w64-mingw32/bitcoin-a51d7abf1e13-win64-setup-unsigned.exe
c8410e5ffcafd792c921c5f065c3bfc6340a7ed08df0d1423387edc264fa3b77 guix-build-a51d7abf1e13/output/x86_64-w64-mingw32/bitcoin-a51d7abf1e13-win64-unsigned.tar.gz
58fa2459e61b593cf8a545118406ca58e312722866cbbadab68a7d38883292de guix-build-a51d7abf1e13/output/x86_64-w64-mingw32/bitcoin-a51d7abf1e13-win64.zip |
a51d7ab guix: Specify symbols in modules explicitly (Hennadii Stepanov) 47d51fb guix: Drop unneeded modules (Hennadii Stepanov) 57fdedd guix: Unify fetch methods (Hennadii Stepanov) Pull request description: This PR cleans up the `contrib/guix/manifest.scm` in the following way: - Unneeded for a successful build modules have be dropped. - Some modules have been enhanced with `#:select` clauses, which improves maintainability (see the commit message for details). ACKs for top commit: TheCharlatan: ACK a51d7ab Tree-SHA512: 380a36d03ec303ff8700893cfaad75ca09d84a77fd08d6c6a1679ac96409014b36f0698eb071e09af25ad36f1bc62aec0eec1092146d879251c6a8cce586169b
|
I get the same hashes. |
Summary: > guix: Unify fetch methods > guix: Drop unneeded modules > guix: Specify symbols in modules explicitly > > This change improves the maintainability of the manifest: > (1) It allows to remove the module when the specified symbols are no longer used. > (2) It prevents accidental use of other symbols, such as `bash` instead of `bash-minimal`. This is a backport of [[bitcoin/bitcoin#27811 | core#27811]] Depends on D15344 Test Plan: `contrib/guix/guix-build` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D15345
, bitcoin#26470, bitcoin#27296, bitcoin#27179, bitcoin#27813, bitcoin#27811, bitcoin#28069, bitcoin#28294, bitcoin#28324, bitcoin#28328, bitcoin#29987 (guix backports: part 4) 1edd121 merge bitcoin#29987: build with glibc 2.31 (Kittywhiskers Van Gogh) 0949825 revert: add exception for 32-bit ARM builds which need glibc 2.28 (Kittywhiskers Van Gogh) 0ac2531 merge bitcoin#28328: update time-machine (Kittywhiskers Van Gogh) ef9c4bd merge bitcoin#28324: pre time-machine bump changes (Linux) (Kittywhiskers Van Gogh) b45a127 merge bitcoin#28294: pre time-machine bump changes (Windows) (Kittywhiskers Van Gogh) ed1f7fe merge bitcoin#28069: Remove librt usage from release binaries (Kittywhiskers Van Gogh) 5d51aa9 merge bitcoin#27811: Clean up manifest (Kittywhiskers Van Gogh) d439e46 merge bitcoin#27813: Update `python-lief` package to 0.13.2 (Kittywhiskers Van Gogh) 70e6283 merge bitcoin#27179: use osslsigncode 2.5 (Kittywhiskers Van Gogh) 3799509 merge bitcoin#27296: import/sync python-lief (0.12.3) package definition from upstream (Kittywhiskers Van Gogh) ac8bd5a refactor: move lief definitions to expected location (Kittywhiskers Van Gogh) eb0ae08 merge bitcoin#26470: Clean up `libexec/build.sh` (Kittywhiskers Van Gogh) d3d7a05 merge bitcoin#24031: don't compress macOS DMG (Kittywhiskers Van Gogh) 6a54603 merge bitcoin#27670: remove redundant glibc patches (Kittywhiskers Van Gogh) 0c988f0 merge bitcoin#21089: Add support for powerpc64{,le} (Kittywhiskers Van Gogh) 808d215 fix: set correct locale in guix ci container (UdjinM6) Pull request description: ## Motivation In preparation for migrating to C++20 and bumping our minimum required compiler to ensure that we have access to greater portions of the C++20 spec, this pull request first of two pull requests aiming to upgrade our Guix setup to use GCC 12 (and GCC 11 for the macOS toolchain). Upgrades to the Clang toolchain for macOS and moving to GCC 12 for the macOS toolchain will be addressed in a separate pull request as the latter requires [bitcoin#21778](bitcoin#21778), which is well outside the scope of this PR. ## Additional Notes * Dependency for #6383 * Newer versions of GCC will not behave as expected with `test-security-check.py` as the stack protector (listed as `Canary` in the test) now behaves as expected and no longer fails. This causes an error when running it as the test expects failure (see below). This isn't an issue upstream as they backported [bitcoin#29987](bitcoin#29987), which gets rid of those tests ([source](bitcoin@b5fc6d4#diff-52aa0cda44721f089e53b128cb1232a876006ef257b211655456b17dfb2ec712)). Therefore, we have backported that pull request as well. <details> <summary>Build failure:</summary> ``` ====================================================================== FAIL: test_ELF (__main__.TestSecurityChecks) ---------------------------------------------------------------------- Traceback (most recent call last): File "/distsrc-base/distsrc-22.0.0-beta.1-70-g7907fab39c17-x86_64-linux-gnu/./contrib/devtools/test-security-check.py", line 61, in test_ELF self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-zexecstack','-fno-stack-protector','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']), AssertionError: Tuples differ: (1, 'test1: failed PIE NX RELRO CONTROL_FLOW') != (1, 'test1: failed PIE NX RELRO Canary CONTROL_FLOW') First differing element 1: 'test1: failed PIE NX RELRO CONTROL_FLOW' 'test1: failed PIE NX RELRO Canary CONTROL_FLOW' - (1, 'test1: failed PIE NX RELRO CONTROL_FLOW') + (1, 'test1: failed PIE NX RELRO Canary CONTROL_FLOW') ? +++++++ ---------------------------------------------------------------------- Ran 1 test in 0.126s FAILED (failures=1) ``` </details> * The backport has the effect of bumping the target glibc version to 2.31, which as the release notes say, cuts off support for RHEL 8 and Ubuntu 18.04 LTS (`bionic`) (i.e. our Guix binaries won't run on these distros anymore, users running those distros will have to compile it themselves). This shouldn't be a problem as full support for RHEL 8 ended May 31, 2024 ([source](https://access.redhat.com/support/policy/updates/errata#Life_Cycle_Dates)) and standard support for `bionic` ended April 2023 ([source](https://ubuntu.com/about/release-cycle)). **Dash Core will still work on Ubuntu 20.04 LTS (`focal`) as it ships with glibc 2.31 ([source](https://packages.ubuntu.com/focal/glibc-doc))** ## Breaking Changes None expected ## Checklist: - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)** - [x] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 1edd121 Tree-SHA512: 194a35a72ede5fb5488cdc41eb15320df7a5fa1fb8815c7342dd15b3983efff417fb1a525fdf497a98ede91958d0a5ed18f3e5f0f27c5cbc5dc25eb58116dbd5
, bitcoin#24031, bitcoin#26470, bitcoin#27296, bitcoin#27179, bitcoin#27813, bitcoin#27811, bitcoin#28069, bitcoin#28294, bitcoin#28324, bitcoin#28328, bitcoin#29987 (guix backports: part 4)" This reverts commit f155ecf, reversing changes made to a8e2316.
This PR cleans up the
contrib/guix/manifest.scmin the following way:#:selectclauses, which improves maintainability (see the commit message for details).