From ddf4ec4612ed98aebaf5e2abdf4780edbf44cec0 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Thu, 28 May 2026 22:04:43 +0200 Subject: [PATCH] feat(pacquet): port dedupeInjectedDeps (#12023) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ports pnpm's [`dedupeInjectedDeps`](https://github.com/pnpm/pnpm/blob/39101f5e37/installing/deps-resolver/src/dedupeInjectedDeps.ts) to pacquet end-to-end, and restructures the resolver to match pnpm's multi-importer shape so the dedupe lives where pnpm puts it. - **Config plumbing** — `dedupe_injected_deps: bool` on `Config` (default `true`), read from `pnpm-workspace.yaml`'s `dedupeInjectedDeps` key, overridable via `PNPM_CONFIG_DEDUPE_INJECTED_DEPS`. Cleared as a workspace-only field in `WorkspaceSettings::clear_workspace_only_fields`. - **`dependenciesMeta.injected` plumbing** — pacquet's deps-resolver previously constructed `WantedDependency` with `..Default::default()`, so the per-package `dependenciesMeta[].injected: true` flag never reached the npm/local resolvers and no install path produced a `file:` direct dep. Reading `dependenciesMeta` at the importer-level wanted-dep collection unlocks the `file:` workspace-pick branch the dedupe consumer is designed to collapse. - **Multi-importer resolver refactor** — new `resolve_workspace` orchestrator (`pacquet/crates/resolving-deps-resolver/src/resolve_workspace.rs`) mirrors pnpm's [`resolveDependencies(importers, opts)`](https://github.com/pnpm/pnpm/blob/39101f5e37/installing/deps-resolver/src/index.ts#L128). It constructs one shared `WorkspaceTreeCtx` (resolved-pkgs dedup, children-spec cache, resolver-call memo, peer-walker seed sets), hands `Arc::clone` to every per-importer `resolve_importer_with_workspace`, then runs a single `resolve_peers_workspace` pass that shares `peersCache` + `purePkgs` across importers. Importer N's tree walk now reuses importer M's resolver hits instead of re-running the chain. - **`dedupeInjectedDeps` lives in `resolve_peers_workspace`** (`pacquet/crates/resolving-deps-resolver/src/dedupe_injected_deps.rs`), matching pnpm's `resolvePeers` integration. The install layer no longer carries any dedupe wiring; it just hands importers + a per-importer `ResolveOptions` closure to `resolve_workspace`. After dedupe, unreachable `file:` snapshots are pruned from the graph so they don't leak into `pnpm-lock.yaml`. - **`ImporterDepVersion::File` arm** — when `dedupeInjectedDeps: false` leaves an injected workspace dep as `file:packages/` at the importer level, the lockfile writer used to panic at `importer_dep_version` (the `parse::().expect(...)` arm). Adds a `File(String)` variant to `ImporterDepVersion`, wires it through `dependencies_graph_to_lockfile` and `symlink_direct_dependencies`, and the new e2e test `injected_workspace_dep_with_dedupe_off_writes_file_arm` exercises that path end-to-end. - **Workspace state** — surfaces `dedupe_injected_deps` in `current_settings` and adds it to the `settings_match` comparison in `optimistic_repeat_install`; drops it from the "deliberately not compared" list so settings drift now triggers a reinstall. Tracked under pnpm/pnpm#12009 (one item; the rest are separate PRs). --- .../crates/cli/tests/dedupe_injected_deps.rs | 162 ++++++++ .../cli/tests/inject_workspace_packages.rs | 7 + pacquet/crates/config/src/env_overlay.rs | 1 + pacquet/crates/config/src/lib.rs | 10 + pacquet/crates/config/src/workspace_yaml.rs | 5 +- .../lockfile/src/resolved_dependency.rs | 62 +++- .../src/dependencies_graph_to_lockfile.rs | 10 + .../install_package_from_registry/tests.rs | 1 + .../src/install_with_fresh_lockfile.rs | 168 ++++----- .../src/optimistic_repeat_install.rs | 3 +- .../src/optimistic_repeat_install/tests.rs | 1 - .../src/symlink_direct_dependencies.rs | 19 +- .../src/dedupe_injected_deps.rs | 351 ++++++++++++++++++ .../crates/resolving-deps-resolver/src/lib.rs | 38 +- .../src/resolve_dependency_tree.rs | 251 ++++++++----- .../src/resolve_importer.rs | 46 ++- .../src/resolve_peers.rs | 116 +++++- .../src/resolve_workspace.rs | 158 ++++++++ 18 files changed, 1196 insertions(+), 213 deletions(-) create mode 100644 pacquet/crates/cli/tests/dedupe_injected_deps.rs create mode 100644 pacquet/crates/resolving-deps-resolver/src/dedupe_injected_deps.rs create mode 100644 pacquet/crates/resolving-deps-resolver/src/resolve_workspace.rs diff --git a/pacquet/crates/cli/tests/dedupe_injected_deps.rs b/pacquet/crates/cli/tests/dedupe_injected_deps.rs new file mode 100644 index 0000000000..c347775c19 --- /dev/null +++ b/pacquet/crates/cli/tests/dedupe_injected_deps.rs @@ -0,0 +1,162 @@ +//! End-to-end coverage for the `dedupeInjectedDeps` setting. +//! +//! Ports the spirit of pnpm's `'injected local packages are deduped'` +//! test at +//! [`installing/deps-installer/test/install/injectLocalPackages.ts:1785`](https://github.com/pnpm/pnpm/blob/39101f5e37/installing/deps-installer/test/install/injectLocalPackages.ts#L1785): +//! a workspace dep marked `dependenciesMeta[].injected: true` +//! whose target project has no transitive deps must collapse from +//! `file:` back to `link:` once the dedupe pass +//! recognizes the children-subset case. + +use assert_cmd::prelude::*; +use command_extra::CommandExtra; +use pacquet_testing_utils::{ + bin::{AddMockedRegistry, CommandTempCwd}, + fs::is_symlink_or_junction, +}; +use std::fs; + +/// Two-project workspace where `a` injects leaf `b`. With the default +/// `dedupeInjectedDeps: true`, the install pass rewrites `a`'s direct +/// dep from `file:packages/b` back to `link:../b` because `b`'s +/// transitive deps are empty (vacuous subset). +#[test] +fn injected_leaf_workspace_dep_is_deduped_to_link() { + let CommandTempCwd { pacquet, root, workspace, npmrc_info, .. } = + CommandTempCwd::init().add_mocked_registry(); + let AddMockedRegistry { mock_instance, .. } = npmrc_info; + + fs::write( + workspace.join("package.json"), + serde_json::json!({ "name": "ws-root", "version": "0.0.0", "private": true }).to_string(), + ) + .expect("write root package.json"); + + let workspace_yaml_path = workspace.join("pnpm-workspace.yaml"); + let mut workspace_yaml = + fs::read_to_string(&workspace_yaml_path).expect("read pnpm-workspace.yaml"); + if !workspace_yaml.ends_with('\n') { + workspace_yaml.push('\n'); + } + workspace_yaml.push_str("packages:\n - 'packages/*'\n"); + fs::write(&workspace_yaml_path, workspace_yaml).expect("write pnpm-workspace.yaml"); + + fs::create_dir_all(workspace.join("packages/a")).expect("mkdir packages/a"); + fs::write( + workspace.join("packages/a/package.json"), + serde_json::json!({ + "name": "a", + "version": "1.0.0", + "dependencies": { "b": "workspace:*" }, + "dependenciesMeta": { "b": { "injected": true } }, + }) + .to_string(), + ) + .expect("write packages/a/package.json"); + + fs::create_dir_all(workspace.join("packages/b")).expect("mkdir packages/b"); + fs::write( + workspace.join("packages/b/package.json"), + serde_json::json!({ "name": "b", "version": "1.0.0" }).to_string(), + ) + .expect("write packages/b/package.json"); + + pacquet.with_arg("install").assert().success(); + + let dep = workspace.join("packages/a/node_modules/b"); + assert!( + is_symlink_or_junction(&dep).expect("query packages/a/node_modules/b"), + "packages/a/node_modules/b should be a symlink — dedupeInjectedDeps was supposed to rewrite the injected file: dep back to link:", + ); + + let lockfile_path = workspace.join("pnpm-lock.yaml"); + let lockfile = fs::read_to_string(&lockfile_path).expect("read pnpm-lock.yaml"); + assert!( + lockfile.contains("link:../b"), + "pnpm-lock.yaml should record b as link:../b under packages/a:\n{lockfile}", + ); + assert!( + !lockfile.contains("file:packages/b"), + "pnpm-lock.yaml should not retain the file:packages/b snapshot after dedupe:\n{lockfile}", + ); + + drop((root, mock_instance)); +} + +/// Same fixture as the dedupe-true test but with +/// `dedupeInjectedDeps: false` in `pnpm-workspace.yaml`: the injected +/// snapshot stays as `file:packages/b` in the importer entry, the +/// lockfile writer's [`ImporterDepVersion::File`] arm formats it, and +/// the on-disk layout points at the virtual store slot instead of a +/// `link:` sibling. Regression test for the writer panic on `file:` +/// importer-level depPaths that the resolver-side dedupe used to hide. +/// +/// Windows-skipped: pacquet's `create_virtual_store` pass does not +/// materialise `file:` snapshots into the virtual store +/// yet (broader gap tracked under pnpm/pnpm#12009's +/// `injectWorkspacePackages` line), so the symlink at +/// `packages/a/node_modules/b` points at a non-existent target. Unix +/// silently tolerates the broken link during the bin-link manifest +/// walk; Windows is stricter and trips `ERROR_INVALID_NAME` reading +/// through it with a mixed-separator path. The lockfile-writer +/// regression this test guards is platform-independent, so the +/// Linux + macOS coverage is enough until the materialise gap closes. +/// +/// [`ImporterDepVersion::File`]: pacquet_lockfile::ImporterDepVersion::File +#[test] +#[cfg_attr(target_os = "windows", ignore = "file: materialisation not ported")] +fn injected_workspace_dep_with_dedupe_off_writes_file_arm() { + let CommandTempCwd { pacquet, root, workspace, npmrc_info, .. } = + CommandTempCwd::init().add_mocked_registry(); + let AddMockedRegistry { mock_instance, .. } = npmrc_info; + + fs::write( + workspace.join("package.json"), + serde_json::json!({ "name": "ws-root", "version": "0.0.0", "private": true }).to_string(), + ) + .expect("write root package.json"); + + let workspace_yaml_path = workspace.join("pnpm-workspace.yaml"); + let mut workspace_yaml = + fs::read_to_string(&workspace_yaml_path).expect("read pnpm-workspace.yaml"); + if !workspace_yaml.ends_with('\n') { + workspace_yaml.push('\n'); + } + workspace_yaml.push_str("packages:\n - 'packages/*'\ndedupeInjectedDeps: false\n"); + fs::write(&workspace_yaml_path, workspace_yaml).expect("write pnpm-workspace.yaml"); + + fs::create_dir_all(workspace.join("packages/a")).expect("mkdir packages/a"); + fs::write( + workspace.join("packages/a/package.json"), + serde_json::json!({ + "name": "a", + "version": "1.0.0", + "dependencies": { "b": "workspace:*" }, + "dependenciesMeta": { "b": { "injected": true } }, + }) + .to_string(), + ) + .expect("write packages/a/package.json"); + + fs::create_dir_all(workspace.join("packages/b")).expect("mkdir packages/b"); + fs::write( + workspace.join("packages/b/package.json"), + serde_json::json!({ "name": "b", "version": "1.0.0" }).to_string(), + ) + .expect("write packages/b/package.json"); + + pacquet.with_arg("install").assert().success(); + + let lockfile_path = workspace.join("pnpm-lock.yaml"); + let lockfile = fs::read_to_string(&lockfile_path).expect("read pnpm-lock.yaml"); + assert!( + lockfile.contains("file:packages/b"), + "pnpm-lock.yaml should retain the file:packages/b entry when dedupe is disabled:\n{lockfile}", + ); + assert!( + !lockfile.contains("link:../b"), + "pnpm-lock.yaml should not rewrite the injected dep to link:../b when dedupe is disabled:\n{lockfile}", + ); + + drop((root, mock_instance)); +} diff --git a/pacquet/crates/cli/tests/inject_workspace_packages.rs b/pacquet/crates/cli/tests/inject_workspace_packages.rs index c765c72bea..f2ee667a21 100644 --- a/pacquet/crates/cli/tests/inject_workspace_packages.rs +++ b/pacquet/crates/cli/tests/inject_workspace_packages.rs @@ -280,6 +280,13 @@ fn dependencies_meta_injected_per_dep_overrides_global_off() { // Explicit `false` so a contributor flipping the global default // can't accidentally make this test pass for the wrong reason. workspace_yaml.push_str("injectWorkspacePackages: false\n"); + // Isolate the resolver-output assertion below from the + // `dedupeInjectedDeps` pass that rewrites an injected workspace + // dep back to `link:` when the target's children are a subset of + // the injected snapshot's. With project-1 being a childless leaf + // and dedupe enabled, the file: resolution this test asserts on + // would otherwise collapse to `link:../project-1`. + workspace_yaml.push_str("dedupeInjectedDeps: false\n"); workspace_yaml.push_str("packages:\n - 'project-*'\n"); fs::write(&workspace_yaml_path, workspace_yaml).expect("write pnpm-workspace.yaml"); diff --git a/pacquet/crates/config/src/env_overlay.rs b/pacquet/crates/config/src/env_overlay.rs index b170b9b1ef..9a2c4007ac 100644 --- a/pacquet/crates/config/src/env_overlay.rs +++ b/pacquet/crates/config/src/env_overlay.rs @@ -147,6 +147,7 @@ impl WorkspaceSettings { json_field!(dedupe_peers, "DEDUPE_PEERS"); json_field!(dedupe_direct_deps, "DEDUPE_DIRECT_DEPS"); json_field!(prefer_workspace_packages, "PREFER_WORKSPACE_PACKAGES"); + json_field!(dedupe_injected_deps, "DEDUPE_INJECTED_DEPS"); json_field!(strict_peer_dependencies, "STRICT_PEER_DEPENDENCIES"); json_field!(resolve_peers_from_workspace_root, "RESOLVE_PEERS_FROM_WORKSPACE_ROOT"); json_field!(block_exotic_subdeps, "BLOCK_EXOTIC_SUBDEPS"); diff --git a/pacquet/crates/config/src/lib.rs b/pacquet/crates/config/src/lib.rs index 3cfb3f3168..555902422d 100644 --- a/pacquet/crates/config/src/lib.rs +++ b/pacquet/crates/config/src/lib.rs @@ -668,6 +668,16 @@ pub struct Config { #[default = true] pub dedupe_direct_deps: bool, + /// When `true`, injected workspace dependencies whose materialised + /// children turn out to be a subset of the target workspace + /// project's own direct dependencies get rewritten back to + /// symlinks. Mirrors pnpm's + /// [`dedupeInjectedDeps`](https://github.com/pnpm/pnpm/blob/39101f5e37/installing/deps-resolver/src/dedupeInjectedDeps.ts). + /// Default `true` to match pnpm's + /// [`extendInstallOptions`](https://github.com/pnpm/pnpm/blob/39101f5e37/installing/deps-installer/src/install/extendInstallOptions.ts#L282). + #[default = true] + pub dedupe_injected_deps: bool, + /// If this is enabled, commands will fail if there is a missing or invalid peer dependency in the tree. pub strict_peer_dependencies: bool, diff --git a/pacquet/crates/config/src/workspace_yaml.rs b/pacquet/crates/config/src/workspace_yaml.rs index fe99046469..9e304b71ae 100644 --- a/pacquet/crates/config/src/workspace_yaml.rs +++ b/pacquet/crates/config/src/workspace_yaml.rs @@ -164,6 +164,7 @@ pub struct WorkspaceSettings { pub dedupe_peers: Option, pub dedupe_direct_deps: Option, pub prefer_workspace_packages: Option, + pub dedupe_injected_deps: Option, pub strict_peer_dependencies: Option, pub resolve_peers_from_workspace_root: Option, pub block_exotic_subdeps: Option, @@ -477,6 +478,7 @@ impl WorkspaceSettings { self.dedupe_peers = None; self.dedupe_direct_deps = None; self.prefer_workspace_packages = None; + self.dedupe_injected_deps = None; self.strict_peer_dependencies = None; self.resolve_peers_from_workspace_root = None; self.block_exotic_subdeps = None; @@ -572,7 +574,8 @@ impl WorkspaceSettings { optimistic_repeat_install, hoist_workspace_packages, hoisting_limits, external_dependencies, - dedupe_peer_dependents, dedupe_peers, dedupe_direct_deps, strict_peer_dependencies, + dedupe_peer_dependents, dedupe_peers, dedupe_direct_deps, dedupe_injected_deps, + strict_peer_dependencies, resolve_peers_from_workspace_root, verify_store_integrity, block_exotic_subdeps, link_workspace_packages, diff --git a/pacquet/crates/lockfile/src/resolved_dependency.rs b/pacquet/crates/lockfile/src/resolved_dependency.rs index 3c2d924775..d8cb06ec8e 100644 --- a/pacquet/crates/lockfile/src/resolved_dependency.rs +++ b/pacquet/crates/lockfile/src/resolved_dependency.rs @@ -41,6 +41,12 @@ pub struct ResolvedDependencySpec { /// sibling at `` relative to the importer's `rootDir`. The /// workspace project is not duplicated in the virtual store — pnpm /// creates a direct symlink to the sibling's directory. +/// - A `file:` value (with an optional `(peer@suffix)` tail), +/// meaning the dependency is an injected workspace package (or +/// plain tarball / directory `file:` dep) materialised into the +/// virtual store by copy. The path lives verbatim after the +/// `file:` prefix; the peer suffix, when present, identifies a +/// peer-specific snapshot variant in `snapshots:`. /// /// `ImporterDepVersion` encodes the distinction so consumers (the /// installer, the build-sequence builder, the reporter) can branch on @@ -69,6 +75,14 @@ pub enum ImporterDepVersion { /// importer's `rootDir`, or absolute) — interpreting it is the /// installer's job, not this layer's. Link(String), + + /// `file:` (possibly with a `(peer@suffix)` tail). Mirrors + /// upstream's importer-level emit for injected workspace + /// dependencies that didn't dedupe back to `link:`. The full + /// payload after `file:` is stored verbatim so the + /// `snapshots:`-level depPath lookup uses the same key as the + /// snapshot writer. + File(String), } impl ImporterDepVersion { @@ -84,7 +98,9 @@ impl ImporterDepVersion { pub fn as_regular(&self) -> Option<&'_ PkgVerPeer> { match self { ImporterDepVersion::Regular(v) => Some(v), - ImporterDepVersion::Alias(_) | ImporterDepVersion::Link(_) => None, + ImporterDepVersion::Alias(_) + | ImporterDepVersion::Link(_) + | ImporterDepVersion::File(_) => None, } } @@ -94,7 +110,9 @@ impl ImporterDepVersion { pub fn as_alias(&self) -> Option<&'_ PkgNameVerPeer> { match self { ImporterDepVersion::Alias(alias) => Some(alias), - ImporterDepVersion::Regular(_) | ImporterDepVersion::Link(_) => None, + ImporterDepVersion::Regular(_) + | ImporterDepVersion::Link(_) + | ImporterDepVersion::File(_) => None, } } @@ -104,36 +122,57 @@ impl ImporterDepVersion { /// prefix. pub fn as_link_target(&self) -> Option<&'_ str> { match self { - ImporterDepVersion::Regular(_) | ImporterDepVersion::Alias(_) => None, + ImporterDepVersion::Regular(_) + | ImporterDepVersion::Alias(_) + | ImporterDepVersion::File(_) => None, ImporterDepVersion::Link(target) => Some(target.as_str()), } } + /// `Some(payload)` when this dependency is an injected `file:` dep; + /// `None` otherwise. The returned string is the path (plus optional + /// peer suffix) *without* the `file:` prefix. + pub fn as_file_target(&self) -> Option<&'_ str> { + match self { + ImporterDepVersion::File(target) => Some(target.as_str()), + ImporterDepVersion::Regular(_) + | ImporterDepVersion::Alias(_) + | ImporterDepVersion::Link(_) => None, + } + } + /// `Some(key)` with the snapshot-map key this dependency resolves /// to; `None` for `link:` siblings. `importer_key` is the key of /// the entry in `importers..dependencies` (the directory name /// inside `node_modules`). For [`Self::Regular`] the resolved key /// is `(importer_key, version)`; for [`Self::Alias`] it's the /// alias's own `(name, suffix)` pair, mirroring upstream's - /// `refToRelative`. + /// `refToRelative`. For [`Self::File`] the key is `(importer_key, + /// file:)` because the `file:` prefix is part of the + /// snapshot key in `snapshots:`. pub fn resolved_key(&self, importer_key: &PkgName) -> Option { match self { ImporterDepVersion::Regular(ver) => { Some(PkgNameVerPeer::new(importer_key.clone(), ver.clone())) } ImporterDepVersion::Alias(alias) => Some(alias.clone()), + ImporterDepVersion::File(payload) => format!("file:{payload}") + .parse::() + .ok() + .map(|ver| PkgNameVerPeer::new(importer_key.clone(), ver)), ImporterDepVersion::Link(_) => None, } } /// The version-with-peer portion of this dependency, or `None` for - /// `link:` siblings. For [`Self::Alias`] this returns the alias's - /// suffix, matching the version present in the snapshot key. + /// `link:` / `file:` siblings. For [`Self::Alias`] this returns the + /// alias's suffix, matching the version present in the snapshot + /// key. pub fn ver_peer(&self) -> Option<&'_ PkgVerPeer> { match self { ImporterDepVersion::Regular(ver) => Some(ver), ImporterDepVersion::Alias(alias) => Some(&alias.suffix), - ImporterDepVersion::Link(_) => None, + ImporterDepVersion::Link(_) | ImporterDepVersion::File(_) => None, } } } @@ -187,6 +226,9 @@ impl FromStr for ImporterDepVersion { if let Some(target) = value.strip_prefix("link:") { return Ok(ImporterDepVersion::Link(target.to_string())); } + if let Some(target) = value.strip_prefix("file:") { + return Ok(ImporterDepVersion::File(target.to_string())); + } if looks_like_alias(value) { return value.parse::().map(ImporterDepVersion::Alias).map_err( |source| ParseImporterDepVersionError::ParseAlias { @@ -214,6 +256,7 @@ impl From for String { ImporterDepVersion::Regular(v) => v.to_string(), ImporterDepVersion::Alias(alias) => alias.to_string(), ImporterDepVersion::Link(target) => format!("link:{target}"), + ImporterDepVersion::File(target) => format!("file:{target}"), } } } @@ -230,6 +273,10 @@ impl Serialize for ImporterDepVersion { let formatted = format!("link:{target}"); serializer.serialize_str(&formatted) } + ImporterDepVersion::File(target) => { + let formatted = format!("file:{target}"); + serializer.serialize_str(&formatted) + } } } } @@ -262,6 +309,7 @@ impl Display for ImporterDepVersion { ImporterDepVersion::Regular(v) => Display::fmt(v, f), ImporterDepVersion::Alias(alias) => Display::fmt(alias, f), ImporterDepVersion::Link(target) => write!(f, "link:{target}"), + ImporterDepVersion::File(target) => write!(f, "file:{target}"), } } } diff --git a/pacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rs b/pacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rs index 232d30b24c..86d62191ed 100644 --- a/pacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rs +++ b/pacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rs @@ -288,6 +288,13 @@ fn read_manifest_specifier(manifest: &PackageManifest, alias: &str) -> Option` +/// plus optional `(peer@suffix)`), emit the +/// [`ImporterDepVersion::File`] arm so the importer entry records +/// the `file:` snapshot key instead of trying to parse it as +/// `name@version`. The injected workspace dep didn't dedupe back to +/// `link:` because its children weren't a subset of the target +/// project's direct deps (or `dedupeInjectedDeps` is off). /// - When the resolved real name equals the manifest alias and the /// depPath starts with `@`, drop the prefix so the importer /// carries just the version-with-peer string. @@ -300,6 +307,9 @@ fn importer_dep_version(alias: &str, node: &DependenciesGraphNode) -> ImporterDe if let Some(target) = dep_path_str.strip_prefix("link:") { return ImporterDepVersion::Link(target.to_string()); } + if let Some(target) = dep_path_str.strip_prefix("file:") { + return ImporterDepVersion::File(target.to_string()); + } let real_name = real_name(&node.resolve_result); if let Some(real) = real_name.as_deref() { diff --git a/pacquet/crates/package-manager/src/install_package_from_registry/tests.rs b/pacquet/crates/package-manager/src/install_package_from_registry/tests.rs index 2fef89ee01..554c187ae5 100644 --- a/pacquet/crates/package-manager/src/install_package_from_registry/tests.rs +++ b/pacquet/crates/package-manager/src/install_package_from_registry/tests.rs @@ -57,6 +57,7 @@ fn create_config(store_dir: &Path, modules_dir: &Path, virtual_store_dir: &Path) dedupe_peer_dependents: false, dedupe_peers: false, dedupe_direct_deps: true, + dedupe_injected_deps: false, strict_peer_dependencies: false, resolve_peers_from_workspace_root: false, block_exotic_subdeps: false, diff --git a/pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs b/pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs index 6b9ae153d9..ecba6265f2 100644 --- a/pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs +++ b/pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs @@ -21,7 +21,7 @@ use pacquet_package_manifest::{DependencyGroup, PackageManifest}; use pacquet_reporter::{LogEvent, LogLevel, Reporter, Stage, StageLog}; use pacquet_resolving_default_resolver::DefaultResolver; use pacquet_resolving_deps_resolver::{ - ResolveDependencyTreeError, ResolveImporterError, ResolveImporterOptions, resolve_importer, + ResolveDependencyTreeError, ResolveImporterError, ResolveImporterOptions, }; use pacquet_resolving_git_resolver::{GitResolver, RealGitProbe, RealGitRunner}; use pacquet_resolving_local_resolver::{ @@ -65,8 +65,9 @@ pub type ResolvedPackages = DashMap>; /// drives this path whenever no `--frozen-lockfile` was requested. /// /// **Brief overview for each package:** -/// * Resolve the dependency through the [`NpmResolver`] chain -/// ([`resolve_importer`] builds the full tree and hoists peers first). +/// * Resolve every importer's dependency through the [`NpmResolver`] chain +/// (`resolve_workspace` builds the per-importer trees, runs the +/// cross-importer peer pass, and applies `dedupeInjectedDeps`). /// * Fetch a tarball of each resolved package and extract it into the /// store directory. /// * Import (by reflink, hardlink, or copy) the files from the store @@ -582,100 +583,91 @@ impl<'a, DependencyGroupList> InstallWithFreshLockfile<'a, DependencyGroupList> // iteration: one shared resolution context, per-importer // direct-deps slices. let phase_start = std::time::Instant::now(); - let mut merged_graph: pacquet_resolving_deps_resolver::DependenciesGraph = - std::collections::HashMap::new(); - let mut direct_by_importer: BTreeMap< - String, - BTreeMap, - > = BTreeMap::new(); - let mut total_nodes = 0usize; + let workspace_importers: Vec> = + importer_manifests + .iter() + .map(|(id, manifest)| pacquet_resolving_deps_resolver::WorkspaceImporter { + id: id.clone(), + manifest, + }) + .collect(); + let peers_suffix_max_length = + usize::try_from(config.peers_suffix_max_length).unwrap_or(usize::MAX); + let workspace_opts = pacquet_resolving_deps_resolver::WorkspaceResolveOptions { + dedupe_peers: config.dedupe_peers, + dedupe_injected_deps: config.dedupe_injected_deps, + exclude_links_from_lockfile: config.exclude_links_from_lockfile, + lockfile_dir: lockfile_dir.to_path_buf(), + peers_suffix_max_length, + manifest_hook: package_extensions_hook.clone(), + }; let modules_basename = config .modules_dir .file_name() .map(std::ffi::OsStr::to_os_string) .unwrap_or_else(|| std::ffi::OsString::from("node_modules")); - for (importer_id, importer_manifest) in &importer_manifests { - let project_dir = importer_manifest - .path() - .parent() - .expect("manifest path always has a parent dir") - .to_path_buf(); - let importer_modules_dir = project_dir.join(&modules_basename); - let importer_opts = ResolveImporterOptions { - auto_install_peers: config.auto_install_peers, - auto_install_peers_from_highest_match: config.auto_install_peers_from_highest_match, - resolve_peers_from_workspace_root: config.resolve_peers_from_workspace_root, - dedupe_peers: config.dedupe_peers, - all_preferred_versions: all_preferred_versions.clone(), - patched_dependencies: patched_dependencies.clone(), - base_opts: ResolveOptions { - default_tag: Some("latest".to_string()), - published_by, - published_by_exclude: published_by_exclude.clone(), - project_dir, - lockfile_dir: lockfile_dir.to_path_buf(), - workspace_packages: workspace_packages.clone(), - block_exotic_subdeps: config.block_exotic_subdeps, - // Pacquet's deps-resolver doesn't thread per-call - // depth into the resolver yet, so the - // `linkWorkspacePackages: true` (direct-only) and - // `"deep"` arms both collapse onto a flag that - // applies at every depth. Matches pnpm's behavior - // for `"deep"`; the (rare) `true`-only case may - // resolve a transitive registry dep through the - // workspace when the names collide. - always_try_workspace_packages: config.link_workspace_packages - != LinkWorkspacePackages::Off, - inject_workspace_packages: config.inject_workspace_packages, - prefer_workspace_packages: config.prefer_workspace_packages, - update_checksums, - ..ResolveOptions::default() - }, - catalogs: catalogs.clone(), - exclude_links_from_lockfile: config.exclude_links_from_lockfile, - lockfile_dir: Some(lockfile_dir.to_path_buf()), - modules_dir: Some(importer_modules_dir), - peers_suffix_max_length: usize::try_from(config.peers_suffix_max_length) - .unwrap_or(usize::MAX), - manifest_hook: package_extensions_hook.clone(), - }; - let importer_result = resolve_importer( - &*resolver, - importer_manifest, - dependency_groups.iter().copied(), - importer_opts, - ) - .await - .map_err(InstallWithFreshLockfileError::ResolveImporter)?; - total_nodes += importer_result.peers_result.graph.len(); - // Merge this importer's per-id graph into the shared - // graph; identical depPaths collapse onto the existing - // entry. Two importers depending on the same package - // version produce equivalent nodes by construction of the - // (deterministic) resolver, so a first-writer-wins merge - // is safe. - for (key, node) in importer_result.peers_result.graph { - merged_graph.entry(key).or_insert(node); - } - direct_by_importer.insert( - importer_id.clone(), - importer_result.peers_result.direct_dependencies_by_alias, + let workspace_result = pacquet_resolving_deps_resolver::resolve_workspace( + &*resolver, + &workspace_importers, + &dependency_groups, + workspace_opts, + |importer| { + let project_dir = importer + .manifest + .path() + .parent() + .expect("manifest path always has a parent dir") + .to_path_buf(); + let importer_modules_dir = project_dir.join(&modules_basename); + ResolveImporterOptions { + auto_install_peers: config.auto_install_peers, + auto_install_peers_from_highest_match: config + .auto_install_peers_from_highest_match, + resolve_peers_from_workspace_root: config.resolve_peers_from_workspace_root, + dedupe_peers: config.dedupe_peers, + all_preferred_versions: all_preferred_versions.clone(), + patched_dependencies: patched_dependencies.clone(), + base_opts: ResolveOptions { + default_tag: Some("latest".to_string()), + published_by, + published_by_exclude: published_by_exclude.clone(), + project_dir, + lockfile_dir: lockfile_dir.to_path_buf(), + workspace_packages: workspace_packages.clone(), + block_exotic_subdeps: config.block_exotic_subdeps, + always_try_workspace_packages: config.link_workspace_packages + != LinkWorkspacePackages::Off, + inject_workspace_packages: config.inject_workspace_packages, + prefer_workspace_packages: config.prefer_workspace_packages, + update_checksums, + ..ResolveOptions::default() + }, + catalogs: catalogs.clone(), + exclude_links_from_lockfile: config.exclude_links_from_lockfile, + lockfile_dir: Some(lockfile_dir.to_path_buf()), + modules_dir: Some(importer_modules_dir), + peers_suffix_max_length, + manifest_hook: package_extensions_hook.clone(), + } + }, + ) + .await + .map_err(InstallWithFreshLockfileError::ResolveImporter)?; + let total_nodes = workspace_result.peers.graph.len(); + for (importer_id, issues) in &workspace_result.peers.peer_dependency_issues_by_importer { + tracing::warn!( + target: "pacquet::install", + importer_id = %importer_id, + missing = issues.missing.len(), + bad = issues.bad.len(), + "Peer dependency issues detected (issue renderer not ported yet)", ); - if !importer_result.peers_result.peer_dependency_issues.missing.is_empty() - || !importer_result.peers_result.peer_dependency_issues.bad.is_empty() - { - tracing::warn!( - target: "pacquet::install", - importer_id = %importer_id, - missing = importer_result.peers_result.peer_dependency_issues.missing.len(), - bad = importer_result.peers_result.peer_dependency_issues.bad.len(), - "Peer dependency issues detected (issue renderer not ported yet)", - ); - } } + let merged_graph = workspace_result.peers.graph; + let direct_by_importer = workspace_result.peers.direct_dependencies_by_importer; tracing::info!( target: "pacquet::install::phase", - phase = "resolve_importer", + phase = "resolve_workspace", elapsed_ms = phase_start.elapsed().as_millis() as u64, importers = importer_manifests.len(), nodes = total_nodes, diff --git a/pacquet/crates/package-manager/src/optimistic_repeat_install.rs b/pacquet/crates/package-manager/src/optimistic_repeat_install.rs index 74bcc8d845..a1e17e1264 100644 --- a/pacquet/crates/package-manager/src/optimistic_repeat_install.rs +++ b/pacquet/crates/package-manager/src/optimistic_repeat_install.rs @@ -198,6 +198,7 @@ fn settings_match( allow_builds_match(recorded.allow_builds.as_ref(), live.allow_builds.as_ref()) && recorded.auto_install_peers == live.auto_install_peers && recorded.dedupe_direct_deps == live.dedupe_direct_deps + && recorded.dedupe_injected_deps == live.dedupe_injected_deps && recorded.dedupe_peer_dependents == live.dedupe_peer_dependents && recorded.dedupe_peers == live.dedupe_peers && recorded.dev == live.dev @@ -222,7 +223,6 @@ fn settings_match( // each from this list once `current_settings` writes its value): // catalogs (pnpm always ignores; see // ignoredSettings.add('catalogs')) - // dedupeInjectedDeps // excludeLinksFromLockfile // minimumReleaseAge* (pacquet supports it but doesn't // round-trip through workspace state @@ -286,6 +286,7 @@ pub(crate) fn current_settings( allow_builds, auto_install_peers: Some(config.auto_install_peers), dedupe_direct_deps: Some(config.dedupe_direct_deps), + dedupe_injected_deps: Some(config.dedupe_injected_deps), dedupe_peer_dependents: Some(config.dedupe_peer_dependents), dedupe_peers: Some(config.dedupe_peers), dev: Some(included.dev_dependencies), diff --git a/pacquet/crates/package-manager/src/optimistic_repeat_install/tests.rs b/pacquet/crates/package-manager/src/optimistic_repeat_install/tests.rs index 22e9950982..0b274a5c19 100644 --- a/pacquet/crates/package-manager/src/optimistic_repeat_install/tests.rs +++ b/pacquet/crates/package-manager/src/optimistic_repeat_install/tests.rs @@ -724,7 +724,6 @@ fn returns_up_to_date_when_state_carries_unported_pnpm_settings() { // Populate every field pacquet doesn't surface through // `current_settings` today. Each is an upstream pnpm setting // listed in pnpm/pnpm#12009. - settings.dedupe_injected_deps = Some(true); settings.exclude_links_from_lockfile = Some(false); // `catalogs` is always ignored by pnpm itself; pacquet // mirrors that. diff --git a/pacquet/crates/package-manager/src/symlink_direct_dependencies.rs b/pacquet/crates/package-manager/src/symlink_direct_dependencies.rs index d23bd53550..dbe1090c06 100644 --- a/pacquet/crates/package-manager/src/symlink_direct_dependencies.rs +++ b/pacquet/crates/package-manager/src/symlink_direct_dependencies.rs @@ -473,13 +473,16 @@ fn link_one_importer( ImporterDepVersion::Regular(ver) => Some(ver.version().to_string()), ImporterDepVersion::Alias(alias) => Some(alias.suffix.version().to_string()), ImporterDepVersion::Link(target) => Some(format!("link:{target}")), + ImporterDepVersion::File(target) => Some(format!("file:{target}")), }; // For aliases, `real_name` is the resolved package's true - // name (different from the importer-map key). For - // `Regular` and `Link` deps, the two match. + // name (different from the importer-map key). For the + // other arms the two match. let real_name = match &spec.version { ImporterDepVersion::Alias(alias) => alias.name.to_string(), - ImporterDepVersion::Regular(_) | ImporterDepVersion::Link(_) => name.to_string(), + ImporterDepVersion::Regular(_) + | ImporterDepVersion::Link(_) + | ImporterDepVersion::File(_) => name.to_string(), }; Reporter::emit(&LogEvent::Root(RootLog { level: LogLevel::Debug, @@ -668,6 +671,16 @@ fn resolve_target_path( }; pacquet_fs::lexical_normalize(&joined) } + ImporterDepVersion::File(_) => { + // Injected workspace dep that didn't dedupe back to + // `link:` — the importer entry references a virtual-store + // slot keyed by `(importer_key, file:)`. Route + // through `resolved_key` so the layout's GVS-suffix map + // sees the same key the snapshot writer used. + let dep_key = + spec.version.resolved_key(name).expect("File arm always produces a resolved_key"); + layout.slot_dir(&dep_key).join("node_modules").join(name_str) + } } } diff --git a/pacquet/crates/resolving-deps-resolver/src/dedupe_injected_deps.rs b/pacquet/crates/resolving-deps-resolver/src/dedupe_injected_deps.rs new file mode 100644 index 0000000000..566d8ec241 --- /dev/null +++ b/pacquet/crates/resolving-deps-resolver/src/dedupe_injected_deps.rs @@ -0,0 +1,351 @@ +//! Port of pnpm's +//! [`dedupeInjectedDeps`](https://github.com/pnpm/pnpm/blob/39101f5e37/installing/deps-resolver/src/dedupeInjectedDeps.ts). +//! +//! Runs at the tail of the multi-importer [`fn@crate::resolve_peers`] +//! pass. Each importer's direct deps map is scanned for entries whose +//! resolved id starts with `file:` and points at another workspace +//! project; if that injected snapshot's children turn out to be a +//! subset of the target project's own direct deps, the importer's +//! entry is rewritten to `link:` so the install pass +//! materializes it as a symlink instead of a copy. Snapshots that +//! become unreachable after the rewrite are pruned from the graph so +//! they don't surface in the lockfile. +//! +//! Pacquet's lockfile importer-version writer does not yet support +//! `file:` direct-dep entries — when an importer's +//! `direct_dependencies_by_alias` still carries an injected workspace +//! depPath after this pass, the lockfile writer panics in +//! `importer_dep_version`. In practice this pass is what keeps the +//! install from hitting that panic on every `dependenciesMeta.injected` +//! workspace edge; with `dedupeInjectedDeps: false`, an injected +//! workspace dep whose children don't subset still trips the writer. + +use std::{ + collections::{BTreeMap, HashSet}, + path::{Path, PathBuf}, +}; + +use pacquet_deps_path::DepPath; + +use crate::dependencies_graph::DependenciesGraph; + +/// Per-importer direct deps map keyed by lockfile importer id (`"."` +/// for the root, POSIX-relative path for siblings). +pub type DirectByImporter = BTreeMap>; + +/// Dedupe injected workspace deps across importers, mutating both +/// `direct_by_importer` (rewriting `file:` direct deps to `link:`) +/// and `graph` (dropping snapshots no importer reaches). +/// +/// `importer_root_dirs` maps each importer id (`"."` for the root, +/// POSIX-relative for siblings) to its absolute project dir; the +/// caller already has this from the resolve loop. +pub fn dedupe_injected_deps( + graph: &mut DependenciesGraph, + direct_by_importer: &mut DirectByImporter, + importer_root_dirs: &BTreeMap, + lockfile_dir: &Path, +) { + let workspace_project_ids: HashSet = importer_root_dirs.keys().cloned().collect(); + let dedupe_map = build_dedupe_map(graph, direct_by_importer, &workspace_project_ids); + if dedupe_map.is_empty() { + return; + } + apply_dedupe_map(&dedupe_map, direct_by_importer, importer_root_dirs, lockfile_dir); + prune_unreachable(graph, direct_by_importer); +} + +/// `importer_id → alias → target_project_id` — the per-importer set of +/// aliases that get rewritten, with the workspace project they redirect +/// to. +type DedupeMap = BTreeMap>; + +fn build_dedupe_map( + graph: &DependenciesGraph, + direct_by_importer: &DirectByImporter, + workspace_project_ids: &HashSet, +) -> DedupeMap { + let mut dedupe_map: DedupeMap = BTreeMap::new(); + for (importer_id, direct) in direct_by_importer { + let mut deduped: BTreeMap = BTreeMap::new(); + for (alias, dep_path) in direct { + let Some(node) = graph.get(dep_path) else { continue }; + let Some(target_project_id) = injected_workspace_target(node, workspace_project_ids) + else { + continue; + }; + // The injected dep can be rewritten to a symlink only when + // every one of its children is also present (as the same + // depPath) on the target project's own direct deps. The + // empty set is trivially a subset. + let target_direct = direct_by_importer.get(&target_project_id); + let children_match = node.children.iter().all(|(child_alias, child_dep_path)| { + target_direct.and_then(|map| map.get(child_alias)) == Some(child_dep_path) + }); + if !children_match { + continue; + } + deduped.insert(alias.clone(), target_project_id); + } + if !deduped.is_empty() { + dedupe_map.insert(importer_id.clone(), deduped); + } + } + dedupe_map +} + +/// Return the workspace project id (lockfile importer key) this node +/// is an injected reference to, or `None` if it isn't a `file:` +/// pointing at a known workspace project. +/// +/// The resolver's `pkg.id` for a `file:` workspace pick is +/// emitted as `@file:` once the manifest name is in scope +/// (see `build_pkg_id_with_patch_hash`) and as the bare `file:` +/// before that — accept both shapes. +fn injected_workspace_target( + node: &crate::dependencies_graph::DependenciesGraphNode, + workspace_project_ids: &HashSet, +) -> Option { + let raw = node.resolved_package_id.as_str(); + let path = raw.strip_prefix("file:").or_else(|| { + let after_at = raw.split_once('@').map(|(_, rest)| rest)?; + after_at.strip_prefix("file:") + })?; + workspace_project_ids.contains(path).then(|| path.to_string()) +} + +fn apply_dedupe_map( + dedupe_map: &DedupeMap, + direct_by_importer: &mut DirectByImporter, + importer_root_dirs: &BTreeMap, + lockfile_dir: &Path, +) { + for (importer_id, aliases) in dedupe_map { + let Some(source_root) = importer_root_dirs.get(importer_id) else { continue }; + let Some(direct) = direct_by_importer.get_mut(importer_id) else { continue }; + for (alias, target_project_id) in aliases { + let Some(target_root) = importer_root_dirs.get(target_project_id) else { continue }; + let link_dep_path = make_link_dep_path(source_root, target_root, lockfile_dir); + direct.insert(alias.clone(), link_dep_path); + } + } +} + +/// Build the `link:` depPath payload, mirroring pnpm's +/// [`link:${normalize(path.relative(id, dedupedProjectId))}`](https://github.com/pnpm/pnpm/blob/39101f5e37/installing/deps-resolver/src/dedupeInjectedDeps.ts#L98). +/// `id` and `dedupedProjectId` upstream are lockfile importer keys +/// (POSIX-relative paths from the lockfile dir), which pacquet stores +/// as absolute project dirs; resolving the relative path through the +/// filesystem layer keeps Windows path separators correct. +fn make_link_dep_path(source_root: &Path, target_root: &Path, lockfile_dir: &Path) -> DepPath { + let rel = + pathdiff::diff_paths(target_root, source_root).unwrap_or_else(|| target_root.to_path_buf()); + let rendered = rel.to_string_lossy().replace('\\', "/"); + let rel_posix = if rendered.is_empty() || rendered == "." { + // Source and target match: emit a path relative to the lockfile + // dir so the resulting symlink still resolves to a real + // directory. In practice this branch is unreachable — + // `injected_workspace_target` only matches when the importer's + // direct dep points at a different workspace project — but the + // fallback keeps the helper total. + let fallback = pathdiff::diff_paths(target_root, lockfile_dir) + .unwrap_or_else(|| target_root.to_path_buf()); + fallback.to_string_lossy().replace('\\', "/") + } else { + rendered + }; + DepPath::from(format!("link:{rel_posix}")) +} + +/// Remove graph nodes no importer reaches after dedupe, mirroring +/// pnpm's [`pruneSharedLockfile`](https://github.com/pnpm/pnpm/blob/39101f5e37/lockfile/pruner/src/index.ts). +/// The deduped `file:` snapshot would otherwise remain in +/// `merged_graph` and surface in `pnpm-lock.yaml` as an orphan. +fn prune_unreachable(graph: &mut DependenciesGraph, direct_by_importer: &DirectByImporter) { + let mut reachable: HashSet = HashSet::new(); + let mut stack: Vec = + direct_by_importer.values().flat_map(|direct| direct.values().cloned()).collect(); + while let Some(dep_path) = stack.pop() { + if !reachable.insert(dep_path.clone()) { + continue; + } + let Some(node) = graph.get(&dep_path) else { continue }; + for child in node.children.values() { + if !reachable.contains(child) { + stack.push(child.clone()); + } + } + } + graph.retain(|key, _| reachable.contains(key)); +} + +#[cfg(test)] +mod tests { + use super::{DirectByImporter, dedupe_injected_deps}; + use crate::dependencies_graph::{DependenciesGraph, DependenciesGraphNode}; + use pacquet_deps_path::DepPath; + use pacquet_lockfile::{DirectoryResolution, LockfileResolution}; + use pacquet_resolving_resolver_base::{PkgResolutionId, ResolveResult}; + use std::collections::{BTreeMap, HashSet}; + use std::path::PathBuf; + + fn make_node(id: &str, children: BTreeMap) -> DependenciesGraphNode { + DependenciesGraphNode { + dep_path: DepPath::from(id.to_string()), + resolved_package_id: id.to_string(), + resolve_result: std::sync::Arc::new(ResolveResult { + id: PkgResolutionId::from(id.to_string()), + name_ver: None, + latest: None, + published_at: None, + manifest: None, + resolution: LockfileResolution::Directory(DirectoryResolution { + directory: "stub".to_string(), + }), + resolved_via: "workspace".to_string(), + normalized_bare_specifier: None, + alias: None, + policy_violation: None, + }), + children, + peer_dependencies: BTreeMap::new(), + transitive_peer_dependencies: HashSet::new(), + resolved_peer_names: HashSet::new(), + depth: 0, + installable: true, + is_pure: true, + optional: false, + } + } + + /// Two-project workspace: `project-2` injects `project-1`. The + /// injected snapshot has no children, so the subset check trivially + /// holds and the importer entry rewrites to a `link:`, with the + /// orphaned `file:` snapshot pruned from the graph. + #[test] + fn rewrites_childless_injected_dep_to_link() { + let lockfile_dir = PathBuf::from("/ws"); + let p1_root = lockfile_dir.join("project-1"); + let p2_root = lockfile_dir.join("project-2"); + + let mut graph: DependenciesGraph = std::collections::HashMap::new(); + let file_path = DepPath::from("file:project-1".to_string()); + graph.insert(file_path.clone(), make_node("file:project-1", BTreeMap::new())); + + let mut direct: DirectByImporter = BTreeMap::new(); + direct.insert("project-1".to_string(), BTreeMap::new()); + direct.insert( + "project-2".to_string(), + BTreeMap::from([("project-1".to_string(), file_path)]), + ); + + let mut roots = BTreeMap::new(); + roots.insert("project-1".to_string(), p1_root); + roots.insert("project-2".to_string(), p2_root); + + dedupe_injected_deps(&mut graph, &mut direct, &roots, &lockfile_dir); + + let after = direct.get("project-2").unwrap().get("project-1").unwrap(); + assert_eq!(after.as_str(), "link:../project-1"); + assert!(graph.is_empty(), "unreachable file: snapshot should be pruned"); + } + + /// When the injected snapshot's child set is not a subset of the + /// target project's direct deps (different version reached + /// transitively), the dedupe must bail and the `file:` direct dep + /// must remain in place. + #[test] + fn leaves_injected_dep_when_children_differ() { + let lockfile_dir = PathBuf::from("/ws"); + let p1_root = lockfile_dir.join("project-1"); + let p2_root = lockfile_dir.join("project-2"); + + let lib_v1 = DepPath::from("lib@1.0.0".to_string()); + let lib_v2 = DepPath::from("lib@2.0.0".to_string()); + + let mut graph: DependenciesGraph = std::collections::HashMap::new(); + graph.insert(lib_v1.clone(), make_node("lib@1.0.0", BTreeMap::new())); + graph.insert(lib_v2.clone(), make_node("lib@2.0.0", BTreeMap::new())); + let injected = DepPath::from("file:project-1".to_string()); + graph.insert( + injected.clone(), + make_node("file:project-1", BTreeMap::from([("lib".to_string(), lib_v2)])), + ); + + let mut direct: DirectByImporter = BTreeMap::new(); + direct.insert("project-1".to_string(), BTreeMap::from([("lib".to_string(), lib_v1)])); + direct + .insert("project-2".to_string(), BTreeMap::from([("project-1".to_string(), injected)])); + + let mut roots = BTreeMap::new(); + roots.insert("project-1".to_string(), p1_root); + roots.insert("project-2".to_string(), p2_root); + + dedupe_injected_deps(&mut graph, &mut direct, &roots, &lockfile_dir); + + let after = direct.get("project-2").unwrap().get("project-1").unwrap(); + assert_eq!(after.as_str(), "file:project-1"); + } + + /// The subset check holds when the injected snapshot's children all + /// match the target project's direct-deps map by alias and depPath + /// — the rewrite goes through even though the snapshot has + /// non-empty children. + #[test] + fn rewrites_when_children_subset_of_target_direct_deps() { + let lockfile_dir = PathBuf::from("/ws"); + let p1_root = lockfile_dir.join("project-1"); + let p2_root = lockfile_dir.join("project-2"); + + let lib = DepPath::from("lib@1.0.0".to_string()); + + let mut graph: DependenciesGraph = std::collections::HashMap::new(); + graph.insert(lib.clone(), make_node("lib@1.0.0", BTreeMap::new())); + let injected = DepPath::from("file:project-1".to_string()); + graph.insert( + injected.clone(), + make_node("file:project-1", BTreeMap::from([("lib".to_string(), lib.clone())])), + ); + + let mut direct: DirectByImporter = BTreeMap::new(); + direct.insert("project-1".to_string(), BTreeMap::from([("lib".to_string(), lib.clone())])); + direct.insert( + "project-2".to_string(), + BTreeMap::from([("project-1".to_string(), injected.clone())]), + ); + + let mut roots = BTreeMap::new(); + roots.insert("project-1".to_string(), p1_root); + roots.insert("project-2".to_string(), p2_root); + + dedupe_injected_deps(&mut graph, &mut direct, &roots, &lockfile_dir); + + let after = direct.get("project-2").unwrap().get("project-1").unwrap(); + assert_eq!(after.as_str(), "link:../project-1"); + // `lib` is still reachable through project-1's direct deps. + assert!(graph.contains_key(&lib)); + // The injected snapshot is orphaned and gets pruned. + assert!(!graph.contains_key(&injected)); + } + + /// `file:` deps that point outside the workspace (a tarball-shaped + /// local file dep, not a workspace project) must not be touched. + #[test] + fn ignores_non_workspace_file_deps() { + let lockfile_dir = PathBuf::from("/ws"); + let mut graph: DependenciesGraph = std::collections::HashMap::new(); + let tarball = DepPath::from("file:vendor/some-tarball.tgz".to_string()); + graph.insert(tarball.clone(), make_node("file:vendor/some-tarball.tgz", BTreeMap::new())); + + let mut direct: DirectByImporter = BTreeMap::new(); + direct.insert(".".to_string(), BTreeMap::from([("some-tarball".to_string(), tarball)])); + + let mut roots = BTreeMap::new(); + roots.insert(".".to_string(), lockfile_dir.clone()); + + dedupe_injected_deps(&mut graph, &mut direct, &roots, &lockfile_dir); + + let after = direct.get(".").unwrap().get("some-tarball").unwrap(); + assert_eq!(after.as_str(), "file:vendor/some-tarball.tgz"); + } +} diff --git a/pacquet/crates/resolving-deps-resolver/src/lib.rs b/pacquet/crates/resolving-deps-resolver/src/lib.rs index fb7bb0b04c..5c021599ae 100644 --- a/pacquet/crates/resolving-deps-resolver/src/lib.rs +++ b/pacquet/crates/resolving-deps-resolver/src/lib.rs @@ -2,8 +2,13 @@ //! [`@pnpm/installing.deps-resolver`](https://github.com/pnpm/pnpm/blob/097983fbca/installing/deps-resolver/src/index.ts). //! //! The public entry point for an install pass is -//! [`fn@resolve_importer`]: it owns three lower-level passes and the -//! `autoInstallPeers` hoist loop that ties them together. +//! [`fn@resolve_workspace`] — it loops over every workspace project, +//! drives [`fn@resolve_importer`] per importer, merges the per-importer +//! trees, and finishes with one [`fn@resolve_peers_workspace`] pass that +//! shares peer caches across importers and applies +//! `dedupeInjectedDeps` once with every importer's direct deps in +//! scope. [`fn@resolve_importer`] still owns three lower-level passes +//! and the `autoInstallPeers` hoist loop that ties them together. //! //! 1. **Tree pass** ([`fn@resolve_dependency_tree`]). Walks a project //! manifest's direct dependencies through a @@ -40,21 +45,31 @@ //! //! This is intentionally a thin slice of upstream: //! -//! - **Single importer.** Pacquet's install doesn't expose workspaces -//! to the resolver yet; the entry point takes one manifest at a -//! time. +//! - **Shared workspace ctx.** [`fn@resolve_workspace`] constructs one +//! [`WorkspaceTreeCtx`] and hands an `Arc::clone` to every per-importer +//! [`TreeCtx`], so the resolver's per-`pkgIdWithPatchHash` dedup +//! (`packages`, `resolved_by_wanted`, `children_specs_by_id`, +//! `children_by_id`) and the peer-walker seed sets +//! (`all_peer_dep_names`, `applied_patches`) carry across importers. +//! `base_opts.project_dir` stays per-importer on each [`TreeCtx`] so +//! transitive local-protocol resolutions keep the right anchor. The +//! downstream [`fn@resolve_peers_workspace`] then walks every +//! importer's direct deps through one Walker so `peersCache` + +//! `purePkgs` share the same way. //! - **No catalog / hook / lockfile-pinned-version bias.** The //! resolver is fed each child's manifest range verbatim. Lockfile- //! seeded preferred versions arrive via the orchestrator's //! `all_preferred_versions` option — callers pre-seed with the //! `pacquet-lockfile-preferred-versions` crate. +mod dedupe_injected_deps; mod dependencies_graph; mod hoist_peers; mod node_id; mod resolve_dependency_tree; mod resolve_importer; mod resolve_peers; +mod resolve_workspace; mod resolved_tree; mod validate_dependency_alias; @@ -68,13 +83,20 @@ pub use hoist_peers::{ pub use node_id::NodeId; pub use pacquet_deps_path::DepPath; pub use resolve_dependency_tree::{ - ManifestHook, ResolveDependencyTreeError, ResolveDependencyTreeOptions, TreeCtx, extend_tree, - resolve_dependency_tree, + ManifestHook, ResolveDependencyTreeError, ResolveDependencyTreeOptions, TreeCtx, + WorkspaceTreeCtx, extend_tree, resolve_dependency_tree, }; pub use resolve_importer::{ ResolveImporterError, ResolveImporterOptions, ResolveImporterResult, resolve_importer, + resolve_importer_with_workspace, +}; +pub use resolve_peers::{ + ImporterPeerInput, ResolvePeersOptions, ResolvePeersResult, WorkspaceResolvePeersResult, + resolve_peers, resolve_peers_workspace, +}; +pub use resolve_workspace::{ + ResolveWorkspaceResult, WorkspaceImporter, WorkspaceResolveOptions, resolve_workspace, }; -pub use resolve_peers::{ResolvePeersOptions, ResolvePeersResult, resolve_peers}; pub use resolved_tree::{ ChildEdge, DependenciesTree, DependenciesTreeNode, DirectDep, PeerDep, ResolvedPackage, ResolvedTree, TreeChildren, diff --git a/pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs b/pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs index aeb6e0a4df..2d938237d2 100644 --- a/pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs +++ b/pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs @@ -243,14 +243,14 @@ pub(crate) fn importer_injected_dependency_names(manifest: &PackageManifest) -> .collect() } -/// Cache key for [`TreeCtx::resolved_by_wanted`]. +/// Cache key for [`WorkspaceTreeCtx`]'s `resolved_by_wanted` map. /// /// The npm-shaped slice pacquet exposes today calls -/// [`Resolver::resolve`] with only three [`WantedDependency`] fields -/// populated — `alias`, `bare_specifier`, and `optional` (see the -/// `WantedDependency` literals in [`extend_tree`] and the recursive +/// [`Resolver::resolve`] with four [`WantedDependency`] fields +/// populated — `alias`, `bare_specifier`, `optional`, and `injected` (see +/// the `WantedDependency` literals in [`extend_tree`] and the recursive /// arm of [`fn@resolve_node`]). Anything else stays at `Default::default()`, -/// so a tuple over those three fields uniquely identifies a wanted +/// so a tuple over those four fields uniquely identifies a wanted /// dep across revisits. /// /// `optional` is part of the key because the npm resolver's @@ -282,63 +282,35 @@ type WantedKey = (Option, Option, Option, Option); /// meta from any manifest. pub(crate) type WantedSpec = (String, String, bool, bool); -/// One entry in [`TreeCtx::children_specs_by_id`] — +/// One entry in [`WorkspaceTreeCtx`]'s `children_specs_by_id` map — /// `(child_alias, child_range, child_optional)` triples extracted from /// a resolved package's manifest's `dependencies` plus /// `optionalDependencies` sections. type ChildSpec = (String, String, bool); -/// Mutable workspace for an in-flight tree walk. The orchestrator -/// (`resolve_importer`) holds one of these across hoist iterations and -/// extends it via [`extend_tree`] so newly-hoisted peer dependencies -/// reuse the existing per-id dedup map instead of restarting the walk. -pub struct TreeCtx { - base_opts: ResolveOptions, +/// Workspace-shared maps. Every per-importer [`TreeCtx`] in a +/// multi-importer install holds an `Arc` so the +/// resolver's per-`pkgIdWithPatchHash` dedup (`packages`, +/// `children_specs_by_id`, `children_by_id`, `resolved_by_wanted`) and +/// the peer-walker's seed sets (`all_peer_dep_names`, +/// `applied_patches`, `policy_violations`) carry across importers. +/// Mirrors the single shared `ctx` pnpm's +/// [`resolveDependencyTree`](https://github.com/pnpm/pnpm/blob/39101f5e37/installing/deps-resolver/src/resolveDependencyTree.ts#L180-L233) +/// hands to every importer's hoist loop. +/// +/// `dependencies_tree` (`NodeId → DependenciesTreeNode`) is keyed by +/// per-occurrence NodeIds, which are unique even across importers, so +/// every importer's walk contributes entries to one combined tree +/// without colliding. +pub struct WorkspaceTreeCtx { packages: Mutex>, dependencies_tree: Mutex>, all_peer_dep_names: Mutex>, policy_violations: Mutex>, - /// Configured `patchedDependencies` (already grouped by name). - /// Shared by `Arc` so the lookup table doesn't get cloned per - /// recursive call. `None` when no patches are configured for this - /// install. - patched_dependencies: Option>, - /// Keys of the `patchedDependencies` entries whose patch was - /// matched against at least one resolved package. Mirrors upstream's - /// `ctx.appliedPatches`. applied_patches: Mutex>, - /// Memoised [`Resolver::resolve`] results keyed by the parts of - /// [`WantedDependency`] the npm slice actually populates (see - /// [`WantedKey`]). - /// - /// pnpm's `resolveDependencies` carries the equivalent skip via the - /// [`isNew` gate](https://github.com/pnpm/pnpm/blob/097983fbca/installing/deps-resolver/src/resolveDependencies.ts#L1584) — - /// the second time a `pkgIdWithPatchHash` is reached, the walker - /// reuses the existing `resolvedPkgsById` envelope and never - /// re-enters the resolver chain. pacquet shortcuts at the layer - /// above that (the wanted-dep edge) because a `(name, range)` - /// pair that's already been resolved doesn't need - /// `pick_package`'s in-memory packument lookup, cache-key - /// formatting, or semver matching repeated. On the `alotta-files` - /// fixture this collapses the ~3 redundant `resolver.resolve()` - /// calls per package the tree walk used to drive into one. resolved_by_wanted: Mutex>>, - /// Cached `extract_children` output keyed by `pkgIdWithPatchHash`. - /// First visit walks the manifest's `dependencies` / - /// `optionalDependencies`; subsequent visits clone the `Arc` and - /// skip the JSON traversal. Paired with [`Self::resolved_by_wanted`] - /// so a revisit's per-call CPU is two HashMap lookups and an - /// `Arc::clone`. children_specs_by_id: Mutex>>>, - /// Per-`pkgIdWithPatchHash` cache of `(alias, child_pkg_id, - /// optional)` produced by the first walk. Threaded out via - /// [`ResolvedTree::children_by_id`] so the peer-resolver's - /// `realize_children` can expand a [`crate::TreeChildren::Lazy`] - /// node without re-running the resolver chain. See [`ChildEdge`] - /// for the field layout. - /// - /// [`ChildEdge`]: crate::ChildEdge children_by_id: Mutex>>>, /// `readPackageHook` applied to every resolved manifest before it /// enters the wanted-dep cache. See [`ManifestHook`] for the @@ -346,17 +318,13 @@ pub struct TreeCtx { manifest_hook: Option, } -impl TreeCtx { - /// Construct an empty context. Calls to [`extend_tree`] populate - /// `packages` / `dependencies_tree` / `all_peer_dep_names`. - pub fn new(base_opts: ResolveOptions) -> Self { - TreeCtx { - base_opts, +impl Default for WorkspaceTreeCtx { + fn default() -> Self { + WorkspaceTreeCtx { packages: Mutex::new(HashMap::new()), dependencies_tree: Mutex::new(HashMap::new()), all_peer_dep_names: Mutex::new(HashSet::new()), policy_violations: Mutex::new(Vec::new()), - patched_dependencies: None, applied_patches: Mutex::new(HashSet::new()), resolved_by_wanted: Mutex::new(HashMap::new()), children_specs_by_id: Mutex::new(HashMap::new()), @@ -364,17 +332,24 @@ impl TreeCtx { manifest_hook: None, } } +} - /// Attach the install's `patchedDependencies` map. When `Some`, - /// the per-node walker looks every resolved `name@version` up via - /// [`get_patch_info`] and appends `(patch_hash=)` to the - /// `pkgIdWithPatchHash` on a match. - pub fn with_patched_dependencies( - mut self, - patched_dependencies: Option>, - ) -> Self { - self.patched_dependencies = patched_dependencies; - self +impl WorkspaceTreeCtx { + /// Snapshot the workspace context into a [`ResolvedTree`] without + /// consuming `self`. `direct` carries the combined direct-dep + /// envelopes the caller built up across importers; multi-importer + /// orchestration usually leaves this empty and threads per-importer + /// direct deps separately into [`fn@crate::resolve_peers_workspace`]. + pub fn snapshot(&self, direct: Vec) -> ResolvedTree { + ResolvedTree { + direct, + packages: lock_recoverable(&self.packages).clone(), + dependencies_tree: lock_recoverable(&self.dependencies_tree).clone(), + all_peer_dep_names: lock_recoverable(&self.all_peer_dep_names).clone(), + policy_violations: lock_recoverable(&self.policy_violations).clone(), + applied_patches: lock_recoverable(&self.applied_patches).clone(), + children_by_id: lock_recoverable(&self.children_by_id).clone(), + } } /// Attach a `readPackageHook` applied to every resolved manifest @@ -385,15 +360,12 @@ impl TreeCtx { self } - /// Take ownership of `self` and emit the final [`ResolvedTree`] - /// the peer-resolution stage consumes. The orchestrator passes its - /// cumulative [`DirectDep`] list (initial walk + each hoist - /// iteration's contributions) as `direct`. + /// Take ownership of `self` and emit the final [`ResolvedTree`]. + /// Pacquet's single-importer path consumes the context via + /// [`TreeCtx::into_resolved_tree`], which routes through here once + /// the last `Arc` reference is the [`TreeCtx`]'s + /// own. pub fn into_resolved_tree(self, direct: Vec) -> ResolvedTree { - // `std::sync::Mutex::into_inner` returns `Result` to surface - // poisoning; recover from it the same way the per-acquire - // `lock_recoverable` helper does so a panic in an unrelated - // task doesn't escalate into a hard install failure here. ResolvedTree { direct, packages: self.packages.into_inner().unwrap_or_else(|err| err.into_inner()), @@ -416,21 +388,105 @@ impl TreeCtx { children_by_id: self.children_by_id.into_inner().unwrap_or_else(|err| err.into_inner()), } } +} + +/// Mutable workspace for an in-flight tree walk. The orchestrator +/// (`resolve_importer`) holds one of these across hoist iterations and +/// extends it via [`extend_tree`] so newly-hoisted peer dependencies +/// reuse the existing per-id dedup map instead of restarting the walk. +/// +/// The shared per-`pkgIdWithPatchHash` dedup maps live on +/// [`WorkspaceTreeCtx`] behind an `Arc`. In single-importer mode this +/// `Arc` is sole-owned by [`TreeCtx`]; in multi-importer mode +/// `Arc::clone(&workspace)` is handed to every per-importer +/// [`TreeCtx`] so importer N's tree walk reuses importer M's resolved +/// envelopes via the shared maps. +pub struct TreeCtx { + base_opts: ResolveOptions, + workspace: Arc, + /// Configured `patchedDependencies` (already grouped by name). + /// Shared by `Arc` so the lookup table doesn't get cloned per + /// recursive call. `None` when no patches are configured for this + /// install. + patched_dependencies: Option>, +} + +impl TreeCtx { + /// Construct a single-importer context with a fresh + /// [`WorkspaceTreeCtx`]. The multi-importer orchestrator uses + /// [`Self::with_workspace`] instead so per-importer contexts share + /// the same workspace ctx. + pub fn new(base_opts: ResolveOptions) -> Self { + TreeCtx { + base_opts, + workspace: Arc::new(WorkspaceTreeCtx::default()), + patched_dependencies: None, + } + } + + /// Construct a per-importer context that shares its dedup maps + /// with `workspace`. The caller is responsible for keeping + /// `workspace` alive across importers (typically via + /// `Arc::clone(&workspace)`). + pub fn with_workspace(workspace: Arc, base_opts: ResolveOptions) -> Self { + TreeCtx { base_opts, workspace, patched_dependencies: None } + } + + /// Borrow the shared workspace ctx so callers can hand the same + /// `Arc::clone` to the next per-importer [`TreeCtx`]. + pub fn workspace(&self) -> &Arc { + &self.workspace + } + + /// Attach the install's `patchedDependencies` map. When `Some`, + /// the per-node walker looks every resolved `name@version` up via + /// [`get_patch_info`] and appends `(patch_hash=)` to the + /// `pkgIdWithPatchHash` on a match. + pub fn with_patched_dependencies( + mut self, + patched_dependencies: Option>, + ) -> Self { + self.patched_dependencies = patched_dependencies; + self + } + + /// Attach a `readPackageHook` to the underlying [`WorkspaceTreeCtx`]. + /// `manifest_hook` is workspace-wide (one hook per install), so this + /// passthrough relies on the workspace ctx being sole-owned — + /// `TreeCtx::new` always satisfies that, and the multi-importer + /// orchestrator [`fn@crate::resolve_workspace`] hands the hook in via + /// [`WorkspaceTreeCtx::with_manifest_hook`] before sharing the + /// `Arc`. Panics if the workspace ctx has already been cloned — + /// callers must set the hook before sharing the context. + pub fn with_manifest_hook(mut self, manifest_hook: Option) -> Self { + Arc::get_mut(&mut self.workspace) + .expect("with_manifest_hook called after the workspace ctx was shared via Arc::clone") + .manifest_hook = manifest_hook; + self + } + + /// Take ownership of `self` and emit the final [`ResolvedTree`] + /// the peer-resolution stage consumes. The orchestrator passes its + /// cumulative [`DirectDep`] list (initial walk + each hoist + /// iteration's contributions) as `direct`. + /// + /// When the [`WorkspaceTreeCtx`] is sole-owned by this context + /// (single-importer install) the inner mutex contents move + /// directly into the [`ResolvedTree`]; otherwise the maps are + /// cloned out via [`WorkspaceTreeCtx::snapshot`]. + pub fn into_resolved_tree(self, direct: Vec) -> ResolvedTree { + match Arc::try_unwrap(self.workspace) { + Ok(ws) => ws.into_resolved_tree(direct), + Err(arc) => arc.snapshot(direct), + } + } /// Build a snapshot of the current tree state without consuming /// `self`. The orchestrator's hoist loop snapshots after each /// [`extend_tree`] call to run [`fn@crate::resolve_peers`] over the /// growing tree and find missing peers to hoist next. pub fn snapshot(&self, direct: Vec) -> ResolvedTree { - ResolvedTree { - direct, - packages: lock_recoverable(&self.packages).clone(), - dependencies_tree: lock_recoverable(&self.dependencies_tree).clone(), - all_peer_dep_names: lock_recoverable(&self.all_peer_dep_names).clone(), - policy_violations: lock_recoverable(&self.policy_violations).clone(), - applied_patches: lock_recoverable(&self.applied_patches).clone(), - children_by_id: lock_recoverable(&self.children_by_id).clone(), - } + self.workspace.snapshot(direct) } /// Iterate over every `(name, version)` pair the walk has resolved @@ -438,7 +494,7 @@ impl TreeCtx { /// in sync — mirrors upstream's resolveDependency-time push at /// [`resolveDependencies.ts:1440`](https://github.com/pnpm/pnpm/blob/097983fbca/installing/deps-resolver/src/resolveDependencies.ts#L1440). pub fn resolved_versions(&self) -> Vec<(String, String)> { - lock_recoverable(&self.packages) + lock_recoverable(&self.workspace.packages) .values() .filter_map(|pkg| { let name_ver = pkg.result.name_ver.as_ref()?; @@ -538,7 +594,8 @@ where // `ResolveResult`). let cache_key: WantedKey = (wanted.alias.clone(), wanted.bare_specifier.clone(), wanted.optional, wanted.injected); - let cached = lock_recoverable(&ctx.resolved_by_wanted).get(&cache_key).map(Arc::clone); + let cached = + lock_recoverable(&ctx.workspace.resolved_by_wanted).get(&cache_key).map(Arc::clone); let result = match cached { Some(result) => result, None => { @@ -558,7 +615,7 @@ where // call at the resolveDependency seam. The hook clones the // inner `Value` only when it modifies it, so unrelated // manifests keep sharing the resolver's cached `Arc`. - if let Some(hook) = ctx.manifest_hook.as_ref() + if let Some(hook) = ctx.workspace.manifest_hook.as_ref() && let Some(manifest) = result_inner.manifest.take() { result_inner.manifest = Some(hook(manifest)); @@ -569,7 +626,7 @@ where // graph node share one heap-allocated `ResolveResult` // instead of cloning every `String` field per occurrence. let result = Arc::new(result); - lock_recoverable(&ctx.resolved_by_wanted) + lock_recoverable(&ctx.workspace.resolved_by_wanted) .entry(cache_key) .or_insert_with(|| Arc::clone(&result)); result @@ -577,7 +634,7 @@ where }; if let Some(violation) = result.policy_violation.clone() { - lock_recoverable(&ctx.policy_violations).push(violation); + lock_recoverable(&ctx.workspace.policy_violations).push(violation); } if ctx.base_opts.block_exotic_subdeps @@ -616,7 +673,7 @@ where // arm. The `is_revisit` flag flows into the children handling // below: a revisit can produce a [`TreeChildren::Lazy`] node // because the first visit already populated - // [`TreeCtx::children_by_id`] for this pkg, and revisits don't + // [`WorkspaceTreeCtx`]'s `children_by_id` for this pkg, and revisits don't // discover new transitive packages. // Leaves (no deps / optional deps / peers / peerDependenciesMeta) // reuse the package id as their `NodeId`, collapsing every parent @@ -646,7 +703,7 @@ where let is_revisit; { - let mut packages = lock_recoverable(&ctx.packages); + let mut packages = lock_recoverable(&ctx.workspace.packages); match packages.get_mut(&id) { Some(existing) => { existing.optional = existing.optional && current_is_optional; @@ -658,7 +715,7 @@ where // Collect peer names for the peer-resolution stage's // `parentPkgs` filter (only peers count as parents). { - let mut all_peers = lock_recoverable(&ctx.all_peer_dep_names); + let mut all_peers = lock_recoverable(&ctx.workspace.all_peer_dep_names); for name in peer_dependencies.keys() { all_peers.insert(name.clone()); } @@ -709,14 +766,14 @@ where // a miss. The cache value is held by `Arc` so revisits clone the // refcount instead of the inner `Vec<(String, String, bool)>`. let child_specs = { - let cache = lock_recoverable(&ctx.children_specs_by_id); + let cache = lock_recoverable(&ctx.workspace.children_specs_by_id); cache.get(&id).map(Arc::clone) }; let child_specs = match child_specs { Some(specs) => specs, None => { let specs = Arc::new(extract_children(&result)?); - lock_recoverable(&ctx.children_specs_by_id) + lock_recoverable(&ctx.workspace.children_specs_by_id) .entry(id.clone()) .or_insert_with(|| Arc::clone(&specs)); specs @@ -765,7 +822,9 @@ where }); realized.insert(dep.alias, dep.node_id); } - lock_recoverable(&ctx.children_by_id).entry(id.clone()).or_insert_with(|| Arc::new(by_id)); + lock_recoverable(&ctx.workspace.children_by_id) + .entry(id.clone()) + .or_insert_with(|| Arc::new(by_id)); crate::resolved_tree::TreeChildren::Realized(realized) }; @@ -780,7 +839,7 @@ where // short-circuits them in `resolve_node`. Mirrors upstream's // `depth: -1` on the `isLinkedDependency` arm. let node_depth = if is_link { -1 } else { depth }; - lock_recoverable(&ctx.dependencies_tree) + lock_recoverable(&ctx.workspace.dependencies_tree) .entry(node_id.clone()) .and_modify(|node| { if node.depth > node_depth { @@ -907,7 +966,7 @@ async fn build_pkg_id_with_patch_hash( let Some(patch) = get_patch_info(Some(groups), &name, &version)? else { return Ok(prefixed); }; - lock_recoverable(&ctx.applied_patches).insert(patch.key.clone()); + lock_recoverable(&ctx.workspace.applied_patches).insert(patch.key.clone()); Ok(format!("{prefixed}(patch_hash={})", patch.hash)) } diff --git a/pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs b/pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs index 2dbee99c11..b7c01cd4bf 100644 --- a/pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs +++ b/pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs @@ -17,8 +17,11 @@ //! preferred version already in scope, and extend the tree with //! those. Re-enter the inner loop if any landed. //! -//! Single-importer slice; multi-importer support waits on pacquet's -//! workspace work. +//! Per-importer slice. The workspace-wide orchestrator +//! [`fn@crate::resolve_workspace`] loops this function for every +//! importer, then runs a single multi-importer +//! [`fn@crate::resolve_peers_workspace`] pass that shares the peer +//! walker's caches across importers and applies `dedupeInjectedDeps`. use std::collections::{BTreeMap, BTreeSet, HashSet}; use std::sync::Arc; @@ -40,7 +43,7 @@ use crate::{ hoist_peers, }, resolve_dependency_tree::{ - ResolveDependencyTreeError, TreeCtx, WantedSpec, extend_tree, + ResolveDependencyTreeError, TreeCtx, WantedSpec, WorkspaceTreeCtx, extend_tree, importer_injected_dependency_names, importer_optional_dependency_names, resolve_catalog_specifiers, }, @@ -181,6 +184,32 @@ pub async fn resolve_importer( dependency_groups: DependencyGroupList, opts: ResolveImporterOptions, ) -> Result +where + DependencyGroupList: IntoIterator, + Chain: Resolver + ?Sized, +{ + // `manifest_hook` lives on the workspace ctx (it's workspace-wide, + // not per-importer). Apply it before sharing the `Arc` — + // `resolve_importer_with_workspace` reads through the shared ctx + // and can't mutate it after the fact. + let workspace = + Arc::new(WorkspaceTreeCtx::default().with_manifest_hook(opts.manifest_hook.clone())); + resolve_importer_with_workspace(resolver, manifest, dependency_groups, opts, workspace).await +} + +/// Same as [`fn@resolve_importer`] but reuses a shared +/// [`WorkspaceTreeCtx`] so the resolver's per-`pkgIdWithPatchHash` +/// dedup carries across importers in a workspace install. The +/// multi-importer orchestrator [`fn@crate::resolve_workspace`] uses +/// this to fold every importer's resolved packages into one shared +/// map. +pub async fn resolve_importer_with_workspace( + resolver: &Chain, + manifest: &PackageManifest, + dependency_groups: DependencyGroupList, + opts: ResolveImporterOptions, + workspace: Arc, +) -> Result where DependencyGroupList: IntoIterator, Chain: Resolver + ?Sized, @@ -198,7 +227,11 @@ where lockfile_dir, modules_dir, peers_suffix_max_length, - manifest_hook, + // `manifest_hook` is workspace-wide; it lives on the shared + // [`WorkspaceTreeCtx`] and the caller (`resolve_importer` or + // `resolve_workspace`) is responsible for setting it there + // before handing the `Arc` to this function. + manifest_hook: _, } = opts; let peers_opts = || ResolvePeersOptions { peers_suffix_max_length, @@ -208,9 +241,8 @@ where modules_dir: modules_dir.clone(), }; - let ctx = TreeCtx::new(base_opts) - .with_patched_dependencies(patched_dependencies) - .with_manifest_hook(manifest_hook); + let ctx = TreeCtx::with_workspace(workspace, base_opts) + .with_patched_dependencies(patched_dependencies); // Mirrors upstream's // [`getAllDependenciesFromManifest({ autoInstallPeers })`](https://github.com/pnpm/pnpm/blob/097983fbca/pkg-manifest/utils/src/getAllDependenciesFromManifest.ts): diff --git a/pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs b/pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs index 93b8516bf2..5ed57816cd 100644 --- a/pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs +++ b/pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs @@ -42,12 +42,14 @@ //! on anyway. use std::collections::{BTreeMap, HashMap, HashSet}; +use std::path::{Path, PathBuf}; use std::sync::Arc; use node_semver::{Range, Version}; use pacquet_deps_path::{DepPath, PeerId, create_peer_dep_graph_hash, link_path_to_peer_version}; use crate::{ + dedupe_injected_deps::dedupe_injected_deps, dependencies_graph::{ DependenciesGraph, DependenciesGraphNode, MissingPeer, ParentPackageRef, PeerDependencyIssue, PeerDependencyIssues, @@ -153,6 +155,34 @@ pub struct ResolvePeersResult { pub peer_dependency_issues: PeerDependencyIssues, } +/// One importer's input to the multi-importer [`fn@resolve_peers_workspace`] +/// — the lockfile importer id, the importer's `directNodeIdsByAlias` +/// slice, the absolute project root, and the per-importer +/// `modules_dir` used by the `excludeLinksFromLockfile` link-remap. +/// Mirrors the per-project payload pnpm's `resolvePeers` reads off +/// `opts.projects`. +#[derive(Debug, Clone)] +pub struct ImporterPeerInput { + pub id: String, + pub direct: Vec, + pub root_dir: PathBuf, + /// Absolute path of this importer's `node_modules` directory. + /// Threaded into [`ResolvePeersOptions::modules_dir`] while this + /// importer is being walked so the `excludeLinksFromLockfile` link + /// remap uses the correct per-importer target. `None` disables + /// the remap for this importer. + pub modules_dir: Option, +} + +/// Output of [`fn@resolve_peers_workspace`] — the cross-importer +/// dedupe map plus per-importer `direct_dependencies_by_alias` slices. +#[derive(Debug, Default)] +pub struct WorkspaceResolvePeersResult { + pub graph: DependenciesGraph, + pub direct_dependencies_by_importer: BTreeMap>, + pub peer_dependency_issues_by_importer: BTreeMap, +} + /// Resolve peer dependencies for `tree` and emit a depPath-keyed graph. /// /// Takes `tree` by `&mut` because lazy [`TreeChildren`] entries are @@ -180,6 +210,82 @@ pub fn resolve_peers(tree: &mut ResolvedTree, opts: ResolvePeersOptions) -> Reso walker.walk() } +/// Resolve peer dependencies for every importer in `importers` against +/// the shared `tree`, then rewrite injected workspace deps that +/// dedupe back to `link:` symlinks. +/// +/// Mirrors pnpm's +/// [`resolvePeers`](https://github.com/pnpm/pnpm/blob/39101f5e37/installing/deps-resolver/src/resolvePeers.ts#L72) +/// multi-importer entry point: one Walker walks every importer's +/// direct deps in sequence so `peersCache` + `purePkgs` are shared +/// across importers, then the in-crate `dedupe_injected_deps` pass +/// runs once with all importers' direct deps in scope. +pub fn resolve_peers_workspace( + tree: &mut ResolvedTree, + importers: &[ImporterPeerInput], + lockfile_dir: &Path, + dedupe_injected_deps_enabled: bool, + opts: ResolvePeersOptions, +) -> WorkspaceResolvePeersResult { + let mut walker = Walker { + tree, + opts, + graph: DependenciesGraph::new(), + issues: PeerDependencyIssues::default(), + node_dep_paths: HashMap::new(), + node_external_peers: HashMap::new(), + node_missing_peers: HashMap::new(), + in_progress: HashSet::new(), + pending_peer_edges: Vec::new(), + pure_pkgs: HashSet::new(), + peers_cache: HashMap::new(), + parent_pkgs_of_node: HashMap::new(), + }; + + let mut direct_dependencies_by_importer: BTreeMap> = + BTreeMap::new(); + let mut peer_dependency_issues_by_importer: BTreeMap = + BTreeMap::new(); + let mut importer_root_dirs: BTreeMap = BTreeMap::new(); + for importer in importers { + importer_root_dirs.insert(importer.id.clone(), importer.root_dir.clone()); + // Swap the per-importer `modules_dir` in before the walk so + // the `excludeLinksFromLockfile` link-remap inside + // `resolve_node` uses the correct importer-scoped target. + walker.opts.modules_dir = importer.modules_dir.clone(); + let importer_parents = walker.build_importer_parents_from(&importer.direct); + let parent_chain_names: Vec = Vec::new(); + let mut direct_by_alias = BTreeMap::new(); + for dep in &importer.direct { + let output = + walker.resolve_node(dep.node_id.clone(), &importer_parents, &parent_chain_names, 0); + direct_by_alias.insert(dep.alias.clone(), output.dep_path); + } + direct_dependencies_by_importer.insert(importer.id.clone(), direct_by_alias); + let issues = std::mem::take(&mut walker.issues); + if !issues.bad.is_empty() || !issues.missing.is_empty() { + peer_dependency_issues_by_importer.insert(importer.id.clone(), issues); + } + } + walker.patch_pending_peer_edges(); + let mut graph = walker.graph; + + if dedupe_injected_deps_enabled { + dedupe_injected_deps( + &mut graph, + &mut direct_dependencies_by_importer, + &importer_root_dirs, + lockfile_dir, + ); + } + + WorkspaceResolvePeersResult { + graph, + direct_dependencies_by_importer, + peer_dependency_issues_by_importer, + } +} + /// Per-name entry in the propagating `ParentRefs` map. Mirrors upstream's /// [`ParentRef`](https://github.com/pnpm/pnpm/blob/c86c423bdc/installing/deps-resolver/src/resolvePeers.ts#L998-L1006). #[derive(Debug, Clone)] @@ -413,8 +519,16 @@ impl<'tree> Walker<'tree> { /// upstream's /// [`target` rewrite in `index.ts`](https://github.com/pnpm/pnpm/blob/094aa6e57b/installing/deps-resolver/src/index.ts#L232-L244). fn build_importer_parents(&self) -> ParentRefs { + self.build_importer_parents_from(&self.tree.direct) + } + + /// Same as [`Self::build_importer_parents`] but seeds from an + /// externally-supplied direct-deps slice — used by the multi-importer + /// [`fn@resolve_peers_workspace`] where each importer's `direct` + /// lives outside [`ResolvedTree`]. + fn build_importer_parents_from(&self, direct_deps: &[DirectDep]) -> ParentRefs { let mut refs = ParentRefs::new(); - for direct in &self.tree.direct { + for direct in direct_deps { let Some(tree_node) = self.tree.dependencies_tree.get(&direct.node_id) else { continue; }; diff --git a/pacquet/crates/resolving-deps-resolver/src/resolve_workspace.rs b/pacquet/crates/resolving-deps-resolver/src/resolve_workspace.rs new file mode 100644 index 0000000000..f97f76de10 --- /dev/null +++ b/pacquet/crates/resolving-deps-resolver/src/resolve_workspace.rs @@ -0,0 +1,158 @@ +//! Multi-importer entry point for an install pass. Mirrors pnpm's +//! [`resolveDependencies`](https://github.com/pnpm/pnpm/blob/39101f5e37/installing/deps-resolver/src/index.ts#L128) +//! shape: take every workspace project the install touches, run the +//! per-importer hoist + peer-resolution loop with shared cross-importer +//! caches, and emit the combined `DependenciesGraph` plus the +//! per-importer `direct_dependencies_by_importer` map the install +//! layer consumes. +//! +//! The cross-importer cache that matters for performance lives on the +//! peer walker (`peersCache` + `purePkgs`); making it workspace-wide +//! means an importer revisiting a `(pkgIdWithPatchHash, +//! parent-peer-context)` pair that an earlier importer already resolved +//! short-circuits straight to the cached `depPath`. Sharing the +//! `TreeCtx` resolved-pkgs map across importers is a separate axis +//! pacquet hasn't landed yet — `base_opts.project_dir` varies per +//! importer, which the existing `TreeCtx` shape ties to one importer +//! at a time. The peer-walker share captures the hot path; the +//! resolved-pkgs share is a follow-up perf win. + +use std::path::PathBuf; +use std::sync::Arc; + +use pacquet_package_manifest::{DependencyGroup, PackageManifest}; +use pacquet_resolving_resolver_base::Resolver; + +use crate::{ + resolve_dependency_tree::{ManifestHook, WorkspaceTreeCtx}, + resolve_importer::{ + ResolveImporterError, ResolveImporterOptions, ResolveImporterResult, + resolve_importer_with_workspace, + }, + resolve_peers::{ + ImporterPeerInput, ResolvePeersOptions, WorkspaceResolvePeersResult, + resolve_peers_workspace, + }, + resolved_tree::ResolvedTree, +}; + +/// One importer's input to [`fn@resolve_workspace`]. +pub struct WorkspaceImporter<'a> { + pub id: String, + pub manifest: &'a PackageManifest, +} + +/// Workspace-shared opts that don't vary per importer. +pub struct WorkspaceResolveOptions { + pub dedupe_peers: bool, + /// `true` enables [`fn@crate::resolve_peers_workspace`]'s cross- + /// importer dedupe pass — `dependenciesMeta[].injected: true` + /// workspace edges collapse back to `link:` when the injected + /// snapshot's children are a subset of the target project's own + /// direct deps. + pub dedupe_injected_deps: bool, + /// Threaded into [`ResolvePeersOptions::exclude_links_from_lockfile`] + /// for the workspace-wide peer pass. Per-importer + /// [`ResolvePeersOptions::modules_dir`] comes from each + /// [`crate::ImporterPeerInput::modules_dir`]. + pub exclude_links_from_lockfile: bool, + pub lockfile_dir: PathBuf, + pub peers_suffix_max_length: usize, + /// `readPackageHook` applied to every resolved manifest before it + /// enters the wanted-dep cache. Workspace-wide (one hook per + /// install); the install layer typically threads + /// `packageExtensions` here. See [`ManifestHook`]. + pub manifest_hook: Option, +} + +/// Result of [`fn@resolve_workspace`]. The combined +/// [`WorkspaceResolvePeersResult`] holds the cross-importer graph + the +/// per-importer `direct_dependencies_by_alias` map; `merged_tree` +/// carries the shared `ResolvedTree` snapshot the workspace ctx +/// produced after every importer's walk folded into the shared maps. +pub struct ResolveWorkspaceResult { + pub merged_tree: ResolvedTree, + pub peers: WorkspaceResolvePeersResult, +} + +/// Resolve every importer's dependencies, then run one workspace-wide +/// peer-resolution + dedupe pass. +/// +/// `per_importer_options` is invoked per importer to build that +/// importer's own [`ResolveImporterOptions`] — the install layer owns +/// the per-importer wiring (project dir, modules dir, lockfile dir, +/// exclude-links-from-lockfile, etc.). The closure shape mirrors how +/// pnpm constructs `ImporterToResolve` per project inside +/// [`resolveDependencyTree`](https://github.com/pnpm/pnpm/blob/39101f5e37/installing/deps-resolver/src/resolveDependencyTree.ts#L236). +pub async fn resolve_workspace<'a, Chain, BuildImporterOptions>( + resolver: &Chain, + importers: &[WorkspaceImporter<'a>], + dependency_groups: &[DependencyGroup], + opts: WorkspaceResolveOptions, + mut per_importer_options: BuildImporterOptions, +) -> Result +where + Chain: Resolver + ?Sized, + BuildImporterOptions: FnMut(&WorkspaceImporter<'a>) -> ResolveImporterOptions, +{ + let WorkspaceResolveOptions { + dedupe_peers, + dedupe_injected_deps, + exclude_links_from_lockfile, + lockfile_dir, + peers_suffix_max_length, + manifest_hook, + } = opts; + let workspace = Arc::new(WorkspaceTreeCtx::default().with_manifest_hook(manifest_hook)); + let mut per_importer_inputs: Vec = Vec::with_capacity(importers.len()); + for importer in importers { + let importer_opts = per_importer_options(importer); + let project_dir = importer_opts.base_opts.project_dir.clone(); + let modules_dir = importer_opts.modules_dir.clone(); + let ResolveImporterResult { resolved_tree, .. } = resolve_importer_with_workspace( + resolver, + importer.manifest, + dependency_groups.iter().copied(), + importer_opts, + Arc::clone(&workspace), + ) + .await?; + let direct = resolved_tree.direct; + per_importer_inputs.push(ImporterPeerInput { + id: importer.id.clone(), + direct, + root_dir: project_dir, + modules_dir, + }); + } + + // Reclaim the workspace ctx now that every per-importer + // `resolve_importer_with_workspace` call has dropped its + // `Arc`. The `try_unwrap` succeeds when this is + // the sole remaining `Arc` reference (the common case); the + // fallback snapshots out via the shared `Arc` for parity. + let mut merged_tree = match Arc::try_unwrap(workspace) { + Ok(ws) => ws.into_resolved_tree(Vec::new()), + Err(arc) => arc.snapshot(Vec::new()), + }; + + let peer_opts = ResolvePeersOptions { + peers_suffix_max_length, + dedupe_peers, + exclude_links_from_lockfile, + lockfile_dir: Some(lockfile_dir.clone()), + // Per-importer; resolve_peers_workspace swaps the + // ImporterPeerInput's modules_dir into walker.opts before each + // importer's walk. + modules_dir: None, + }; + let peers = resolve_peers_workspace( + &mut merged_tree, + &per_importer_inputs, + &lockfile_dir, + dedupe_injected_deps, + peer_opts, + ); + + Ok(ResolveWorkspaceResult { merged_tree, peers }) +}