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({