Skip to content

build: WebKit → preview-pr-181-ebf11e28 #29250

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

Closed
Jarred-Sumner wants to merge 4 commits into from
+1 −1
Closed

build: WebKit → preview-pr-181-ebf11e28 #29250

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2c2b1ca
build: WebKit → preview-pr-181-e38dd2fc (Windows libpas/WTF resource …
Jarred-Sumner Apr 13, 2026
6f011fb
build: WebKit → preview-pr-181-68e2ebfa (restore do_mprotect gate in …
Jarred-Sumner Apr 13, 2026
c63706b
build: WebKit → preview-pr-181-4596a6f6 (is_exiting flag for TLC tear…
Jarred-Sumner Apr 13, 2026
7d5ca23
build: WebKit → preview-pr-181-ebf11e28 (fix pthread_once on Windows)
Jarred-Sumner Apr 13, 2026
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 @@ -3,7 +3,7 @@
* for local mode. Override via `--webkit-version=<hash>` to test a branch.
* From https://github.com/oven-sh/WebKit releases.
*/
export const WEBKIT_VERSION = "42f80a684c5df57121a97e20825a3bcab7a0741b";
export const WEBKIT_VERSION = "preview-pr-181-68e2ebfa";

Check failure on line 6 in scripts/build/deps/webkit.ts

View check run for this annotation

Claude / Claude Code Review

WEBKIT_VERSION preview tag breaks sync-webkit-source.ts (checkout failure + broken early-exit) 

Changing WEBKIT_VERSION to a preview tag string breaks `scripts/sync-webkit-source.ts` in two ways: (1) the `git checkout` on line 24 uses the tag directly, but the actual ref in oven-sh/WebKit is `autobuild-preview-pr-181-68e2ebfa` — `prebuiltUrl()` adds the `autobuild-` prefix for downloads but sync-webkit-source.ts has no such normalization, causing a hard `pathspec did not match` failure; (2) the already-synced early-exit on line 17 compares `git rev-parse HEAD` (a 40-char SHA) to the new ta
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 13, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Blocker: defaulting to this preview is unsafe with current CI crash profile.

This bump is currently tied to 99 failures with broad Windows segfaults (multiple configs/tests). Please avoid setting it as the default WEBKIT_VERSION until that regression is resolved; keep preview testing behind explicit --webkit-version override.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@scripts/build/deps/webkit.ts` at line 6, The constant WEBKIT_VERSION should
not default to the preview string; revert it so the default is the stable
release (or an empty/undefined value) and ensure preview testing only occurs
when an explicit flag is passed; modify the exported WEBKIT_VERSION (symbol
WEBKIT_VERSION) to remove the "preview-pr-..." default and change consuming
logic to require/accept the --webkit-version override instead of relying on the
preview as a fallback.

Copy link
Copy Markdown
Contributor

@claude claude bot Apr 13, 2026

Choose a reason for hiding this comment

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

🔴 Changing WEBKIT_VERSION to a preview tag string breaks scripts/sync-webkit-source.ts in two ways: (1) the git checkout on line 24 uses the tag directly, but the actual ref in oven-sh/WebKit is autobuild-preview-pr-181-68e2ebfa — prebuiltUrl() adds the autobuild- prefix for downloads but sync-webkit-source.ts has no such normalization, causing a hard pathspec did not match failure; (2) the already-synced early-exit on line 17 compares git rev-parse HEAD (a 40-char SHA) to the new tag string — these formats can never match, so every invocation unconditionally re-syncs even when already at the correct commit. Fix: normalize the ref in sync-webkit-source.ts (prepend autobuild- when not already present) and resolve the tag to its SHA for the equality check.

Extended reasoning...

What the bugs are

sync-webkit-source.ts uses WEBKIT_VERSION in two places that both break when the value is a named preview tag instead of a raw 40-char SHA.

Bug 1 — hard checkout failure (lines 15, 24)

sync-webkit-source.ts imports WEBKIT_VERSION as expectedCommit and passes it directly to git checkout ${expectedCommit}. With the old SHA value (42f80a684c5df57121a97e20825a3bcab7a0741b), git could check it out directly. With the new value preview-pr-181-68e2ebfa, git looks for a tag or branch of that exact name. However, prebuiltUrl() in webkit.ts shows that the actual release tag in oven-sh/WebKit is constructed as autobuild-${version} (line 79: const tag = version.startsWith("autobuild-") ? version : autobuild-${version}``). There is no tag named preview-pr-181-68e2ebfa — only `autobuild-preview-pr-181-68e2ebfa`. The checkout fails with: `error: pathspec 'preview-pr-181-68e2ebfa' did not match any file(s) known to git.`

Bug 2 — early-exit optimization permanently broken (lines 14–17)

Before the checkout, the script runs git rev-parse HEAD and compares the output (always a 40-char hex SHA, e.g. 68e2ebfa... expanded to 40 chars) to expectedCommit = "preview-pr-181-68e2ebfa". A SHA and a tag-name string can never be equal, so the already at commit fast path never fires. Every invocation falls through to git checkout main + git pull + git checkout, even on a repo already at the right commit. Even if bug 1 is fixed (checkout uses the autobuild- prefixed tag), git checks out the tag in detached-HEAD mode — subsequent git rev-parse HEAD still returns the SHA, not the tag name, so the early-exit remains broken.

Why existing code doesn't prevent it

prebuiltUrl() correctly handles the prefix for artifact downloads, but sync-webkit-source.ts is a separate script that was written when WEBKIT_VERSION was always a raw SHA and has no normalization logic. The two code paths diverged silently.

Step-by-step proof

  1. This PR sets WEBKIT_VERSION = "preview-pr-181-68e2ebfa".
  2. Developer runs sync-webkit-source.ts to sync their local WebKit clone.
  3. Line 14: checkedOutCommit = await $(git rev-parse HEAD) → e.g. "abc123...40chars".
  4. Line 17: "abc123...40chars" == "preview-pr-181-68e2ebfa" → false — no early exit.
  5. Script falls through; line 24: git checkout preview-pr-181-68e2ebfa.
  6. Git finds no tag/branch named preview-pr-181-68e2ebfa → pathspec did not match error.
  7. Script exits with failure; local WebKit sync is broken for all developers using local mode.

How to fix it

In sync-webkit-source.ts: (a) for the checkout, normalize the ref — prepend autobuild- when the version string does not already start with it; (b) for the equality check, resolve the expected tag to its SHA via git rev-parse autobuild-${version} or git rev-parse refs/tags/autobuild-${version}, then compare that SHA against git rev-parse HEAD.

🔬 also observed by 3071289855


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