Skip to content
Merged
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
56 changes: 53 additions & 3 deletions src/install/lockfile/bun.lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1505,6 +1505,38 @@ impl<T> PkgMap<T> {
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 one level above a
/// bundled package, mirroring `Tree::hoist_dependency`, which never
/// searches past a bundled dependency's hoist root when it re-derives
/// optional peer edges (#37346).
///
/// Only `"bundled": true` entries bound the walk: a transitive dependency
/// of a bundled package also inherits the bundle's hoist root in
/// `Tree.rs`, but its lockfile path (`a/c`, no bundled marker) is
/// indistinguishable from an ordinary conflict-nested package whose hoist
/// root is the lockfile root.
Comment thread
robobun marked this conversation as resolved.
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 +1548,7 @@ impl<T> PkgMap<T> {
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 +1558,14 @@ impl<T> PkgMap<T> {
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]);
}
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 +2994,21 @@ pub(crate) fn parse_into_binary_lockfile(
let res_id = match peer_res_id {
Some(id) => id,
None => {
match pkg_map.find_resolution(pkg_path, dep, string_buf, &mut path_buf[..])
{
// Bounded so the loaded lockfile binds optional peers
// exactly like the hoister that re-derives them after
// `Package::clone` resets them (#37346).
Comment thread
robobun marked this conversation as resolved.
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": ""
}