bun patch: use the isolated store folder when the hoisted path does not exist

Closed
RO
1mo ago
+489-26

Problem

  • With linker = "isolated", bun patch <name> puts the copy at the hoisted path of the package. The isolated layout can have nothing there. The root depends on b, b depends on a: bun patch a creates a real node_modules/a that stays after --commit and after every install.
  • A copy at node_modules/<workspace>/node_modules/<name> later stops the install from linking that workspace (error: Cannot find package 'w'). A hoisted path through a link puts the copy inside another store entry.
  • Cause: prepare_patch and do_patch_commit (src/install/PackageManager/patchPackage.rs) take the folder from the hoisted tree.

Fix

  • When nothing exists at the hoisted path, both commands use the store folder, node_modules/.bun/<entry>/node_modules/<name>. build_store names the entry, and the entry that the current workspace links wins. The install after --commit rebuilds it in place, and dependents load the edits before the commit.
  • bun patch --commit follows a symlink argument. git diff --no-index reads a symlink as a file and wrote new file mode 120000.
  • An entry that links into the global store is not used. An install puts that link back over a detached copy.
  • Verified: test/cli/install/bun-patch.test.ts (9 new tests, 3 updated, 11 fail without the fix). Also bun-install-patch.test.ts, isolated-install.test.ts.

Background

  • The hoisted tree is the npm layout that every lockfile stores.
  • The isolated linker installs each package once, in its store entry. It links a package only into the packages that declare it.
  • bun patch <pkg> replaces the package folder with a copy that shares no files with the cache. --commit diffs the copy against the cache and installs again.
Notes

Repro output (loopback registry, root depends on b, b depends on a, linker = "isolated").

Before (1.4.3-canary.1+b52d51348):

$ bun patch a
  node_modules/a
$ bun patch --commit a
node_modules/a: REAL dir [index.js, package.json]
node_modules/.bun/a@1.0.0/node_modules/a: [index.js, .bun-tag-41b812740eb4408b, package.json]
$ bun install
node_modules/a: REAL dir [index.js, package.json]

After:

$ bun patch a
  node_modules/.bun/a@1.0.0/node_modules/a
$ bun patch --commit a
node_modules/a: missing
node_modules/.bun/a@1.0.0/node_modules/a: [.bun-tag-..., index.js, package.json]

The three shapes that the new tests cover:

  • A transitive dependency. The hoisted path is node_modules/<name>.
  • A dependency of a workspace that the root does not depend on. The hoisted path is node_modules/<workspace>/node_modules/<name>. After the root adds "w": "workspace:*", node_modules/w must be a link. Symlinker::ensure_symlink keeps a real directory it finds there, so the leftover broke require("w").
  • A dependency that the hoisted tree nests under its dependent, node_modules/<dependent>/node_modules/<name>. node_modules/<dependent> is a link, so the copy landed inside the store entry of the dependent and shadowed the real package for it.

Why the store folder:

  • It is the only folder of the package in the isolated layout. Every dependent links to it, so the edits are live. The copy at the hoisted path was not: b resolves a through its own link.
  • It already works as a target. bun patch node_modules/.bun/a@1.0.0/node_modules/a and --commit with the same path behave correctly on the release build: the install finds no .bun-tag-<hash> of the new patch and rebuilds the folder.
  • The rule reads the file system, not the linker option. With the hoisted linker the hoisted path exists after an install, so nothing changes there. A path that is a link (the root or a linked workspace depends on the package) also keeps the current behavior.

Peer variants: a package with peer dependencies has one store entry for each peer resolution (peer-deps@1.0.0+<hash>). build_store is the function that bun install and bun prune use to name the entries. The search goes breadth first from the entry of the root, or of the workspace of the current directory, so bun patch in a workspace prepares the entry that this workspace loads. Its node_modules/<name> link then leads to the edits, for its own code and for --commit node_modules/<name>. The walk visits every entry of a workspace package, because the entry that a workspace: range links has no dependencies of its own. An entry that the walk does not reach is found in store order. The install after --commit patches every variant.

--commit <name> uses the same rule, so it names the prepared folder when both commands run in the same directory. When bun patch ran in another directory, the named entry has no changes. The output then lists the other store folders of the package with the command that commits each one. The command that bun patch prints works from every directory. I did not make --commit <name> search for the entry that differs from the cache: nothing marks a folder as prepared, an entry that a lifecycle script changed also differs, and for an already patched package every entry differs from the unpatched cache.

Global store (install.globalStore = true, off by default): node_modules/.bun/<entry> is a link into <cache>/links. bun patch on a store folder replaces that link with a real directory. The next install sees a real directory where the link belongs and replaces it (link_project_to_global_store), so edits made before --commit are lost. This happens on the release build with an explicit store path. To not make it reachable by name, the store folder is used only when the entry is a real directory. With the global store the copy is still made at the hoisted path. One test pins that the edits survive an install there.

Symlink argument: on the release build, bun patch --commit node_modules/b from packages/w, where that path is the link to the store folder, writes

diff --git a/packages/w/node_modules/b b/packages/w/node_modules/b
new file mode 120000

and the install fails with failed to parse patchfile: bad_file_mode. The existing test "relative node_modules/" (#12200, #12882) passed only because the copy was at the root node_modules/is-odd. That path no longer exists, so the command resolves the link.

Existing tests that changed: "inside workspace with hoisting > @types/ws@8.5.4" expected node_modules/@types/ws, a folder that only bun patch created, and read the leftover copy after --commit. The two "committing with the path bun suggested" tests wrote to a fixed root path. They now write to the path that bun patch prints, and assert that the root folder does not exist.

Related work:

  • #43365 restores the link when the copy replaced one (the root or a workspace depends on the package). It lists this bug under "What stays as it is". It adds a guard at the same place in do_patch_commit that refuses a folder that is a link. After this PR a link can lead to the folder that bun patch prepared, so if both land the guard must check the resolved folder. The resolved folder never reaches git diff as a link, which is what the guard protects.
  • #41453 replaces an empty directory in a link slot. It would repair node_modules/w, but not the copy.

Not in this PR:

  • A directory that an older build left at a hoisted path stays. bun prune reports it.
  • When an already patched package is committed again, the old .bun-tag-<hash> file gets into the new patch. The release build does the same with the hoisted linker (#19327).

Review follow-up (two rounds of automated review, every thread has an answer):

  • A reviewer found that bun patch in a workspace could prepare the peer variant of another workspace (first entry in store order). 53f2b4e6b6 fixes it with the breadth first search above, and two tests cover it. ed14bb8c2b makes the walk continue through a workspace that another workspace depends on, and adds the list of other folders to the No changes detected output. Two more tests cover these.
  • With the global store, bun patch <name> still writes through node_modules/<dependent> into <cache>/links when the hoisted tree nests the package under a dependent. Main does the same (checked with is-odd@3.0.1 and is-number@7.0.0: a second project that shares the cache loads the edited copy). The fix needs the installer to keep a detached node_modules/.bun/<entry>, so it is a separate change.

Tests run: bun-patch.test.ts 46 pass on Linux (debug, ASAN), and the new block plus "workspace interactions" pass on a Windows debug build. bun-install-patch.test.ts 31 pass, isolated-install.test.ts 85 pass.