Allowed named header sets in beman install#19
Allowed named header sets in beman install#19steve-downey wants to merge 4 commits intobemanproject:mainfrom
Conversation
|
Argument parsing is a good idea, but it is needed to install multiple targets! The get_target_property(
header_sets
${target}
INTERFACE_HEADER_SETS
) |
Will the INTERFACE_HEADER_SETS work for a target that isn't an interface? I don't care terribly much about how the install finds the correct set to install, just that naming them all HEADER may cause collisions in what we've been calling "workspace" projects, groups of projects aggregated together into a single larger CMake project. If we can make everything work from just the target name to install, I'll happily rebase optional and fix that side up. |
|
The FILE_SET are target properties, and we should only use the default names. Only one per target! I have a running test here: ClausKlein/exemplar#2 |
|
@steve-downey I would like to use the |
|
I don't remember at the moment why I arrived at needing names for filesets instead of just HEADERS, but it wasn't just because there was a separate TYPE added. I tend to avoid "private" implementation only headers in my practice, but I can see the utility of the distinction. Although it may break down in the face of modules, if they are needed internally to construct the BMI. Pure implementation partitions are just library TU and don't contribute to the built module interface, and don't need to be shipped to package consumers needing to rebuild the BMI. |
1 similar comment
|
I don't remember at the moment why I arrived at needing names for filesets instead of just HEADERS, but it wasn't just because there was a separate TYPE added. I tend to avoid "private" implementation only headers in my practice, but I can see the utility of the distinction. Although it may break down in the face of modules, if they are needed internally to construct the BMI. Pure implementation partitions are just library TU and don't contribute to the built module interface, and don't need to be shipped to package consumers needing to rebuild the BMI. |
|
I have a fully working prototype that handles all what must be at least an option IMHO to install a project: |
my proposal:Installing a LibraryInstallation is handled by the Function Signaturebeman_install_library(name
TARGETS <target1> [<target2> ...]
[DEPENDENCIES <dep1> [<dep2> ...]]
[NAMESPACE <namespace>]
[EXPORT_NAME <export-name>]
[DESTINATION <module-destination>]
)Arguments
This function installs the specified project TARGETS and its It also handles the installation of the CMake config package files if
CaveatsOnly one Which form would you prefer for dependencies:include(./cmake/beman-install-library.cmake)
beman_install_library(${PROJECT_NAME} TARGETS ${PROJECT_NAME} beman.exemplar_headers #
# FIXME: DEPENDENCIES [===[beman.inplace_vector 1.0.0]===] [===[beman.scope 0.0.1 EXACT]===] fmt
# TODO(CK): XXX OR XXX DEPENDENCIES "beman.inplace_vector 1.0.0;beman.scope 0.0.1 EXACT;fmt"
) |
|
@ednolan which solution should be use, to install CXX_MODULES, multiple targets, FILE_SET, describe the Target dependencies, ... |
|
I don't think I want to constrain the name of the FILE_SET. If the header file sets for a target can always be read out from a target, that is OK, but I think it may fail in the face of pure implementation header files, that is ones only used in an implementation file, and not installed, but still necessary to be found during compilation of a target. This might be a distinction between the visibility of the header file set? |
I will check it, but AFAIK I is based on a suggestion and reviewed from Ben Boeckel |
|
What do you think about this cmake config packages install tree: |
|
This tends to break the principle that everything within an install prefix is part of a consistent whole. It's mixing opt style discreet packaging with shared FSHS packaging. In a shared install, there's a -I include pointing at the top, but Beman/exemplar/identity.hpp is not available that way, meaning that the installed include directory is not performing its function. We also don't version Beman as a project, unlike Boost, so beman-0.1.0 isn't meaningful. But in any case it should not be installed in parallel with a hypothetical beman-1.2.5. A third package can not depend on both of them. If you want, or need, multiple package versions generally visible, the would be in, e.g., /opt/Beman/beman-optional-1.0.0/{include, lib, share} , although you still have the problem of the installed lib possibly not using the correct deps. This is of course why closed source libraries can't have dependencies beyond a specific std lib, and why library ABI is such a problem for vendors. |
|
It was recommended from Ben Boeckel (KITWARE)! It works: https://github.com/bemanproject/exemplar/actions/runs/21921464251 There is no
|
|
Boost doesn't do just the same, though, they install into a shared Task currently depends on Execution. But in any case, a PREFIX ought not to be a random collection of stuff, it is intended to be coherent at least within the PREFIX. |
|
OK, from the cmake build site, everything works fine. bash-5.3$ _CPack_Packages/Darwin/TGZ/runtime_destination_dependency-1.5.0-Darwin/bin/PackageProjectTest-1.0/main
dyld[93924]: Library not loaded: @rpath/libdependency.dylib
Referenced from: <5E289461-23E1-39F9-8475-94B16B1FD661> /Users/clausklein/cmake/TheLartians/PackageProject/test/build/_CPack_Packages/Darwin/TGZ/runtime_destination_dependency-1.5.0-Darwin/bin/PackageProjectTest-1.0/main
Reason: tried: '/Users/clausklein/cmake/TheLartians/PackageProject/test/build/_CPack_Packages/Darwin/TGZ/runtime_destination_dependency-1.5.0-Darwin/bin/PackageProjectTest-1.0/libdependency.dylib' (no such file), '/usr/local/Cellar/gcc/15.2.0/lib/gcc/current/gcc/x86_64-apple-darwin23/15/libdependency.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OS/usr/local/Cellar/gcc/15.2.0/lib/gcc/current/gcc/x86_64-apple-darwin23/15/libdependency.dylib' (no such file), '/usr/local/Cellar/gcc/15.2.0/lib/gcc/current/gcc/libdependency.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OS/usr/local/Cellar/gcc/15.2.0/lib/gcc/current/gcc/libdependency.dylib' (no such file), '/usr/local/Cellar/gcc/15.2.0/lib/gcc/current/libdependency.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OS/usr/local/Cellar/gcc/15.2.0/lib/gcc/current/libdependency.dylib' (no such file), '/Users/clausklein/cmake/TheLartians/PackageProject/test/build/_CPack_Packages/Darwin/TGZ/runtime_destination_dependency-1.5.0-Darwin/bin/PackageProjectTest-1.0/libdependency.dylib' (no such file), '/usr/local/Cellar/gcc/15.2.0/lib/gcc/current/gcc/x86_64-apple-darwin23/15/libdependency.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OS/usr/local/Cellar/gcc/15.2.0/lib/gcc/current/gcc/x86_64-apple-darwin23/15/libdependency.dylib' (no such file), '/usr/local/Cellar/gcc/15.2.0/lib/gcc/current/gcc/libdependency.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OS/usr/local/Cellar/gcc/15.2.0/lib/gcc/current/gcc/libdependency.dylib' (no such file), '/usr/local/Cellar/gcc/15.2.0/lib/gcc/current/libdependency.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OS/usr/local/Cellar/gcc/15.2.0/lib/gcc/current/libdependency.dylib' (no such file)
Abort trap: 6 _CPack_Packages/Darwin/TGZ/runtime_destination_dependency-1.5.0-Darwin/bin/PackageProjectTest-1.0/main
bash-5.3$ !tree
tree _CPack_Packages/Darwin/TGZ/runtime_destination_dependency-1.5.0-Darwin/
_CPack_Packages/Darwin/TGZ/runtime_destination_dependency-1.5.0-Darwin/
├── bin
│ └── PackageProjectTest-1.0
│ └── main
├── include
│ ├── dependency-1.2
│ │ └── dependency
│ │ ├── dependency.h
│ │ └── version.h
│ ├── header_only-1.0
│ │ └── header_only
│ │ ├── adder.h
│ │ └── version.h
│ ├── namespaced_dependency-4.5.6
│ │ └── namespaced_dependency
│ │ ├── namespaced_dependency.h
│ │ └── version.h
│ └── runtime_destination_dependency-1.5
│ └── runtime_destination_dependency
│ ├── dependency.h
│ └── version.h
├── lib
│ ├── cmake
│ │ ├── dependency-1.2
│ │ │ ├── dependencyConfig.cmake
│ │ │ ├── dependencyConfigVersion.cmake
│ │ │ ├── dependencyTargets-noconfig.cmake
│ │ │ └── dependencyTargets.cmake
│ │ ├── namespaced_dependency-4.5.6
│ │ │ ├── namespaced_dependencyConfig.cmake
│ │ │ ├── namespaced_dependencyConfigVersion.cmake
│ │ │ ├── namespaced_dependencyTargets-noconfig.cmake
│ │ │ └── namespaced_dependencyTargets.cmake
│ │ └── runtime_destination_dependency-1.5
│ │ ├── runtime_destination_dependencyConfig.cmake
│ │ ├── runtime_destination_dependencyConfigVersion.cmake
│ │ ├── runtime_destination_dependencyTargets-noconfig.cmake
│ │ └── runtime_destination_dependencyTargets.cmake
│ ├── dependency-1.2
│ │ └── libdependency.dylib
│ ├── namespaced_dependency-4.5.6
│ │ └── libnamespaced_dependency.dylib
│ └── runtime_destination_dependency-1.5
│ └── libruntime_destination_dependency.dylib
└── share
└── cmake
└── header_only-1.0
├── header_onlyConfig.cmake
├── header_onlyConfigVersion.cmake
└── header_onlyTargets.cmake
23 directories, 27 files
bash-5.3$ |
|
@ClausKlein Do you have an alternate patch that just makes the changes to install that you'd like to see? I can then try that out against Optional, where I'm using this patch now. |
https://github.com/ClausKlein/exemplar/blob/feature/add_cxx_module/cmake/beman-install-library.cmake |
Iterate over the target property for header sets to gather which header sets are installed. Extracted from https://github.com/ClausKlein/exemplar/blob/feature/add_cxx_module/cmake/beman-install-library.cmake and discussed in bemanproject/infra#19
Install based on the INTERFACE_HEADER_SETS for the target. Accept a list of TARGETS to install if the package is not also the installable target, or not the only target.
| if(NOT TARGET "${name}") | ||
| message(FATAL_ERROR "Target '${name}' does not exist and TARGETS not specified.") | ||
| endif() | ||
| set(BEMAN_INSTALL_LIBRARY_TARGETS ${name}) |
There was a problem hiding this comment.
[pre-commit] reported by reviewdog 🐶
| if(NOT TARGET "${name}") | |
| message(FATAL_ERROR "Target '${name}' does not exist and TARGETS not specified.") | |
| endif() | |
| set(BEMAN_INSTALL_LIBRARY_TARGETS ${name}) | |
| if(NOT TARGET "${name}") | |
| message( | |
| FATAL_ERROR | |
| "Target '${name}' does not exist and TARGETS not specified." | |
| ) | |
| endif() | |
| set(BEMAN_INSTALL_LIBRARY_TARGETS ${name}) |
| if(NOT TARGET "${_tgt}") | ||
| message( | ||
| WARNING | ||
| "beman_install_library(${name}): '${_tgt}' is not a target" |
There was a problem hiding this comment.
[pre-commit] reported by reviewdog 🐶
| if(NOT TARGET "${_tgt}") | |
| message( | |
| WARNING | |
| "beman_install_library(${name}): '${_tgt}' is not a target" | |
| if(NOT TARGET "${_tgt}") | |
| message( | |
| WARNING | |
| "beman_install_library(${name}): '${_tgt}' is not a target" | |
| ) | |
| continue() | |
| endif() | |
| set(target_name "${_tgt}") | |
| set(install_component_name "${_tgt}") | |
| set(export_name "${name}") | |
| set(package_name "${name}") | |
| list(GET name_parts -1 component_name) | |
| set(_install_header_set_args) | |
| get_target_property( | |
| _available_header_sets | |
| ${_tgt} | |
| INTERFACE_HEADER_SETS |
| continue() | ||
| endif() | ||
|
|
||
| set(target_name "${_tgt}") | ||
| set(install_component_name "${_tgt}") | ||
| set(export_name "${name}") | ||
| set(package_name "${name}") | ||
| list(GET name_parts -1 component_name) | ||
|
|
||
| set(_install_header_set_args) | ||
| get_target_property( | ||
| _available_header_sets | ||
| ${_tgt} | ||
| INTERFACE_HEADER_SETS | ||
| ) | ||
| if(_available_header_sets) | ||
| message( | ||
| VERBOSE | ||
| "beman-install-library(${name}): '${_tgt}' has INTERFACE_HEADER_SETS=${_available_header_sets}" |
There was a problem hiding this comment.
[pre-commit] reported by reviewdog 🐶
| continue() | |
| endif() | |
| set(target_name "${_tgt}") | |
| set(install_component_name "${_tgt}") | |
| set(export_name "${name}") | |
| set(package_name "${name}") | |
| list(GET name_parts -1 component_name) | |
| set(_install_header_set_args) | |
| get_target_property( | |
| _available_header_sets | |
| ${_tgt} | |
| INTERFACE_HEADER_SETS | |
| ) | |
| if(_available_header_sets) | |
| message( | |
| VERBOSE | |
| "beman-install-library(${name}): '${_tgt}' has INTERFACE_HEADER_SETS=${_available_header_sets}" | |
| if(_available_header_sets) | |
| message( | |
| VERBOSE | |
| "beman-install-library(${name}): '${_tgt}' has INTERFACE_HEADER_SETS=${_available_header_sets}" | |
| ) | |
| foreach(_install_header_set IN LISTS _available_header_sets) | |
| list( | |
| APPEND | |
| _install_header_set_args | |
| FILE_SET | |
| "${_install_header_set}" | |
| COMPONENT | |
| "${install_component_name}" | |
| ) | |
| endforeach() | |
| else() | |
| set(_install_header_set_args FILE_SET HEADERS) | |
| endif() | |
| install( | |
| TARGETS "${target_name}" | |
| EXPORT "${export_name}" | |
| ${_install_header_set_args} | |
| ) | |
| set_target_properties( | |
| "${target_name}" | |
| PROPERTIES EXPORT_NAME "${component_name}" |
| foreach(_install_header_set IN LISTS _available_header_sets) | ||
| list( | ||
| APPEND _install_header_set_args | ||
| FILE_SET | ||
| "${_install_header_set}" | ||
| COMPONENT | ||
| "${install_component_name}" | ||
| ) | ||
| endforeach() | ||
| else() | ||
| set(_install_header_set_args FILE_SET HEADERS) | ||
| endif() | ||
|
|
||
| install( | ||
| TARGETS "${target_name}" | ||
| COMPONENT "${install_component_name}" | ||
| EXPORT "${export_name}" | ||
| FILE_SET HEADERS | ||
| ) | ||
| ${_install_header_set_args} | ||
| ) | ||
|
|
||
| set_target_properties( | ||
| set_target_properties( | ||
| "${target_name}" | ||
| PROPERTIES EXPORT_NAME "${component_name}" | ||
| ) | ||
| ) |
There was a problem hiding this comment.
[pre-commit] reported by reviewdog 🐶
| foreach(_install_header_set IN LISTS _available_header_sets) | |
| list( | |
| APPEND _install_header_set_args | |
| FILE_SET | |
| "${_install_header_set}" | |
| COMPONENT | |
| "${install_component_name}" | |
| ) | |
| endforeach() | |
| else() | |
| set(_install_header_set_args FILE_SET HEADERS) | |
| endif() | |
| install( | |
| TARGETS "${target_name}" | |
| COMPONENT "${install_component_name}" | |
| EXPORT "${export_name}" | |
| FILE_SET HEADERS | |
| ) | |
| ${_install_header_set_args} | |
| ) | |
| set_target_properties( | |
| set_target_properties( | |
| "${target_name}" | |
| PROPERTIES EXPORT_NAME "${component_name}" | |
| ) | |
| ) |
|
@ClausKlein I think I pulled out the interface header part of your work, putting the module parts to the side for now. This fixes my concern about naming and collecting the header set(s) to install. I've used this now in beman.optional without having to change the calling site with the assumption that the "name" is also the target name if the list of targets is empty. |
Add an optional argument for named header sets in the beman install function.