-
Notifications
You must be signed in to change notification settings - Fork 5k
inspector: bump WebKit so Runtime.evaluate survives validateExceptionChecks #35333
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
de8e181
b29be6c
eefc40e
afe1ec4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,10 @@ | |
| // Windows ICU data table filtered + per-item zstd compressed, and Windows | ||
| // unwind info (RtlAddGrowableFunctionTable) registered for the fixed JIT | ||
| // pool (LLInt pending offlineasm .seh_* emission). | ||
| export const WEBKIT_VERSION = "a40d462206e1caf8388062120acde61e37a4ae7d"; | ||
| // Preview of oven-sh/WebKit#328: RELEASE_AND_RETURN on the inspector | ||
| // injected-script prototype host functions so Runtime.evaluate survives | ||
| // validateExceptionChecks=1. | ||
| export const WEBKIT_VERSION = "autobuild-preview-pr-328-31913c3e"; | ||
|
Comment on lines
+13
to
+16
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '\n== scripts/build/deps/webkit.ts ==\n'
cat -n scripts/build/deps/webkit.ts | sed -n '1,120p'
printf '\n== references to WEBKIT_VERSION ==\n'
rg -n "WEBKIT_VERSION|autobuild-preview-pr-328-31913c3e|webkit" scripts packages -g '!**/node_modules/**' | sed -n '1,200p'
printf '\n== dependency docs or notes ==\n'
rg -n "preview-pr|WebKit#328|validateExceptionChecks|autobuild-preview" . -g '!**/node_modules/**' | sed -n '1,200p'Repository: oven-sh/bun Length of output: 21931 🌐 Web query:
💡 Result: The term "validateExceptionChecks" refers to a specific diagnostic option in WebKit's JavaScriptCore (JSC) engine [1][2]. When set to true (or 1), this option enables verification that necessary exception checks are performed within the engine [2], which helps identify and debug missing exception handling in the code [3][4]. The identifier "autobuild-preview-pr-328-31913c3e" appears to be a specific build artifact or container image tag, likely generated by a CI/CD pipeline for a pull request (PR). While "pr-328" may refer to a specific pull request in a repository, the exact combination provided does not correspond to a singular, globally recognized WebKit release or public document. The structure is consistent with automated PR preview environments where unique tags (like 31913c3e) are used to track specific builds [5][6][7]. In summary: - validateExceptionChecks: A JSC engine configuration option used for validating exception handling [1][2]. - autobuild-preview-pr-328-31913c3e: An internal identifier for a transient build environment, typical of automated CI systems [5][6][7]. Citations:
Replace the preview WebKit pin before release. 🤖 Prompt for AI Agents
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Acknowledged: the pin stays on the preview tag while iterating on this branch, and afe1ec4 adds a source-lint test that fails on any
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== referenced commit =='
git show --no-ext-diff --format=fuller --stat afe1ec4
printf '%s\n' '== referenced commit patch =='
git show --no-ext-diff --format= --find-renames afe1ec4 -- \
scripts/build/deps/webkit.ts test scripts | sed -n '1,280p'
printf '%s\n' '== preview-pin guard candidates on PR head =='
rg -n -C 4 'autobuild-preview|WEBKIT_VERSION' \
scripts test .github -g '!**/node_modules/**' -g '!**/.git/**' | sed -n '1,280p'Length of output: 19755
That fully addresses the risk during iteration while preserving the required pre-merge action. Keeping this thread open as the merge-gate reminder is appropriate. ✏️ Learnings added
🧠 Learnings used |
||
|
|
||
| /** | ||
| * WebKit (JavaScriptCore) — the JS engine. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴
WEBKIT_VERSIONis pinned toautobuild-preview-pr-328-31913c3e, an ephemeral preview tag that GitHub deletes as soon as oven-sh/WebKit#328 merges or closes — at which point every fresh build of bunmain404s downloading WebKit. Before this lands, swap the pin to the merged 40-hexoven-sh/WebKitmain sha (and drop the "Preview of…" comment).Extended reasoning...
What the bug is
scripts/build/deps/webkit.ts:16setsWEBKIT_VERSION = "autobuild-preview-pr-328-31913c3e". This is anautobuild-preview-pr-*tag — a temporary GitHub release that oven-sh/WebKit's CI publishes while a WebKit PR is open, and deletes when that PR merges or closes. The previous pin was a permanent 40-hex commit sha (a40d462206e1caf8388062120acde61e37a4ae7d).The code path that triggers it
For a prebuilt WebKit (the default,
cfg.webkit === "prebuilt"),webkit.source()callsprebuiltUrl(), which turnscfg.webkitVersioninto a release-download URL.autobuild-*prefixes are handled (const tag = version.startsWith("autobuild-") ? version : \autobuild-${version}`), so the pin *works today* — the URL resolves tohttps://github.com/oven-sh/WebKit/releases/download/autobuild-preview-pr-328-31913c3e/bun-webkit--.tar.gz. At build time, ninja invokesfetch-cli.ts→download.ts::downloadWithRetry()to fetch that tarball intocfg.cacheDir`.Why existing code doesn't prevent it
The build system explicitly supports preview pins so a WebKit PR can be tested inside a bun PR — that's why
prebuiltUrl(),prebuiltDestDir(), anddepVersionsHeader.tsall special-case theautobuild-prefix. But the same codebase also documents that preview pins must not persist.scripts/build/download.ts:276-295has a dedicatedprebuiltDownloadError()branch for exactly this failure:and the remediation it prints is:
So the repo's own error-handling treats a preview pin on
mainas a bad pin, not a supported state. Nothing at configure time rejects the preview tag — the build only fails once the release is gone, which is precisely when it's most disruptive (post-merge, on other people's machines).Impact
If this PR merges to
mainas-is and oven-sh/WebKit#328 subsequently merges (which is the intended outcome — this bun PR exists to ship that WebKit fix), the preview release is deleted. From that moment:mainfailsbun bd/bun run buildat the WebKit download step with HTTP 404.$BUN_INSTALL/build-cache/webkit-preview-pr-328-31913c3e*extraction fails the same way.The PR description already acknowledges this is a preview build ("the first CI run here will fail … until the preview build … finishes publishing"), and the added code comment says "Preview of oven-sh/WebKit#328" — so the author is aware. This finding is the merge-gate reminder, not an oversight report.
Step-by-step proof
mainwithWEBKIT_VERSION = "autobuild-preview-pr-328-31913c3e".autobuild-preview-pr-328-31913c3erelease (per the doc comment atdownload.ts:278-279).mainfresh and runsbun bd.dep_fetchedge for WebKit; ninja runsfetch-cli.ts dep WebKit … https://github.com/oven-sh/WebKit/releases/download/autobuild-preview-pr-328-31913c3e/bun-webkit-linux-amd64-debug.tar.gz.downloadWithRetry()throws;prebuiltDownloadError()matchesname === "WebKit" && missing && url.includes("/autobuild-preview-pr-")and raisesBuildError: WebKit preview release is gone: … HTTP 404.mainis broken until someone lands a follow-up commit changingWEBKIT_VERSION.How to fix
Before merging this PR: merge oven-sh/WebKit#328 first, then update
WEBKIT_VERSIONhere to the resulting 40-hex commit sha on oven-sh/WebKitmain(and replace the "Preview of oven-sh/WebKit#328" comment with the permanent description, matching the style of the surrounding lines). The PR description's own workflow — "re-run onceautobuild-preview-pr-328-31913c3eappears" — is fine for iterating on the branch, but the final merged commit must carry the permanent sha.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Intentional for now: the preview pin lets CI exercise the fix before oven-sh/WebKit#328 merges. I will swap this to the merged 40-hex main sha (and drop the preview comment) before this PR is marked ready to merge. Noted in the PR body under Notes as well.