Skip to content

Fix searching for dll paths, instead of adding .lib path. #6410

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
larshg wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Fix searching for dll paths, instead of adding .lib path. #6410

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
40dfb73
Fix searching for dll paths, instead of adding .lib path.
larshg Feb 24, 2026
cc05184
Remove dublicated $.
larshg Feb 27, 2026
9b43f0e
Don't special case io_ply as it is inserted as a component earlier.
larshg Feb 27, 2026
f78996d
Address suggestion from copilot.
larshg Feb 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 63 additions & 14 deletions PCLConfig.cmake.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,21 +632,70 @@ foreach(component ${PCL_TO_FIND_COMPONENTS})
endforeach()
if(_is_header_only EQUAL -1)
add_library(${pcl_component} @PCL_LIB_TYPE@ IMPORTED)
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_IMPLIB_RELEASE "${PCL_${COMPONENT}_LIBRARY}"
IMPORTED_IMPLIB_DEBUG "${PCL_${COMPONENT}_LIBRARY_DEBUG}"
)

if(WIN32 AND NOT MINGW)
# On Windows, shared libraries are split into .dll (runtime) and .lib (import library).
# 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)
Comment on lines +640 to +651
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The find_file results for DLL paths are not validated before being used in set_target_properties. If a DLL is not found, the variables PCL_${COMPONENT}DLL_PATH or PCL${COMPONENT}_DLL_PATH_DEBUG will be empty or set to NOTFOUND, which could result in invalid IMPORTED_LOCATION properties.

Consider adding validation after the find_file calls, such as checking if the variables are defined and not empty, and providing appropriate error messages or fallback behavior if the DLLs cannot be found. This is especially important for shared library builds on Windows where the DLLs are required for runtime.

Copilot uses AI. Check for mistakes.
Comment on lines +640 to +651
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The newly introduced DLL path variables PCL_${COMPONENT}DLL_PATH and PCL${COMPONENT}DLL_PATH_DEBUG should be marked as advanced, similar to how other found paths like PCL${COMPONENT}_LIBRARY are marked at line 621. This follows the established pattern in the codebase and prevents these cache variables from cluttering the CMake GUI.

Copilot uses AI. Check for mistakes.

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()

Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The special case for the "io" component appears to be incorrect. When the component is "io", pcl_component is "pcl_io", not "pcl_io_ply". The io_ply is a separate component that gets processed in its own iteration of the loop (as seen in lines 460-462 where io_ply is inserted as a separate component).

This special case should be removed entirely, as the general case at lines 639-649 already correctly uses ${pcl_component} which will be "pcl_io_ply" when processing the io_ply component and "pcl_io" when processing the io component.

Additionally, there's an inconsistency in the debug path search: line 661 searches for ${pcl_component}${PCL_DEBUG_SUFFIX}.dll (which would be pcl_io), but lines 652-658 search for pcl_io_ply DLLs for release builds.

Suggested change
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()

Copilot uses AI. Check for mistakes.
if(PCL_${COMPONENT}_LIBRARY_DEBUG)
set_target_properties(${pcl_component}
PROPERTIES
IMPORTED_CONFIGURATIONS "RELEASE;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}_DLL_PATH}"
IMPORTED_IMPLIB "${PCL_${COMPONENT}_LIBRARY}"
)
endif()
Comment on lines +654 to +684
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code doesn't handle the case where only debug libraries and DLLs are available (analogous to lines 588-590 for .lib files). When only debug is available, PCL_${COMPONENT}LIBRARY_DEBUG will be set, causing the code to enter the dual-configuration branch at line 667. However, PCL${COMPONENT}_DLL_PATH (release) may not be found, leading to an empty or NOTFOUND value being set for IMPORTED_LOCATION_RELEASE.

Consider adding similar fallback logic for DLL paths: if PCL_${COMPONENT}DLL_PATH is not found but PCL${COMPONENT}_DLL_PATH_DEBUG is found, use the debug DLL path as a fallback for release builds on Windows.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DLL search logic should only be applied when PCL is built as shared libraries. When PCL is built as static libraries (PCL_SHARED_LIBS is OFF), there are no DLLs to find, and the code should use the .lib files for IMPORTED_LOCATION instead.

Consider wrapping the DLL search and the Windows-specific set_target_properties calls in a condition that checks PCL_SHARED_LIBS. When PCL_SHARED_LIBS is OFF, the Windows path should behave like the non-Windows path at lines 684-698, using PCL_${COMPONENT}_LIBRARY for IMPORTED_LOCATION.

Copilot uses AI. Check for mistakes.
else()
set_target_properties(${pcl_component}
PROPERTIES
IMPORTED_LOCATION "${PCL_${COMPONENT}_LIBRARY}"
IMPORTED_IMPLIB "${PCL_${COMPONENT}_LIBRARY}"
)
# On Linux/macOS/MINGW, the shared library (.so/.dylib) is both the
# link-time and runtime artifact, so IMPORTED_LOCATION is sufficient.
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}"
)
else()
set_target_properties(${pcl_component}
PROPERTIES
IMPORTED_LOCATION "${PCL_${COMPONENT}_LIBRARY}"
)
endif()
endif()
else() # header-only
add_library(${pcl_component} INTERFACE IMPORTED)
Expand Down
Loading