diff --git a/src/install/PackageInstall.rs b/src/install/PackageInstall.rs index 22a227516ac6..82089252218c 100644 --- a/src/install/PackageInstall.rs +++ b/src/install/PackageInstall.rs @@ -1356,21 +1356,34 @@ impl<'a> PackageInstall<'a> { } } EntryKind::File => { + // `dest` can be a hardlink of `src`, which CopyFileW cannot overwrite. // 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 +1422,23 @@ impl<'a> PackageInstall<'a> { destination_dir_.fd(), bstr::BStr::new(entry.path.as_bytes()) ); - // Open O_WRONLY|O_CREAT|O_TRUNC, mode 0o666. + // The file can be a hardlink of `in_file`, and O_TRUNC would empty both. 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, diff --git a/test/cli/install/bun-workspaces-self-contained.test.ts b/test/cli/install/bun-workspaces-self-contained.test.ts index 062c428a09db..d7921bb6a6a3 100644 --- a/test/cli/install/bun-workspaces-self-contained.test.ts +++ b/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 = {}) { // --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", }); @@ -197,6 +197,101 @@ it("without either setting the workspace is hoisted normally", async () => { 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. +describe.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", (_label, rootDependencies) => { + it("each resolves its own version inside the self-contained workspace, and the cache stays intact", async () => { + 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" }); + }); +}); + // The lockfile does not record the setting. Bun 1.4.0 ignored // `installConfig.hoistingLimits`, so a repo that upgrades keeps its lockfile, and every // install, a frozen one too, reads the setting from the manifests. diff --git a/test/cli/install/bun-workspaces.test.ts b/test/cli/install/bun-workspaces.test.ts index bbfe6925b439..7f0ddf3f8bed 100644 --- a/test/cli/install/bun-workspaces.test.ts +++ b/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); +});