cmake: Set POSITION_INDEPENDENT_CODE property properly #1232

pull hebasto wants to merge 1 commits into bitcoin-core:master from hebasto:230309-pic changing 2 files +24 −7
  1. hebasto commented at 0:52 am on March 10, 2023: member

    This PR is an alternative to #1230.

    Benefits of this PR in comparison to #1230:

    • keeps ability to build a shared library and a static one simultaneously, which is convenient, for example, during a process of creating a distributable package like libsecp256k1-dev using CPack tool
    • has smaller diff
    • does not require CMake version bumping

    Now, a user can set PIC/non-PIC mode for a static library using CMAKE_POSITION_INDEPENDENT_CODE, and this setting is independent from a shared library.

  2. hebasto commented at 1:40 am on March 10, 2023: member
    cc @theuni
  3. cmake: Set `POSITION_INDEPENDENT_CODE` property properly a3a4587408
  4. hebasto force-pushed on Mar 10, 2023
  5. theuni commented at 5:19 pm on March 10, 2023: contributor

    Concept NACK for the reasons described in this thread: #1113 (review). Though take that for what it’s worth given my obvious bias from #1230.

    • keeps ability to build a shared library and a static one simultaneously, which is convenient

    I won’t disagree this is convenient, but I don’t agree that it’s worth the complexity.

    • has smaller diff

    I don’t think that’s relevant here as I believe all reviewers of #1113 assumed we’d be making some structural changes in a follow-up. If I thought this was locked in I wouldn’t have ACKed.

    • does not require CMake version bumping @real-or-random doesn’t seem too concerned about this (quite the opposite actually).

    I guess this is really just a question for maintainers. I’m not opposed to simultaneous libs, I just don’t think it’s very idiomatic or worth the trouble.

    I’m also not trying to keep bumping up the relevance of something so trivial as PIC. I suspect the reason it keeps coming up is because it’s an example of something that’s correctly handled (technically anyway, style aside) rather simply in a one-at-a-time build, but gets complex with simultaneous builds..

  6. hebasto commented at 5:32 pm on March 10, 2023: member

    Concept NACK for the reasons described in this thread: #1113 (comment). Though take that for what it’s worth given my obvious bias from #1230.

    • keeps ability to build a shared library and a static one simultaneously, which is convenient

    I won’t disagree this is convenient, but I don’t agree that it’s worth the complexity.

    If we follow #1230, what packaging process at some point in the future can be imagined? Run cmake / cmake --build procedure twice? Isn’t it another type of complexity?

  7. real-or-random commented at 6:10 pm on March 10, 2023: contributor

    I tend to think that not setting PIC globally is a bit closer to the philosophy we followed so far, where we let the user decide as much as possible. This is just because the goal is to target a broad range of platforms and thus want to avoid making assumptions about our users as much as possible (and maybe that part is also biased/inspired by the philosophy of autotools). I don’t think I’m very competent to judge how relevant it is to let the user decide in the specific case of PIC. But if you ask me, I’d also lean towards #1230.


    If we follow #1230, what packaging process at some point in the future can be imagined? Run cmake / cmake --build procedure twice? Isn’t it another type of complexity?

    To be honest, I’m convinced that we’ll ever ship binaries. So, it probably won’t be relevant to us. (And if it is, it would occur about every three months, so it’s not a big deal to run things twice.) As for others packaging our library, I think they’ll anyway be only interested in the shared library in 99% of the cases.

  8. hebasto commented at 6:13 pm on March 10, 2023: member
    Closing in favour of #1230.
  9. hebasto closed this on Mar 10, 2023

  10. hebasto deleted the branch on Apr 29, 2023

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/secp256k1. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-10-30 03:15 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me