guix: Clean up manifest #27811
Merged
guix: Clean up manifest #27811
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
|
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. |
This was referenced Jun 3, 2023
TheCharlatan
reviewed
Jun 6, 2023
Sjors
reviewed
Jun 6, 2023
|
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: |
TheCharlatan
reviewed
Jun 7, 2023
| @@ -247,12 +246,13 @@ and abstract ELF, PE and MachO formats.") | |||
| (name "osslsigncode") | |||
| (version "2.5") | |||
| (source (origin | |||
| (method url-fetch) | |||
There was a problem hiding this comment.
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.
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.
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.
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 |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Jun 30, 2023
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. |
Fabcien
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Feb 1, 2024
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 cleans up the
contrib/guix/manifest.scmin the following way:#:selectclauses, which improves maintainability (see the commit message for details).