Skip to content

Commit

Permalink
Thrust when exported now automatically calls thrust_create_target (#…
Browse files Browse the repository at this point in the history
…467)

Instead of having each consuming project inject a code block into the `project-config.cmake` we instead have rapids-cmake call `thrust_create_target` right after `find_package(Thrust)`, only when thrust was found.

This fixes two existing issues:
  1. It removes the need for `rapids_cpm_thrust` to remove the the custom Thrust target from the global target set. Now that target will exist when rapids-cmake tries to promote it to global
  2. It removes issues when multiple calls to `find_package(thrust)` occur before any calls to `thrust_create_target`. This creates some vexing issues when the thrust find calls resolve to different major versions of thrust

Authors:
  - Robert Maynard (https://github.com/robertmaynard)

Approvers:
  - Bradley Dice (https://github.com/bdice)
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: #467
  • Loading branch information
robertmaynard authored Oct 9, 2023
1 parent 9214ee3 commit 3bc1394
Show file tree
Hide file tree
Showing 17 changed files with 393 additions and 69 deletions.
12 changes: 10 additions & 2 deletions cmake-format-rapids-cmake.json
Original file line number Diff line number Diff line change
Expand Up @@ -230,14 +230,22 @@
},
"rapids_export_find_package_file": {
"pargs": {
"nargs": 3,
"nargs": "3+",
"flags": ["INSTALL", "BUILD"]
},
"kwargs": {
"EXPORT_SET": 1,
"CONDITION": 1
}
},
"rapids_export_find_package_root": {
"pargs": {
"nargs": 4,
"nargs": "3+",
"flags": ["INSTALL", "BUILD"]
},
"kwargs": {
"EXPORT_SET": 1,
"CONDITION": 1
}
},
"rapids_export_package": {
Expand Down
12 changes: 4 additions & 8 deletions rapids-cmake/cpm/libcudacxx.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -101,18 +101,14 @@ function(rapids_cpm_libcudacxx)
set(multi_value)
cmake_parse_arguments(_RAPIDS "${options}" "${one_value}" "${multi_value}" ${ARGN})

if(libcudacxx_SOURCE_DIR AND _RAPIDS_BUILD_EXPORT_SET)
if(libcudacxx_SOURCE_DIR)
# Store where CMake can find our custom libcudacxx
include("${rapids-cmake-dir}/export/find_package_root.cmake")
rapids_export_find_package_root(BUILD libcudacxx "${libcudacxx_SOURCE_DIR}/lib/cmake"
${_RAPIDS_BUILD_EXPORT_SET})
endif()

if(libcudacxx_SOURCE_DIR AND to_install)
include("${rapids-cmake-dir}/export/find_package_root.cmake")
rapids_export_find_package_root(BUILD libcudacxx "${libcudacxx_SOURCE_DIR}/lib/cmake" FOR
EXPORT_SET ${_RAPIDS_BUILD_EXPORT_SET})
rapids_export_find_package_root(INSTALL libcudacxx
[=[${CMAKE_CURRENT_LIST_DIR}/../../rapids/cmake/libcudacxx]=]
${_RAPIDS_INSTALL_EXPORT_SET})
EXPORT_SET ${_RAPIDS_INSTALL_EXPORT_SET} CONDITION to_install)
endif()

# Propagate up variables that CPMFindPackage provide
Expand Down
8 changes: 4 additions & 4 deletions rapids-cmake/cpm/nvcomp.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -173,9 +173,9 @@ function(rapids_cpm_nvcomp)
install(FILES "${nvcomp_ROOT}/LICENSE" DESTINATION info/ RENAME NVCOMP_LICENSE)
endif()

if(_RAPIDS_BUILD_EXPORT_SET AND nvcomp_proprietary_binary)
# point our consumers to where they can find the pre-built version
rapids_export_find_package_root(BUILD nvcomp "${nvcomp_ROOT}" ${_RAPIDS_BUILD_EXPORT_SET})
endif()
# point our consumers to where they can find the pre-built version
rapids_export_find_package_root(BUILD nvcomp "${nvcomp_ROOT}"
EXPORT_SET ${_RAPIDS_BUILD_EXPORT_SET}
CONDITION nvcomp_proprietary_binary)

endfunction()
44 changes: 18 additions & 26 deletions rapids-cmake/cpm/thrust.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,13 @@ across all RAPIDS projects.
.. |PKG_NAME| replace:: Thrust
.. include:: common_package_args.txt
.. versionadded:: v23.12.00
When `BUILD_EXPORT_SET` is specified the generated build export set dependency
file will automatically call `thrust_create_target(<namespace>::Thrust FROM_OPTIONS)`.
When `INSTALL_EXPORT_SET` is specified the generated install export set dependency
file will automatically call `thrust_create_target(<namespace>::Thrust FROM_OPTIONS)`.
Result Targets
^^^^^^^^^^^^^^
<namespace>::Thrust target will be created
Expand Down Expand Up @@ -90,18 +97,23 @@ function(rapids_cpm_thrust NAMESPACE namespaces_name)
set(multi_value)
cmake_parse_arguments(_RAPIDS "${options}" "${one_value}" "${multi_value}" ${ARGN})

if(Thrust_SOURCE_DIR AND _RAPIDS_BUILD_EXPORT_SET)
set(post_find_code "if(NOT TARGET ${namespaces_name}::Thrust)"
" thrust_create_target(${namespaces_name}::Thrust FROM_OPTIONS)" "endif()")

if(Thrust_SOURCE_DIR)
# Store where CMake can find the Thrust-config.cmake that comes part of Thrust source code
include("${rapids-cmake-dir}/export/find_package_root.cmake")
include("${rapids-cmake-dir}/export/detail/post_find_package_code.cmake")
rapids_export_find_package_root(BUILD Thrust "${Thrust_SOURCE_DIR}/cmake"
${_RAPIDS_BUILD_EXPORT_SET})
endif()
EXPORT_SET ${_RAPIDS_BUILD_EXPORT_SET})
rapids_export_post_find_package_code(BUILD Thrust "${post_find_code}" EXPORT_SET
${_RAPIDS_BUILD_EXPORT_SET})

