Skip to content

Commit

Permalink
More updates needed when porting libcudf
Browse files Browse the repository at this point in the history
  • Loading branch information
robertmaynard committed Apr 10, 2023
1 parent d60491a commit cfd5f72
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 40 deletions.
65 changes: 29 additions & 36 deletions rapids-cmake/cpm/libcudacxx.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,32 @@ function(rapids_cpm_libcudacxx)
include("${rapids-cmake-dir}/cpm/detail/package_details.cmake")
rapids_cpm_package_details(libcudacxx version repository tag shallow exclude)

set(to_install OFF)
if(INSTALL_EXPORT_SET IN_LIST ARGN AND NOT exclude)
set(to_install ON)
# By default if we allow libcudacxx to install into `CMAKE_INSTALL_INCLUDEDIR` alongside rmm (or
# other packages) we will get a install tree that looks like this:

# install/include/rmm install/include/cub install/include/libcudacxx

# This is a problem for CMake+NVCC due to the rules around import targets, and user/system
# includes. In this case both rmm and libcudacxx will specify an include path of
# `install/include`, while libcudacxx tries to mark it as an user include, since rmm uses
# CMake's default of system include. Compilers when provided the same include as both user and
# system always goes with system.

# Now while rmm could also mark `install/include` as system this just pushes the issue to
# another dependency which isn't built by RAPIDS and comes by and marks `install/include` as
# system.

# Instead the more reliable option is to make sure that we get libcudacxx to be placed in an
# unique include path that the other project will use. In the case of rapids-cmake we install
# the headers to `include/rapids/libcudacxx`
include(GNUInstallDirs)
set(CMAKE_INSTALL_INCLUDEDIR "${CMAKE_INSTALL_INCLUDEDIR}/rapids/libcudacxx")
set(CMAKE_INSTALL_LIBDIR "${CMAKE_INSTALL_LIBDIR}/rapids/")
endif()

include("${rapids-cmake-dir}/cpm/detail/generate_patch_command.cmake")
rapids_cpm_generate_patch_command(libcudacxx ${version} patch_command)

Expand All @@ -65,7 +91,8 @@ function(rapids_cpm_libcudacxx)
GIT_TAG ${tag}
GIT_SHALLOW ${shallow}
PATCH_COMMAND ${patch_command}
EXCLUDE_FROM_ALL ${exclude})
EXCLUDE_FROM_ALL ${exclude}
OPTIONS "libcudacxx_ENABLE_INSTALL_RULES ${to_install}")

include("${rapids-cmake-dir}/cpm/detail/display_patch_status.cmake")
rapids_cpm_display_patch_status(libcudacxx)
Expand All @@ -82,45 +109,11 @@ function(rapids_cpm_libcudacxx)
${_RAPIDS_BUILD_EXPORT_SET})
endif()

if(libcudacxx_SOURCE_DIR AND _RAPIDS_INSTALL_EXPORT_SET AND NOT exclude)
# By default if we allow libcudacxx to install into `CMAKE_INSTALL_INCLUDEDIR` alongside rmm (or
# other packages) we will get a install tree that looks like this:

# install/include/rmm install/include/cub install/include/libcudacxx

# This is a problem for CMake+NVCC due to the rules around import targets, and user/system
# includes. In this case both rmm and libcudacxx will specify an include path of
# `install/include`, while libcudacxx tries to mark it as an user include, since rmm uses
# CMake's default of system include. Compilers when provided the same include as both user and
# system always goes with system.

# Now while rmm could also mark `install/include` as system this just pushes the issue to
# another dependency which isn't built by RAPIDS and comes by and marks `install/include` as
# system.

# Instead the more reliable option is to make sure that we get libcudacxx to be placed in an
# unique include path that the other project will use. In the case of rapids-cmake we install
# the headers to `include/rapids/libcudacxx`
include(GNUInstallDirs)
set(CMAKE_INSTALL_INCLUDEDIR "${CMAKE_INSTALL_INCLUDEDIR}/rapids/libcudacxx")
set(CMAKE_INSTALL_LIBDIR "${CMAKE_INSTALL_LIBDIR}/rapids/")

if(to_install)
include("${rapids-cmake-dir}/export/find_package_root.cmake")
rapids_export_find_package_root(INSTALL libcudacxx
[=[${CMAKE_CURRENT_LIST_DIR}/../../rapids/cmake/libcudacxx]=]
${_RAPIDS_INSTALL_EXPORT_SET})

# libcudacxx 1.8 has a bug where it doesn't generate proper exclude rules for the
# `[cub|libcudacxx]-header-search` files, which causes the build tree version to be installed
# instead of the install version
if(NOT EXISTS "${libcudacxx_BINARY_DIR}/cmake/libcudacxxInstallRulesForRapids.cmake")
file(READ "${libcudacxx_SOURCE_DIR}/cmake/libcudacxxInstallRules.cmake" contents)
string(REPLACE "PATTERN libcudacxx-header-search EXCLUDE"
"REGEX libcudacxx-header-search.* EXCLUDE" contents "${contents}")
file(WRITE "${libcudacxx_BINARY_DIR}/cmake/libcudacxxInstallRulesForRapids.cmake" ${contents})
endif()
set(libcudacxx_ENABLE_INSTALL_RULES ON)
include("${libcudacxx_BINARY_DIR}/cmake/libcudacxxInstallRulesForRapids.cmake")
endif()

# Propagate up variables that CPMFindPackage provide
Expand Down
66 changes: 66 additions & 0 deletions rapids-cmake/cpm/patches/libcudacxx/install_rules.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
diff --git a/cmake/libcudacxxInstallRules.cmake b/cmake/libcudacxxInstallRules.cmake
index 446ccb50..ff622bb7 100644
--- a/cmake/libcudacxxInstallRules.cmake
+++ b/cmake/libcudacxxInstallRules.cmake
@@ -1,5 +1,5 @@
option(libcudacxx_ENABLE_INSTALL_RULES
- "Enable installation of libcudacxx" ${libcudacxx_TOPLEVEL_PROJECT}
+ "Enable installation of libcudacxx" ${LIBCUDACXX_TOPLEVEL_PROJECT}
)

if (NOT libcudacxx_ENABLE_INSTALL_RULES)
@@ -12,24 +12,27 @@ include(GNUInstallDirs)
# Libcudacxx headers
install(DIRECTORY "${libcudacxx_SOURCE_DIR}/include/cuda"
DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}"
+ PATTERN CMakeLists.txt EXCLUDE
)
install(DIRECTORY "${libcudacxx_SOURCE_DIR}/include/nv"
DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}"
+ PATTERN CMakeLists.txt EXCLUDE
)

# Libcudacxx cmake package
install(DIRECTORY "${libcudacxx_SOURCE_DIR}/lib/cmake/libcudacxx"
DESTINATION "${CMAKE_INSTALL_LIBDIR}/cmake"
- PATTERN libcudacxx-header-search EXCLUDE
+ PATTERN *.cmake.in EXCLUDE
)

# Need to configure a file to store CMAKE_INSTALL_INCLUDEDIR
# since it can be defined by the user. This is common to work around collisions
# with the CTK installed headers.
+set(install_location "${CMAKE_INSTALL_LIBDIR}/cmake/libcudacxx")
configure_file("${libcudacxx_SOURCE_DIR}/lib/cmake/libcudacxx/libcudacxx-header-search.cmake.in"
"${libcudacxx_BINARY_DIR}/lib/cmake/libcudacxx/libcudacxx-header-search.cmake"
@ONLY
)
install(FILES "${libcudacxx_BINARY_DIR}/lib/cmake/libcudacxx/libcudacxx-header-search.cmake"
- DESTINATION "${CMAKE_INSTALL_LIBDIR}/cmake/libcudacxx"
+ DESTINATION "${install_location}"
)

diff --git a/lib/cmake/libcudacxx/libcudacxx-header-search.cmake.in b/lib/cmake/libcudacxx/libcudacxx-header-search.cmake.in
index 9e7e187c..cb3b946f 100644
--- a/lib/cmake/libcudacxx/libcudacxx-header-search.cmake.in
+++ b/lib/cmake/libcudacxx/libcudacxx-header-search.cmake.in
@@ -1,8 +1,18 @@
# Parse version information from version header:
unset(_libcudacxx_VERSION_INCLUDE_DIR CACHE) # Clear old result to force search
+
+# Find CMAKE_INSTALL_INCLUDEDIR=@CMAKE_INSTALL_INCLUDEDIR@ directory"
+set(from_install_prefix "@install_location@")
+
+# Transform to a list of directories, replace each directory with "../"
+# and convert back to a string
+string(REGEX REPLACE "/" ";" from_install_prefix "${from_install_prefix}")
+list(TRANSFORM from_install_prefix REPLACE ".+" "../")
+list(JOIN from_install_prefix "" from_install_prefix)
+
find_path(_libcudacxx_VERSION_INCLUDE_DIR cuda/std/detail/__config
NO_DEFAULT_PATH # Only search explicit paths below:
PATHS
- "${CMAKE_CURRENT_LIST_DIR}/../../../@CMAKE_INSTALL_INCLUDEDIR@" # Install tree
+ "${CMAKE_CURRENT_LIST_DIR}/${from_install_prefix}/@CMAKE_INSTALL_INCLUDEDIR@" # Install tree
)
set_property(CACHE _libcudacxx_VERSION_INCLUDE_DIR PROPERTY TYPE INTERNAL)
6 changes: 3 additions & 3 deletions rapids-cmake/cpm/thrust.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ Result Variables
function(rapids_cpm_thrust NAMESPACE namespaces_name)
list(APPEND CMAKE_MESSAGE_CONTEXT "rapids.cpm.thrust")

include("${rapids-cmake-dir}/cpm/detail/package_details.cmake")
rapids_cpm_package_details(Thrust version repository tag shallow exclude)

set(to_install OFF)
if(INSTALL_EXPORT_SET IN_LIST ARGN)
set(to_install ON)
Expand All @@ -65,9 +68,6 @@ function(rapids_cpm_thrust NAMESPACE namespaces_name)
set(CMAKE_INSTALL_LIBDIR "${CMAKE_INSTALL_LIBDIR}/rapids")
endif()

include("${rapids-cmake-dir}/cpm/detail/package_details.cmake")
rapids_cpm_package_details(Thrust version repository tag shallow exclude)

include("${rapids-cmake-dir}/cpm/detail/generate_patch_command.cmake")
rapids_cpm_generate_patch_command(Thrust ${version} patch_command)

Expand Down
9 changes: 8 additions & 1 deletion rapids-cmake/cpm/versions.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,14 @@
"libcudacxx" : {
"version" : "1.9.1",
"git_url" : "https://github.com/NVIDIA/libcudacxx.git",
"git_tag" : "branch/${version}"
"git_tag" : "branch/${version}",
"patches" : [
{
"file" : "libcudacxx/install_rules.diff",
"issue" : "libcudacxx 1.X installs incorrect files",
"fixed_in" : "2.0"
}
]
},
"nvbench" : {
"version" : "0.0",
Expand Down

0 comments on commit cfd5f72

Please sign in to comment.