Simplify cmake targets exporting#930
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #930 +/- ##
=======================================
Coverage 92.63% 92.63%
=======================================
Files 55 55
Lines 2673 2673
Branches 839 839
=======================================
Hits 2476 2476
Misses 112 112
Partials 85 85 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
I forgot how the CMake install mechanism works in detail, but do we need to keep the alias target declarations?
add_library(DDC::core ALIAS ddc_core)
# ...
add_library(DDC::fft ALIAS ddc_fft)
# etcThere was a problem hiding this comment.
It is meant for evil users that want to mix add_subdirectory(ddc) and find_package(DDC) :). The alias makes it transparent to use both. If it were not present, doing add_subdirectory(ddc), they could only access ddc_core.
CMakeLists.txt
Outdated
| ## installation | ||
|
|
||
| install(EXPORT DDCTargets NAMESPACE DDC::impl:: DESTINATION ${DDC_INSTALL_CMAKEDIR}) | ||
| install(EXPORT DDCTargets NAMESPACE DDC:: DESTINATION ${DDC_INSTALL_CMAKEDIR}) |
There was a problem hiding this comment.
Will it be better to call DDCTargets as DDCCoreTargets and first modify install(TARGETS ddc_core EXPORT DDCTargets FILE_SET HEADERS) as install(TARGETS ddc_core EXPORT DDCCoreTargets FILE_SET HEADERS)?
There was a problem hiding this comment.
Yes seems reasonable, let me try.
3 participants
The PR both simplifies the logic of exporting targets and makes DDC compliant with the new policy of namespaces, see https://www.kitware.com/psa-your-package-name-and-target-namespace-should-match/. This is done by removing the intermediate
impl::namespace and renaming the DDC targets in the install phase.The file
DDCTargets.cmakeis also renamedDDCCoreTargets.cmake.