Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 30 additions & 1 deletion src/install/PackageInstaller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1376,7 +1376,7 @@ impl<'a> PackageInstaller<'a> {
}
installer.cache_dir = Fd::cwd();
} else {
// transitive folder dependencies are relative to their parent. they are not hoisted
// transitive folder dependencies are not hoisted
if folder.len() >= self.folder_path_buf.len()
|| (bin::bin_target_escapes_package_dir(folder) && {
// overrides/resolutions are only ever parsed from the root
Expand Down Expand Up @@ -1717,6 +1717,24 @@ impl<'a> PackageInstaller<'a> {
{
// This is a transitive folder dependency. It is installed with a single symlink to the target folder/file,
// and is not hoisted.
//
// A transitive `Resolution::Folder` declared by a local `file:` package
// is relative to the top-level dir (`Package::parse` normalized it), so
// install it from `installer.cache_dir` (the cwd, set in the switch above).
if resolution.tag == resolution::Tag::Folder
&& self.lockfile().is_folder_tree_id(self.current_tree_id)
{
break 'result installer.install(
self.skip_delete,
&destination_dir,
installer.get_install_method(),
resolution.tag,
);
}

// One declared by an npm manifest (`Package::from_npm`) is verbatim,
// 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::<platform::Auto>(self.node_modules.path.as_slice());
if d.is_empty() {
Expand Down Expand Up @@ -1755,6 +1773,8 @@ impl<'a> PackageInstaller<'a> {
)
};

// npm packages can declare `file:` paths missing from the published
// tarball (e.g. excluded by `files`); nothing to link is not a failure.
if let package_install::InstallResult::Failure(f) = &result {
if f.err == bun_core::err!("ENOENT")
|| f.err == bun_core::err!("FileNotFound")
Expand Down Expand Up @@ -1963,6 +1983,15 @@ impl<'a> PackageInstaller<'a> {
bstr::BStr::new(self.names[package_id as usize].slice(string_buf!())),
);
self.summary.fail += 1;
} else if resolution.tag == resolution::Tag::Folder
&& cause.is_package_missing_from_cache()
{
bun_core::pretty_errorln!(
"<r><red>error<r>: Could not find folder \"file:{}\" for dependency \"{}\"",
bstr::BStr::new(resolution.folder().slice(string_buf!())),
bstr::BStr::new(alias.slice(string_buf!())),
);
self.summary.fail += 1;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
} else if cause.err == bun_core::err!("AccessDenied") {
// there are two states this can happen
// - Access Denied because node_modules/ is unwritable
Expand Down
14 changes: 14 additions & 0 deletions src/install/lockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -998,6 +998,20 @@ 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`), unlike npm's (`Package::from_npm`).
pub fn is_folder_tree_id(&self, id: tree::Id) -> bool {
if id == 0 {
return false;
}
let dependency_id = self.buffers.trees[id as usize].dependency_id;
let package_id = self.buffers.resolutions[dependency_id as usize];
package_id != invalid_package_id
&& self.packages.slice().items_resolution()[package_id as usize].tag
== ResolutionTag::Folder
}

/// Returns the package id of the workspace the install is taking place in.
pub fn get_workspace_package_id(
&self,
Expand Down
94 changes: 94 additions & 0 deletions test/cli/install/bun-install.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9546,6 +9546,100 @@ for (const field of ["resolutions", "overrides"]) {
});
}

it("installs the transitive file: dependency of a file: dependency", async () => {
using dir = tempDir("transitive-file-dep", {
"package.json": JSON.stringify({
name: "my-app",
version: "1.0.0",
dependencies: {
lib: "file:./vendor/lib",
},
}),
"vendor/lib/package.json": JSON.stringify({
name: "lib",
version: "1.0.0",
main: "index.js",
dependencies: {
nested: "file:../nested",
},
}),
"vendor/lib/index.js": `module.exports = require("nested");`,
"vendor/nested/package.json": JSON.stringify({
name: "nested",
version: "1.0.0",
main: "index.js",
}),
"vendor/nested/index.js": `module.exports = "it worked";`,
});

// The first pass resolves from package.json; the second installs from the
// lockfile the first pass wrote.
for (const args of [["install"], ["install", "--frozen-lockfile"]]) {
await rm(join(String(dir), "node_modules"), { recursive: true, force: true });

const { stdout, stderr, exited } = spawn({
cmd: [bunExe(), ...args],
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
env,
});
const [err, out, exitCode] = await Promise.all([stderr.text(), stdout.text(), exited]);

expect(err).not.toContain("error:");
expect(out).toContain("2 packages installed");
expect(exitCode).toBe(0);

// `lib/index.js` requires "nested", so this only passes when the
// transitive file: dependency is materialized under node_modules.
await using runProc = spawn({
cmd: [bunExe(), "-e", `console.log(require("lib"))`],
cwd: String(dir),
env,
stdout: "pipe",
stderr: "pipe",
});
const [runOut, runErr, runExit] = await Promise.all([runProc.stdout.text(), runProc.stderr.text(), runProc.exited]);
expect(runErr).not.toContain("error:");
expect(runOut.trim()).toBe("it worked");
expect(runExit).toBe(0);
}
});

it("fails when a transitive file: dependency's folder does not exist", async () => {
using dir = tempDir("transitive-file-dep-missing", {
"package.json": JSON.stringify({
name: "my-app",
version: "1.0.0",
dependencies: {
lib: "file:./vendor/lib",
},
}),
"vendor/lib/package.json": JSON.stringify({
name: "lib",
version: "1.0.0",
dependencies: {
nested: "file:../nested",
},
}),
"vendor/lib/index.js": `module.exports = require("nested");`,
});

const { stdout, stderr, exited } = spawn({
cmd: [bunExe(), "install"],
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
env,
});
const [err, out, exitCode] = await Promise.all([stderr.text(), stdout.text(), exited]);

// The printed folder path uses the platform separator on Windows.
expect(err.replaceAll(sep, "/")).toContain('Could not find folder "file:vendor/nested" for dependency "nested"');
expect(out).not.toContain("2 packages installed");
expect(exitCode).toBe(1);
});

it("does not extract a local file: tarball outside the temp dir for a dependency alias containing '..' path segments", async () => {
// For `file:` tarball dependencies, the dependency alias (the key in
// `dependencies`) is used to derive the temporary extraction folder name.
Expand Down
Loading