fix(macos): serve ONNX Runtime from a build that respects the 13.0 floor - #599
Conversation
`npm run build:mac` could not package at all. Microsoft's published `onnxruntime-osx-arm64-1.27.1.tgz` is built with `minos 14.0`, the app declares a 13.0 floor, and `before-pack.cjs` refuses a payload that demands more than the app promises — correctly, since the deployment target decides which symbols the linker resolves against the OS rather than emitting locally (#515). No published release fixes it: every one from 1.24 on is 14.0, and every one before it is at least 13.3. So the archive now comes from `build-onnxruntime-macos.yml`, which builds the same upstream commit with the target pinned, checks `minos` and the ABI surface, and attests provenance. Only the origin of the bytes moves; the URL is still immutable and the SHA-256 is still verified before the archive is opened. Also stops that workflow from triggering on tag pushes: `on.push` had `paths:` but no `branches:`, so publishing the artifact under its own tag started a fresh 22-minute build of what had just been attached to the release. Closes #591
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe macOS ARM64 ONNX Runtime pin now uses a repository-built release with a new checksum. The macOS build workflow now runs automatically only for pushes to ChangesmacOS ONNX Runtime artifact
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to macOS ARM64 packaging now obtains ONNX Runtime from a project-built, checksum-pinned release intended for the macOS 13.0 support floor, and automatic artifact builds are limited to main. No merge-blocking risk is currently identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
One consequence I would rather state than let you find: merging this will start a ~24-minute ONNX build. Both files this PR touches are in that workflow's That build is redundant — it will produce an artifact already published and already pinned here — but I left it rather than narrowing the filter further. The trigger cannot express "only when Verified on the branch push that the |
1 participant
npm run build:macdoes not package on this branch's parent. It fails inbefore-pack.cjs, and it is right to: Microsoft's publishedonnxruntime-osx-arm64-1.27.1.tgzis built withminos 14.0, the app declares a 13.0 floor, and a payload may not demand more than the app promises — the deployment target decides which symbols the linker resolves against the OS instead of emitting locally (#515).No published release fixes this. Every ONNX Runtime release from 1.24 on is
minos 14.0; every one before it is at least 13.3. There is no 13.0 build to point at, so this points at one we make.What changes
The
darwin-arm64entry now resolves to an archive built bybuild-onnxruntime-macos.yml(merged in #595) from the same upstream commit already recorded in this file, with the deployment target pinned andminos,OrtGetApiBaseand the CPU provider all checked before upload. Provenance is attested:Everything else about the fetch is untouched: immutable URL, SHA-256 verified before the archive is opened, license extracted and checked. What moved is who built the bytes, not how far they are trusted.
Second, unrelated but found while publishing: that workflow's
on.pushhadpaths:and nobranches:, so pushing the artifact's own tag started a fresh 22-minute build of what had just been attached to the release (run 33887404444, cancelled). Now restricted tomain.What I verified
Not "it should work" — run, on this machine, against these exact bytes.
node scripts/fetch-onnxruntime.mjs --forcesha256 ok (7 MB), license MIT, exit 0minos 13.0, 21.7 MB, exportsOrtGetApiBase+…AppendExecutionProvider_CPUnpx electron-builder --mac dir --arm64before-pack.cjsuntouched and un-disarmed — this is the thing that was brokencargo test -p openscreen-compositor --features segmentation --lib→ 8 passed, log saysONNX Runtime présent, so the inference tests ran rather than skippedORT_DYLIB_PATHpointing at nothing, three of those tests printtest sauté— proving the pass above was not vacuousWhat I don't know
minos 13.0is a statement about what the linker targeted, not a test that the library executes there. It is the same guarantee every other native binary in the bundle ships with, and no stronger.ImageOSfor that reason. Two builds of the same commit are not guaranteed byte-identical, so the SHA-256 here pins this artifact, not "any build ofdf2ba1cf".What a reviewer should contest
v0.0.0-*tag, which is the marker this repo uses for a binary needing a permanent URL that is not a product version. ci(build): do not run the release matrix for internal v0.0.0-* tags #598 taughtbuild.ymlto ignore that prefix; the package-manager workflows already skip prereleases. The one thing that still fires is the Discord announcement.segmentation.rsstill says the macOS CI "ne la stage pas encore" — with this merged, CI could stage the library and actually run those three inference tests instead of skipping them. I did not widen the scope to do it; it is worth a follow-up.Closes #591.
Summary by CodeRabbit