diff --git a/src/install/lockfile/bun.lock.rs b/src/install/lockfile/bun.lock.rs index be1254346f20..55f4556cd090 100644 --- a/src/install/lockfile/bun.lock.rs +++ b/src/install/lockfile/bun.lock.rs @@ -650,6 +650,7 @@ impl Stringifier { let mut tree_deps_sort_buf: Vec = Vec::new(); let mut pkg_deps_sort_buf: Vec = Vec::new(); + let mut entry_path_buf: Vec = Vec::new(); Self::write_indent(writer, *indent)?; writer.write_all(b"\"packages\": {")?; @@ -707,29 +708,23 @@ impl Stringifier { Self::write_indent(writer, *indent)?; } - writer.write_byte(b'"')?; - // relative_path is empty string for root resolutions - write!( - writer, - "{}", - bun_core::fmt::format_json_string_utf8( - relative_path, - bun_core::fmt::JSONFormatterUTF8Options { quote: false } - ), - )?; + let dep = &deps_buf[dep_id as usize]; + let dep_name = dep.name.slice(buf); + // The entry key doubles as the tree path this entry's dependencies resolve from. + entry_path_buf.clear(); + entry_path_buf.extend_from_slice(relative_path); if *depth != 0 { - writer.write_byte(b'/')?; + entry_path_buf.push(b'/'); } - - let dep = &deps_buf[dep_id as usize]; - let dep_name = dep.name.slice(buf); + entry_path_buf.extend_from_slice(dep_name); + let entry_path: &[u8] = entry_path_buf.as_slice(); write!( writer, - "{}\": ", + "\"{}\": ", bun_core::fmt::format_json_string_utf8( - dep_name, + entry_path, bun_core::fmt::JSONFormatterUTF8Options { quote: false } ), )?; @@ -826,7 +821,7 @@ impl Stringifier { &mut optional_peers_buf, extern_strings, &pkg_map, - relative_path, + entry_path, &mut path_buf, )?; @@ -851,7 +846,7 @@ impl Stringifier { &mut optional_peers_buf, extern_strings, &pkg_map, - relative_path, + entry_path, &mut path_buf, )?; @@ -881,7 +876,7 @@ impl Stringifier { &mut optional_peers_buf, extern_strings, &pkg_map, - relative_path, + entry_path, &mut path_buf, )?; @@ -910,7 +905,7 @@ impl Stringifier { &mut optional_peers_buf, extern_strings, &pkg_map, - relative_path, + entry_path, &mut path_buf, )?; @@ -957,7 +952,7 @@ impl Stringifier { &mut optional_peers_buf, extern_strings, &pkg_map, - relative_path, + entry_path, &mut path_buf, )?; @@ -1005,7 +1000,7 @@ impl Stringifier { &mut optional_peers_buf, extern_strings, &pkg_map, - relative_path, + entry_path, &mut path_buf, )?; @@ -1054,7 +1049,8 @@ impl Stringifier { optional_peers_buf: &mut Vec, extern_strings: &[ExternalString], pkg_map: &PkgMap<()>, - relative_path: &[u8], + // The package's own tree path (its `packages` key). + pkg_path: &[u8], path_buf: &mut [u8], ) -> Result<(), WriteError> { // `optional_peers_buf` is cleared at the fn tail. @@ -1109,7 +1105,7 @@ impl Stringifier { && pkg_map.map.len() > 0 { if pkg_map - .find_resolution(relative_path, dep, buf, path_buf) + .find_resolution(pkg_path, dep, buf, path_buf) .is_err() { optional_peers_buf.push(dep.name); @@ -1230,7 +1226,8 @@ impl Stringifier { workspace_versions: &VersionHashMap, optional_peers_buf: &mut Vec, pkg_map: &PkgMap<()>, - relative_path: &[u8], + // The workspace package's tree path ("" for the root package). + pkg_path: &[u8], path_buf: &mut [u8], ) -> Result<(), WriteError> { // `optional_peers_buf` is cleared at the fn tail. @@ -1351,7 +1348,7 @@ impl Stringifier { && !dep.behavior.contains(Behavior::OPTIONAL) && pkg_map.map.len() > 0 { - if let Err(err) = pkg_map.find_resolution(relative_path, dep, buf, path_buf) { + if let Err(err) = pkg_map.find_resolution(pkg_path, dep, buf, path_buf) { if err == ResolveError::Unresolvable { optional_peers_buf.push(dep.name); } diff --git a/test/cli/install/migration/migrate.test.ts b/test/cli/install/migration/migrate.test.ts index 7155b6b44b66..50200f0c6719 100644 --- a/test/cli/install/migration/migrate.test.ts +++ b/test/cli/install/migration/migrate.test.ts @@ -484,6 +484,75 @@ test.concurrent.each([ }, ); +test.concurrent( + "migrated bun.lock with a nested-placed peer of a file: dependency round-trips --frozen-lockfile", + async () => { + // delta is a regular peer of gamma with only a nested placement ("gamma/delta"). + // The migration writer must not also list it in "optionalPeers", or reloading + // drops the entry and --frozen-lockfile fails on an unchanged tree. + await using testDir = tempDir("migrate-nested-peer-fixed-point", { + "package.json": JSON.stringify({ + name: "sandbox", + version: "1.0.0", + workspaces: ["packages/ws-a"], + dependencies: { gamma: "file:vendor/gamma" }, + }), + "vendor/gamma/package.json": JSON.stringify({ + name: "gamma", + version: "1.0.0", + peerDependencies: { delta: "*" }, + }), + "vendor/delta/package.json": JSON.stringify({ name: "delta", version: "2.0.0" }), + "packages/ws-a/package.json": JSON.stringify({ + name: "ws-a", + version: "0.1.0", + dependencies: { delta: "file:../../vendor/delta" }, + }), + // What `npm install --package-lock-only` produces for this tree. + "package-lock.json": JSON.stringify({ + name: "sandbox", + version: "1.0.0", + lockfileVersion: 3, + requires: true, + packages: { + "": { + name: "sandbox", + version: "1.0.0", + dependencies: { gamma: "file:vendor/gamma" }, + workspaces: ["packages/ws-a"], + }, + "node_modules/gamma": { resolved: "vendor/gamma", link: true }, + "vendor/gamma": { name: "gamma", version: "1.0.0", peerDependencies: { delta: "*" } }, + "node_modules/delta": { resolved: "vendor/delta", link: true }, + "vendor/delta": { name: "delta", version: "2.0.0" }, + "node_modules/ws-a": { resolved: "packages/ws-a", link: true }, + "packages/ws-a": { name: "ws-a", version: "0.1.0", dependencies: { delta: "file:../../vendor/delta" } }, + }, + }), + }); + + const first = await install(testDir); + expect(first.stderr).toContain("migrated lockfile from package-lock.json"); + expect(first.exitCode).toBe(0); + + const lock = await Bun.file(join(testDir, "bun.lock")).text(); + // The peer's resolution is recorded right below in the same lockfile, so the + // peer is not optional. + expect(lock).toContain('"gamma/delta"'); + expect(lock).not.toContain("optionalPeers"); + + // An unchanged tree satisfies --frozen-lockfile... + const frozen = await install(testDir, "--frozen-lockfile"); + expect(frozen.stderr).not.toContain("lockfile had changes"); + expect(frozen.exitCode).toBe(0); + + // ...and a plain re-install does not rewrite the lockfile. + const second = await install(testDir); + expect(second.exitCode).toBe(0); + expect(await Bun.file(join(testDir, "bun.lock")).text()).toBe(lock); + }, +); + test.concurrent("package-lock.json migration does not platform-skip a regular file: tarball dependency", async () => { // Same divergence as the folder variant, for a `LocalTarball` resolution. npm records // the packed package's `os`/`cpu` arrays in its lockfile entry, and a fresh resolve of