From 9cd72d209d133c2a8f56d23a67e241be22e765d8 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Wed, 23 Sep 2026 11:39:43 +0200 Subject: [PATCH] fix: support semver build metadata in workspace dependency resolution (pnpm/pnpm#6483) (#15340) When a workspace package version carries SemVer build metadata, exact version specifiers (e.g. `workspace:0.5.6-next.3+f60facc` or `workspace:0.5.6-next.3`) failed to resolve because version-selector-type and Version normalization strip build metadata before matching keys in the workspace packages map. Fall back to SemVer range resolution for Version specifiers in both pnpm v11 (TypeScript) and v12 (Rust) to honor SemVer precedence rules (spec item 10) where build metadata is ignored. Closes pnpm/pnpm#6483 --- ...uet-workspace-dependency-build-metadata.md | 5 ++ .../workspace-dependency-build-metadata.md | 7 ++ .../cli/tests/suite/workspace_install.rs | 41 ++++++++++ .../src/resolve_from_workspace.rs | 10 ++- .../src/resolve_from_workspace/tests.rs | 59 ++++++++++++++ .../test/install/multipleImporters.ts | 42 ++++++++++ .../src/wantedDepIsLocallyAvailable.ts | 27 ++----- .../test/wantedDepIsLocallyAvailable.test.ts | 70 ++++++++++++++++ pnpm11/resolving/npm-resolver/src/index.ts | 7 +- pnpm11/resolving/npm-resolver/test/index.ts | 81 +++++++++++++++++++ 10 files changed, 324 insertions(+), 25 deletions(-) create mode 100644 .changeset/pacquet-workspace-dependency-build-metadata.md create mode 100644 .changeset/workspace-dependency-build-metadata.md create mode 100644 pnpm11/installing/deps-resolver/test/wantedDepIsLocallyAvailable.test.ts diff --git a/.changeset/pacquet-workspace-dependency-build-metadata.md b/.changeset/pacquet-workspace-dependency-build-metadata.md new file mode 100644 index 0000000000..ddd306769c --- /dev/null +++ b/.changeset/pacquet-workspace-dependency-build-metadata.md @@ -0,0 +1,5 @@ +--- +"pacquet": patch +--- + +Support SemVer build metadata in workspace dependency resolution diff --git a/.changeset/workspace-dependency-build-metadata.md b/.changeset/workspace-dependency-build-metadata.md new file mode 100644 index 0000000000..b89c455d46 --- /dev/null +++ b/.changeset/workspace-dependency-build-metadata.md @@ -0,0 +1,7 @@ +--- +"@pnpm/resolving.npm-resolver": patch +"@pnpm/installing.deps-resolver": patch +"pnpm": patch +--- + +Support SemVer build metadata in workspace dependency resolution diff --git a/pnpm/crates/cli/tests/suite/workspace_install.rs b/pnpm/crates/cli/tests/suite/workspace_install.rs index a64a73a3a1..f0513490c6 100644 --- a/pnpm/crates/cli/tests/suite/workspace_install.rs +++ b/pnpm/crates/cli/tests/suite/workspace_install.rs @@ -995,4 +995,45 @@ fn a_circular_peers_optional_peer_is_shared_by_every_importer() { drop((root, mock_instance)); } +#[test] +fn workspace_install_with_build_metadata_version() { + let CommandTempCwd { pacquet, root, workspace, .. } = CommandTempCwd::init(); + + fs::write(workspace.join("pnpm-workspace.yaml"), "packages:\n - 'packages/*'\n") + .expect("write pnpm-workspace.yaml"); + + fs::create_dir_all(workspace.join("packages/lib")).expect("mkdir packages/lib"); + fs::write( + workspace.join("packages/lib/package.json"), + serde_json::json!({ + "name": "lib", + "version": "0.5.6-next.3+f60facc", + }) + .to_string(), + ) + .expect("write packages/lib/package.json"); + + fs::create_dir_all(workspace.join("packages/app")).expect("mkdir packages/app"); + fs::write( + workspace.join("packages/app/package.json"), + serde_json::json!({ + "name": "app", + "dependencies": { "lib": "workspace:0.5.6-next.3+f60facc" }, + }) + .to_string(), + ) + .expect("write packages/app/package.json"); + + pacquet + .with_args(["install", "--lockfile-only"]) + .assert() + .success(); + + let lockfile = + fs::read_to_string(workspace.join("pnpm-lock.yaml")).expect("read pnpm-lock.yaml"); + assert!(lockfile.contains("link:../lib")); + + drop(root); +} + mod freshness; diff --git a/pnpm/crates/resolving-npm-resolver/src/resolve_from_workspace.rs b/pnpm/crates/resolving-npm-resolver/src/resolve_from_workspace.rs index f178a69e36..fe636955f3 100644 --- a/pnpm/crates/resolving-npm-resolver/src/resolve_from_workspace.rs +++ b/pnpm/crates/resolving-npm-resolver/src/resolve_from_workspace.rs @@ -231,7 +231,11 @@ pub fn pick_matching_local_version_or_null( resolve_workspace_range("*", &raw) } RegistryPackageSpecType::Version => { - versions.contains_key(&spec.fetch_spec).then(|| spec.fetch_spec.clone()) + if versions.contains_key(&spec.fetch_spec) { + return Some(spec.fetch_spec.clone()); + } + let raw: Vec = versions.keys().cloned().collect(); + resolve_workspace_range(&spec.fetch_spec, &raw) } RegistryPackageSpecType::Range => { let raw: Vec = versions.keys().cloned().collect(); @@ -368,7 +372,9 @@ fn pathdiff_string(base: &Path, target: &Path) -> Option { /// reverse so the message at least stays stable. fn rcompare_versions(left: &str, right: &str) -> std::cmp::Ordering { match (Version::parse(left), Version::parse(right)) { - (Ok(left_parsed), Ok(right_parsed)) => right_parsed.cmp(&left_parsed), + (Ok(left_parsed), Ok(right_parsed)) => right_parsed + .cmp(&left_parsed) + .then_with(|| right.cmp(left)), _ => right.cmp(left), } } diff --git a/pnpm/crates/resolving-npm-resolver/src/resolve_from_workspace/tests.rs b/pnpm/crates/resolving-npm-resolver/src/resolve_from_workspace/tests.rs index 75a2171056..fb845e8162 100644 --- a/pnpm/crates/resolving-npm-resolver/src/resolve_from_workspace/tests.rs +++ b/pnpm/crates/resolving-npm-resolver/src/resolve_from_workspace/tests.rs @@ -42,9 +42,19 @@ fn build_packages() -> WorkspacePackages { }, ); + let mut meta: WorkspacePackagesByVersion = BTreeMap::new(); + meta.insert( + "0.5.6-next.3+f60facc".to_string(), + WorkspacePackage { + root_dir: Path::new("/repo/packages/meta").to_path_buf(), + manifest: json!({ "name": "meta", "version": "0.5.6-next.3+f60facc" }), + }, + ); + let mut packages: WorkspacePackages = BTreeMap::new(); packages.insert("foo".to_string(), foo); packages.insert("bar".to_string(), bar); + packages.insert("meta".to_string(), meta); packages } @@ -148,6 +158,25 @@ fn workspace_exact_version_picks_that_entry() { assert_eq!(result.id.as_str(), "link:../bar"); } +#[test] +fn workspace_build_metadata_resolution() { + let packages = build_packages(); + let opts = opts(&packages); + for specifier in [ + "workspace:0.5.6-next.3+f60facc", + "workspace:0.5.6-next.3", + "workspace:^0.5.6-next.3+f60facc", + "workspace:^0.5.6-next.3", + "workspace:~0.5.6-next.3+f60facc", + "workspace:*", + ] { + let result = try_resolve_from_workspace(&wanted("meta", specifier), &opts) + .expect("ok") + .unwrap_or_else(|| panic!("expected Some for {specifier}")); + assert_eq!(result.id.as_str(), "link:../meta", "specifier: {specifier}"); + } +} + #[test] fn aliased_workspace_form_routes_through_package_name() { let packages = build_packages(); @@ -178,6 +207,36 @@ fn no_matching_version_surfaces_pnpm_error_code() { assert!(matches!(err, ResolveFromWorkspaceError::NoMatchingVersionInsideWorkspace { .. })); } +#[test] +fn no_matching_version_available_versions_deterministic_ordering() { + let mut entries: WorkspacePackagesByVersion = BTreeMap::new(); + entries.insert( + "1.0.0+B".to_string(), + WorkspacePackage { + root_dir: Path::new("/repo/packages/b").to_path_buf(), + manifest: json!({ "name": "cased", "version": "1.0.0+B" }), + }, + ); + entries.insert( + "1.0.0+a".to_string(), + WorkspacePackage { + root_dir: Path::new("/repo/packages/a").to_path_buf(), + manifest: json!({ "name": "cased", "version": "1.0.0+a" }), + }, + ); + let mut packages: WorkspacePackages = BTreeMap::new(); + packages.insert("cased".to_string(), entries); + + let opts = opts(&packages); + let err = try_resolve_from_workspace(&wanted("cased", "workspace:^2.0.0"), &opts).unwrap_err(); + match err { + ResolveFromWorkspaceError::NoMatchingVersionInsideWorkspace { available, .. } => { + assert_eq!(available, ". Available versions: 1.0.0+a, 1.0.0+B"); + } + other => panic!("expected NoMatchingVersionInsideWorkspace, got {other:?}"), + } +} + #[test] fn workspace_packages_unset_surfaces_error() { let packages = build_packages(); diff --git a/pnpm11/installing/deps-installer/test/install/multipleImporters.ts b/pnpm11/installing/deps-installer/test/install/multipleImporters.ts index 70d3e25969..45aa0c4b1a 100644 --- a/pnpm11/installing/deps-installer/test/install/multipleImporters.ts +++ b/pnpm11/installing/deps-installer/test/install/multipleImporters.ts @@ -653,6 +653,48 @@ test('headless install is used with an up-to-date lockfile when package referenc projects['project-2'].has('is-negative') }) +test('workspace protocol resolves package with semver build metadata', async () => { + const pkg1 = { + name: 'project-1', + version: '1.0.0', + + dependencies: { + 'project-2': 'workspace:0.5.6-next.3+f60facc', + }, + } + const pkg2 = { + name: 'project-2', + version: '0.5.6-next.3+f60facc', + } + const projects = preparePackages([pkg1, pkg2]) + + const importers: MutatedProject[] = [ + { + mutation: 'install', + rootDir: path.resolve('project-1') as ProjectRootDir, + }, + { + mutation: 'install', + rootDir: path.resolve('project-2') as ProjectRootDir, + }, + ] + const allProjects = [ + { + buildIndex: 0, + manifest: pkg1, + rootDir: path.resolve('project-1') as ProjectRootDir, + }, + { + buildIndex: 0, + manifest: pkg2, + rootDir: path.resolve('project-2') as ProjectRootDir, + }, + ] + await mutateModules(importers, testDefaults({ allProjects })) + + projects['project-1'].has('project-2') +}) + test('headless install is used when packages are not linked from the workspace (unless workspace ranges are used)', async () => { const foo = { name: '@pnpm.e2e/foo', diff --git a/pnpm11/installing/deps-resolver/src/wantedDepIsLocallyAvailable.ts b/pnpm11/installing/deps-resolver/src/wantedDepIsLocallyAvailable.ts index 66eccd210b..07b479a4ce 100644 --- a/pnpm11/installing/deps-resolver/src/wantedDepIsLocallyAvailable.ts +++ b/pnpm11/installing/deps-resolver/src/wantedDepIsLocallyAvailable.ts @@ -1,5 +1,5 @@ -import { parseBareSpecifier, type RegistryPackageSpec } from '@pnpm/resolving.npm-resolver' -import type { WorkspacePackages, WorkspacePackagesByVersion } from '@pnpm/resolving.resolver-base' +import { parseBareSpecifier, pickMatchingLocalVersionOrNull } from '@pnpm/resolving.npm-resolver' +import type { WorkspacePackages } from '@pnpm/resolving.resolver-base' import semver from 'semver' import type { WantedDependency } from './getNonDevWantedDependencies.js' @@ -14,23 +14,10 @@ export function wantedDepIsLocallyAvailable ( ): boolean { const spec = parseBareSpecifier(wantedDependency.bareSpecifier, wantedDependency.alias, opts.defaultTag || 'latest', opts.registry) if ((spec == null) || !workspacePackages.has(spec.name)) return false - return pickMatchingLocalVersionOrNull(workspacePackages.get(spec.name)!, spec) !== null + const matchingVersions = workspacePackages.get(spec.name)! + if (spec.type === 'tag') { + return semver.maxSatisfying(Array.from(matchingVersions.keys()), '*') !== null + } + return pickMatchingLocalVersionOrNull(matchingVersions, spec) !== null } -// TODO: move this function to separate package or import from @pnpm/resolving.npm-resolver -function pickMatchingLocalVersionOrNull ( - versions: WorkspacePackagesByVersion, - spec: RegistryPackageSpec -): string | null { - const localVersions = Array.from(versions.keys()) - switch (spec.type) { - case 'tag': - return semver.maxSatisfying(localVersions, '*') - case 'version': - return versions.has(spec.fetchSpec) ? spec.fetchSpec : null - case 'range': - return semver.maxSatisfying(localVersions, spec.fetchSpec, true) - default: - return null - } -} diff --git a/pnpm11/installing/deps-resolver/test/wantedDepIsLocallyAvailable.test.ts b/pnpm11/installing/deps-resolver/test/wantedDepIsLocallyAvailable.test.ts new file mode 100644 index 0000000000..eea53e1683 --- /dev/null +++ b/pnpm11/installing/deps-resolver/test/wantedDepIsLocallyAvailable.test.ts @@ -0,0 +1,70 @@ +import { expect, test } from '@jest/globals' +import type { WorkspacePackages } from '@pnpm/resolving.resolver-base' +import type { ProjectRootDir } from '@pnpm/types' + +import { wantedDepIsLocallyAvailable } from '../src/wantedDepIsLocallyAvailable.js' + +test('wantedDepIsLocallyAvailable matches packages with build metadata', () => { + const workspacePackages: WorkspacePackages = new Map([ + ['foo', new Map([ + ['0.5.6-next.3+f60facc', { + rootDir: '/workspace/foo' as ProjectRootDir, + manifest: { + name: 'foo', + version: '0.5.6-next.3+f60facc', + }, + }], + ])], + ]) + + for (const bareSpecifier of [ + '0.5.6-next.3+f60facc', + '0.5.6-next.3', + '^0.5.6-next.3+f60facc', + '^0.5.6-next.3', + '~0.5.6-next.3+f60facc', + '*', + ]) { + expect(wantedDepIsLocallyAvailable( + workspacePackages, + { alias: 'foo', bareSpecifier, dev: false, optional: false }, + { defaultTag: 'latest', registry: 'https://registry.npmjs.org/' } + )).toBe(true) + } +}) + +test('wantedDepIsLocallyAvailable preserves stable-only behavior for tag selectors', () => { + const workspacePackages: WorkspacePackages = new Map([ + ['foo', new Map([ + ['0.5.6-next.3+f60facc', { + rootDir: '/workspace/foo' as ProjectRootDir, + manifest: { + name: 'foo', + version: '0.5.6-next.3+f60facc', + }, + }], + ])], + ['bar', new Map([ + ['1.0.0', { + rootDir: '/workspace/bar' as ProjectRootDir, + manifest: { + name: 'bar', + version: '1.0.0', + }, + }], + ])], + ]) + + expect(wantedDepIsLocallyAvailable( + workspacePackages, + { alias: 'foo', bareSpecifier: 'latest', dev: false, optional: false }, + { defaultTag: 'latest', registry: 'https://registry.npmjs.org/' } + )).toBe(false) + + expect(wantedDepIsLocallyAvailable( + workspacePackages, + { alias: 'bar', bareSpecifier: 'latest', dev: false, optional: false }, + { defaultTag: 'latest', registry: 'https://registry.npmjs.org/' } + )).toBe(true) +}) + diff --git a/pnpm11/resolving/npm-resolver/src/index.ts b/pnpm11/resolving/npm-resolver/src/index.ts index f53f3eb8e6..22b66371cb 100644 --- a/pnpm11/resolving/npm-resolver/src/index.ts +++ b/pnpm11/resolving/npm-resolver/src/index.ts @@ -1120,7 +1120,7 @@ function tryResolveFromWorkspacePackages ( opts.update ? { name: spec.name, fetchSpec: '*', type: 'range' } : spec ) if (!localVersion) { - const availableVersions = Array.from(workspacePkgsMatchingName.keys()).sort((a, b) => semver.rcompare(a, b)) + const availableVersions = Array.from(workspacePkgsMatchingName.keys()).sort((a, b) => semver.rcompare(a, b) || (b < a ? -1 : b > a ? 1 : 0)) throw new PnpmError( 'NO_MATCHING_VERSION_INSIDE_WORKSPACE', `In ${path.relative(process.cwd(), opts.projectDir)}: No matching version found for ${opts.wantedDependency.alias ?? ''}@${opts.wantedDependency.bareSpecifier ?? ''} inside the workspace` + @@ -1135,7 +1135,7 @@ function tryResolveFromWorkspacePackages ( return resolveFromLocalPackage(workspacePkgsMatchingName.get(localVersion)!, spec, opts) } -function pickMatchingLocalVersionOrNull ( +export function pickMatchingLocalVersionOrNull ( versions: WorkspacePackagesByVersion, spec: RegistryPackageSpec ): string | null { @@ -1145,7 +1145,8 @@ function pickMatchingLocalVersionOrNull ( includePrerelease: true, }) case 'version': - return versions.has(spec.fetchSpec) ? spec.fetchSpec : null + if (versions.has(spec.fetchSpec)) return spec.fetchSpec + return resolveWorkspaceRange(spec.fetchSpec, Array.from(versions.keys())) case 'range': return resolveWorkspaceRange(spec.fetchSpec, Array.from(versions.keys())) default: diff --git a/pnpm11/resolving/npm-resolver/test/index.ts b/pnpm11/resolving/npm-resolver/test/index.ts index 782fce2c88..79fc951405 100644 --- a/pnpm11/resolving/npm-resolver/test/index.ts +++ b/pnpm11/resolving/npm-resolver/test/index.ts @@ -2795,6 +2795,87 @@ test('workspace protocol: resolution fails if there is no matching local package expect(err.message).toBe(`In ${path.relative(process.cwd(), projectDir)}: No matching version found for is-positive@workspace:^3.0.0 inside the workspace. Available versions: 2.0.0`) }) +test.each([ + 'workspace:0.5.6-next.3+f60facc', + 'workspace:0.5.6-next.3', + 'workspace:^0.5.6-next.3+f60facc', + 'workspace:^0.5.6-next.3', + 'workspace:~0.5.6-next.3+f60facc', + 'workspace:*', +])('workspace protocol: resolves a package with build metadata in version (%s)', async (bareSpecifier) => { + const cacheDir = temporaryDirectory() + const { resolveFromNpm } = createResolveFromNpm({ + storeDir: temporaryDirectory(), + cacheDir, + registriesByScope, + }) + + const projectDir = '/home/istvan/src' + const workspacePackages = new Map([ + ['is-positive', new Map([ + ['0.5.6-next.3+f60facc', { + rootDir: '/home/istvan/src/is-positive' as ProjectRootDir, + manifest: { + name: 'is-positive', + version: '0.5.6-next.3+f60facc', + }, + }], + ])], + ]) + + const resolution = await resolveFromNpm({ alias: 'is-positive', bareSpecifier }, { + projectDir, + workspacePackages, + }) + + expect(resolution).toBeTruthy() + expect(resolution!.resolvedVia).toBe('workspace') + expect(resolution!.manifest.version).toBe('0.5.6-next.3+f60facc') +}) + +test('workspace protocol: resolution fails with deterministic ordinal ordering for available versions with equal semver precedence', async () => { + const cacheDir = temporaryDirectory() + const { resolveFromNpm } = createResolveFromNpm({ + storeDir: temporaryDirectory(), + cacheDir, + registriesByScope, + }) + + const projectDir = '/home/istvan/src' + const workspacePackages = new Map([ + ['is-positive', new Map([ + ['1.0.0+B', { + rootDir: '/home/istvan/src/is-positive-b' as ProjectRootDir, + manifest: { + name: 'is-positive', + version: '1.0.0+B', + }, + }], + ['1.0.0+a', { + rootDir: '/home/istvan/src/is-positive-a' as ProjectRootDir, + manifest: { + name: 'is-positive', + version: '1.0.0+a', + }, + }], + ])], + ]) + + let err!: Error + try { + await resolveFromNpm({ alias: 'is-positive', bareSpecifier: 'workspace:^2.0.0' }, { + projectDir, + workspacePackages, + }) + } catch (_err: any) { // eslint-disable-line + err = _err + } + + expect(err).toBeTruthy() + expect(err.message).toBe(`In ${path.relative(process.cwd(), projectDir)}: No matching version found for is-positive@workspace:^2.0.0 inside the workspace. Available versions: 1.0.0+a, 1.0.0+B`) +}) + + test('workspace protocol: resolution fails if there are no local packages', async () => { const cacheDir = temporaryDirectory() const { resolveFromNpm } = createResolveFromNpm({