-
Notifications
You must be signed in to change notification settings - Fork 4
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
Migrate all Python builds from scikit-build to scikit-build-core #2
Comments
I'm +1 on this. For reference, the README has a thorough and direct list of differences between |
Contributes to rapidsai/build-planning#2 Authors: - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - Bradley Dice (https://github.com/bdice) - AJ Schmidt (https://github.com/ajschmidt8) URL: #1287
Contributes to rapidsai/build-planning#2 Authors: - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - AJ Schmidt (https://github.com/ajschmidt8) - Corey J. Nolet (https://github.com/cjnolet) URL: #2051
Contributes to rapidsai/build-planning#2 Authors: - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - Bradley Dice (https://github.com/bdice) - AJ Schmidt (https://github.com/ajschmidt8) - Mark Harris (https://github.com/harrism) URL: #1313
Contributes to rapidsai/build-planning#2 Authors: - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - Bradley Dice (https://github.com/bdice) - AJ Schmidt (https://github.com/ajschmidt8) - Dante Gama Dessavre (https://github.com/dantegd) URL: #5693
Contributes to rapidsai/build-planning#2 Authors: - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - Bradley Dice (https://github.com/bdice) - AJ Schmidt (https://github.com/ajschmidt8) - Richard (Rick) Zamora (https://github.com/rjzamora) URL: #13531
Contributes to rapidsai/build-planning#2 Authors: - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - Bradley Dice (https://github.com/bdice) - AJ Schmidt (https://github.com/ajschmidt8) - Mads R. B. Kristensen (https://github.com/madsbk) URL: #325
Contributes to rapidsai/build-planning#2 Authors: - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - Bradley Dice (https://github.com/bdice) - AJ Schmidt (https://github.com/ajschmidt8) - Rick Ratzel (https://github.com/rlratzel) URL: #4053
@vyasr Would it be possible to migrate https://github.com/rapidsai/cugraph-ops/blob/96a388691c8d42a9f3b7a56bf6f29d24d8ffe166/pylibcugraphops/pyproject.toml#L20 |
Also, this might not need migrated, but |
I chose not to include cugraph-ops in this work because that package uses nanobind instead of Cython for its bindings, so it's not sensitive to rapids-cython changes. I also don't know much about its nanobind usage, and it looks like they do something bespoke with setting the CMake install directory. I'd rather leave that one to the devs to manage the transition since it doesn't impact rapids-cython at all. |
Regarding rapids-cmake, I'm pretty sure that dependency is superfluous and can be removed altogether. We never actually launch builds via Python in rapids-cmake. Removed that dependency in rapidsai/rapids-cmake#512. |
Contributes to rapidsai/build-planning#2 Authors: - Vyas Ramasubramani (https://github.com/vyasr) - Peter Andreas Entschev (https://github.com/pentschev) Approvers: - Bradley Dice (https://github.com/bdice) - AJ Schmidt (https://github.com/ajschmidt8) - Robert Maynard (https://github.com/robertmaynard) - Peter Andreas Entschev (https://github.com/pentschev) URL: #146
I think there's one more block of work that should be done as part of this. There are some projects with configuration in environment variables that
So I see 3 small pieces of additional work:
For all projects that have recently cut over to I didn't find a ton of these from GitHub search (e.g. this link), but there are a few. For example, |
Good catches @jameslamb! Let me work through these. As far as build options we've just been removing the build options everywhere, so I'll do that, and hopefully the configure options are mostly fixed already since those need to be right for everything to work correctly. |
These are the remaining issues in repos that have made the scikit-build-core transition. We can close this issue once the following PRs are merged: |
Closing this now. All the associated work has been merged. |
Contributes to rapidsai/build-planning#2 Authors: - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - AJ Schmidt (https://github.com/ajschmidt8) - Corey J. Nolet (https://github.com/cjnolet) URL: rapidsai#2051
See rapidsai/build-planning#2. This repo was overlooked at the time. Authors: - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - Jake Awe (https://github.com/AyodeAwe) URL: #150
RAPIDS libraries are generally built with CMake. To facilitate better integration of the C++ builds with Python builds, we switched from using pure setuptools builds to using scikit-build. This change was crucial to enabling wheels by providing a single standard entrypoint (all the usual Python
pip [install|wheel|etc]
machinery) for building a Python package while also compiling the required C++ components. However, scikit-build's approach to enabling this is fundamentally limited because it relies on plugging into setuptools directly in ways that setuptools only marginally supports. The result is a tool that works most of the time, but has various sharp edges (e.g. incomplete support for MANIFEST.in, broken installations in certain cases, etc) and limitations (an inability to support true editable installations, mixed support for pyproject.toml/setup.py, etc).The solution is to switch to the newer scikit-build-core builder, a modern standards-based builder that offers the same class of functionality as scikit-build (integrating a Python build with CMake) in a more reliable manner. Doing so will allow us to completely removed deprecated components of our build systems (various uses of setup.py), get rid of workarounds for scikit-build (e.g. the languages we must specify at the CMake level), and get full support for critical features like editable installs.
PRs Contributing To This Effort
The text was updated successfully, but these errors were encountered: