ci: pin the build toolchain (cibuildwheel, build, scikit-build-core) - #91
Merged
Conversation
Tier 1 of the dependency float audit. These three build, repair, test and publish every wheel, and all three were unbounded -- the same exposure that took ruff, nanobind and GitPython out of service today. It had already happened, silently. CI currently resolves cibuildwheel 4.2.0 and scikit-build-core 1.0.3, so BOTH crossed a major under the open bounds: the wheels published as 5.10.2 were built and tested by tooling nobody selected. They worked, which is luck rather than policy. cibuildwheel unpinned -> ==4.2.0 (wheels.yml, two sites) build unpinned -> ==1.6.0 (wheels.yml, sdist) scikit-build-core >=0.10 -> >=0.10,<2 (pyproject build-system) cibuildwheel and build are exact-pinned: they are tools, and reproducible wheels are the point. scikit-build-core is capped at the major instead, since patch fixes to a build backend are worth taking automatically and a major is not. Note it also supplies the CMake that runs (4.4.2 here), which is a constraint arriving from a dependency the project never names. Also corrects a stale comment: the arch note reasoned about "cibuildwheel 3.0" default archs while CI ran 4.2.0. The config is insulated by an explicit CIBW_ARCHS/CIBW_BUILD, so this was documentation drift rather than a break -- but it is the symptom that showed the version drift. numpy is deliberately left at >=1.24. It looks like the ABI trap and is not one: no numpy C headers appear anywhere in the extension, since nb::ndarray goes through DLPack and the buffer protocol, so numpy is a stub-generation build dep and a runtime import with no compile-time coupling. The ecosystem test dependencies (torch, jax, pandas, scikit-learn) are left floating on purpose -- that is a policy decision about what ecosystem-test is for, tracked in #90. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
On-demand reviews are free for the next 24 days. After that, they cost $0.25 per reviewed file. Or wait 50 minutes for your next included review. View limit detailsLimit details: You’ve used the included review currently available. Your 76 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
Comment |
Ravenwater
added a commit
that referenced
this pull request
Aug 28, 2026
Resolves #90. The issue framed this as a tradeoff -- early warning of upstream drift versus a PR queue an upstream release can block -- and named the deciding fact: whether ecosystem-test is a required check. It is not, and neither half of the objection survives checking: * the protect-main ruleset carries only deletion and non_fast_forward. There are NO required status checks, so a red job cannot block a merge. * nothing depends on this job. wheels.yml's release chain gates on validate-wheels, in a different workflow, so a failure here cannot stop a release either. The dilemma rested on a premise this repo does not have. A failure is loud (a red check, a failed run) and cheap (blocks nothing), which is what an early-warning lane should be. Floating is already the right answer and already in place; what was missing was the reasoning. These differ from cibuildwheel/build/scikit-build-core, pinned in #91: those build or publish an artifact, so an upstream release retroactively changes what ships. This job's value IS moving with upstream. No behaviour change -- a comment, so the next dependency audit does not 'fix' this by pinning it. Records the one condition that would flip the tradeoff: adding required status checks to the ruleset.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tier 1 of the dependency float audit. Tier 2 is a policy question and is filed separately as #90.
These three build, repair, test and publish every wheel, and all three were unbounded — the same exposure that took ruff, nanobind and GitPython out of service today.
cibuildwheel==4.2.0(two sites)build==1.6.0scikit-build-core>=0.10>=0.10,<2It had already happened, silently
This is not a hypothetical. Read out of the CI log for the run that published 5.10.2:
Both crossed a major under the open bounds — cibuildwheel 3.x → 4.x and scikit-build-core 0.x → 1.0. The wheels published as 5.10.2 an hour ago were built and tested by tooling versions nobody selected. They worked, which is luck rather than policy.
There was a visible symptom nobody had cause to look at: the arch comment in
wheels.ymlreasons about "cibuildwheel 3.0" default archs while CI runs 4.2.0. The config is insulated by an explicitCIBW_ARCHS: auto64/CIBW_BUILD, so this was documentation drift rather than a break — but it is exactly the trace the version drift left. Corrected here.Why the specifiers differ
cibuildwheelandbuildare exact-pinned: they are tools, and a reproducible artifact is the entire point of pinning them.scikit-build-coreis capped at the major instead, because patch fixes to a PEP 517 backend are worth taking automatically while a major is not. Worth knowing that it also supplies the CMake that runs — 4.4.2 here, which dropped compatibility withcmake_minimum_required(VERSION < 3.5). That is a real constraint on this build arriving from a dependency the project never names;3.22is fine today, and a future MTL5 or universal bump is where it could surface.Deliberately not pinned
numpystays>=1.24. This looks like the classic ABI trap — an extension built against numpy 2 headers failing on numpy 1 — and it is not one here. There are no numpy C headers anywhere inpython/src/orpython/include/:nb::ndarraygoes through DLPack and the buffer protocol. numpy is a stub-generation build dependency and a runtime import, with no compile-time coupling. Pinning it would cost users flexibility for a risk that does not exist.The ecosystem test dependencies (torch, jax, pandas, scikit-learn) are left floating on purpose. Whether to pin them is a decision about what
ecosystem-testis for — early warning of upstream drift, versus a PR queue that upstream cannot block — and it deserves an answer rather than a reflex. #90.Already correctly pinned, for the record
ruff==0.16.1·python-semantic-release>=10,<11+GitPython<3.1.60·nanobind>=2.0,<4· FetchContent MTL5v5.10.0, universalv4.7.9, nanobindv3.0.0· Highway1.4.0(transitively, via MTL5's own pin) ·actions/*at major tags ·pypa/gh-action-pypi-publish@release/v1, a moving ref but the form Trusted Publishing requires.Lands as
ci:, which is not inpatch_tags, so this cuts no release.🤖 Generated with Claude Code