-
Notifications
You must be signed in to change notification settings - Fork 38.7k
guix: Make source tarball using git-archive #18741
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
Conversation
|
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. |
|
Concept ACK. |
Guix builds
|
Gitian builds
|
|
You'll also need to adjust this line in guix for the correct naming: |
74e8b36 to
39f12a0
Compare
2aa48ed refactor: Drop unused ${WRAP_DIR}/${HOST} directory (Hennadii Stepanov) 1362be0 build: Drop make dist in gitian builds (Hennadii Stepanov) Pull request description: After the merge of #18331, the packaged source tarball is created by `git archive`, but the binaries are built from another one which is made by `make dist`. With this PR the only source tarball, created by `git archive`, is used both for binaries building and for packaging to users. Close #16588. Close #18547. As a good side-effect, #18349 becomes redundant. **Change in behavior** The following variables https://github.com/bitcoin/bitcoin/blob/1b151e3ffce7c1a2ee46bf280cc1d96775d1f91e/configure.ac#L2-L6 are no longer used for naming of directories and tarballs. Instead of them the gitian descriptors use a git tag (if available) or a commit hash. --- Also a small refactor commit picked from #18404. ACKs for top commit: dongcarl: ACK 2aa48ed MarcoFalke: ACK 2aa48ed fanquake: ACK 2aa48ed - I've had a quick look over this, and don't want to block merging if this actually gets as closer to finally having this all sorted out. Obviously we've still got #18741, and after speaking to Carl this morning, there will likely be even more changes after that (not Guix specific). Tree-SHA512: d3b16f87e48d1790a3264940c28acd5d881bfd10f3ce94fb0c8a6af76d8039289d01e0cd4972adac49ae24362857251f6c1e5e09e3e9fbf636c10708b4015a7c
|
Needs rebase |
Previously, the sourced script would create the source tarball. Now, it only assigns variables and the source-ing script has more flexibility in determining what to do with these variables. See later commit showing how this flexibility is useful in our Guix builds.
When using worktrees or submodules, you'll see a `.git' plain text file at the root of your working tree instead of the usual `.git' directory. This plain text file will point to the real GIT_DIR, under the GIT_COMMON_DIR. From experimentation, the full GIT_COMMON_DIR is required to exist for operations such as git-archive(1), so we expose it as readonly inside the container.
Previously, we would specify the makensis output file path twice: 1. At the top of Makefile.am as BITCOIN_WIN_INSTALLER, and 2. In share/setup.nsi.ini This commit uses the -X flag of makensis to eliminate the need for the second instance mentioned above, referring makensis directly to the value of BITCOIN_WIN_INSTALLER
39f12a0 to
eab02e3
Compare
2aa48ed refactor: Drop unused ${WRAP_DIR}/${HOST} directory (Hennadii Stepanov) 1362be0 build: Drop make dist in gitian builds (Hennadii Stepanov) Pull request description: After the merge of bitcoin#18331, the packaged source tarball is created by `git archive`, but the binaries are built from another one which is made by `make dist`. With this PR the only source tarball, created by `git archive`, is used both for binaries building and for packaging to users. Close bitcoin#16588. Close bitcoin#18547. As a good side-effect, bitcoin#18349 becomes redundant. **Change in behavior** The following variables https://github.com/bitcoin/bitcoin/blob/1b151e3ffce7c1a2ee46bf280cc1d96775d1f91e/configure.ac#L2-L6 are no longer used for naming of directories and tarballs. Instead of them the gitian descriptors use a git tag (if available) or a commit hash. --- Also a small refactor commit picked from bitcoin#18404. ACKs for top commit: dongcarl: ACK 2aa48ed MarcoFalke: ACK 2aa48ed fanquake: ACK 2aa48ed - I've had a quick look over this, and don't want to block merging if this actually gets as closer to finally having this all sorted out. Obviously we've still got bitcoin#18741, and after speaking to Carl this morning, there will likely be even more changes after that (not Guix specific). Tree-SHA512: d3b16f87e48d1790a3264940c28acd5d881bfd10f3ce94fb0c8a6af76d8039289d01e0cd4972adac49ae24362857251f6c1e5e09e3e9fbf636c10708b4015a7c
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.
| VERSION="$(git rev-parse --short HEAD)" | |
| VERSION="$(git rev-parse --short=10 HEAD)" |
Not sure what the git logic is, but I couldn't verify from the doc that this is deterministic when the git version changes (and/or the number of objects in the repo)
|
Also in the guix readme. Could you remove the requirement that guix needs two cores. I am running this on half a core and everything is fine. |
|
Also ran a Guix build of the second to last commit here: find output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
25aeed36ffc1eba8aff7e12f62011575e2270f07f8c25d14ffb3d0691996d919 output/bitcoin-27e63e01c-aarch64-linux-gnu-debug.tar.gz
7423aad2b0306a828b1dbe7eeb51c22c301298d9a6ae409c53546433374c8e29 output/bitcoin-27e63e01c-aarch64-linux-gnu.tar.gz
f8f027993c373698fb5f1b868769f903e246594eba69fdfe2861508828fecf0c output/bitcoin-27e63e01c-arm-linux-gnueabihf-debug.tar.gz
6e4c079f55024348237cd55f394db1d09a1b91b8f520e64b025984d6bb7bddb0 output/bitcoin-27e63e01c-arm-linux-gnueabihf.tar.gz
7159dc6c2acabcf089b37a8c618e8558f68a44b24a733861c2b88c865c72b3f5 output/bitcoin-27e63e01c-riscv64-linux-gnu-debug.tar.gz
a85afdeff32b003f11a60831b6654a4fb1fb02b899f10392006f33fbaf2b3e4a output/bitcoin-27e63e01c-riscv64-linux-gnu.tar.gz
273958496232e58fc4204b28947e8b10ebd0244f7016d0098b391574639aceca output/bitcoin-27e63e01c-win-unsigned.tar.gz
49844ea00fcaebb141cc5b0e6a0a135df045dec1bb79256ef0f65e2a11d8ad1d output/bitcoin-27e63e01c-win64-debug.zip
2c7c70a33492f17a8f35c6c7348a10ad795e85855a929196d499956b048d09a2 output/bitcoin-27e63e01c-win64-setup-unsigned.exe
8985a7d8d0b8511c2e36e050c0f006346ef66c2fb436936d083690068368efbc output/bitcoin-27e63e01c-win64.zip
3f98116028552453e059e3761b9231bd1b98ba96f9322798c4f7f586779593f9 output/bitcoin-27e63e01c-x86_64-linux-gnu-debug.tar.gz
82ba38c6bb96ce0716646205a4f9054c0eeff32fe223d654f5234204afa599f4 output/bitcoin-27e63e01c-x86_64-linux-gnu.tar.gz
a8daae18536b433d05ae7972355fc99d535f2687f2e078d1a4a0893cc2c6bf5f output/src/bitcoin-27e63e01c.tar.gz |
|
This should be good to go, let me know if there are other things to do. |
|
Performed builds of bfe1ba2 locally: Gitian Linux: Gitian Win: All available Guix architectures: |
|
@dongcarl , @fanquake This conflicts with
which is tagged 0.20.0. Just saying, to be sure we don't accidentally close doors behind us. |
|
@MarcoFalke We can do this one of 2 ways:
Personally, I'd prefer the second option, as this PR is quite well-tested now, and fixes the broken Linux Guix builds (which we now ship with release tarballs since the move to |
|
Sorry, I don't follow. Should this pull be marked for backport then? |
| ;; | ||
| *linux*) | ||
| cp "${DISTSRC}/doc/README.md" "${DISTNAME}/" | ||
| cp "${DISTSRC}/README.md" "${DISTNAME}/" |
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.
why is this changed?
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.
Oh, I see c4a3c25
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.
Neither README make sense. The top level one talks about testing and quality assurance, which is meta for the tarball. The doc README has some dead links, but at least gives some more context. 🤷
bfe1ba2 rel-builds: Specify core.abbrev for git-rev-parse (Carl Dong) 27e63e0 build: Accomodate makensis v2.x (Carl Dong) 1f2c39a guix: Remove logical cores requirement (Carl Dong) a4f6ffa lint: Also enable source statements for non-gitian (Carl Dong) d256f91 rel-builds: Directly deploy win installer to OUTDIR (Carl Dong) fa791da nsis: Specify OutFile path only once (Carl Dong) 1470160 guix: Expose GIT_COMMON_DIR in container as readonly (Carl Dong) f5a6ac4 guix: Make source tarball using git-archive (Carl Dong) 395c113 gitian: Limit sourced script to just assignments (Carl Dong) Pull request description: Based on: bitcoin#18556 Related: bitcoin#17595 (comment) ACKs for top commit: fanquake: ACK bfe1ba2 - I agree with Carl, and am going to merge this. I'd like for Linux Guix builds to be working again, and we can rebase bitcoin#18818. Tree-SHA512: c87ada7e3de17ca0b692a91029b86573442ded5780fc081c214773f6b374a0cdbeaf6f6898c36669c2e247ee32aa7f82defb1180f8decac52c65f0c140f18674
Previously, the sourced script would create the source tarball. Now, it only assigns variables and the source-ing script has more flexibility in determining what to do with these variables. See later commit showing how this flexibility is useful in our Guix builds. Github-Pull: bitcoin#18741 Rebased-From: 395c113
Chose 12 because the kernel uses it: https://public-inbox.org/git/CA+55aFy0_pwtFOYS1Tmnxipw9ZkRNCQHmoYyegO00pjMiZQfbg@mail.gmail.com/raw And also because it's a nice number. Github-Pull: bitcoin#18741 Rebased-From: bfe1ba2
2aa48ed refactor: Drop unused ${WRAP_DIR}/${HOST} directory (Hennadii Stepanov) 1362be0 build: Drop make dist in gitian builds (Hennadii Stepanov) Pull request description: After the merge of bitcoin#18331, the packaged source tarball is created by `git archive`, but the binaries are built from another one which is made by `make dist`. With this PR the only source tarball, created by `git archive`, is used both for binaries building and for packaging to users. Close bitcoin#16588. Close bitcoin#18547. As a good side-effect, bitcoin#18349 becomes redundant. **Change in behavior** The following variables https://github.com/bitcoin/bitcoin/blob/1b151e3ffce7c1a2ee46bf280cc1d96775d1f91e/configure.ac#L2-L6 are no longer used for naming of directories and tarballs. Instead of them the gitian descriptors use a git tag (if available) or a commit hash. --- Also a small refactor commit picked from bitcoin#18404. ACKs for top commit: dongcarl: ACK 2aa48ed MarcoFalke: ACK 2aa48ed fanquake: ACK 2aa48ed - I've had a quick look over this, and don't want to block merging if this actually gets as closer to finally having this all sorted out. Obviously we've still got bitcoin#18741, and after speaking to Carl this morning, there will likely be even more changes after that (not Guix specific). Tree-SHA512: d3b16f87e48d1790a3264940c28acd5d881bfd10f3ce94fb0c8a6af76d8039289d01e0cd4972adac49ae24362857251f6c1e5e09e3e9fbf636c10708b4015a7c
bfe1ba2 rel-builds: Specify core.abbrev for git-rev-parse (Carl Dong) 27e63e0 build: Accomodate makensis v2.x (Carl Dong) 1f2c39a guix: Remove logical cores requirement (Carl Dong) a4f6ffa lint: Also enable source statements for non-gitian (Carl Dong) d256f91 rel-builds: Directly deploy win installer to OUTDIR (Carl Dong) fa791da nsis: Specify OutFile path only once (Carl Dong) 1470160 guix: Expose GIT_COMMON_DIR in container as readonly (Carl Dong) f5a6ac4 guix: Make source tarball using git-archive (Carl Dong) 395c113 gitian: Limit sourced script to just assignments (Carl Dong) Pull request description: Based on: bitcoin#18556 Related: bitcoin#17595 (comment) ACKs for top commit: fanquake: ACK bfe1ba2 - I agree with Carl, and am going to merge this. I'd like for Linux Guix builds to be working again, and we can rebase bitcoin#18818. Tree-SHA512: c87ada7e3de17ca0b692a91029b86573442ded5780fc081c214773f6b374a0cdbeaf6f6898c36669c2e247ee32aa7f82defb1180f8decac52c65f0c140f18674
2aa48ed refactor: Drop unused ${WRAP_DIR}/${HOST} directory (Hennadii Stepanov) 1362be0 build: Drop make dist in gitian builds (Hennadii Stepanov) Pull request description: After the merge of bitcoin#18331, the packaged source tarball is created by `git archive`, but the binaries are built from another one which is made by `make dist`. With this PR the only source tarball, created by `git archive`, is used both for binaries building and for packaging to users. Close bitcoin#16588. Close bitcoin#18547. As a good side-effect, bitcoin#18349 becomes redundant. **Change in behavior** The following variables https://github.com/bitcoin/bitcoin/blob/1b151e3ffce7c1a2ee46bf280cc1d96775d1f91e/configure.ac#L2-L6 are no longer used for naming of directories and tarballs. Instead of them the gitian descriptors use a git tag (if available) or a commit hash. --- Also a small refactor commit picked from bitcoin#18404. ACKs for top commit: dongcarl: ACK 2aa48ed MarcoFalke: ACK 2aa48ed fanquake: ACK 2aa48ed - I've had a quick look over this, and don't want to block merging if this actually gets as closer to finally having this all sorted out. Obviously we've still got bitcoin#18741, and after speaking to Carl this morning, there will likely be even more changes after that (not Guix specific). Tree-SHA512: d3b16f87e48d1790a3264940c28acd5d881bfd10f3ce94fb0c8a6af76d8039289d01e0cd4972adac49ae24362857251f6c1e5e09e3e9fbf636c10708b4015a7c
bfe1ba2 rel-builds: Specify core.abbrev for git-rev-parse (Carl Dong) 27e63e0 build: Accomodate makensis v2.x (Carl Dong) 1f2c39a guix: Remove logical cores requirement (Carl Dong) a4f6ffa lint: Also enable source statements for non-gitian (Carl Dong) d256f91 rel-builds: Directly deploy win installer to OUTDIR (Carl Dong) fa791da nsis: Specify OutFile path only once (Carl Dong) 1470160 guix: Expose GIT_COMMON_DIR in container as readonly (Carl Dong) f5a6ac4 guix: Make source tarball using git-archive (Carl Dong) 395c113 gitian: Limit sourced script to just assignments (Carl Dong) Pull request description: Based on: bitcoin#18556 Related: bitcoin#17595 (comment) ACKs for top commit: fanquake: ACK bfe1ba2 - I agree with Carl, and am going to merge this. I'd like for Linux Guix builds to be working again, and we can rebase bitcoin#18818. Tree-SHA512: c87ada7e3de17ca0b692a91029b86573442ded5780fc081c214773f6b374a0cdbeaf6f6898c36669c2e247ee32aa7f82defb1180f8decac52c65f0c140f18674
Based on: #18556
Related: #17595 (comment)