Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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
85 changes: 85 additions & 0 deletions src/install/lockfile/Tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1107,12 +1107,97 @@ impl Tree {
}
}

// a bundle's dependency stops here, but must not shadow what resolves through here
if !AS_DEFINED
&& this.id == hoist_root_id
&& Tree::hoist_root_resolves_elsewhere(
this.id,
package_id,
dependency.name_hash,
builder,
)
{
return Ok(HoistDependencyResult::DependencyLoop); // 3
}

// place the dependency in the current tree
Ok(HoistDependencyResult::Placement(Placement {
id: this.id,
bundled: false,
})) // 2
}

/// The bundling package and the regular dependencies nested under `hoist_root` resolve
/// `name_hash` through it, and a loaded `bun.lock` rebinds their edges by walking up the saved
/// paths, so `package_id` can only go there if those edges already resolve to it.
///
/// An unbound optional peer saves no path; once bound, `Lockfile::resolve` hoists again.
Comment thread
robobun marked this conversation as resolved.
Outdated
fn hoist_root_resolves_elsewhere<const METHOD: BuilderMethod>(
hoist_root: Id,
package_id: PackageID,
name_hash: PackageNameHash,
builder: &Builder<'_, METHOD>,
) -> bool {
if package_id == invalid_package_id {
return false;
}

let trees = builder.list.items_tree();
if trees[hoist_root as usize].parent == INVALID_ID {
return false;
}

let entry_lists = builder.list.items_dependencies();
let deps: &[Dependency] = builder.dependencies;
let resolutions: &[PackageID] = &*builder.resolutions;
let resolution_lists = builder.resolution_lists;

let resolves_elsewhere = |pkg_id: PackageID| -> bool {
let pkg_deps = resolution_lists[pkg_id as usize];
(pkg_deps.begin()..pkg_deps.end()).any(|dep_id| {
let dep = &deps[dep_id as usize];
dep.name_hash == name_hash
&& !dep.behavior.is_bundled()
&& resolutions[dep_id as usize] != package_id
})
};

let owner = resolutions[trees[hoist_root as usize].dependency_id as usize];
if resolves_elsewhere(owner) {
return true;
}

// (tree, package whose node_modules it is); children are appended after their parent
let mut scope: Vec<(Id, PackageID)> = vec![(hoist_root, owner)];
for (id, tree) in trees.iter().enumerate().skip(hoist_root as usize + 1) {
let Some(&(_, parent_owner)) =
scope.iter().find(|(scope_id, _)| *scope_id == tree.parent)
else {
continue;
};
if !resolution_lists[parent_owner as usize].contains(tree.dependency_id)
|| deps[tree.dependency_id as usize].behavior.is_bundled()
{
continue;
}
scope.push((id as Id, resolutions[tree.dependency_id as usize]));
}

scope.iter().any(|&(tree_id, tree_owner)| {
let own_deps = resolution_lists[tree_owner as usize];
trees[tree_id as usize]
.dependencies
.get(entry_lists[tree_id as usize].as_slice())
.iter()
.any(|&dep_id| {
let nested = resolutions[dep_id as usize];
nested != invalid_package_id
&& own_deps.contains(dep_id)
&& !deps[dep_id as usize].behavior.is_bundled()
&& resolves_elsewhere(nested)
})
})
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

// ──────────────────────────────────────────────────────────────────────────
Expand Down
171 changes: 171 additions & 0 deletions test/cli/install/bun-lock.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1376,3 +1376,174 @@ it("an optional peer is rebound when another version of its package takes the sl
await run(["install", "--lockfile-only"]);
expect(await file(join(packageDir, "bun.lock")).text()).toBe(lockfile);
});

