Skip to content

cmake: Avoid contaminating parent project's cache with BUILD_SHARED_LIBS #1688

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Jun 19, 2025

The CMake cache is global in scope. Therefore, setting the standard cache variable BUILD_SHARED_LIBS can inadvertently affect the behavior of a parent project.

Consider configuring Bitcoin Core without explicit setting BUILD_SHARED_LIBS:

$ cmake -B build -DBUILD_KERNEL_LIB=ON

According to CMake’s documentation, this should configure libbitcoinkernel as STATIC.
However, that's not the case:

$ cmake --build build -t libbitcoinkernel
[143/143] Linking CXX shared library lib/libbitcoinkernel.so

This PR:

  1. Sets the BUILD_SHARED_LIBS cache variable only when libsecp256k1 is the top-level project.
  2. Uses the SECP256K1_DISABLE_SHARED cache variable only when libsecp256k1 is included as a subproject.

Alternatively, the entire code block can be dropped. In that case:

  1. The CMake-base build system will build a static library by default.
  2. During integration into a parent project, the static library can be forced as demonstrated here.

The CMake cache is global in scope. Therefore, setting the standard
cache variable `BUILD_SHARED_LIBS` can inadvertently affect the behavior
of a parent project.

This change:
1. Sets the `BUILD_SHARED_LIBS` cache variable only when libsecp256k1 is
   the top-level project.
2. Uses the `SECP256K1_DISABLE_SHARED` cache variable only when
   libsecp256k1 is included as a subproject.
@hebasto
Copy link
Member Author

hebasto commented Jun 19, 2025

cc @theuni

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants