From feaf99ecc1714babaccca9a004a2d70271a89be2 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Wed, 6 Dec 2023 12:35:41 -0800 Subject: [PATCH] Remove CUSPATIAL_BUILD_WHEELS and standardize Python builds (#1304) 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: https://github.com/rapidsai/cuspatial/pull/1304 --- ci/build_wheel_cuspatial.sh | 2 - python/cuspatial/CMakeLists.txt | 46 +++++------------- .../cmake/Modules/WheelHelpers.cmake | 48 +++++++------------ 3 files changed, 31 insertions(+), 65 deletions(-) diff --git a/ci/build_wheel_cuspatial.sh b/ci/build_wheel_cuspatial.sh index c98184d13..fd37f873a 100755 --- a/ci/build_wheel_cuspatial.sh +++ b/ci/build_wheel_cuspatial.sh @@ -3,6 +3,4 @@ set -euo pipefail -export SKBUILD_CONFIGURE_OPTIONS="-DCUSPATIAL_BUILD_WHEELS=ON" - ci/build_wheel.sh cuspatial python/cuspatial diff --git a/python/cuspatial/CMakeLists.txt b/python/cuspatial/CMakeLists.txt index e913a1fb2..c7eb3d439 100644 --- a/python/cuspatial/CMakeLists.txt +++ b/python/cuspatial/CMakeLists.txt @@ -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}) @@ -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) diff --git a/python/cuspatial/cmake/Modules/WheelHelpers.cmake b/python/cuspatial/cmake/Modules/WheelHelpers.cmake index 41d720c52..278d6751c 100644 --- a/python/cuspatial/cmake/Modules/WheelHelpers.cmake +++ b/python/cuspatial/cmake/Modules/WheelHelpers.cmake @@ -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 @@ -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) @@ -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 $ DESTINATION ${__DESTINATION}) + message(VERBOSE "install(FILES $ DESTINATION ${__DESTINATION})") + else() + install(IMPORTED_RUNTIME_ARTIFACTS ${target} DESTINATION ${__DESTINATION}) + message( + VERBOSE + "install(IMPORTED_RUNTIME_ARTIFACTS $ 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()