diff --git a/src/install/PackageInstall.rs b/src/install/PackageInstall.rs index 50043c09ed1e..8219093b1540 100644 --- a/src/install/PackageInstall.rs +++ b/src/install/PackageInstall.rs @@ -1800,7 +1800,7 @@ impl<'a> PackageInstall<'a> { } let _ = sys::unlinkat(destination_dir, entry.path); - sys::symlinkat(entry.basename, destination_dir.fd(), entry.path)?; + sys::symlinkat(target, destination_dir.fd(), entry.path)?; } } _ => {} @@ -1850,6 +1850,12 @@ impl<'a> PackageInstall<'a> { } EntryKind::File => match sys::symlink_w(dest, src, Default::default()) { Err(err) => { + if err.get_errno() == sys::E::EEXIST { + let _ = sys::unlink_w(dest); + sys::symlink_w(dest, src, Default::default())?; + continue; + } + if let Some(entry_dirname) = bun_paths::Dirname::dirname_u16(entry.path.as_slice()) { diff --git a/src/install/lockfile/bun.lock.rs b/src/install/lockfile/bun.lock.rs index 1741eb7dbf7e..490578a8480e 100644 --- a/src/install/lockfile/bun.lock.rs +++ b/src/install/lockfile/bun.lock.rs @@ -2542,11 +2542,9 @@ pub(crate) fn parse_into_binary_lockfile( return Err(ParseError::InvalidPackageInfo); }; - let pkg_info = pkg_info.items(); - if pkg_info.len() < 3 { - continue; - } - let Some(maybe_info_obj) = pkg_info[2].as_object() else { + // Index 2 for an npm resolution (after the registry string), index 1 otherwise. + let Some(maybe_info_obj) = pkg_info.items().iter().find_map(|item| item.as_object()) + else { continue; }; let Some(&JSON::E::JsonValue::Boolean(bundled)) = maybe_info_obj.get(b"bundled") else { diff --git a/test/cli/install/bun-install-registry.test.ts b/test/cli/install/bun-install-registry.test.ts index fb5d4f931e60..08bdf5c9bd11 100644 --- a/test/cli/install/bun-install-registry.test.ts +++ b/test/cli/install/bun-install-registry.test.ts @@ -1494,6 +1494,97 @@ describe("bundledDependencies", () => { await check(); }); } + + // bundled-file@1.0.0 depends on "bundled-file-dep" through a `file:` spec and + // bundles it. The tarball ships the bundled copy in its node_modules. The + // install from bun.lock must keep treating the dependency as bundled and must + // not replace the bundled copy with symlinks. + test("(bun.lock) file: dependency stays bundled when installing from the lockfile", async () => { + await write( + packageJson, + JSON.stringify({ + name: "bundled-file-root", + dependencies: { + "bundled-file": "1.0.0", + }, + }), + ); + + const bundledDepDir = join(packageDir, "node_modules", "bundled-file", "node_modules", "bundled-file-dep"); + + async function check() { + const [pkgJsonStat, indexStat] = await Promise.all([ + lstat(join(bundledDepDir, "package.json")), + lstat(join(bundledDepDir, "index.js")), + ]); + expect([pkgJsonStat.isFile(), indexStat.isFile()]).toEqual([true, true]); + + await using proc = spawn({ + cmd: [bunExe(), "-e", `console.log(require("bundled-file"))`], + cwd: packageDir, + stdout: "pipe", + stderr: "pipe", + env, + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("bundled-file-dep\n"); + expect(exitCode).toBe(0); + } + + let { out } = await runBunInstall(env, packageDir, { saveTextLockfile: true }); + expect(out).toContain("1 package installed"); + await check(); + + const lockfile = await file(join(packageDir, "bun.lock")).text(); + expect(lockfile).toContain( + `"bundled-file/bundled-file-dep": ["bundled-file-dep@file:vendor/bundled-file-dep", { "bundled": true }],`, + ); + + await rm(join(packageDir, "node_modules"), { recursive: true, force: true }); + + ({ out } = await runBunInstall(env, packageDir, { frozenLockfile: true })); + expect(out).toContain("1 package installed"); + await check(); + }); + + // bundled-file@2.0.0 ships node_modules/bundled-file-dep in its tarball but + // does not bundle it. The install links the `file:` dependency over those + // files. The links must point at the cached vendor copy, not at themselves. + test("file: dependency links over files the tarball already shipped", async () => { + await write( + packageJson, + JSON.stringify({ + name: "unbundled-file-root", + dependencies: { + "bundled-file": "2.0.0", + }, + }), + ); + + const { out } = await runBunInstall(env, packageDir, { saveTextLockfile: true }); + expect(out).toContain("2 packages installed"); + + const depDir = join(packageDir, "node_modules", "bundled-file", "node_modules", "bundled-file-dep"); + const [pkgJsonStat, indexStat] = await Promise.all([ + lstat(join(depDir, "package.json")), + lstat(join(depDir, "index.js")), + ]); + expect([pkgJsonStat.isSymbolicLink(), indexStat.isSymbolicLink()]).toEqual([true, true]); + expect(await readlink(join(depDir, "package.json"))).toEndWith(join("vendor", "bundled-file-dep", "package.json")); + + await using proc = spawn({ + cmd: [bunExe(), "-e", `console.log(require("bundled-file"))`], + cwd: packageDir, + stdout: "pipe", + stderr: "pipe", + env, + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("bundled-file-dep\n"); + expect(exitCode).toBe(0); + }); }); describe("optionalDependencies", () => { diff --git a/test/cli/install/registry/packages/bundled-file/bundled-file-1.0.0.tgz b/test/cli/install/registry/packages/bundled-file/bundled-file-1.0.0.tgz new file mode 100644 index 000000000000..dbe0552b9f18 Binary files /dev/null and b/test/cli/install/registry/packages/bundled-file/bundled-file-1.0.0.tgz differ diff --git a/test/cli/install/registry/packages/bundled-file/bundled-file-2.0.0.tgz b/test/cli/install/registry/packages/bundled-file/bundled-file-2.0.0.tgz new file mode 100644 index 000000000000..6a1f0e3e4692 Binary files /dev/null and b/test/cli/install/registry/packages/bundled-file/bundled-file-2.0.0.tgz differ diff --git a/test/cli/install/registry/packages/bundled-file/package.json b/test/cli/install/registry/packages/bundled-file/package.json new file mode 100644 index 000000000000..3190852462f2 --- /dev/null +++ b/test/cli/install/registry/packages/bundled-file/package.json @@ -0,0 +1,38 @@ +{ + "_id": "bundled-file", + "name": "bundled-file", + "dist-tags": { + "latest": "2.0.0" + }, + "versions": { + "1.0.0": { + "name": "bundled-file", + "version": "1.0.0", + "dependencies": { + "bundled-file-dep": "file:vendor/bundled-file-dep" + }, + "bundleDependencies": [ + "bundled-file-dep" + ], + "_id": "bundled-file@1.0.0", + "dist": { + "integrity": "sha512-1kQd7SfufR6wmrYo6L8J2jZm34WCaJyJj7KoskI+P3S+PuYpZ01rpuH+N15mtjUBcoVkVorPBxaOcSrX0H7ytg==", + "shasum": "7e59fee2055f735ed039b470f15d95c3612e99b9", + "tarball": "http://localhost:4873/bundled-file/-/bundled-file-1.0.0.tgz" + } + }, + "2.0.0": { + "name": "bundled-file", + "version": "2.0.0", + "dependencies": { + "bundled-file-dep": "file:vendor/bundled-file-dep" + }, + "_id": "bundled-file@2.0.0", + "dist": { + "integrity": "sha512-tFSzHwAOxEKDbBAHK/MY8RQEQ1KdQu6+bJYnW8/lZybGuZG2MoYWvksteDGjsWyXBimCSz7XWjo/ENEU9tsDNg==", + "shasum": "cd4c6b205128b53321bad299e60d7c6afb1f6c0e", + "tarball": "http://localhost:4873/bundled-file/-/bundled-file-2.0.0.tgz" + } + } + } +} \ No newline at end of file diff --git a/test/cli/install/registry/packages/create-bundled-file-packages.ts b/test/cli/install/registry/packages/create-bundled-file-packages.ts new file mode 100644 index 000000000000..836d009ea924 --- /dev/null +++ b/test/cli/install/registry/packages/create-bundled-file-packages.ts @@ -0,0 +1,75 @@ +#!/usr/bin/env bun +/** + * Generates the `bundled-file` fixture used by bun-install-registry.test.ts. + * + * Both versions depend on `bundled-file-dep` through a `file:` spec + * (`file:vendor/bundled-file-dep`) and ship both `vendor/bundled-file-dep/` + * and a copy under `node_modules/bundled-file-dep/`, the way `npm pack` does + * for a bundled `file:` dependency. + * + * - bundled-file@1.0.0 lists the dependency in `bundleDependencies`. A `file:` + * resolution has its `bundled: true` marker at a different index in bun.lock + * than an npm resolution, so an install from the lockfile must still treat + * the dependency as bundled. + * - bundled-file@2.0.0 does not bundle it. The install has to link the + * `file:` dependency over the files the tarball already put in place. + */ + +import { mkdir, writeFile } from "fs/promises"; +import { join } from "path"; + +const packagesDir = import.meta.dir; + +const name = "bundled-file"; +const depName = "bundled-file-dep"; + +const dir = join(packagesDir, name); +await mkdir(dir, { recursive: true }); + +const depPkgJson = JSON.stringify({ name: depName, version: "1.0.0", main: "index.js" }, null, 2); +const depIndex = `module.exports = "${depName}";\n`; + +const versions: Record = {}; +let latest = ""; +for (const [version, bundled] of [ + ["1.0.0", true], + ["2.0.0", false], +] as const) { + const pkgJson = { + name, + version, + dependencies: { [depName]: `file:vendor/${depName}` }, + ...(bundled ? { bundleDependencies: [depName] } : {}), + }; + + const files: Record = { + "package/package.json": JSON.stringify(pkgJson, null, 2), + "package/index.js": `module.exports = require("${depName}");\n`, + [`package/vendor/${depName}/package.json`]: depPkgJson, + [`package/vendor/${depName}/index.js`]: depIndex, + [`package/node_modules/${depName}/package.json`]: depPkgJson, + [`package/node_modules/${depName}/index.js`]: depIndex, + }; + + const tarball = join(dir, `${name}-${version}.tgz`); + await Bun.Archive.write(tarball, files, { compress: "gzip" }); + + const bytes = await Bun.file(tarball).bytes(); + versions[version] = { + ...pkgJson, + _id: `${name}@${version}`, + dist: { + integrity: `sha512-${Buffer.from(new Bun.CryptoHasher("sha512").update(bytes).digest()).toString("base64")}`, + shasum: new Bun.CryptoHasher("sha1").update(bytes).digest("hex"), + tarball: `http://localhost:4873/${name}/-/${name}-${version}.tgz`, + }, + }; + latest = version; +} + +await writeFile( + join(dir, "package.json"), + JSON.stringify({ _id: name, name, "dist-tags": { latest }, versions }, null, 2), +); + +console.log("Created bundled-file test packages");