From 8cdc670197d915ab613b9f1d3db9eb28bc55f10f Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sat, 19 Sep 2026 12:05:46 +0200 Subject: [PATCH] feat(catalogs): support the file: and link: protocols in catalog entries (#15108) A catalog is declared in `pnpm-workspace.yaml`, so a relative path in an entry has to be measured from that directory rather than from the project that dereferences it. Catalog resolution now re-anchors a `file:` / `link:` entry on the consuming project's directory. That is the job the root-level `pnpm.overrides` rewrite already did for its own local targets, so its `LocalTarget` moves into the new `pnpm-local-spec` crate and both read from it. The shared version anchors through `pnpm_fs::relative_path`, which lexically normalizes both sides and refuses to diff across Windows path prefixes. `resolve_from_catalog` takes the anchor as a required argument, so every call site states whether it installs from the entry or only compares, displays, or re-anchors it itself. The install resolver re-anchors on the declaring manifest's directory; `pnpm dlx` renders an absolute path because it installs outside the workspace; overrides, peer ranges, `catalogMode`, `pnpm outdated`, and the published manifest read the entry as written. The lockfile's `catalogs:` snapshot keeps the entry exactly as `pnpm-workspace.yaml` writes it, and now records an entry with no version of its own at all, so editing a local entry's path invalidates the lockfile. Its `version` holds the resolved path for such an entry, taken from the lowest-sorting importer that resolves it. A `~/` path is not absolute as far as `Path::is_absolute` is concerned, so re-anchoring joined it onto the declaring file's directory and produced a path through a literal `~` component. The local resolver expands `~/` against the home directory itself and records the specifier verbatim, so such a path names the same place from every directory and is left where it is written, like an absolute one. `pnpm.overrides` shared the flaw and is fixed with it. `pnpm pack` and `pnpm publish` re-anchor a local entry on the package being exported. A `file:` dependency written directly in a project's manifest is exported unchanged and resolves from that project, so a catalog entry carrying the same text has to mean the same place rather than a path from the workspace root. Closes pnpm/pnpm#8642 --- .changeset/catalog-local-protocols.md | 5 + Cargo.lock | 10 + Cargo.toml | 1 + pnpm/crates/catalogs-resolver/Cargo.toml | 1 + pnpm/crates/catalogs-resolver/src/lib.rs | 61 +++-- pnpm/crates/catalogs-resolver/src/tests.rs | 106 +++++--- pnpm/crates/cli/src/cli_args/dlx.rs | 3 +- pnpm/crates/cli/src/cli_args/dlx/cache.rs | 21 +- pnpm/crates/cli/src/cli_args/outdated.rs | 3 +- .../crates/cli/src/cli_args/outdated/query.rs | 16 +- pnpm/crates/cli/src/cli_args/pack.rs | 1 + pnpm/crates/cli/src/cli_args/publish.rs | 12 +- .../tests/suite/add/aliasless_selectors.rs | 9 +- .../cli/tests/suite/catalog_local_deps.rs | 254 ++++++++++++++++++ pnpm/crates/cli/tests/suite/main.rs | 1 + pnpm/crates/cli/tests/suite/publish.rs | 48 ++++ pnpm/crates/config-parse-overrides/src/lib.rs | 13 +- pnpm/crates/deps-inspection-peers/src/lib.rs | 3 +- .../deps-inspection-peers/src/linked.rs | 12 +- pnpm/crates/exportable-manifest/src/create.rs | 24 +- .../exportable-manifest/src/create/tests.rs | 74 +++++ pnpm/crates/exportable-manifest/src/tests.rs | 1 + pnpm/crates/local-spec/Cargo.toml | 20 ++ pnpm/crates/local-spec/src/lib.rs | 110 ++++++++ pnpm/crates/local-spec/src/tests.rs | 71 +++++ pnpm/crates/lockfile/src/catalog_snapshots.rs | 3 +- pnpm/crates/napi/src/pack.rs | 1 + pnpm/crates/pack/src/lib.rs | 1 + pnpm/crates/pack/src/options.rs | 3 + pnpm/crates/pack/src/tests.rs | 4 + pnpm/crates/package-manager/Cargo.toml | 1 + .../package-manager/src/catalog_mode.rs | 19 +- .../src/dependencies_graph_to_lockfile.rs | 6 +- .../importers.rs | 35 ++- .../src/fast_update_catalog_versions.rs | 5 +- .../install_with_fresh_lockfile/resolve.rs | 1 + .../src/optimistic_repeat_install.rs | 4 +- .../local_file_deps.rs | 42 ++- pnpm/crates/package-manager/src/overrides.rs | 15 +- .../src/overrides/local_targets.rs | 64 ----- .../src/resolve_dependency_tree.rs | 2 +- .../src/resolve_dependency_tree/catalogs.rs | 24 +- .../src/resolve_dependency_tree/importer.rs | 13 +- .../src/resolve_dependency_tree/manifest.rs | 12 +- .../src/resolve_dependency_tree/tree_ctx.rs | 11 +- .../src/resolve_dependency_tree/walk.rs | 5 +- .../walk/child_seeds.rs | 32 ++- .../walk/warm_children.rs | 8 +- .../src/resolve_importer.rs | 7 +- .../src/resolve_importer/tests.rs | 1 + .../src/resolve_workspace/tests.rs | 1 + .../src/resolve_workspace/time_based.rs | 1 + .../src/tests/importer_wanted_specs.rs | 8 + .../src/workspace_resolution.rs | 1 + 54 files changed, 975 insertions(+), 235 deletions(-) create mode 100644 .changeset/catalog-local-protocols.md create mode 100644 pnpm/crates/cli/tests/suite/catalog_local_deps.rs create mode 100644 pnpm/crates/local-spec/Cargo.toml create mode 100644 pnpm/crates/local-spec/src/lib.rs create mode 100644 pnpm/crates/local-spec/src/tests.rs delete mode 100644 pnpm/crates/package-manager/src/overrides/local_targets.rs diff --git a/.changeset/catalog-local-protocols.md b/.changeset/catalog-local-protocols.md new file mode 100644 index 0000000000..099463a213 --- /dev/null +++ b/.changeset/catalog-local-protocols.md @@ -0,0 +1,5 @@ +--- +"pacquet": minor +--- + +Catalog entries can now use the `file:` and `link:` protocols. A relative path in an entry is measured from the directory holding `pnpm-workspace.yaml`, not from the project that references it [#8642](https://github.com/pnpm/pnpm/issues/8642). diff --git a/Cargo.lock b/Cargo.lock index 672f84ee04..521482580b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4594,6 +4594,7 @@ dependencies = [ "miette 7.6.0", "pnpm-catalogs-protocol-parser", "pnpm-catalogs-types", + "pnpm-local-spec", ] [[package]] @@ -5356,6 +5357,14 @@ dependencies = [ "which 8.0.4", ] +[[package]] +name = "pnpm-local-spec" +version = "0.0.1" +dependencies = [ + "pnpm-fs", + "pretty_assertions", +] + [[package]] name = "pnpm-lockfile" version = "0.0.1" @@ -5682,6 +5691,7 @@ dependencies = [ "pnpm-git-fetcher", "pnpm-graph-hasher", "pnpm-hooks", + "pnpm-local-spec", "pnpm-lockfile", "pnpm-lockfile-preferred-versions", "pnpm-lockfile-verification", diff --git a/Cargo.toml b/Cargo.toml index e8fdd60f5a..7496cdf810 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -48,6 +48,7 @@ pnpm-package-name = { path = "pnpm/crates/package-name" } pnpm-package-manifest = { path = "pnpm/crates/package-manifest" } pnpm-package-manager = { path = "pnpm/crates/package-manager" } pnpm-package-is-installable = { path = "pnpm/crates/package-is-installable" } +pnpm-local-spec = { path = "pnpm/crates/local-spec" } pnpm-lockfile = { path = "pnpm/crates/lockfile" } pnpm-lockfile-import = { path = "pnpm/crates/lockfile-import" } pnpm-lockfile-preferred-versions = { path = "pnpm/crates/lockfile-preferred-versions" } diff --git a/pnpm/crates/catalogs-resolver/Cargo.toml b/pnpm/crates/catalogs-resolver/Cargo.toml index 6d84eaf9f1..f6768e7018 100644 --- a/pnpm/crates/catalogs-resolver/Cargo.toml +++ b/pnpm/crates/catalogs-resolver/Cargo.toml @@ -13,6 +13,7 @@ repository.workspace = true [dependencies] pnpm-catalogs-protocol-parser = { workspace = true } pnpm-catalogs-types = { workspace = true } +pnpm-local-spec = { workspace = true } derive_more = { workspace = true } miette = { workspace = true } diff --git a/pnpm/crates/catalogs-resolver/src/lib.rs b/pnpm/crates/catalogs-resolver/src/lib.rs index cda529ca8c..5851fdd972 100644 --- a/pnpm/crates/catalogs-resolver/src/lib.rs +++ b/pnpm/crates/catalogs-resolver/src/lib.rs @@ -5,10 +5,13 @@ //! resolved [`CatalogResolutionFound::resolution`] feeds back in as a //! plain bare specifier. +use std::path::Path; + use derive_more::{Display, Error}; use miette::Diagnostic; use pnpm_catalogs_protocol_parser::parse_catalog_protocol; use pnpm_catalogs_types::Catalogs; +use pnpm_local_spec::LocalSpec; /// Subset of `pnpm-resolving-resolver-base`'s [`WantedDependency`] /// that catalog resolution needs. Modeled as its own type so this @@ -20,12 +23,31 @@ pub struct WantedDependency { pub bare_specifier: String, } +/// Which directory a `file:` / `link:` catalog entry's relative path is +/// measured from once resolved. +/// +/// A catalog is written in `pnpm-workspace.yaml`, so its relative paths +/// start at the workspace directory, while every consumer reads a +/// specifier relative to itself. Each call site states which of the two +/// it needs. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum CatalogAnchor<'a> { + /// Re-anchor the entry from `workspace_dir` to `consumer_dir`. A + /// `None` consumer yields the absolute path, for a consumer that + /// installs somewhere unrelated to the workspace. + Reanchor { workspace_dir: &'a Path, consumer_dir: Option<&'a Path> }, + /// Return the entry exactly as it is written. For call sites that + /// compare or display an entry rather than install from it, and for + /// those that anchor the resolved specifier themselves. + AsWritten, +} + /// Outcome of [`resolve_from_catalog`]. #[derive(Debug, Clone, PartialEq, Eq)] pub enum CatalogResolutionResult { /// The catalog protocol resolved to a usable specifier. Found(CatalogResolutionFound), - /// The catalog entry was missing or used a forbidden protocol. + /// The catalog entry was missing or referenced a catalog itself. Misconfiguration(CatalogResolutionMisconfiguration), /// The wanted dependency does not use the catalog protocol. Unused, @@ -54,7 +76,7 @@ pub struct CatalogResolutionMisconfiguration { pub error: CatalogResolutionError, } -/// The three ways a `catalog:` lookup can fail. Each variant carries the +/// The ways a `catalog:` lookup can fail. Each variant carries the /// `pnpm` error code reported for that failure. #[derive(Debug, Display, Error, Diagnostic, Clone, PartialEq, Eq)] #[non_exhaustive] @@ -68,12 +90,6 @@ pub enum CatalogResolutionError { )] #[diagnostic(code(ERR_PNPM_CATALOG_ENTRY_INVALID_RECURSIVE_DEFINITION))] EntryInvalidRecursiveDefinition { alias: String, catalog_name: String }, - - #[display( - "The entry for '{alias}' in catalog '{catalog_name}' declares a dependency using the '{protocol}' protocol. This is not yet supported, but may be in a future version of pnpm." - )] - #[diagnostic(code(ERR_PNPM_CATALOG_ENTRY_INVALID_SPEC))] - EntryInvalidSpec { alias: String, catalog_name: String, protocol: String }, } /// Resolve a wanted dependency through the catalogs map. @@ -81,6 +97,7 @@ pub enum CatalogResolutionError { pub fn resolve_from_catalog( catalogs: &Catalogs, wanted_dependency: &WantedDependency, + anchor: CatalogAnchor<'_>, ) -> CatalogResolutionResult { let Some(catalog_name) = parse_catalog_protocol(&wanted_dependency.bare_specifier) else { return CatalogResolutionResult::Unused; @@ -103,29 +120,25 @@ pub fn resolve_from_catalog( return recursive_catalog_error(catalog_name, &wanted_dependency.alias); } - let protocol_of_lookup = catalog_lookup - .split(':') - .next() - .unwrap_or(""); - if matches!(protocol_of_lookup, "link" | "file") { - return CatalogResolutionResult::Misconfiguration(CatalogResolutionMisconfiguration { - catalog_name: catalog_name.to_string(), - error: CatalogResolutionError::EntryInvalidSpec { - alias: wanted_dependency.alias.clone(), - catalog_name: catalog_name.to_string(), - protocol: protocol_of_lookup.to_string(), - }, - }); - } - CatalogResolutionResult::Found(CatalogResolutionFound { resolution: CatalogResolution { catalog_name: catalog_name.to_string(), - specifier: catalog_lookup.clone(), + specifier: anchored_specifier(catalog_lookup, anchor), }, }) } +/// Move a `file:` / `link:` entry from the workspace directory to the +/// directory that consumes it. Every other specifier is independent of +/// where it was written, so it passes through. +fn anchored_specifier(catalog_lookup: &str, anchor: CatalogAnchor<'_>) -> String { + let CatalogAnchor::Reanchor { workspace_dir, consumer_dir } = anchor else { + return catalog_lookup.to_string(); + }; + LocalSpec::parse(catalog_lookup, workspace_dir) + .map_or_else(|| catalog_lookup.to_string(), |spec| spec.render(consumer_dir)) +} + fn recursive_catalog_error(catalog_name: &str, alias: &str) -> CatalogResolutionResult { CatalogResolutionResult::Misconfiguration(CatalogResolutionMisconfiguration { catalog_name: catalog_name.to_string(), diff --git a/pnpm/crates/catalogs-resolver/src/tests.rs b/pnpm/crates/catalogs-resolver/src/tests.rs index f057955ab9..6e07c9db42 100644 --- a/pnpm/crates/catalogs-resolver/src/tests.rs +++ b/pnpm/crates/catalogs-resolver/src/tests.rs @@ -1,6 +1,8 @@ +use std::path::Path; + use super::{ - CatalogResolution, CatalogResolutionError, CatalogResolutionFound, CatalogResolutionResult, - WantedDependency, resolve_from_catalog, + CatalogAnchor, CatalogResolution, CatalogResolutionError, CatalogResolutionFound, + CatalogResolutionResult, WantedDependency, resolve_from_catalog, }; use pnpm_catalogs_types::{Catalog, Catalogs}; @@ -25,7 +27,7 @@ fn wanted(alias: &str, bare_specifier: &str) -> WantedDependency { fn default_catalog_resolves_using_implicit_name() { let catalogs = catalogs_from(&[("default", &[("foo", "1.0.0")])]); assert_eq!( - resolve_from_catalog(&catalogs, &wanted("foo", "catalog:")), + resolve_from_catalog(&catalogs, &wanted("foo", "catalog:"), CatalogAnchor::AsWritten), CatalogResolutionResult::Found(CatalogResolutionFound { resolution: CatalogResolution { catalog_name: "default".to_string(), @@ -39,7 +41,11 @@ fn default_catalog_resolves_using_implicit_name() { fn default_catalog_resolves_using_explicit_name() { let catalogs = catalogs_from(&[("default", &[("foo", "1.0.0")])]); assert_eq!( - resolve_from_catalog(&catalogs, &wanted("foo", "catalog:default")), + resolve_from_catalog( + &catalogs, + &wanted("foo", "catalog:default"), + CatalogAnchor::AsWritten + ), CatalogResolutionResult::Found(CatalogResolutionFound { resolution: CatalogResolution { catalog_name: "default".to_string(), @@ -53,7 +59,7 @@ fn default_catalog_resolves_using_explicit_name() { fn resolves_named_catalog() { let catalogs = catalogs_from(&[("foo", &[("bar", "1.0.0")])]); assert_eq!( - resolve_from_catalog(&catalogs, &wanted("bar", "catalog:foo")), + resolve_from_catalog(&catalogs, &wanted("bar", "catalog:foo"), CatalogAnchor::AsWritten), CatalogResolutionResult::Found(CatalogResolutionFound { resolution: CatalogResolution { catalog_name: "foo".to_string(), @@ -67,7 +73,7 @@ fn resolves_named_catalog() { fn returns_unused_for_specifier_not_using_catalog_protocol() { let catalogs = catalogs_from(&[("foo", &[("bar", "1.0.0")])]); assert_eq!( - resolve_from_catalog(&catalogs, &wanted("bar", "^2.0.0")), + resolve_from_catalog(&catalogs, &wanted("bar", "^2.0.0"), CatalogAnchor::AsWritten), CatalogResolutionResult::Unused, ); } @@ -80,7 +86,8 @@ fn returns_error_for_missing_unresolved_catalog() { ("bar", "catalog:baz", "baz"), ("foo", "catalog:foo", "foo"), ] { - let result = resolve_from_catalog(&catalogs, &wanted(alias, bare)); + let result = + resolve_from_catalog(&catalogs, &wanted(alias, bare), CatalogAnchor::AsWritten); let CatalogResolutionResult::Misconfiguration(misconfig) = &result else { panic!("expected misconfiguration for ({alias}, {bare}), got {result:?}"); }; @@ -102,7 +109,8 @@ fn returns_error_for_missing_unresolved_catalog() { #[test] fn returns_error_for_recursive_catalog() { let catalogs = catalogs_from(&[("foo", &[("bar", "catalog:foo")])]); - let result = resolve_from_catalog(&catalogs, &wanted("bar", "catalog:foo")); + let result = + resolve_from_catalog(&catalogs, &wanted("bar", "catalog:foo"), CatalogAnchor::AsWritten); let CatalogResolutionResult::Misconfiguration(misconfig) = &result else { panic!("expected misconfiguration, got {result:?}"); }; @@ -124,7 +132,7 @@ fn returns_error_for_recursive_catalog() { fn resolves_workspace_protocol_from_catalog() { let catalogs = catalogs_from(&[("foo", &[("bar", "workspace:*")])]); assert_eq!( - resolve_from_catalog(&catalogs, &wanted("bar", "catalog:foo")), + resolve_from_catalog(&catalogs, &wanted("bar", "catalog:foo"), CatalogAnchor::AsWritten), CatalogResolutionResult::Found(CatalogResolutionFound { resolution: CatalogResolution { catalog_name: "foo".to_string(), @@ -134,46 +142,58 @@ fn resolves_workspace_protocol_from_catalog() { ); } +#[cfg(windows)] +const WORKSPACE_DIR: &str = r"C:\workspace"; +#[cfg(not(windows))] +const WORKSPACE_DIR: &str = "/workspace"; + +fn reanchored(entry: &str, consumer: Option<&str>) -> String { + let catalogs = catalogs_from(&[("foo", &[("bar", entry)])]); + let workspace_dir = Path::new(WORKSPACE_DIR); + let consumer_dir = consumer.map(|dir| workspace_dir.join(dir)); + let anchor = CatalogAnchor::Reanchor { workspace_dir, consumer_dir: consumer_dir.as_deref() }; + match resolve_from_catalog(&catalogs, &wanted("bar", "catalog:foo"), anchor) { + CatalogResolutionResult::Found(found) => found.resolution.specifier, + other => panic!("expected the entry to resolve, got {other:?}"), + } +} + #[test] -fn returns_error_for_file_protocol_in_catalog() { - let catalogs = catalogs_from(&[("foo", &[("bar", "file:./bar.tgz")])]); - let result = resolve_from_catalog(&catalogs, &wanted("bar", "catalog:foo")); - let CatalogResolutionResult::Misconfiguration(misconfig) = &result else { - panic!("expected misconfiguration, got {result:?}"); - }; +fn reanchors_a_file_entry_on_the_consuming_project() { assert_eq!( - misconfig.error, - CatalogResolutionError::EntryInvalidSpec { - alias: "bar".to_string(), - catalog_name: "foo".to_string(), - protocol: "file".to_string(), - }, - ); - assert_eq!( - misconfig.error.to_string(), - "The entry for 'bar' in catalog 'foo' declares a dependency using the 'file' protocol. \ - This is not yet supported, but may be in a future version of pnpm.", + reanchored("file:./tarballs/bar-1.0.0.tgz", Some("packages/foo")), + "file:../../tarballs/bar-1.0.0.tgz", ); } #[test] -fn returns_error_for_link_protocol_in_catalog() { - let catalogs = catalogs_from(&[("foo", &[("bar", "link:./bar")])]); - let result = resolve_from_catalog(&catalogs, &wanted("bar", "catalog:foo")); - let CatalogResolutionResult::Misconfiguration(misconfig) = &result else { - panic!("expected misconfiguration, got {result:?}"); - }; +fn reanchors_a_link_entry_on_the_consuming_project() { + assert_eq!(reanchored("link:libs/bar", Some("packages/foo")), "link:../../libs/bar"); +} + +#[test] +fn renders_a_local_entry_absolute_for_a_consumer_outside_the_workspace() { + let expected = format!("file:{}/tarballs/bar-1.0.0.tgz", WORKSPACE_DIR.replace('\\', "/")); + assert_eq!(reanchored("file:./tarballs/bar-1.0.0.tgz", None), expected); +} + +#[test] +fn leaves_a_local_entry_alone_when_the_anchor_is_as_written() { + let catalogs = catalogs_from(&[("foo", &[("bar", "file:./tarballs/bar-1.0.0.tgz")])]); assert_eq!( - misconfig.error, - CatalogResolutionError::EntryInvalidSpec { - alias: "bar".to_string(), - catalog_name: "foo".to_string(), - protocol: "link".to_string(), - }, - ); - assert_eq!( - misconfig.error.to_string(), - "The entry for 'bar' in catalog 'foo' declares a dependency using the 'link' protocol. \ - This is not yet supported, but may be in a future version of pnpm.", + resolve_from_catalog(&catalogs, &wanted("bar", "catalog:foo"), CatalogAnchor::AsWritten), + CatalogResolutionResult::Found(CatalogResolutionFound { + resolution: CatalogResolution { + catalog_name: "foo".to_string(), + specifier: "file:./tarballs/bar-1.0.0.tgz".to_string(), + }, + }), ); } + +#[test] +fn leaves_a_registry_entry_alone_while_reanchoring() { + assert_eq!(reanchored("^1.2.3", Some("packages/foo")), "^1.2.3"); + assert_eq!(reanchored("workspace:*", Some("packages/foo")), "workspace:*"); + assert_eq!(reanchored("npm:other@^1", Some("packages/foo")), "npm:other@^1"); +} diff --git a/pnpm/crates/cli/src/cli_args/dlx.rs b/pnpm/crates/cli/src/cli_args/dlx.rs index 54fe9a472c..77941bc83b 100644 --- a/pnpm/crates/cli/src/cli_args/dlx.rs +++ b/pnpm/crates/cli/src/cli_args/dlx.rs @@ -14,7 +14,8 @@ use derive_more::{Display, Error}; use miette::{Context, Diagnostic, IntoDiagnostic}; use pnpm_catalogs_protocol_parser::parse_catalog_protocol; use pnpm_catalogs_resolver::{ - CatalogResolutionResult, WantedDependency as CatalogWantedDependency, resolve_from_catalog, + CatalogAnchor, CatalogResolutionResult, WantedDependency as CatalogWantedDependency, + resolve_from_catalog, }; use pnpm_cmd_shim::{Host as CmdShimHost, get_bins_from_package_manifest}; use pnpm_config::Config; diff --git a/pnpm/crates/cli/src/cli_args/dlx/cache.rs b/pnpm/crates/cli/src/cli_args/dlx/cache.rs index 5ac99074fe..1c8d7e66c9 100644 --- a/pnpm/crates/cli/src/cli_args/dlx/cache.rs +++ b/pnpm/crates/cli/src/cli_args/dlx/cache.rs @@ -1,9 +1,10 @@ use super::{ - BTreeMap, CatalogResolutionResult, CatalogWantedDependency, Config, Context, DependencyGroup, - DlxError, Duration, IntoDiagnostic, Path, PathBuf, RangeSpecStyle, Reporter, State, - SupportedArchitectures, SupportedArchitecturesArgs, SystemTime, UNIX_EPOCH, Value, add_package, - configured_catalogs, create_short_hash, force_symlink_dir, fs, json, parse_catalog_protocol, - parse_manifest, parse_overrides_iter, parse_wanted_dependency, resolve_from_catalog, + BTreeMap, CatalogAnchor, CatalogResolutionResult, CatalogWantedDependency, Config, Context, + DependencyGroup, DlxError, Duration, IntoDiagnostic, Path, PathBuf, RangeSpecStyle, Reporter, + State, SupportedArchitectures, SupportedArchitecturesArgs, SystemTime, UNIX_EPOCH, Value, + add_package, configured_catalogs, create_short_hash, force_symlink_dir, fs, json, + parse_catalog_protocol, parse_manifest, parse_overrides_iter, parse_wanted_dependency, + resolve_from_catalog, }; /// Install the packages into a fresh prepare directory and point the @@ -106,6 +107,10 @@ pub(super) fn dlx_command_cache_dir(config: &Config, cache_key: &str) -> miette: /// untouched, and the catalogs are only read when at least one spec needs /// them. A misconfigured entry is reported as the pnpm error the caller /// would get from `pnpm add`. +/// +/// A `file:` / `link:` entry becomes an absolute path: dlx installs into +/// a cache directory outside the workspace, so nothing there can read a +/// path measured from `pnpm-workspace.yaml`. pub(super) fn resolve_catalog_specs( pkgs: &[String], config: &Config, @@ -118,6 +123,10 @@ pub(super) fn resolve_catalog_specs( return Ok(pkgs.to_vec()); } let catalogs = configured_catalogs(config)?; + let anchor = match config.workspace_dir.as_deref() { + Some(workspace_dir) => CatalogAnchor::Reanchor { workspace_dir, consumer_dir: None }, + None => CatalogAnchor::AsWritten, + }; pkgs.iter() .map(|pkg| { let parsed = parse_wanted_dependency(pkg); @@ -125,7 +134,7 @@ pub(super) fn resolve_catalog_specs( return Ok(pkg.clone()); }; let wanted = CatalogWantedDependency { alias: alias.clone(), bare_specifier }; - match resolve_from_catalog(&catalogs, &wanted) { + match resolve_from_catalog(&catalogs, &wanted, anchor) { CatalogResolutionResult::Found(found) => { Ok(format!("{alias}@{}", found.resolution.specifier)) } diff --git a/pnpm/crates/cli/src/cli_args/outdated.rs b/pnpm/crates/cli/src/cli_args/outdated.rs index fd36275a1d..f94fab7052 100644 --- a/pnpm/crates/cli/src/cli_args/outdated.rs +++ b/pnpm/crates/cli/src/cli_args/outdated.rs @@ -39,7 +39,8 @@ use node_semver::Version; use owo_colors::Stream; use pnpm_catalogs_protocol_parser::parse_catalog_protocol; use pnpm_catalogs_resolver::{ - CatalogResolutionResult, WantedDependency as CatalogWantedDependency, resolve_from_catalog, + CatalogAnchor, CatalogResolutionResult, WantedDependency as CatalogWantedDependency, + resolve_from_catalog, }; use pnpm_catalogs_types::Catalogs; use pnpm_config::Config; diff --git a/pnpm/crates/cli/src/cli_args/outdated/query.rs b/pnpm/crates/cli/src/cli_args/outdated/query.rs index a1214aa7f4..4594916874 100644 --- a/pnpm/crates/cli/src/cli_args/outdated/query.rs +++ b/pnpm/crates/cli/src/cli_args/outdated/query.rs @@ -1,9 +1,9 @@ use super::{ - Arc, CatalogResolutionResult, CatalogWantedDependency, Catalogs, Config, Cow, DependencyGroup, - HashMap, InMemoryPackageMetaCache, LatestQuery, Lockfile, Matcher, NpmResolver, - PackageManifest, PickPolicy, ResolveOptions, ResolverWantedDependency, ThrottledClient, - Version, configured_catalogs, create_configured_npm_resolver, create_matcher, github_actions, - parse_catalog_protocol, resolve_from_catalog, + Arc, CatalogAnchor, CatalogResolutionResult, CatalogWantedDependency, Catalogs, Config, Cow, + DependencyGroup, HashMap, InMemoryPackageMetaCache, LatestQuery, Lockfile, Matcher, + NpmResolver, PackageManifest, PickPolicy, ResolveOptions, ResolverWantedDependency, + ThrottledClient, Version, configured_catalogs, create_configured_npm_resolver, create_matcher, + github_actions, parse_catalog_protocol, resolve_from_catalog, }; use pnpm_resolving_resolver_base::Resolver; @@ -285,6 +285,10 @@ async fn outdated_dependency( /// so both the queried package name and the `--compatible` range come /// from the catalog entry — which may itself be an npm alias /// (`npm:@types/table@^6`). Any other specifier passes through. +/// +/// The entry is reported as the catalog writes it: `pnpm outdated` +/// displays and version-matches the specifier rather than installing +/// from it, and a registry query has no use for a re-anchored path. fn dereference_catalog<'a>( catalogs: &Catalogs, alias: &str, @@ -300,7 +304,7 @@ fn dereference_catalog<'a>( alias: alias.to_string(), bare_specifier: bare_specifier.to_string(), }; - match resolve_from_catalog(catalogs, &wanted) { + match resolve_from_catalog(catalogs, &wanted, CatalogAnchor::AsWritten) { CatalogResolutionResult::Found(found) => Ok(Cow::Owned(found.resolution.specifier)), CatalogResolutionResult::Misconfiguration(misconfiguration) => { Err(miette::Report::new(misconfiguration.error)) diff --git a/pnpm/crates/cli/src/cli_args/pack.rs b/pnpm/crates/cli/src/cli_args/pack.rs index 860b816f8b..520bcf5b18 100644 --- a/pnpm/crates/cli/src/cli_args/pack.rs +++ b/pnpm/crates/cli/src/cli_args/pack.rs @@ -218,6 +218,7 @@ impl PackArgs { }, manifest: pnpm_pack::PackManifestOptions { catalogs, + catalogs_dir: config.workspace_dir.clone(), embed_readme: config.embed_readme, node_linker: config.node_linker, skip_obfuscation: resolve_bool_override( diff --git a/pnpm/crates/cli/src/cli_args/publish.rs b/pnpm/crates/cli/src/cli_args/publish.rs index 39b04a0b8a..863379cfec 100644 --- a/pnpm/crates/cli/src/cli_args/publish.rs +++ b/pnpm/crates/cli/src/cli_args/publish.rs @@ -182,9 +182,16 @@ impl PublishArgs { if let Some(package) = self.package.as_deref().filter(|path| is_tarball_path(path)) { self.publish_tarball::(package, &opts, &network).await? } else { - let project_dir = self.package.as_deref().map_or(dir, Path::new); + // Resolved against the command directory so every path the + // pack derives from it — the re-anchored `file:` / `link:` + // catalog entries among them — can be related to the + // absolute workspace directory. `join` keeps an absolute + // argument as it is. + let project_dir = self.package + .as_deref() + .map_or_else(|| dir.to_path_buf(), |path| dir.join(path)); self.publish_directory::( - project_dir, + &project_dir, config, &opts, &network, @@ -337,6 +344,7 @@ impl PublishArgs { }, manifest: pnpm_pack::PackManifestOptions { catalogs: crate::cli_args::catalogs::configured_catalogs(config)?, + catalogs_dir: config.workspace_dir.clone(), embed_readme: resolve_bool_override( self.flags.manifest.embed_readme, self.flags.manifest.no_embed_readme, diff --git a/pnpm/crates/cli/tests/suite/add/aliasless_selectors.rs b/pnpm/crates/cli/tests/suite/add/aliasless_selectors.rs index fb7b96583a..efd9755c06 100644 --- a/pnpm/crates/cli/tests/suite/add/aliasless_selectors.rs +++ b/pnpm/crates/cli/tests/suite/add/aliasless_selectors.rs @@ -140,11 +140,10 @@ fn a_remote_tarball_url_is_saved_verbatim() { drop((root, mock_instance)); } -/// A catalog entry is read by every project referencing it, so it -/// cannot hold a path that resolves against the project declaring it. -/// `catalogMode` has to leave such a specifier direct — cataloging it -/// writes an entry the next install refuses with -/// `ERR_PNPM_CATALOG_ENTRY_INVALID_SPEC`. +/// A catalog measures a relative path from `pnpm-workspace.yaml`'s own +/// directory, not from the project that declares the dependency, so +/// `catalogMode` has to leave a local specifier direct — cataloging it +/// would point it somewhere else than where `pnpm add` was run. #[test] fn a_local_directory_is_not_auto_cataloged() { let CommandTempCwd { diff --git a/pnpm/crates/cli/tests/suite/catalog_local_deps.rs b/pnpm/crates/cli/tests/suite/catalog_local_deps.rs new file mode 100644 index 0000000000..656fa9bae2 --- /dev/null +++ b/pnpm/crates/cli/tests/suite/catalog_local_deps.rs @@ -0,0 +1,254 @@ +//! End-to-end coverage for `file:` and `link:` catalog entries. +//! +//! A catalog lives in `pnpm-workspace.yaml`, so its relative paths are +//! written from the workspace directory while the projects that +//! dereference them sit anywhere below it. +//! +//! Covers . + +use assert_cmd::prelude::*; +use command_extra::CommandExtra; +use pnpm_lockfile::Lockfile; +use pnpm_testing_utils::{bin::CommandTempCwd, fixtures::tarball_with_manifest}; +use pretty_assertions::assert_eq; +use std::{fs, path::Path, process::Command}; + +const TARBALL: &str = "tarballs/pkg-from-tarball-1.0.0.tgz"; + +fn workspace_with_local_catalog(workspace: &Path, projects: &[&str]) { + let yaml = fs::read_to_string(workspace.join("pnpm-workspace.yaml")) + .expect("read pnpm-workspace.yaml"); + let yaml = format!( + "{yaml}packages:\n - projects/*\n - projects/*/*\ncatalog:\n \ + pkg-from-tarball: file:./{TARBALL}\n local-lib: link:./libs/local-lib\n", + ); + fs::write(workspace.join("pnpm-workspace.yaml"), yaml).expect("write pnpm-workspace.yaml"); + + fs::create_dir_all(workspace.join("tarballs")).expect("create the tarball directory"); + fs::write( + workspace.join(TARBALL), + tarball_with_manifest( + &serde_json::json!({ "name": "pkg-from-tarball", "version": "1.0.0" }), + ), + ) + .expect("write the tarball"); + + fs::create_dir_all(workspace.join("libs/local-lib")).expect("create the local library"); + write_manifest( + &workspace.join("libs/local-lib"), + &serde_json::json!({ "name": "local-lib", "version": "2.0.0" }), + ); + + write_manifest( + workspace, + &serde_json::json!({ "name": "root", "version": "1.0.0", "private": true }), + ); + for project in projects { + let dir = workspace.join(project); + fs::create_dir_all(&dir).expect("create the project directory"); + write_manifest( + &dir, + &serde_json::json!({ + "name": Path::new(project).file_name().unwrap().to_str().unwrap(), + "version": "1.0.0", + "dependencies": { + "pkg-from-tarball": "catalog:", + "local-lib": "catalog:", + }, + }), + ); + } +} + +fn write_manifest(dir: &Path, manifest: &serde_json::Value) { + fs::write(dir.join("package.json"), manifest.to_string()).expect("write package.json"); +} + +fn installed_version(project_dir: &Path, dep: &str) -> String { + let manifest = project_dir + .join("node_modules") + .join(dep) + .join("package.json"); + let manifest: serde_json::Value = serde_json::from_str( + &fs::read_to_string(&manifest) + .unwrap_or_else(|error| panic!("read {}: {error}", manifest.display())), + ) + .expect("parse the installed manifest"); + manifest["version"] + .as_str() + .expect("the installed manifest names a version") + .to_string() +} + +#[test] +fn local_catalog_entries_resolve_from_the_workspace_directory() { + let CommandTempCwd { + pacquet, + root, + workspace, + npmrc_info, + .. + } = CommandTempCwd::init().add_mocked_registry(); + let projects = ["projects/foo", "projects/nested/bar"]; + workspace_with_local_catalog(&workspace, &projects); + + pacquet + .with_arg("install") + .assert() + .success(); + + for project in projects { + let project_dir = workspace.join(project); + assert_eq!( + installed_version(&project_dir, "pkg-from-tarball"), + "1.0.0", + "{project} must install the tarball the catalog names", + ); + assert_eq!( + installed_version(&project_dir, "local-lib"), + "2.0.0", + "{project} must link the directory the catalog names", + ); + assert_eq!( + fs::canonicalize(project_dir.join("node_modules/local-lib")) + .expect("resolve the linked directory"), + fs::canonicalize(workspace.join("libs/local-lib")).expect("resolve the local library"), + "{project}'s link must point at the workspace-relative directory", + ); + } + + drop((root, npmrc_info)); +} + +#[test] +fn retargeting_a_local_catalog_entry_reinstalls_from_the_new_path() { + let CommandTempCwd { + pacquet, + root, + workspace, + npmrc_info, + .. + } = CommandTempCwd::init().add_mocked_registry(); + workspace_with_local_catalog(&workspace, &["projects/foo"]); + + pacquet + .with_arg("install") + .assert() + .success(); + assert_eq!(installed_version(&workspace.join("projects/foo"), "pkg-from-tarball"), "1.0.0"); + + fs::write( + workspace.join("tarballs/pkg-from-tarball-2.0.0.tgz"), + tarball_with_manifest( + &serde_json::json!({ "name": "pkg-from-tarball", "version": "2.0.0" }), + ), + ) + .expect("write the second tarball"); + let yaml = fs::read_to_string(workspace.join("pnpm-workspace.yaml")) + .expect("read pnpm-workspace.yaml") + .replace("pkg-from-tarball-1.0.0.tgz", "pkg-from-tarball-2.0.0.tgz"); + fs::write(workspace.join("pnpm-workspace.yaml"), yaml).expect("write pnpm-workspace.yaml"); + + Command::cargo_bin("pnpm") + .expect("find the pnpm binary") + .with_current_dir(&workspace) + .with_arg("install") + .assert() + .success(); + assert_eq!(installed_version(&workspace.join("projects/foo"), "pkg-from-tarball"), "2.0.0"); + + drop((root, npmrc_info)); +} + +#[test] +fn the_lockfile_records_local_catalog_entries_as_the_catalog_writes_them() { + let CommandTempCwd { + pacquet, + root, + workspace, + npmrc_info, + .. + } = CommandTempCwd::init().add_mocked_registry(); + workspace_with_local_catalog(&workspace, &["projects/foo"]); + + pacquet + .with_arg("install") + .assert() + .success(); + + let lockfile: Lockfile = serde_saphyr::from_str( + &fs::read_to_string(workspace.join("pnpm-lock.yaml")).expect("read pnpm-lock.yaml"), + ) + .expect("parse pnpm-lock.yaml"); + let catalog = lockfile.catalogs + .as_ref() + .and_then(|catalogs| catalogs.get("default")) + .expect("the lockfile records the default catalog"); + // A local entry has no version of its own, so the recorded version + // repeats the specifier instead of naming one importer's path. + assert_eq!(catalog["pkg-from-tarball"].specifier, format!("file:./{TARBALL}")); + assert_eq!(catalog["pkg-from-tarball"].version, format!("file:./{TARBALL}")); + assert_eq!(catalog["local-lib"].specifier, "link:./libs/local-lib"); + assert_eq!(catalog["local-lib"].version, "link:./libs/local-lib"); + + // The recorded entries are complete enough to install from without + // re-resolving, which is what a `--frozen-lockfile` install proves. + fs::remove_dir_all(workspace.join("node_modules")).expect("remove node_modules"); + fs::remove_dir_all(workspace.join("projects/foo/node_modules")).expect("remove node_modules"); + Command::cargo_bin("pnpm") + .expect("find the pnpm binary") + .with_current_dir(&workspace) + .with_args(["install", "--frozen-lockfile"]) + .assert() + .success(); + assert_eq!(installed_version(&workspace.join("projects/foo"), "pkg-from-tarball"), "1.0.0"); + + drop((root, npmrc_info)); +} + +/// A catalog measures a relative path from `pnpm-workspace.yaml`, while +/// the packed manifest is read from the package's own directory, so +/// packing a nested project has to move the path between the two. +#[test] +fn packing_a_nested_project_reanchors_its_local_catalog_entries() { + let CommandTempCwd { root, workspace, npmrc_info, .. } = + CommandTempCwd::init().add_mocked_registry(); + let project = "projects/nested/bar"; + workspace_with_local_catalog(&workspace, &[project]); + let project_dir = workspace.join(project); + + Command::cargo_bin("pnpm") + .expect("find the pnpm binary") + .with_current_dir(&project_dir) + .with_arg("pack") + .assert() + .success(); + + let manifest = read_packed_manifest(&project_dir.join("bar-1.0.0.tgz")); + assert_eq!( + manifest["dependencies"], + serde_json::json!({ + "pkg-from-tarball": "file:../../../tarballs/pkg-from-tarball-1.0.0.tgz", + "local-lib": "link:../../../libs/local-lib", + }), + ); + + drop((root, npmrc_info)); +} + +fn read_packed_manifest(tarball: &Path) -> serde_json::Value { + use std::io::Read as _; + + let bytes = fs::read(tarball).expect("read tarball"); + let decoder = flate2::read::GzDecoder::new(bytes.as_slice()); + let mut archive = tar::Archive::new(decoder); + for entry in archive.entries().expect("iterate tarball entries") { + let mut entry = entry.expect("read tarball entry"); + if entry.path().expect("entry path") == Path::new("package/package.json") { + let mut contents = String::new(); + entry.read_to_string(&mut contents).expect("read manifest"); + return serde_json::from_str(&contents).expect("parse manifest"); + } + } + panic!("package/package.json not found in {}", tarball.display()); +} diff --git a/pnpm/crates/cli/tests/suite/main.rs b/pnpm/crates/cli/tests/suite/main.rs index 61710c063e..0b3bc92001 100644 --- a/pnpm/crates/cli/tests/suite/main.rs +++ b/pnpm/crates/cli/tests/suite/main.rs @@ -22,6 +22,7 @@ mod cargo_install; mod cat_file; mod cat_index; mod catalog; +mod catalog_local_deps; mod change; mod ci_frozen_lockfile; mod clean; diff --git a/pnpm/crates/cli/tests/suite/publish.rs b/pnpm/crates/cli/tests/suite/publish.rs index 5c474ed3bc..107beed167 100644 --- a/pnpm/crates/cli/tests/suite/publish.rs +++ b/pnpm/crates/cli/tests/suite/publish.rs @@ -519,3 +519,51 @@ fn ignore_scripts_skips_the_publish_lifecycle_scripts() { ); mock.assert(); } + +/// A positional package path is resolved against the command directory +/// before the pack reads it. Left relative, it cannot be related to the +/// absolute workspace directory, and a `file:` / `link:` catalog entry +/// re-anchored against it would fall back to this machine's absolute +/// path and ship inside the published manifest. +#[test] +fn publishing_a_nested_project_by_relative_path_keeps_catalog_entries_relative() { + let workspace = tempfile::tempdir().expect("workspace"); + let mut server = mockito::Server::new(); + let project_dir = workspace.path().join("projects/nested/bar"); + fs::create_dir_all(&project_dir).expect("create the project directory"); + fs::write(workspace.path().join(".npmrc"), format!("registry={}/\n", server.url())) + .expect("write .npmrc"); + fs::write( + workspace.path().join("pnpm-workspace.yaml"), + "packages:\n - projects/*/*\ncatalog:\n \ + pkg-from-tarball: file:./tarballs/pkg-from-tarball-1.0.0.tgz\n \ + local-lib: link:./libs/local-lib\n", + ) + .expect("write pnpm-workspace.yaml"); + fs::write( + project_dir.join("package.json"), + json!({ + "name": "test-publish-nested", + "version": "1.0.0", + "dependencies": { "pkg-from-tarball": "catalog:", "local-lib": "catalog:" }, + }) + .to_string(), + ) + .expect("write package.json"); + + let mock = server + .mock("PUT", "/test-publish-nested") + .match_body(Matcher::PartialJsonString( + r#"{"versions":{"1.0.0":{"dependencies":{ + "pkg-from-tarball":"file:../../../tarballs/pkg-from-tarball-1.0.0.tgz", + "local-lib":"link:../../../libs/local-lib"}}}}"# + .to_owned(), + )) + .with_status(200) + .with_body(r#"{"ok":true}"#) + .expect(1) + .create(); + + assert_success(&publish(workspace.path(), &["./projects/nested/bar"])); + mock.assert(); +} diff --git a/pnpm/crates/config-parse-overrides/src/lib.rs b/pnpm/crates/config-parse-overrides/src/lib.rs index c47587cbf7..421dd2c421 100644 --- a/pnpm/crates/config-parse-overrides/src/lib.rs +++ b/pnpm/crates/config-parse-overrides/src/lib.rs @@ -14,7 +14,9 @@ use derive_more::{Display, Error}; use miette::Diagnostic; use node_semver::Version; -use pnpm_catalogs_resolver::{CatalogResolutionResult, WantedDependency, resolve_from_catalog}; +use pnpm_catalogs_resolver::{ + CatalogAnchor, CatalogResolutionResult, WantedDependency, resolve_from_catalog, +}; use pnpm_catalogs_types::Catalogs; use pnpm_resolving_parse_wanted_dependency::parse_wanted_dependency; use std::collections::HashMap; @@ -247,9 +249,14 @@ fn parse_pkg_selector(selector: &str) -> Result Ok(found.resolution.specifier), CatalogResolutionResult::Unused => Ok(new_bare_specifier.to_string()), CatalogResolutionResult::Misconfiguration(misconfiguration) => { diff --git a/pnpm/crates/deps-inspection-peers/src/lib.rs b/pnpm/crates/deps-inspection-peers/src/lib.rs index d24f8f3467..a5d79879cf 100644 --- a/pnpm/crates/deps-inspection-peers/src/lib.rs +++ b/pnpm/crates/deps-inspection-peers/src/lib.rs @@ -29,7 +29,8 @@ use owo_colors::Stream; use serde::Serialize; use pnpm_catalogs_resolver::{ - CatalogResolutionError, CatalogResolutionResult, WantedDependency, resolve_from_catalog, + CatalogAnchor, CatalogResolutionError, CatalogResolutionResult, WantedDependency, + resolve_from_catalog, }; use pnpm_catalogs_types::Catalogs; use pnpm_config::PeerDependencyRules; diff --git a/pnpm/crates/deps-inspection-peers/src/linked.rs b/pnpm/crates/deps-inspection-peers/src/linked.rs index f72a428274..3c90865626 100644 --- a/pnpm/crates/deps-inspection-peers/src/linked.rs +++ b/pnpm/crates/deps-inspection-peers/src/linked.rs @@ -1,8 +1,8 @@ use super::{ - BadPeerIssue, CatalogResolutionError, CatalogResolutionResult, Catalogs, Lockfile, - LockfileResolution, MissingPeerIssue, PackageManifest, ParentPkg, Path, PathBuf, PeerIssues, - PkgName, ProjectSnapshot, ResolvedDependencySpec, WantedDependency, get_peer_version_range, - resolve_from_catalog, satisfies, + BadPeerIssue, CatalogAnchor, CatalogResolutionError, CatalogResolutionResult, Catalogs, + Lockfile, LockfileResolution, MissingPeerIssue, PackageManifest, ParentPkg, Path, PathBuf, + PeerIssues, PkgName, ProjectSnapshot, ResolvedDependencySpec, WantedDependency, + get_peer_version_range, resolve_from_catalog, satisfies, }; pub(super) struct CanonicalPathWithin { @@ -265,7 +265,9 @@ fn resolve_peer_range( let Some(catalogs) = catalogs else { return Ok(peer_range.to_string()) }; let wanted = WantedDependency { alias: peer_name.to_string(), bare_specifier: peer_range.to_string() }; - match resolve_from_catalog(catalogs, &wanted) { + // A peer range names a version range, never a path, so a `file:` / + // `link:` entry has nothing to re-anchor. + match resolve_from_catalog(catalogs, &wanted, CatalogAnchor::AsWritten) { CatalogResolutionResult::Found(found) => Ok(found.resolution.specifier), CatalogResolutionResult::Unused => Ok(peer_range.to_string()), CatalogResolutionResult::Misconfiguration(misconfiguration) => Err(misconfiguration.error), diff --git a/pnpm/crates/exportable-manifest/src/create.rs b/pnpm/crates/exportable-manifest/src/create.rs index bc3d4ff907..f1748bb7bb 100644 --- a/pnpm/crates/exportable-manifest/src/create.rs +++ b/pnpm/crates/exportable-manifest/src/create.rs @@ -37,7 +37,8 @@ use crate::{ use derive_more::{Display, Error}; use miette::Diagnostic; use pnpm_catalogs_resolver::{ - CatalogResolutionError, CatalogResolutionResult, WantedDependency, resolve_from_catalog, + CatalogAnchor, CatalogResolutionError, CatalogResolutionResult, WantedDependency, + resolve_from_catalog, }; use pnpm_catalogs_types::Catalogs; use pnpm_resolving_jsr_specifier_parser::{ParseJsrSpecifierError, parse_jsr_specifier}; @@ -83,6 +84,11 @@ const PUBLISH_CONFIG_WHITELIST: &[&str] = &[ pub struct CreateExportableManifestOptions<'a> { /// Parsed workspace catalogs, used to resolve `catalog:` specifiers. pub catalogs: &'a Catalogs, + /// Directory holding `pnpm-workspace.yaml`, which a `file:` / + /// `link:` catalog entry's relative path is measured from. `None` + /// leaves such an entry as the catalog writes it, for a caller with + /// no workspace — and so no catalogs — of its own. + pub workspace_dir: Option<&'a Path>, /// Where workspace dependencies are installed. Defaults to /// `/node_modules` when `None`. pub modules_dir: Option<&'a Path>, @@ -224,7 +230,7 @@ fn convert_dependency_for_publish( opts: &CreateExportableManifestOptions<'_>, kind: DependencyKind, ) -> Result { - let after_catalog = replace_catalog_protocol(dep_name, spec, opts.catalogs)?; + let after_catalog = replace_catalog_protocol(dep_name, spec, dir, opts)?; let after_workspace = match kind { DependencyKind::Regular => { replace_workspace_protocol(dep_name, &after_catalog, dir, opts.modules_dir) @@ -242,13 +248,23 @@ fn convert_dependency_for_publish( /// Dereference a `catalog:` specifier; pass any other specifier /// through unchanged. +/// +/// A `file:` / `link:` entry is re-anchored on `dir`, the directory of +/// the package being exported, so it means the same place a local +/// dependency written directly in that package's manifest would. Both +/// are read relative to the exported manifest. fn replace_catalog_protocol( alias: &str, spec: &str, - catalogs: &Catalogs, + dir: &Path, + opts: &CreateExportableManifestOptions<'_>, ) -> Result { let wanted = WantedDependency { alias: alias.to_string(), bare_specifier: spec.to_string() }; - match resolve_from_catalog(catalogs, &wanted) { + let anchor = match opts.workspace_dir { + Some(workspace_dir) => CatalogAnchor::Reanchor { workspace_dir, consumer_dir: Some(dir) }, + None => CatalogAnchor::AsWritten, + }; + match resolve_from_catalog(opts.catalogs, &wanted, anchor) { CatalogResolutionResult::Found(found) => Ok(found.resolution.specifier), CatalogResolutionResult::Unused => Ok(spec.to_string()), CatalogResolutionResult::Misconfiguration(misconfiguration) => { diff --git a/pnpm/crates/exportable-manifest/src/create/tests.rs b/pnpm/crates/exportable-manifest/src/create/tests.rs index bab5418763..b955c44a0b 100644 --- a/pnpm/crates/exportable-manifest/src/create/tests.rs +++ b/pnpm/crates/exportable-manifest/src/create/tests.rs @@ -17,6 +17,7 @@ fn build(dir: &Path, manifest: &Value, opts: &CreateExportableManifestOptions<'_ fn default_opts(catalogs: &Catalogs) -> CreateExportableManifestOptions<'_> { CreateExportableManifestOptions { catalogs, + workspace_dir: None, modules_dir: None, skip_manifest_obfuscation: false, embed_readme: false, @@ -63,6 +64,7 @@ fn skip_obfuscation_keeps_scripts_and_package_manager() { let catalogs = empty_catalogs(); let opts = CreateExportableManifestOptions { catalogs: &catalogs, + workspace_dir: None, modules_dir: None, skip_manifest_obfuscation: true, embed_readme: false, @@ -295,6 +297,7 @@ fn readme_is_embedded_when_requested() { let catalogs = empty_catalogs(); let opts = CreateExportableManifestOptions { catalogs: &catalogs, + workspace_dir: None, modules_dir: None, skip_manifest_obfuscation: false, embed_readme: true, @@ -325,6 +328,7 @@ fn readme_symlink_is_not_embedded() { let catalogs = empty_catalogs(); let opts = CreateExportableManifestOptions { catalogs: &catalogs, + workspace_dir: None, modules_dir: None, skip_manifest_obfuscation: false, embed_readme: true, @@ -345,3 +349,73 @@ fn missing_name_surfaces_transform_error() { .unwrap_err(); assert!(matches!(err, CreateExportableManifestError::Transform(_))); } + +/// A catalog measures a relative path from `pnpm-workspace.yaml`, while +/// the exported manifest is read from the package's own directory, so +/// the two have to name the same place. +#[test] +fn local_catalog_entry_is_reanchored_on_the_exported_package() { + let workspace = tempdir().unwrap(); + let package_dir = workspace.path().join("packages/foo"); + std::fs::create_dir_all(&package_dir).unwrap(); + let catalogs = Catalogs::from([( + "default".to_string(), + Catalog::from([ + ("from-tarball".to_string(), "file:./tarballs/from-tarball-1.0.0.tgz".to_string()), + ("local-lib".to_string(), "link:./libs/local-lib".to_string()), + ]), + )]); + let opts = CreateExportableManifestOptions { + catalogs: &catalogs, + workspace_dir: Some(workspace.path()), + modules_dir: None, + skip_manifest_obfuscation: false, + embed_readme: false, + }; + + let out = build( + &package_dir, + &json!({ + "name": "foo", + "version": "1.0.0", + "dependencies": { "from-tarball": "catalog:", "local-lib": "catalog:" }, + }), + &opts, + ); + + assert_eq!( + out["dependencies"], + json!({ + "from-tarball": "file:../../tarballs/from-tarball-1.0.0.tgz", + "local-lib": "link:../../libs/local-lib", + }), + ); +} + +/// Without a workspace directory there is no anchor to measure from, so +/// the entry is emitted as the catalog wrote it. +#[test] +fn local_catalog_entry_without_a_workspace_dir_is_emitted_as_written() { + let dir = tempdir().unwrap(); + let catalogs = Catalogs::from([( + "default".to_string(), + Catalog::from([( + "from-tarball".to_string(), + "file:./tarballs/from-tarball-1.0.0.tgz".to_string(), + )]), + )]); + let out = build( + dir.path(), + &json!({ + "name": "foo", + "version": "1.0.0", + "dependencies": { "from-tarball": "catalog:" }, + }), + &default_opts(&catalogs), + ); + + assert_eq!( + out["dependencies"], + json!({ "from-tarball": "file:./tarballs/from-tarball-1.0.0.tgz" }), + ); +} diff --git a/pnpm/crates/exportable-manifest/src/tests.rs b/pnpm/crates/exportable-manifest/src/tests.rs index c6e4ca2fbb..a033b38f7f 100644 --- a/pnpm/crates/exportable-manifest/src/tests.rs +++ b/pnpm/crates/exportable-manifest/src/tests.rs @@ -180,6 +180,7 @@ fn published_dependencies_keep_declaration_order() { &manifest, &CreateExportableManifestOptions { catalogs: &catalogs, + workspace_dir: None, modules_dir: None, skip_manifest_obfuscation: false, embed_readme: false, diff --git a/pnpm/crates/local-spec/Cargo.toml b/pnpm/crates/local-spec/Cargo.toml new file mode 100644 index 0000000000..e8eb29a00b --- /dev/null +++ b/pnpm/crates/local-spec/Cargo.toml @@ -0,0 +1,20 @@ +[package] +name = "pnpm-local-spec" +description = "Re-anchoring `file:` and `link:` specifiers between the directory they are written in and the directory that consumes them" +version = "0.0.1" +publish = false +authors.workspace = true +edition.workspace = true +homepage.workspace = true +keywords.workspace = true +license.workspace = true +repository.workspace = true + +[dependencies] +pnpm-fs = { workspace = true } + +[dev-dependencies] +pretty_assertions = { workspace = true } + +[lints] +workspace = true diff --git a/pnpm/crates/local-spec/src/lib.rs b/pnpm/crates/local-spec/src/lib.rs new file mode 100644 index 0000000000..7776930764 --- /dev/null +++ b/pnpm/crates/local-spec/src/lib.rs @@ -0,0 +1,110 @@ +//! A `file:` / `link:` specifier means "this path, relative to the file it +//! is written in". pnpm reads such specifiers from files that do not sit in +//! the project consuming them — `pnpm-workspace.yaml` holds the catalogs and +//! the `overrides` map — so the path has to be re-anchored before the +//! resolver, which reads every specifier relative to the importing project, +//! can see it. +//! +//! [`LocalSpec::parse`] anchors the written path at the directory of the +//! file that declared it; [`LocalSpec::render`] writes it back out for the +//! directory that consumes it. + +use std::path::{Path, PathBuf}; + +use pnpm_fs::{lexical_normalize, relative_path}; + +/// The two local-filesystem protocols a specifier can carry. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum LocalSpecProtocol { + Link, + File, +} + +impl LocalSpecProtocol { + fn as_str(self) -> &'static str { + match self { + LocalSpecProtocol::Link => "link:", + LocalSpecProtocol::File => "file:", + } + } +} + +/// A `file:` / `link:` specifier with its path resolved against the +/// directory it was written in. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct LocalSpec { + protocol: LocalSpecProtocol, + absolute_path: PathBuf, + /// Whether the specifier was written as a relative path. An absolute + /// one names the same location from everywhere, so it is rendered + /// verbatim rather than re-anchored. + specified_via_relative_path: bool, +} + +impl LocalSpec { + /// Parse a `link:` / `file:` specifier whose relative path is written + /// from `base_dir`. Returns `None` for any other shape — semver ranges, + /// tarball URLs, npm-alias specs, `catalog:` and `workspace:` + /// specifiers, and bare paths carrying no protocol. + /// + /// Only a path written relative to `base_dir` moves. An absolute one, + /// and a `~/` one that the resolver expands against the home + /// directory, name the same place from every directory. + #[must_use] + pub fn parse(specifier: &str, base_dir: &Path) -> Option { + let (protocol, pkg_path) = if let Some(rest) = specifier.strip_prefix("file:") { + (LocalSpecProtocol::File, rest) + } else { + (LocalSpecProtocol::Link, specifier.strip_prefix("link:")?) + }; + + let candidate = Path::new(pkg_path); + let specified_via_relative_path = !candidate.is_absolute() && !is_home_relative(pkg_path); + let absolute_path = lexical_normalize(&if specified_via_relative_path { + base_dir.join(candidate) + } else { + candidate.to_path_buf() + }); + Some(LocalSpec { protocol, absolute_path, specified_via_relative_path }) + } + + /// Render the specifier for the directory that consumes it. + /// Relative-form specifiers are re-anchored against `consumer_dir` so + /// they read sensibly from the consumer's perspective; absolute-form + /// ones, and every specifier when `consumer_dir` is `None`, are emitted + /// as the absolute path. + #[must_use] + pub fn render(&self, consumer_dir: Option<&Path>) -> String { + // Every branch routes through `normalize_path` so the absolute + // shape also gets backslash → forward-slash rewriting on Windows; + // `link:` / `file:` specifiers must use forward slashes regardless + // of host OS. + let path = match (self.specified_via_relative_path, consumer_dir) { + (true, Some(dir)) => normalize_path(&relative_path(dir, &self.absolute_path)), + _ => normalize_path(&self.absolute_path), + }; + // A specifier that names the consumer's own directory diffs to the + // empty string, which reads as a missing path rather than as "here". + let path = if path.is_empty() { ".".to_string() } else { path }; + format!("{}{path}", self.protocol.as_str()) + } +} + +/// Whether the path starts at the home directory. The local resolver +/// expands `~/` itself and records the specifier verbatim, so such a +/// path is anchored at neither the declaring nor the consuming +/// directory. +fn is_home_relative(path: &str) -> bool { + path.starts_with("~/") || path.starts_with(r"~\") +} + +/// Replace `\` with `/` to normalize the path. `link:` / `file:` +/// specifiers must use forward slashes regardless of host OS — the +/// lockfile and pacquet's downstream consumers expect that shape. +fn normalize_path(path: &Path) -> String { + let display = path.display().to_string(); + if cfg!(windows) { display.replace('\\', "/") } else { display } +} + +#[cfg(test)] +mod tests; diff --git a/pnpm/crates/local-spec/src/tests.rs b/pnpm/crates/local-spec/src/tests.rs new file mode 100644 index 0000000000..41ab229444 --- /dev/null +++ b/pnpm/crates/local-spec/src/tests.rs @@ -0,0 +1,71 @@ +use std::path::Path; + +use pretty_assertions::assert_eq; + +use super::LocalSpec; + +#[cfg(windows)] +const ROOT: &str = r"C:\workspace"; +#[cfg(not(windows))] +const ROOT: &str = "/workspace"; + +fn root() -> &'static Path { + Path::new(ROOT) +} + +fn render(specifier: &str, consumer: Option<&str>) -> String { + let consumer = consumer.map(|dir| root().join(dir)); + LocalSpec::parse(specifier, root()).expect("a local specifier").render(consumer.as_deref()) +} + +#[test] +fn declines_specifiers_without_a_local_protocol() { + for specifier in ["^1.2.3", "catalog:", "workspace:*", "npm:foo@1", "./foo", "https://x/y.tgz"] + { + assert_eq!(LocalSpec::parse(specifier, root()), None, "{specifier}"); + } +} + +#[test] +fn reanchors_a_relative_path_against_the_consumer() { + assert_eq!(render("file:./tarballs/x.tgz", Some("packages/foo")), "file:../../tarballs/x.tgz"); + assert_eq!(render("link:libs/x", Some("packages/foo")), "link:../../libs/x"); +} + +#[test] +fn reanchors_a_relative_path_that_climbs_above_the_base() { + assert_eq!( + render("file:../outside/x.tgz", Some("packages/foo")), + "file:../../../outside/x.tgz", + ); +} + +#[test] +fn keeps_a_relative_path_unchanged_for_a_consumer_in_the_base_directory() { + assert_eq!(render("file:./tarballs/x.tgz", Some(".")), "file:tarballs/x.tgz"); +} + +#[test] +fn renders_the_consumers_own_directory_as_a_dot() { + assert_eq!(render("link:packages/foo", Some("packages/foo")), "link:."); +} + +#[test] +fn renders_an_absolute_path_without_a_consumer_directory() { + let expected = format!("file:{}/tarballs/x.tgz", ROOT.replace('\\', "/")); + assert_eq!(render("file:./tarballs/x.tgz", None), expected); +} + +#[test] +fn leaves_an_absolute_specifier_alone() { + let absolute = format!("file:{}/tarballs/x.tgz", ROOT.replace('\\', "/")); + assert_eq!(render(&absolute, Some("packages/foo")), absolute); +} + +#[test] +fn leaves_a_home_relative_specifier_alone() { + for specifier in ["file:~/tarballs/x.tgz", "link:~/libs/x"] { + assert_eq!(render(specifier, Some("packages/foo")), specifier); + assert_eq!(render(specifier, None), specifier); + } +} diff --git a/pnpm/crates/lockfile/src/catalog_snapshots.rs b/pnpm/crates/lockfile/src/catalog_snapshots.rs index 8c5cf49332..b6c44c295a 100644 --- a/pnpm/crates/lockfile/src/catalog_snapshots.rs +++ b/pnpm/crates/lockfile/src/catalog_snapshots.rs @@ -20,6 +20,7 @@ pub struct ResolvedCatalogEntry { /// The specifier recorded under the catalog in `pnpm-workspace.yaml` /// (e.g. the `^1.2.3` of `catalog: { foo: ^1.2.3 }`). pub specifier: String, - /// The concrete version the specifier resolved to. + /// The concrete version the specifier resolved to. A `file:` / + /// `link:` entry has no version, so it repeats the specifier. pub version: String, } diff --git a/pnpm/crates/napi/src/pack.rs b/pnpm/crates/napi/src/pack.rs index 2d61c9cb7c..f7a003f628 100644 --- a/pnpm/crates/napi/src/pack.rs +++ b/pnpm/crates/napi/src/pack.rs @@ -104,6 +104,7 @@ fn pack_options(options: PackOptions) -> pnpm_pack::PackOptions { // Bit does not use catalog: specifiers; workspace catalog loading is // deferred until a consumer needs it. See pnpm/plans/NAPI.md. catalogs: Catalogs::default(), + catalogs_dir: None, embed_readme: options.embed_readme.unwrap_or(false), node_linker: NodeLinker::default(), skip_obfuscation: false, diff --git a/pnpm/crates/pack/src/lib.rs b/pnpm/crates/pack/src/lib.rs index 53fe0c290c..effa5a78a4 100644 --- a/pnpm/crates/pack/src/lib.rs +++ b/pnpm/crates/pack/src/lib.rs @@ -503,6 +503,7 @@ impl PackManifestOptions { manifest, &CreateExportableManifestOptions { catalogs: &self.catalogs, + workspace_dir: self.catalogs_dir.as_deref(), modules_dir: Some(&modules_dir), skip_manifest_obfuscation: self.skip_obfuscation, embed_readme: self.embed_readme, diff --git a/pnpm/crates/pack/src/options.rs b/pnpm/crates/pack/src/options.rs index 201aa022da..fd9fe6b54d 100644 --- a/pnpm/crates/pack/src/options.rs +++ b/pnpm/crates/pack/src/options.rs @@ -31,6 +31,9 @@ pub struct PackScripts { pub struct PackManifestOptions { /// Parsed workspace catalogs, for `catalog:` specifier rewriting. pub catalogs: Catalogs, + /// Directory holding `pnpm-workspace.yaml`, which a `file:` / + /// `link:` catalog entry's relative path is measured from. + pub catalogs_dir: Option, /// Embed the project's `README.md` into the published manifest. pub embed_readme: bool, /// Node linker mode; `bundledDependencies` only work under diff --git a/pnpm/crates/pack/src/tests.rs b/pnpm/crates/pack/src/tests.rs index fa8f4888cf..cbe949f35d 100644 --- a/pnpm/crates/pack/src/tests.rs +++ b/pnpm/crates/pack/src/tests.rs @@ -34,6 +34,7 @@ fn fixture(manifest: &Value) -> (TempDir, PackOptions) { }, manifest: crate::PackManifestOptions { catalogs: BTreeMap::new(), + catalogs_dir: None, embed_readme: false, node_linker: NodeLinker::Isolated, skip_obfuscation: false, @@ -646,6 +647,7 @@ fn workspace_license_is_injected_into_a_sub_package() { }, manifest: crate::PackManifestOptions { catalogs: BTreeMap::new(), + catalogs_dir: None, embed_readme: false, node_linker: NodeLinker::Isolated, skip_obfuscation: false, @@ -706,6 +708,7 @@ fn symlinked_workspace_license_is_not_injected() { }, manifest: crate::PackManifestOptions { catalogs: BTreeMap::new(), + catalogs_dir: None, embed_readme: false, node_linker: NodeLinker::Isolated, skip_obfuscation: false, @@ -756,6 +759,7 @@ fn workspace_root_gitignore_excludes_workspace_package_files() { }, manifest: crate::PackManifestOptions { catalogs: BTreeMap::new(), + catalogs_dir: None, embed_readme: false, node_linker: NodeLinker::Isolated, skip_obfuscation: false, diff --git a/pnpm/crates/package-manager/Cargo.toml b/pnpm/crates/package-manager/Cargo.toml index 16cbb2fe44..ef8c159ccb 100644 --- a/pnpm/crates/package-manager/Cargo.toml +++ b/pnpm/crates/package-manager/Cargo.toml @@ -38,6 +38,7 @@ pnpm-modules-yaml = { workspace = true } pnpm-network = { workspace = true } pnpm-config = { workspace = true } pnpm-config-parse-overrides = { workspace = true } +pnpm-local-spec = { workspace = true } pnpm-deps-inspection-peers = { workspace = true } pnpm-graph-hasher = { workspace = true } pnpm-hooks = { workspace = true } diff --git a/pnpm/crates/package-manager/src/catalog_mode.rs b/pnpm/crates/package-manager/src/catalog_mode.rs index 3491cb8d9d..63d5255925 100644 --- a/pnpm/crates/package-manager/src/catalog_mode.rs +++ b/pnpm/crates/package-manager/src/catalog_mode.rs @@ -15,7 +15,9 @@ use derive_more::{Display, Error}; use miette::Diagnostic; use node_semver::{Range, Version}; use pnpm_catalogs_protocol_parser::parse_catalog_protocol; -use pnpm_catalogs_resolver::{CatalogResolutionResult, WantedDependency, resolve_from_catalog}; +use pnpm_catalogs_resolver::{ + CatalogAnchor, CatalogResolutionResult, WantedDependency, resolve_from_catalog, +}; use pnpm_catalogs_types::{Catalogs, DEFAULT_CATALOG_NAME}; use pnpm_config::CatalogMode; use pnpm_reporter::{LogEvent, LogLevel, PnpmLog, Reporter}; @@ -156,7 +158,10 @@ fn decide_catalog_entry( alias: dep.alias.to_string(), bare_specifier: catalog_specifier.clone(), }; - let entry = match resolve_from_catalog(catalogs, &wanted) { + // The entry is compared against the specifier `pnpm add` was given + // and repeated back in a mismatch error, never installed from, so it + // reads best exactly as `pnpm-workspace.yaml` writes it. + let entry = match resolve_from_catalog(catalogs, &wanted, CatalogAnchor::AsWritten) { CatalogResolutionResult::Found(found) => found.resolution.specifier, _ => { return Ok(CatalogDecisionOutcome { @@ -217,12 +222,10 @@ fn catalog_mismatch( /// declares it — a `file:` / `link:` protocol, a bare path or tarball /// filename, or a `workspace:` pointing at a directory rather than a range. /// -/// A catalog entry is read by every project that references it, so it -/// cannot mean the same directory for all of them. The catalog resolver -/// already refuses a `link:` / `file:` entry outright -/// (`ERR_PNPM_CATALOG_ENTRY_INVALID_SPEC`); it accepts a `workspace:` one, -/// which is worse — every consumer silently resolves the relative path from -/// its own directory. Auto-cataloging leaves all of them alone. +/// A catalog measures such a path from `pnpm-workspace.yaml`'s own +/// directory, so moving the specifier into one would point it somewhere +/// else than where `pnpm add` was run. Auto-cataloging leaves it alone +/// and keeps the dependency direct. fn is_project_relative_path(specifier: &str) -> bool { is_local_filesystem_specifier(specifier) || is_workspace_local_path_specifier(specifier) } diff --git a/pnpm/crates/package-manager/src/dependencies_graph_to_lockfile.rs b/pnpm/crates/package-manager/src/dependencies_graph_to_lockfile.rs index 2ba3532d31..99f1cb3f0b 100644 --- a/pnpm/crates/package-manager/src/dependencies_graph_to_lockfile.rs +++ b/pnpm/crates/package-manager/src/dependencies_graph_to_lockfile.rs @@ -15,7 +15,7 @@ use packages::build_packages_and_snapshots; mod importers; -use importers::{build_importers, importer_resolved_version, manifest_alias_to_group}; +use importers::{build_importers, catalog_snapshot_version, manifest_alias_to_group}; use std::collections::{BTreeMap, HashMap, HashSet}; @@ -241,7 +241,9 @@ fn build_catalog_snapshots( else { continue; }; - let Some(version) = importer_resolved_version(importer, alias) else { continue }; + let Some(version) = catalog_snapshot_version(importer, alias, entry_specifier) else { + continue; + }; snapshots .entry(catalog_name.to_string()) .or_default() diff --git a/pnpm/crates/package-manager/src/dependencies_graph_to_lockfile/importers.rs b/pnpm/crates/package-manager/src/dependencies_graph_to_lockfile/importers.rs index abe8822455..2a120f49b7 100644 --- a/pnpm/crates/package-manager/src/dependencies_graph_to_lockfile/importers.rs +++ b/pnpm/crates/package-manager/src/dependencies_graph_to_lockfile/importers.rs @@ -66,17 +66,32 @@ pub(super) fn effective_update_reuse_scope<'o>( opts.reuse.scopes_by_importer.get(importer_id).unwrap_or(&opts.reuse.scope) } } -/// The concrete version `alias` resolved to in `importer`, read from whichever -/// dependency group carries it. Returns the peer-stripped version recorded as -/// the `version` in a catalog snapshot. -pub(super) fn importer_resolved_version(importer: &ProjectSnapshot, alias: &str) -> Option { +/// The `version` a catalog snapshot records for `alias` in `importer`: +/// the peer-stripped version it resolved to, read from whichever +/// dependency group carries it. `None` when the importer does not carry +/// the dependency at all. +/// +/// A `file:` / `link:` dependency has no version of its own, so +/// `entry_specifier` stands in. It identifies the entry just as well, +/// and unlike the resolved `link:` path — which is written relative to +/// the importer that declared it — it does not depend on which importer +/// the snapshot happened to read. +pub(super) fn catalog_snapshot_version( + importer: &ProjectSnapshot, + alias: &str, + entry_specifier: &str, +) -> Option { let key = PkgName::parse(alias).ok()?; - [&importer.dependencies, &importer.dev_dependencies, &importer.optional_dependencies] - .into_iter() - .flatten() - .find_map(|map| map.get(&key)) - .and_then(|spec| spec.version.ver_peer()) - .map(|version| version.version().to_string()) + let resolved = + [&importer.dependencies, &importer.dev_dependencies, &importer.optional_dependencies] + .into_iter() + .flatten() + .find_map(|map| map.get(&key))?; + Some( + resolved.version + .ver_peer() + .map_or_else(|| entry_specifier.to_string(), |version| version.version().to_string()), + ) } /// Build an importer's [`ProjectSnapshot`] from its on-disk manifest /// plus the per-alias `DepPath` map the resolver produced for that diff --git a/pnpm/crates/package-manager/src/fast_update_catalog_versions.rs b/pnpm/crates/package-manager/src/fast_update_catalog_versions.rs index 72077c2982..f7725bb7cf 100644 --- a/pnpm/crates/package-manager/src/fast_update_catalog_versions.rs +++ b/pnpm/crates/package-manager/src/fast_update_catalog_versions.rs @@ -75,10 +75,13 @@ fn catalog_version_update( entry: &ResolvedCatalogEntry, ) -> Option { let specifier = catalogs.get(catalog_name)?.get(alias)?; - let locked = Version::parse(&entry.version).ok()?; if specifier == &entry.specifier { return Some(CatalogVersionUpdate::Unmoved(entry.clone())); } + // Read after the unchanged case, which holds for an entry with no + // version of its own — a `file:` / `link:` entry records its path + // here — so one of those does not close the path for its siblings. + let locked = Version::parse(&entry.version).ok()?; // A specifier the locked version still satisfies moves nothing // but the specifier, exactly as the range-only path would. if Range::parse(specifier).is_ok_and(|range| locked.satisfies(&range)) { diff --git a/pnpm/crates/package-manager/src/install_with_fresh_lockfile/resolve.rs b/pnpm/crates/package-manager/src/install_with_fresh_lockfile/resolve.rs index f9e20b2f0a..23f6ce62e5 100644 --- a/pnpm/crates/package-manager/src/install_with_fresh_lockfile/resolve.rs +++ b/pnpm/crates/package-manager/src/install_with_fresh_lockfile/resolve.rs @@ -246,6 +246,7 @@ impl ImporterInputs<'_> { pick_lowest_direct: self.versions.pick_lowest, subdep_published_by: self.versions.published_by, catalogs: self.catalogs.clone(), + catalogs_dir: self.config.workspace_dir.clone(), catalog_server: false, }, hooks: self.hooks.clone(), diff --git a/pnpm/crates/package-manager/src/optimistic_repeat_install.rs b/pnpm/crates/package-manager/src/optimistic_repeat_install.rs index 5a428dbf18..d1b1a34398 100644 --- a/pnpm/crates/package-manager/src/optimistic_repeat_install.rs +++ b/pnpm/crates/package-manager/src/optimistic_repeat_install.rs @@ -96,7 +96,9 @@ use std::{ time::SystemTime, }; -use pnpm_catalogs_resolver::{CatalogResolutionResult, WantedDependency, resolve_from_catalog}; +use pnpm_catalogs_resolver::{ + CatalogAnchor, CatalogResolutionResult, WantedDependency, resolve_from_catalog, +}; use pnpm_catalogs_types::Catalogs; use pnpm_config::{Config, LinkWorkspacePackages, NodeLinker, TrustPolicy}; use pnpm_lockfile::{ImporterDepVersion, Lockfile, MaybeLazyLockfile, ProjectSnapshot}; diff --git a/pnpm/crates/package-manager/src/optimistic_repeat_install/local_file_deps.rs b/pnpm/crates/package-manager/src/optimistic_repeat_install/local_file_deps.rs index ea81e2b76b..13d3bf4883 100644 --- a/pnpm/crates/package-manager/src/optimistic_repeat_install/local_file_deps.rs +++ b/pnpm/crates/package-manager/src/optimistic_repeat_install/local_file_deps.rs @@ -1,8 +1,9 @@ //! Detecting dependency specs that point at the local filesystem. use super::{ - CatalogResolutionResult, Catalogs, Config, DependencyGroup, IncludedDependencies, Lockfile, - OptimisticRepeatInstallCheck, Path, PathBuf, WantedDependency, resolve_from_catalog, + CatalogAnchor, CatalogResolutionResult, Catalogs, Config, DependencyGroup, + IncludedDependencies, Lockfile, OptimisticRepeatInstallCheck, Path, PathBuf, WantedDependency, + resolve_from_catalog, }; use pnpm_lockfile::{LockfileResolution, PkgName}; use pnpm_resolving_local_resolver::local_tarball_path; @@ -78,7 +79,13 @@ fn scan_local_tarball_deps(check: &OptimisticRepeatInstallCheck<'_>) -> LocalTar if !group_included { continue; } - let scan = FieldTarballScan { catalogs: check.catalogs, project_dir, field, group }; + let scan = FieldTarballScan { + catalogs: check.catalogs, + workspace_dir: check.config.workspace_dir.as_deref(), + project_dir, + field, + group, + }; if !scan_field_tarballs(&scan, manifest, &mut tarballs) { return LocalTarballScan::RequiresInstall; } @@ -90,6 +97,10 @@ fn scan_local_tarball_deps(check: &OptimisticRepeatInstallCheck<'_>) -> LocalTar /// One manifest field of one project, as the tarball scan reads it. struct FieldTarballScan<'a> { catalogs: &'a Catalogs, + /// Where `pnpm-workspace.yaml` sits, so a `file:` catalog entry's + /// relative path is measured from the same directory the install + /// measures it from. + workspace_dir: Option<&'a Path>, project_dir: &'a Path, field: &'a str, group: DependencyGroup, @@ -110,7 +121,7 @@ fn scan_field_tarballs( return true; }; for (alias, spec) in deps { - match local_tarball_candidate(scan.catalogs, scan.project_dir, alias, spec) { + match local_tarball_candidate(scan, alias, spec) { LocalTarballCandidate::Skip => {} LocalTarballCandidate::Unresolvable => return false, LocalTarballCandidate::Found { path, must_be_local } => { @@ -139,19 +150,18 @@ enum LocalTarballCandidate { } fn local_tarball_candidate( - catalogs: &Catalogs, - project_dir: &Path, + scan: &FieldTarballScan<'_>, alias: &str, spec: &serde_json::Value, ) -> LocalTarballCandidate { let Some(spec) = spec.as_str() else { return LocalTarballCandidate::Skip }; - let resolved_spec = resolve_catalog_spec(catalogs, alias, spec); + let resolved_spec = resolve_catalog_spec(scan, alias, spec); let Some(spec) = resolved_spec.as_deref() else { return LocalTarballCandidate::Skip }; if !is_local_file_spec(spec) { return LocalTarballCandidate::Skip; } let must_be_local = is_unambiguous_local_file_spec(spec); - let path = local_tarball_path(spec, project_dir); + let path = local_tarball_path(spec, scan.project_dir); if must_be_local && path.is_none() { return LocalTarballCandidate::Unresolvable; } @@ -159,7 +169,7 @@ fn local_tarball_candidate( } fn resolve_catalog_spec<'a>( - catalogs: &Catalogs, + scan: &FieldTarballScan<'_>, alias: &str, spec: &'a str, ) -> Option> { @@ -167,8 +177,14 @@ fn resolve_catalog_spec<'a>( return Some(Cow::Borrowed(spec)); } match resolve_from_catalog( - catalogs, + scan.catalogs, &WantedDependency { alias: alias.to_string(), bare_specifier: spec.to_string() }, + match scan.workspace_dir { + Some(workspace_dir) => { + CatalogAnchor::Reanchor { workspace_dir, consumer_dir: Some(scan.project_dir) } + } + None => CatalogAnchor::AsWritten, + }, ) { CatalogResolutionResult::Found(found) => Some(Cow::Owned(found.resolution.specifier)), _ => None, @@ -275,9 +291,12 @@ pub(crate) fn catalog_resolves_to_local_file(catalogs: &Catalogs, alias: &str, s if !spec.starts_with("catalog:") { return false; } + // Only the shape of the entry decides this, and re-anchoring a + // relative path never changes it, so the entry is read as written. match resolve_from_catalog( catalogs, &WantedDependency { alias: alias.to_string(), bare_specifier: spec.to_string() }, + CatalogAnchor::AsWritten, ) { CatalogResolutionResult::Found(found) => is_local_file_spec(&found.resolution.specifier), _ => false, @@ -352,8 +371,7 @@ pub(crate) fn has_local_file_package_extension( /// Such specs (and anything else carrying a protocol or URL) stay on /// the fast path. `catalog:` specs also return `false` here — callers /// dereference them through the workspace catalogs first, because a -/// catalog entry may hold a bare local path (the catalog resolver only -/// bans the `workspace:`, `link:`, and `file:` protocols). +/// catalog entry may itself hold a local path. pub(crate) fn is_local_file_spec(spec: &str) -> bool { if is_unambiguous_local_file_spec(spec) { return true; diff --git a/pnpm/crates/package-manager/src/overrides.rs b/pnpm/crates/package-manager/src/overrides.rs index 11b317643e..e11e3f0af4 100644 --- a/pnpm/crates/package-manager/src/overrides.rs +++ b/pnpm/crates/package-manager/src/overrides.rs @@ -16,13 +16,12 @@ pub(crate) use selectors::parse_declared_range; -mod local_targets; mod selectors; -use local_targets::{LocalTarget, parse_local_target, resolve_local_override_spec}; use selectors::{matches_target, semver_satisfies, sort_by_specificity}; use node_semver::{Range, Version}; use pnpm_config_parse_overrides::{PackageSelector, VersionOverride}; +use pnpm_local_spec::LocalSpec; use pnpm_package_manifest::{DependencyGroup, PackageManifest}; use pnpm_resolving_resolver_base::is_valid_peer_range; use serde_json::Value; @@ -67,12 +66,12 @@ struct ConvergeOverride { version: Option, } -/// `VersionOverride` augmented with a pre-parsed [`LocalTarget`] for +/// `VersionOverride` augmented with a pre-parsed [`LocalSpec`] for /// the local-protocol forms. Splitting once at construction time /// avoids re-parsing the prefix on every manifest read. struct ResolvedOverride { inner: VersionOverride, - local_target: Option, + local_target: Option, } /// Answers whether an override governs a dependency declared as a given @@ -116,7 +115,7 @@ impl VersionsOverrider { } let resolved = ResolvedOverride { inner: override_entry.clone(), - local_target: parse_local_target(&override_entry.new_bare_specifier, root_dir), + local_target: LocalSpec::parse(&override_entry.new_bare_specifier, root_dir), }; if override_entry.parent_pkg.is_some() { parent_scoped.push(resolved); @@ -309,7 +308,7 @@ impl VersionsOverrider { .as_ref() .map_or_else( || chosen.inner.new_bare_specifier.clone(), - |target| resolve_local_override_spec(target, manifest_dir), + |target| target.render(manifest_dir), ); map.insert(name, Value::String(new_spec)); @@ -365,7 +364,7 @@ impl VersionsOverrider { .as_ref() .map_or_else( || chosen.inner.new_bare_specifier.clone(), - |target| resolve_local_override_spec(target, manifest_dir), + |target| target.render(manifest_dir), ); if is_valid_peer_range(&new_spec) { insert_peer_dependency(value, name, new_spec); @@ -401,7 +400,7 @@ impl VersionsOverrider { .as_ref() .map_or_else( || chosen.inner.new_bare_specifier.clone(), - |target| resolve_local_override_spec(target, Some(pkg_dir)), + |target| target.render(Some(pkg_dir)), ), ); } diff --git a/pnpm/crates/package-manager/src/overrides/local_targets.rs b/pnpm/crates/package-manager/src/overrides/local_targets.rs deleted file mode 100644 index 7bfbb65e39..0000000000 --- a/pnpm/crates/package-manager/src/overrides/local_targets.rs +++ /dev/null @@ -1,64 +0,0 @@ -use std::path::{Path, PathBuf}; - -#[derive(Debug, Clone, Copy)] -pub(super) enum LocalProtocol { - Link, - File, -} -impl LocalProtocol { - fn as_str(self) -> &'static str { - match self { - LocalProtocol::Link => "link:", - LocalProtocol::File => "file:", - } - } -} -pub(super) struct LocalTarget { - protocol: LocalProtocol, - absolute_path: PathBuf, - specified_via_relative_path: bool, -} -/// Parse the override's `new_bare_specifier` for the `link:` / `file:` -/// prefix. Returns `None` for any other shape — semver ranges, tarball -/// URLs, npm-alias specs, etc. -pub(super) fn parse_local_target(new_bare_specifier: &str, root_dir: &Path) -> Option { - let (protocol, pkg_path) = if let Some(rest) = new_bare_specifier.strip_prefix("file:") { - (LocalProtocol::File, rest) - } else { - (LocalProtocol::Link, new_bare_specifier.strip_prefix("link:")?) - }; - - let candidate = Path::new(pkg_path); - let specified_via_relative_path = !candidate.is_absolute(); - let absolute_path = if specified_via_relative_path { - root_dir.join(candidate) - } else { - candidate.to_path_buf() - }; - Some(LocalTarget { protocol, absolute_path, specified_via_relative_path }) -} -/// Render a `link:` / `file:` override against the importing -/// package's directory. Relative-form targets are re-anchored against -/// `pkg_dir` so they read sensibly from the consumer's perspective; -/// absolute-form targets are emitted verbatim. -pub(super) fn resolve_local_override_spec(target: &LocalTarget, pkg_dir: Option<&Path>) -> String { - // Every branch routes through `normalize_path` so absolute and - // diff-paths-fallback shapes also get backslash → forward-slash - // rewriting on Windows; `link:` / `file:` specifiers - // must use forward slashes regardless of host OS. - let path_str = match (target.specified_via_relative_path, pkg_dir) { - (true, Some(dir)) => pathdiff::diff_paths(&target.absolute_path, dir) - .as_deref() - .map_or_else(|| normalize_path(&target.absolute_path), normalize_path), - _ => normalize_path(&target.absolute_path), - }; - format!("{}{path_str}", target.protocol.as_str()) -} -/// Replace `\\` with `/` to normalize the path. -/// `link:` / `file:` specifiers must use forward slashes regardless -/// of host OS — the lockfile and pacquet's downstream consumers -/// expect that shape. -pub(super) fn normalize_path(path: &Path) -> String { - let display = path.display().to_string(); - if cfg!(windows) { display.replace('\\', "/") } else { display } -} diff --git a/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs b/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs index d87b957a72..90a29e0421 100644 --- a/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs +++ b/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs @@ -20,7 +20,7 @@ use derive_more::{Display, Error}; use futures_util::future; use miette::Diagnostic; use pipe_trait::Pipe; -use pnpm_catalogs_resolver::CatalogResolutionError; +use pnpm_catalogs_resolver::{CatalogAnchor, CatalogResolutionError}; use pnpm_hooks::PnpmfileHooks; use pnpm_package_manifest::{DependencyGroup, PackageManifest}; use pnpm_patching::{PatchGroupRecord, PatchKeyConflictError}; diff --git a/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree/catalogs.rs b/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree/catalogs.rs index 28985aac3c..3ae8054d35 100644 --- a/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree/catalogs.rs +++ b/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree/catalogs.rs @@ -1,12 +1,28 @@ //! `catalog:` specifier resolution for importer-level dependencies. +use std::path::Path; + use pnpm_catalogs_resolver::{ - CatalogResolutionResult, WantedDependency as CatalogWantedDependency, resolve_from_catalog, + CatalogAnchor, CatalogResolutionResult, WantedDependency as CatalogWantedDependency, + resolve_from_catalog, }; use pnpm_catalogs_types::Catalogs; use super::{ResolveDependencyTreeError, WantedSpec}; +/// The anchor for an entry the manifest in `consumer_dir` dereferences. +/// An install with no `pnpm-workspace.yaml` declares no catalogs, so +/// there is no path to move. +pub(super) fn catalog_anchor<'a>( + workspace_dir: Option<&'a Path>, + consumer_dir: Option<&'a Path>, +) -> CatalogAnchor<'a> { + match workspace_dir { + Some(workspace_dir) => CatalogAnchor::Reanchor { workspace_dir, consumer_dir }, + None => CatalogAnchor::AsWritten, + } +} + /// Replace `catalog:` bare specifiers on direct dependencies with the /// version recorded in the catalogs map. Non-`catalog:` specifiers /// pass through unchanged. @@ -17,11 +33,12 @@ use super::{ResolveDependencyTreeError, WantedSpec}; pub(crate) fn resolve_catalog_specifiers( specs: Vec, catalogs: &Catalogs, + anchor: CatalogAnchor<'_>, ) -> Result, ResolveDependencyTreeError> { specs .into_iter() .map(|(name, range, optional, injected)| { - resolve_catalog_specifier(name, range, catalogs) + resolve_catalog_specifier(name, range, catalogs, anchor) .map(|(name, range)| (name, range, optional, injected)) }) .collect() @@ -31,9 +48,10 @@ pub(super) fn resolve_catalog_specifier( name: String, range: String, catalogs: &Catalogs, + anchor: CatalogAnchor<'_>, ) -> Result<(String, String), ResolveDependencyTreeError> { let wanted = CatalogWantedDependency { alias: name.clone(), bare_specifier: range.clone() }; - match resolve_from_catalog(catalogs, &wanted) { + match resolve_from_catalog(catalogs, &wanted, anchor) { CatalogResolutionResult::Found(found) => Ok((name, found.resolution.specifier)), CatalogResolutionResult::Unused => Ok((name, range)), CatalogResolutionResult::Misconfiguration(misconfig) => { diff --git a/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree/importer.rs b/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree/importer.rs index c2fdb144cf..7269a1795e 100644 --- a/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree/importer.rs +++ b/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree/importer.rs @@ -4,6 +4,8 @@ //! what the walk reads off each resolved package — lives in //! [`super::manifest`]. +use std::path::Path; + use pnpm_catalogs_types::Catalogs; use pnpm_package_manifest::{DependencyGroup, PackageManifest}; use pnpm_resolving_resolver_base::is_acceptable_peer_spec; @@ -11,7 +13,8 @@ use rustc_hash::{FxHashMap as HashMap, FxHashSet as HashSet}; use serde_json::Value; use super::{ - ResolveDependencyTreeError, WantedSpec, dependency_meta_is_injected, resolve_catalog_specifiers, + ResolveDependencyTreeError, WantedSpec, catalogs::catalog_anchor, dependency_meta_is_injected, + resolve_catalog_specifiers, }; /// Collect the names of the importer manifest's `optionalDependencies` @@ -50,6 +53,10 @@ fn injected_dependency_names(manifest: &Value) -> HashSet { /// `peerDependencies`) tagged with the right `optional` / `injected` /// flags and with `catalog:` specifiers resolved. /// +/// `workspace_dir` is where `pnpm-workspace.yaml` sits, so a `file:` / +/// `link:` catalog entry lands on the path the manifest's own directory +/// would have written. +/// /// An alias declared in several groups yields one spec, merged by /// spreading the groups in order: `peerDependencies` first (when /// `auto_install_peers`), then `devDependencies` < `dependencies` < @@ -77,6 +84,7 @@ pub(crate) fn importer_direct_wanted_specs( dependency_groups: DependencyGroupList, auto_install_peers: bool, catalogs: &Catalogs, + workspace_dir: Option<&Path>, ) -> Result, ResolveDependencyTreeError> where DependencyGroupList: IntoIterator, @@ -118,7 +126,8 @@ where ) }) .collect(); - resolve_catalog_specifiers(wanted, catalogs) + let consumer_dir = manifest.path().parent(); + resolve_catalog_specifiers(wanted, catalogs, catalog_anchor(workspace_dir, consumer_dir)) } /// Reject a `peerDependencies` value that is neither a peer range nor a diff --git a/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree/manifest.rs b/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree/manifest.rs index d6e7d6ff4c..d7f1b2f733 100644 --- a/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree/manifest.rs +++ b/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree/manifest.rs @@ -12,7 +12,7 @@ use std::collections::BTreeMap; use crate::resolved_tree::PeerDep; use super::{ - Deprecation, ResolveDependencyTreeError, catalogs::resolve_catalog_specifier, + CatalogAnchor, Deprecation, ResolveDependencyTreeError, catalogs::resolve_catalog_specifier, dependency_is_injected, lock_recoverable, tree_ctx::TreeCtx, workspace_ctx::ChildSpec, }; @@ -301,7 +301,15 @@ fn insert_declared_peers( let Some(range_str) = range.as_str() else { continue }; let version = match catalogs { Some(catalogs) => { - resolve_catalog_specifier(name.clone(), range_str.to_string(), catalogs)?.1 + // A peer range names a version range, never a path, so a + // `file:` / `link:` entry has nothing to re-anchor. + resolve_catalog_specifier( + name.clone(), + range_str.to_string(), + catalogs, + CatalogAnchor::AsWritten, + )? + .1 } None => range_str.to_string(), }; diff --git a/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree/tree_ctx.rs b/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree/tree_ctx.rs index 60b79edca2..de060acfcd 100644 --- a/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree/tree_ctx.rs +++ b/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree/tree_ctx.rs @@ -118,6 +118,9 @@ pub struct TreeCtx { /// workspace packages. Other transitive dependencies keep catalog /// resolution disabled. pub(super) catalogs: Catalogs, + /// Directory `pnpm-workspace.yaml` sits in, which a `file:` / + /// `link:` catalog entry's relative path is measured from. + pub(super) catalogs_dir: Option, pub(super) workspace: Arc, /// Configured `patchedDependencies` (already grouped by name). /// Shared by `Arc` so the lookup table doesn't get cloned per @@ -187,6 +190,7 @@ impl TreeCtx { super::workspace_ctx::WorkspaceResolutionOptionsKey::new(&base_opts), ), catalogs: Catalogs::new(), + catalogs_dir: None, workspace: Arc::new(WorkspaceTreeCtx::default()), patched_dependencies: None, importer: TreeImporterContext { @@ -226,6 +230,7 @@ impl TreeCtx { super::workspace_ctx::WorkspaceResolutionOptionsKey::new(&base_opts), ), catalogs: Catalogs::new(), + catalogs_dir: None, workspace, patched_dependencies: None, importer: TreeImporterContext { @@ -284,9 +289,13 @@ impl TreeCtx { self } + /// `catalogs_dir` is where `pnpm-workspace.yaml` sits — the + /// directory a `file:` / `link:` catalog entry's relative path is + /// measured from. #[must_use] - pub fn with_catalogs(mut self, catalogs: Catalogs) -> Self { + pub fn with_catalogs(mut self, catalogs: Catalogs, catalogs_dir: Option) -> Self { self.catalogs = catalogs; + self.catalogs_dir = catalogs_dir; self } diff --git a/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree/walk.rs b/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree/walk.rs index 594f6e5804..b7880aaecd 100644 --- a/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree/walk.rs +++ b/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree/walk.rs @@ -59,8 +59,9 @@ use crate::{ }; use super::{ - ResolveDependencyTreeError, SkippedOptionalDependency, SkippedOptionalDependencyParent, - catalogs::resolve_catalog_specifier, + CatalogAnchor, ResolveDependencyTreeError, SkippedOptionalDependency, + SkippedOptionalDependencyParent, + catalogs::{catalog_anchor, resolve_catalog_specifier}, lock_recoverable, manifest::{ build_pkg_id_with_patch_hash, emit_deprecation_if_needed, extract_children, diff --git a/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree/walk/child_seeds.rs b/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree/walk/child_seeds.rs index 5a448eaff7..c5949d8fc2 100644 --- a/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree/walk/child_seeds.rs +++ b/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree/walk/child_seeds.rs @@ -1,9 +1,9 @@ use super::{ - Arc, Catalogs, ChildEdge, ChildSpec, Cow, FrontierNode, HashSet, NodeSeed, Path, PendingNode, - Pipe, PkgNameVerPeer, PreferredVersionsOverlay, ResolveDependencyTreeError, Resolver, - ReuseSource, SeededNode, SnapshotEntry, TreeCtx, WantedDependency, async_recursion, - declaring_manifest_dir, extract_children, future, higher_direct_dep_version, level_aliases, - level_versions, lock_recoverable, prior_child_key, real_package_name_of, + Arc, CatalogAnchor, Catalogs, ChildEdge, ChildSpec, Cow, FrontierNode, HashSet, NodeSeed, Path, + PendingNode, Pipe, PkgNameVerPeer, PreferredVersionsOverlay, ResolveDependencyTreeError, + Resolver, ReuseSource, SeededNode, SnapshotEntry, TreeCtx, WantedDependency, async_recursion, + catalog_anchor, declaring_manifest_dir, extract_children, future, higher_direct_dep_version, + level_aliases, level_versions, lock_recoverable, prior_child_key, real_package_name_of, resolve_catalog_specifier, resolve_node_seed, warm_children_resolutions, }; @@ -74,12 +74,16 @@ pub(super) fn child_specs_of( .pipe(Arc::new) }; Ok(match catalogs_for_children(ctx, pending.resolves_children_through_catalogs) { - Some(catalogs) => child_specs - .iter() - .cloned() - .collect::>() - .pipe(|specs| resolve_catalog_child_specs(specs, catalogs))? - .pipe(Arc::new), + Some(catalogs) => { + let declaring_dir = declaring_manifest_dir(ctx, &pending.result); + let anchor = catalog_anchor(ctx.catalogs_dir.as_deref(), declaring_dir.as_deref()); + child_specs + .iter() + .cloned() + .collect::>() + .pipe(|specs| resolve_catalog_child_specs(specs, catalogs, anchor))? + .pipe(Arc::new) + } None => child_specs, }) } @@ -263,14 +267,18 @@ pub(super) fn catalogs_for_children( (resolves_children_through_catalogs && !ctx.catalogs.is_empty()).then_some(&ctx.catalogs) } +/// `anchor` names the injected workspace package's own directory as the +/// consumer — that manifest declares these children — so a `file:` / +/// `link:` catalog entry lands on the path it would have written. pub(super) fn resolve_catalog_child_specs( child_specs: Vec, catalogs: &Catalogs, + anchor: CatalogAnchor<'_>, ) -> Result, ResolveDependencyTreeError> { child_specs .into_iter() .map(|(name, range, optional, injected)| { - resolve_catalog_specifier(name, range, catalogs) + resolve_catalog_specifier(name, range, catalogs, anchor) .map(|(name, range)| (name, range, optional, injected)) }) .collect() diff --git a/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree/walk/warm_children.rs b/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree/walk/warm_children.rs index 60b8a02284..cbafe207a1 100644 --- a/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree/walk/warm_children.rs +++ b/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree/walk/warm_children.rs @@ -1,7 +1,7 @@ use super::{ ChildSpec, HashSet, NodeSeed, ParentPkgAliases, Pipe, ResolveOptions, Resolver, TreeCtx, - WantedDependency, WantedKey, async_recursion, catalogs_for_children, claim_children_warmup, - declaring_manifest_dir, extract_children, future, is_update_target, + WantedDependency, WantedKey, async_recursion, catalog_anchor, catalogs_for_children, + claim_children_warmup, declaring_manifest_dir, extract_children, future, is_update_target, opts_relative_to_declaring_manifest, peer_shadowed_dependencies, project_relative_cache_scope, resolve_catalog_child_specs, resolve_wanted_cached, resolves_children_through_catalogs, }; @@ -101,7 +101,9 @@ pub(super) fn warm_child_specs( .collect() }; let Some(catalogs) = catalogs_for_children(ctx, through_catalogs) else { return Some(specs) }; - resolve_catalog_child_specs(specs, catalogs).ok() + let declaring_dir = declaring_manifest_dir(ctx, result); + let anchor = catalog_anchor(ctx.catalogs_dir.as_deref(), declaring_dir.as_deref()); + resolve_catalog_child_specs(specs, catalogs, anchor).ok() } /// Warm one child edge through the same per-wanted dedup cache, under the diff --git a/pnpm/crates/resolving-deps-resolver/src/resolve_importer.rs b/pnpm/crates/resolving-deps-resolver/src/resolve_importer.rs index 2605084bd6..51ab6f5cc6 100644 --- a/pnpm/crates/resolving-deps-resolver/src/resolve_importer.rs +++ b/pnpm/crates/resolving-deps-resolver/src/resolve_importer.rs @@ -175,6 +175,10 @@ pub struct ImporterResolutionInputs { /// Catalogs parsed from `pnpm-workspace.yaml`. Applied to importer /// dependencies and to children of injected workspace packages. pub catalogs: Catalogs, + /// Directory `pnpm-workspace.yaml` sits in, which a `file:` / + /// `link:` catalog entry's relative path is measured from. `None` + /// when the install has no workspace manifest, and so no catalogs. + pub catalogs_dir: Option, pub catalog_server: bool, } @@ -390,6 +394,7 @@ impl DirectSeeds { dependency_groups, opts.peers.auto_install_peers, &opts.resolution.catalogs, + opts.resolution.catalogs_dir.as_deref(), )?; Ok(Self { wanted_specifier_by_alias: initial_wanted @@ -440,7 +445,7 @@ impl ResolveImporterOptions { self.resolution.pick_lowest_direct, self.resolution.subdep_published_by, ) - .with_catalogs(self.resolution.catalogs); + .with_catalogs(self.resolution.catalogs, self.resolution.catalogs_dir.clone()); let settings = HoistSettings { all_preferred_versions: self.resolution.all_preferred_versions, override_bare_specifier: self.resolution.override_bare_specifier, diff --git a/pnpm/crates/resolving-deps-resolver/src/resolve_importer/tests.rs b/pnpm/crates/resolving-deps-resolver/src/resolve_importer/tests.rs index 5a99e9c4ac..9b6197805a 100644 --- a/pnpm/crates/resolving-deps-resolver/src/resolve_importer/tests.rs +++ b/pnpm/crates/resolving-deps-resolver/src/resolve_importer/tests.rs @@ -244,6 +244,7 @@ fn default_opts() -> ResolveImporterOptions { pick_lowest_direct: false, subdep_published_by: None, catalogs: pnpm_catalogs_types::Catalogs::new(), + catalogs_dir: None, catalog_server: false, }, hooks: crate::ManifestTransformHooks { diff --git a/pnpm/crates/resolving-deps-resolver/src/resolve_workspace/tests.rs b/pnpm/crates/resolving-deps-resolver/src/resolve_workspace/tests.rs index 45e544a6f0..c4f97d1007 100644 --- a/pnpm/crates/resolving-deps-resolver/src/resolve_workspace/tests.rs +++ b/pnpm/crates/resolving-deps-resolver/src/resolve_workspace/tests.rs @@ -259,6 +259,7 @@ fn importer_opts( pick_lowest_direct: false, subdep_published_by: published_by, catalogs: pnpm_catalogs_types::Catalogs::new(), + catalogs_dir: None, catalog_server: false, }, hooks: crate::ManifestTransformHooks { diff --git a/pnpm/crates/resolving-deps-resolver/src/resolve_workspace/time_based.rs b/pnpm/crates/resolving-deps-resolver/src/resolve_workspace/time_based.rs index 07ff0a24f8..ee20193885 100644 --- a/pnpm/crates/resolving-deps-resolver/src/resolve_workspace/time_based.rs +++ b/pnpm/crates/resolving-deps-resolver/src/resolve_workspace/time_based.rs @@ -101,6 +101,7 @@ pub(super) async fn record_direct_publish_dates( dependency_groups.iter().copied(), opts.peers.auto_install_peers, &opts.resolution.catalogs, + opts.resolution.catalogs_dir.as_deref(), ) else { return; }; diff --git a/pnpm/crates/resolving-deps-resolver/src/tests/importer_wanted_specs.rs b/pnpm/crates/resolving-deps-resolver/src/tests/importer_wanted_specs.rs index 8445f2b57e..7b91579e28 100644 --- a/pnpm/crates/resolving-deps-resolver/src/tests/importer_wanted_specs.rs +++ b/pnpm/crates/resolving-deps-resolver/src/tests/importer_wanted_specs.rs @@ -40,6 +40,7 @@ fn regular_dep_wins_over_own_peer_with_auto_install_peers() { ALL_GROUPS, true, &pnpm_catalogs_types::Catalogs::new(), + None, ) .unwrap(); assert_eq!(wanted, vec![("foo".to_string(), "workspace:*".to_string(), false, false)]); @@ -55,6 +56,7 @@ fn peer_only_dep_is_wanted_with_auto_install_peers() { ALL_GROUPS, true, &pnpm_catalogs_types::Catalogs::new(), + None, ) .unwrap(); assert_eq!(wanted, vec![("peer-only".to_string(), "^2.0.0".to_string(), false, false)]); @@ -71,6 +73,7 @@ fn peer_only_dep_is_not_wanted_without_auto_install_peers() { ALL_GROUPS, false, &pnpm_catalogs_types::Catalogs::new(), + None, ) .unwrap(); assert_eq!(wanted, vec![("regular".to_string(), "^1.0.0".to_string(), false, false)]); @@ -87,6 +90,7 @@ fn later_regular_group_range_replaces_earlier_one() { ALL_GROUPS, false, &pnpm_catalogs_types::Catalogs::new(), + None, ) .unwrap(); assert_eq!(wanted, vec![("foo".to_string(), "^2.0.0".to_string(), true, false)]); @@ -108,6 +112,7 @@ fn regular_dep_range_wins_over_dev_range_of_same_alias() { ALL_GROUPS, false, &pnpm_catalogs_types::Catalogs::new(), + None, ) .unwrap(); assert_eq!(wanted, vec![("foo".to_string(), "1.0.0".to_string(), false, false)]); @@ -124,6 +129,7 @@ fn rejects_invalid_peer_dependency_specification() { ALL_GROUPS, false, &pnpm_catalogs_types::Catalogs::new(), + None, ) .unwrap_err(); let ResolveDependencyTreeError::InvalidPeerDependencySpecification { @@ -150,6 +156,7 @@ fn names_an_unnamed_project_by_its_directory() { ALL_GROUPS, false, &pnpm_catalogs_types::Catalogs::new(), + None, ) .unwrap_err(); let ResolveDependencyTreeError::InvalidPeerDependencySpecification { project_id, .. } = err @@ -169,6 +176,7 @@ fn accepts_scheme_carrying_peer_specifiers() { ALL_GROUPS, true, &pnpm_catalogs_types::Catalogs::new(), + None, ) .expect("scheme-carrying peer specifiers are accepted"); } diff --git a/pnpm/tasks/micro-benchmark/src/workspace_resolution.rs b/pnpm/tasks/micro-benchmark/src/workspace_resolution.rs index a62fcc644e..d4483a37ce 100644 --- a/pnpm/tasks/micro-benchmark/src/workspace_resolution.rs +++ b/pnpm/tasks/micro-benchmark/src/workspace_resolution.rs @@ -265,6 +265,7 @@ fn importer_options(importer: &WorkspaceImporter<'_>) -> ResolveImporterOptions pick_lowest_direct: false, subdep_published_by: None, catalogs: pnpm_catalogs_types::Catalogs::new(), + catalogs_dir: None, catalog_server: false, }, hooks: pnpm_resolving_deps_resolver::ManifestTransformHooks {