Fix searching for dll paths, instead of adding .lib path. by larshg · Pull Request #6410 · PointCloudLibrary/pcl

larshg · 2026-02-24T16:16:42Z

Copilot

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 .lib paths in IMPORTED_IMPLIB[_<CONFIG>].
Introduces a special-case branch for the io component DLL naming.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

PCLConfig.cmake.in

Wrap dll search for windows in "if WIN32".

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-27T08:19:50Z

PCLConfig.cmake.in

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


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 · 2026-02-27T08:19:50Z

PCLConfig.cmake.in

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


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 · 2026-02-27T08:19:51Z

PCLConfig.cmake.in

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


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 · 2026-02-27T08:19:51Z

PCLConfig.cmake.in

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


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 · 2026-02-27T08:19:51Z

PCLConfig.cmake.in

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


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.

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

40dfb73

larshg added module: cmake changelog: fix Meta-information for changelog generation labels Feb 24, 2026

larshg requested a review from Copilot February 24, 2026 16:26

larshg added this to the pcl-1.16.0 milestone Feb 24, 2026

Copilot started reviewing on behalf of larshg February 24, 2026 16:27 View session

Copilot AI reviewed Feb 24, 2026

View reviewed changes

PCLConfig.cmake.in Outdated Show resolved Hide resolved

PCLConfig.cmake.in Outdated Show resolved Hide resolved

PCLConfig.cmake.in Outdated Show resolved Hide resolved

Remove dublicated $.

cc05184

Wrap dll search for windows in "if WIN32".

larshg requested a review from Copilot February 27, 2026 08:15

Copilot started reviewing on behalf of larshg February 27, 2026 08:15 View session

Copilot AI reviewed Feb 27, 2026

View reviewed changes

larshg added 2 commits February 27, 2026 10:27

Don't special case io_ply as it is inserted as a component earlier.

9b43f0e

Address suggestion from copilot.

f78996d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

Fix searching for dll paths, instead of adding .lib path.#6410
larshg wants to merge 4 commits intoPointCloudLibrary:masterfrom
larshg:fixdllpath

larshg commented Feb 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

larshg commented Feb 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants