Skip to content

Build and publish macOS amd64 baseline (-march=nehalem) artifacts#291

Closed
robobun wants to merge 1 commit into
mainfrom
farm/237eef58/macos-x64-baseline
Closed

Build and publish macOS amd64 baseline (-march=nehalem) artifacts#291
robobun wants to merge 1 commit into
mainfrom
farm/237eef58/macos-x64-baseline

Conversation

@robobun

@robobun robobun commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

Note: #290 by @tobocop2 (the reporter of oven-sh/bun#34206) was opened first and covers the non-LTO baseline lane. This PR is the same change plus two pieces #290 does not include (details below). If #290 lands, this can be rebased down to just those pieces.

Problem

There is no baseline (pre-AVX2) WebKit prebuilt for macOS x64, so oven-sh/bun cannot build a real bun-darwin-x64-baseline binary. The @oven/bun-darwin-x64-baseline npm package currently ships the default -march=haswell binary 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-cross job: add bun-webkit-macos-amd64-baseline and bun-webkit-macos-amd64-baseline-lto matrix entries with MARCH_FLAG: "-march=nehalem", mirroring the linux amd64 baseline lanes. The -lto variant matters: bun's release pipeline cross-compiles darwin from Linux with LTO on by default (ltoDefault = release && (linux || darwinCross) && ci in bun's scripts/build/config.ts), so a darwin-x64-baseline release build requests bun-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.
  • Forward matrix.MARCH_FLAG to macos-cross-release.sh. The script already accepts it (--build-arg MARCH_FLAG) and falls back to -march=haswell / -mcpu=apple-m1 when empty, so existing lanes are unchanged.
  • release job: download and publish the two new tarballs.
  • Also publish bun-webkit-windows-amd64-baseline-lto: the windows-cross job has been building it every run, but the release job never downloaded or uploaded it (bun's scripts/build/config.ts references this missing variant).

Artifact names follow the scheme prebuiltSuffix() in bun's scripts/build/deps/webkit.ts already computes, so no bun-side URL changes are needed, only a WEBKIT_VERSION bump once this is in an autobuild.

@tobocop2 verified in oven-sh/bun#34206 that an unmodified macos-cross-release.sh run with MACOS_ARCH=x86_64 MARCH_FLAG="-march=nehalem" builds cleanly and produces a libJavaScriptCore.a with zero %ymm instructions (vs ~59k in the haswell build). The preview build on this PR exercises both new lanes.

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.
@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 17 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: fdaacc72-3762-4a46-b5c5-f294235c937a

📥 Commits

Reviewing files that changed from the base of the PR and between 4895f45 and 45bcadc.

📒 Files selected for processing (1)
  • .github/workflows/build-reusable.yml

Comment @coderabbitai help to get the list of available commands.

tobocop2 added a commit to tobocop2/WebKit that referenced this pull request Jul 15, 2026
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.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — mechanical CI matrix addition mirroring the existing linux/windows baseline lanes.

What was reviewed:

  • New macos-cross matrix entries match the existing amd64/amd64-lto entries plus MARCH_FLAG; macos-cross-release.sh uses := fallback so empty MARCH_FLAG on existing lanes still resolves to -march=haswell/-mcpu=apple-m1.
  • bun-webkit-windows-amd64-baseline-lto is already built by the windows-cross matrix; this just wires it into the release job.
  • All three new artifacts are threaded consistently through download-artifact → mvaction-gh-release files 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.

@tobocop2

Copy link
Copy Markdown

Thanks — the -lto catch is correct and I've verified it, so I've folded it into #290.

For the record, the reasoning checks out against bun's source:

// scripts/build/config.ts
const ltoDefault = release && (linux || darwinCross) && ci && !assertions && !asan;

darwinCross is in the default, and the LTO-disabling guard below it covers windows && baseline but not darwin — so with prebuiltSuffix() composing -baseline + -lto, a darwin-x64-baseline release build does request bun-webkit-macos-amd64-baseline-lto.tar.gz. Every other baseline platform already ships both variants (linux-amd64-baseline + -lto, linux-amd64-musl-baseline + -lto, windows-amd64-baseline + -lto), so macOS shipping only the plain one would have been the odd one out — and my bun PR (oven-sh/bun#34207) would have 404'd on the artifact it actually needs. Good catch; I'd built locally with --lto=off, which is exactly why I missed it.

#290 now covers both macOS lanes (bun-webkit-macos-amd64-baseline and -baseline-lto) plus the release plumbing for both.

I deliberately left out the third piece here — publishing bun-webkit-windows-amd64-baseline-lto, which the windows-cross job builds every run but the release job never downloads or uploads. That's a real pre-existing bug and a genuinely separate concern from "macOS has no baseline", so it seems better as its own change rather than widening mine. Happy either way: if you'd rather land #291 whole and close #290, that's completely fine by me — the goal is pre-AVX2 Macs working, not whose PR does it.

tobocop2 added a commit to tobocop2/WebKit that referenced this pull request Jul 15, 2026
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.
@github-actions

Copy link
Copy Markdown

Preview Builds

Commit Release Date
45bcadc5 autobuild-preview-pr-291-45bcadc5 2026-07-15 04:43:06 UTC

@robobun

robobun commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator Author

Closing in favor of #290, which now includes the -baseline-lto lane and the windows -baseline-lto publishing fix from this PR. The preview build here ran the full matrix successfully and published all three new artifacts: https://github.com/oven-sh/WebKit/releases/tag/autobuild-preview-pr-291-45bcadc5 (verification details in #290 (comment)).

@robobun robobun closed this Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants