From 0291f8673dc04b8c4de42c0789b042b13b7c0513 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 02:04:54 +0000 Subject: [PATCH 1/5] install: keep file: paths declared by git and tarball packages relative to the package Package::parse rebased every file: dependency onto the top-level dir. For the root, workspace members and local file: folders that is the form the resolver and the hoisted installer expect, but a git or tarball package is parsed out of the install cache, so its file: paths became paths into the cache that the transitive folder check then rejected (or, with a cache inside the project, paths the installer could not find), failing the install. Registry packages never had the problem because Package::from_npm keeps the path as declared, and the installer resolves such paths against the installed copy of the declaring package. Mark Features::FOLDER with is_folder and only rebase for manifests that live in the project (is_main, is_workspace, is_folder); git, tarball and cached npm manifests keep the path as declared, like registry packages. --- src/install/PackageInstaller.rs | 2 +- src/install/lockfile.rs | 3 +- src/install/lockfile/Package.rs | 9 +- src/install_types/resolver_hooks.rs | 9 + test/cli/install/bun-install-git-deps.test.ts | 179 +++++++++++++++++- 5 files changed, 189 insertions(+), 13 deletions(-) diff --git a/src/install/PackageInstaller.rs b/src/install/PackageInstaller.rs index 6c96867f593b..0814b337ffda 100644 --- a/src/install/PackageInstaller.rs +++ b/src/install/PackageInstaller.rs @@ -1847,7 +1847,7 @@ impl<'a> PackageInstaller<'a> { ); } - // One declared by an npm manifest (`Package::from_npm`) is verbatim, + // One declared by a registry, git or tarball package is verbatim, // i.e. relative to the declaring package, which installs at // `dirname(node_modules.path)` because transitive folders never hoist. let dir_name = { diff --git a/src/install/lockfile.rs b/src/install/lockfile.rs index 73d55df73e17..f65d0dd5ebf3 100644 --- a/src/install/lockfile.rs +++ b/src/install/lockfile.rs @@ -802,7 +802,8 @@ impl Lockfile { /// Is the package whose `node_modules` this tree represents resolved from a /// local `file:` folder? Its `Resolution::Folder` dependencies were normalized - /// relative to the top-level dir (`Package::parse`), unlike npm's (`Package::from_npm`). + /// relative to the top-level dir (`Package::parse` with `Features::FOLDER`), unlike + /// those of registry, git and tarball packages, which stay relative to the package. pub(crate) fn is_folder_tree_id(&self, id: tree::Id) -> bool { if id == 0 { return false; diff --git a/src/install/lockfile/Package.rs b/src/install/lockfile/Package.rs index fcd5b545bd4d..bd3602fe70f8 100644 --- a/src/install/lockfile/Package.rs +++ b/src/install/lockfile/Package.rs @@ -1843,7 +1843,14 @@ impl Package { } match dependency_version.tag { - dependency::version::Tag::Folder => { + // Only a manifest that lives in the project can have its `file:` paths + // rebased onto the top-level dir. A git/tarball package is parsed out of + // the cache: its paths stay relative to the package, as `Package::from_npm` + // stores them, and the installer resolves them against the installed copy + // of the declaring package. + dependency::version::Tag::Folder + if features.is_main || features.is_workspace || features.is_folder => + { let folder = *dependency_version.folder(); let mut folder_buf = PathBuffer::uninit(); let Some(joined) = resolve_path::join_abs_string_buf_checked::( diff --git a/src/install_types/resolver_hooks.rs b/src/install_types/resolver_hooks.rs index 902921588134..5b10084bbca8 100644 --- a/src/install_types/resolver_hooks.rs +++ b/src/install_types/resolver_hooks.rs @@ -1204,6 +1204,12 @@ pub struct Features { pub dev_dependencies: bool, pub is_main: bool, pub is_workspace: bool, + /// The package.json belongs to a local `file:` folder dependency. Together + /// with `is_main` / `is_workspace` this marks the manifests that live in the + /// project, whose `file:` dependency paths are stored relative to the + /// top-level dir. `NPM` (registry, git and tarball packages) is parsed out of + /// the cache and keeps them relative to the package instead. + pub is_folder: bool, pub optional_dependencies: bool, pub peer_dependencies: bool, pub trusted_dependencies: bool, @@ -1218,6 +1224,7 @@ impl Default for Features { dev_dependencies: false, is_main: false, is_workspace: false, + is_folder: false, optional_dependencies: false, peer_dependencies: true, trusted_dependencies: false, @@ -1240,6 +1247,7 @@ impl Features { dev_dependencies: false, is_main: false, is_workspace: false, + is_folder: false, optional_dependencies: false, peer_dependencies: true, trusted_dependencies: false, @@ -1262,6 +1270,7 @@ impl Features { pub const FOLDER: Self = Self { dev_dependencies: true, + is_folder: true, optional_dependencies: true, ..Self::base() }; diff --git a/test/cli/install/bun-install-git-deps.test.ts b/test/cli/install/bun-install-git-deps.test.ts index 76997319f218..e01c07f083bc 100644 --- a/test/cli/install/bun-install-git-deps.test.ts +++ b/test/cli/install/bun-install-git-deps.test.ts @@ -1,13 +1,15 @@ // Tests for installing git dependencies that live in ONE repository as -// multiple branches (issue #35420), `git+file://` dependencies, and +// multiple branches (issue #35420), `git+file://` dependencies, // tarball-URL / `github:` dependencies that appear both directly and -// transitively (issues #10915, #8501, #11348, #28284). Everything is local: -// a bare repo on disk (served over git's dumb HTTP protocol by Bun.serve +// transitively (issues #10915, #8501, #11348, #28284), and git / tarball +// packages whose own package.json declares `file:` dependencies. Everything is +// local: a bare repo on disk (served over git's dumb HTTP protocol by Bun.serve // when an http URL is needed) or static tarballs. import { expect, test } from "bun:test"; -import { mkdirSync, writeFileSync } from "fs"; +import { mkdirSync, rmSync, writeFileSync } from "fs"; +import { lstat, readdir, readlink } from "fs/promises"; import { bunEnv, bunExe, tempDir } from "harness"; -import { join } from "path"; +import { dirname, join, resolve } from "path"; import { pathToFileURL } from "url"; const gitEnv = { @@ -35,6 +37,8 @@ interface BranchPackage { name: string; branch: string; dependencies?: Record; + /** Files committed next to package.json; an `index.js` entry replaces the default one. */ + files?: Record; } // Creates `/shared-repo.git`, a bare repo with one orphan branch per @@ -47,14 +51,21 @@ async function makeSharedRepo(root: string, packages: BranchPackage[]): Promise< await git(work, "init", "-q"); for (const pkg of packages) { await git(work, "checkout", "-q", "--orphan", pkg.branch); - writeFileSync( - join(work, "package.json"), - JSON.stringify({ name: pkg.name, version: "1.0.0", dependencies: pkg.dependencies }, null, 2), - ); - writeFileSync(join(work, "index.js"), `module.exports = ${JSON.stringify(pkg.branch)};\n`); + const files: Record = { + "package.json": JSON.stringify({ name: pkg.name, version: "1.0.0", dependencies: pkg.dependencies }, null, 2), + "index.js": `module.exports = ${JSON.stringify(pkg.branch)};\n`, + ...pkg.files, + }; + for (const [path, contents] of Object.entries(files)) { + mkdirSync(dirname(join(work, path)), { recursive: true }); + writeFileSync(join(work, path), contents); + } await git(work, "add", "-A"); await git(work, "commit", "-q", "-m", pkg.branch, "--no-gpg-sign"); await git(work, "push", "-q", bare, pkg.branch); + // `checkout --orphan` keeps the working tree, so the next branch would + // otherwise inherit this one's files. + for (const path of Object.keys(files)) rmSync(join(work, path), { force: true }); } // dumb HTTP clients read the static files this generates await git(bare, "update-server-info"); @@ -423,3 +434,151 @@ test.concurrent("installs a git+file:// dependency", async () => { expect(await installedVersionOf(project, "@scope/pkg-b")).toBe("pkg-b"); expect(exitCode).toBe(0); }); + +// A git or tarball package whose own package.json declares `file:` dependencies +// (the shape registry packages publish too, see registry/packages/file-dep). +// The paths are relative to the package, which is extracted into the cache, so +// resolving them against the project dir turned them into paths into the cache +// that were then rejected for escaping the declaring package: +// error: Could not find package.json for "file:..//@T@.../sub" dependency "sub" +// error: sub@file:./sub failed to resolve +// Registry packages keep the path as declared; git and tarball packages must too. +const FILE_DEPS_NAME = "has-file-deps"; + +// `file:` dependencies on a subfolder, on the package itself, and on a folder +// that is not part of the package. +const fileDepsManifest = { + name: FILE_DEPS_NAME, + version: "1.0.0", + dependencies: { + "sub": "file:./sub", + [`${FILE_DEPS_NAME}-self`]: "file:.", + "gone": "file:./not-in-package", + }, +}; + +const fileDepsFiles = { + "index.js": `module.exports = require("sub");\n`, + "sub/package.json": JSON.stringify({ name: "sub", version: "1.0.0" }), + "sub/index.js": `module.exports = "sub-ok";\n`, +}; + +// Writes `/work/package/` and packs it into `tarball`, a `.tgz` path. +async function packTarball(root: string, tarball: string, manifest: object, files: Record) { + const pkgDir = join(root, "work", "package"); + for (const [path, contents] of Object.entries({ "package.json": JSON.stringify(manifest), ...files })) { + mkdirSync(dirname(join(pkgDir, path)), { recursive: true }); + writeFileSync(join(pkgDir, path), contents); + } + mkdirSync(dirname(tarball), { recursive: true }); + await run(root, ["tar", "-czf", tarball, "-C", join(root, "work"), "package"], "tar"); +} + +function writeProject(root: string, dependencies: Record): string { + const project = join(root, "project"); + mkdirSync(project, { recursive: true }); + writeFileSync(join(project, "package.json"), JSON.stringify({ name: "project", version: "1.0.0", dependencies })); + return project; +} + +async function linkedJson(link: string) { + expect((await lstat(link)).isSymbolicLink()).toBe(true); + return Bun.file(resolve(dirname(link), await readlink(link))).json(); +} + +async function expectFileDepsInstalled(project: string) { + const name = FILE_DEPS_NAME; + const nested = join(project, "node_modules", name, "node_modules"); + const lockfile = await Bun.file(join(project, "bun.lock")).text(); + expect(lockfile).toContain(`"${name}/sub": ["sub@file:./sub", {}]`); + expect(lockfile).toContain(`"${name}/${name}-self": ["${name}-self@file:.", {}]`); + expect(lockfile).toContain(`"${name}/gone": ["gone@file:./not-in-package", {}]`); + + // Transitive folder dependencies are not hoisted: each one is linked file by + // file into the declaring package's own node_modules, relative to that + // package. The folder missing from the package is skipped, as it is for a + // registry package. + expect((await readdir(nested)).sort()).toEqual([`${name}-self`, "sub"]); + expect(await linkedJson(join(nested, "sub", "package.json"))).toEqual({ name: "sub", version: "1.0.0" }); + expect(await linkedJson(join(nested, `${name}-self`, "package.json"))).toEqual(fileDepsManifest); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", `console.log(require(${JSON.stringify(name)}))`], + cwd: project, + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr, exitCode }).toEqual({ stdout: "sub-ok\n", stderr: "", exitCode: 0 }); +} + +for (const spec of ["file: path", "http url"] as const) { + test.concurrent(`installs the file: dependencies declared by a tarball package (${spec})`, async () => { + using dir = tempDir("tarball-file-deps", {}); + const root = String(dir); + const tarballs = join(root, "tarballs"); + await packTarball(root, join(tarballs, `${FILE_DEPS_NAME}.tgz`), fileDepsManifest, fileDepsFiles); + await using server = serveStatic(tarballs); + const project = writeProject(root, { + [FILE_DEPS_NAME]: + spec === "file: path" + ? `file:../tarballs/${FILE_DEPS_NAME}.tgz` + : `http://localhost:${server.port}/${FILE_DEPS_NAME}.tgz`, + }); + + const { stderr, exitCode } = await runInstall(project, join(root, "cache"), {}); + expect(stderr).not.toContain("error:"); + expect(exitCode).toBe(0); + await expectFileDepsInstalled(project); + + // The stub rows written to bun.lock install again without re-resolving. + rmSync(join(project, "node_modules"), { recursive: true }); + const frozen = await runInstall(project, join(root, "cache"), {}, "--frozen-lockfile"); + expect(frozen.stderr).not.toContain("error:"); + expect(frozen.exitCode).toBe(0); + await expectFileDepsInstalled(project); + }); +} + +test.concurrent( + "installs the file: dependencies declared by a git dependency", + async () => { + using dir = tempDir("git-dep-file-deps", {}); + const root = String(dir); + const bare = await makeSharedRepo(root, [ + { name: FILE_DEPS_NAME, branch: "main", dependencies: fileDepsManifest.dependencies, files: fileDepsFiles }, + ]); + const project = writeProject(root, { [FILE_DEPS_NAME]: `git+${pathToFileURL(bare)}#main` }); + + const { stderr, exitCode } = await runInstall(project, join(root, "cache"), {}); + expect(stderr).not.toContain("error:"); + expect(exitCode).toBe(0); + await expectFileDepsInstalled(project); + }, + 30_000, +); + +test.concurrent("does not install a file: dependency of a tarball package that points outside of it", async () => { + using dir = tempDir("tarball-escaping-file-dep", { + "outside/package.json": JSON.stringify({ name: "outside", version: "1.0.0" }), + }); + const root = String(dir); + const tarball = join(root, "tarballs", "escaping.tgz"); + await packTarball( + root, + tarball, + { name: "escaping", version: "1.0.0", dependencies: { outside: "file:../outside" } }, + {}, + ); + const project = writeProject(root, { escaping: `file:../tarballs/escaping.tgz` }); + + const { stderr, exitCode } = await runInstall(project, join(root, "cache"), {}); + expect(stderr).toContain('error: Could not find package.json for "file:../outside" dependency "outside"'); + expect(stderr).toContain("error: outside@file:../outside failed to resolve"); + expect(await Bun.file(join(project, "node_modules", "outside", "package.json")).exists()).toBe(false); + expect( + await Bun.file(join(project, "node_modules", "escaping", "node_modules", "outside", "package.json")).exists(), + ).toBe(false); + expect(exitCode).toBe(1); +}); From e6c97573896d1f495e05bfe22db6b0ecbf521abc Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 09:06:46 +0000 Subject: [PATCH 2/5] Name the folder form and the producers in the comments and test titles --- src/install/PackageInstaller.rs | 5 ++-- src/install/lockfile.rs | 3 ++- src/install/lockfile/Package.rs | 10 +++---- src/install_types/resolver_hooks.rs | 7 ++--- test/cli/install/bun-install-git-deps.test.ts | 27 ++++++++++--------- 5 files changed, 28 insertions(+), 24 deletions(-) diff --git a/src/install/PackageInstaller.rs b/src/install/PackageInstaller.rs index 0814b337ffda..78bdb8ba91d6 100644 --- a/src/install/PackageInstaller.rs +++ b/src/install/PackageInstaller.rs @@ -1847,8 +1847,9 @@ impl<'a> PackageInstaller<'a> { ); } - // One declared by a registry, git or tarball package is verbatim, - // i.e. relative to the declaring package, which installs at + // One produced by `Package::from_npm` or by `Package::parse` with + // `Features::NPM` (registry, git and tarball packages) is stored as + // declared, i.e. relative to the declaring package, which installs at // `dirname(node_modules.path)` because transitive folders never hoist. let dir_name = { let d = dirname::(self.node_modules.path.as_slice()); diff --git a/src/install/lockfile.rs b/src/install/lockfile.rs index f65d0dd5ebf3..895a6204d9cb 100644 --- a/src/install/lockfile.rs +++ b/src/install/lockfile.rs @@ -803,7 +803,8 @@ impl Lockfile { /// Is the package whose `node_modules` this tree represents resolved from a /// local `file:` folder? Its `Resolution::Folder` dependencies were normalized /// relative to the top-level dir (`Package::parse` with `Features::FOLDER`), unlike - /// those of registry, git and tarball packages, which stay relative to the package. + /// those `Package::from_npm` and `Package::parse` with `Features::NPM` produce, + /// which are stored as declared, relative to the package. pub(crate) fn is_folder_tree_id(&self, id: tree::Id) -> bool { if id == 0 { return false; diff --git a/src/install/lockfile/Package.rs b/src/install/lockfile/Package.rs index bd3602fe70f8..3a9fea919377 100644 --- a/src/install/lockfile/Package.rs +++ b/src/install/lockfile/Package.rs @@ -1843,11 +1843,11 @@ impl Package { } match dependency_version.tag { - // Only a manifest that lives in the project can have its `file:` paths - // rebased onto the top-level dir. A git/tarball package is parsed out of - // the cache: its paths stay relative to the package, as `Package::from_npm` - // stores them, and the installer resolves them against the installed copy - // of the declaring package. + // Only a manifest that lives in the project can have its `file:` folder + // paths rebased onto the top-level dir. A git/tarball package is parsed out + // of the cache (`Features::NPM`): its folder paths are kept as declared, + // like `Package::from_npm` keeps them, and the hoisted installer resolves + // them against the installed copy of the declaring package. dependency::version::Tag::Folder if features.is_main || features.is_workspace || features.is_folder => { diff --git a/src/install_types/resolver_hooks.rs b/src/install_types/resolver_hooks.rs index 5b10084bbca8..5d1b1ed3c2d8 100644 --- a/src/install_types/resolver_hooks.rs +++ b/src/install_types/resolver_hooks.rs @@ -1206,9 +1206,10 @@ pub struct Features { pub is_workspace: bool, /// The package.json belongs to a local `file:` folder dependency. Together /// with `is_main` / `is_workspace` this marks the manifests that live in the - /// project, whose `file:` dependency paths are stored relative to the - /// top-level dir. `NPM` (registry, git and tarball packages) is parsed out of - /// the cache and keeps them relative to the package instead. + /// project: `Package::parse` stores their `file:` folder dependency paths + /// relative to the top-level dir. Manifests parsed with `NPM` (git and + /// tarball packages, extracted into the cache) keep those paths as declared, + /// the same form `Package::from_npm` stores for registry packages. pub is_folder: bool, pub optional_dependencies: bool, pub peer_dependencies: bool, diff --git a/test/cli/install/bun-install-git-deps.test.ts b/test/cli/install/bun-install-git-deps.test.ts index e01c07f083bc..ecb2b0f90554 100644 --- a/test/cli/install/bun-install-git-deps.test.ts +++ b/test/cli/install/bun-install-git-deps.test.ts @@ -2,9 +2,9 @@ // multiple branches (issue #35420), `git+file://` dependencies, // tarball-URL / `github:` dependencies that appear both directly and // transitively (issues #10915, #8501, #11348, #28284), and git / tarball -// packages whose own package.json declares `file:` dependencies. Everything is -// local: a bare repo on disk (served over git's dumb HTTP protocol by Bun.serve -// when an http URL is needed) or static tarballs. +// packages whose own package.json declares `file:` folder dependencies. +// Everything is local: a bare repo on disk (served over git's dumb HTTP +// protocol by Bun.serve when an http URL is needed) or static tarballs. import { expect, test } from "bun:test"; import { mkdirSync, rmSync, writeFileSync } from "fs"; import { lstat, readdir, readlink } from "fs/promises"; @@ -435,18 +435,19 @@ test.concurrent("installs a git+file:// dependency", async () => { expect(exitCode).toBe(0); }); -// A git or tarball package whose own package.json declares `file:` dependencies -// (the shape registry packages publish too, see registry/packages/file-dep). -// The paths are relative to the package, which is extracted into the cache, so -// resolving them against the project dir turned them into paths into the cache -// that were then rejected for escaping the declaring package: +// A git or tarball package whose own package.json declares `file:` folder +// dependencies (the shape registry packages publish too, see +// registry/packages/file-dep). The paths are relative to the package, which is +// extracted into the cache, so resolving them against the project dir turned +// them into paths into the cache that were then rejected for escaping the +// declaring package: // error: Could not find package.json for "file:..//@T@.../sub" dependency "sub" // error: sub@file:./sub failed to resolve // Registry packages keep the path as declared; git and tarball packages must too. const FILE_DEPS_NAME = "has-file-deps"; -// `file:` dependencies on a subfolder, on the package itself, and on a folder -// that is not part of the package. +// `file:` folder dependencies on a subfolder, on the package itself, and on a +// folder that is not part of the package. const fileDepsManifest = { name: FILE_DEPS_NAME, version: "1.0.0", @@ -514,7 +515,7 @@ async function expectFileDepsInstalled(project: string) { } for (const spec of ["file: path", "http url"] as const) { - test.concurrent(`installs the file: dependencies declared by a tarball package (${spec})`, async () => { + test.concurrent(`installs the file: folder dependencies declared by a tarball package (${spec})`, async () => { using dir = tempDir("tarball-file-deps", {}); const root = String(dir); const tarballs = join(root, "tarballs"); @@ -542,7 +543,7 @@ for (const spec of ["file: path", "http url"] as const) { } test.concurrent( - "installs the file: dependencies declared by a git dependency", + "installs the file: folder dependencies declared by a git dependency", async () => { using dir = tempDir("git-dep-file-deps", {}); const root = String(dir); @@ -559,7 +560,7 @@ test.concurrent( 30_000, ); -test.concurrent("does not install a file: dependency of a tarball package that points outside of it", async () => { +test.concurrent("rejects a file: folder dependency of a tarball package that points outside of it", async () => { using dir = tempDir("tarball-escaping-file-dep", { "outside/package.json": JSON.stringify({ name: "outside", version: "1.0.0" }), }); From 08ab2df904693581a50fdc96b76ed20bc50c2dc9 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 09:16:50 +0000 Subject: [PATCH 3/5] Shorten the comments --- src/install/PackageInstaller.rs | 7 +++---- src/install/lockfile.rs | 8 +++----- src/install/lockfile/Package.rs | 7 ++----- src/install_types/resolver_hooks.rs | 8 ++------ 4 files changed, 10 insertions(+), 20 deletions(-) diff --git a/src/install/PackageInstaller.rs b/src/install/PackageInstaller.rs index 78bdb8ba91d6..8f8f8654e95d 100644 --- a/src/install/PackageInstaller.rs +++ b/src/install/PackageInstaller.rs @@ -1847,10 +1847,9 @@ impl<'a> PackageInstaller<'a> { ); } - // One produced by `Package::from_npm` or by `Package::parse` with - // `Features::NPM` (registry, git and tarball packages) is stored as - // declared, i.e. relative to the declaring package, which installs at - // `dirname(node_modules.path)` because transitive folders never hoist. + // One from `Package::from_npm` or a `Features::NPM` parse is relative to + // the declaring package, which installs at `dirname(node_modules.path)` + // because transitive folders never hoist. let dir_name = { let d = dirname::(self.node_modules.path.as_slice()); if d.is_empty() { diff --git a/src/install/lockfile.rs b/src/install/lockfile.rs index 895a6204d9cb..ed8034f580ee 100644 --- a/src/install/lockfile.rs +++ b/src/install/lockfile.rs @@ -800,11 +800,9 @@ impl Lockfile { .is_workspace() } - /// Is the package whose `node_modules` this tree represents resolved from a - /// local `file:` folder? Its `Resolution::Folder` dependencies were normalized - /// relative to the top-level dir (`Package::parse` with `Features::FOLDER`), unlike - /// those `Package::from_npm` and `Package::parse` with `Features::NPM` produce, - /// which are stored as declared, relative to the package. + /// Is the package whose `node_modules` this tree represents resolved from a local + /// `file:` folder? A `Features::FOLDER` parse rebased its `Resolution::Folder` dependencies + /// onto the top-level dir; `from_npm` / `Features::NPM` ones are package-relative. pub(crate) fn is_folder_tree_id(&self, id: tree::Id) -> bool { if id == 0 { return false; diff --git a/src/install/lockfile/Package.rs b/src/install/lockfile/Package.rs index 3a9fea919377..bc8087d5e5c6 100644 --- a/src/install/lockfile/Package.rs +++ b/src/install/lockfile/Package.rs @@ -1843,11 +1843,8 @@ impl Package { } match dependency_version.tag { - // Only a manifest that lives in the project can have its `file:` folder - // paths rebased onto the top-level dir. A git/tarball package is parsed out - // of the cache (`Features::NPM`): its folder paths are kept as declared, - // like `Package::from_npm` keeps them, and the hoisted installer resolves - // them against the installed copy of the declaring package. + // A manifest parsed out of the cache (`Features::NPM`) keeps folder paths + // relative to its package, like `Package::from_npm`; see `PackageInstaller`. dependency::version::Tag::Folder if features.is_main || features.is_workspace || features.is_folder => { diff --git a/src/install_types/resolver_hooks.rs b/src/install_types/resolver_hooks.rs index 5d1b1ed3c2d8..175f251c78fb 100644 --- a/src/install_types/resolver_hooks.rs +++ b/src/install_types/resolver_hooks.rs @@ -1204,12 +1204,8 @@ pub struct Features { pub dev_dependencies: bool, pub is_main: bool, pub is_workspace: bool, - /// The package.json belongs to a local `file:` folder dependency. Together - /// with `is_main` / `is_workspace` this marks the manifests that live in the - /// project: `Package::parse` stores their `file:` folder dependency paths - /// relative to the top-level dir. Manifests parsed with `NPM` (git and - /// tarball packages, extracted into the cache) keep those paths as declared, - /// the same form `Package::from_npm` stores for registry packages. + /// The package.json of a local `file:` folder dependency. With `is_main` and + /// `is_workspace`, the manifests whose folder paths are rebased onto the top-level dir. pub is_folder: bool, pub optional_dependencies: bool, pub peer_dependencies: bool, From a6376f54c9d5a75fba897bd8d4e0d4166c1d459b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 09:23:43 +0000 Subject: [PATCH 4/5] Trim the comments to one why each --- src/install/PackageInstaller.rs | 4 ++-- src/install/lockfile.rs | 5 ++--- src/install/lockfile/Package.rs | 3 +-- src/install_types/resolver_hooks.rs | 2 -- 4 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/install/PackageInstaller.rs b/src/install/PackageInstaller.rs index 8f8f8654e95d..994f4ec17c65 100644 --- a/src/install/PackageInstaller.rs +++ b/src/install/PackageInstaller.rs @@ -1847,8 +1847,8 @@ impl<'a> PackageInstaller<'a> { ); } - // One from `Package::from_npm` or a `Features::NPM` parse is relative to - // the declaring package, which installs at `dirname(node_modules.path)` + // One declared by a cache package (`from_npm`, `Features::NPM`) is + // relative to that package, which installs at `dirname(node_modules.path)` // because transitive folders never hoist. let dir_name = { let d = dirname::(self.node_modules.path.as_slice()); diff --git a/src/install/lockfile.rs b/src/install/lockfile.rs index ed8034f580ee..e139d7be45c6 100644 --- a/src/install/lockfile.rs +++ b/src/install/lockfile.rs @@ -800,9 +800,8 @@ impl Lockfile { .is_workspace() } - /// Is the package whose `node_modules` this tree represents resolved from a local - /// `file:` folder? A `Features::FOLDER` parse rebased its `Resolution::Folder` dependencies - /// onto the top-level dir; `from_npm` / `Features::NPM` ones are package-relative. + /// Is the package whose `node_modules` this tree represents a local `file:` folder? Its + /// folder dependencies are top-level-dir relative; a cache package's are package-relative. pub(crate) fn is_folder_tree_id(&self, id: tree::Id) -> bool { if id == 0 { return false; diff --git a/src/install/lockfile/Package.rs b/src/install/lockfile/Package.rs index bc8087d5e5c6..1056e11cfd69 100644 --- a/src/install/lockfile/Package.rs +++ b/src/install/lockfile/Package.rs @@ -1843,8 +1843,7 @@ impl Package { } match dependency_version.tag { - // A manifest parsed out of the cache (`Features::NPM`) keeps folder paths - // relative to its package, like `Package::from_npm`; see `PackageInstaller`. + // Cache packages (`Features::NPM`) keep these package-relative, like `from_npm`. dependency::version::Tag::Folder if features.is_main || features.is_workspace || features.is_folder => { diff --git a/src/install_types/resolver_hooks.rs b/src/install_types/resolver_hooks.rs index 175f251c78fb..2878b87bd28c 100644 --- a/src/install_types/resolver_hooks.rs +++ b/src/install_types/resolver_hooks.rs @@ -1204,8 +1204,6 @@ pub struct Features { pub dev_dependencies: bool, pub is_main: bool, pub is_workspace: bool, - /// The package.json of a local `file:` folder dependency. With `is_main` and - /// `is_workspace`, the manifests whose folder paths are rebased onto the top-level dir. pub is_folder: bool, pub optional_dependencies: bool, pub peer_dependencies: bool, From 95abf24d8aff235a499d15a58baf63087fa37a32 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 09:29:08 +0000 Subject: [PATCH 5/5] Trim the test comment --- test/cli/install/bun-install-git-deps.test.ts | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/test/cli/install/bun-install-git-deps.test.ts b/test/cli/install/bun-install-git-deps.test.ts index ecb2b0f90554..e68d82e08421 100644 --- a/test/cli/install/bun-install-git-deps.test.ts +++ b/test/cli/install/bun-install-git-deps.test.ts @@ -435,15 +435,10 @@ test.concurrent("installs a git+file:// dependency", async () => { expect(exitCode).toBe(0); }); -// A git or tarball package whose own package.json declares `file:` folder -// dependencies (the shape registry packages publish too, see -// registry/packages/file-dep). The paths are relative to the package, which is -// extracted into the cache, so resolving them against the project dir turned -// them into paths into the cache that were then rejected for escaping the -// declaring package: -// error: Could not find package.json for "file:..//@T@.../sub" dependency "sub" -// error: sub@file:./sub failed to resolve -// Registry packages keep the path as declared; git and tarball packages must too. +// `file:` folder dependencies declared by a git or tarball package are relative +// to that package, like a registry package's (registry/packages/file-dep). They +// used to be resolved against the project dir, which pointed them into the cache +// and failed the install with `sub@file:./sub failed to resolve`. const FILE_DEPS_NAME = "has-file-deps"; // `file:` folder dependencies on a subfolder, on the package itself, and on a