Fix searching for dll paths, instead of adding .lib path.#6410
Fix searching for dll paths, instead of adding .lib path.#6410larshg wants to merge 1 commit intoPointCloudLibrary:masterfrom
Conversation
larshg
added
module: cmake
changelog: fix
There was a problem hiding this comment.
Pull request overview
This PR addresses Windows CMake import target metadata so that IMPORTED_LOCATION points to the actual runtime .dll (not the .lib import library), fixing $<TARGET_RUNTIME_DLLS:...> behavior (Issue #6345).
Changes:
- Adds
find_file()lookups for per-component DLL paths under${PCL_ROOT}/bin. - Updates imported PCL component targets to use found DLL paths for
IMPORTED_LOCATION[_<CONFIG>]while keeping.libpaths inIMPORTED_IMPLIB[_<CONFIG>]. - Introduces a special-case branch for the
iocomponent DLL naming.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if(PCL_${COMPONENT}_LIBRARY_DEBUG) | ||
| set_target_properties(${pcl_component} | ||
| PROPERTIES | ||
| IMPORTED_CONFIGURATIONS "RELEASE;DEBUG" | ||
| IMPORTED_LOCATION_RELEASE "${PCL_${COMPONENT}_LIBRARY}" | ||
| IMPORTED_LOCATION_DEBUG "${PCL_${COMPONENT}_LIBRARY_DEBUG}" | ||
| IMPORTED_LOCATION_RELEASE "${PCL_${COMPONENT}_DLL_PATH}" | ||
| IMPORTED_LOCATION_DEBUG "${PCL_${COMPONENT}_DLL_PATH_DEBUG}" | ||
| IMPORTED_IMPLIB_RELEASE "${PCL_${COMPONENT}_LIBRARY}" | ||
| IMPORTED_IMPLIB_DEBUG "${PCL_${COMPONENT}_LIBRARY_DEBUG}" | ||
| ) | ||
| else() | ||
| set_target_properties(${pcl_component} | ||
| PROPERTIES | ||
| IMPORTED_LOCATION "${PCL_${COMPONENT}_LIBRARY}" | ||
| IMPORTED_LOCATION "${PCL_${COMPONENT}_DLL_PATH}" | ||
| IMPORTED_IMPLIB "${PCL_${COMPONENT}_LIBRARY}" |
There was a problem hiding this comment.
IMPORTED_LOCATION* is being set to ${PCL_${COMPONENT}_DLL_PATH} unconditionally. On non-Windows platforms and on Windows static builds there may be no .dll (and find_file(... .dll) will fail), resulting in an empty/incorrect IMPORTED_LOCATION and a broken imported target. Please gate the DLL lookup + IMPORTED_LOCATION* override behind WIN32 and PCL_SHARED_LIBS (or @PCL_LIB_TYPE@ STREQUAL "SHARED"), and otherwise keep using ${PCL_${COMPONENT}_LIBRARY} for IMPORTED_LOCATION* as before.
| if ("${component}" STREQUAL "io") | ||
| find_file(PCL_${COMPONENT}_DLL_PATH | ||
| NAMES | ||
| pcl_io_ply$${PCL_RELEASE_SUFFIX}.dll | ||
| pcl_io_ply$${PCL_RELWITHDEBINFO_SUFFIX}.dll | ||
| pcl_io_ply$${PCL_MINSIZEREL_SUFFIX}.dll | ||
| HINTS "${PCL_ROOT}/bin" | ||
| NO_DEFAULT_PATH) | ||
|
|
||
| find_file(PCL_${COMPONENT}_DLL_PATH_DEBUG | ||
| NAMES ${pcl_component}${PCL_DEBUG_SUFFIX}.dll | ||
| HINTS "${PCL_ROOT}/bin" | ||
| NO_DEFAULT_PATH) | ||
|
|
||
| endif() | ||
|
|
There was a problem hiding this comment.
The special-casing for the io component looks incorrect and currently breaks the DLL name patterns: pcl_io_ply$${PCL_RELEASE_SUFFIX}.dll includes an extra $ (will expand to a literal $ plus the suffix), and the debug search still uses ${pcl_component}${PCL_DEBUG_SUFFIX}.dll (i.e., pcl_io...) instead of pcl_io_ply.... Also, since io_ply is already inserted as its own component (and pcl_io depends on it), this io branch likely shouldn’t override PCL_IO_DLL_PATH at all.
| if ("${component}" STREQUAL "io") | |
| find_file(PCL_${COMPONENT}_DLL_PATH | |
| NAMES | |
| pcl_io_ply$${PCL_RELEASE_SUFFIX}.dll | |
| pcl_io_ply$${PCL_RELWITHDEBINFO_SUFFIX}.dll | |
| pcl_io_ply$${PCL_MINSIZEREL_SUFFIX}.dll | |
| HINTS "${PCL_ROOT}/bin" | |
| NO_DEFAULT_PATH) | |
| find_file(PCL_${COMPONENT}_DLL_PATH_DEBUG | |
| NAMES ${pcl_component}${PCL_DEBUG_SUFFIX}.dll | |
| HINTS "${PCL_ROOT}/bin" | |
| NO_DEFAULT_PATH) | |
| endif() |
| # find dll paths | ||
| find_file(PCL_${COMPONENT}_DLL_PATH | ||
| NAMES | ||
| ${pcl_component}${PCL_RELEASE_SUFFIX}.dll | ||
| ${pcl_component}${PCL_RELWITHDEBINFO_SUFFIX}.dll | ||
| ${pcl_component}${PCL_MINSIZEREL_SUFFIX}.dll | ||
| HINTS "${PCL_ROOT}/bin" | ||
| NO_DEFAULT_PATH) | ||
| find_file(PCL_${COMPONENT}_DLL_PATH_DEBUG | ||
| NAMES ${pcl_component}${PCL_DEBUG_SUFFIX}.dll | ||
| HINTS "${PCL_ROOT}/bin" | ||
| NO_DEFAULT_PATH) |
There was a problem hiding this comment.
The new find_file(PCL_${COMPONENT}_DLL_PATH[_DEBUG] ...) results are not marked advanced like the other find_* cache variables in this config. Consider adding mark_as_advanced(PCL_${COMPONENT}_DLL_PATH PCL_${COMPONENT}_DLL_PATH_DEBUG) to avoid cluttering the CMake cache/UI.
1 participant
Fixes #6345