fanquake
commented at 5:11 pm on March 22, 2024:
member
Set CMAKE_INSTALL_LIBDIR=lib/ and CMAKE_POSITION_INDEPENDENT_CODE=ON globally in depends, rather than per-package. CMAKE_INSTALL_LIBDIR=lib/ is needed to override the annoying GNUInstallDirslib vs lib64 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).
DrahtBot
commented at 5:11 pm on March 22, 2024:
contributor
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Conflicts
Reviewers, this pull request conflicts with the following ones:
#29488 (depends: always configure with --with-pic by fanquake)
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.
DrahtBot added the label
Build system
on Mar 22, 2024
fanquake removed the label
Build system
on Mar 22, 2024
fanquake added the label
DrahtBot Guix build requested
on Mar 22, 2024
DrahtBot
commented at 5:12 am on March 23, 2024:
contributor
DrahtBot removed the label
DrahtBot Guix build requested
on Mar 23, 2024
DrahtBot added the label
Build system
on Mar 23, 2024
hebasto approved
hebasto
commented at 12:13 pm on March 24, 2024:
member
ACK6166bf45ca57a3506c91660f0fe613e2ad6bdcbc, I have reviewed the code and it looks OK.
depends: always set CMAKE_INSTALL_LIBDIR=lib/
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.
d04623678c
depends: always set CMAKE_POSITION_INDEPENDENT_CODE=ON
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 https://github.com/bitcoin/bitcoin/pull/29488, which is the
Autotools equivalent.
76045bb9d6
fanquake force-pushed
on Mar 25, 2024
fanquake
commented at 10:52 am on March 25, 2024:
member
Rebased for #29488 and addressed the comment from that PR.
hebasto approved
hebasto
commented at 10:54 am on March 25, 2024:
member
re-ACK76045bb9d6808931cd0f2933203b5b611e032ec8.
fanquake
commented at 12:19 pm on March 25, 2024:
member
theuni
commented at 3:22 pm on March 25, 2024:
member
utACK76045bb9d6808931cd0f2933203b5b611e032ec8. Both changes make sense to me, and both can be overridden if needed, though I can’t imagine we’d need to.
fanquake merged this
on Mar 25, 2024
fanquake closed this
on Mar 25, 2024
fanquake deleted the branch
on Mar 25, 2024
TheCharlatan
commented at 6:37 pm on March 25, 2024:
contributor
Post merge ACK76045bb9d6808931cd0f2933203b5b611e032ec8
This is a metadata mirror of the GitHub repository
bitcoin/bitcoin.
This site is not affiliated with GitHub.
Content is generated from a GitHub metadata backup.
generated: 2024-09-28 22:12 UTC
This site is hosted by @0xB10C More mirrored repositories can be found on mirror.b10c.me
