cmake: Use `PUBLIC_HEADER` target property in installation logic #1679

hebasto commented at 12:35 PM on June 4, 2025: member

This PR updates the installation logic to the idiomatic approach.

cmake, move-only: Move module option processing to `src/CMakeLists.txt`

This change simplifies the following commit.

c32715b2a0

cmake: Use `PUBLIC_HEADER` target property

This change simplifies the installation logic.

6f67151ee2

in src/CMakeLists.txt:13 in ec41a35568 outdated

   5 | +# Processing must be done in a topological sorting of the dependency graph
   6 | +# (dependent module first).
   7 | +if(SECP256K1_ENABLE_MODULE_ELLSWIFT)
   8 | +  add_compile_definitions(ENABLE_MODULE_ELLSWIFT=1)
   9 | +  set_property(TARGET secp256k1 APPEND PROPERTY PUBLIC_HEADER ${PROJECT_SOURCE_DIR}/include/secp256k1_ellswift.h)
  10 | +endif()

hebasto commented at 12:36 PM on June 4, 2025:

add_compile_definitions() still affects all targets created in the same directory scope, regardless of whether those targets are created before or after add_compile_definitions() is called.

hebasto force-pushed on Jun 4, 2025

real-or-random added the label build on Jun 4, 2025

real-or-random added the label refactor/smell on Jun 4, 2025

real-or-random commented at 6:31 AM on June 6, 2025: contributor

Does this make a difference, or is it just more idiomatic? The docs for PUBLIC_HEADER look very specific to Apple and specific to shared libraries, but according to the discussion in the comments of https://stackoverflow.com/a/54285948, it's just that the docs are confusing... "File sets" seem to be better, but unfortunately, they require CMake 3.23...

hebasto commented at 7:19 AM on June 6, 2025: member

Does this make a difference, or is it just more idiomatic? The docs for PUBLIC_HEADER look very specific to Apple and specific to shared libraries, but according to the discussion in the comments of https://stackoverflow.com/a/54285948, it's just that the docs are confusing... "File sets" seem to be better, but unfortunately, they require CMake 3.23...

From the docs for install(TARGETS <target>... [...]):

PUBLIC_HEADER

Any PUBLIC_HEADER files associated with a library are installed in the destination specified by the PUBLIC_HEADER argument on non-Apple platforms. Rules defined by this argument are ignored for FRAMEWORK libraries on Apple platforms because the associated files are installed into the appropriate locations inside the framework folder.

Additionally, from Professional CMake: A Practical Guide 21st Edition, Section 35.5.2. Explicit Public And Private Headers:

If the target is not a framework, or if building for a non-Apple platform, no special handling is performed at build time. When the target is installed, files listed in these properties are also installed, but with their paths removed.

Here is an example of using PUBLIC_HEADER in a sibling project:

set_target_properties(multiprocess PROPERTIES
    PUBLIC_HEADER "${MP_PUBLIC_HEADERS}")
install(TARGETS multiprocess EXPORT LibTargets
  ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR} COMPONENT lib
  PUBLIC_HEADER DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/mp COMPONENT lib)

cc @ryanofsky

real-or-random commented at 9:45 AM on June 6, 2025: contributor

Concept ACK, this seems indeed more idiomatic

theuni approved

theuni commented at 5:39 PM on June 11, 2025: contributor

utACK 6f67151ee2e94a0ce14522632edcc153f1cde7e6

real-or-random merged this on Jun 16, 2025

real-or-random closed this on Jun 16, 2025

hebasto deleted the branch on Jun 16, 2025

hebasto referenced this in commit 68094d6972 on Jun 30, 2025

Sjors referenced this in commit 1b7453ea88 on Jul 17, 2025

hebasto referenced this in commit 28310efba4 on Jul 18, 2025

fanquake referenced this in commit 5600e6fc4b on Jul 22, 2025

saikiran57 referenced this in commit abe11bd67c on Jul 28, 2025

janus referenced this in commit e0ffd31e87 on Sep 14, 2025

vmta referenced this in commit 2b25f561a0 on Sep 21, 2025

github-actions[bot] referenced this in commit 758d4e90b4 on Mar 1, 2026

github-actions[bot] referenced this in commit 68a2178f22 on Mar 1, 2026

github-actions[bot] referenced this in commit a8bc1a0b2b on Mar 1, 2026

0x000000000019d6689c085ae165831e934ff76 referenced this in commit 3b9450150d on Mar 2, 2026

csjones referenced this in commit a4d92824ae on Mar 2, 2026