Separate libucxx build and ucxx build (#257)

Rather than building `libucxx_python.so` in the `libucxx` build, and transferring it to the `ucxx` package, build it in the `ucxx` build. Nothing other than the Python wheel is currently using `ucxx::python`, so stop exporting it and installing the headers as well. Authors: - Kyle Edwards (https://github.com/KyleFromNVIDIA) Approvers: - Peter Andreas Entschev (https://github.com/pentschev) - James Lamb (https://github.com/jameslamb) URL: #257
rapidsai · Aug 6, 2024 · 357b0b8 · 357b0b8
1 parent c6df39c
commit 357b0b8
Show file tree

Hide file tree

Showing 6 changed files with 132 additions and 67 deletions.
diff --git a/build.sh b/build.sh
@@ -40,9 +40,10 @@ HELP="$0 [clean] [libucxx] [libucxx_python] [ucxx] [distributed_ucxx] [benchmark
    default action (no args) is to build and install 'libucxx' and 'libucxx_python', then 'ucxx' targets, and finally 'distributed_ucxx'
 "
 LIB_BUILD_DIR=${LIB_BUILD_DIR:=${REPODIR}/cpp/build}
+PYTHON_BUILD_DIR=${PYTHON_BUILD_DIR:=${REPODIR}/cpp/python/build}
 UCXX_BUILD_DIR=${REPODIR}/python/ucxx/build
 
-BUILD_DIRS="${LIB_BUILD_DIR} ${UCXX_BUILD_DIR}"
+BUILD_DIRS="${LIB_BUILD_DIR} ${PYTHON_BUILD_DIR} ${UCXX_BUILD_DIR}"
 
 # Set defaults for vars modified by flags to this script
 VERBOSE_FLAG=""
@@ -53,7 +54,6 @@ BUILD_TESTS=OFF
 BUILD_EXAMPLES=OFF
 BUILD_DISABLE_DEPRECATION_WARNINGS=ON
 BUILD_COMPILE_COMMANDS=OFF
-UCXX_ENABLE_PYTHON=OFF
 UCXX_ENABLE_RMM=OFF
 
 # Set defaults for vars that may not have been defined externally
@@ -139,7 +139,6 @@ if hasArg --show_depr_warn; then
 fi
 
 if buildAll || hasArg libucxx_python; then
-  UCXX_ENABLE_PYTHON=ON
   UCXX_ENABLE_RMM=ON
 fi
 
@@ -168,7 +167,6 @@ fi
 
 if buildAll || hasArg libucxx; then
     CMAKE_GENERATOR="${CMAKE_GENERATOR:-Ninja}"
-    pwd
     cmake -S $REPODIR/cpp -B ${LIB_BUILD_DIR} \
           -G${CMAKE_GENERATOR} \
           -DCMAKE_INSTALL_PREFIX=${INSTALL_PREFIX} \
@@ -178,26 +176,19 @@ if buildAll || hasArg libucxx; then
           -DDISABLE_DEPRECATION_WARNINGS=${BUILD_DISABLE_DEPRECATION_WARNINGS} \
           -DCMAKE_BUILD_TYPE=${BUILD_TYPE} \
           -DCMAKE_EXPORT_COMPILE_COMMANDS=${BUILD_COMPILE_COMMANDS} \
-          -DUCXX_ENABLE_PYTHON=${UCXX_ENABLE_PYTHON} \
           -DUCXX_ENABLE_RMM=${UCXX_ENABLE_RMM} \
           ${EXTRA_CMAKE_ARGS}
 
     cd ${LIB_BUILD_DIR}
 
-    compile_start=$(date +%s)
     cmake --build . -j${PARALLEL_LEVEL} ${VERBOSE_FLAG}
-    compile_end=$(date +%s)
-    compile_total=$(( compile_end - compile_start ))
 
     if [[ ${BUILD_COMPILE_COMMANDS} == "ON" ]]; then
       cp compile_commands.json ..
     fi
 
     if [[ ${INSTALL_TARGET} != "" ]]; then
         cmake --build . -j${PARALLEL_LEVEL} --target install ${VERBOSE_FLAG}
-        if [[ ${UCXX_ENABLE_PYTHON} == "ON" ]]; then
-          cmake --install . --component python
-        fi
         if [[ ${BUILD_BENCHMARKS} == "ON" ]]; then
           cmake --install . --component benchmarks
         fi
@@ -218,6 +209,30 @@ fi
 # Replace spaces with semicolons in SKBUILD_EXTRA_CMAKE_ARGS
 SKBUILD_EXTRA_CMAKE_ARGS=$(echo ${EXTRA_CMAKE_ARGS} | sed 's/ /;/g')
 
+# Build and install libucxx_python.so
+if buildAll || hasArg libucxx_python; then
+    CMAKE_GENERATOR="${CMAKE_GENERATOR:-Ninja}"
+    cmake -S $REPODIR/cpp/python -B ${PYTHON_BUILD_DIR} \
+          -G${CMAKE_GENERATOR} \
+          -DCMAKE_INSTALL_PREFIX=${INSTALL_PREFIX} \
+          -DDISABLE_DEPRECATION_WARNINGS=${BUILD_DISABLE_DEPRECATION_WARNINGS} \
+          -DCMAKE_BUILD_TYPE=${BUILD_TYPE} \
+          -DCMAKE_EXPORT_COMPILE_COMMANDS=${BUILD_COMPILE_COMMANDS} \
+          ${EXTRA_CMAKE_ARGS}
+
+    cd ${PYTHON_BUILD_DIR}
+
+    cmake --build . -j${PARALLEL_LEVEL} ${VERBOSE_FLAG}
+
+    if [[ ${BUILD_COMPILE_COMMANDS} == "ON" ]]; then
+      cp compile_commands.json ..
+    fi
+
+    if [[ ${INSTALL_TARGET} != "" ]]; then
+        cmake --build . -j${PARALLEL_LEVEL} --target install ${VERBOSE_FLAG}
+    fi
+fi
+
 # Build and install the UCXX Python package
 if buildAll || hasArg ucxx; then
     if hasArg -g; then

diff --git a/ci/build_wheel.sh b/ci/build_wheel.sh
@@ -18,7 +18,7 @@ if [[ ${package_name} == "distributed-ucxx" ]]; then
 
     RAPIDS_PY_WHEEL_NAME="distributed_ucxx_${RAPIDS_PY_CUDA_SUFFIX}" rapids-upload-wheels-to-s3 ${package_dir}/dist
 elif [[ ${package_name} == "ucxx" ]]; then
-    SKBUILD_CMAKE_ARGS="-DUCXX_ENABLE_RMM=ON" \
+    SKBUILD_CMAKE_ARGS="-DUCXX_ENABLE_RMM=ON;-DCMAKE_INSTALL_LIBDIR=ucxx/lib64;-DCMAKE_INSTALL_INCLUDEDIR=ucxx/include" \
         python -m pip wheel "${package_dir}"/ -w "${package_dir}"/dist -vvv --no-deps --disable-pip-version-check
 
     python -m auditwheel repair -w ${package_dir}/final_dist --exclude "libucp.so.0" ${package_dir}/dist/*

diff --git a/conda/recipes/ucxx/build_and_install_ucxx.sh b/conda/recipes/ucxx/build_and_install_ucxx.sh
@@ -3,5 +3,5 @@
 # SPDX-FileCopyrightText: Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES.
 # SPDX-License-Identifier: BSD-3-Clause
 
-cmake --install cpp/build --component python
-./build.sh ucxx
+cmake --install cpp/python/build
+./build.sh ucxx --cmake-args="\"-DFIND_UCXX_PYTHON=ON -Ducxx-python_DIR=$(realpath ./cpp/python/build)\""
diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt
@@ -37,15 +37,13 @@ option(BUILD_TESTS "Configure CMake to build tests" ON)
 option(BUILD_BENCHMARKS "Configure CMake to build benchmarks" OFF)
 option(BUILD_EXAMPLES "Configure CMake to build examples" OFF)
 option(BUILD_SHARED_LIBS "Build UCXX shared libraries" ON)
-option(UCXX_ENABLE_PYTHON "Enable support for Python notifier thread" OFF)
 option(UCXX_ENABLE_RMM "Enable support for CUDA multi-buffer transfer with RMM" OFF)
 option(DISABLE_DEPRECATION_WARNINGS "Disable warnings generated from deprecated declarations." OFF)
 
 message(VERBOSE "UCXX: Configure CMake to build tests: ${BUILD_TESTS}")
 message(VERBOSE "UCXX: Configure CMake to build benchmarks: ${BUILD_BENCHMARKS}")
 message(VERBOSE "UCXX: Configure CMake to build examples: ${BUILD_EXAMPLES}")
 message(VERBOSE "UCXX: Build UCXX shared libraries: ${BUILD_SHARED_LIBS}")
-message(VERBOSE "UCXX: Enable support for Python notifier thread: ${UCXX_ENABLE_PYTHON}")
 message(VERBOSE "UCXX: Enable support for CUDA multi-buffer transfer with RMM: ${UCXX_ENABLE_RMM}")
 message(
   VERBOSE
@@ -81,13 +79,13 @@ rapids_cmake_support_conda_env(conda_env MODIFY_PREFIX_PATH)
 # dependency because users would then have no control over whether ucx is found
 # multiple times, causing potential configure errors. Therefore, we use a raw
 # find_package call instead of rapids_find_package and skip exporting the ucx
-# dependency. Consumers of ucxx must find ucx themselves. Once the above issue
-# is resolved (see https://github.com/openucx/ucx/pull/9622) we can remove the
-# above find_package in favor of the commented out lines below. For the same
-# reason, we must also gate this find_package call behind a check for the
-# target already existing so that consumers can use tools like CPM.cmake to
-# either find or build ucxx from source if it cannot be found (i.e. both cases
-# must allow prior finding of ucx).
+# dependency. Consumers of ucxx must find ucx themselves. Once we move the
+# minimum version to UCX 1.16 we can remove the above find_package in favor of
+# the commented out lines below. For the same reason, we must also gate this
+# find_package call behind a check for the target already existing so that
+# consumers can use tools like CPM.cmake to either find or build ucxx from
+# source if it cannot be found (i.e. both cases must allow prior finding of
+# ucx).
 if(NOT TARGET ucx::ucp)
   find_package(ucx REQUIRED)
 endif()
@@ -206,13 +204,6 @@ endif()
 
 add_library(ucxx::ucxx ALIAS ucxx)
 
-# Build Python if requested
-if(UCXX_ENABLE_PYTHON)
-  add_subdirectory(python)
-else()
-  set(UCXX_PYTHON_TARGET_TARGET "")
-endif()
-
 # ##################################################################################################
 # * tests and benchmarks --------------------------------------------------------------------------
 # ##################################################################################################
@@ -258,17 +249,6 @@ install(
   EXPORT ucxx-exports
 )
 
-if(TARGET ucxx_python)
-  install(
-    TARGETS ucxx_python
-    COMPONENT python
-    DESTINATION ${lib_dir}
-    EXCLUDE_FROM_ALL
-    EXPORT ucxx-python-exports
-  )
-  set(_components_export_string COMPONENTS python COMPONENTS_EXPORT_SET ucxx-python-exports)
-endif()
-
 install(DIRECTORY ${UCXX_SOURCE_DIR}/include/ucxx
                   DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
 )
@@ -295,7 +275,7 @@ targets:
 rapids_export(
   INSTALL ucxx
   EXPORT_SET ucxx-exports ${_components_export_string}
-  GLOBAL_TARGETS ucxx python
+  GLOBAL_TARGETS ucxx
   NAMESPACE ucxx::
   DOCUMENTATION doc_string
 )
@@ -305,7 +285,7 @@ rapids_export(
 rapids_export(
   BUILD ucxx
   EXPORT_SET ucxx-exports ${_components_export_string}
-  GLOBAL_TARGETS ucxx python
+  GLOBAL_TARGETS ucxx
   NAMESPACE ucxx::
   DOCUMENTATION doc_string
 )

diff --git a/cpp/python/CMakeLists.txt b/cpp/python/CMakeLists.txt
@@ -3,13 +3,78 @@
 # SPDX-License-Identifier: BSD 3-Clause License
 # ======================================================================================================
 
+cmake_minimum_required(VERSION 3.26.4 FATAL_ERROR)
+
+include(../../fetch_rapids.cmake)
+include(rapids-cmake)
+include(rapids-cpm)
+include(rapids-export)
+include(rapids-find)
+
+file(READ "${CMAKE_CURRENT_LIST_DIR}/../../VERSION" _version_contents)
+if(_version_contents MATCHES "^([0-9]+)\\.([0-9]+)\\.([0-9]+).*$")
+  set(libucxx_version "${CMAKE_MATCH_1}.${CMAKE_MATCH_2}.${CMAKE_MATCH_3}")
+else()
+  string(REPLACE "\n" "\n  " _version_contents_formatted "${_version_contents}")
+  message(FATAL_ERROR "Could not determine ucxx version. Contents of VERSION file:\n  ${_version_contents_formatted}")
+endif()
+
+project(
+  UCXX_PYTHON
+  VERSION ${libucxx_version}
+  LANGUAGES C CXX
+)
+
+option(BUILD_SHARED_LIBS "Build UCXX Python shared libraries" ON)
+option(FIND_UCXX_CPP "Search for existing UCXX C++ installations before defaulting to local files"
+       OFF
+)
+
+# add third party dependencies using CPM
+rapids_cpm_init()
+# find rmm
+include(../cmake/thirdparty/get_rmm.cmake)
+
+if(FIND_UCXX_CPP)
+  rapids_find_package(
+    ucxx REQUIRED
+    BUILD_EXPORT_SET ucxx-python-exports
+    INSTALL_EXPORT_SET ucxx-python-exports
+  )
+else()
+  set(UCXX_ENABLE_RMM ON)
+  add_subdirectory(.. ucxx-cpp)
+endif()
+
 rapids_find_package(
   Python3 REQUIRED COMPONENTS Development
   BUILD_EXPORT_SET ucxx-python-exports
   INSTALL_EXPORT_SET ucxx-python-exports
 )
 set(UCXX_PYTHON_LIB Python3::Python)
 
+# ##################################################################################################
+# * compiler options ------------------------------------------------------------------------------
+# Due to https://github.com/openucx/ucx/issues/9614, we cannot export the ucx
+# dependency because users would then have no control over whether ucx is found
+# multiple times, causing potential configure errors. Therefore, we use a raw
+# find_package call instead of rapids_find_package and skip exporting the ucx
+# dependency. Consumers of ucxx must find ucx themselves. Once we move the
+# minimum version to UCX 1.16 we can remove the above find_package in favor of
+# the commented out lines below. For the same reason, we must also gate this
+# find_package call behind a check for the target already existing so that
+# consumers can use tools like CPM.cmake to either find or build ucxx from
+# source if it cannot be found (i.e. both cases must allow prior finding of
+# ucx).
+if(NOT TARGET ucx::ucp)
+  find_package(ucx REQUIRED)
+endif()
+#rapids_find_package(
+#  ucx REQUIRED
+#  BUILD_EXPORT_SET ucxx-exports
+#  INSTALL_EXPORT_SET ucxx-exports
+#)
+
 # ##################################################################################################
 # * python library --------------------------------------------------------------------------------
 add_library(
@@ -38,12 +103,14 @@ target_compile_options(
   ucxx_python PRIVATE "$<$<COMPILE_LANGUAGE:CXX>:${UCXX_CXX_FLAGS}>"
 )
 
+get_filename_component(ucxx_dir "${CMAKE_CURRENT_SOURCE_DIR}" DIRECTORY)
+
 # Specify include paths for the current target and dependents
 target_include_directories(
   ucxx_python
-  PUBLIC "$<BUILD_INTERFACE:${UCXX_SOURCE_DIR}/include>"
-         "$<BUILD_INTERFACE:${UCXX_SOURCE_DIR}/python/include>"
-  PRIVATE "$<BUILD_INTERFACE:${UCXX_SOURCE_DIR}/src>"
+  PUBLIC "$<BUILD_INTERFACE:${ucxx_dir}/include>"
+         "$<BUILD_INTERFACE:${ucxx_dir}/python/include>"
+  PRIVATE "$<BUILD_INTERFACE:${ucxx_dir}/src>"
   INTERFACE "$<INSTALL_INTERFACE:include>"
 )
 
@@ -68,13 +135,23 @@ endif()
 
 include(GNUInstallDirs)
 add_library(ucxx::python ALIAS ucxx_python)
-install(DIRECTORY ${UCXX_SOURCE_DIR}/python/include/ucxx
+install(DIRECTORY ${ucxx_dir}/python/include/ucxx
                   DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
-                  COMPONENT python
-                  EXCLUDE_FROM_ALL
+)
+install(TARGETS ucxx_python DESTINATION ${CMAKE_INSTALL_LIBDIR} EXPORT ucxx-python-exports)
+
+rapids_export(
+  INSTALL ucxx-python
+  EXPORT_SET ucxx-python-exports
+  GLOBAL_TARGETS python
+  NAMESPACE ucxx::
 )
 
-include("${rapids-cmake-dir}/export/find_package_root.cmake")
-rapids_export_find_package_root(
-  BUILD Python3 [=[${CMAKE_CURRENT_LIST_DIR}]=] EXPORT_SET ucxx-python-exports
+# ##################################################################################################
+# * build export -------------------------------------------------------------------------------
+rapids_export(
+  BUILD ucxx-python
+  EXPORT_SET ucxx-python-exports
+  GLOBAL_TARGETS python
+  NAMESPACE ucxx::
 )
diff --git a/python/ucxx/CMakeLists.txt b/python/ucxx/CMakeLists.txt
@@ -21,34 +21,27 @@ project(
   LANGUAGES CXX
 )
 
-option(FIND_UCXX_CPP "Search for existing UCXX C++ installations before defaulting to local files"
+option(FIND_UCXX_PYTHON "Search for existing UCXX Python library installations before defaulting to local files"
        OFF
 )
 
 # If the user requested it we attempt to find UCXX.
-if(FIND_UCXX_CPP)
+if(FIND_UCXX_PYTHON)
   find_package(ucx REQUIRED)
-  find_package(ucxx ${ucxx_version} REQUIRED COMPONENTS python)
+  find_package(ucxx-python ${ucxx_version} REQUIRED)
 else()
-  set(ucxx_FOUND OFF)
+  set(ucxx-python_FOUND OFF)
 endif()
 
 include(rapids-cython-core)
 
-if(NOT ucxx_FOUND)
+if(NOT ucxx-python_FOUND)
   set(BUILD_TESTS OFF)
   set(BUILD_BENCHMARKS OFF)
-  set(UCXX_ENABLE_PYTHON ON)
 
   set(_exclude_from_all "")
 
-  add_subdirectory(../../cpp ucxx-cpp ${_exclude_from_all})
-
-  # Since ucxx._lib requires access to libucxx, we place the library in the ucxx directory
-  # and modify the rpaths appropriately.
-  set(cython_lib_dir ucxx)
-  install(TARGETS ucxx DESTINATION ${cython_lib_dir})
-  install(TARGETS ucxx_python DESTINATION ${cython_lib_dir})
+  add_subdirectory(../../cpp/python ucxx-python ${_exclude_from_all})
 endif()
 
 rapids_cython_init()
@@ -59,6 +52,6 @@ find_package(
 add_subdirectory(ucxx/examples)
 add_subdirectory(ucxx/_lib)
 
-if(DEFINED cython_lib_dir)
-  rapids_cython_add_rpath_entries(TARGET ucxx PATHS "${cython_lib_dir}")
+if(NOT ucxx-python_FOUND)
+  rapids_cython_add_rpath_entries(TARGET ucxx_python PATHS ${CMAKE_INSTALL_LIBDIR})
 endif()