Skip to content
Open
Changes from all commits
Commits
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
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 = "3167a44fb92c268c83f09b232b38a9f3e7f9655a";
export const WEBKIT_VERSION = "autobuild-preview-pr-234-c8f7dc7f";

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

View check run for this annotation

Claude / Claude Code Review

sync-webkit-source.ts breaks with tag-form WEBKIT_VERSION

Heads-up: pointing `WEBKIT_VERSION` at a tag name instead of a SHA breaks `scripts/sync-webkit-source.ts` for the local-WebKit workflow — `git rev-parse HEAD` can never equal the tag string so the early-exit at line 17 is dead, and `git pull` on `main` won't auto-follow a preview tag pointing at an unmerged PR commit, so `git checkout autobuild-preview-pr-234-c8f7dc7f` at line 24 will fail with `pathspec ... did not match` on existing clones. Prebuilt/CI is fine (the `autobuild-` prefix is alrea

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.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Verify that the preview build URLs are accessible for common platform combinations.
# Expected: HTTP 200 or 302 redirect for each artifact URL.

version="autobuild-preview-pr-234-c8f7dc7f"

# Test a representative subset of platform/arch/variant combinations
artifacts=(
  "bun-webkit-linux-amd64-lto.tar.gz"
  "bun-webkit-linux-arm64-lto.tar.gz"
  "bun-webkit-macos-amd64-lto.tar.gz"
  "bun-webkit-macos-arm64-lto.tar.gz"
  "bun-webkit-windows-amd64-lto.tar.gz"
  "bun-webkit-windows-arm64-lto.tar.gz"
)

base_url="https://github.com/oven-sh/WebKit/releases/download/${version}"

for artifact in "${artifacts[@]}"; do
  url="${base_url}/${artifact}"
  echo "Checking: ${url}"
  http_code=$(curl -s -o /dev/null -w "%{http_code}" -L -I "${url}")
  if [[ "${http_code}" =~ ^(200|302)$ ]]; then
    echo "${http_code}"
  else
    echo "${http_code} (artifact may not exist)"
  fi
done

Repository: oven-sh/bun

Length of output: 987


The preview build is incomplete — missing artifacts for macOS and Windows platforms.

Verification confirms the preview build artifacts are unavailable for macOS (both amd64 and arm64) and Windows (both amd64 and arm64), returning HTTP 404. Only Linux artifacts (amd64 and arm64) are accessible. Since the build system requires artifacts for all platforms, this version cannot be used until the preview release is complete.

Additionally, per the PR objectives, update this to the final merge SHA once oven-sh/WebKit#234 lands—do not merge with the temporary preview version.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/build/deps/webkit.ts` at line 6, The WEBKIT_VERSION constant is
pointing to a temporary preview build that lacks macOS and Windows artifacts; do
not merge as-is — wait for oven-sh/WebKit#234 to land and then update the
WEBKIT_VERSION export (symbol: WEBKIT_VERSION) to the final merge SHA string for
that PR so the build system can fetch artifacts for all platforms; replace the
current "autobuild-preview-..." value with the final commit/merge SHA once
available and ensure the change is committed before merging.

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.

🟡 Heads-up: pointing WEBKIT_VERSION at a tag name instead of a SHA breaks scripts/sync-webkit-source.ts for the local-WebKit workflow — git rev-parse HEAD can never equal the tag string so the early-exit at line 17 is dead, and git pull on main won't auto-follow a preview tag pointing at an unmerged PR commit, so git checkout autobuild-preview-pr-234-c8f7dc7f at line 24 will fail with pathspec ... did not match on existing clones. Prebuilt/CI is fine (the autobuild- prefix is already handled in prebuiltUrl) and the PR notes this pin is temporary, so probably just worth mentioning that local-mode contributors will need a manual git fetch --tags until the merge SHA lands.

Extended reasoning...

What breaks

