install: replace an empty directory in an isolated link slot with the link

src/install/isolated_install/Symlinker.rs

@@ -75,34 +75,17 @@ impl Symlinker {
                                         _ => Err(symlink_err),
                                     },
                                 },
-                                // readlink failed for a reason other than NOENT —
-                                // dest exists but isn't a symlink. If it's a real
-                                // directory, leave it: this is the `bun patch <pkg>`
-                                // workspace (a detached copy the user is editing
-                                // before `--commit`), and `deleteTree` here would
-                                // silently destroy their in-progress edits. If it's
-                                // a regular file, replace it.
+                                // A real directory with a package.json is a `bun patch` workspace: keep it.
                                 _ => {
-                                    #[cfg(windows)]
-                                    let is_dir = if let Some(a) =
-                                        bun_sys::get_file_attributes(self.dest.slice_z())
-                                    {
-                                        a.is_directory && !a.is_reparse_point
-                                    } else {
-                                        false
+                                    let has_package_json = {
+                                        let mut dest = self.dest.save();
+                                        let _ = dest.append(b"package.json");
+                                        bun_sys::exists_z(dest.slice_z())
                                     };
-                                    #[cfg(not(windows))]
-                                    let is_dir = if let Ok(st) = bun_sys::lstat(self.dest.slice_z())
-                                    {
-                                        // `mode_t` is `u16` on darwin/freebsd/android, `u32` on linux.
-                                        bun_sys::posix::s_isdir(st.st_mode as u32)
-                                    } else {
-                                        false
-                                    };
-                                    if is_dir {
+                                    if has_package_json {
                                         return Ok(false);
                                     }
-                                    let _ = bun_sys::unlink(self.dest.slice_z());
+                                    let _ = Fd::cwd().delete_tree(self.dest.slice_z());
                                     return self.symlink().map(|()| true);
                                 }
                             };

test/cli/install/isolated-relink.test.ts

@@ -1,7 +1,7 @@
 import { file, write } from "bun";
 import { afterAll, beforeAll, expect, test } from "bun:test";
 import { existsSync, readFileSync } from "fs";
-import { lstat, mkdir, realpath, unlink } from "fs/promises";
+import { lstat, mkdir, readlink, realpath, unlink } from "fs/promises";
 import { VerdaccioRegistry, bunEnv, bunExe, isWindows, normalizeBunSnapshot } from "harness";
 import { dirname, join } from "path";
 
@@ -170,6 +170,129 @@ test.concurrent(
   },
 );
 
