diff --git a/src/install/PackageManager/PackageManagerEnqueue.rs b/src/install/PackageManager/PackageManagerEnqueue.rs index f0c91da56b53..06bbdee272dc 100644 --- a/src/install/PackageManager/PackageManagerEnqueue.rs +++ b/src/install/PackageManager/PackageManagerEnqueue.rs @@ -687,13 +687,15 @@ pub fn enqueue_dependency_with_main_and_success_fn( } } - // allow overriding all dependencies unless the dependency is coming directly from an alias, "npm:" or - // if it's a workspaceOnly dependency + // allow overriding all dependencies unless the dependency is coming directly from an alias, "npm:", + // is a workspaceOnly dependency, or links a workspace member of this install if !dependency.behavior.is_workspace() && (dependency.version.tag != dependency::version::Tag::Npm || !dependency.version.npm().is_alias) { - if let Some(new) = this.lockfile.overrides.get(name_hash) { + if let Some(new) = this.lockfile.overrides.get(name_hash) + && !links_workspace_member(this, &dependency.version, name, name_hash) + { bun_output::scoped_log!( PackageManager, "override: {} -> {}", @@ -1908,6 +1910,62 @@ fn enqueue_local_tarball( unsafe { &raw mut (*task).threadpool_task } } +/// Would this edge, absent overrides, resolve to a workspace member of the +/// current install? Overrides skip such edges so they never displace a member +/// link. Mirrors the linking rules: `Package::parse` rewrites satisfying npm +/// ranges on versioned members to workspace links, and +/// `get_or_put_resolved_package` links wildcard ranges through the root's +/// workspace edges (`'resolve_from_workspace`). A `catalog:` edge links +/// through whatever version the catalog materializes to. Ranges that do not +/// link, and names whose member is missing from this run, stay overridable. +fn links_workspace_member( + this: &PackageManager, + version: &dependency::Version, + name: SemverString, + name_hash: PackageNameHash, +) -> bool { + let catalog_version; + let version = if version.tag == dependency::version::Tag::Catalog { + match this + .lockfile + .catalogs + .get(&this.lockfile, *version.catalog(), name) + { + Some(catalog_dep) => { + catalog_version = catalog_dep.version; + &catalog_version + } + None => return false, + } + } else { + version + }; + + match version.tag { + dependency::version::Tag::Workspace => this.lockfile.is_workspace_member_name(name_hash), + dependency::version::Tag::Npm => { + let npm = version.npm(); + if npm.is_alias + || !this.options.link_workspace_packages + || !this.lockfile.is_workspace_member_name(name_hash) + { + return false; + } + if npm.version.is_star() { + return true; + } + let buf = this.lockfile.buffers.string_bytes.as_slice(); + this.lockfile + .workspace_versions + .get(&name_hash) + .is_some_and(|workspace_version| { + npm.version.satisfies(*workspace_version, buf, buf) + }) + } + _ => false, + } +} + fn update_name_and_name_hash_from_version_replacement( lockfile: &Lockfile::Lockfile, original_name: SemverString, diff --git a/src/install/lockfile.rs b/src/install/lockfile.rs index fdc2d43b1e3f..dcdeeac01b03 100644 --- a/src/install/lockfile.rs +++ b/src/install/lockfile.rs @@ -850,6 +850,23 @@ impl Lockfile { self.packages.items_dependencies()[0].contains(id) } + /// Is this name a workspace member of the current install, i.e. does the + /// root package carry an injected workspace edge for it? Unlike + /// `workspace_paths` (which a `workspace:` dependency with no matching + /// member also registers in) and `package_index` (which can still hold a + /// member a loaded lockfile resolved before the member directory was + /// removed), the root's workspace edges are rebuilt from package.json on + /// every install. + pub(crate) fn is_workspace_member_name(&self, name_hash: PackageNameHash) -> bool { + let Some(root) = self.root_package() else { + return false; + }; + root.dependencies + .get(self.buffers.dependencies.as_slice()) + .iter() + .any(|dep| dep.behavior.is_workspace() && dep.name_hash == name_hash) + } + /// Is this a direct dependency of the workspace the install is taking place in? pub(crate) fn is_root_dependency( &self, diff --git a/src/install/lockfile/bun.lock.rs b/src/install/lockfile/bun.lock.rs index be1254346f20..f64c4ea668fe 100644 --- a/src/install/lockfile/bun.lock.rs +++ b/src/install/lockfile/bun.lock.rs @@ -3081,7 +3081,10 @@ fn resolve_peer_dep_version_based( // already reflects the overridden resolution, so overridden edges keep // the path walk. The exemptions mirror // `enqueue_dependency_with_main_and_success_fn`: `npm:` aliases and - // workspace-only edges are never overridden. + // workspace-only edges are never overridden. The fresh resolver's + // member-link exemption needs no clause here: a workspace-tagged edge + // falls through to `None` either way (`satisfies_dependency_version` + // and the kind fallback below never match the Workspace tag). let overridable = !dep.behavior.is_workspace() && (dep.version.tag != DependencyVersionTag::Npm || !dep.version.npm().is_alias); if overridable && overrides.get(name_hash).is_some() { diff --git a/test/cli/install/bun-workspaces.test.ts b/test/cli/install/bun-workspaces.test.ts index 8892276eaf73..9cea5e384102 100644 --- a/test/cli/install/bun-workspaces.test.ts +++ b/test/cli/install/bun-workspaces.test.ts @@ -1912,6 +1912,563 @@ test.concurrent("can override npm package with workspace package under a differe }); }); +// An override for a name that is also a workspace member must not displace dependencies +// that link to the member. Previously the override rewrote the workspace-linked edge back +// to a registry range, leaving the root with two conflicting resolutions for the same name +// and every install failed with "Package \"no-deps@2.0.0\" has a dependency loop (DependencyLoop)". +test.concurrent("override for a workspace member's name does not displace the member", async () => { + using ctx = await setupTest(); + const { packageDir, packageJson, env } = ctx; + await Promise.all([ + write( + packageJson, + JSON.stringify({ + name: "root", + workspaces: ["packages/*"], + dependencies: { + "no-deps": "^1.0.0", + "one-dep": "1.0.0", + }, + overrides: { + "no-deps": "2.0.0", + }, + }), + ), + write(join(packageDir, "packages", "m", "package.json"), JSON.stringify({ name: "no-deps", version: "1.0.0" })), + ]); + + let { exited, stderr } = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: "ignore", + stderr: "pipe", + env, + }); + let stderrText = await stderr.text(); + expect(stderrText).not.toContain("dependency loop"); + expect(stderrText).toContain("Saved lockfile"); + expect(await exited).toBe(0); + + // the satisfying range links the member; the override does not unlink it + expect(await file(join(packageDir, "node_modules", "no-deps", "package.json")).json()).toEqual({ + name: "no-deps", + version: "1.0.0", + }); + // the override still applies to registry edges with the same name + expect( + await file(join(packageDir, "node_modules", "one-dep", "node_modules", "no-deps", "package.json")).json(), + ).toEqual({ + name: "no-deps", + version: "2.0.0", + }); + + // repeat install does not change the lockfile + ({ exited, stderr } = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: "ignore", + stderr: "pipe", + env, + })); + stderrText = await stderr.text(); + expect(stderrText).not.toContain("Saved lockfile"); + expect(await exited).toBe(0); + + // --frozen-lockfile reinstalls from the lockfile + await rm(join(packageDir, "node_modules"), { recursive: true, force: true }); + ({ exited, stderr } = spawn({ + cmd: [bunExe(), "install", "--frozen-lockfile"], + cwd: packageDir, + stdout: "ignore", + stderr: "pipe", + env, + })); + stderrText = await stderr.text(); + expect(stderrText).not.toContain("dependency loop"); + expect(await exited).toBe(0); + expect(await file(join(packageDir, "node_modules", "no-deps", "package.json")).json()).toEqual({ + name: "no-deps", + version: "1.0.0", + }); +}); + +test.concurrent("override for a workspace member's name does not apply to workspace: dependencies", async () => { + using ctx = await setupTest(); + const { packageDir, packageJson, env } = ctx; + await Promise.all([ + write( + packageJson, + JSON.stringify({ + name: "root", + workspaces: ["packages/*"], + dependencies: { + "no-deps": "workspace:*", + }, + overrides: { + "no-deps": "2.0.0", + }, + }), + ), + write(join(packageDir, "packages", "m", "package.json"), JSON.stringify({ name: "no-deps", version: "1.0.0" })), + ]); + + const { exited, stderr } = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: "ignore", + stderr: "pipe", + env, + }); + expect(await stderr.text()).not.toContain("dependency loop"); + expect(await exited).toBe(0); + expect(await file(join(packageDir, "node_modules", "no-deps", "package.json")).json()).toEqual({ + name: "no-deps", + version: "1.0.0", + }); +}); + +test.concurrent("override for a workspace member's name does not apply to a sibling member's dependency", async () => { + using ctx = await setupTest(); + const { packageDir, packageJson, env } = ctx; + await Promise.all([ + write( + packageJson, + JSON.stringify({ + name: "root", + workspaces: ["packages/*"], + overrides: { + "no-deps": "2.0.0", + }, + }), + ), + write(join(packageDir, "packages", "m", "package.json"), JSON.stringify({ name: "no-deps", version: "1.0.0" })), + write( + join(packageDir, "packages", "bar", "package.json"), + JSON.stringify({ name: "bar", version: "1.0.0", dependencies: { "no-deps": "^1.0.0" } }), + ), + ]); + + const { exited, stderr } = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: "ignore", + stderr: "pipe", + env, + }); + expect(await stderr.text()).not.toContain("dependency loop"); + expect(await exited).toBe(0); + // bar's satisfying range links the sibling member, so no overridden registry + // copy is nested under bar + expect(await file(join(packageDir, "node_modules", "no-deps", "package.json")).json()).toEqual({ + name: "no-deps", + version: "1.0.0", + }); + expect(await exists(join(packageDir, "node_modules", "bar", "node_modules", "no-deps"))).toBe(false); +}); + +test.concurrent("override still applies to a workspace: dependency with no matching member", async () => { + using ctx = await setupTest(); + const { packageDir, packageJson, env } = ctx; + // without the override this install fails with "Workspace dependency \"no-deps\" not found" + await write( + packageJson, + JSON.stringify({ + name: "root", + workspaces: ["packages/*"], + dependencies: { + "no-deps": "workspace:*", + }, + overrides: { + "no-deps": "2.0.0", + }, + }), + ); + + const proc = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: "ignore", + stderr: "pipe", + env, + }); + const [stderrText, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + expect(stderrText).not.toContain("error:"); + expect(exitCode).toBe(0); + expect(await file(join(packageDir, "node_modules", "no-deps", "package.json")).json()).toEqual({ + name: "no-deps", + version: "2.0.0", + }); +}); + +test.concurrent("override still applies when the range does not link the member", async () => { + using ctx = await setupTest(); + const { packageDir, packageJson, env } = ctx; + await Promise.all([ + write( + packageJson, + JSON.stringify({ + name: "root", + workspaces: ["packages/*"], + dependencies: { + "no-deps": "^3.0.0", + }, + overrides: { + "no-deps": "2.0.0", + }, + }), + ), + write(join(packageDir, "packages", "m", "package.json"), JSON.stringify({ name: "no-deps", version: "1.0.0" })), + ]); + + const proc = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: "ignore", + stderr: "pipe", + env, + }); + const [stderrText, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + expect(stderrText).not.toContain("error:"); + expect(exitCode).toBe(0); + // ^3.0.0 does not link the member, so the name resolves from the registry and + // the override picks the version + expect(await file(join(packageDir, "node_modules", "no-deps", "package.json")).json()).toEqual({ + name: "no-deps", + version: "2.0.0", + }); +}); + +test.concurrent("override for a workspace member's name does not apply to a member's peer dependency", async () => { + using ctx = await setupTest(); + const { packageDir, packageJson, env } = ctx; + await Promise.all([ + write( + packageJson, + JSON.stringify({ + name: "root", + workspaces: ["packages/*"], + overrides: { + "no-deps": "2.0.0", + }, + }), + ), + write(join(packageDir, "packages", "m", "package.json"), JSON.stringify({ name: "no-deps", version: "1.0.0" })), + write( + join(packageDir, "packages", "bar", "package.json"), + JSON.stringify({ name: "bar", version: "1.0.0", peerDependencies: { "no-deps": "^1.0.0" } }), + ), + ]); + + let { exited, stderr } = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: "ignore", + stderr: "pipe", + env, + }); + let stderrText = await stderr.text(); + expect(stderrText).not.toContain("dependency loop"); + expect(await exited).toBe(0); + // the peer binds to the member, not an overridden registry copy + expect(await file(join(packageDir, "node_modules", "no-deps", "package.json")).json()).toEqual({ + name: "no-deps", + version: "1.0.0", + }); + expect(await exists(join(packageDir, "node_modules", "bar", "node_modules", "no-deps"))).toBe(false); + + // a reinstall from the written lockfile binds the same way and does not re-save + ({ exited, stderr } = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: "ignore", + stderr: "pipe", + env, + })); + stderrText = await stderr.text(); + expect(stderrText).not.toContain("Saved lockfile"); + expect(await exited).toBe(0); +}); + +// Version-less members link through the resolver's wildcard rule instead of the +// parse-time rewrite, so their edges stay npm-tagged; the override exemption must +// cover that flavor too or the same DependencyLoop returns. +test.concurrent("override for a version-less workspace member's name does not displace the member", async () => { + using ctx = await setupTest(); + const { packageDir, packageJson, env } = ctx; + await Promise.all([ + write( + packageJson, + JSON.stringify({ + name: "root", + workspaces: ["packages/*"], + dependencies: { + "no-deps": "*", + }, + overrides: { + "no-deps": "2.0.0", + }, + }), + ), + write(join(packageDir, "packages", "m", "package.json"), JSON.stringify({ name: "no-deps" })), + write( + join(packageDir, "packages", "bar", "package.json"), + JSON.stringify({ name: "bar", version: "1.0.0", dependencies: { "no-deps": "*" } }), + ), + ]); + + const proc = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: "ignore", + stderr: "pipe", + env, + }); + const [stderrText, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + expect(stderrText).not.toContain("dependency loop"); + expect(exitCode).toBe(0); + expect(await file(join(packageDir, "node_modules", "no-deps", "package.json")).json()).toEqual({ + name: "no-deps", + }); + expect(await exists(join(packageDir, "node_modules", "bar", "node_modules", "no-deps"))).toBe(false); +}); + +test.concurrent("override for a workspace member's name does not apply to a catalog: dependency", async () => { + using ctx = await setupTest(); + const { packageDir, packageJson, env } = ctx; + await Promise.all([ + write( + packageJson, + JSON.stringify({ + name: "root", + workspaces: ["packages/*"], + catalog: { + "no-deps": "^1.0.0", + }, + dependencies: { + "no-deps": "catalog:", + }, + overrides: { + "no-deps": "2.0.0", + }, + }), + ), + write(join(packageDir, "packages", "m", "package.json"), JSON.stringify({ name: "no-deps", version: "1.0.0" })), + ]); + + const proc = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: "ignore", + stderr: "pipe", + env, + }); + const [stderrText, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + expect(stderrText).not.toContain("dependency loop"); + expect(exitCode).toBe(0); + // the catalog range links the member, so the override does not displace it + expect(await file(join(packageDir, "node_modules", "no-deps", "package.json")).json()).toEqual({ + name: "no-deps", + version: "1.0.0", + }); +}); + +// The exemption keys on the members of the current install, not on what the loaded +// lockfile resolved: removing the member directory must hand the name back to the +// override instead of failing on the stale workspace link forever. +test.concurrent("removing the member migrates an exempted workspace: dependency to the override", async () => { + using ctx = await setupTest(); + const { packageDir, packageJson, env } = ctx; + await Promise.all([ + write( + packageJson, + JSON.stringify({ + name: "root", + workspaces: ["packages/*"], + dependencies: { + "no-deps": "workspace:*", + }, + overrides: { + "no-deps": "2.0.0", + }, + }), + ), + write(join(packageDir, "packages", "m", "package.json"), JSON.stringify({ name: "no-deps", version: "1.0.0" })), + ]); + + let { exited, stderr } = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: "ignore", + stderr: "pipe", + env, + }); + let stderrText = await stderr.text(); + expect(stderrText).not.toContain("dependency loop"); + expect(await exited).toBe(0); + expect(await file(join(packageDir, "node_modules", "no-deps", "package.json")).json()).toEqual({ + name: "no-deps", + version: "1.0.0", + }); + + await rm(join(packageDir, "packages", "m"), { recursive: true, force: true }); + + // the stale lockfile no longer matches: frozen installs report it instead of + // failing on the missing workspace + ({ exited, stderr } = spawn({ + cmd: [bunExe(), "install", "--frozen-lockfile"], + cwd: packageDir, + stdout: "ignore", + stderr: "pipe", + env, + })); + stderrText = await stderr.text(); + expect(stderrText).toContain("lockfile had changes"); + expect(stderrText).not.toContain("Workspace dependency"); + expect(await exited).toBe(1); + + // a plain install migrates the name to the override + ({ exited, stderr } = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: "ignore", + stderr: "pipe", + env, + })); + stderrText = await stderr.text(); + expect(stderrText).not.toContain("Workspace dependency"); + expect(stderrText).toContain("Saved lockfile"); + expect(await exited).toBe(0); + expect(await file(join(packageDir, "node_modules", "no-deps", "package.json")).json()).toEqual({ + name: "no-deps", + version: "2.0.0", + }); + + // and converges + ({ exited, stderr } = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: "ignore", + stderr: "pipe", + env, + })); + stderrText = await stderr.text(); + expect(stderrText).not.toContain("Saved lockfile"); + expect(await exited).toBe(0); +}); + +test.concurrent("removing the member migrates an exempted npm-range dependency to the override", async () => { + using ctx = await setupTest(); + const { packageDir, packageJson, env } = ctx; + await Promise.all([ + write( + packageJson, + JSON.stringify({ + name: "root", + workspaces: ["packages/*"], + dependencies: { + "no-deps": "^1.0.0", + }, + overrides: { + "no-deps": "2.0.0", + }, + }), + ), + write(join(packageDir, "packages", "m", "package.json"), JSON.stringify({ name: "no-deps", version: "1.0.0" })), + ]); + + let { exited, stderr } = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: "ignore", + stderr: "pipe", + env, + }); + let stderrText = await stderr.text(); + expect(stderrText).not.toContain("dependency loop"); + expect(await exited).toBe(0); + expect(await file(join(packageDir, "node_modules", "no-deps", "package.json")).json()).toEqual({ + name: "no-deps", + version: "1.0.0", + }); + + await rm(join(packageDir, "packages", "m"), { recursive: true, force: true }); + + ({ exited, stderr } = spawn({ + cmd: [bunExe(), "install", "--frozen-lockfile"], + cwd: packageDir, + stdout: "ignore", + stderr: "pipe", + env, + })); + stderrText = await stderr.text(); + expect(stderrText).toContain("lockfile had changes"); + expect(await exited).toBe(1); + + ({ exited, stderr } = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: "ignore", + stderr: "pipe", + env, + })); + stderrText = await stderr.text(); + expect(stderrText).not.toContain("Workspace dependency"); + expect(stderrText).toContain("Saved lockfile"); + expect(await exited).toBe(0); + expect(await file(join(packageDir, "node_modules", "no-deps", "package.json")).json()).toEqual({ + name: "no-deps", + version: "2.0.0", + }); + + // and converges + ({ exited, stderr } = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: "ignore", + stderr: "pipe", + env, + })); + stderrText = await stderr.text(); + expect(stderrText).not.toContain("Saved lockfile"); + expect(await exited).toBe(0); +}); + +// A ranged `workspace:` dependency a present member cannot satisfy is rejected while +// parsing package.json, before overrides are consulted, with or without this fix. The +// override neither rescues it nor turns it into a resolution conflict. +test.concurrent("override does not rescue a workspace: range the member does not satisfy", async () => { + using ctx = await setupTest(); + const { packageDir, packageJson, env } = ctx; + await Promise.all([ + write( + packageJson, + JSON.stringify({ + name: "root", + workspaces: ["packages/*"], + dependencies: { + "no-deps": "workspace:^3.0.0", + }, + overrides: { + "no-deps": "2.0.0", + }, + }), + ), + write(join(packageDir, "packages", "m", "package.json"), JSON.stringify({ name: "no-deps", version: "1.0.0" })), + ]); + + const proc = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: "ignore", + stderr: "pipe", + env, + }); + const [stderrText, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + expect(stderrText).toContain('No matching version for workspace dependency "no-deps"'); + expect(stderrText).not.toContain("dependency loop"); + expect(exitCode).toBe(1); +}); + describe("LinkWorkspacePackages", () => { // Shared setup previously done in a `beforeEach`: each test gets its own dir, // a root workspace package.json, and a `no-deps` workspace package.