Skip to content

Fix several issues with wheel builds - #439

Merged
rapids-bot[bot] merged 4 commits into
rapidsai:branch-24.10from
KyleFromNVIDIA:fix-wheel-build
Aug 16, 2024
Merged

rapids-bot[bot] merged 4 commits into
rapidsai:branch-24.10from
KyleFromNVIDIA:fix-wheel-build

Conversation

@KyleFromNVIDIA

Copy link
Copy Markdown
Member
  • Ensure correct directory layout in both wheels
  • Add CMake entry point to libkvikio wheel
  • Require kvikio wheel to use libkvikio wheel
  • Exclude unneeded source files from wheel

* Ensure correct directory layout in both wheels
* Add CMake entry point to libkvikio wheel
* Require kvikio wheel to use libkvikio wheel
* Exclude unneeded source files from wheel
@KyleFromNVIDIA
KyleFromNVIDIA requested review from a team as code owners August 16, 2024 16:35
@KyleFromNVIDIA KyleFromNVIDIA added bug Something isn't working non-breaking Introduces a non-breaking change labels Aug 16, 2024
Comment thread cpp/cmake/rapids_config.cmake Outdated

@jameslamb jameslamb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these changes all make sense to me, thanks

Comment thread python/libkvikio/pyproject.toml
Comment thread python/kvikio/pyproject.toml Outdated
@KyleFromNVIDIA

Copy link
Copy Markdown
Member Author

cc @msarahan

@msarahan msarahan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good fixes! Thanks.

Comment thread python/kvikio/pyproject.toml Outdated
Comment thread ci/build_wheel_python.sh Outdated
@KyleFromNVIDIA

Copy link
Copy Markdown
Member Author

/merge

@rapids-bot
rapids-bot Bot merged commit 56d859e into rapidsai:branch-24.10 Aug 16, 2024
rapids-bot Bot pushed a commit that referenced this pull request Sep 23, 2024
As of #439 the `FIND_KVIKIO_CPP` variable is largely superfluous. In principle we could still choose to support users who want to build the Python package without having the C++ library already available somewhere, but that use case is vanishingly small now that we have C++ wheels and it would require the developer to intentionally build without isolation and omit the C++ wheel from the environment. For advanced use cases like that it is reasonable to ask the user to handle building the C++ library separately on their own and making it available for the Python package to find. None of the normal build cases that we support (conda, pip, devcontainers, DLFW) require this any longer.

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

Approvers:
  - Bradley Dice (https://github.com/bdice)

URL: #466
vyasr added a commit to rapidsai/devcontainers that referenced this pull request Sep 23, 2024
There have been a number of changes to RAPIDS builds over the course of
this release and not all changes were fully propagated to the
devcontainers repo. This repo addresses the following:
- As of rapidsai/kvikio#369, kvikio produces
wheels, and rapidsai/kvikio#439 contains
critical fixes that allow the kvikio Python wheel to use the C++
libkvikio wheel. In RAPIDS Python builds we have consistently removed
support for the Python build triggering the C++ build as we have created
C++ wheels since in both conda and pip environments we now expect the
library to be found and we do not need to automatically support the more
esoteric use case of someone turning off build isolation but not having
the C++ library available (devs can handle this case themselves if they
wish). As a result, once rapidsai/kvikio#466 is
merged, the `FIND_KVIKIO_CPP` variable will be completely superfluous
and we can remove that here.
- As of NVIDIA/cudf#16640 libcudf no longer
links to libarrow and `USE_LIBARROW_FROM_PYARROW` is no longer used.
- The libcudf and libcuspatial Python package builds in the devcontainer
should (like all other Python packages) omit the CUDA version suffix.
For that, they need to use the `rapids_build_backend_args`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working non-breaking Introduces a non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants