Skip to content
This repository has been archived by the owner on Mar 21, 2024. It is now read-only.

Fix version checks in CMake packages. #1599

Merged
merged 1 commit into from
Jan 24, 2022

Conversation

alliepiper
Copy link
Collaborator

No description provided.

@alliepiper alliepiper added type: bug: functional Does not work as intended. P0: must have Absolutely necessary. Critical issue, major blocker, etc. only: cmake CMake changes only. Doesn't need internal NVIDIA CI. only: gpuci Changes to gpuCI only. Doesn't need internal NVIDIA CI. helps: rapids Helps or needed by RAPIDS. labels Jan 21, 2022
@alliepiper alliepiper added this to the 1.16.0 milestone Jan 21, 2022
@alliepiper
Copy link
Collaborator Author

run tests

@alliepiper alliepiper added the testing: gpuCI in progress Started gpuCI testing. label Jan 21, 2022
@alliepiper alliepiper merged commit 4bfa8f2 into NVIDIA:main Jan 24, 2022
@alliepiper alliepiper deleted the fix_version_check branch January 24, 2022 19:20
@robertmaynard
Copy link
Collaborator

robertmaynard commented Jan 24, 2022

This set of changes is insufficient for the following example:

list(APPEND CMAKE_PREFIX_PATH <path_to_thrust_1.12> <path_to_thrust_1.15>)
find_package(Thrust REQUIRED)
thrust_create_target(A::Thrust)

find_package(Thrust 1.15 REQUIRED)
thrust_create_target(B::Thrust)

The second find_package(Thrust) call will occur since we have added a VERSION value, but it will use the cached find_package(CUB) call since we don't propagate the requested VERSION argument down.

The naive solution ( as seen below ), resolves this issue:

diff --git a/thrust/cmake/thrust-config.cmake b/thrust/cmake/thrust-config.cmake
index 50e84ce7..ae108319 100644
--- a/thrust/cmake/thrust-config.cmake
+++ b/thrust/cmake/thrust-config.cmake
@@ -497,7 +497,8 @@ endfunction()
 macro(_thrust_find_CUDA required)
   if (NOT TARGET Thrust::CUDA)
     thrust_debug("Searching for CUB ${required}" internal)
-    find_package(CUB CONFIG
+    find_package(CUB ${THRUST_VERSION} CONFIG
       ${_THRUST_QUIET_FLAG}
       ${required}
       NO_DEFAULT_PATH # Only check the explicit HINTS below:

A full reproducer can be found at: https://gist.github.com/robertmaynard/c6a784e7e78d318cba2ae1e71805c618

@alliepiper
Copy link
Collaborator Author

Fixed in #1603

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
helps: rapids Helps or needed by RAPIDS. only: cmake CMake changes only. Doesn't need internal NVIDIA CI. only: gpuci Changes to gpuCI only. Doesn't need internal NVIDIA CI. P0: must have Absolutely necessary. Critical issue, major blocker, etc. testing: gpuCI in progress Started gpuCI testing. type: bug: functional Does not work as intended.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants