From cfd5f722c5d77c6f99c6dc15132be1b9aabcd2c7 Mon Sep 17 00:00:00 2001 From: Robert Maynard Date: Mon, 10 Apr 2023 14:54:14 -0400 Subject: [PATCH] More updates needed when porting libcudf --- rapids-cmake/cpm/libcudacxx.cmake | 65 ++++++++---------- .../cpm/patches/libcudacxx/install_rules.diff | 66 +++++++++++++++++++ rapids-cmake/cpm/thrust.cmake | 6 +- rapids-cmake/cpm/versions.json | 9 ++- 4 files changed, 106 insertions(+), 40 deletions(-) create mode 100644 rapids-cmake/cpm/patches/libcudacxx/install_rules.diff diff --git a/rapids-cmake/cpm/libcudacxx.cmake b/rapids-cmake/cpm/libcudacxx.cmake index 525693beb..64805f86b 100644 --- a/rapids-cmake/cpm/libcudacxx.cmake +++ b/rapids-cmake/cpm/libcudacxx.cmake @@ -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) @@ -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) @@ -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 diff --git a/rapids-cmake/cpm/patches/libcudacxx/install_rules.diff b/rapids-cmake/cpm/patches/libcudacxx/install_rules.diff new file mode 100644 index 000000000..b2c7f2a30 --- /dev/null +++ b/rapids-cmake/cpm/patches/libcudacxx/install_rules.diff @@ -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) diff --git a/rapids-cmake/cpm/thrust.cmake b/rapids-cmake/cpm/thrust.cmake index aa2ef1b0b..0743b00ef 100644 --- a/rapids-cmake/cpm/thrust.cmake +++ b/rapids-cmake/cpm/thrust.cmake @@ -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) @@ -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) diff --git a/rapids-cmake/cpm/versions.json b/rapids-cmake/cpm/versions.json index 15fbcc360..268d7d892 100644 --- a/rapids-cmake/cpm/versions.json +++ b/rapids-cmake/cpm/versions.json @@ -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",