Skip to content

Commit

Permalink
Remove CUSPATIAL_BUILD_WHEELS and standardize Python builds (#1304)
Browse files Browse the repository at this point in the history
Some minor simplification in advance of the scikit-build-core migration to better align wheel and non-wheel Python builds. Additionally, this PR simplifies handling of the nvcomp targets and their installation.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Mark Harris (https://github.com/harrism)
  - Ray Douglass (https://github.com/raydouglass)

URL: #1304
  • Loading branch information
vyasr authored Dec 6, 2023
1 parent 13b297f commit feaf99e
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 65 deletions.
2 changes: 0 additions & 2 deletions ci/build_wheel_cuspatial.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,4 @@

set -euo pipefail

export SKBUILD_CONFIGURE_OPTIONS="-DCUSPATIAL_BUILD_WHEELS=ON"

ci/build_wheel.sh cuspatial python/cuspatial
46 changes: 13 additions & 33 deletions python/cuspatial/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ project(
option(FIND_CUSPATIAL_CPP "Search for existing cuspatial C++ installations before defaulting to local files"
OFF)

option(CUSPATIAL_BUILD_WHEELS "Whether this build is generating a Python wheel." OFF)

# If the user requested it we attempt to find cuspatial.
if(FIND_CUSPATIAL_CPP)
find_package(cuspatial ${cuspatial_version})
Expand All @@ -44,40 +42,22 @@ endif()
if(NOT cuspatial_FOUND)
set(BUILD_TESTS OFF)
set(BUILD_BENCHMARKS OFF)
set(_exclude_from_all "")
if(CUSPATIAL_BUILD_WHEELS)

# Statically link cudart if building wheels
set(CUDA_STATIC_RUNTIME ON)
set(CUSPATIAL_USE_CUDF_STATIC ON)
set(CUSPATIAL_EXCLUDE_CUDF_FROM_ALL ON)

# Always build wheels against the pyarrow libarrow.
set(USE_LIBARROW_FROM_PYARROW ON)

# Need to set this so all the nvcomp targets are global, not only nvcomp::nvcomp
# https://cmake.org/cmake/help/latest/variable/CMAKE_FIND_PACKAGE_TARGETS_GLOBAL.html#variable:CMAKE_FIND_PACKAGE_TARGETS_GLOBAL
set(CMAKE_FIND_PACKAGE_TARGETS_GLOBAL ON)
set(CUDA_STATIC_RUNTIME ON)
set(CUSPATIAL_USE_CUDF_STATIC ON)
set(CUSPATIAL_EXCLUDE_CUDF_FROM_ALL ON)

# Don't install the cuSpatial C++ targets into wheels
set(_exclude_from_all EXCLUDE_FROM_ALL)
endif()

add_subdirectory(../../cpp cuspatial-cpp ${_exclude_from_all})
add_subdirectory(../../cpp cuspatial-cpp EXCLUDE_FROM_ALL)

set(cython_lib_dir cuspatial)

if(CUSPATIAL_BUILD_WHEELS)
include(cmake/Modules/WheelHelpers.cmake)
get_target_property(_nvcomp_link_libs nvcomp::nvcomp INTERFACE_LINK_LIBRARIES)
# Ensure all the shared objects we need at runtime are in the wheel
add_target_libs_to_wheel(LIB_DIR ${cython_lib_dir} TARGETS arrow_shared nvcomp::nvcomp ${_nvcomp_link_libs})
endif()

# Since there are multiple subpackages of cuspatial._lib that require access to libcuspatial, we place the
# library in the cuspatial directory as a single source of truth and modify the other rpaths
# appropriately.
install(TARGETS cuspatial DESTINATION ${cython_lib_dir})
include(cmake/Modules/WheelHelpers.cmake)
# TODO: This install is currently overzealous. We should only install the libraries that are
# downloaded by CPM during the build, not libraries that were found on the system. However, in
# practice this would only be a problem if libcudf was not found but some of the
# dependencies were, and we have no real use cases where that happens.
install_aliased_imported_targets(
TARGETS cuspatial arrow_shared nvcomp::nvcomp nvcomp::nvcomp_gdeflate nvcomp::nvcomp_bitcomp
DESTINATION ${cython_lib_dir}
)
endif()

include(rapids-cython)
Expand Down
48 changes: 18 additions & 30 deletions python/cuspatial/cmake/Modules/WheelHelpers.cmake
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# =============================================================================
# Copyright (c) 2023, NVIDIA CORPORATION.
# Copyright (c) 2022-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
Expand All @@ -14,15 +14,15 @@
include_guard(GLOBAL)

# Making libraries available inside wheels by installing the associated targets.
function(add_target_libs_to_wheel)
list(APPEND CMAKE_MESSAGE_CONTEXT "add_target_libs_to_wheel")
function(install_aliased_imported_targets)
list(APPEND CMAKE_MESSAGE_CONTEXT "install_aliased_imported_targets")

set(options "")
set(one_value "LIB_DIR")
set(one_value "DESTINATION")
set(multi_value "TARGETS")
cmake_parse_arguments(_ "${options}" "${one_value}" "${multi_value}" ${ARGN})

message(VERBOSE "Installing targets '${__TARGETS}' into lib_dir '${__LIB_DIR}'")
message(VERBOSE "Installing targets '${__TARGETS}' into lib_dir '${__DESTINATION}'")

foreach(target IN LISTS __TARGETS)

Expand All @@ -38,34 +38,22 @@ function(add_target_libs_to_wheel)

get_target_property(is_imported ${target} IMPORTED)
if(NOT is_imported)
# If the target isn't imported, install it into the the wheel
install(TARGETS ${target} DESTINATION ${__LIB_DIR})
message(VERBOSE "install(TARGETS ${target} DESTINATION ${__LIB_DIR})")
# If the target isn't imported, install it into the wheel
install(TARGETS ${target} DESTINATION ${__DESTINATION})
message(VERBOSE "install(TARGETS ${target} DESTINATION ${__DESTINATION})")
else()
# If the target is imported, make sure it's global
get_target_property(already_global ${target} IMPORTED_GLOBAL)
if(NOT already_global)
set_target_properties(${target} PROPERTIES IMPORTED_GLOBAL TRUE)
get_target_property(type ${target} TYPE)
if(${type} STREQUAL "UNKNOWN_LIBRARY")
install(FILES $<TARGET_FILE:${target}> DESTINATION ${__DESTINATION})
message(VERBOSE "install(FILES $<TARGET_FILE:${target}> DESTINATION ${__DESTINATION})")
else()
install(IMPORTED_RUNTIME_ARTIFACTS ${target} DESTINATION ${__DESTINATION})
message(
VERBOSE
"install(IMPORTED_RUNTIME_ARTIFACTS $<TARGET_FILE:${target}> DESTINATION ${__DESTINATION})"
)
endif()

# Find the imported target's library so we can copy it into the wheel
set(lib_loc)
foreach(prop IN ITEMS IMPORTED_LOCATION IMPORTED_LOCATION_RELEASE IMPORTED_LOCATION_DEBUG)
get_target_property(lib_loc ${target} ${prop})
if(lib_loc)
message(VERBOSE "Found ${prop} for ${target}: ${lib_loc}")
break()
endif()
message(VERBOSE "${target} has no value for property ${prop}")
endforeach()

if(NOT lib_loc)
message(FATAL_ERROR "Found no libs to install for target ${target}")
endif()

# Copy the imported library into the wheel
install(FILES ${lib_loc} DESTINATION ${__LIB_DIR})
message(VERBOSE "install(FILES ${lib_loc} DESTINATION ${__LIB_DIR})")
endif()
endforeach()
endfunction()

0 comments on commit feaf99e

Please sign in to comment.