Port to scikit-build-core #397

maxbachmann · 2024-08-01T21:26:44Z

@henryiii this is a first attempt at porting to scikit-build-core. It did work on my machine, but still has to be tested on things like android where no cmake wheels are available.

maxbachmann · 2024-08-01T21:36:01Z

I should have ported over all build system features I was previously using and didn't run into any problems so far.

Can you have a quick look over the build system?

maxbachmann · 2024-08-01T21:37:44Z

Amazing experience so far. This allows me to get rid of quite a few things in the build system. From a development perspective I am really pleased with the fact that I don't have to delete the _skbuild directory between each build anymore 👍

codecov · 2024-08-01T21:54:39Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.87%. Comparing base (d8238f5) to head (32fcaad).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #397   +/-   ##
=======================================
  Coverage   82.87%   82.87%           
=======================================
  Files          46       46           
  Lines        4496     4496           
=======================================
  Hits         3726     3726           
  Misses        770      770

Flag	Coverage Δ
unittests	`82.87% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pyproject.toml

tools/sdist.patch

maxbachmann · 2024-08-02T20:10:32Z

In the released version I generate the cython files and ship them as part of the sdist. These generated files get the cxx file ending and are gitignored. The patch is applied before packing the sdist to remove cython from the dependency list since it's not required if the generated files are packed.

pyproject.toml

henryiii · 2024-08-06T20:12:20Z

FYI, 0.10 is out. It takes a bit for it to filter down everywhere (conda takes ~24 hours to pick it up), but just letting you know it's becoming available. :)

pyproject.toml

tools/sdist.patch

pyproject.toml

Co-authored-by: Henry Schreiner <[email protected]>

maxbachmann · 2024-09-19T23:13:33Z

@henryiii I took this up again and started to test the fallback handling.

As a first step I did add invalid code in C++ to test the Python fallback. This falls back, but then crashes:

  *** CMake build failed - retrying due to override...
  Traceback (most recent call last):
    File "/tmp/pip-build-env-ca_6j1zw/overlay/lib/python3.12/site-packages/scikit_build_core/build/wheel.py", line 175, in _build_wheel_impl
      return _build_wheel_impl_impl(
             ^^^^^^^^^^^^^^^^^^^^^^^
    File "/tmp/pip-build-env-ca_6j1zw/overlay/lib/python3.12/site-packages/scikit_build_core/build/wheel.py", line 423, in _build_wheel_impl_impl
      builder.build(build_args=build_args)
    File "/tmp/pip-build-env-ca_6j1zw/overlay/lib/python3.12/site-packages/scikit_build_core/builder/builder.py", line 265, in build
      self.config.build(
    File "/tmp/pip-build-env-ca_6j1zw/overlay/lib/python3.12/site-packages/scikit_build_core/cmake.py", line 264, in build
      self._build(*local_args, *build_args)
    File "/tmp/pip-build-env-ca_6j1zw/overlay/lib/python3.12/site-packages/scikit_build_core/cmake.py", line 275, in _build
      raise FailedLiveProcessError(msg) from None
  scikit_build_core.errors.FailedLiveProcessError: CMake build failed

  During handling of the above exception, another exception occurred:

  Traceback (most recent call last):
    File "/usr/lib/python3.12/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 353, in <module>
      main()
    File "/usr/lib/python3.12/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 335, in main
      json_out['return_val'] = hook(**hook_input['kwargs'])
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/usr/lib/python3.12/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 251, in build_wheel
      return _build_backend().build_wheel(wheel_directory, config_settings,
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/tmp/pip-build-env-ca_6j1zw/overlay/lib/python3.12/site-packages/scikit_build_core/build/__init__.py", line 31, in build_wheel
      return _build_wheel_impl(
             ^^^^^^^^^^^^^^^^^^
    File "/tmp/pip-build-env-ca_6j1zw/overlay/lib/python3.12/site-packages/scikit_build_core/build/wheel.py", line 204, in _build_wheel_impl
      return _build_wheel_impl_impl(
             ^^^^^^^^^^^^^^^^^^^^^^^
    File "/tmp/pip-build-env-ca_6j1zw/overlay/lib/python3.12/site-packages/scikit_build_core/build/wheel.py", line 232, in _build_wheel_impl_impl
      metadata = get_standard_metadata(pyproject, settings)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/tmp/pip-build-env-ca_6j1zw/overlay/lib/python3.12/site-packages/scikit_build_core/build/metadata.py", line 35, in get_standard_metadata
      raise KeyError(msg)
  KeyError: 'version is not in project.dynamic'

I did attempt a couple of other things:

hard code the version using version = "3.9.7" + removing tool.scikit-build.metadata.version and dynamic = ["version"]. This still fails with:

  *** CMake build failed - retrying due to override...
  *** scikit-build-core 0.10.6 (wheel)
  *** Making wheel...
  2024-09-20 01:10:50,905 - scikit_build_core - ERROR - Metadata mismatch in WHEEL: b'Wheel-Version: 1.0\nGenerator: scikit-build-core 0.10.6\nRoot-Is-Purelib: false\nTag: cp312-cp312-linux_x86_64\n\n' != b'Wheel-Version: 1.0\nGenerator: scikit-build-core 0.10.6\nRoot-Is-Purelib: true\nTag: py3-none-any\n\n'
  Traceback (most recent call last):
    File "/tmp/pip-build-env-zd1ax4eg/overlay/lib/python3.12/site-packages/scikit_build_core/build/wheel.py", line 175, in _build_wheel_impl
      return _build_wheel_impl_impl(
             ^^^^^^^^^^^^^^^^^^^^^^^
    File "/tmp/pip-build-env-zd1ax4eg/overlay/lib/python3.12/site-packages/scikit_build_core/build/wheel.py", line 423, in _build_wheel_impl_impl
      builder.build(build_args=build_args)
    File "/tmp/pip-build-env-zd1ax4eg/overlay/lib/python3.12/site-packages/scikit_build_core/builder/builder.py", line 265, in build
      self.config.build(
    File "/tmp/pip-build-env-zd1ax4eg/overlay/lib/python3.12/site-packages/scikit_build_core/cmake.py", line 264, in build
      self._build(*local_args, *build_args)
    File "/tmp/pip-build-env-zd1ax4eg/overlay/lib/python3.12/site-packages/scikit_build_core/cmake.py", line 275, in _build
      raise FailedLiveProcessError(msg) from None
  scikit_build_core.errors.FailedLiveProcessError: CMake build failed

  During handling of the above exception, another exception occurred:

  Traceback (most recent call last):
    File "/usr/lib/python3.12/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 353, in <module>
      main()
    File "/usr/lib/python3.12/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 335, in main
      json_out['return_val'] = hook(**hook_input['kwargs'])
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/usr/lib/python3.12/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 251, in build_wheel
      return _build_backend().build_wheel(wheel_directory, config_settings,
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/tmp/pip-build-env-zd1ax4eg/overlay/lib/python3.12/site-packages/scikit_build_core/build/__init__.py", line 31, in build_wheel
      return _build_wheel_impl(
             ^^^^^^^^^^^^^^^^^^
    File "/tmp/pip-build-env-zd1ax4eg/overlay/lib/python3.12/site-packages/scikit_build_core/build/wheel.py", line 204, in _build_wheel_impl
      return _build_wheel_impl_impl(
             ^^^^^^^^^^^^^^^^^^^^^^^
    File "/tmp/pip-build-env-zd1ax4eg/overlay/lib/python3.12/site-packages/scikit_build_core/build/wheel.py", line 508, in _build_wheel_impl_impl
      raise AssertionError(msg)
  AssertionError: Metadata mismatch in WHEEL
  error: subprocess-exited-with-error

I then tried to change the order to:

[[tool.scikit-build.overrides]]
if.failed = true
wheel.cmake = false

[[tool.scikit-build.overrides]]
if.any.system-cmake = ">=3.15"
if.any.cmake-wheel = true
wheel.cmake = true

my expectation here was that the second is implicitly if.failed = false and so this would still fallback to Python. However it attempts to build using cmake twice. It's unclear to me why this happens.

henryiii · 2024-09-20T03:49:45Z

Is the current state of the PR the one that produces your first traceback? Might be an issue with pyproject-metadata - I've just rewritten the way this works in pyproject-metadata, so we might get this fixed for free by upgrading.

For the second one, that might be tricky. prepare_metadata_for_build_wheel is required to return the same metadata that the wheel build itself produces. But you can't know that CMake is going to fail. We could either check to see if there's an override that uses if.failed, and not provide this method if that's the case (it's great that it's only an override and not a config-setting!), or we could ease up on our check and not verify some fields like Root-Is-Purelib: false and Tag are included in the verification. Or we could reduce the verification to a warning (this technically must have also happened with setuptools + skbuild before the way you were doing it, we just didn't verify back then).

Also we produce the correct error message at the top, I should see if we can get rid of the traceback.

my expectation here was that the second is implicitly if.failed = false and so this would still fallback to Python. However it attempts to build using cmake twice. It's unclear to me why this happens.

Isn't that still what this is asking for? The order only matters if both match, then the second replaces any fields defined (unless you set inherit).

henryiii · 2024-09-20T04:01:20Z

uv venv --python python3.13
uv pip install -e ../../scikit-build/scikit-build-core
FORCE_COLOR=1 uv build --no-build-isolation --wheel

This reproduces, and you even get color in Python 3.13's traceback too! :)

And something is modifying the pyproject dictionary, dynamic.version is empty at this point.

henryiii · 2024-09-20T04:06:20Z

Ah, bug, and before it gets to pyproject-metadata. It's making a shallow copy but then modifying project.dynamic which is the same array. A deep copy fixes that bug.

I didn't get the collision in metadata, though, what tool are you using? Ahh, uv must not call the (optional) metadata hook. Pretty sure build doesn't either. But pip does.

maxbachmann · 2024-09-20T08:38:00Z

Is the current state of the PR the one that produces your first traceback?

yes

Isn't that still what this is asking for? The order only matters if both match, then the second replaces any fields defined (unless you set inherit).

well I was just surprised since this means that this override triggers the rebuild feature, but the overrides which are implicitly if.failed = false still manage to override it again. So I guess I would need to explicitly exclude the if.failed = true overload in case the defines are set.

The env variable check doesn't appear to work properly for me. As a minimum example I attempted to use:

[tool.scikit-build]
...
wheel.cmake = false

[[tool.scikit-build.overrides]]
if.env.CIBUILDWHEEL = true
wheel.cmake = true
messages.after-failure = "failed to build C++ Extension in a packaged build"

However this didn't appear to trigger the override. So I assumed that maybe I was setting the env variable incorrectly for some reason and tried:

[tool.scikit-build]
...
wheel.cmake = false

[[tool.scikit-build.overrides]]
if.env.CIBUILDWHEEL = false
wheel.cmake = true
messages.after-failure = "failed to build C++ Extension in a packaged build"

However this still didn't trigger the override. I would always expect one of the two versions to trigger the override.

I didn't get the collision in metadata, though, what tool are you using? Ahh, uv must not call the (optional) metadata hook. Pretty sure build doesn't either. But pip does.

yes this was using plain old pip

Reported in rapidfuzz/RapidFuzz#397. Signed-off-by: Henry Schreiner <[email protected]>

henryiii · 2024-09-20T14:49:32Z

implicitly if.failed = false still manage to override it again

Ahh, I see what you are expecting. No, this is not implicit on an override. Overrides match based on their if contents. If if.failed/if.any.failed is not present, then that doesn't factor into the match at all. If you want something to only match on the first run, you need if.failed = False.

The env vars should work, I'll check.

Reported in rapidfuzz/RapidFuzz#397. This avoids declaring the optional hooks if there's a good chance it can't be computed reliably. Having an override with `failed` can't be computed without trying to build. --------- Signed-off-by: Henry Schreiner <[email protected]>

henryiii · 2024-09-20T19:16:23Z

 [[tool.scikit-build.overrides]]
 if.failed = true
+if.env.CIBUILDWHEEL = false
+if.env.CONDA_BUILD = false
+if.env.PIWHEELS_BUILD = false
+if.env.RAPIDFUZZ_BUILD_EXTENSION = false
 wheel.cmake = false
 messages.after-success = "failed to build C++ Extension, falling back to pure Python Extension"

You need the above or it will try to build twice. We might be able to simplify this in the future by adding a "rebuilds" parameter, perhaps.

henryiii · 2024-09-20T19:19:57Z

Ahh, we don't treat a missing environment variable the same as a falsey environment variable. I think that's a bug.

Noticed in rapidfuzz/RapidFuzz#397. Signed-off-by: Henry Schreiner <[email protected]>

henryiii · 2024-09-20T21:07:03Z

Could you retry with 0.10.7? Pushed three fixes.

henryiii · 2024-09-21T15:56:36Z

allow-prereleases needs to be set for setup-python to get 3.13.

maxbachmann · 2024-09-21T16:01:10Z

Upgraded scikit-build-core and excluded the failed overload based on the env variables.
From my perspective everything is working in my tests:

normal build -> builds cmake
normal build with error in code -> falls back to pure Python + prints a warning
packaged build -> builds cmake
packaged build with error in code -> fails with an error message
build on target without cmake -> falls back to pure Python + prints warning
packaged build on target without cmake -> fails with an error message

maxbachmann · 2024-09-21T16:34:43Z

It's great to finally have this replacement done.
Thanks @henryiii for your help on this and the awesome work you do for Python packaging in general.

henryiii · 2024-09-21T16:48:28Z

pyproject.toml

+]
+wheel.packages = ["src/rapidfuzz"]
+wheel.cmake = false
+messages.after-success = "{yellow}Cmake unavailable, falling back to pure Python Extension"


Suggested change

messages.after-success = "{yellow}Cmake unavailable, falling back to pure Python Extension"

messages.after-success = "{yellow}CMake unavailable, falling back to pure Python Extension"

Unnecessary stylistic nitpick. ;)

henryiii reviewed Aug 2, 2024

View reviewed changes

pyproject.toml Outdated Show resolved Hide resolved

henryiii reviewed Aug 2, 2024

View reviewed changes

pyproject.toml Show resolved Hide resolved

henryiii reviewed Aug 2, 2024

View reviewed changes

tools/sdist.patch Show resolved Hide resolved

henryiii reviewed Aug 6, 2024

View reviewed changes

pyproject.toml Outdated Show resolved Hide resolved

henryiii reviewed Aug 6, 2024

View reviewed changes

pyproject.toml Outdated Show resolved Hide resolved

pyproject.toml Show resolved Hide resolved

pyproject.toml Show resolved Hide resolved

tools/sdist.patch Outdated Show resolved Hide resolved

henryiii reviewed Aug 6, 2024

View reviewed changes

pyproject.toml Outdated Show resolved Hide resolved

maxbachmann force-pushed the scikit-build-core branch 2 times, most recently from a8e0bc6 to 0b0e045 Compare August 7, 2024 09:09

maxbachmann force-pushed the scikit-build-core branch from 0b0e045 to 9fc8891 Compare September 19, 2024 23:01

maxbachmann and others added 11 commits September 20, 2024 01:04

port to scikit-build-core

13561cc

fix build

2d3c291

update patch

70edc21

fix project properties

aa7b4d3

Update pyproject.toml

fb9ff36

Update pyproject.toml

69aa805

Co-authored-by: Henry Schreiner <[email protected]>

Update pyproject.toml

1cf3643

Co-authored-by: Henry Schreiner <[email protected]>

Update pyproject.toml

615a9ac

Co-authored-by: Henry Schreiner <[email protected]>

Update tools/sdist.patch

0bc454b

Co-authored-by: Henry Schreiner <[email protected]>

fix patch

1d1abe8

further cleanup

35b379a

maxbachmann force-pushed the scikit-build-core branch from 9fc8891 to 35b379a Compare September 19, 2024 23:05

maxbachmann added 2 commits September 20, 2024 01:19

fix failure message

03e5167

add warning message

344c3cd

This was referenced Sep 20, 2024

fix: avoid modifing the input dict scikit-build/scikit-build-core#903

Merged

fix: avoid providing prepare-metadata methods if rebuild is True scikit-build/scikit-build-core#904

Merged

henryiii added a commit to scikit-build/scikit-build-core that referenced this pull request Sep 20, 2024

fix: avoid modifing the input dict (#903)

e903ec9

Reported in rapidfuzz/RapidFuzz#397. Signed-off-by: Henry Schreiner <[email protected]>

henryiii mentioned this pull request Sep 20, 2024

fix: support negative envvar values correctly scikit-build/scikit-build-core#907

Merged

henryiii added a commit to scikit-build/scikit-build-core that referenced this pull request Sep 20, 2024

fix: support negative envvar values correctly (#907)

bf800ac

Noticed in rapidfuzz/RapidFuzz#397. Signed-off-by: Henry Schreiner <[email protected]>

maxbachmann added 4 commits September 21, 2024 16:41

update scikit-build-core

0a265a8

add colors

6410843

update messaging

4bd16f7

further improve message

32fcaad

allow pre releases

2294922

update changelog

6c0b80a

maxbachmann merged commit 2e85e5f into main Sep 21, 2024
40 checks passed

maxbachmann mentioned this pull request Sep 21, 2024

Migration to scikit-build-core #390

Closed

henryiii reviewed Sep 21, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port to scikit-build-core #397

Port to scikit-build-core #397

maxbachmann commented Aug 1, 2024

maxbachmann commented Aug 1, 2024

maxbachmann commented Aug 1, 2024

codecov bot commented Aug 1, 2024 •

edited

Loading

maxbachmann commented Aug 2, 2024

henryiii commented Aug 6, 2024

maxbachmann commented Sep 19, 2024 •

edited

Loading

henryiii commented Sep 20, 2024

henryiii commented Sep 20, 2024

henryiii commented Sep 20, 2024 •

edited

Loading

maxbachmann commented Sep 20, 2024 •

edited

Loading

henryiii commented Sep 20, 2024

henryiii commented Sep 20, 2024

henryiii commented Sep 20, 2024

henryiii commented Sep 20, 2024

henryiii commented Sep 21, 2024

maxbachmann commented Sep 21, 2024 •

edited

Loading

maxbachmann commented Sep 21, 2024

henryiii Sep 21, 2024

maxbachmann Sep 21, 2024

	messages.after-success = "{yellow}Cmake unavailable, falling back to pure Python Extension"
	messages.after-success = "{yellow}CMake unavailable, falling back to pure Python Extension"

Port to scikit-build-core #397

Port to scikit-build-core #397

Conversation

maxbachmann commented Aug 1, 2024

maxbachmann commented Aug 1, 2024

maxbachmann commented Aug 1, 2024

codecov bot commented Aug 1, 2024 • edited Loading

Codecov Report

maxbachmann commented Aug 2, 2024

henryiii commented Aug 6, 2024

maxbachmann commented Sep 19, 2024 • edited Loading

henryiii commented Sep 20, 2024

henryiii commented Sep 20, 2024

henryiii commented Sep 20, 2024 • edited Loading

maxbachmann commented Sep 20, 2024 • edited Loading

henryiii commented Sep 20, 2024

henryiii commented Sep 20, 2024

henryiii commented Sep 20, 2024

henryiii commented Sep 20, 2024

henryiii commented Sep 21, 2024

maxbachmann commented Sep 21, 2024 • edited Loading

maxbachmann commented Sep 21, 2024

henryiii Sep 21, 2024

Choose a reason for hiding this comment

maxbachmann Sep 21, 2024

Choose a reason for hiding this comment

codecov bot commented Aug 1, 2024 •

edited

Loading

maxbachmann commented Sep 19, 2024 •

edited

Loading

henryiii commented Sep 20, 2024 •

edited

Loading

maxbachmann commented Sep 20, 2024 •

edited

Loading

maxbachmann commented Sep 21, 2024 •

edited

Loading