if(Thrust_SOURCE_DIR AND _RAPIDS_INSTALL_EXPORT_SET AND to_install)
include("${rapids-cmake-dir}/export/find_package_root.cmake")
rapids_export_find_package_root(INSTALL Thrust
[=[${CMAKE_CURRENT_LIST_DIR}/../../rapids/cmake/thrust]=]
${_RAPIDS_INSTALL_EXPORT_SET})
EXPORT_SET ${_RAPIDS_INSTALL_EXPORT_SET} CONDITION to_install)
rapids_export_post_find_package_code(INSTALL Thrust "${post_find_code}" EXPORT_SET
${_RAPIDS_INSTALL_EXPORT_SET} CONDITION to_install)
endif()

# Check for the existence of thrust_create_target so we support fetching Thrust with DOWNLOAD_ONLY
Expand All @@ -113,26 +125,6 @@ function(rapids_cpm_thrust NAMESPACE namespaces_name)
endif()
endif()

# Since `GLOBAL_TARGET ${namespaces_name}::Thrust` will list the target to be promoted to global
# by `rapids_export` this will break consumers as the target doesn't exist when generating the
# dependencies.cmake file, but requires a call to `thrust_create_target`
#
# So determine what `BUILD_EXPORT_SET` and `INSTALL_EXPORT_SET` this was added to and remove
# ${namespaces_name}::Thrust
if(_RAPIDS_BUILD_EXPORT_SET)
set(target_name rapids_export_build_${_RAPIDS_BUILD_EXPORT_SET})
get_target_property(global_targets ${target_name} GLOBAL_TARGETS)
list(REMOVE_ITEM global_targets "${namespaces_name}::Thrust")
set_target_properties(${target_name} PROPERTIES GLOBAL_TARGETS "${global_targets}")
endif()

if(_RAPIDS_INSTALL_EXPORT_SET)
set(target_name rapids_export_install_${_RAPIDS_INSTALL_EXPORT_SET})
get_target_property(global_targets ${target_name} GLOBAL_TARGETS)
list(REMOVE_ITEM global_targets "${namespaces_name}::Thrust")
set_target_properties(${target_name} PROPERTIES GLOBAL_TARGETS "${global_targets}")
endif()

# Propagate up variables that CPMFindPackage provide
set(Thrust_SOURCE_DIR "${Thrust_SOURCE_DIR}" PARENT_SCOPE)
set(Thrust_BINARY_DIR "${Thrust_BINARY_DIR}" PARENT_SCOPE)
Expand Down
84 changes: 84 additions & 0 deletions rapids-cmake/export/detail/post_find_package_code.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
#=============================================================================
# Copyright (c) 2023, NVIDIA CORPORATION.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#=============================================================================
include_guard(GLOBAL)

#[=======================================================================[.rst:
rapids_export_post_find_package_code
------------------------------------
.. versionadded:: v23.12.00
Record for <PackageName> a set of CMake instructions to be executed after the package
has been found successfully.
.. code-block:: cmake
rapids_export_post_find_package_code((BUILD|INSTALL)
<PackageName>
<code>
EXPORT_SET [ExportSetName]
[CONDITION <variableName>]
)
When using complicated find modules like `Thrust` you might need to run some code after
execution. Multiple calls to :cmake:command:`rapids_export_post_find_package_code` will append the
instructions to execute in call order.
.. note:
The code will only be run if the package was found
``BUILD``
Record code to be executed immediately after `PackageName` has been found
for our our build directory export set.
``INSTALL``
Record code to be executed immediately after `PackageName` has been found
for our our install directory export set.
``EXPORT_SET``
List the export set name that this code should be attached too. If
no name is given the associated call will be ignored.
``CONDITION``
A boolean variable name, that when evaluates to undefined or a false value
will cause the associated call to be ignored.
#]=======================================================================]
function(rapids_export_post_find_package_code type name code)
list(APPEND CMAKE_MESSAGE_CONTEXT "rapids.export.post_find_package_code")

set(options "")
set(one_value EXPORT_SET CONDITION)
set(multi_value "")
cmake_parse_arguments(_RAPIDS "${options}" "${one_value}" "${multi_value}" ${ARGN})
# Early terminate conditions
if(NOT _RAPIDS_EXPORT_SET OR NOT ${_RAPIDS_CONDITION})
return()
endif()

string(TOLOWER ${type} type)
set(export_set ${_RAPIDS_EXPORT_SET})

if(NOT TARGET rapids_export_${type}_${export_set})
add_library(rapids_export_${type}_${export_set} INTERFACE)
endif()

# if the code coming in is a list of string we will have `;`, so transform those to "\n" so we
# have a single string
string(REPLACE ";" "\n" code "${code}")
set_property(TARGET rapids_export_${type}_${export_set} APPEND_STRING
PROPERTY "${name}_POST_FIND_CODE" "${code}\n")
endfunction()
34 changes: 31 additions & 3 deletions rapids-cmake/export/find_package_file.cmake
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#=============================================================================
# Copyright (c) 2021, NVIDIA CORPORATION.
# Copyright (c) 2021-2023, NVIDIA CORPORATION.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -28,7 +28,8 @@ the associated export set.
rapids_export_find_package_file( (BUILD|INSTALL)
<file_path>
<ExportSet>
(<ExportSetName> | EXPORT_SET [ExportSetName])
[CONDITION <variableName>]
)
When constructing export sets, espically installed ones it is
Expand All @@ -50,11 +51,38 @@ rapids-cmake to ensure it is installed correct and added to
of our install export set. This means that it will be installed as
part of our packages CMake export set infrastructure
``EXPORT_SET``
List the export set name that this code should be attached too. If
no name is given the associated call will be ignored.
``CONDITION``
A boolean variable name, that when evaluates to undefined or a false value
will cause the associated call to be ignored.
#]=======================================================================]
function(rapids_export_find_package_file type file_path export_set)
function(rapids_export_find_package_file type file_path)
list(APPEND CMAKE_MESSAGE_CONTEXT "rapids.export.find_package_file")
include("${rapids-cmake-dir}/cmake/detail/policy.cmake")

set(options "")
set(one_value EXPORT_SET CONDITION)
set(multi_value "")
cmake_parse_arguments(_RAPIDS "${options}" "${one_value}" "${multi_value}" ${ARGN})
# handle when we are given just an export set name and not `EXPORT_SET <name>`
if(_RAPIDS_UNPARSED_ARGUMENTS AND NOT _RAPIDS_COMPONENTS_EXPORT_SET)
rapids_cmake_policy(DEPRECATED_IN 23.12
REMOVED_IN 24.02
MESSAGE [=[Usage of `rapids_export_find_package_file` without an explicit `EXPORT_SET` key has been deprecated.]=]
)
set(_RAPIDS_EXPORT_SET ${_RAPIDS_UNPARSED_ARGUMENTS})
endif()
# Early terminate conditions
if(NOT _RAPIDS_EXPORT_SET OR NOT ${_RAPIDS_CONDITION})
return()
endif()

string(TOLOWER ${type} type)
set(export_set ${_RAPIDS_EXPORT_SET})

if(NOT TARGET rapids_export_${type}_${export_set})
add_library(rapids_export_${type}_${export_set} INTERFACE)
Expand Down
34 changes: 31 additions & 3 deletions rapids-cmake/export/find_package_root.cmake
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#=============================================================================
# Copyright (c) 2021, NVIDIA CORPORATION.
# Copyright (c) 2021-2023, NVIDIA CORPORATION.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -29,7 +29,8 @@ needs to be set to the provided path.
rapids_export_find_package_root( (BUILD|INSTALL)
<PackageName>
<directory_path>
<ExportSet>
(<ExportSetName> | EXPORT_SET [ExportSetName])
[CONDITION <variableName>]
)
When constructing complicated export sets, espically ones that
Expand All @@ -47,11 +48,38 @@ will find the packaged dependency.
before any find_dependency calls for `PackageName` for our install directory
export set.
``EXPORT_SET``
List the export set name that the `directory_path` should be attached too. If
no name is given the associated call will be ignored.
``CONDITION``
A boolean variable name, that when evaluates to undefined or a false value
will cause the associated call to be ignored.
#]=======================================================================]
function(rapids_export_find_package_root type name dir_path export_set)
function(rapids_export_find_package_root type name dir_path)
list(APPEND CMAKE_MESSAGE_CONTEXT "rapids.export.find_package_root_dir")
include("${rapids-cmake-dir}/cmake/detail/policy.cmake")

set(options "")
set(one_value EXPORT_SET CONDITION)
set(multi_value "")
cmake_parse_arguments(_RAPIDS "${options}" "${one_value}" "${multi_value}" ${ARGN})
# handle when we are given just an export set name and not `EXPORT_SET <name>`
if(_RAPIDS_UNPARSED_ARGUMENTS AND NOT _RAPIDS_COMPONENTS_EXPORT_SET)
rapids_cmake_policy(DEPRECATED_IN 23.12
REMOVED_IN 24.02
MESSAGE [=[Usage of `rapids_export_find_package_root` without an explicit `EXPORT_SET` key has been deprecated.]=]
)
set(_RAPIDS_EXPORT_SET ${_RAPIDS_UNPARSED_ARGUMENTS})
endif()
# Early terminate conditions
if(NOT _RAPIDS_EXPORT_SET OR NOT ${_RAPIDS_CONDITION})
return()
endif()

