More updates needed when porting libcudf

rapidsai · Apr 10, 2023 · ffc1f7c · ffc1f7c
1 parent d60491a
commit ffc1f7c
Show file tree

Hide file tree

Showing 4 changed files with 103 additions and 40 deletions.
diff --git 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
@@ -0,0 +1,63 @@
+diff --git a/cmake/libcudacxxInstallRules.cmake b/cmake/libcudacxxInstallRules.cmake
+index 446ccb50..f44a61f1 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,15 +12,17 @@ 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
+@@ -30,6 +32,7 @@ configure_file("${libcudacxx_SOURCE_DIR}/lib/cmake/libcudacxx/libcudacxx-header-
+   "${libcudacxx_BINARY_DIR}/lib/cmake/libcudacxx/libcudacxx-header-search.cmake"
+   @ONLY
+ )
++set(install_location "${CMAKE_INSTALL_LIBDIR}/cmake/libcudacxx")
+ 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
@@ -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
@@ -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",