-
Notifications
You must be signed in to change notification settings - Fork 54
chore: try using scikit-build-core #20
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
chore: try using scikit-build-core #20
Conversation
|
This looks amazing! To me it looks like a nice improvement without any downside. Are there potential gotchas/limitations that I am not aware of? Regarding ABI3 builds: Ideally, the normal pip install should target the current Python version, while the GHA wheel action additionally makes an ABI3 wheel to be future-proof. However, nanobind's ABI3 support depends on Python 3.12, and that wasn't officially supported by cibuildwheel yet. (But it might be soon, with the Python3.12b1 having been released 2 days ago). Anyways, that's why ABI3 isn't mentioned in this example project (yet!) Minor: I saw that you reduced the minimum CMake version to 3.15. I am not 100% sure that this will be compatible with the nanobind CMake scripts (have not tested it though.). Is there a particular reason for this change? |
|
It's not as hackable as something based on setuptools (which is not really a downside...). Features are carefully being added as needed. Otherwise, I think it should be a solid upgrade. A way to detect ABI 3 should be available in the next release (though I might change the in-main version before release, there is something there now). I think overrides will be coming soon too, hopefully 0.5. I checked nanobind and it puts 3.15 (or was it 3.14?) as the minimum version - if that's the case, scikit-build-core supports and is tested on 3.15+. Since it backports FindPython from 3.26.1 (configurable), you actually can use the features required for making wheels, like Development.Module, ABI3, etc. in CMake 3.15+. I didn't run a test on 3.15 though. |
c188b43 to
38e70a3
Compare
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com> bump cibuildwheel to try builds with Python 3.12b1 tweaks another attempt WIP: try branch for Windows fix Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com> chore: use py-abi fix Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com> WIP: try nanobind patch Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com> fix: try new branch
44d7c86 to
9e07cde
Compare
henryiii
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I could PR that later)
|
Going to try it without the require statement. It’s a cibuildwheel bug if it doesn’t work. |
|
Looks good to me: |
|
Amazing 🎊. Thank you very much, Henry! |
This commit switches from scikit-build to scikit-build-core, which significantly simplifies the example build system and brings other benefits like a stable ABI3 wheels.
This is an experiment using scikit-build-core for the example. Several things are handled by scikit-build-core automatically, like auto downloading cmake/ninja if needed, backporting FindPython 3.26.1 if an older CMake is used, auto-discovery of properly placed cmake Config files (which nanobind seems to do!), release types (which technically were always handled by scikit-build), and better SDist & wheel directory handling.
How is ABI3 specified? I didn't see it set for the old setup.py. There's a setting in scikit-build-core for it; like every setting, it can be in pyproject.toml, as a
--config-settingsoption, or via an environment variable.. I can work on implementing an overrides system to control settings based on the Python being used, would that be enough? I can also provide a nice way to communicate to CMakeLists.txt if ABI3 is requested. Just need to know what the expectation here is.Closes #19.