+test.concurrent("an empty real directory in a root dependency's slot is replaced with the link", async () => {
+  const { packageDir, packageJson } = await registry.createTestDir({ bunfigOpts: { linker: "isolated" } });
+
+  await write(packageJson, oneRangeDep());
+  await installOk(packageDir);
+  const link = join(packageDir, "node_modules", "one-range-dep");
+  expect((await lstat(link)).isSymbolicLink()).toBeTrue();
+
+  await unlink(link);
+  await mkdir(link);
+  expect(existsSync(join(link, "package.json"))).toBeFalse();
+
+  // Root links are not counted in the summary, so only the tree is checked here.
+  await installOk(packageDir);
+  expect((await lstat(link)).isSymbolicLink()).toBeTrue();
+  expect(await file(join(link, "package.json")).json()).toMatchObject({ name: "one-range-dep", version: "1.0.0" });
+});
+
+test.concurrent("an empty real directory in a store entry's dependency slot is replaced with the link", async () => {
+  const { packageDir, packageJson } = await registry.createTestDir({ bunfigOpts: { linker: "isolated" } });
+
+  await write(packageJson, oneRangeDep());
+  await installOk(packageDir);
+  const link = await nestedNoDeps(packageDir);
+
+  await unlink(link);
+  await mkdir(link);
+
+  const out = await installOk(packageDir);
+  expect(out).not.toContain("(no changes)");
+  expect((await lstat(link)).isSymbolicLink()).toBeTrue();
+  expect(await nestedNoDepsPackageJson(link)).toStrictEqual({ name: "no-deps", version: "1.1.0" });
+
+  expect(await installOk(packageDir)).toContain("(no changes)");
+});
+
+test.concurrent("a regular file in a root dependency's slot is replaced with the link", async () => {
+  const { packageDir, packageJson } = await registry.createTestDir({ bunfigOpts: { linker: "isolated" } });
+
+  await write(packageJson, oneRangeDep());
+  await installOk(packageDir);
+  const link = join(packageDir, "node_modules", "one-range-dep");
+
+  await unlink(link);
+  await write(link, "not a package");
+
+  await installOk(packageDir);
+  expect((await lstat(link)).isSymbolicLink()).toBeTrue();
+  expect(await file(join(link, "package.json")).json()).toMatchObject({ name: "one-range-dep", version: "1.0.0" });
+});
+
+// ninja creates the directory of every declared output before it runs the command, so bun's own build
+// runs `bun install` with an empty `node_modules/<new dep>/` already in place.
+test.concurrent("a new dependency is linked over an empty directory created before the install", async () => {
+  const { packageDir, packageJson } = await registry.createTestDir({ bunfigOpts: { linker: "isolated" } });
+
+  await write(packageJson, oneRangeDep());
+  await installOk(packageDir);
+
+  const nm = join(packageDir, "node_modules");
+  await write(
+    packageJson,
+    JSON.stringify({ name: "foo", dependencies: { "one-range-dep": "1.0.0", "what-bin": "1.0.0" } }),
+  );
+  await mkdir(join(nm, "what-bin"));
+
+  const out = await installOk(packageDir);
+  expect(out).toContain("what-bin@1.0.0");
+  expect((await lstat(join(nm, "what-bin"))).isSymbolicLink()).toBeTrue();
+  expect(await file(join(nm, "what-bin", "package.json")).json()).toMatchObject({ name: "what-bin", version: "1.0.0" });
+  expect(binFiles(nm, "what-bin").map(bin => existsSync(bin))).toStrictEqual(binFiles(nm, "what-bin").map(() => true));
+  expect(await binTargetContents(nm, "what-bin")).toContain("what-bin@1.0.0");
+});
+
+test.concurrent("empty directories in the .bun/node_modules and scoped slots are replaced with links", async () => {
+  const { packageDir, packageJson } = await registry.createTestDir({ bunfigOpts: { linker: "isolated" } });
+
+  await write(
+    packageJson,
+    JSON.stringify({ name: "foo", dependencies: { "no-deps": "1.0.0", "@types/is-number": "1.0.0" } }),
+  );
+  await installOk(packageDir);
+
+  const nm = join(packageDir, "node_modules");
+  const links = [
+    join(nm, "no-deps"),
+    join(nm, "@types", "is-number"),
+    join(nm, ".bun", "node_modules", "no-deps"),
+    join(nm, ".bun", "node_modules", "@types", "is-number"),
+  ];
+  const expected = await Promise.all(links.map(link => readlink(link)));
+  for (const link of links) {
+    await unlink(link);
+    await mkdir(link);
+  }
+
+  await installOk(packageDir);
+  expect(await Promise.all(links.map(link => readlink(link)))).toStrictEqual(expected);
+});
+
+test.concurrent("an empty directory in a workspace package's dependency slot is replaced with the link", async () => {
+  const { packageDir } = await registry.createTestDir({
+    bunfigOpts: { linker: "isolated" },
+    files: {
+      "package.json": JSON.stringify({ name: "foo", workspaces: ["packages/*"] }),
+      "packages/pkg-1/package.json": JSON.stringify({
+        name: "pkg-1",
+        version: "1.0.0",
+        dependencies: { "a-dep": "1.0.1" },
+      }),
+    },
+  });
+  await installOk(packageDir);
+
+  const link = join(packageDir, "packages", "pkg-1", "node_modules", "a-dep");
+  const expected = await readlink(link);
+  await unlink(link);
+  await mkdir(link);
+
+  await installOk(packageDir);
+  expect(await readlink(link)).toBe(expected);
+});
+
 test.concurrent.skipIf(!isWindows)("junction-mode warm install reports no changes", async () => {
   const { packageDir, packageJson } = await registry.createTestDir({ bunfigOpts: { linker: "isolated" } });
   const env = { BUN_FEATURE_FLAG_FORCE_WINDOWS_JUNCTIONS: "1" };