string(TOLOWER ${type} type)
set(export_set ${_RAPIDS_EXPORT_SET})

if(NOT TARGET rapids_export_${type}_${export_set})
add_library(rapids_export_${type}_${export_set} INTERFACE)
Expand Down
21 changes: 13 additions & 8 deletions rapids-cmake/export/write_dependencies.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ a given export_set for the requested mode.
CMake config module.
#]=======================================================================]
# cmake-lint: disable=R0915
function(rapids_export_write_dependencies type export_set file_path)
list(APPEND CMAKE_MESSAGE_CONTEXT "rapids.export.write_dependencies")

Expand Down Expand Up @@ -88,14 +89,12 @@ endif()\n")
endif()
endif()

if(find_root_dirs)
foreach(package IN LISTS find_root_dirs)
get_property(root_dir_path TARGET rapids_export_${type}_${export_set}
PROPERTY "FIND_ROOT_FOR_${package}")
set(dep_content "set(${package}_ROOT \"${root_dir_path}\")")
string(APPEND _RAPIDS_EXPORT_CONTENTS "${dep_content}\n")
endforeach()
endif()
foreach(package IN LISTS find_root_dirs)
get_property(root_dir_path TARGET rapids_export_${type}_${export_set}
PROPERTY "FIND_ROOT_FOR_${package}")
set(dep_content "set(${package}_ROOT \"${root_dir_path}\")")
string(APPEND _RAPIDS_EXPORT_CONTENTS "${dep_content}\n")
endforeach()

if(find_modules)
cmake_path(GET file_path PARENT_PATH find_module_dest)
Expand All @@ -116,6 +115,12 @@ endif()\n")
file(READ "${dep_dir}/package_${dep}.cmake" dep_content)
endif()
string(APPEND _RAPIDS_EXPORT_CONTENTS "${dep_content}\n")

get_property(post_find_code TARGET rapids_export_${type}_${export_set}
PROPERTY "${dep}_POST_FIND_CODE")
if(post_find_code)
string(APPEND _RAPIDS_EXPORT_CONTENTS "if(${dep}_FOUND)\n${post_find_code}\nendif()\n")
endif()
endforeach()
string(APPEND _RAPIDS_EXPORT_CONTENTS "\n")

Expand Down
1 change: 1 addition & 0 deletions testing/cpm/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,4 @@ add_cmake_config_test( cpm_spdlog-simple.cmake )

add_cmake_config_test( cpm_thrust-export.cmake )
add_cmake_config_test( cpm_thrust-simple.cmake )
add_cmake_build_test( cpm_thrust-verify-post-find-code )
12 changes: 1 addition & 11 deletions testing/cpm/cpm_thrust-export.cmake
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#=============================================================================
# Copyright (c) 2021, NVIDIA CORPORATION.
# Copyright (c) 2021-2023, NVIDIA CORPORATION.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -30,13 +30,3 @@ get_target_property(packages rapids_export_install_test2 PACKAGE_NAMES)
if(NOT Thrust IN_LIST packages)
message(FATAL_ERROR "rapids_cpm_thrust failed to record thrust needs to be exported")
endif()

get_target_property(packages rapids_export_build_test GLOBAL_TARGETS)
if(A::Thrust IN_LIST packages)
message(FATAL_ERROR "rapids_cpm_thrust incorrectly added A::Thrust to build GLOBAL_TARGETS")
endif()

get_target_property(packages rapids_export_install_test2 GLOBAL_TARGETS)
if(B::Thrust IN_LIST packages)
message(FATAL_ERROR "rapids_cpm_thrust incorrectly added B::Thrust to install GLOBAL_TARGETS")
endif()
Loading

0 comments on commit 3bc1394

Please sign in to comment.