// The bundled-shadow-* fixtures are described in
// registry/packages/create-bundled-shadow-packages.ts. In short: host depends
// on shared@1.0.0, which is hoisted to the root, and bundles inner, which
// depends on shared@2.0.0. A bundle's dependencies hoist no further than the
// bundling package's node_modules, but `host/shared` in bun.lock is also what
// host's own `shared` edge resolves to when the lockfile is loaded again, and
// likewise for anything nested under host that resolves shared from the root.
// https://github.com/oven-sh/bun/issues/29263
it("a bundled dependency's dependency does not take a slot the bundling package resolves through", async () => {
const { packageDir, packageJson } = await registry.createTestDir({
bunfigOpts: { saveTextLockfile: true, linker: "hoisted" },
});
const run = makeInstallRunner(packageDir);
const hostNodeModules = join(packageDir, "node_modules", "bundled-shadow-host", "node_modules");

await write(
packageJson,
JSON.stringify({
name: "foo",
dependencies: { "bundled-shadow-host": "1.0.0", "bundled-shadow-shared": "1.0.0" },
}),
);
await run(["install"]);
const fresh = await file(join(packageDir, "bun.lock")).text();
expect(fresh).toContain('"bundled-shadow-shared": ["bundled-shadow-shared@1.0.0"');
expect(fresh).toContain(
'"bundled-shadow-host/bundled-shadow-inner/bundled-shadow-shared": ["bundled-shadow-shared@2.0.0"',
);
expect(fresh).not.toContain('"bundled-shadow-host/bundled-shadow-shared"');

// inner's shared@2.0.0 ships inside host's tarball; host itself uses the root copy.
const layout = async () => ({
root: (await file(join(packageDir, "node_modules", "bundled-shadow-shared", "package.json")).json()).version,
host: await readdirSorted(hostNodeModules),
inner: (
await file(
join(hostNodeModules, "bundled-shadow-inner", "node_modules", "bundled-shadow-shared", "package.json"),
).json()
).version,
});
const expectedLayout = { root: "1.0.0", host: ["bundled-shadow-inner"], inner: "2.0.0" };
expect(await layout()).toEqual(expectedLayout);

// Every install that starts from the lockfile has to give host the same
// shared as the install that wrote it.
await run(["install"]);
expect(await layout()).toEqual(expectedLayout);

await rm(join(packageDir, "node_modules"), { recursive: true, force: true });
await run(["install", "--frozen-lockfile"]);
expect(await layout()).toEqual(expectedLayout);

await run(["install", "--lockfile-only"]);
expect(await file(join(packageDir, "bun.lock")).text()).toBe(fresh);
});

it("the isolated linker links the bundling package against the same dependency on a reinstall", async () => {
const { packageDir, packageJson } = await registry.createTestDir({
bunfigOpts: { saveTextLockfile: true, linker: "isolated" },
});
const run = makeInstallRunner(packageDir);
const sharedLinkedIntoHost = async () =>
(
await file(
join(
packageDir,
"node_modules",
".bun",
"bundled-shadow-host@1.0.0",
"node_modules",
"bundled-shadow-shared",
"package.json",
),
).json()
).version;

await write(
packageJson,
JSON.stringify({
name: "foo",
dependencies: { "bundled-shadow-host": "1.0.0", "bundled-shadow-shared": "1.0.0" },
}),
);
await run(["install"]);
expect(await sharedLinkedIntoHost()).toBe("1.0.0");

// The store is rebuilt from the resolutions bun.lock loads, which come from
// the saved paths.
await rm(join(packageDir, "node_modules"), { recursive: true, force: true });
await run(["install", "--frozen-lockfile"]);
expect(await sharedLinkedIntoHost()).toBe("1.0.0");
Comment thread
robobun marked this conversation as resolved.
Outdated
});

