install: stop the copy backend from emptying hardlinked cache files

src/install/PackageInstall.rs

@@ -1356,21 +1356,36 @@ impl<'a> PackageInstall<'a> {
                             }
                         }
                         EntryKind::File => {
+                            // `bFailIfExists`: an existing destination is deleted and
+                            // copied again, never overwritten in place. It can be a
+                            // hardlink of the cache entry being copied (an earlier
+                            // install of the same package into this folder), and
+                            // overwriting it would empty the file in the cache.
                             // SAFETY: FFI — src/dest are valid NUL-terminated WStr buffers.
-                            if unsafe { windows::CopyFileW(src.as_ptr(), dest.as_ptr(), 0) } == 0 {
-                                if let Some(entry_dirname) =
-                                    bun_paths::Dirname::dirname_u16(entry.path.as_slice())
-                                {
-                                    let _ = bun_sys::MakePath::make_path_u16(
-                                        destination_dir_,
-                                        entry_dirname,
-                                    );
-                                    // SAFETY: FFI — src/dest are valid NUL-terminated WStr buffers.
-                                    if unsafe { windows::CopyFileW(src.as_ptr(), dest.as_ptr(), 0) }
-                                        != 0
-                                    {
-                                        continue;
+                            let copy_file =
+                                || unsafe { windows::CopyFileW(src.as_ptr(), dest.as_ptr(), 1) } != 0;
+                            if !copy_file() {
+                                let copied = match windows::Win32Error::get() {
+                                    windows::Win32Error::FILE_EXISTS
+                                    | windows::Win32Error::ALREADY_EXISTS => {
+                                        // SAFETY: FFI — dest is a valid NUL-terminated WStr buffer.
+                                        unsafe { windows::DeleteFileW(dest.as_ptr()) };
+                                        copy_file()
                                     }
+                                    _ => match bun_paths::Dirname::dirname_u16(entry.path.as_slice())
+                                    {
+                                        Some(entry_dirname) => {
+                                            let _ = bun_sys::MakePath::make_path_u16(
+                                                destination_dir_,
+                                                entry_dirname,
+                                            );
+                                            copy_file()
+                                        }
+                                        None => false,
+                                    },
+                                };
+                                if copied {
+                                    continue;
                                 }
 
                                 let err = windows::last_system_errno();
@@ -1409,14 +1424,26 @@ impl<'a> PackageInstall<'a> {
                         destination_dir_.fd(),
                         bstr::BStr::new(entry.path.as_bytes())
                     );
-                    // Open O_WRONLY|O_CREAT|O_TRUNC, mode 0o666.
+                    // An existing destination is replaced, never truncated in
+                    // place: it can be a hardlink of the cache entry being copied
+                    // (an earlier install of the same package into this folder),
+                    // and `O_TRUNC` through it would empty the file in the cache.
                     let create = |path: &ZStr| {
-                        sys::openat(
-                            destination_dir_.fd(),
-                            path,
-                            sys::O::WRONLY | sys::O::CREAT | sys::O::TRUNC,
-                            0o666,
-                        )
+                        let open = || {
+                            sys::openat(
+                                destination_dir_.fd(),
+                                path,
+                                sys::O::WRONLY | sys::O::CREAT | sys::O::EXCL,
+                                0o666,
+                            )
+                        };
+                        match open() {
+                            Err(err) if err.get_errno() == sys::E::EEXIST => {
+                                let _ = sys::unlinkat(destination_dir_, path);
+                                open()
+                            }
+                            result => result,
+                        }
                     };
                     let outfile = match create(entry.path) {
                         Ok(f) => f,

test/cli/install/bun-workspaces-self-contained.test.ts

@@ -1,9 +1,9 @@
 import { spawn } from "bun";
 import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, setDefaultTimeout } from "bun:test";
-import { existsSync, lstatSync, readlinkSync, statSync } from "fs";
+import { existsSync, lstatSync, readFileSync, readlinkSync, statSync } from "fs";
 import { mkdir, rm, writeFile } from "fs/promises";
 import { bunExe, bunEnv as env, isWindows, readdirSorted } from "harness";
-import { join } from "path";
+import { dirname, join } from "path";
 import {
   dummyAfterAll,
   dummyAfterEach,
@@ -37,13 +37,13 @@ beforeEach(async () => {
 });
 afterEach(dummyAfterEach);
 
-async function install(cwd: string, args: string[] = []) {
+async function install(cwd: string, args: string[] = [], extraEnv: Record<string, string> = {}) {
   // --backend=hardlink so the "physical copy" assertions are meaningful on macOS too
   // (its default, clonefile, also yields nlink 1)
   await using proc = spawn({
     cmd: [bunExe(), "install", "--backend=hardlink", ...args],
     cwd,
-    env,
+    env: { ...env, ...extraEnv },
     stdout: "pipe",
     stderr: "pipe",
   });
@@ -176,3 +176,99 @@ it("without either setting the workspace is hoisted normally", async () => {
   expect(existsSync(join(package_dir, "node_modules", "@barn", "moo", "package.json"))).toBeTrue();
   expect(existsSync(join(package_dir, "node_modules", "baz", "package.json"))).toBeTrue();
 });
+
+// The package.json of `name` that `from` resolves without leaving `root`. It walks up
+// from `from` and does not resolve symlinks, the way a tool that copies `root` sees it.
+function findWithin(root: string, from: string, name: string): string | undefined {
+  for (let dir = from; ; dir = dirname(dir)) {
+    const candidate = join(dir, "node_modules", name, "package.json");
+    if (existsSync(candidate)) return candidate;
+    if (dir === root || dirname(dir) === dir) return undefined;
+  }
+}
+
+// `c` is self-contained and depends on its siblings `a` and `b`, which need different
+// versions of `baz`. The siblings stay symlinks, so the version that cannot hoist into
+// c's node_modules goes into the sibling's own node_modules, through the symlink. The
+// sibling's own tree can write the same directory. The second write used to truncate
+// the hardlinks of the first, and with them the files in the cache.
+it.each([
+  // baz@0.0.3 hoists to the root, so b's own tree also nests baz@0.0.5 in packages/b
+  ["one nested in the sibling's own node_modules", {}],
+  // b uses the root's baz@0.0.5, so only c's tree writes it into packages/b
+  ["one provided by the root", { baz: "0.0.5" }],
+] as const)(
+  "siblings that need two versions of a package, %s: each resolves its own version inside the self-contained workspace, and the cache stays intact",
+  async (_label, rootDependencies) => {
+    setHandler(dummyRegistry([], { "0.0.3": {}, "0.0.5": {} }));
+    // a cache outside node_modules, like the global one. The variable takes precedence
+    // over the bunfig `cache` key, and the test runner sets it.
+    const cacheDir = join(package_dir, ".bun-cache");
+    const cacheEnv = { BUN_INSTALL_CACHE_DIR: cacheDir };
+    await writeFile(
+      join(package_dir, "bunfig.toml"),
+      Bun.TOML.stringify({
+        install: { registry: root_url + "/", saveTextLockfile: true, linker: "hoisted" },
+      }),
+    );
+    for (const ws of ["a", "b", "c"]) {
+      await mkdir(join(package_dir, "packages", ws), { recursive: true });
+    }
+    await writeFile(
+      join(package_dir, "package.json"),
+      JSON.stringify({
+        name: "root",
+        private: true,
+        workspaces: { packages: ["packages/*"], selfContained: ["packages/c"] },
+        dependencies: rootDependencies,
+      }),
+    );
+    await writeFile(
+      join(package_dir, "packages", "a", "package.json"),
+      JSON.stringify({ name: "@p/a", version: "1.0.0", dependencies: { baz: "0.0.3" } }),
+    );
+    await writeFile(
+      join(package_dir, "packages", "b", "package.json"),
+      JSON.stringify({ name: "@p/b", version: "1.0.0", dependencies: { baz: "0.0.5" } }),
+    );
+    await writeFile(
+      join(package_dir, "packages", "c", "package.json"),
+      JSON.stringify({
+        name: "@p/c",
+        version: "1.0.0",
+        dependencies: { "@p/a": "workspace:*", "@p/b": "workspace:*" },
+      }),
+    );
+
+    const r = await install(package_dir, [], cacheEnv);
+    expect(r.err).not.toContain("error:");
+    expect(r.code).toBe(0);
+
+    const versionOf = (pkgJson: string) => {
+      expect(statSync(pkgJson).size).toBeGreaterThan(0);
+      return JSON.parse(readFileSync(pkgJson, "utf8")).version;
+    };
+    const cDir = join(package_dir, "packages", "c");
+    const bazFrom = (sibling: string) => {
+      const found = findWithin(cDir, join(cDir, "node_modules", "@p", sibling), "baz");
+      expect(found).toBeDefined();
+      return versionOf(found!);
+    };
+
+    expect({ a: bazFrom("a"), b: bazFrom("b") }).toEqual({ a: "0.0.3", b: "0.0.5" });
+
+    for (const version of ["0.0.3", "0.0.5"]) {
+      const cached = (await readdirSorted(cacheDir)).filter(name => name.startsWith(`baz@${version}@`));
+      expect(cached).toHaveLength(1);
+      expect(versionOf(join(cacheDir, cached[0], "package.json"))).toBe(version);
+      expect(statSync(join(cacheDir, cached[0], "index.js")).size).toBeGreaterThan(0);
+    }
+
+    // a repeat install has nothing to do
+    const again = await install(package_dir, [], cacheEnv);
+    expect(again.err).not.toContain("error:");
+    expect(again.out).toContain("(no changes)");
+    expect(again.code).toBe(0);
+    expect({ a: bazFrom("a"), b: bazFrom("b") }).toEqual({ a: "0.0.3", b: "0.0.5" });
+  },
+);

test/cli/install/bun-workspaces.test.ts

@@ -1,12 +1,13 @@
 import { file, spawn, write } from "bun";
 import { install_test_helpers } from "bun:internal-for-testing";
 import { afterAll, beforeAll, describe, expect, setDefaultTimeout, test } from "bun:test";
-import { mkdirSync, rmSync, writeFileSync } from "fs";
+import { mkdirSync, readFileSync, rmSync, statSync, writeFileSync } from "fs";
 import { cp, exists, mkdir, rm } from "fs/promises";
 import {
   assertManifestsPopulated,
   bunEnv as baseEnv,
   bunExe,
+  isWindows,
   readdirSorted,
   runBunInstall,
   toMatchNodeModulesAt,
@@ -2670,3 +2671,66 @@ describe("packages whose version label is longer than 512 bytes", () => {
     );
   });
 });
+
+// A hardlink install leaves hardlinks of cache files in each node_modules it writes.
+// After only the root node_modules is removed, the next install does not delete
+// anything first, so a workspace's own node_modules still holds those hardlinks. A copy
+// over them must replace the files. Writing through them empties the cache files.
+test.concurrent("a copyfile install over a workspace's hardlinked files does not empty the cache", async () => {
+  using ctx = await setupTest();
+  const { packageDir, packageJson, env } = ctx;
+  await Promise.all([
+    write(
+      packageJson,
+      JSON.stringify({ name: "foo", workspaces: ["packages/*"], dependencies: { "no-deps": "1.0.0" } }),
+    ),
+    write(
+      join(packageDir, "packages", "pkg1", "package.json"),
+      JSON.stringify({ name: "pkg1", version: "1.0.0", dependencies: { "no-deps": "2.0.0" } }),
+    ),
+  ]);
+
+  async function install(backend: "hardlink" | "copyfile") {
+    await using proc = spawn({
+      cmd: [bunExe(), "install", `--backend=${backend}`],
+      cwd: packageDir,
+      stdout: "pipe",
+      stderr: "pipe",
+      env,
+    });
+    const [, err, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    expect(err).not.toContain("error:");
+    expect(exitCode).toBe(0);
+  }
+
+  const readJson = (path: string) => {
+    expect(statSync(path).size).toBeGreaterThan(0);
+    return JSON.parse(readFileSync(path, "utf8"));
+  };
+
+  const nestedPkgJson = join(packageDir, "packages", "pkg1", "node_modules", "no-deps", "package.json");
+  await install("hardlink");
+  expect(readJson(nestedPkgJson)).toEqual({ name: "no-deps", version: "2.0.0" });
+  if (!isWindows) {
+    expect(statSync(nestedPkgJson).nlink).toBeGreaterThan(1);
+  }
+
+  await rm(join(packageDir, "node_modules"), { recursive: true, force: true });
+  await install("copyfile");
+
+  expect(readJson(nestedPkgJson)).toEqual({ name: "no-deps", version: "2.0.0" });
+  expect(readJson(join(packageDir, "node_modules", "no-deps", "package.json"))).toEqual({
+    name: "no-deps",
+    version: "1.0.0",
+  });
+  if (!isWindows) {
+    // a new file, not the cache file's inode
+    expect(statSync(nestedPkgJson).nlink).toBe(1);
+  }
+
+  const cacheDir = join(packageDir, ".bun-cache");
+  const cached = (await readdirSorted(cacheDir)).filter(name => name.startsWith("no-deps@2.0.0@"));
+  expect(cached).toHaveLength(1);
+  expect(readJson(join(cacheDir, cached[0], "package.json"))).toEqual({ name: "no-deps", version: "2.0.0" });
+  expect(statSync(join(cacheDir, cached[0], "index.js")).size).toBeGreaterThan(0);
+});