cmake: Some improvements using `PROJECT_IS_TOP_LEVEL` variable #1284

pull hebasto wants to merge 3 commits into bitcoin-core:master from hebasto:230420-parent changing 2 files +17 −3

hebasto commented at 10:29 PM on April 20, 2023: member
This PR:
1. Emulates PROJECT_IS_TOP_LEVEL variable for CMake versions where it is not available.
2. Makes the SECP256K1_INSTALL option dependent on PROJECT_IS_TOP_LEVEL (a follow up of #1263).
3. Makes integration of this project as a subtree easier. A top project can #include <secp256k1.h> with no additional target_include_directories() commands. For example, see https://github.com/hebasto/secp256k1-CMake-example/tree/subtree.
cmake: Emulate `PROJECT_IS_TOP_LEVEL` for CMake<3.21 162608cc98

in CMakeLists.txt:48 in b9112aa61b outdated

  45 |    set(BUILD_SHARED_LIBS OFF)
  46 |  endif()
  47 |  
  48 | -option(SECP256K1_INSTALL "Enable installation" ON)
  49 | +include(CMakeDependentOption)
  50 | +cmake_dependent_option(SECP256K1_INSTALL "Enable installation." ON "PROJECT_IS_TOP_LEVEL" OFF)

real-or-random commented at 8:22 AM on April 21, 2023:

Does it really make sense to forbid installation if used as a subproject? I think we could let the user decide here.

hebasto commented at 11:07 AM on April 21, 2023:

Thanks! Updated.

real-or-random commented at 8:22 AM on April 21, 2023: contributor

Concept ACK

cmake: Make `SECP256K1_INSTALL` default depend on `PROJECT_IS_TOP_LEVEL`

Also full stops have been added to the option help texts for consistency
in cmake-gui.

5431b9decd

hebasto force-pushed on Apr 21, 2023
hebasto commented at 11:06 AM on April 21, 2023: member
Updated b9112aa61b9a0cd9e6f790b920568b85be1d5b8a -> 009faea5c6f48b0d7b7c2bf87caf44621b6c6b21 (pr1284.01 -> pr1284.02, diff):
- addressed @real-or-random's comment
real-or-random approved
real-or-random commented at 11:24 AM on April 21, 2023: contributor

utACK 009faea5c6f48b0d7b7c2bf87caf44621b6c6b21
real-or-random approved
real-or-random commented at 3:44 PM on April 26, 2023: contributor

[edited: nevermind, I acked the wrong PR...]

in src/CMakeLists.txt:36 in 009faea5c6 outdated

  30 | @@ -31,7 +31,8 @@ endif()
  31 |  get_target_property(use_pic secp256k1 POSITION_INDEPENDENT_CODE)
  32 |  set_target_properties(secp256k1_precomputed PROPERTIES POSITION_INDEPENDENT_CODE ${use_pic})
  33 |  
  34 | -target_include_directories(secp256k1 PUBLIC
  35 | +target_include_directories(secp256k1 INTERFACE
  36 | +  $<BUILD_INTERFACE:$<$<NOT:$<BOOL:${PROJECT_IS_TOP_LEVEL}>>:${PROJECT_SOURCE_DIR}/include>>

theuni commented at 2:12 PM on April 27, 2023:

Can we get a comment? Something like: "Add the include path for parent projects so that they don't have to manually add it."

theuni commented at 2:12 PM on April 27, 2023: contributor

ACK modulo comment request.
cmake: Include `include` directory for subtree builds 71f746c057
hebasto force-pushed on Apr 27, 2023
theuni commented at 2:17 PM on April 27, 2023: contributor

Hah, that was fast!

utACK 71f746c057a66d5ae0dfdb5d439bc49592c0d16e.
real-or-random merged this on Apr 27, 2023
real-or-random closed this on Apr 27, 2023
hebasto deleted the branch on Apr 28, 2023
sipa referenced this in commit b4eb644b6c on May 12, 2023
hebasto referenced this in commit 49c52ea2b1 on May 13, 2023
vmta referenced this in commit e1120c94a1 on Jun 4, 2023
sipa referenced this in commit 901336eee7 on Jun 21, 2023
vmta referenced this in commit 8f03457eed on Jul 1, 2023
delta1 referenced this in commit 3f32c20932 on Aug 8, 2023
delta1 referenced this in commit 31ac0c1081 on Aug 31, 2023
janus referenced this in commit c4348d88db on Sep 11, 2023
div72 referenced this in commit af627d47c3 on Apr 12, 2025
str4d referenced this in commit 5a6bf5f178 on Jun 4, 2025

Contributors

hebasto

real-or-random

theuni

Linked (view graph)

#1235 build: Meta-issue for follow-ups to initial CMake merge (#1113)#1263 cmake: Make installation optional