Skip to content

Upgrade WebKit to 6d586e29 #31724

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Jarred-Sumner merged 1 commit into from
Jun 2, 2026
Merged

Upgrade WebKit to 6d586e29 #31724

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion scripts/build/deps/webkit.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
// -lto variants built with ThinLTO (per-module summaries for cross-language
// importing), and the Windows ICU data table filtered + per-item zstd
// compressed (lazily decompressed via bun_icu_decompress.cpp).
export const WEBKIT_VERSION = "963f8758c29e965471c191668d5776a1a1b014b6";
export const WEBKIT_VERSION = "6d586e293f008f0e74e5697611a379b1b24815c9";

Check warning on line 10 in scripts/build/deps/webkit.ts

View check run for this annotation

Claude / Claude Code Review

Stale windows-baseline LTO gate after WebKit bump adds -baseline-lto variant 

This bump includes [`c787a5a7`](https://github.com/oven-sh/WebKit/commit/c787a5a7ce04046e9f70963aac46e7848082e218) ("windows cross: add the amd64-baseline ThinLTO variant"), so `bun-webkit-windows-amd64-baseline-lto.tar.gz` now exists in the pinned release. However, `scripts/build/config.ts:769` still hard-forces `lto = false` for `(windows && baseline)`, and the comments at `config.ts:746-748` and `config.ts:767-768` still claim the pinned `WEBKIT_VERSION` lacks/predates the `-baseline-lto` var

@claude claude Bot Jun 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 This bump includes c787a5a7 ("windows cross: add the amd64-baseline ThinLTO variant"), so bun-webkit-windows-amd64-baseline-lto.tar.gz now exists in the pinned release. However, scripts/build/config.ts:769 still hard-forces lto = false for (windows && baseline), and the comments at config.ts:746-748 and config.ts:767-768 still claim the pinned WEBKIT_VERSION lacks/predates the -baseline-lto variant — both are now factually wrong. Consider dropping || baseline from the gate (or at minimum updating the comments) alongside this bump.

Extended reasoning...

What the issue is

scripts/build/config.ts contains a hard override that forces LTO off for Windows baseline builds, with an inline rationale tied directly to the pinned WebKit version:

// config.ts:765-771
// Windows arm64 / baseline: same — oven-sh/WebKit ships no
// bun-webkit-windows-arm64-lto (LLVM's CodeView emitter aborts on ARM64
// NEON tuple registers during LTO codegen), and the pinned WEBKIT_VERSION
// predates the -baseline-lto variant.
if ((asan && lto) || abi === "android" || (windows && (arm64 || baseline))) {
  lto = false;
}

And earlier:

// config.ts:746-748
// Resolved early because the LTO defaults below need it (the windows
// -baseline WebKit prebuilt has no -lto variant).
const baseline = partial.baseline ?? false;

Both comments assert that the pinned WebKit release lacks a windows-amd64-baseline-lto artifact. That was true at 963f8758, but this PR bumps WEBKIT_VERSION to 6d586e29, whose range includes commit c787a5a7 "windows cross: add the amd64-baseline ThinLTO variant". The release autobuild-6d586e29… therefore does publish bun-webkit-windows-amd64-baseline-lto.tar.gz.

Step-by-step proof

  1. Before this PR, WEBKIT_VERSION = "963f8758…". The autobuild-963f8758… release has no …-windows-amd64-baseline-lto.tar.gz, so the gate at config.ts:769 is necessary — without it, prebuiltSuffix() would emit -baseline-lto and the download would 404.
  2. This PR changes WEBKIT_VERSION to "6d586e29…". Per the PR description, that range includes c787a5a7 "windows cross: add the amd64-baseline ThinLTO variant", and the linked release page now contains the -baseline-lto asset.
  3. config.ts is not touched by this PR, so line 769 still reads (windows && (arm64 || baseline)) and line 768 still says "the pinned WEBKIT_VERSION predates the -baseline-lto variant".
  4. A user runs a Windows amd64 baseline cross build with an explicit --lto=on. The condition windows && baseline matches, so lto is silently forced to false. prebuiltSuffix() (webkit.ts:62-74) then returns -baseline instead of -baseline-lto, fetching the non-LTO artifact even though the LTO one exists.
  5. The PR's own check — "all tarball names prebuiltSuffix() produces are present in the release" — cannot catch this, because the gate makes the -baseline-lto suffix unreachable from any config.

Why nothing else justifies the gate

The ThinLTO miscompile concern at config.ts:750-760 only removes Windows from the LTO default (ltoDefault); explicit --lto=on is still permitted for non-baseline Windows x64. So the only documented reason for the baseline-specific hard override is the missing artifact — and that reason is invalidated by this exact bump. The arm64 half of the gate has its own separate (still-valid) rationale about the CodeView emitter.

Impact

No correctness regression — builds still succeed and produce a working binary. The effects are:

  • An explicit --lto=on for Windows amd64 baseline is silently downgraded with no warning, even though the matching prebuilt now exists.
  • Two comments in config.ts make a factual claim about the pinned WEBKIT_VERSION that becomes false the moment this PR lands, which will confuse the next person reading this code.

Suggested fix

Alongside this bump, either:

  • Drop || baseline from the (windows && (arm64 || baseline)) clause at config.ts:769 and update both comments accordingly, or
  • At minimum, rewrite the comments at config.ts:746-748 and config.ts:767-768 so they no longer claim the pinned version lacks the artifact (and explain why the gate remains, if it should).


/**
* WebKit (JavaScriptCore) — the JS engine.
Expand Down
Loading