it("a bundled dependency's dependency does not take a slot a dependency nested under the bundling package resolves through", async () => {
const { packageDir, packageJson } = await registry.createTestDir({
bunfigOpts: { saveTextLockfile: true, linker: "hoisted" },
});
const run = makeInstallRunner(packageDir);

// deep-host itself does not depend on shared. consumer@1.0.0 nests under it
// (consumer@2.0.0 holds the root), mid@1.0.0 nests under consumer (mid@2.0.0
// holds the root), and mid's shared@1.0.0 is hoisted to the root, up through
// deep-host's node_modules. deep-host bundles wrapper, whose inner depends on
// shared@2.0.0, and that is hoisted after mid's shared is already at the root.
await write(
packageJson,
JSON.stringify({
name: "foo",
dependencies: {
"bundled-shadow-deep-host": "1.0.0",
"bundled-shadow-consumer": "2.0.0",
"bundled-shadow-mid": "2.0.0",
},
}),
);
await run(["install"]);
const fresh = await file(join(packageDir, "bun.lock")).text();
expect(fresh).toContain('"bundled-shadow-shared": ["bundled-shadow-shared@1.0.0"');
expect(fresh).toContain(
'"bundled-shadow-deep-host/bundled-shadow-consumer/bundled-shadow-mid": ["bundled-shadow-mid@1.0.0"',
);
expect(fresh).toContain(
'"bundled-shadow-deep-host/bundled-shadow-inner/bundled-shadow-shared": ["bundled-shadow-shared@2.0.0"',
);
expect(fresh).not.toContain('"bundled-shadow-deep-host/bundled-shadow-shared"');

// mid@1.0.0 has to keep finding shared@1.0.0 at the root on a reinstall.
await rm(join(packageDir, "node_modules"), { recursive: true, force: true });
await run(["install", "--frozen-lockfile"]);
expect({
root: (await file(join(packageDir, "node_modules", "bundled-shadow-shared", "package.json")).json()).version,
deepHost: await readdirSorted(join(packageDir, "node_modules", "bundled-shadow-deep-host", "node_modules")),
}).toEqual({ root: "1.0.0", deepHost: ["bundled-shadow-consumer", "bundled-shadow-wrapper"] });

await run(["install", "--lockfile-only"]);
expect(await file(join(packageDir, "bun.lock")).text()).toBe(fresh);
});

