bitcoin-core / secp256k1

Optimized C library for EC operations on curve secp256k1
MIT License
2.02k stars 977 forks source link

[POC, DO NOT MERGE] cmake: Switch to CMake utilities repository #1552

Closed hebasto closed 2 weeks ago

hebasto commented 2 weeks ago

This PR proposes reducing code repetition by reusing CMake utilities (functions and macros) shared between this project and Bitcoin Core. Both projects use similar functionality, including:

  1. Defining a default build configuration.
  2. Modifying configuration-specific flags.
  3. Checking compiler and linker flags.
  4. Reporting a summary.

To achieve this, I suggest creating a dedicated repository under https://github.com/bitcoin-core to house these utilities. Such a repository could also benefit other projects like https://github.com/sipa/minisketch and https://github.com/chaincodelabs/libmultiprocess. Additionally, the CMake utilities repository could include CI tests for the provided utilities.

Seeking Concept (N)ACKs for this proposal. Please note that only one function is replaced in this PR.

fanquake commented 2 weeks ago

Concept NACK - It's hard to evaluate how much benefit this would actually bring (without a full example), and there's a number of downsides / other things that'd need to be considered before doing anything like this:

real-or-random commented 2 weeks ago

I get a bit anxious when I see duplicated code, and I think sharing code is, in general, pretty desirable. But I can't argue with @fanquake's very convincing list of disadvantages, and so I agree with the Concept NACK.