Skip to content
Open
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
37 changes: 20 additions & 17 deletions src/install/PackageManager/PackageManagerEnqueue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2642,7 +2642,24 @@ fn get_or_put_resolved_package(
dependency::version::Tag::Folder => {
let folder = *version.folder();
let res: FolderResolutionValue = 'res: {
if this.lockfile.is_workspace_dependency(dependency_id) {
if !this.lockfile.is_workspace_dependency(dependency_id)
&& crate::bin::bin_target_escapes_package_dir(this.lockfile.str(&folder))
{
// overrides/resolutions are only ever parsed from the root
// package.json, so a folder path that reached here via an
// override was written by the user and is trusted the same
// as a direct dependency of the root.
Comment thread
robobun marked this conversation as resolved.
let buf = this.lockfile.buffers.string_bytes.as_slice();
if !this.lockfile.overrides.contains_name(
dependency.name_hash,
dependency.name.slice(buf),
buf,
) {
break 'res FolderResolutionValue::Err(crate::Error::MissingPackageJSON);
}
}

if this.lockfile.is_dependency_of_local_package(dependency_id) {
// relative to cwd
// reshaped for borrowck — `folder_path` borrows
// `string_bytes`; detach the slice lifetime so the
Expand Down Expand Up @@ -2676,22 +2693,8 @@ fn get_or_put_resolved_package(
);
}

// transitive folder dependencies do not have their dependencies resolved
if crate::bin::bin_target_escapes_package_dir(this.lockfile.str(&folder)) {
// overrides/resolutions are only ever parsed from the root
// package.json, so a folder path that reached here via an
// override was written by the user and is trusted the same
// as a direct dependency of the root.
let buf = this.lockfile.buffers.string_bytes.as_slice();
if !this.lockfile.overrides.contains_name(
dependency.name_hash,
dependency.name.slice(buf),
buf,
) {
break 'res FolderResolutionValue::Err(crate::Error::MissingPackageJSON);
}
}

// Declared by a registry package: `Package::from_npm` keeps the path
// relative to that package, which is not on disk until it is installed.
Comment thread
robobun marked this conversation as resolved.
let mut package = Package::default();

{
Expand Down
9 changes: 4 additions & 5 deletions src/install/PackageManager/PackageManagerResolution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,11 +330,10 @@ impl PackageManager {
continue;
}

let features = match pkg_resolutions[parent_id].tag {
ResolutionTag::Root | ResolutionTag::Workspace | ResolutionTag::Folder => {
self.options.local_package_features
}
_ => self.options.remote_package_features,
let features = if pkg_resolutions[parent_id].tag.is_local_package() {
self.options.local_package_features
} else {
self.options.remote_package_features
};
// even if optional dependencies are enabled, it's still allowed to fail
if failed_dep.behavior.is_optional() || !failed_dep.behavior.is_enabled(features) {
Expand Down
25 changes: 25 additions & 0 deletions src/install/lockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -802,6 +802,31 @@ impl Lockfile {
invalid_package_id
}

/// The package whose dependency list contains `id`, or `invalid_package_id` when
/// no package declares it.
Comment thread
robobun marked this conversation as resolved.
pub(crate) fn get_parent_pkg_of_dependency(&self, id: DependencyID) -> PackageID {
self.packages
.items_dependencies()
.iter()
.position(|dependencies| dependencies.contains(id))
.map_or(invalid_package_id, |pkg_id| {
PackageID::try_from(pkg_id).expect("int cast")
})
}

/// Is this a direct dependency of a local package (`resolution::Tag::is_local_package`)?
///
/// A folder package declared by a registry package gets no dependency list (see the
/// Folder arm of `get_or_put_resolved_package`), so every folder package that
/// declares anything is reached from the root through local packages only.
Comment thread
robobun marked this conversation as resolved.
pub(crate) fn is_dependency_of_local_package(&self, id: DependencyID) -> bool {
let parent_id = self.get_parent_pkg_of_dependency(id);
parent_id != invalid_package_id
&& self.packages.items_resolution()[parent_id as usize]
.tag
.is_local_package()
}

/// Does this tree id belong to a workspace (including workspace root)?
/// TODO(dylan-conway) fix!
pub(crate) fn is_workspace_tree_id(&self, id: tree::Id) -> bool {
Expand Down
9 changes: 4 additions & 5 deletions src/install/lockfile/Tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -602,11 +602,10 @@ pub(crate) fn is_filtered_dependency_or_workspace(
return true;
}

let dep_features = match parent_res.tag {
crate::resolution::Tag::Root
| crate::resolution::Tag::Workspace
| crate::resolution::Tag::Folder => manager.options.local_package_features,
_ => manager.options.remote_package_features,
let dep_features = if parent_res.tag.is_local_package() {
manager.options.local_package_features
} else {
manager.options.remote_package_features
};

if !dep.behavior.is_enabled(dep_features) {
Expand Down
7 changes: 7 additions & 0 deletions src/install/resolution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -965,6 +965,13 @@ impl Tag {
self == Tag::Git || self == Tag::Github
}

/// The root, a workspace, or a `file:` folder: a package.json of the project's own,
/// so its dependencies get `local_package_features` and `Package::parse` stored its
/// `file:` paths relative to the top-level dir.
Comment thread
robobun marked this conversation as resolved.
pub(crate) fn is_local_package(self) -> bool {
self == Tag::Root || self == Tag::Workspace || self == Tag::Folder
}

pub(crate) fn can_enqueue_install_task(self) -> bool {
self == Tag::Npm
|| self == Tag::LocalTarball
Expand Down
91 changes: 91 additions & 0 deletions test/cli/install/bun-install-registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5243,6 +5243,97 @@ describe("transitive file dependencies", () => {
version: "1.1.1",
});
});

// Unlike the registry packages above, whose file: targets only exist once the
// package is installed, a local file: package's own file: dependency is on
// disk while resolving, so it is read like one declared by the root.
for (const linker of ["hoisted", "isolated"] as const) {
test(`${linker}: a file: dependency of a local file: package is resolved like a root one`, async () => {
const { packageDir } = await registry.createTestDir({
bunfigOpts: { linker },
files: {
"package.json": JSON.stringify({
name: "foo",
dependencies: {
lib: "file:./vendor/lib",
},
}),
"vendor/lib/package.json": JSON.stringify({
name: "lib",
version: "1.0.0",
dependencies: {
tool: "file:../tool",
},
}),
"vendor/lib/index.js": `module.exports = "lib->" + require("tool");`,
"vendor/tool/package.json": JSON.stringify({
name: "tool",
version: "1.0.0",
bin: { tool: "cli.js" },
dependencies: {
"no-deps": "1.0.0",
},
}),
"vendor/tool/cli.js": `#!/usr/bin/env node\nconsole.log("tool");`,
"vendor/tool/index.js": `module.exports = "tool->no-deps@" + require("no-deps").version;`,
},
});
const libNodeModules =
linker === "hoisted"
? join(packageDir, "node_modules", "lib", "node_modules")
: join(packageDir, "node_modules", ".bun", "lib@file+vendor+lib", "node_modules");

let { out } = await runBunInstall(env, packageDir);
expect(out).toContain("3 packages installed");

const lock = (await file(join(packageDir, "bun.lock")).text()).replaceAll(/localhost:\d+/g, "localhost:1234");
expect(normalizeBunSnapshot(lock)).toMatchInlineSnapshot(`
"{
"lockfileVersion": 2,
"configVersion": 1,
"workspaces": {
"": {
"name": "foo",
"dependencies": {
"lib": "file:./vendor/lib",
},
},
},
"packages": {
"lib": ["lib@file:vendor/lib", { "dependencies": { "tool": "file:../tool" } }],

"no-deps": ["no-deps@1.0.0", "http://localhost:1234/no-deps/-/no-deps-1.0.0.tgz", {}, "sha512-v4w12JRjUGvfHDUP8vFDwu0gUWu04j0cv9hLb1Abf9VdaXu4XcrddYFTMVBVvmldKViGWH7jrb6xPJRF0wq6gw=="],

"lib/tool": ["tool@file:vendor/tool", { "dependencies": { "no-deps": "1.0.0" }, "bin": { "tool": "cli.js" } }],
}
}"
`);

// Once from the package.json files, once from the lockfile they produced.
for (const frozenLockfile of [false, true]) {
if (frozenLockfile) {
await rm(join(packageDir, "node_modules"), { recursive: true, force: true });
({ out } = await runBunInstall(env, packageDir, { frozenLockfile }));
expect(out).toContain("3 packages installed");
}

expect(await readdirSorted(join(libNodeModules, ".bin"))).toHaveBins(["tool"]);
expect(join(libNodeModules, ".bin", "tool")).toBeValidBin(join("..", "tool", "cli.js"));

await using proc = spawn({
cmd: [bunExe(), "-e", `console.log(require("lib"))`],
cwd: packageDir,
env,
stdout: "pipe",
stderr: "pipe",
});
const [runOut, runErr, runExit] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(runErr).toBe("");
expect(runOut).toBe("lib->tool->no-deps@1.0.0\n");
expect(runExit).toBe(0);
}
});
}
});

test("name from manifest is scoped and url encoded", async () => {
Expand Down
Loading
Loading