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
depends: set two CMake options globally #29706
Conversation
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. 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. |
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.
ACK 6166bf4, I have reviewed the code and it looks OK.
Rather than setting this per package, set it globally, as this is always what we want. Without doing this, later commit will have to add the same doc + change to more packages.
Rather than potentially having to set this per-package, set it globally, as this should always be what we want. Without doing this, changes in later commits will have to add this per-package. Similar to bitcoin#29488, which is the Autotools equivalent.
6166bf4
to
76045bb
Compare
Rebased for #29488 and addressed the comment from that PR. |
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.
re-ACK 76045bb.
Guix Build (aarch64): 0b95e2e1393950de30c9c3d94e0408825fb38e3baa08390f524e6d863dc76ee1 guix-build-76045bb9d680/output/aarch64-linux-gnu/SHA256SUMS.part
7d3769bca1ef1c08575d572c10e58467da77d8e88743182269a275eba401eedd guix-build-76045bb9d680/output/aarch64-linux-gnu/bitcoin-76045bb9d680-aarch64-linux-gnu-debug.tar.gz
3cc9d1b4bee351f02f86c58ca9f692a31e1ac71146ac48265cec7da095029ee7 guix-build-76045bb9d680/output/aarch64-linux-gnu/bitcoin-76045bb9d680-aarch64-linux-gnu.tar.gz
19ef707cfe4deb58698746ade6bbe5f45b190e3e9fcb4d8c8620c01cb0fe06fe guix-build-76045bb9d680/output/arm-linux-gnueabihf/SHA256SUMS.part
4b55fe175ed65047b876380654421e4cec1ea390f42651cda949ffc4e43fbae1 guix-build-76045bb9d680/output/arm-linux-gnueabihf/bitcoin-76045bb9d680-arm-linux-gnueabihf-debug.tar.gz
1091ba17b80f748e5d0d3817ef4b3032dead635952aff34f58f71820639dee5f guix-build-76045bb9d680/output/arm-linux-gnueabihf/bitcoin-76045bb9d680-arm-linux-gnueabihf.tar.gz
2e2c2633ae3f0b3accf04bd80f0c989bf0fc5c4c7fadace1a344b1595211f210 guix-build-76045bb9d680/output/arm64-apple-darwin/SHA256SUMS.part
df8edf0f441385d4f81fc76b859a8123f234175102b47472ee07a2b96660284c guix-build-76045bb9d680/output/arm64-apple-darwin/bitcoin-76045bb9d680-arm64-apple-darwin-unsigned.tar.gz
5db038889701aacd1eb75e484d33a744181a83cd69b045dd1f12e14b197ea4b9 guix-build-76045bb9d680/output/arm64-apple-darwin/bitcoin-76045bb9d680-arm64-apple-darwin-unsigned.zip
0f22de3d630e3af60e1bdc515b1a9e78cf76cfd7930c36b84a6b07033cb18ae8 guix-build-76045bb9d680/output/arm64-apple-darwin/bitcoin-76045bb9d680-arm64-apple-darwin.tar.gz
1f7636a2f0bfa751f31002ea7ccd510c86bd7cfbe194d94a53e3ba4a8adb07fd guix-build-76045bb9d680/output/dist-archive/bitcoin-76045bb9d680.tar.gz
a461c18dcee5cc34e266466c5e96a89705ed25d36673b2450724f45c68d56b2a guix-build-76045bb9d680/output/powerpc64-linux-gnu/SHA256SUMS.part
09e18e5e91aed2bac05909f75489e4fe0b8eb74a7247749b113687d74eff769a guix-build-76045bb9d680/output/powerpc64-linux-gnu/bitcoin-76045bb9d680-powerpc64-linux-gnu-debug.tar.gz
42af567c2b8fcfae09409e2b5973ac12a10c9ca7a2a0a872e8a619e3fd37550e guix-build-76045bb9d680/output/powerpc64-linux-gnu/bitcoin-76045bb9d680-powerpc64-linux-gnu.tar.gz
76c4605b41d0ebef80aa9c4f65dca6068902ee2c10069d85adc7f1df09e66e7a guix-build-76045bb9d680/output/riscv64-linux-gnu/SHA256SUMS.part
017b0851ed279b5186c87715e5826e9cc31739d0cc1a83a3b33501fde05fa41b guix-build-76045bb9d680/output/riscv64-linux-gnu/bitcoin-76045bb9d680-riscv64-linux-gnu-debug.tar.gz
bfcd63aabf57b2bb80c62ab49584b99cb45e0992ed2efb4d24a9e347f8f81ad7 guix-build-76045bb9d680/output/riscv64-linux-gnu/bitcoin-76045bb9d680-riscv64-linux-gnu.tar.gz
8c16bb230b482d502533f7deaf1f175bee83a6dc705100e88b11417bd4c701ef guix-build-76045bb9d680/output/x86_64-apple-darwin/SHA256SUMS.part
47db4862b3c0fc9bfdb1a298a73be0cc7fc1152396dedddf2e199029bd257f89 guix-build-76045bb9d680/output/x86_64-apple-darwin/bitcoin-76045bb9d680-x86_64-apple-darwin-unsigned.tar.gz
4b45b44110c4ad65670c02caf3662dc99800d72189626dcdb2e3a45e7105dd64 guix-build-76045bb9d680/output/x86_64-apple-darwin/bitcoin-76045bb9d680-x86_64-apple-darwin-unsigned.zip
3df17878029b793e722861a3fc670e6c087347f91889b32e3ab3d53b389b2903 guix-build-76045bb9d680/output/x86_64-apple-darwin/bitcoin-76045bb9d680-x86_64-apple-darwin.tar.gz
9b2592d48bd53f41d83fa2d3e61d34057bd8c30678187d0436c210d0c7cf385b guix-build-76045bb9d680/output/x86_64-linux-gnu/SHA256SUMS.part
c96b1c4eade5e5309a0f77d60f0bcd2139ede8faa1ed5ea20cc97394700153f7 guix-build-76045bb9d680/output/x86_64-linux-gnu/bitcoin-76045bb9d680-x86_64-linux-gnu-debug.tar.gz
282fa857f67d1c76778210b50dfc38eac572339d3654fc7a47efc04d224d5238 guix-build-76045bb9d680/output/x86_64-linux-gnu/bitcoin-76045bb9d680-x86_64-linux-gnu.tar.gz
61694126644cf2ffe7ec7dbad1111b3e8ba2c1236142f3ea1db84bafb4ffec41 guix-build-76045bb9d680/output/x86_64-w64-mingw32/SHA256SUMS.part
8328c1e8832dfb86151d7b6612dd98aaa3708df035d0d50815533b2a6659653e guix-build-76045bb9d680/output/x86_64-w64-mingw32/bitcoin-76045bb9d680-win64-debug.zip
dccc134738cf7215c74c3b0500e2928444184110e6d3705b630ed160b954fd77 guix-build-76045bb9d680/output/x86_64-w64-mingw32/bitcoin-76045bb9d680-win64-setup-unsigned.exe
9c8a5a3d994bee9ad9f4a0c3e24dcfc40ff2b57cc32640c09ef932c21ae0f597 guix-build-76045bb9d680/output/x86_64-w64-mingw32/bitcoin-76045bb9d680-win64-unsigned.tar.gz
b6bf76e10a0e1808693969a55c6e3732f3a0b3b6f7e4c76267f66d9d079bf30c guix-build-76045bb9d680/output/x86_64-w64-mingw32/bitcoin-76045bb9d680-win64.zip |
My Guix build:
|
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.
utACK 76045bb. Both changes make sense to me, and both can be overridden if needed, though I can't imagine we'd need to.
Post merge ACK 76045bb Also get a matching build. |
Set
CMAKE_INSTALL_LIBDIR=lib/
andCMAKE_POSITION_INDEPENDENT_CODE=ON
globally in depends, rather than per-package.CMAKE_INSTALL_LIBDIR=lib/
is needed to override the annoyingGNUInstallDirs
lib
vslib64
behaviour, and we always want PIC code. The PIC commit is the counterpart to the same Autotools change in #29488. I'm PRing these commits as I have a CMake branch building on top, and want to avoid adding the same workarounds to every package we are going to touch, but these can go in separately as the build should be tested for existing packages (i.e multiprocess).