install: fold the open bun install / pm / pack+publish PRs into one branch (171 PRs) - #39403
Jarred-Sumner wants to merge 259 commits into
Conversation
…bun update <name> (#38847)
… tarball versions (#38331)
…ut a trailing slash (#38698)
…rn.lock migration (#38795)
…json relative to the package (#38990)
…cal file: package (#38814)
… relative to the package (#38816)
…action folder name (#39011)
…n the path buffers (#38359)
…n the path join buffer (#38368)
… the path buffer (#38390)
…json does not fit the path buffer (#38575)
…nger than the path buffer (#38784)
…arball downloads (#38817)
…nd keep namedRegistries credentials out of bun.lock (#38997)
…pen PRs use BranchPackage gets the optional name and files fields and makeSharedRepo the repoName parameter that #38816 and #38681 add (both are in #39403). Commit names its ref in full and takes an explicit parent, so a commit can land on a tag or start a branch from another one, which is what the cases in #35566 need. The tarball builder is split so that the GitHub shaped part has the signature of the githubTarball helper in #39115.
…assertions (#39737) ### Problem - This file took 23s on the windows 11 aarch64 lane (build 101560). Process creation is the cost: 139 fixture processes, a `checkout`, `add`, `commit` and `push` per branch, a repo per test. - 8 assertions were a bare exit code. No test read bun's output or bun.lock. ### Fix - `makeSharedRepo` writes all branches with one `git fast-import`. The 16-branch repo is built once in `beforeAll` for the three tests that only read it. The two that move a branch build their own. `Bun.Archive` builds the tarballs. 13 fixture processes remain. - The helpers take the shapes the open PRs for this file use: `name?`, `files?` and `repoName` (#38816 and #38681, in #39403), and a `Commit` with an explicit ref and parent (the tag cases of #35566). Checks in Notes. - Every install now checks stdout (what each package resolved to), stderr, the installed markers and the bun.lock rows. The tarball tests also check that each tarball is downloaded once. The scenarios are unchanged. - Verified: `bun bd test test/cli/install/bun-install-git-deps.test.ts`, 15 of 15 Linux runs. Windows 11 aarch64: 22s to 12s debug, 20s to 11s release (Notes). ### Background - git's dumb HTTP protocol serves a bare repo as static files. `update-server-info` writes `info/refs`, the branch tips the tests read as expected commits. - `git fast-import` writes a stream of commits, file contents inline, to any refs in one process. `from <ref>^0` parents a commit on the ref as the repo has it. - For a `github:` dependency bun reads the resolved committish off the tarball's first entry, the `<owner>-<repo>-<committish>` directory. `Bun.Archive` writes a key ending in `/` as that entry. <details><summary>Notes</summary> Open PRs that touch this file. This PR can land before or after them. What each order costs: - #39403 (the install fold, which carries #38816 and #38681) has its own version of this file: the old helpers plus `name?`, `files?` and `repoName`, five tests, a `tar` based `packTarball` and a `writeProject` identical to the one here. Merging it onto this PR conflicts in the helper region (take this PR's) and duplicates `writeProject` (delete one). Checked: this PR's helpers with the fold's five tests appended run all 12 tests. This PR's 7 pass. The fold's 5 build their fixtures (the `files` of the `file:` test, the unnamed package, the `odd@repo.git` name) and fail only on the assertions its source changes make pass, for example the lockfile name `odd@repo.git@...` instead of `unnamed-package@...`. - #35566 adds `commitOn`, `pushRef` and `installedFromBranch`, which drive a work tree that no longer exists, plus 8 tests. On this PR they become `moveBranch` or `commitTo` calls and `git tag` / `git branch` in the bare repo. Checked with a scratch test: `git tag v1 main`, a commit that only `refs/tags/v2` reaches, `refs/heads/release/2.0` started from `main`, and `v1` moved to a new commit, in 3 `fast-import` runs. `main` kept its commit, and `bun install` of `#main`, `#v2`, `#release/2.0` and `#v1` installed `main`, `main-v2`, `release-2.0` and `v1-moved`. - #39115 adds `githubTarball(rootDir, files)` to the harness, built the same way. `tarballOf` here has that signature, so whichever PR lands second deletes the local copy. Timings. The old and the new file were run alternately on the same machine. - Windows 11 aarch64, 16 vCPUs, debug build (`bun bd`): old 22.2s, 21.0s, 21.8s. New 11.9s, 11.9s, 11.6s. Same machine, release canary (`USE_SYSTEM_BUN=1`, what the CI lanes run): old 19.8s, 19.9s, 20.1s, 20.4s. New 10.4s, 10.6s, 11.2s, 11.5s. All runs passed. The 16-branch test alone went from 20.4s to 10.1s and is now the whole wall time of the file. A cold install of it makes `bun install` spawn about 60 `git` processes (one bare clone, then `clone --no-checkout` and `checkout` per package, and one `git log` per dependency edge), and the test does two of them on purpose. - Linux x64 debug+ASAN, `bun bd test`: old 4.8s, 5.0s, 5.0s. New 4.0s, 4.0s, 4.1s (the final revision: 4.1s to 4.4s on a busier machine). A test file that only imports the harness takes 2.3s on this build, so the work of the file went from about 2.7s to about 1.7s. CPU time (user+sys, children included): 8.9s to 7.8s. - Processes, counted with `git` and `tar` shims on PATH. Old: 299. The fixtures spawned 139 of them (10 `init`, 25 `checkout`, 23 `add`, 25 `commit`, 25 `push`, 7 `update-server-info`, 24 `tar`) and `bun install` 160 (54 `clone`, 106 `git -C`). New: 173 in each of 5 runs. The fixtures spawn 13 (3 `init`, 5 `fast-import`, 5 `update-server-info`) and `bun install` the same 160. Each `git push` also forked `receive-pack` and `pack-objects`, which the shims do not see. The Linux numbers therefore understate the gain, and the Linux timings understate it more, because process creation is cheap there. Assertion changes, per test. - 16 branches, directly and transitively (2 attempts): stderr inline snapshot (`[17]` tasks: one clone, 16 checkouts), stdout with the commit each of the 16 branches resolved to, the 16 installed markers as one object, the 16 bun.lock rows including pkg-a's dependencies, exit code. Before: two `not.toContain` on stderr, the markers, exit code. - Tarball URLs and `github:` (2 attempts each): the same, with `[32]` and `[16]` tasks (download and extract per package), bun.lock rows with the integrity of the served bytes (and, for GitHub, the resolved tag `scope-pkg-x-0000000`), stdout with `#0000000` for GitHub, and each tarball downloaded exactly once per attempt although 11 (GitHub: 7) of them are depended on twice. - Lockfile on a cold cache: both installs check stdout, stderr (the frozen install prints nothing), the markers and the bun.lock rows, which the frozen install must leave unchanged. Before: the first install checked the markers and the exit code. - Hoisted and isolated moved branch: the warm install checks stdout, stderr, markers and bun.lock. After `moveBranch` the test checks that `pkg-m` points at a new commit and `pkg-n` does not. The cold install checks stdout (the locked commits, not the new tip), empty stderr, markers and unchanged bun.lock. Before: `not.toContain("error:")`, markers, exit code. - `git+file://`: stderr snapshot (`[2]`), stdout with the commit, marker, bun.lock row, exit code. Before: `not.toContain`, marker, exit code. Fixture details. - Below 100 objects `fast-import` writes loose objects (`fastimport.unpackLimit`), the same layout the old `push` produced. A pack would work too, because `update-server-info` lists it in `objects/info/packs` for dumb HTTP clients. - The commits carry a fixed committer date, so the SHAs of a repo depend only on its contents. The tests still read them from `info/refs` instead of hard-coding them. - The old tarballs were `tar -czf` of a directory, so they also started with the directory entry. The bun.lock integrity is the sha512 of the tarball bytes, which the test computes from the bytes it serves. - `bun install` prints the `+` lines in name order (a package's dependencies are sorted when its package.json is parsed, `src/install/lockfile/Package.rs`). `expectInstalled` sorts its expectations the same way. - `test/expected-durations.json` is not touched. CI regenerates it. - Commits: f33a5e5 the rewrite, c0dd54e the sort (review), 635fe1f the helper shapes above (self-review). </details>
… fit the path buffer (#39735) ### Problem - `bun install` from a project directory close to PATH_MAX aborts once a dependency declares a bin: `panic: index out of bounds: the len is 4096 but the index is 4096`. `bun link` with a global directory that deep aborts too. Found by an audit of this file. There is no user report. - `Linker::build_target_package_dir` (`src/install/bin.rs:1523`) and `build_destination_dir` (`bin.rs:1554`) copy `<node_modules>/<package>/` and `<node_modules>/.bin/` into their buffers with no length check. `link()` and `unlink()` call both before the bin name checks. The Zig version had the same copy (notes). ### Fix - Both builders return `None` when the directory is `MAX_PATH_BYTES` or longer, so no NUL fits after it. `link()` and `unlink()` then set `Error::Sys(ENAMETOOLONG)` and return. - The bin name checks in `link()` use this bound and this error. The OS refuses such a path with `ENAMETOOLONG`, so this is the OS error, one step earlier. A directory that does not fit never got a bin linked, so `unlink()` gets the same bound. - The callers already print `Linker::err` and exit 1. `bun install` prints `error: Failed to link has-bin: ENAMETOOLONG` and keeps the package installed. - Verified: `test/cli/install/bun-install-registry.test.ts`, block `directories longer than the path buffer`. Its three tests abort without the fix. Also the rest of its `binaries` block and the `isolated-install`, `native-binlink`, `bun-link` and `shebang-normalize` suites. ### Background - `bin::Linker` links the bins of one package. Per bin it builds two NUL terminated absolute paths in buffers of the caller: `<node_modules>/<package>/<file>` and `<node_modules>/.bin/<name>` (`<global bin dir>/<name>` for `-g` and `bun link`). - `MAX_PATH_BYTES` is the size of these buffers: 4096 on Linux, 1024 on macOS, about 96 KiB on Windows. PATH_MAX counts the NUL, so a usable path has at most `MAX_PATH_BYTES - 1` bytes. - One site of the path buffer sweep that #39403 folds. Written to be folded (notes). <details><summary>Notes</summary> Relation to the fold (#39403, branch `claude/install-mega`). The fold converts every other raw path write in `bin.rs` (#38954) but leaves these two builders as they are, so this is the rest of that file. The diff touches the two builders and their two call sites only. Checked with a cherry-pick onto the fold head `65bd4dd91d`: `bin.rs` merges without a conflict. The test block lands inside the fold's `binaries` block. It needs two things there: the import conflict of two adjacent lines, and `const { packageDir, env } = await setupTest();` as the first line of its two test bodies, because the fold creates the project directory per test. The helpers take the directory and the env as arguments for that reason. `componentsUpTo` has the shape of the helper #38954 adds, so one of the two can go. Sibling sites, each with its own open PR. They are independent of the two builders, so each lands on its own: - `<package>/<bin file>` under a package directory that still fits: `resolve_bin_target` joins it in a thread local buffer and aborts on main. #38954 checks those joins in this file, #39658 makes the thread local joins spill to the heap. A 4065 byte cwd with `typescript` is in that range. From the point where the directory itself no longer fits, this PR is the one that applies. - The second `bun install` in the same project aborts in `PackageInstall::uninstall_before_install` (`src/install/PackageInstall.rs:1967`), a thread local `join_abs_string` of `<node_modules>/.old-<random>`. #39658 covers it with no change at that caller. - `bun unlink` with such a global directory aborts in `unlink_command.rs:115` before it reaches `unlink()`, in its own thread local `join_abs_string_z` of `<global node_modules>/<name>`. #39658 covers that one too. - The isolated linker aborts while it builds the store path (`isolated_install/Installer.rs:2784`), before it links bins. That is the area of #37400 and #37424, so there is no isolated test here. Why `unlink()` has no test of its own. Its new early return has no observable effect: one byte below the bound, `bun unlink` prints `success: unlinked package` and exits 0 with and without this change, because `bun link` never created a bin for such a directory, and `unlink_command` does not read `Linker::err` (`unlink_command.rs:216`, as before). At the bound it aborts earlier, see above. Callers of `link()`: the hoisted installer (`PackageInstaller::link_tree_bins`, prints `Failed to link <alias>: <error>`), the isolated installer (`Step::Binaries` and `link_dependency_bins`, print `failed to link binaries for package`), and `link_command` (prints `failed to link bin due to error ENAMETOOLONG`). Test shapes. Each test builds a directory under the test's tmp directory out of names of at most 200 bytes, so that one path is exactly `MAX_PATH_BYTES` long: - `<project>/node_modules/has-bin`: the `has-bin/` prefix is one byte too long (`build_target_package_dir`, through `bun install`). - `<project>/node_modules/.bin` with a package named `a`: the package directory fits and `.bin/` is one byte too long (`build_destination_dir`, through `bun install`). - `<global dir>/node_modules/has-bin` through `BUN_INSTALL_GLOBAL_DIR`: `build_target_package_dir` through `bun link`, which also runs the global branch of `build_destination_dir`. After the command, the test renames the first long directory name so that it can inspect and delete the paths below the limit. Windows is skipped: its buffer is larger than any path the OS accepts. History. The Zig builders (`bin.zig`, `buildTargetPackageDir` and `buildDestinationDir`) had the same unchecked copies. A release build of Bun 1.3 wrote past the buffer and went on, which is why 1.3.14 exits 0 here. That was not correct behavior, so this is not a regression test case. `bun-link.test.ts` has one failure with a debug build on main as well: `should link dependency without crashing` expects exact stdout, and a debug build prints the failure trace. It is unrelated to this change. </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/cli/install/bun-install-registry.test.ts <!-- robobun:evidence:end -->
### Problem - `bun pm ls` without `--all` prints a root dependency once per entry the root has for it. A workspace `a` that is also in the root's `devDependencies` prints twice under a `node_modules (1)` header. So does a name declared in two groups (`typescript` in #8283's example). - Cause: `src/runtime/cli/package_manager_command.rs:644` prints the root's dependency list as is: one entry per declaration plus one per workspace member. `node_modules` has one folder per name. ### Fix - Stable sort the root's entries by name and drop the later entries of a name (`dedup_by`). `--trusted` filters the result. - Correct because one name is one `node_modules/<name>` folder, the same collapse `Tree::hoist_dependency` applies. The list is in the tree builder's order, so the entry kept is the one the tree kept. An alias has its own name and keeps its row. The listing does not read the stored tree, so an older `bun.lockb` prints as before (Notes). - Verified: `test/cli/install/bun-pm.test.ts`. Three new tests (bun.lock, bun.lockb, `--trusted`) fail on the released bun. One more pins that a root optional peer bound to a dependency's copy stays listed. On this repo's lockfile the only change is the removed `@types/bun` row. ### Background - Each package owns one range of `buffers.dependencies`. The root's range holds its declared dependencies plus one `Behavior::WORKSPACE` entry per workspace member, sorted by group and name. Entries of one name share one folder. - The hoisted tree (`buffers.trees`, `buffers.hoisted_dependencies`) models `node_modules`: one tree per folder, one entry per name. `--all` prints it and the header `(N)` counts its entries. The default listing prints only the root's range. <details><summary>Notes</summary> Repro without a registry: ```sh mkdir -p d/packages/a && cd d echo '{"name":"root","workspaces":["packages/*"],"devDependencies":{"a":"workspace:*"}}' > package.json echo '{"name":"a","version":"1.0.0"}' > packages/a/package.json bun install && bun pm ls ``` Released bun prints `a@workspace:packages/a` on two lines, three when `a` is also in `dependencies`. `bun pm ls --all` prints it once. `bun pm ls --trusted` with `trustedDependencies: ["a"]` prints it twice. In this repo `bun pm ls` prints `@types/bun@workspace:packages/@types/bun` twice. - The first version of this PR printed the tree's root folder filtered to the root's id range. A self-review found a case it drops: a root optional peer that a dependency provides. When the tree builder binds that peer late, it stores the folder entry under the provider's dependency id (`Tree.rs`, `ResolveReplace`). bun 1.4.0 re-hoists on load and on a second pass stores the root's own id, but a `bun.lockb` written by bun 1.3.x keeps the provider's id, and bun does not rewrite a lockfile that needs no changes. Checked with a `bun.lockb` written by bun 1.3.14 from the `optional-peer-hoist-provider` and `-target` registry fixtures: the tree version listed only the provider, the current version lists both, as 1.3.14 and the released 1.4.0 do. The new optional peer test covers the fresh lockfile shape of this case. - Dedupe by package id was rejected: `bar` and `bar-alias: npm:bar` share one package id but are two folders. The test keeps both rows. - The `--trusted` test uses bun.lock only. bun.lockb stores only hashes of `trustedDependencies`, and `has_trusted_dependency` does not trust a hash alone since #31339, so `--trusted` lists nothing from a bun.lockb. Released bun does the same. Not related to this change. - A real conflict (workspace `a` plus `"a": "file:..."` at the root) does not reach `pm ls` today: `bun install` writes a duplicate package path and the lockfile fails to load (the area of #33156). - #8283 asks for grouped output and `--json`. This PR removes the duplicate row in its example and nothing else, so it does not close that issue. - Open PRs that touch this branch and do not fix this: #38923 (filter by what is on disk), #38952 (header wording). The fold branch of #39403 has the same loop. - Suites run with the debug build: `test/cli/install/bun-pm.test.ts` (22 pass), `bun-install-lifecycle-scripts.test.ts -t trusted` (55 pass), `bun-install-registry.test.ts -t "migration is out of sync"`, `test/regression/issue/24502`. `cargo fmt --check` is clean. `bun pm ls --all` on this repo is unchanged byte for byte. </details> <!-- robobun:evidence:begin --> --- **[review]** gate passed · iteration 0 · 2 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 3 FAILED $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/cli/install/bun-pm.test.ts bun test v1.4.0 (6e906e4) test/cli/install/bun-pm.test.ts: (pass) should list top-level dependency [386.82ms] (pass) should list all dependencies [333.90ms] (pass) should list top-level aliased dependency [329.02ms] (pass) should list aliased dependencies [346.24ms] (pass) should list only trusted dependencies with --trusted [464.07ms] (pass) should list only trusted dependencies with --all --trusted [339.68ms] (pass) should list trusted transitive dependencies under untrusted parents with --all --trusted (isolated) [356.01ms] (pass) should list nothing with --trusted when no dependencies are trusted [346.58ms] 597 | expect(await exists(join(package_dir, lockfile))).toBeTrue(); 598 | urls.length = 0; 599 | 600 | const [stdout, stderr, exitCode] = await spawnAndCollect("pm", "ls"); 601 | expect(stderr).toBe(""); 602 | expect(normalizeBunSnapshot(stdout, package_dir)).toMatchInlineSnapshot(` ^ error: expect(received).toMatchInlineSnaps ... (truncated) release without fix: 3 FAILED bun test v1.4.0-canary.1 (6e906e4) test/cli/install/bun-pm.test.ts: (pass) should list top-level dependency [17.59ms] (pass) should list all dependencies [15.04ms] (pass) should list top-level aliased dependency [18.13ms] (pass) should list aliased dependencies [26.89ms] (pass) should list only trusted dependencies with --trusted [29.53ms] (pass) should list only trusted dependencies with --all --trusted [30.36ms] (pass) should list trusted transitive dependencies under untrusted parents with --all --trusted (isolated) [19.13ms] (pass) should list nothing with --trusted when no dependencies are trusted [20.57ms] 597 | expect(await exists(join(package_dir, lockfile))).toBeTrue(); 598 | urls.length = 0; 599 | 600 | const [stdout, stderr, exitCode] = await spawnAndCollect("pm", "ls"); 601 | expect(stderr).toBe(""); 602 | expect(normalizeBunSnapshot(stdout, package_dir)).toMatchInlineSnapshot(` ^ error: expect(received).toMatchInlineSnapshot(expected) "<dir> node_modules (5) ├── bar@0.0.2 + ├── bar@0.0.2 ├── bar-alias@0.0.2 ├── ws-once@workspace:packages/ws-once + ... (truncated) ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: all passed $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/cli/install/bun-pm.test.ts bun test v1.4.0 (6e906e4) test/cli/install/bun-pm.test.ts: (pass) should list top-level dependency [381.86ms] (pass) should list all dependencies [333.18ms] (pass) should list top-level aliased dependency [351.49ms] (pass) should list aliased dependencies [347.67ms] (pass) should list only trusted dependencies with --trusted [463.21ms] (pass) should list only trusted dependencies with --all --trusted [348.99ms] (pass) should list trusted transitive dependencies under untrusted parents with --all --trusted (isolated) [321.16ms] (pass) should list nothing with --trusted when no dependencies are trusted [309.86ms] (pass) should list a workspace the root also depends on once (bun.lock) [541.88ms] (pass) should list a workspace the root also depends on once (bun.lockb) [314.14ms] (pass) should list a trusted workspace the root also depends on once with --trusted [315.39ms] (pass) should list a root optional peer that a dependency provides [328.74ms] (pass) should remove all cache [465.82ms] (pass) bun pm m ... (truncated) release with fix: all passed $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 683ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [1/5] gen generated_host_exports.rs generated_host_exports.rs: 92 exports (host=3, lazy=10, generic=79, rust=0); 240 extern-C blocks audited [1/5] cargo bun_bin → libbun_rust.a (--target x86_64-unknown-linux-gnu) nightly-2026-07-20-x86_64-unknown-linux-gnu unchanged - rustc 1.99.0-nightly (9f36de775 2026-07-19) �[1m�[92m Compiling�[0m bun_runtime v0.0.0 (/workspace/bun/src/runtime) �[1m�[92m Compiling�[0m bun_bin v0.0.0 (/workspace/bun/src/bun_bin) �[1m�[92m Finished�[0m `release` profile [optimized + debuginfo] target(s) in 4m 36s [2/5] link bun-profile [4/5] strip bun [4/5] bun-profile --revision 1.4.0-canary.1+b263001b0 [build] done bun test v1.4.0-canary.1 (b263001) test/cli/install/bun-pm.test.ts: (pass) should list top-level dependency [15.40ms] (pass) should list all dependencies [11.19ms] (pass) should list top-level aliased dependency [9.40ms] (pass) should list aliased dependencies [9.71ms] (pass) should list only trusted dependencies with --trusted [10.18ms] (pass) should list on ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` src/runtime/cli/package_manager_command.rs | 8 +- test/cli/install/bun-pm.test.ts | 150 ++++++++++++++++++++++++++++- 2 files changed, 152 insertions(+), 6 deletions(-) ``` </details> **gate history** · 1 passed · 0 rejected · iteration 0 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/runtime/cli/package_manager_command.rs 5 4 0 test/cli/install/bun-pm.test.ts 10 9 0 ``` </details> **root cause** · written by the author bot The root package's dependency list receives one entry per workspace member plus one per declared dependency, so a workspace that the root also declares as a dependency has two entries resolving to the same package, and the default bun pm ls loop printed each of them. The fix sorts the root's entries by name with a stable sort and then collapses adjacent entries with the same name, so each package is printed once, while the --trusted filtering in the same branch is unaffected. <!-- robobun:evidence:end -->
…ites it (#40029) ### Problem - `bun add` in a project that still has `pnpm-lock.yaml` and `pnpm-workspace.yaml` crashes or writes garbage into `package.json`. Sentry BUN-4NQS (1.4.0): `Segmentation fault at address 0xF3E9AD2800000000` in `extend_from_slice` under `EString::resolve_rope_if_needed`, `print_property`, `edit_after_resolve`. `bun update -i` hits the same bug (#23694). - The cause is `update_package_json_after_migration` (`src/install/pnpm.rs:2334`). It edits the cached root `package.json` tree in place, replaces `entry.source.contents`, and returns. The tree now points into the freed contents, and its new nodes live in the thread-local AST `Store`, which the install resets (`src/install/lockfile/Package.rs:1676`) before the write-back prints the tree. ### Fix - Print the edited tree into the cache entry and call `MapEntry::reparse_root`, as `store_entry` (`add_remove_with_filter.rs:280`) does. When the migration returns, the entry owns its tree again. - A print or re-parse failure now crashes like the other editors do. Before, a print failure left the dangling tree in the cache. - Verified: two new tests in `test/cli/install/migration/pnpm-lock-v9.test.ts` (`bun add`, and the `bun update -i` shape from #23694). Both fail on main with the debug build and with the 1.4.0 release. The other `pnpm-*` files pass. ### Background - `WorkspacePackageJSONCache` keeps one `MapEntry` per `package.json`: the contents, a parsed tree, and the arena that owns the tree. `bun add` prints the root entry again after the install (`package_json_write_back.rs:63`). - `Expr::init` allocates nodes in a thread-local `Store`. `initialize_store()` resets it before every `package.json` parse. `reparse_root` rebuilds the tree in an arena the entry owns. - String nodes point into the entry's contents, so a writer that replaces the contents has to rebuild the tree. <details><summary>Notes</summary> - Repro without a registry: a `package.json`, a `pnpm-lock.yaml` with `lockfileVersion: '9.0'` and empty importers, a `pnpm-workspace.yaml` with a `packages:` list, then `bun add ./some-folder`. The 1.4.0 release writes `"\x00\x00\x00e"` in place of the `name` key, so `package.json` is no longer valid JSON. The debug build reports `heap-use-after-free` in `EString::eql_bytes` under `edit_after_resolve`, freed at `pnpm.rs:2759` (the `source.contents` assignment on main). Any yaml field the migration moves (`packages`, `catalog`, `overrides`, `patchedDependencies`) or a `pnpm.overrides` block triggers it, so every pnpm workspace does. - Why the two builds differ: the debug `Store` reset poisons its blocks with `0xAA`, so the second print crashes every time. In release, the freed contents buffer gets a free-list pointer written over its first bytes (the `name` key), and the `Store` slots are reused by the install's parse of the root `package.json`, which gives the rope walk in the Sentry stack. - Readers of the root entry's tree after the lockfile load, all covered by the one re-parse: the `bun add` / `bun update <name>` / `bun link` write-back and `sync_lockfile` (the Sentry crash), `bun update -i` (#23694), `bun update` with patches (`warn_orphaned_patches`), `bun dedupe` with an empty root, `bun audit --fix`, and the `--frozen-lockfile` error message (`overrides_field_name`). Plain `bun install` only reads `entry.source` after the migration, so it does not crash. - The open #38775, #38754, and #38804 each add this same print and re-parse to `update_package_json_after_migration` as part of larger behavior changes (all three are in the fold branch #39403). None of them is in 1.4.0. This PR is only the crash fix, so it can land on its own. Whichever lands second gets a conflict in this block that resolves to the same code. - Not changed here: the `patch --commit` branch in `update_package_json_and_install_with_manager_with_updates` (`updatePackageJSONAndInstall.rs:573`) replaces the root entry's contents the same way without a re-parse. Nothing reads that entry's tree afterwards today, and #38754 reworks that block. Also separate: `bun remove` during a pnpm migration writes its pre-install print back to disk and loses the fields the migration moved. That is a different bug and is tracked on its own. - Suites run with the debug build: `pnpm-lock-v9` (84 pass), `pnpm-migration`, `pnpm-lock-migration`, `pnpm-comprehensive`, `pnpm-migration-complete` (the last one is a single test that sits near the 5 s default timeout when run next to other files, unrelated to this change). `complex-workspace` and `yarn-lock-migration` need network access and fail here with the release build too. </details> <!-- robobun:evidence:begin --> --- **[review]** gate passed · iteration 0 · 2 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 2 FAILED $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" "test/cli/install/migration/pnpm-lock-v9.test.ts" bun test v1.4.1 (4448a2e) test/cli/install/migration/pnpm-lock-v9.test.ts: (pass) pnpm-lock.yaml v9 > v9 git and userinfo-tarball references migrate [372.39ms] (pass) pnpm-lock.yaml v9 > v9 alias in snapshot optionalDependencies gets the npm: prefix [531.51ms] (pass) pnpm-lock.yaml v9 > reports the missing packages entry of a workspace importer [188.78ms] (pass) pnpm-lock.yaml v9 > reports the missing packages entry of a transitive dependency [164.22ms] (pass) pnpm-lock.yaml v9 > reports the missing packages entry [227.90ms] (pass) pnpm-lock.yaml v9 > reports an importer whose package.json is missing [170.19ms] (pass) pnpm-lock.yaml v9 > registry-qualified dep path resolves from the configured registry with a warning [258.83ms] (pass) pnpm-lock.yaml v9 > named registries > built-in npmjs: entries record the npmjs registry [214.16ms] (pass) pnpm-lock.yaml v9 > named registries > namedRegistries entry pointing at the configured registry needs no warning [445.93ms] (pass) pnpm-lock.yaml ... (truncated) release without fix: 2 FAILED bun test v1.4.0-canary.1 (4448a2e) test/cli/install/migration/pnpm-lock-v9.test.ts: (pass) pnpm-lock.yaml v9 > v9 git and userinfo-tarball references migrate [9.21ms] (pass) pnpm-lock.yaml v9 > v9 alias in snapshot optionalDependencies gets the npm: prefix [49.92ms] (pass) pnpm-lock.yaml v9 > reports the missing packages entry [5.92ms] (pass) pnpm-lock.yaml v9 > reports the missing packages entry of a workspace importer [4.76ms] (pass) pnpm-lock.yaml v9 > reports the missing packages entry of a transitive dependency [3.30ms] (pass) pnpm-lock.yaml v9 > reports an importer whose package.json is missing [3.40ms] (pass) pnpm-lock.yaml v9 > registry-qualified dep path resolves from the configured registry with a warning [13.91ms] (pass) pnpm-lock.yaml v9 > named registries > built-in npmjs: entries record the npmjs registry [9.93ms] (pass) pnpm-lock.yaml v9 > named registries > namedRegistries entry pointing at the configured registry needs no warning [20.87ms] (pass) pnpm-lock.yaml v9 > named registries > namedRegistries entry pointing at another registry is used for the tarballs [19.37ms] (pass) pnpm-lock.yaml v9 > named registries > two packages from one unknown re ... (truncated) ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: all passed $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" "test/cli/install/migration/pnpm-lock-v9.test.ts" bun test v1.4.1 (4448a2e) test/cli/install/migration/pnpm-lock-v9.test.ts: (pass) pnpm-lock.yaml v9 > v9 git and userinfo-tarball references migrate [309.70ms] (pass) pnpm-lock.yaml v9 > v9 alias in snapshot optionalDependencies gets the npm: prefix [405.45ms] (pass) pnpm-lock.yaml v9 > reports the missing packages entry [167.07ms] (pass) pnpm-lock.yaml v9 > reports the missing packages entry of a workspace importer [174.28ms] (pass) pnpm-lock.yaml v9 > reports the missing packages entry of a transitive dependency [172.30ms] (pass) pnpm-lock.yaml v9 > reports an importer whose package.json is missing [157.78ms] (pass) pnpm-lock.yaml v9 > registry-qualified dep path resolves from the configured registry with a warning [197.44ms] (pass) pnpm-lock.yaml v9 > named registries > built-in npmjs: entries record the npmjs registry [193.97ms] (pass) pnpm-lock.yaml v9 > named registries > namedRegistries entry pointing at the configured registry needs no warning [335.05ms] (pass) pnpm-lock.yaml ... (truncated) release with fix: all passed $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) target linux-x64-gnu build type Release build dir ./build/release revision 2a65176 features baseline 23 deps, 129 codegen, 1172 objects in 1101ms ninja: Entering directory `/workspace/bun/build/release' [1/1244] install /workspace/bun bun install v1.4.0-canary.1 (4448a2e) Checked 26 installs across 63 packages (no changes) [7.00ms] [2/1244] install /workspace/bun/packages/bun-error bun install v1.4.0-canary.1 (4448a2e) Checked 1 install across 2 packages (no changes) [2.00ms] [3/1244] gen ErrorCode+*.h [4/1244] fetch tinycc [tinycc] up to date [5/1243] install /workspace/bun/src/node-fallbacks bun install v1.4.0-canary.1 (4448a2e) Checked 111 installs across 104 packages (no changes) [10.00ms] [6/1243] gen bindgenv2 [7/1243] fetch zlib [zlib] up to date [8/1243] fetch libjpeg-turbo [libjpeg-turbo] up to date [9/1216] gen node-fallbacks/react-refresh.js Bundled 1 module in 10ms react-refresh.js 4.81 KB (entry point) [10/1216] gen .bind.ts → GeneratedBindings.cpp [11/1216] gen ProcessBindingConstants.lut.h Generating /workspace/bun/bu ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` src/install/pnpm.rs | 37 ++------ test/cli/install/migration/pnpm-lock-v9.test.ts | 113 ++++++++++++++++++++++++ 2 files changed, 121 insertions(+), 29 deletions(-) ``` </details> **gate history** · 1 passed · 0 rejected · iteration 0 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/install/pnpm.rs 4 3 0 test/cli/install/migration/pnpm-lock-v9.test.ts 3 2 0 ``` </details> <!-- robobun:evidence:end -->
|
@Jarred-Sumner Status on #37150 and its stacked follow-up #37283: Both predate the Aug 12 cutoff, but overlap substantially with this branch. #37283 currently has 8 conflict hunks against Their core behavior is not covered by the fold:
Several folded PRs overlap with parts of the implementation: #39096 ( I’d prefer #37150 and #37283 to remain their existing stack and land separately before the mega branch, rather than folding their remaining behavior into it. That preserves the reviewable boundary between the package-selection fix and the anonymous-URL follow-up. Also related, but not mine: #36827 (@robobun) is a small case-insensitive |
Folds the open
bun install/bun pm/ pack+publish PRs from Aug 12–17 into one branch so they can be reviewed and landed together. Each original PR is one squashed commit here (<title> (#N)), in dependency order, sogit log/git blameattribute every hunk to its PR; conflict resolutions between overlapping PRs live in the later PR's commit. Cross-PR API reconciliation is in themega-merge fixup:commits at the end, so the per-PR commits after #38804 (e2cf8c4305) do not build on their own until0adcafaf8b— bisect across the fold with--first-parenton the fixups, not inside it.171 PRs folded, 3 excluded (see below); #38557 was re-ported by hand onto the newer
audit_command.rs. A simplification pass then roughly halved the netsrc/addition (see below).cargo check --workspaceandcargo clippy --workspaceare clean, the debug build links, and all 94test/cli/install/**files pass locally on Linux (debug+ASAN); see Verification.Not folded
prepareis rarely needed and runs a nested install (fetching and, via prepare, executing third-party devDependencies chosen by the git repo) outside the project's bunfig protections — too much attack surface for the benefitSecurity screen
Every folded diff was reviewed for regressions (path traversal, credential leakage, TLS/integrity weakening, lifecycle-script trust widening, terminal injection, new
unsafeon untrusted input). One PR was dropped for it (#38777 above) and one more on review (#38810). Items a reviewer may still want to eyeball — none were judged exploitable regressions:.bun-tag) no longer apply to any lockfile created after this lands. The reduction is defence-in-depth only: the checks are keyed on a digit the lockfile tamperer also controls, main already stamps v1 whenever bun's own output would trip them, a tamperer can supply a matching integrity for their own tarball anyway, and the git tag is re-validated at checkout/extract time (repository.rs:923, extract_tarball.rs:484; github tags stay checked at every version).?after#leak the fragment into the request path, though no attacker-chosen tarball results.file:targets inside a registry/tarball/git package are now migrated as package-relative folder rows without normalising the sub-path, so a hand-edited key with..is no longer skipped at migration time and containment relies on the installer (hoisted linker checks it, with an overrides-name exemption; isolated linker does not).Product-behaviour changes that need a maintainer decision before this lands: #38741 (new
bun.lockfiles are stampedlockfileVersion: 1again — its own PR defers this to a maintainer, and it obsoletes the open #36464; the sibling snapshots are already re-stamped inside the fold, so pulling it back out is one revert +fixlv), #38797 (libcfiltering needs the full registry document for every optional dependency on every OS, since npm's abbreviated document omitslibc; measured:@esbuild/linux-x6443 KB → 54 KB gzipped,@rollup/rollup-linux-x64-gnu40 KB → 58 KB, one-time per package per manifest cache, in exchange for not downloading the other libc's multi-MB tarballs on Linux and a host-independent lockfile), #38804 (a migrated lockfile and its package.json edits are only written when the command saves a lockfile), #38923 (bun pm lslists what is installed and exits 1 without node_modules), #38813 (pack/publish resolveworkspace:/catalog:from package.json files, not bun.lock).Simplification pass (after folding)
The PRs were written independently, so the folded diff carried duplicate helpers and a lot of narrative comments. A follow-up pass (the
simplify(...)commits) cut thesrc/diff against main from +12,081 / −6,723 (net +5,358) to +10,284 / −7,559 (net +2,725) with no behaviour change intended and the fulltest/cli/install/**suite green before and after. Main items:Lockfile::packages_named/package_satisfying/patched_package_satisfyingreplaces nine "package_index → first candidate whose resolution satisfies a range and a predicate" helpers spread over lockfile.rs, bun.lock.rs, PackageManagerEnqueue.rs, update_transitive.rs and prune.rs.update_transitive.rs(Hold a transitive update that would re-fork a deduped package #38919/install: keep one copy of a package when one version satisfies every range #38770) no longer re-implements the resolver's rules: it callskeep_locked_if_ahead,is_named_update_row(split out ofshould_update) anddedupe::applied_overridedirectly, and itsPlan/Planned/InstanceEdges/KeepLockedscaffolding is gone; reachability useslockfile::reachable.resolve_path::join_abs_string_buf_z_checked) and one spill helper replacebin.rs::join_z_checked,PackageManager::join_path_z, and pack'snormalize_buf_spill; the bunx install lock (bunx: serialize concurrent installs into the shared cache dir with a file lock #37851) is a singleInstallLockRAII type.append_store_path_at(buf, entry, Which)family;PackageInstall::destination_dir_subpath_bufand theDestinationSubpathguard removed in favour ofjoin_z_spill; staging installs re-target aPackageInstallinstead of threadingdest_subpaththrough every backend;remove_linkshared.Entry::set_git_source, root/workspace package.json parsing shared; pnpm.rs: one merge-basedcopy_into_root, oneworkspacesobject path;finish_migrationbuilds the one error shape.RegistryPath,set_scope_registryand the second credential lookup removed in favour ofscope_for_package_name+UrlAuth::find;fmt::for_terminal(x)=EscapeControlChars(Redacted(x))at every call site;MinimumReleaseAgeExcludesis one list.Two small behaviour fixes fell out of the review agents' cross-checks and are their own commits:
bun audit fixnow rewrites a matching rule inresolutionsas well asoverrides(the parser reads both since #38811), and a failed root package.json write saysfailed to write package.json: <errno>instead offor workspace ''.Conflict resolutions that changed a PR's code (not just context)
*peers after their siblings, not on arrival order #37713 × install: resolve a range onto an existing version only when every install has it #38832: kept install: resolve deferred*peers after their siblings, not on arrival order #37713's rule that a deferred peer never range-matches on sight (no*exemption); install: resolve a range onto an existing version only when every install has it #38832'sAppendedFor/settled_package_countmodel replaces theexact_pinnedguard, with install: keep one copy of a package when one version satisfies every range #38770's direct-dependency clause folded into the reusable predicate.resolve_peer_dep_version_based_amongkeeps install: rebind ranged peers whose target the saved tree drops #38768's candidate filter but drops the highest-candidate fallback install: keep a peer nothing in bun.lock satisfies where the file records it #38892 removed.collect_bundled_depsrefactor: took install: keep bundleDependencies from every dependency group when parsing a registry manifest #38857's per-versionbundled_dependencieslist.pnpm.rs: oneupdate_package_json_after_migration(lockfile: Option<&mut Lockfile>, …); everything is reported as “copied … in package.json” (pnpm keeps reading its own block), the write is deferred to lockfile save (install: write a migrated lockfile and its package.json edits only when the command saves a lockfile #38804), andbun pm migrateon a read-only package.json reports the failed write but keeps the migrated bun.lock (install: only open package.json for writing in the commands that rewrite it #38745's intent).url_authmodel instead of adding a second.npmrccredential table;--registrygoes through the sameset_default_registrypath unless the URL itself carries credentials (install: send credentials embedded in a registry URL that comes from an env var #38834).EscapeControlChars(redacted(…))(redact inside, escape outside); oneEscapeControlCharsimplementation (the multiline-capable one from pm view: escape control characters coming from the registry #38536) is kept.symlink_dependenciesreturnsResult<bool, TaskError>soLinkPathTooLongstill names the package.fail_fnremoved as in the PR; the invalid-name path from install: reject dependency names containing control characters #38615 now logs like the other root-resolution errors.append_dependency_path, which now joins with the..-normalising checked join, so alink:/file:target that only overflows once joined reportsLinkPathTooLonginstead of hitting the "packages have no store path" unreachable.count_blocked_scriptsreads lifecycle scripts from the final node_modules location for local (non-global-store) entries.bun updateno longer treats a workspace member that was removed fromworkspacesas a direct-dependency owner, so its former deps can move.describe.concurrent).failed to write package.json: EACCESafter the lockfile is saved (test updated).bun update mookeeps a>=0.1.0literal that still covers the new version (main's behaviour); the test asserted^0.2.0.packageDirnow callsetupTest().Folded PRs
Peer binding, hoisting, bun.lock load/round-trip, resolver dedupe (21)
*peers after their siblings, not on arrival order #37713 install: resolve deferred*peers after their siblings, not on arrival orderbun update / bun add package.json edits, overrides (7)
Lockfile migration (yarn / pnpm / npm) (17)
file: / link: / tarball / git local dependencies (16)
Path-buffer overflows: panic → error (15)
Terminal control-character escaping and credential redaction (9)
Registry auth, .npmrc, proxies, URL parsing (16)
bun pm subcommands, audit, help text (14)
bun pm pack / bun publish (13)
Linkers, bins, node_modules layout (9)
Lifecycle scripts, env, minimum-release-age, semver, auto-install (16)
Test-only changes (13)
Platform-specific (5)
Verification
cargo check --workspacecargo clippy --workspace --no-depstest/cli/install/**,test/cli/run/**,test/regression/issue/*install*locally with the debug build. Remaining local-only failures are environmental and reproduce on main in this sandbox:bun-pm.test.tsread-only package.json cases (run as root;nobodycannot exec a binary under/root),thread-spawn-failure(LD_PRELOAD shim vs ASAN).