Repository navigation
Conversation
… pass oven-sh/WebKit#807 fixes an abort in bun build --bytecode on a function whose jump-size relaxation settles at the pass where the optimizer forces every branch wide. This points WEBKIT_VERSION at its preview build and adds a test with the reduced fixture.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 4 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Updated 5:20 AM PT - Oct 22nd, 2026
❌ @dylan-conway, your commit deddd94 has 4 failures in
The baseline build contains instructions not available on Static scan violations |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| * From https://github.com/oven-sh/WebKit releases. | ||
| */ | ||
| export const WEBKIT_VERSION = "87ee10da087b63e011bd1ada64d1814a850a4395"; | ||
| export const WEBKIT_VERSION = "autobuild-preview-pr-807-cc56b17b"; |
There was a problem hiding this comment.
🔴 Every CI and local prebuilt build of bun will fail with an HTTP 404 the moment oven-sh/WebKit PR #807 merges or closes, and this commit can never be built again. WEBKIT_VERSION at scripts/build/deps/webkit.ts:6 is an autobuild-preview-pr-* tag; scripts/build/download.ts:320-321 states GitHub deletes that release on merge or close. The repo rule in .claude/docs/landing-prs.md:47 is to never merge a pin to a preview tag. Fix: before merging, land oven-sh/WebKit#807 and set WEBKIT_VERSION to the merge commit's 40-hex sha (or autobuild-<sha>), after confirming the prebuilt tarball exists for every os x arch x flavor suffix prebuiltUrl() can emit. The PR text says the pin moves later; that step must happen in this PR, not after.
Why this was flagged
The pin at scripts/build/deps/webkit.ts:6 is "autobuild-preview-pr-807-cc56b17b". prebuiltUrl() at scripts/build/deps/webkit.ts:81-82 uses it verbatim as the GitHub release tag for every prebuilt download, which is the default mode for bun bd and for CI's ci-build profile (scripts/build/CLAUDE.md line 180). scripts/build/download.ts:319-321 documents that GitHub deletes autobuild-preview-pr-* releases when the WebKit PR merges or closes, so every build then throws the BuildError at download.ts:329 ("WebKit preview release is gone"). On the base branch the pin is a 40-hex sha whose autobuild-<sha> release is permanent. .claude/commands/upgrade-webkit.md:34 and .claude/docs/landing-prs.md:47 both require swapping to the merged sha before the bun PR merges; the PR description instead defers that to after merge, leaving a window where main does not build and leaving this commit permanently unbuildable for bisects. No code path in the repo re-resolves a deleted preview tag to the merged sha.
Verification: scripts/build/deps/webkit.ts:6 sets WEBKIT_VERSION to the preview tag autobuild-preview-pr-807-cc56b17b (was a 40-hex sha on the base); webkit.ts:80-82 uses it verbatim as the GitHub release for every prebuilt download. scripts/build/download.ts:318-338 states GitHub deletes the preview release when the PR merges or closes, so every build 404s at once. Nothing in the repo enforces the swap before merge.
There was a problem hiding this comment.
Agreed, the preview tag must not merge. The PR is a draft until oven-sh/WebKit#807 lands. Then the pin moves to the merge commit's sha in this PR, before it leaves draft.
There was a problem hiding this comment.
Code review found no new issues
No new issues were found in this update; 1 finding from earlier reviews is still open above.
Still open from earlier reviews (1):
- 🔴
scripts/build/deps/webkit.ts:6—Every CI and local prebuilt build of bun will fail with an HTTP 404 the moment oven-sh/WebKit PR #807 merges or closes,…
If you have decided not to act on one of these findings, resolve its thread (a reply alone leaves it open) and the next review stops counting it. To review this commit again now, use Re-run on its "Claude Code Review" check.
…6270) ### What does this PR do? Hardening follow-up to #46021. It fixes regressions from #46021 and #44618 that were never released, includes more of the open pull requests that fix a crash, and fixes a few crashes that had no pull request. #### Regressions fixed (none was released) - `spyOn(Bun, "$")`, and the same for `"sql"`, `"postgres"` and `"SQL"`, threw "does not support accessor properties yet". - Async `fs.read`, `fs.write` and `zlib.brotliCompress` on a view over a `WebAssembly.Memory` could lose data or copy far more than they needed. - `node:vm` with `vm.constants.DONT_CONTEXTIFY`: `this` in host accessors, prototype handling, `util.types.isProxy`, `Bun.deepEquals`, `assert.deepEqual` and `console.log` of a context. - `zlib` `stream.flush()` threw for arguments it used to accept, and streams had a new enumerable property. - `Bun.Transpiler` `exports.replace` with the name `"default"`. - `process.env.X = 3000` could store the number once the function was JIT-compiled. - `node:sqlite` and `bun:sqlite` could lose rows at exit. - Dev server: stylesheets shared between pages after a build error. - Shell `cp` of a file onto itself exited 1. - `error.stack`: the frame of `require()` reads `at anonymous (unknown)` again. - `import * as process from "process"` followed by `process.on(...)` threw at load. The EventEmitter methods that the module exports now act on `process`. - `delete res.statusCode; res.end()` in a `node:http` handler threw. It sends 200, as in Node. (from #46128) - `node:tls`, `node:https` and `node:http2`: connection teardown and write callbacks behave as in 1.4.3 and Node again. #### Crash fixes Bundler, parser and CSS: #42286, #46092, #46097, #46177, #46239, #36334, #43024, #40769. `bun test`, `--hot` and `--watch`: #30776, #38206, #38974. Shell: #43183, #37743, #46168, #46207, #34900. `node:http`, `node:http2`, `Bun.serve`, streams: #43449, #45380, #44457, #42197, #42485, #38082, #38124, #46074, #45999, #46045. `node:vm`, native addons, workers: #46129, #46198, #45615, #33412, #38031. Dev server: #42790, #42850, #39488. Other: #35991, #38030, #38586, #40526, #41830, #46029, #46078, #46114, and the bug of #42215. #### Behavior changes - `crypto.randomFill(view, cb)` reports `ERR_CRYPTO_OPERATION_FAILED` if it could not fill the view. It used to report success. - A function registered twice on `process` runs twice, as in Node. (#41830) - Under `--hot`, a reload after an uncaught exception cancels the timers that the failed run created. The new run's timers fire, and so do timers and `fs.watchFile` watchers that earlier runs kept on `globalThis`. The reload also resets the exit code to 0. (#38206) - The parent of a Worker no longer gets an `'error'` event for a failed `Bun.serve` request in that Worker. The error is printed, as on the main thread. - `hash.write()` after `digest()` drops the chunk, as in Node. (#35991) - `http2` over TLS with `server.emit("connection", socket)`: `session.socket` is a `TLSSocket` and a `'connection'` listener fires, as in Node. (#38124) - A `slice()` with no type of a body `Blob` has `type === ""`, as in Node. (#46114) - `util.types.isProxy(globalThis)` is false, as in Node. - A write on a `node:net` or `node:tls` socket that is still pending when the connection closes calls back with `ECANCELED`, as in Node. - `HTMLRewriter.on()` throws for a selector that is nested too deeply. - `Bun.Image`: `maxPixels` also bounds PNG metadata, `metadata()` of a GIF reports the size that gets decoded, and a path with a NUL byte throws. (#40526) - `bun build --no-bundle` with the `base64` or `dataurl` loader prints an error. (#36334) - Bundled output for a default-exported function or class expression changes to `var file_default = function name() {}`, as in esbuild. `.name` is unchanged. (#46097) - A Proxy in the prototype chain of a `node:vm` global is allowed. jsdom 30 with `runScripts` needs this. - `bun:sqlite` commits a write that `iterate()` left open when the main thread exits. An explicit `BEGIN` is still rolled back. - When a Worker stops while a native addon's async work is still running, the addon's finalizers run after that work is done, as in Node. - Shell: `mv -n` and `cp -n` don't overwrite, and `cp` reads `R`, `v` and `n` anywhere in a flag group. #### Performance - Ordinary `node:vm` contexts run at the same speed. With `DONT_CONTEXTIFY`, stores to globals and reads of inherited names in a hot loop cost 10 to 20 % more than on `main`. - A `process.env` write costs 20 to 35 % less than after #46021. - An async read into a `WebAssembly.Memory` view looks through the live memories: about 1.5 µs more with one memory. #### Not included - Taken in part, so these should stay open for the rest: #36334, #38974, #39488, #40526, #40769, #42485, #45615, #45999, #46029, #46092, #46114, #46168, #46207, #46239. - Needs a WebKit change: #34655 #39204 #40170 #40253 #41587 #41900 #42699 #42897 #42903 #43084 #43813 #44518 #44949 #45770 #45804 #45827. - #44723 (covered by #46097), #41675, #42753, #38762, #41475, #46016. ### How did you verify your code works? - Every item was reproduced first on release builds of `main`, of its parent and of 1.4.3, and compared with Node v26 where Node has the API. - Every fix has a test that fails on `main` and passes on a debug + ASAN build of this branch, except where that isn't practical (huge inputs, code that only runs on Windows or macOS). - The regressions were checked on grids of calls against the build before #46021 and against 1.4.3. - Bun's own test suites and Node's `test-vm-*`, `test-http2-*` and `test-zlib-*` files for the areas touched.
…code-optimizer-relaxation # Conflicts: # scripts/build/deps/webkit.ts
dylan-conway
changed the title
This branch has not been deployed
2 participants
Fixes #44926
Problem
bun build --bytecodeaborts on@google-cloud/tasks7.2.1 withpanic(main thread): abort() called.--no-optimize-bytecodeavoids it.Fix
WEBKIT_VERSIONmoves fromd6375c72to5cfcac0f, the merge commit of that PR.verify-baselinejobs,test/bundler/bundler_bytecode_portable.test.ts, andtest/js/node/sqlite/node-sqlite.test.tson x64 ASAN. This PR is a draft until Upgrade WebKit to 33ce3bf11a71 #46279 has landed.Test
test/bundler/bun-build-compile.test.tsgenerates functions in which each branch jumps over some padding and the next branch. With the right padding a branch's offset fits in 8 bits only until the next branch is widened, so each pass widens one more branch and the number of branches sets the number of passes.--bytecode, and runs the output from the bytecode cache. Without the fix the function with 15 branches and 39 padding statements aborts the build.Verified
Two release builds of Bun for Linux x64 from CI, both on WebKit with oven-sh/WebKit#860: one without the fix (WebKit
5e3554fa) and one with it (the preview build of oven-sh/WebKit#807, the same tree as5cfcac0f).bun build index.ts --compile --bytecode --format=esm --outfile=app, on@google-cloud/tasks7.2.1panic(main thread): abort() calledbun-build-compile.test.ts. On x64 ASAN the file has 79 pass and 0 fail in 118.30 s. A build without the new test took 135.49 s for the same file on that lane.Downsides