diff --git a/src/install/lockfile/Package.rs b/src/install/lockfile/Package.rs index da74ede1d3b9..c313da5b5cfc 100644 --- a/src/install/lockfile/Package.rs +++ b/src/install/lockfile/Package.rs @@ -111,6 +111,26 @@ pub(crate) fn value_loc_of(source: &bun_ast::Source, key_loc: bun_ast::Loc) -> b crate::bun_json::property_value_loc(&source.contents, key_loc).unwrap_or(key_loc) } +/// True when `folder`, a folder-dependency path relative to the workspace +/// root (either platform's separators), names the directory of the member +/// registered at `workspace_path` (posix). +fn folder_path_is_workspace_path(folder: &[u8], workspace_path: &[u8]) -> bool { + let mut buf = path::path_buffer_pool::get(); + let normalized = resolve_path::join_string_buf::(&mut buf[..], &[folder]); + if normalized.len() != workspace_path.len() { + return false; + } + if cfg!(windows) { + for (&a, &b) in normalized.iter().zip(workspace_path) { + if a != b && !(a == b'\\' && b == b'/') { + return false; + } + } + return true; + } + strings::eql(normalized, workspace_path) +} + #[cold] fn invalid_trusted_dependencies( log: &mut bun_ast::Log, @@ -1748,6 +1768,36 @@ impl Package { // if relative is empty, we are linking the package to itself dependency_version.value.folder = string_builder .append::(if relative.is_empty() { b"." } else { relative }); + + // The root package's dependencies share one node_modules + // entry per name with the workspace members, so a root folder + // dependency colliding with a member resolves to the member, + // as in npm. Both placed side by side would serialize the + // same bun.lock key twice. + if let Some(workspace_path) = workspace_path { + let collides = package_dependencies[0..dependencies_count as usize] + .iter() + .any(|dep| dep.name_hash == name_hash && dep.behavior.is_workspace()); + if collides + && !folder_path_is_workspace_path(relative, workspace_path.slice(buf)) + { + let path = workspace_path.sliced(buf); + if let Some(mut dep) = dependency::parse_with_tag( + external_alias.value, + Some(external_alias.hash), + path.slice, + dependency::version::Tag::Workspace, + &path, + Some(&mut *log), + Some(&mut *pm), + ) { + // Whole-struct move so `Drop` frees the old + // chain; keep the existing `literal`. + dep.literal = dependency_version.literal; + dependency_version = dep; + } + } + } } dependency::version::Tag::Npm => { if let Some(workspace_version) = workspace_version { diff --git a/test/cli/install/bun-workspaces.test.ts b/test/cli/install/bun-workspaces.test.ts index 8892276eaf73..13d683716bf2 100644 --- a/test/cli/install/bun-workspaces.test.ts +++ b/test/cli/install/bun-workspaces.test.ts @@ -278,6 +278,205 @@ test("dependency on same name as workspace and dist-tag", async () => { ]); }); +describe("workspace and file: dependency sharing a name", () => { + // A root `file:` dependency and a same-named workspace member share one + // root node_modules entry. The member wins and the dependency resolves to + // it, as in npm. Both used to be placed side by side, writing duplicate + // bun.lock keys that broke --frozen-lockfile. + + test.concurrent("resolves to the member and the lockfile round-trips", async () => { + using ctx = await setupTest(); + const { packageDir, env } = ctx; + await Promise.all([ + write( + join(packageDir, "package.json"), + JSON.stringify({ + name: "sandbox", + version: "1.0.0", + workspaces: ["packages/*"], + dependencies: { alpha: "file:./vendor/alpha" }, + }), + ), + write(join(packageDir, "packages", "alpha", "package.json"), JSON.stringify({ name: "alpha" })), + write(join(packageDir, "vendor", "alpha", "package.json"), JSON.stringify({ name: "alpha", version: "2.0.0" })), + ]); + + await runBunInstall(env, packageDir); + + const lockfile = await file(join(packageDir, "bun.lock")).text(); + expect(lockfile.match(/"alpha": \[/g)).toHaveLength(1); + expect(lockfile).toContain(`"alpha": ["alpha@workspace:packages/alpha"]`); + expect(lockfile).not.toContain("alpha@file:"); + expect(await file(join(packageDir, "node_modules", "alpha", "package.json")).json()).toEqual({ + name: "alpha", + }); + + const second = await runBunInstall(env, packageDir, { savesLockfile: false }); + expect(second.err).not.toContain("Saved lockfile"); + expect(await file(join(packageDir, "bun.lock")).text()).toBe(lockfile); + + await runBunInstall(env, packageDir, { frozenLockfile: true }); + }); + + test.concurrent("member with its own dependencies still resolves them", async () => { + using ctx = await setupTest(); + const { packageDir, env } = ctx; + await Promise.all([ + write( + join(packageDir, "package.json"), + JSON.stringify({ + name: "sandbox", + version: "1.0.0", + workspaces: ["packages/*"], + dependencies: { alpha: "file:./vendor/alpha" }, + }), + ), + write( + join(packageDir, "packages", "alpha", "package.json"), + JSON.stringify({ + name: "alpha", + version: "1.0.0", + dependencies: { "alpha-dep": "file:../../vendor/alpha-dep" }, + }), + ), + write( + join(packageDir, "packages", "beta", "package.json"), + JSON.stringify({ name: "beta", version: "1.0.0", dependencies: { alpha: "workspace:*" } }), + ), + write(join(packageDir, "vendor", "alpha", "package.json"), JSON.stringify({ name: "alpha", version: "2.0.0" })), + write( + join(packageDir, "vendor", "alpha-dep", "package.json"), + JSON.stringify({ name: "alpha-dep", version: "3.0.0" }), + ), + ]); + + await runBunInstall(env, packageDir); + + const lockfile = await file(join(packageDir, "bun.lock")).text(); + expect(lockfile.match(/"alpha": \[/g)).toHaveLength(1); + expect(lockfile).toContain(`"alpha": ["alpha@workspace:packages/alpha"]`); + expect(lockfile).not.toContain("alpha@file:"); + expect(await file(join(packageDir, "node_modules", "alpha", "package.json")).json()).toEqual({ + name: "alpha", + version: "1.0.0", + dependencies: { "alpha-dep": "file:../../vendor/alpha-dep" }, + }); + expect( + await file(join(packageDir, "node_modules", "alpha", "node_modules", "alpha-dep", "package.json")).json(), + ).toEqual({ + name: "alpha-dep", + version: "3.0.0", + }); + + const second = await runBunInstall(env, packageDir, { savesLockfile: false }); + expect(second.err).not.toContain("Saved lockfile"); + expect(await file(join(packageDir, "bun.lock")).text()).toBe(lockfile); + + await runBunInstall(env, packageDir, { frozenLockfile: true }); + }); + + test.concurrent("file: dependency folder depending back on the member", async () => { + using ctx = await setupTest(); + const { packageDir, env } = ctx; + await Promise.all([ + write( + join(packageDir, "package.json"), + JSON.stringify({ + name: "sandbox", + version: "1.0.0", + workspaces: ["packages/*"], + dependencies: { alpha: "file:./vendor/alpha" }, + }), + ), + write(join(packageDir, "packages", "alpha", "package.json"), JSON.stringify({ name: "alpha", version: "1.0.0" })), + write( + join(packageDir, "vendor", "alpha", "package.json"), + JSON.stringify({ name: "alpha", version: "2.0.0", dependencies: { alpha: "workspace:*" } }), + ), + ]); + + await runBunInstall(env, packageDir); + + const lockfile = await file(join(packageDir, "bun.lock")).text(); + expect(lockfile.match(/"alpha": \[/g)).toHaveLength(1); + expect(lockfile).toContain(`"alpha": ["alpha@workspace:packages/alpha"]`); + expect(lockfile).not.toContain("alpha@file:"); + expect(lockfile).not.toContain("alpha/alpha"); + + const second = await runBunInstall(env, packageDir, { savesLockfile: false }); + expect(second.err).not.toContain("Saved lockfile"); + expect(await file(join(packageDir, "bun.lock")).text()).toBe(lockfile); + + await runBunInstall(env, packageDir, { frozenLockfile: true }); + }); + + test.concurrent.each(["relative", "absolute"])( + "%s file: dependency naming the member's directory still links the member", + async kind => { + using ctx = await setupTest(); + const { packageDir, env } = ctx; + const literal = kind === "absolute" ? `file:${join(packageDir, "packages", "alpha")}` : "file:./packages/alpha"; + await Promise.all([ + write( + join(packageDir, "package.json"), + JSON.stringify({ + name: "sandbox", + version: "1.0.0", + workspaces: ["packages/*"], + dependencies: { alpha: literal }, + }), + ), + write( + join(packageDir, "packages", "alpha", "package.json"), + JSON.stringify({ name: "alpha", version: "1.0.0" }), + ), + ]); + + await runBunInstall(env, packageDir); + + const lockfile = await file(join(packageDir, "bun.lock")).text(); + expect(lockfile.match(/"alpha": \[/g)).toHaveLength(1); + expect(lockfile).toContain(`"alpha": ["alpha@workspace:packages/alpha"]`); + expect(lockfile).not.toContain("alpha@file:"); + expect(await file(join(packageDir, "node_modules", "alpha", "package.json")).json()).toEqual({ + name: "alpha", + version: "1.0.0", + }); + }, + ); + + test.concurrent("aliased file: dependency colliding with another member's name", async () => { + using ctx = await setupTest(); + const { packageDir, env } = ctx; + await Promise.all([ + write( + join(packageDir, "package.json"), + JSON.stringify({ + name: "sandbox", + version: "1.0.0", + workspaces: ["packages/*"], + dependencies: { beta: "file:./vendor/alpha" }, + }), + ), + write(join(packageDir, "packages", "beta", "package.json"), JSON.stringify({ name: "beta", version: "1.0.0" })), + write(join(packageDir, "vendor", "alpha", "package.json"), JSON.stringify({ name: "alpha", version: "2.0.0" })), + ]); + + await runBunInstall(env, packageDir); + + const lockfile = await file(join(packageDir, "bun.lock")).text(); + expect(lockfile.match(/"beta": \[/g)).toHaveLength(1); + expect(lockfile).toContain(`"beta": ["beta@workspace:packages/beta"]`); + expect(lockfile).not.toContain("@file:"); + expect(await file(join(packageDir, "node_modules", "beta", "package.json")).json()).toEqual({ + name: "beta", + version: "1.0.0", + }); + + await runBunInstall(env, packageDir, { frozenLockfile: true }); + }); +}); + test.concurrent("successfully installs workspace when path already exists in node_modules", async () => { using ctx = await setupTest(); const { packageDir, env } = ctx;