Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix several issues with wheel builds #439

Merged
merged 4 commits into from
Aug 16, 2024

Conversation

KyleFromNVIDIA
Copy link
Contributor

  • 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
Copy link
Member

@jameslamb jameslamb left a comment

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

python/libkvikio/pyproject.toml Show resolved Hide resolved
python/kvikio/pyproject.toml Outdated Show resolved Hide resolved
@KyleFromNVIDIA
Copy link
Contributor Author

cc @msarahan

Copy link
Contributor

@msarahan msarahan left a comment

Choose a reason for hiding this comment

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

good fixes! Thanks.

python/kvikio/pyproject.toml Outdated Show resolved Hide resolved
ci/build_wheel_python.sh Outdated Show resolved Hide resolved
@KyleFromNVIDIA
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit 56d859e into rapidsai:branch-24.10 Aug 16, 2024
48 checks passed
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 rapidsai/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