Fix "INSTALL(EXPORT) given unknown export "pffft-targets" " - #89
Conversation
…configuration time
marton78
left a comment
There was a problem hiding this comment.
Review
Thanks for the fix! The root cause diagnosis is correct and the minimal approach is appreciated. However there are a few issues to address before merging.
⚠️ Critical — pffft-config.cmake.in unconditionally includes pffft-targets.cmake
The generated pffft-config.cmake is still installed unconditionally (via configure_package_config_file / install(FILES ... pffft-config.cmake ...) further down the file), but it always does:
include("${CMAKE_CURRENT_LIST_DIR}/pffft-targets.cmake")When INSTALL_PFFFT=OFF, this PR correctly suppresses writing pffft-targets.cmake — but the config file still gets installed and still tries to include it. Any downstream find_package(pffft) call after such an install will immediately fail with a fatal include() error. The configure_package_config_file / write_basic_package_version_file / install(FILES pffft-config.cmake pffft-config-version.cmake ...) block should be wrapped in the same guard as the install(EXPORT ...) block.
ℹ️ Minor — empty install(FILES ${INSTALL_HEADERS} ...) (line 297)
When all INSTALL flags are OFF, INSTALL_HEADERS is empty. CMake silently creates an empty ${CMAKE_INSTALL_INCLUDEDIR}/pffft directory. Harmless, but cosmetically messy. Wrapping with if (INSTALL_HEADERS) would clean this up.
| NAMESPACE PFFFT:: | ||
| DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/pffft | ||
| ) | ||
| if (INSTALL_PFFFT) |
There was a problem hiding this comment.
The guard variable here should be INSTALL_TARGETS rather than INSTALL_PFFFT.
INSTALL_TARGETS can be non-empty even when INSTALL_PFFFT=OFF — for example if INSTALL_PFDSP=ON or INSTALL_PFFASTCONV=ON. In those cases, targets are added to the pffft-targets export set (so the export exists), but this guard would suppress writing pffft-targets.cmake, breaking the install for those components.
The semantically correct condition is whether any target was actually added to the export set:
if (INSTALL_TARGETS)
install(EXPORT pffft-targets
FILE pffft-targets.cmake
NAMESPACE PFFFT::
DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/pffft
)
endif()This is also more robust across CMake versions, where the behaviour of install(TARGETS "" EXPORT foo) with an empty list is not consistently specified.
…ruft into the conditional scope
When using this library as a cmake dependency with the option
INSTALL_PFFFTset to OFF it fails during configuration time with:INSTALL(EXPORT) given unknown export "pffft-targets"This is due to the fact that installation is done like so:
If
${INSTALL_TARGETS}is empty (which happens withINSTALL_PFFFTset to OFF) then pffft-targets does not exist and the subsequent command fails.This is just a workaround, I guess the entire CMake file could do with some rework but at least it unblocks anyone in the same situation as me.