Build and publish macOS amd64 baseline (-march=nehalem) artifacts#291
Build and publish macOS amd64 baseline (-march=nehalem) artifacts#291robobun wants to merge 1 commit into
Conversation
Adds bun-webkit-macos-amd64-baseline and bun-webkit-macos-amd64-baseline-lto lanes to the macos-cross job, mirroring the existing linux amd64 baseline lanes, and wires them into the release job. Also publishes bun-webkit-windows-amd64-baseline-lto, which the windows-cross job already builds but the release job never uploaded.
|
Warning Review limit reached
Next review available in: 17 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
Comment |
bun's release cross-compiles darwin from Linux with LTO on by default (ltoDefault = release && (linux || darwinCross) && ci), and prebuiltSuffix() composes -baseline with -lto, so a darwin-x64-baseline release build requests bun-webkit-macos-amd64-baseline-lto.tar.gz. Without this lane that 404s. Mirrors the existing linux/windows amd64 baseline lanes, which each ship both a plain and an -lto variant. Caught by robobun in oven-sh#291.
There was a problem hiding this comment.
LGTM — mechanical CI matrix addition mirroring the existing linux/windows baseline lanes.
What was reviewed:
- New
macos-crossmatrix entries match the existingamd64/amd64-ltoentries plusMARCH_FLAG;macos-cross-release.shuses:=fallback so emptyMARCH_FLAGon existing lanes still resolves to-march=haswell/-mcpu=apple-m1. bun-webkit-windows-amd64-baseline-ltois already built by thewindows-crossmatrix; this just wires it into the release job.- All three new artifacts are threaded consistently through download-artifact →
mv→action-gh-releasefiles list, with the same pinned action SHAs as neighbors.
Extended reasoning...
Overview
This PR adds two new build matrix entries (bun-webkit-macos-amd64-baseline and bun-webkit-macos-amd64-baseline-lto) to the macos-cross job in .github/workflows/build-reusable.yml, forwards matrix.MARCH_FLAG to macos-cross-release.sh, and threads three new artifacts (the two macOS baseline variants plus the already-built-but-never-published bun-webkit-windows-amd64-baseline-lto) through the release job's download/rename/publish steps. No source code is touched — this is purely CI workflow YAML.
Security risks
None identified. The change adds matrix rows and artifact-plumbing lines only; no new GitHub Actions are introduced, all uses: references reuse the same pinned commit SHAs already present in the file, no new secrets or permissions are requested, and no untrusted input flows into shell commands beyond the same ${{matrix.*}} interpolation pattern used by every existing lane.
Level of scrutiny
Low. This is a copy-paste of the established baseline-lane pattern (linux/linux-musl/windows already have -march=nehalem baseline entries) applied to macOS x64. The new matrix entries are byte-for-byte the existing bun-webkit-macos-amd64/-lto entries with one added key. I verified in macos-cross-release.sh that MARCH_FLAG uses : "${MARCH_FLAG:=...}" default assignment, so passing an empty string (as the non-baseline lanes now do via MARCH_FLAG="${{matrix.MARCH_FLAG}}") still falls back to the arch-appropriate default — existing lanes are unaffected. I also confirmed the windows-cross matrix already contains bun-webkit-windows-amd64-baseline-lto, so the new release-job wiring for it references a real artifact. The workflow is self-validating: if any artifact name were misspelled, the download or mv step would fail the run.
Other factors
Recent history (#283, mimalloc matrix entries) shows the same class of change being merged. The .github CODEOWNERS entry is inherited upstream-WebKit boilerplate naming an Apple contributor and isn't operationally meaningful for this Bun-fork-specific workflow file. The PR description notes overlap with #290; that's a maintainer coordination question, not a correctness concern — this PR is a superset and stands correct on its own.
|
Thanks — the For the record, the reasoning checks out against bun's source: // scripts/build/config.ts
const ltoDefault = release && (linux || darwinCross) && ci && !assertions && !asan;
#290 now covers both macOS lanes ( I deliberately left out the third piece here — publishing |
The windows-cross job builds bun-webkit-windows-amd64-baseline-lto on every run, but the release job never downloaded, staged, or published it, so the tarball 404s. bun's scripts/build/config.ts references this variant. Caught by robobun in oven-sh#291.
Preview Builds
|
|
Closing in favor of #290, which now includes the |
Problem
There is no baseline (pre-AVX2) WebKit prebuilt for macOS x64, so oven-sh/bun cannot build a real
bun-darwin-x64-baselinebinary. The@oven/bun-darwin-x64-baselinenpm package currently ships the default-march=haswellbinary byte-for-byte, which crashes with SIGILL on pre-Haswell Intel Macs (Sandy/Ivy Bridge). See oven-sh/bun#34206.Linux and Windows amd64 both have baseline lanes here; macOS x64 was the gap.
Changes
macos-crossjob: addbun-webkit-macos-amd64-baselineandbun-webkit-macos-amd64-baseline-ltomatrix entries withMARCH_FLAG: "-march=nehalem", mirroring the linux amd64 baseline lanes. The-ltovariant matters: bun's release pipeline cross-compiles darwin from Linux with LTO on by default (ltoDefault = release && (linux || darwinCross) && ciin bun'sscripts/build/config.ts), so a darwin-x64-baseline release build requestsbun-webkit-macos-amd64-baseline-lto.tar.gz. With only the non-LTO artifact, bun would need to special-case LTO off for darwin baseline, diverging from the default darwin-x64 build.matrix.MARCH_FLAGtomacos-cross-release.sh. The script already accepts it (--build-arg MARCH_FLAG) and falls back to-march=haswell/-mcpu=apple-m1when empty, so existing lanes are unchanged.releasejob: download and publish the two new tarballs.bun-webkit-windows-amd64-baseline-lto: thewindows-crossjob has been building it every run, but the release job never downloaded or uploaded it (bun'sscripts/build/config.tsreferences this missing variant).Artifact names follow the scheme
prebuiltSuffix()in bun'sscripts/build/deps/webkit.tsalready computes, so no bun-side URL changes are needed, only aWEBKIT_VERSIONbump once this is in an autobuild.@tobocop2 verified in oven-sh/bun#34206 that an unmodified
macos-cross-release.shrun withMACOS_ARCH=x86_64 MARCH_FLAG="-march=nehalem"builds cleanly and produces alibJavaScriptCore.awith zero%ymminstructions (vs ~59k in the haswell build). The preview build on this PR exercises both new lanes.