scripts/sync-webkit-source.ts is the convenience script for the documented local-WebKit workflow (CONTRIBUTING.md:278 — "Check out the commit hash specified in WEBKIT_VERSION"). It imports WEBKIT_VERSION and assumes it is a 40-char commit SHA. With this PR it is now the release-tag string autobuild-preview-pr-234-c8f7dc7f, which trips two assumptions in the script.

Code path

// scripts/sync-webkit-source.ts
14  const checkedOutCommit = (await Bun.$`git rev-parse HEAD`.text()).trim();
15  const { WEBKIT_VERSION: expectedCommit } = await import("./build/deps/webkit.ts");
17  if (checkedOutCommit == expectedCommit) { ... }   // (1)
21  await Bun.$`git checkout main`;
22  await Bun.$`git pull`;
24  await Bun.$`git checkout ${expectedCommit}`;      // (2)

(1) Dead early-exit. git rev-parse HEAD always returns a 40-char SHA, so comparing it against "autobuild-preview-pr-234-c8f7dc7f" can never succeed. Even when vendor/WebKit is already at the right commit, the script falls through to checkout main → pull → checkout. Minor, but it does extra network work every run.

(2) Checkout failure on existing clones. Line 22's git pull on main only auto-follows tags that point at commits being fetched into main's history (git's default fetch.tagopt behavior). The autobuild-preview-pr-234-* tag points at the head of an unmerged PR branch in oven-sh/WebKit, which is not reachable from main, so the tag is not fetched. Line 24 then runs git checkout autobuild-preview-pr-234-c8f7dc7f against a clone that has neither the tag nor the underlying object, and git fails with error: pathspec 'autobuild-preview-pr-234-c8f7dc7f' did not match any file(s) known to git.

Why nothing else catches this

The only other consumer of WEBKIT_VERSION is prebuiltUrl() in scripts/build/deps/webkit.ts, which already special-cases the autobuild- prefix when constructing the release-asset URL — so prebuilt mode and CI are unaffected. The doc comment at webkit.ts:1-4 and CONTRIBUTING.md still describe the constant as a "commit hash", but sync-webkit-source.ts is the only place that mechanically depends on the SHA shape.

Step-by-step proof

  1. Contributor has an existing vendor/WebKit clone at the previous SHA 3167a44f….
  2. They pull this PR and run bun scripts/sync-webkit-source.ts.
  3. Line 14: checkedOutCommit = "3167a44f…".
  4. Line 15: expectedCommit = "autobuild-preview-pr-234-c8f7dc7f".
  5. Line 17: "3167a44f…" == "autobuild-preview-pr-234-c8f7dc7f" → false.
  6. Line 21–22: checkout main, git pull — fetches main plus tags pointing into main's history; the preview tag points at PR chalk package cannot be imported #234's branch tip, so it is skipped.
  7. Line 24: git checkout autobuild-preview-pr-234-c8f7dc7f → ref unknown → pathspec did not match → script exits non-zero.

A fresh git clone would fetch all tags by default and so would succeed; the failure is specific to existing clones, which is the common case for active contributors.

Impact & severity

  • Only affects the opt-in local-WebKit dev script; CI and the default prebuilt path are fine.
  • The PR description already states the value is temporary pending Disable libpas JIT heap on Windows ARM64 WebKit#234 merging, after which it'll become a SHA again.
  • Workaround is trivial: (cd vendor/WebKit && git fetch --tags) before running the script.

So this is a nit — worth a heads-up since the PR description doesn't mention the local-mode side-effect, but not blocking.

Possible fix

If you want the script to keep working during the preview window, swap line 22 for git fetch --tags origin (or add it before the checkout) and compare against git rev-parse HEAD vs git rev-parse "${expectedCommit}^{commit}" so tag refs and SHAs both resolve. Otherwise, just noting the manual git fetch --tags step in the PR description is probably sufficient given the pin is short-lived.


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