install: link over an empty directory left at node_modules/<pkg> with the isolated linker

Closed
RO
2mo ago
+111-3

Problem

  • With linker = "isolated", if node_modules/<pkg> already exists as an empty directory, bun install prints + <pkg>@x.y.z and exits 0, but the path stays an empty directory and the package's bins are missing from node_modules/.bin. Re-running install repairs nothing.
  • This is how bun bd breaks on a branch that adds a root dependency: ninja creates the directory of every declared output, including node_modules/<dep>/, before running bun install, and esbuild then fails with Could not resolve "<pkg>". (The ENOSPC suspected in the report was unrelated.)
  • Cause: since #29489 the isolated linker leaves any real directory at the link path alone, assuming it is a bun patch workspace. An empty directory matches too, so the link is silently never created.

Fix

  • When the link path is a real directory, rmdir it first. If that succeeds the link is created as usual; if it fails the directory is left alone, as today.
  • Correct because rmdir only removes an empty directory (on Windows as well): an empty directory has nothing to preserve, and a directory with contents, the only thing #29489 protected, is never deleted.
  • The same code creates the .bun/node_modules/<pkg> links, so those heal too. The hoisted linker does not link, so it is unaffected.
  • Verification: four new tests. Three fail on the current release and pass with this change; the fourth pins that a directory with contents is preserved. Existing patch tests still pass on a debug build.

Background

  • Isolated linker: each package is unpacked once into a store at node_modules/.bun/<pkg>@<ver>/, and node_modules/<pkg> in the root or a workspace is a symlink into that store. A missing link is a missing dependency even when the store is complete.
  • The changed function reads the link at the destination: right target means done, nothing there means create it, anything else is in the way. Only the "in the way and it is a directory" arm changes here.
  • bun patch <pkg> replaces node_modules/<pkg> with a real directory holding a copy of the package for the user to edit until --commit; #29489 made install leave it alone. Such a directory always has contents.
  • Bin linking runs after package linking and, like npm, silently skips a bin whose target file is missing, which is why the broken state produces no error.

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/isolated-install.test.ts

Original description

Repro

Any project using linker = "isolated" that already has a node_modules/.bun:

# add a new dependency to package.json, then
mkdir node_modules/<pkg>
bun install          # prints "+ <pkg>@x.y.z", exits 0
ls -ld node_modules/<pkg>   # still an empty directory, not a symlink
ls node_modules/.bin        # <pkg>'s bins are missing
bun install          # exits 0 again, nothing is repaired

This is how bun bd breaks on a branch that adds a root dependency: the build's install edge declares node_modules/<dep>/package.json as implicit outputs of bun install, and ninja creates the parent directory of every output before running the command (checked with a three line build.ninja). The store entry node_modules/.bun/<pkg>@<ver>/ and the hoisted .bun/node_modules/<pkg> link were written correctly; only node_modules/<pkg> stayed an empty directory, and esbuild then failed with Could not resolve "<pkg>". (The report that came in suspected ENOSPC on the same machine; that turned out to be unrelated, the repro above needs no disk pressure.)

Cause

Root and workspace dependencies are linked with Symlinker::ensure_symlink(Strategy::ExpectExisting). When readlink fails because the path is a real directory, #29489 made it return Ok unconditionally, so that a bun install run while a bun patch <pkg> workspace (a detached real directory at node_modules/<pkg>) is being edited does not deleteTree the user's changes. That exemption also matches a directory with nothing in it, so the install succeeds without creating the link. The bin linker then finds no node_modules/<pkg>/<bin> and skips the bin without an error (by design, same as npm), and since every later install takes the same branch, the tree never heals.

Fix

In the real-directory branch, rmdir the directory before linking. rmdir only succeeds on an empty directory (on Windows this goes through DeleteFileBun, which NTFS likewise refuses for a non-empty directory), so:

  • an empty directory is removed and the link is created; if creating it fails, the error propagates as a link failure like before (failed to symlink dependencies for package, exit 1);
  • a directory with contents, which is what bun patch leaves and the only thing the exemption was protecting, is still left untouched.

This is the narrowest rule that tells the two apart without inspecting contents: there is nothing inside an empty directory to preserve, and a non-empty directory is never deleted, so this cannot destroy anything #29489 wanted to keep. The same function also creates the .bun/node_modules/<pkg> fallback links, so an empty directory there is repaired as well. The hoisted linker is unaffected: it installs files into node_modules/<pkg>/ directly, so an existing empty directory is harmless there.

Tests

test/cli/install/isolated-install.test.ts, new describe("empty directory where a dependency symlink belongs"):

  • a dependency added to package.json is linked over a pre-created empty node_modules/<pkg> and its bin is linked (the repro above);
  • re-running install repairs root links (plain and scoped) and .bun/node_modules links that were replaced by empty directories;
  • the same for a workspace package's node_modules/<pkg>;
  • a real directory with contents at node_modules/<pkg> is left in place.

The first three fail on the current release (node_modules/<pkg> is still a directory after install, exit code 0) and pass with this change; the fourth passes both before and after and pins the preserved behavior, alongside the existing preserves bun patch workspace when install runs before --commit test. isolated-install.test.ts (66 tests), bun-install-patch.test.ts and bun-patch.test.ts pass with the debug build.