-
Notifications
You must be signed in to change notification settings - Fork 915
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
This PR makes a number of changes to streamline the CMake code in the Python build. There are additional potential improvements that may be possible but I'd prefer to wait on until after the scikit-build-core migration because I suspect that there may be subtle differences (particularly around the install rules). The major improvements here are: - nvcomp installation no longer needs as much special casing thanks to rapidsai/rapids-cmake#496 adding missing targets - get_arrow.cmake now determines how to find arrow by checking for 1) the presence of Python, 2) the presence of pyarrow, and 3) the presence of libarrow.so.* inside the pyarrow directory. This approach works for both pip and conda packages as well as pure C++ builds and removes the need for manual determination of where to look by the builder. The assumption baked in is that the developer has installed the desired pyarrow package when building. - The `CUDF_BUILD_WHEELS` variable is now removed. All Python builds that don't find the C++ library all go down the same path now. This is particularly relevant for doing something like a local `pip install`. This is the desired behavior because if you're building the Python package in that kind of environment you always want the same behaviors. The different case is when the Python build finds the C++ build (e.g. in a conda environment where libcudf is a separate package). - The logic for linking to pyarrow headers is now centralized in a single function since it needs to be used by every Cython target. Authors: - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - https://github.com/brandon-b-miller - Robert Maynard (https://github.com/robertmaynard) - Ray Douglass (https://github.com/raydouglass)
- Loading branch information
Showing
13 changed files
with
141 additions
and
226 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| # ============================================================================= | ||
| # Copyright (c) 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 | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software distributed under the License | ||
| # is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express | ||
| # or implied. See the License for the specific language governing permissions and limitations under | ||
| # the License. | ||
| # ============================================================================= | ||
| include_guard(GLOBAL) | ||
|
|
||
| # TODO: Finding NumPy currently requires finding Development due to a bug in CMake. This bug was | ||
| # fixed in https://gitlab.kitware.com/cmake/cmake/-/merge_requests/7410 and will be available in | ||
| # CMake 3.24, so we can remove the Development component once we upgrade to CMake 3.24. | ||
| # find_package(Python REQUIRED COMPONENTS Development NumPy) | ||
|
|
||
| # Note: The bug noted above prevents us from finding NumPy successfully using FindPython.cmake | ||
| # inside the manylinux images used to build wheels because manylinux images do not contain | ||
| # libpython.so and therefore Development cannot be found. Until we upgrade to CMake 3.24, we should | ||
| # use FindNumpy.cmake instead (provided by scikit-build). When we switch to 3.24 we can try | ||
| # switching back, but it may not work if that implicitly still requires Python libraries. In that | ||
| # case we'll need to follow up with the CMake team to remove that dependency. The stopgap solution | ||
| # is to unpack the static lib tarballs in the wheel building jobs so that there are at least static | ||
| # libs to be found, but that should be a last resort since it implies a dependency that isn't really | ||
| # necessary. The relevant command is tar -xf /opt/_internal/static-libs-for-embedding-only.tar.xz -C | ||
| # /opt/_internal" | ||
| find_package(NumPy REQUIRED) | ||
|
|
||
| execute_process( | ||
| COMMAND "${Python_EXECUTABLE}" -c "import pyarrow; print(pyarrow.get_include())" | ||
| OUTPUT_VARIABLE PYARROW_INCLUDE_DIR | ||
| ERROR_VARIABLE PYARROW_ERROR | ||
| RESULT_VARIABLE PYARROW_RESULT | ||
| OUTPUT_STRIP_TRAILING_WHITESPACE | ||
| ) | ||
|
|
||
| if(${PYARROW_RESULT}) | ||
| message(FATAL_ERROR "Error while trying to obtain pyarrow include directory:\n${PYARROW_ERROR}") | ||
| endif() | ||
|
|
||
| # Due to cudf's scalar.pyx needing to cimport pylibcudf's scalar.pyx (because there are parts of | ||
| # cudf Cython that need to directly access the c_obj underlying the pylibcudf Scalar) the | ||
| # requirement for arrow headers infects all of cudf. These requirements will go away once all | ||
| # scalar-related Cython code is removed from cudf. | ||
| function(link_to_pyarrow_headers targets) | ||
| foreach(target IN LISTS targets) | ||
| # PyArrow headers require numpy headers. | ||
| target_include_directories(${target} PRIVATE "${NumPy_INCLUDE_DIRS}") | ||
| target_include_directories(${target} PRIVATE "${PYARROW_INCLUDE_DIR}") | ||
| endforeach() | ||
| endfunction() |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.