Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
55 changes: 52 additions & 3 deletions src/install/lockfile/bun.lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1505,6 +1505,33 @@
dep: &Dependency,
string_buf: &[u8],
path_buf: &mut [u8],
) -> Result<&T, ResolveError> {
self.find_resolution_impl(pkg_path, dep, string_buf, path_buf, None)
}

/// Like `find_resolution`, but stops the upward walk at the bundle hoist
/// root: once a directory on the walk is a bundled package, only one more
/// level (the tree node containing the bundled package) is searched. This
/// mirrors `Tree::hoist_dependency`, which never walks past a bundled
/// dependency's hoist root when it re-derives optional peer edges.
fn find_resolution_bounded_at_bundle(
&self,
pkg_path: &[u8],
dep: &Dependency,
string_buf: &[u8],
path_buf: &mut [u8],
bundled_pkgs: &PkgPathSet,
) -> Result<&T, ResolveError> {
self.find_resolution_impl(pkg_path, dep, string_buf, path_buf, Some(bundled_pkgs))
}

fn find_resolution_impl(
&self,
pkg_path: &[u8],
dep: &Dependency,
string_buf: &[u8],
path_buf: &mut [u8],
bundled_pkgs: Option<&PkgPathSet>,
) -> Result<&T, ResolveError> {
let dep_name = dep.name.slice(string_buf);

Expand All @@ -1516,6 +1543,7 @@
path_buf[pkg_path.len()] = b'/';
let mut offset = pkg_path.len() + 1;

let mut at_bundle_root = false;
let mut valid = true;
while valid {
path_buf[offset..offset + dep_name.len()].copy_from_slice(dep_name);
Expand All @@ -1525,10 +1553,14 @@
return Ok(entry);
}

if offset == 0 {
if offset == 0 || at_bundle_root {
return Err(ResolveError::Unresolvable);
}

if let Some(bundled_pkgs) = bundled_pkgs {
at_bundle_root = bundled_pkgs.contains(&path_buf[0..offset - 1]);
}

Check warning on line 1562 in src/install/lockfile/bun.lock.rs

View check run for this annotation

Claude / Claude Code Review

Bundle-bounded optional-peer walk misses transitive deps of bundled packages

The bounded walk only covers *directly* bundled packages — a transitive dep of a bundled package (e.g. `A` bundles `B`, `B` depends on `C`, `C` has an optional peer) lands at `A/C` **without** `"bundled": true`, so `bundled_pkgs` doesn't contain it and the walk still escapes to root, producing the same `--frozen-lockfile` failure as #37346 one level deeper. This is pre-existing and the PR is a strict improvement, but per the "fix the whole class" guideline it's in scope; a full fix likely needs
Comment thread
claude[bot] marked this conversation as resolved.

let Some(slash) = strings::last_index_of_char(&path_buf[0..offset - 1], b'/') else {
offset = 0;
continue;
Expand Down Expand Up @@ -2957,8 +2989,25 @@
let res_id = match peer_res_id {
Some(id) => id,
None => {
match pkg_map.find_resolution(pkg_path, dep, string_buf, &mut path_buf[..])
{
// Optional peer edges are re-derived by the hoister
// (`Package::clone` resets them), which never searches
// past a bundle hoist root. Bound the path walk the
// same way so a loaded lockfile resolves them exactly
// like a fresh install does; otherwise the frozen
// lockfile `eql` check sees a tree the fresh resolve
// would never produce (#37346).
Comment thread
robobun marked this conversation as resolved.
Outdated
let found = if dep.behavior.is_optional_peer() {
pkg_map.find_resolution_bounded_at_bundle(
pkg_path,
dep,
string_buf,
&mut path_buf[..],
&bundled_pkgs,
)
} else {
pkg_map.find_resolution(pkg_path, dep, string_buf, &mut path_buf[..])
};
match found {
Ok(&id) => id,
Err(ResolveError::InvalidPackageKey) => {
log.add_error(Some(source), row.key_loc, b"Invalid package path");
Expand Down
32 changes: 32 additions & 0 deletions test/cli/install/bun-lock.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,38 @@ it("should sort overrides before comparing", async () => {
expect(secondLockfile).toBe(lockfile);
});

it("should pass frozen lockfile check when a bundled dependency has an optional peer satisfiable from the root", async () => {
// A bundled dependency's optional peer must not resolve across the bundle
// hoist root when the lockfile is loaded, otherwise a fresh install and a
// loaded lockfile disagree about the tree and --frozen-lockfile rejects a
// lockfile bun itself just wrote (issue #37346).
const { packageDir, packageJson } = await registry.createTestDir();

await write(
packageJson,
JSON.stringify({
name: "frozen-bundled-optional-peer",
dependencies: {
// bundles `optional-peer-deps`, which has an optional peer on `no-deps`
"bundled-optional-peer": "1.0.0",
"no-deps": "1.0.0",
},
}),
);

await runBunInstall(env, packageDir);
const lockfile = await file(join(packageDir, "bun.lock")).text();
expect(lockfile).toContain('"bundled": true');

await runBunInstall(env, packageDir, { frozenLockfile: true });

// and from a cold start with no node_modules
await rm(join(packageDir, "node_modules"), { recursive: true, force: true });
await runBunInstall(env, packageDir, { frozenLockfile: true });

expect(await file(join(packageDir, "bun.lock")).text()).toBe(lockfile);
});

it("should include unused resolutions in the lockfile", async () => {
const { packageDir, packageJson } = await registry.createTestDir();

Expand Down
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
{
"name": "bundled-optional-peer",
"versions": {
"1.0.0": {
"name": "bundled-optional-peer",
"version": "1.0.0",
"dependencies": {
"optional-peer-deps": "1.0.0"
},
"bundleDependencies": [
"optional-peer-deps"
],
"_id": "bundled-optional-peer@1.0.0",
"integrity": "sha512-4Gc/aWTtJ9ePN9UqxA9tPkIMZKHyDoSQ2dmx5us9rBGUPWpz6+9ChrKV3i/zsfd3Hb/kjav9YTnZAOu0Fz6NZA==",
"shasum": "b0afca067791d99523e6e378f52cbd7f128ef730",
"dist": {
"integrity": "sha512-4Gc/aWTtJ9ePN9UqxA9tPkIMZKHyDoSQ2dmx5us9rBGUPWpz6+9ChrKV3i/zsfd3Hb/kjav9YTnZAOu0Fz6NZA==",
"shasum": "b0afca067791d99523e6e378f52cbd7f128ef730",
"tarball": "http://localhost:4873/bundled-optional-peer/-/bundled-optional-peer-1.0.0.tgz"
},
"contributors": []
}
},
"time": {
"modified": "2024-12-30T18:14:22.143Z",
"created": "2024-12-30T18:14:22.143Z",
"1.0.0": "2024-12-30T18:14:22.143Z"
},
"users": {},
"dist-tags": {
"latest": "1.0.0"
},
"_uplinks": {},
"_distfiles": {},
"_attachments": {
"bundled-optional-peer-1.0.0.tgz": {
"shasum": "b0afca067791d99523e6e378f52cbd7f128ef730",
"version": "1.0.0"
}
},
"_rev": "",
"_id": "bundled-optional-peer",
"readme": ""
}