it("a bundled dependency's optional peer bound late does not take a slot the bundling package resolves through", async () => {
const { packageDir, packageJson } = await registry.createTestDir({
bunfigOpts: { saveTextLockfile: true, linker: "hoisted" },
});
const run = makeInstallRunner(packageDir);

// peer-inner (bundled) has an optional peer on shared and depends on
// peer-leaf, which depends on shared@2.0.0. The peer is still unbound when
// peer-inner is hoisted and gets bound to peer-leaf's shared@2.0.0 inside the
// bundle; peer-host's own shared@1.0.0 at the root must not be shadowed by
// either of them.
await write(packageJson, JSON.stringify({ name: "foo", dependencies: { "bundled-shadow-peer-host": "1.0.0" } }));
await run(["install"]);
const fresh = await file(join(packageDir, "bun.lock")).text();
expect(fresh).toContain('"bundled-shadow-shared": ["bundled-shadow-shared@1.0.0"');
expect(fresh).toContain(
'"bundled-shadow-peer-host/bundled-shadow-peer-inner/bundled-shadow-shared": ["bundled-shadow-shared@2.0.0"',
);
expect(fresh).toContain(
'"bundled-shadow-peer-host/bundled-shadow-peer-leaf/bundled-shadow-shared": ["bundled-shadow-shared@2.0.0"',
);
expect(fresh).not.toContain('"bundled-shadow-peer-host/bundled-shadow-shared"');

await rm(join(packageDir, "node_modules"), { recursive: true, force: true });
await run(["install", "--frozen-lockfile"]);
expect(await readdirSorted(join(packageDir, "node_modules", "bundled-shadow-peer-host", "node_modules"))).toEqual([
"bundled-shadow-peer-inner",
]);

await run(["install", "--lockfile-only"]);
expect(await file(join(packageDir, "bun.lock")).text()).toBe(fresh);
});
Binary file not shown.
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
{
"_id": "bundled-shadow-consumer",
"name": "bundled-shadow-consumer",
"dist-tags": {
"latest": "2.0.0"
},
"versions": {
"1.0.0": {
"name": "bundled-shadow-consumer",
"version": "1.0.0",
"dependencies": {
"bundled-shadow-mid": "1.0.0"
},
"_id": "bundled-shadow-consumer@1.0.0",
"dist": {
"integrity": "sha512-bYPAnSxRZS5jNQuvfR+PV/RC9MhWSCW0C+aB2GZi0xEQ9jmhUO+HAgK18dvtgF7IaXTqXM3F2ToI1eH5nsVLTA==",
"shasum": "825e428cd7ddc7af4be726cc1728ab0c735385da",
"tarball": "http://localhost:4873/bundled-shadow-consumer/-/bundled-shadow-consumer-1.0.0.tgz"
}
},
"2.0.0": {
"name": "bundled-shadow-consumer",
"version": "2.0.0",
"_id": "bundled-shadow-consumer@2.0.0",
"dist": {
"integrity": "sha512-5rOVsz3v9j2P1FaFAxsgnp50QmpGwjlPs3KO29gvUoO3RWlmYS5WQDJStSGMO8lzyWW+UCoR08IW3/WodfMjSg==",
"shasum": "79c93410f8ab5a6135323994a946978174e46dca",
"tarball": "http://localhost:4873/bundled-shadow-consumer/-/bundled-shadow-consumer-2.0.0.tgz"
}
}
}
}
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
{
"_id": "bundled-shadow-deep-host",
"name": "bundled-shadow-deep-host",
"dist-tags": {
"latest": "1.0.0"
},
"versions": {
"1.0.0": {
"name": "bundled-shadow-deep-host",
"version": "1.0.0",
"dependencies": {
"bundled-shadow-consumer": "1.0.0",
"bundled-shadow-wrapper": "1.0.0"
},
"bundleDependencies": [
"bundled-shadow-wrapper"
],
"_id": "bundled-shadow-deep-host@1.0.0",
"dist": {
"integrity": "sha512-0AapvZZN+czyjr1l05qbGQaBHKNiLw+2BFxouwkst7BvNfJUv09ZgHtRL1tb7QGHhm1VwwR/Vg70Uz+IOzmeXA==",
"shasum": "9e14944bae0586dc76c6ec9f0cd68af710da5a94",
"tarball": "http://localhost:4873/bundled-shadow-deep-host/-/bundled-shadow-deep-host-1.0.0.tgz"
}
}
}
}
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
{
"_id": "bundled-shadow-host",
"name": "bundled-shadow-host",
"dist-tags": {
"latest": "1.0.0"
},
"versions": {
"1.0.0": {
"name": "bundled-shadow-host",
"version": "1.0.0",
"dependencies": {
"bundled-shadow-inner": "1.0.0",
"bundled-shadow-shared": "1.0.0"
},
"bundleDependencies": [
"bundled-shadow-inner"
],
"_id": "bundled-shadow-host@1.0.0",
"dist": {
"integrity": "sha512-8OfobtACp9H+WHwCFhoRxW43zR9DIrtdPucaIwom3zp5jNJYQ1d6yu7y4x2FG8bixlhxR+iQ8E3/p0P+c4Ir1A==",
"shasum": "44e59de3f83ac5536a0391ec996b6a7cda06d203",
"tarball": "http://localhost:4873/bundled-shadow-host/-/bundled-shadow-host-1.0.0.tgz"
}
}
}
}
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"_id": "bundled-shadow-inner",
"name": "bundled-shadow-inner",
"dist-tags": {
"latest": "1.0.0"
},
"versions": {
"1.0.0": {
"name": "bundled-shadow-inner",
"version": "1.0.0",
"dependencies": {
"bundled-shadow-shared": "2.0.0"
},
"_id": "bundled-shadow-inner@1.0.0",
"dist": {
"integrity": "sha512-vLarWIx0f5OlsgpY6rtzIKtoQAjH7HRTrhDBpFtK5eMreRh8i9MmNNBIkmQogQGHxnl8f/A2aqYwUPtmMKb84A==",
"shasum": "9247c2d44500bedc5f78cff3e90fd15d0facfd71",
"tarball": "http://localhost:4873/bundled-shadow-inner/-/bundled-shadow-inner-1.0.0.tgz"
}
}
}
}
Binary file not shown.
Binary file not shown.
Loading
Loading