cmake: Use PUBLIC_HEADER target property in installation logic #1679

1. 
   hebasto commented at 12:35 pm on June 4, 2025: member

   This PR updates the installation logic to the idiomatic approach.
2. cmake, move-only: Move module option processing to `src/CMakeLists.txt`

   This change simplifies the following commit.

   c32715b2a0
3. cmake: Use `PUBLIC_HEADER` target property

   This change simplifies the installation logic.

   6f67151ee2
4. 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.
5. hebasto force-pushed on Jun 4, 2025
6. real-or-random added the label build on Jun 4, 2025
7. real-or-random added the label refactor/smell on Jun 4, 2025
8. 
   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…
9. 
   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:

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

   cc @ryanofsky

10. 
    real-or-random commented at 9:45 am on June 6, 2025: contributor

    Concept ACK, this seems indeed more idiomatic
11. theuni approved
12. 
    theuni commented at 5:39 pm on June 11, 2025: contributor

    utACK 6f67151ee2e94a0ce14522632edcc153f1cde7e6
13. real-or-random merged this on Jun 16, 2025
14. real-or-random closed this on Jun 16, 2025

    ---

15. hebasto deleted the branch on Jun 16, 2025

Contributors
hebasto real-or-random theuni

Labels
build refactor/smell