From d2b42c2dfc87bf2e822e83d7fcbd813698d1fd9b Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Fri, 12 Jun 2026 22:00:59 +0200 Subject: [PATCH] fix(pacquet): per-level preferred-version fold + all-importers hoist rounds (#12357) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Two parity changes for pacquet's resolver, continuing https://github.com/pnpm/pnpm/issues/12266. Measured on the monorepo (fresh state, `install --lockfile-only`, back-to-back vs **pnpm 11.6.0**), the real-lockfile document diff drops from **128 to 5 changed lines** (re-measured after rebasing over #12361/#12362: **132 → 11**, where 8 of the 11 are a divergence the pacquet side of #12362 itself introduced — see the analysis on pnpm/pnpm#12266 — and 3 are the known cycle-closing-edge gap). ### 1. Per-level preferred-version fold pnpm extends the preferred-versions map per resolution level: after a package's direct dependencies settle, their `(name, version)` pairs join the map the *children's* subtree resolutions pick against ([resolveDependencies.ts#L717-L746](https://github.com/pnpm/pnpm/blob/ce9c096e8e/installing/deps-resolver/src/resolveDependencies.ts#L717-L746)). So `signed-varint`'s `varint@~5.0.0` dedupes to the `varint@5.0.0` its parent pinned as a sibling instead of drifting to `5.0.2`. pacquet picked against a static seed only; besides `varint`/`es-abstract`, this turned out to drive the remaining `jest`/`@types/node` duplicate variants too. - The walk resolves a whole sibling level before any child subtree starts (upstream's postponed-resolution barrier): `resolve_node` splits into `resolve_node_seed` + `walk_node_children`. - Each level layers its versions onto a new `PreferredVersionsOverlay` (O(1) `Arc`-chained layers in `resolver-base`); the npm picker folds the per-name view in as plain `version` selectors at both registry seams. - The overlay's per-name view joins the per-wanted dedup cache key; lockfile-reuse subtrees keep the no-overlay path (exact pins). ### 2. Hoist rounds across all importers (deterministic barrier, same logic as pnpm) pnpm resolves **every importer's initial wave before any peer hoist**, then repeats global hoist rounds (per round: each importer's required-peer loop to a fixpoint, then one optional-peer hoist) until no importer hoists ([resolveDependencies.ts#L335-L445](https://github.com/pnpm/pnpm/blob/ce9c096e8e/installing/deps-resolver/src/resolveDependencies.ts#L335-L445)). pacquet ran each importer's whole hoist loop before the next importer's initial wave, so an early importer's optional-peer pick couldn't see versions a later importer resolves — `@cyclonedx`'s `spdx-expression-parse` hoisted `3.0.1` where pnpm's barrier-visible map picks `4.0.0`. `resolve_importer_with_workspace` is now an `ImporterHoistState` (`init` / `run_required_round` / `hoist_optional_round`) driven by `resolve_workspace` in upstream's exact phase order. Both implementations are deterministic here; the rule is identical. ## Verification - New regression test `child_resolution_prefers_parent_level_sibling_versions` (fails with the fold disabled) + full `resolving-*`, `package-manager`, `cli` suites: 1,242 tests pass; clippy `--deny warnings`, rustfmt, typos clean. - Whole-monorepo diff vs fresh pnpm 11.6.0: 128 → 5 changed lines; consecutive pacquet runs byte-identical. --- Cargo.lock | 1 + .../crates/resolving-deps-resolver/Cargo.toml | 21 +- .../src/resolve_dependency_tree.rs | 551 +++++++++++++++--- .../src/resolve_importer.rs | 328 +++++++---- .../src/resolve_workspace.rs | 68 ++- .../resolving-deps-resolver/src/tests.rs | 197 +++++++ .../crates/resolving-npm-resolver/src/lib.rs | 1 + .../src/named_registry_resolver.rs | 6 +- .../src/npm_resolver.rs | 6 +- .../src/preferred_overlay.rs | 27 + .../crates/resolving-resolver-base/src/lib.rs | 10 +- .../resolving-resolver-base/src/resolve.rs | 53 ++ 12 files changed, 1042 insertions(+), 227 deletions(-) create mode 100644 pacquet/crates/resolving-npm-resolver/src/preferred_overlay.rs diff --git a/Cargo.lock b/Cargo.lock index db81082a29..2a8cdfa283 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4203,6 +4203,7 @@ dependencies = [ "pacquet-lockfile", "pacquet-package-manifest", "pacquet-patching", + "pacquet-resolving-jsr-specifier-parser", "pacquet-resolving-parse-wanted-dependency", "pacquet-resolving-resolver-base", "pathdiff", diff --git a/pacquet/crates/resolving-deps-resolver/Cargo.toml b/pacquet/crates/resolving-deps-resolver/Cargo.toml index a0f201bd48..17ecc0b057 100644 --- a/pacquet/crates/resolving-deps-resolver/Cargo.toml +++ b/pacquet/crates/resolving-deps-resolver/Cargo.toml @@ -22,16 +22,17 @@ pacquet-patching = { workspace = true } pacquet-resolving-parse-wanted-dependency = { workspace = true } pacquet-resolving-resolver-base = { workspace = true } -async-recursion = { workspace = true } -chrono = { workspace = true } -derive_more = { workspace = true } -futures-util = { workspace = true } -miette = { workspace = true } -node-semver = { workspace = true } -pathdiff = { workspace = true } -pipe-trait = { workspace = true } -serde_json = { workspace = true } -tokio = { workspace = true, features = ["sync"] } +async-recursion = { workspace = true } +chrono = { workspace = true } +derive_more = { workspace = true } +futures-util = { workspace = true } +miette = { workspace = true } +node-semver = { workspace = true } +pacquet-resolving-jsr-specifier-parser = { workspace = true } +pathdiff = { workspace = true } +pipe-trait = { workspace = true } +serde_json = { workspace = true } +tokio = { workspace = true, features = ["sync"] } [dev-dependencies] node-semver = { workspace = true } 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 06d0be0622..35b6b19b1e 100644 --- a/pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs +++ b/pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs @@ -11,7 +11,9 @@ use pacquet_catalogs_types::Catalogs; use pacquet_hooks::PnpmfileHooks; use pacquet_package_manifest::{DependencyGroup, PackageManifest}; use pacquet_patching::{PatchGroupRecord, PatchKeyConflictError, get_patch_info}; -use pacquet_resolving_resolver_base::{ResolveError, ResolveOptions, Resolver, WantedDependency}; +use pacquet_resolving_resolver_base::{ + PreferredVersionsOverlay, ResolveError, ResolveOptions, Resolver, WantedDependency, +}; use pipe_trait::Pipe; use serde_json::Value; use std::{ @@ -460,6 +462,7 @@ type WantedKey = ( Option>, Option, Option, + Vec<(String, Vec)>, ); /// Whether a wanted dep's resolution is computed relative to the @@ -1027,7 +1030,12 @@ where } else { ReuseSource::Off }; - let results = wanted + // Phase 1: resolve every direct dep before any subtree walk, so + // the level's resolved versions seed the children's + // preferred-versions overlay (upstream's per-level fold; the + // direct deps themselves resolve against the importer's static + // preferred map only). + let seeds = wanted .into_iter() .map(|(name, range, optional, injected)| { let reuse = reuse.clone(); @@ -1050,7 +1058,34 @@ where injected: injected.then_some(true), ..WantedDependency::default() }; - resolve_node(ctx, resolver, wanted, &[], 0, false, reuse).await + let base_overlay = ctx.base_opts.preferred_versions_overlay.clone(); + let seed = + resolve_node_seed(ctx, resolver, wanted, &[], 0, false, reuse, base_overlay) + .await?; + warm_children_resolutions(ctx, resolver, &seed).await; + Ok::(seed) + } + }) + .pipe(future::try_join_all) + .await?; + // The level chain extends any caller-seeded overlay so descendant + // picks and cache keys keep honoring it. + let children_overlay = PreferredVersionsOverlay::layer( + ctx.base_opts.preferred_versions_overlay.clone(), + level_versions(ctx, &seeds), + ); + // Phase 2: walk each direct dep's children with the level overlay. + let results = seeds + .into_iter() + .map(|seed| { + let overlay = children_overlay.clone(); + async move { + match seed { + NodeSeed::Done(dep) => Ok(dep), + NodeSeed::Pending(pending) => { + walk_node_children(ctx, resolver, *pending, overlay).await + } + } } }) .pipe(future::try_join_all) @@ -1058,20 +1093,11 @@ where Ok(results.into_iter().flatten().collect()) } -/// Resolve one `(alias, range)` edge, register the resolved package in -/// the dedup map if absent, allocate a fresh [`NodeId`] for this -/// occurrence, and recurse into children. -/// -/// `ancestor_ids` is the chain of `pkgIdWithPatchHash` values from the -/// root importer down to the current node's parent. Mirrors upstream's -/// `parentIds` / `parentDepPathsChain`. When the resolved id appears -/// in the chain, this call is a cycle re-entry: pacquet drops the -/// edge entirely (returns `Ok(None)`) so the parent's `children` map -/// omits the cycled child — same shape as upstream's -/// [`parentIdsContainSequence`](https://github.com/pnpm/pnpm/blob/097983fbca/installing/deps-resolver/src/resolveDependencyTree.ts#L378) -/// gate in `buildTree`. Without this, two nodes for the same id race -/// each other into `graph.insert`, and an empty-children entry for the -/// cycled occurrence can overwrite the real one. +/// Resolve one `(alias, range)` edge end-to-end with no +/// preferred-versions overlay: [`fn@resolve_node_seed`] then +/// [`fn@walk_node_children`]. Used where per-level preference folding +/// does not apply — the lockfile-reuse subtree walk, whose versions +/// are exact pins. #[async_recursion] async fn resolve_node( ctx: &TreeCtx, @@ -1082,6 +1108,98 @@ async fn resolve_node( parent_optional: bool, reuse: ReuseSource, ) -> Result, ResolveDependencyTreeError> +where + Chain: Resolver + ?Sized, +{ + let base_overlay = ctx.base_opts.preferred_versions_overlay.clone(); + match resolve_node_seed( + ctx, + resolver, + wanted, + ancestor_ids, + depth, + parent_optional, + reuse, + base_overlay.clone(), + ) + .await? + { + NodeSeed::Done(dep) => Ok(dep), + NodeSeed::Pending(pending) => { + walk_node_children(ctx, resolver, *pending, base_overlay).await + } + } +} + +/// Outcome of [`fn@resolve_node_seed`]: either the edge completed +/// without a children walk (lockfile reuse, cycle break), or the +/// package resolved and its children walk is still pending — the +/// caller runs it via [`fn@walk_node_children`] once every sibling +/// seed settled, so the children's resolution sees the whole level's +/// versions in its preferred-versions overlay. +enum NodeSeed { + Done(Option), + Pending(Box), +} + +/// A resolved-but-not-walked node: everything +/// [`fn@walk_node_children`] needs to recurse into the children. +struct PendingNode { + result: Arc, + id: String, + alias: String, + node_id: NodeId, + is_link: bool, + next_ancestors: Arc>, + /// The deterministic children-ownership claim taken at seed time; + /// the walk phase re-checks it before recording the children, so + /// a better-placed occurrence seeded after this one still wins. + children_owner: ChildrenOwnerClaim, + depth: i32, + current_is_optional: bool, + /// The edge's recorded snapshot key in the prior lockfile, if + /// any — threads each child's `currentPkg` through the walk + /// phase via `ReuseSource::PriorOnly`. + prior_key: Option, +} + +/// Resolve one `(alias, range)` edge and register the resolved package +/// in the dedup map if absent — the per-package half of the old +/// monolithic walk, run for a whole sibling level before any child +/// subtree starts. +/// +/// `pick_overlay` carries the per-level preferred-version additions +/// (the parent level's resolved versions) consulted by the npm +/// resolver's version pick; it participates in the per-wanted dedup +/// cache key so the same range can legitimately pick different +/// versions under different levels, mirroring upstream's per-level +/// `Object.create(preferredVersions)` fold. +/// +/// `ancestor_ids` is the chain of `pkgIdWithPatchHash` values from the +/// root importer down to the current node's parent. Mirrors upstream's +/// `parentIds` / `parentDepPathsChain`. When the resolved id appears +/// in the chain, this call is a cycle re-entry: pacquet drops the +/// edge entirely (returns `Done(None)`) so the parent's `children` map +/// omits the cycled child — same shape as upstream's +/// [`parentIdsContainSequence`](https://github.com/pnpm/pnpm/blob/097983fbca/installing/deps-resolver/src/resolveDependencyTree.ts#L378) +/// gate in `buildTree`. Without this, two nodes for the same id race +/// each other into `graph.insert`, and an empty-children entry for the +/// cycled occurrence can overwrite the real one. +#[expect( + clippy::too_many_arguments, + reason = "internal walker helper threading per-node context through the recursion" +)] +#[async_recursion] +async fn resolve_node_seed( + ctx: &TreeCtx, + resolver: &Chain, + wanted: WantedDependency, + ancestor_ids: &[String], + depth: i32, + parent_optional: bool, + reuse: ReuseSource, + pick_overlay: Option>, +) -> Result where Chain: Resolver + ?Sized, { @@ -1113,7 +1231,8 @@ where current_is_optional, reused, ) - .await; + .await + .map(NodeSeed::Done); } // Memoise the per-wanted resolve. The first caller for a given @@ -1157,6 +1276,34 @@ where // versions never share a `currentPkg`-dependent result. let project_scope = is_project_relative_specifier(wanted.bare_specifier.as_deref()) .then(|| ctx.base_opts.project_dir.clone()); + // The overlay's view for this edge joins the cache key: the same + // range can legitimately pick different versions under levels + // that resolved different siblings. The view keeps each candidate + // name (alias, `npm:` inner target, folded `jsr:` name) paired + // with its versions — the picker consults the overlay per name, + // so a flat union of versions could collide two overlays that + // distribute the same versions across different names. Empty for + // almost every edge, so the dedup keeps working where it matters. + let overlay_versions: Vec<(String, Vec)> = pick_overlay + .as_ref() + .map(|overlay| { + let mut view: Vec<(String, Vec)> = overlay_lookup_names(&wanted) + .into_iter() + .filter_map(|name| { + let mut versions: Vec = + overlay.versions_for(&name).into_iter().map(str::to_string).collect(); + if versions.is_empty() { + return None; + } + versions.sort_unstable(); + versions.dedup(); + Some((name, versions)) + }) + .collect(); + view.sort_unstable(); + view + }) + .unwrap_or_default(); let cache_key: WantedKey = ( wanted.alias.clone(), wanted.bare_specifier.clone(), @@ -1166,64 +1313,11 @@ where opts.published_by, project_scope, prior_key.clone(), + overlay_versions.clone(), ); - let cached = - lock_recoverable(&ctx.workspace.resolved_by_wanted).get(&cache_key).map(Arc::clone); - let result = if let Some(result) = cached { - result - } else { - let mut result = resolver - .resolve(&wanted, opts) - .await - .map_err(|err: ResolveError| ResolveDependencyTreeError::Resolve(err.to_string()))?; - let Some(result_inner) = result.as_mut() else { - return Err(ResolveDependencyTreeError::SpecNotSupported { - specifier: render_specifier(&wanted), - }); - }; - // Apply the configured `readPackageHook` (today: - // `packageExtensions`) to the manifest fragment before - // anything downstream sees it. Mirrors upstream's - // [`ctx.readPackageHook(pkg)`](https://github.com/pnpm/pnpm/blob/39101f5e37/installing/deps-resolver/src/resolveDependencies.ts#L1481-L1483) - // 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.workspace.manifest_hook.as_ref() - && let Some(manifest) = result_inner.manifest.take() - { - result_inner.manifest = Some(hook(manifest)); - } - - if let Some(pnpmfile_hook) = ctx.workspace.pnpmfile_hook.as_ref() - && let Some(manifest) = result_inner.manifest.take() - { - let log = ctx.workspace.read_package_log.clone().unwrap_or_else(|| Arc::new(|_| {})); - let hook_ctx = pacquet_hooks::HookContext { log }; - - let updated = pnpmfile_hook - .read_package((*manifest).clone(), hook_ctx) - .await - .map_err(ResolveDependencyTreeError::PnpmfileHook)?; - result_inner.manifest = Some(updated); - } - - if ctx.workspace.auto_install_peers - && let Some(manifest) = result_inner.manifest.take() - { - result_inner.manifest = Some(omit_peer_shadowed_dependencies(manifest)); - } - - let result = result.expect("Some-guarded above"); - // Wrap in `Arc` once so the cache, the per-id - // `ResolvedPackage` envelope, and the later peer-resolved - // graph node share one heap-allocated `ResolveResult` - // instead of cloning every `String` field per occurrence. - let result = Arc::new(result); - lock_recoverable(&ctx.workspace.resolved_by_wanted) - .entry(cache_key) - .or_insert_with(|| Arc::clone(&result)); - result - }; + let result = + resolve_wanted_cached(ctx, resolver, &wanted, opts, pick_overlay.as_ref(), cache_key) + .await?; if let Some(violation) = result.policy_violation.clone() { lock_recoverable(&ctx.workspace.policy_violations).push(violation); @@ -1247,7 +1341,7 @@ where // Cycle break — see the doc comment above. if ancestor_ids.iter().any(|prev| prev == &id) { - return Ok(None); + return Ok(NodeSeed::Done(None)); } let alias = result @@ -1324,9 +1418,53 @@ where let next_ancestors = Arc::new(next_ancestors); let children_owner = claim_children_owner(ctx, &id, depth, ancestor_ids); - // Only the deterministic children owner walks this package's - // manifest children. Other occurrences stay lazy and expand from - // `children_by_id`, applying their own `parent_ids` cycle break. + Ok(NodeSeed::Pending(Box::new(PendingNode { + result, + id, + alias, + node_id, + is_link, + next_ancestors, + children_owner, + depth, + current_is_optional, + prior_key, + }))) +} + +/// Walk a seeded node's children: the second half of the old +/// monolithic walk. `children_overlay` is the preferred-versions +/// overlay covering this node's own level (built by the caller from +/// every sibling seed); the grandchildren's overlay layers this +/// node's resolved children on top, mirroring upstream's per-level +/// fold at +/// [`resolveDependencies.ts#L717-L746`](https://github.com/pnpm/pnpm/blob/ce9c096e8e/installing/deps-resolver/src/resolveDependencies.ts#L717-L746). +/// +/// Only the deterministic children owner walks this package's +/// manifest children. Other occurrences stay lazy and expand from +/// `children_by_id`, applying their own `parent_ids` cycle break. +#[async_recursion] +async fn walk_node_children( + ctx: &TreeCtx, + resolver: &Chain, + pending: PendingNode, + children_overlay: Option>, +) -> Result, ResolveDependencyTreeError> +where + Chain: Resolver + ?Sized, +{ + let PendingNode { + result, + id, + alias, + node_id, + is_link, + next_ancestors, + children_owner, + depth, + current_is_optional, + prior_key, + } = pending; let children = if is_link { // Linked nodes don't walk their manifest's deps — see the // `is_link` comment block above. Empty `Realized` map matches @@ -1363,7 +1501,11 @@ where .as_ref() .filter(|key| landed_on_prior_entry(key, &id)) .and_then(|key| ctx.workspace.wanted_lockfile.as_ref()?.snapshots.as_ref()?.get(key)); - let child_results = child_specs + // Phase 1: resolve every child package before any grandchild + // walk starts, so the level's resolved versions can feed the + // grandchildren's preferred-versions overlay — upstream's + // postponed-resolution barrier. + let child_seeds = child_specs .iter() .map(|(child_name, child_range, child_optional)| { let child_wanted = WantedDependency { @@ -1375,8 +1517,9 @@ where let child_prior = prior_children_snapshot .and_then(|snapshot| prior_child_key(snapshot, child_name, child_range)); let next_ancestors = Arc::clone(&next_ancestors); + let pick_overlay = children_overlay.clone(); async move { - resolve_node( + let seed = resolve_node_seed( ctx, resolver, child_wanted, @@ -1384,8 +1527,32 @@ where depth + 1, current_is_optional, ReuseSource::PriorOnly { key: child_prior }, + pick_overlay, ) - .await + .await?; + warm_children_resolutions(ctx, resolver, &seed).await; + Ok::(seed) + } + }) + .pipe(future::try_join_all) + .await?; + let grandchild_overlay = PreferredVersionsOverlay::layer( + children_overlay.clone(), + level_versions(ctx, &child_seeds), + ); + // Phase 2: walk each child's own children with the extended + // overlay. + let child_results = child_seeds + .into_iter() + .map(|seed| { + let overlay = grandchild_overlay.clone(); + async move { + match seed { + NodeSeed::Done(dep) => Ok(dep), + NodeSeed::Pending(pending) => { + walk_node_children(ctx, resolver, *pending, overlay).await + } + } } }) .pipe(future::try_join_all) @@ -1461,6 +1628,223 @@ fn landed_on_prior_entry(prior_key: &PkgNameVerPeer, resolved_pkg_id: &str) -> b prior_key.without_peer().to_string() == pacquet_deps_path::remove_suffix(resolved_pkg_id) } +/// The package names the npm picker may consult the preferred-versions +/// overlay under for one wanted edge: the alias itself, plus the inner +/// target of an `npm:` alias and the folded `@jsr/...` name of a +/// `jsr:` specifier — mirroring the name derivation in the npm +/// resolver's `parse_bare_specifier`, which keys its overlay merge by +/// the resolved `spec.name` rather than the outer alias. +fn overlay_lookup_names(wanted: &WantedDependency) -> Vec { + let mut names: Vec = Vec::new(); + if let Some(alias) = wanted.alias.as_deref() + && !alias.is_empty() + { + names.push(alias.to_string()); + } + let Some(bare) = wanted.bare_specifier.as_deref() else { return names }; + if let Some(rest) = bare.strip_prefix("npm:") { + let alias_keeps_name = wanted + .alias + .as_deref() + .is_some_and(|alias| !alias.is_empty() && rest.parse::().is_ok()); + if !alias_keeps_name { + let last_at = + rest.bytes().enumerate().rev().find_map(|(i, b)| (b == b'@').then_some(i)); + let inner = match last_at { + Some(idx) if idx >= 1 => &rest[..idx], + _ => rest, + }; + if !inner.is_empty() && !names.iter().any(|name| name == inner) { + names.push(inner.to_string()); + } + } + } else if bare.starts_with("jsr:") + && let Ok(Some(spec)) = pacquet_resolving_jsr_specifier_parser::parse_jsr_specifier( + bare, + wanted.alias.as_deref(), + ) + && !names.contains(&spec.npm_pkg_name) + { + names.push(spec.npm_pkg_name); + } + names +} + +/// Look the wanted edge up in the per-wanted dedup cache or run the +/// resolver chain and the manifest-hook pipeline, caching the +/// `Arc` under `cache_key`. Concurrent first-callers +/// can both miss and resolve in parallel — the resolver's own +/// per-cache-key fetch locker coalesces the network work, and the +/// second `or_insert` loses the race harmlessly. +async fn resolve_wanted_cached( + ctx: &TreeCtx, + resolver: &Chain, + wanted: &WantedDependency, + opts: &ResolveOptions, + pick_overlay: Option<&Arc>, + cache_key: WantedKey, +) -> Result, ResolveDependencyTreeError> +where + Chain: Resolver + ?Sized, +{ + let cached = + lock_recoverable(&ctx.workspace.resolved_by_wanted).get(&cache_key).map(Arc::clone); + if let Some(result) = cached { + return Ok(result); + } + let overlay_opts; + let opts = if cache_key.8.is_empty() { + opts + } else { + let mut owned = opts.clone(); + owned.preferred_versions_overlay = pick_overlay.map(Arc::clone); + overlay_opts = owned; + &overlay_opts + }; + let mut result = resolver + .resolve(wanted, opts) + .await + .map_err(|err: ResolveError| ResolveDependencyTreeError::Resolve(err.to_string()))?; + let Some(result_inner) = result.as_mut() else { + return Err(ResolveDependencyTreeError::SpecNotSupported { + specifier: render_specifier(wanted), + }); + }; + // Apply the configured `readPackageHook` (today: + // `packageExtensions`) to the manifest fragment before + // anything downstream sees it. Mirrors upstream's + // [`ctx.readPackageHook(pkg)`](https://github.com/pnpm/pnpm/blob/39101f5e37/installing/deps-resolver/src/resolveDependencies.ts#L1481-L1483) + // 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.workspace.manifest_hook.as_ref() + && let Some(manifest) = result_inner.manifest.take() + { + result_inner.manifest = Some(hook(manifest)); + } + + if let Some(pnpmfile_hook) = ctx.workspace.pnpmfile_hook.as_ref() + && let Some(manifest) = result_inner.manifest.take() + { + let log = ctx.workspace.read_package_log.clone().unwrap_or_else(|| Arc::new(|_| {})); + let hook_ctx = pacquet_hooks::HookContext { log }; + + let updated = pnpmfile_hook + .read_package((*manifest).clone(), hook_ctx) + .await + .map_err(ResolveDependencyTreeError::PnpmfileHook)?; + result_inner.manifest = Some(updated); + } + + if ctx.workspace.auto_install_peers + && let Some(manifest) = result_inner.manifest.take() + { + result_inner.manifest = Some(omit_peer_shadowed_dependencies(manifest)); + } + + let result = result.expect("Some-guarded above"); + // Wrap in `Arc` once so the cache, the per-id + // `ResolvedPackage` envelope, and the later peer-resolved + // graph node share one heap-allocated `ResolveResult` + // instead of cloning every `String` field per occurrence. + let result = Arc::new(result); + lock_recoverable(&ctx.workspace.resolved_by_wanted) + .entry(cache_key) + .or_insert_with(|| Arc::clone(&result)); + Ok(result) +} + +/// Speculatively warm a freshly-seeded node's children resolutions so +/// their packuments download while the sibling level's barrier waits +/// for its slowest member. Results are discarded — the real picks run +/// in the walk phase with the level's preferred-versions overlay and +/// hit the warm metadata caches — and errors are swallowed: a +/// speculative fetch must never fail the install (the real resolve +/// will surface it). Recovers the cross-level pipelining the +/// postponed-resolution barrier otherwise serializes; pure overlap, +/// no behavioral effect. +async fn warm_children_resolutions(ctx: &TreeCtx, resolver: &Chain, seed: &NodeSeed) +where + Chain: Resolver + ?Sized, +{ + // A configured pnpmfile hook is externally observable per call + // (`readPackage` IPC, `context.log`, custom resolvers), so + // speculative resolutions must not fire it; the pure in-memory + // manifest hook (packageExtensions / overrides) is idempotent and + // cache-deduped, indistinguishable from a first-caller win in the + // pre-existing concurrent-miss race. + if ctx.workspace.pnpmfile_hook.is_some() { + return; + } + let NodeSeed::Pending(pending) = seed else { return }; + if pending.is_link || !pending.children_owner.owns_children { + return; + } + let Ok(specs) = extract_children(&pending.result) else { return }; + let opts = ctx.opts_for_depth(pending.depth + 1); + specs + .iter() + .map(|(name, range, optional)| { + let wanted = WantedDependency { + alias: Some(name.clone()), + bare_specifier: Some(range.clone()), + optional: Some(*optional), + ..WantedDependency::default() + }; + async move { + // Warm through the same per-wanted dedup cache, under + // the empty-overlay-view key: when the real pick's + // view is empty too (the overwhelmingly common case) + // it reuses this entry outright; otherwise it misses + // into its own bucket and re-picks from the warm + // metadata caches. + let project_scope = is_project_relative_specifier(wanted.bare_specifier.as_deref()) + .then(|| ctx.base_opts.project_dir.clone()); + let cache_key: WantedKey = ( + wanted.alias.clone(), + wanted.bare_specifier.clone(), + wanted.optional, + wanted.injected, + opts.pick_lowest_version, + opts.published_by, + project_scope, + // No prior-lockfile key: a warm entry must only be + // reused by edges that carry no currentPkg either. + None, + Vec::new(), + ); + let _ = resolve_wanted_cached(ctx, resolver, &wanted, opts, None, cache_key).await; + } + }) + .pipe(future::join_all) + .await; +} + +/// The `(name → versions)` additions one resolved level contributes +/// to its children's preferred-versions overlay. Linked nodes carry no +/// `name_ver` and contribute nothing, mirroring upstream's +/// linked-dependency skip in the fold. +fn level_versions(ctx: &TreeCtx, seeds: &[NodeSeed]) -> BTreeMap> { + let packages = lock_recoverable(&ctx.workspace.packages); + let mut level: BTreeMap> = BTreeMap::new(); + for seed in seeds { + let name_ver = match seed { + NodeSeed::Pending(pending) => pending.result.name_ver.as_ref(), + NodeSeed::Done(Some(dep)) => { + packages.get(&dep.id).and_then(|pkg| pkg.result.name_ver.as_ref()) + } + NodeSeed::Done(None) => None, + }; + let Some(name_ver) = name_ver else { continue }; + let versions = level.entry(name_ver.name.to_string()).or_default(); + let version = name_ver.suffix.to_string(); + if !versions.contains(&version) { + versions.push(version); + } + } + level +} + /// One reusable node: its prior-lockfile snapshot key plus the /// `ResolveResult` synthesized from the lockfile metadata. struct ReusedNode { @@ -1676,6 +2060,9 @@ where opts.published_by, project_scope, Some(key.clone()), + // Reused resolutions are exact pins — preference overlays + // can't change the pick, so the no-overlay bucket is right. + Vec::new(), ); lock_recoverable(&ctx.workspace.resolved_by_wanted) .entry(cache_key) diff --git a/pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs b/pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs index 61fb1a63a7..e352fdd16a 100644 --- a/pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs +++ b/pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs @@ -246,10 +246,7 @@ where /// 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. +/// dedup carries across importers in a workspace install. pub async fn resolve_importer_with_workspace( resolver: &Chain, importer_id: &str, @@ -263,92 +260,188 @@ where DependencyGroupList: IntoIterator, Chain: Resolver + ?Sized, { - let ResolveImporterOptions { - auto_install_peers, - auto_install_peers_from_highest_match, - resolve_peers_from_workspace_root, - dedupe_peers, - mut all_preferred_versions, - patched_dependencies, - base_opts, - pick_lowest_direct, - subdep_published_by, - catalogs, - exclude_links_from_lockfile, - lockfile_dir, - modules_dir, - peers_suffix_max_length, - catalog_server: _, - // `manifest_hook` and `pnpmfile_hook` are workspace-wide; they live - // on the shared [`WorkspaceTreeCtx`] and the caller (`resolve_importer` - // or `resolve_workspace`) is responsible for setting them there before - // handing the `Arc` to this function. - manifest_hook: _, - pnpmfile_hook: _, - } = opts; - let peers_opts = || ResolvePeersOptions { - peers_suffix_max_length, - dedupe_peers, - exclude_links_from_lockfile, - lockfile_dir: lockfile_dir.clone(), - modules_dir: modules_dir.clone(), - hoist_missing_scope: None, - }; - - let ctx = TreeCtx::with_workspace(workspace, base_opts) - .with_importer_id(importer_id) - .with_importer_order(importer_order) - .with_patched_dependencies(patched_dependencies) - .with_resolution_mode(pick_lowest_direct, subdep_published_by); - - let initial_wanted = - importer_direct_wanted_specs(manifest, dependency_groups, auto_install_peers, &catalogs)?; - let mut direct = extend_tree(&ctx, resolver, initial_wanted, importer_id).await?; - // Both hoists read the run-extended preferred-versions map: - // upstream folds every resolved package's version into - // `ctx.allPreferredVersions` - // ([`resolveDependencies.ts#L1483-L1488`](https://github.com/pnpm/pnpm/blob/01b3d45ddb/installing/deps-resolver/src/resolveDependencies.ts#L1483-L1488)) - // and consults it for the optional hoist after each wave. What - // keeps e.g. `debug`'s `supports-color` from being hoisted against - // a deep-tree provider is not the map but the missing-peer set: - // meta-only peers never enter it (see `partition_missing_peers`). - update_preferred_versions_with_ctx(&ctx, &mut all_preferred_versions); - - let mut parent_pkg_aliases: HashSet = - direct.iter().map(|dep| dep.alias.clone()).collect(); - let mut all_missing_optional_peers: BTreeMap> = BTreeMap::new(); - - // The hoist input must not see missing peers declared inside a - // subtree owned by another importer's shared children context — - // upstream reuses the owner walk's children report there, so those - // peers never reach a non-owner importer's hoist. The final - // per-importer pass below keeps the unscoped options so warnings - // stay complete. - // Snapshotted per peer pass because auto-hoisting can extend the - // tree and move children ownership to a shallower occurrence. - let hoist_peers_opts = || { - let mut opts = peers_opts(); - opts.hoist_missing_scope = Some(Arc::new(crate::resolve_peers::HoistMissingScope { - importer_id: importer_id.to_string(), - first_importer_by_pkg: ctx.workspace().first_importer_by_pkg(), - first_walk_missing_by_pkg: ctx.workspace().first_walk_missing_by_pkg(), - })); - opts - }; + let mut state = ImporterHoistState::init( + resolver, + importer_id, + importer_order, + manifest, + dependency_groups, + opts, + workspace, + ) + .await?; loop { + state.run_required_round(resolver).await?; + if !state.hoist_optional_round(resolver).await? { + break; + } + } + Ok(state.into_result()) +} + +/// One importer's resolution state across the workspace's hoist +/// rounds. The multi-importer orchestrator +/// [`fn@crate::resolve_workspace`] initializes every importer before +/// running any hoist round, mirroring upstream's +/// [`resolveRootDependencies`](https://github.com/pnpm/pnpm/blob/ce9c096e8e/installing/deps-resolver/src/resolveDependencies.ts#L335-L445) +/// barrier: every importer's initial wave completes first, then +/// per-round required-peer loops and one optional-peer hoist per +/// round repeat across all importers until no importer hoists — so an +/// optional-peer pick sees every importer's resolved versions, not +/// just the importers processed so far. +pub(crate) struct ImporterHoistState { + importer_id: String, + ctx: TreeCtx, + direct: Vec, + parent_pkg_aliases: HashSet, + all_missing_optional_peers: BTreeMap>, + all_preferred_versions: PreferredVersions, + auto_install_peers: bool, + auto_install_peers_from_highest_match: bool, + resolve_peers_from_workspace_root: bool, + peers_suffix_max_length: usize, + dedupe_peers: bool, + exclude_links_from_lockfile: bool, + lockfile_dir: Option, + modules_dir: Option, +} + +impl ImporterHoistState { + /// Resolve the importer's initial direct-dependency wave and set + /// up the hoist-round state. + pub(crate) async fn init( + resolver: &Chain, + importer_id: &str, + importer_order: usize, + manifest: &PackageManifest, + dependency_groups: DependencyGroupList, + opts: ResolveImporterOptions, + workspace: Arc, + ) -> Result + where + DependencyGroupList: IntoIterator, + Chain: Resolver + ?Sized, + { + let ResolveImporterOptions { + auto_install_peers, + auto_install_peers_from_highest_match, + resolve_peers_from_workspace_root, + dedupe_peers, + all_preferred_versions, + patched_dependencies, + base_opts, + pick_lowest_direct, + subdep_published_by, + catalogs, + exclude_links_from_lockfile, + lockfile_dir, + modules_dir, + peers_suffix_max_length, + catalog_server: _, + // `manifest_hook` and `pnpmfile_hook` are workspace-wide; they live + // on the shared [`WorkspaceTreeCtx`] and the caller (`resolve_importer` + // or `resolve_workspace`) is responsible for setting them there before + // handing the `Arc` to this function. + manifest_hook: _, + pnpmfile_hook: _, + } = opts; + + let ctx = TreeCtx::with_workspace(workspace, base_opts) + .with_importer_id(importer_id) + .with_importer_order(importer_order) + .with_patched_dependencies(patched_dependencies) + .with_resolution_mode(pick_lowest_direct, subdep_published_by); + + let initial_wanted = importer_direct_wanted_specs( + manifest, + dependency_groups, + auto_install_peers, + &catalogs, + )?; + let direct = extend_tree(&ctx, resolver, initial_wanted, importer_id).await?; + let parent_pkg_aliases: HashSet = + direct.iter().map(|dep| dep.alias.clone()).collect(); + Ok(ImporterHoistState { + importer_id: importer_id.to_string(), + ctx, + direct, + parent_pkg_aliases, + all_missing_optional_peers: BTreeMap::new(), + all_preferred_versions, + auto_install_peers, + auto_install_peers_from_highest_match, + resolve_peers_from_workspace_root, + peers_suffix_max_length, + dedupe_peers, + exclude_links_from_lockfile, + lockfile_dir, + modules_dir, + }) + } + + fn peers_opts(&self) -> ResolvePeersOptions { + ResolvePeersOptions { + peers_suffix_max_length: self.peers_suffix_max_length, + dedupe_peers: self.dedupe_peers, + exclude_links_from_lockfile: self.exclude_links_from_lockfile, + lockfile_dir: self.lockfile_dir.clone(), + modules_dir: self.modules_dir.clone(), + hoist_missing_scope: None, + } + } + + /// Resolve the importer's missing *required* peers to a fixpoint, + /// rebuilding the missing-*optional* buckets the round's + /// [`Self::hoist_optional_round`] consumes. Upstream's inner + /// `while` per importer per round. + pub(crate) async fn run_required_round( + &mut self, + resolver: &Chain, + ) -> Result<(), ResolveImporterError> + where + Chain: Resolver + ?Sized, + { + // Both hoists read the run-extended preferred-versions map: + // upstream folds every resolved package's version into + // `ctx.allPreferredVersions` + // ([`resolveDependencies.ts#L1483-L1488`](https://github.com/pnpm/pnpm/blob/01b3d45ddb/installing/deps-resolver/src/resolveDependencies.ts#L1483-L1488)) + // and consults it for the optional hoist after each wave. + // Refreshed per round so it carries every importer's versions, + // not just this importer's. + update_preferred_versions_with_ctx(&self.ctx, &mut self.all_preferred_versions); + // The hoist input must not see missing peers declared inside a + // subtree owned by another importer's shared children context — + // upstream reuses the owner walk's children report there, so + // those peers never reach a non-owner importer's hoist. The + // final peer pass keeps the unscoped options so warnings stay + // complete. + self.all_missing_optional_peers.clear(); loop { - let mut snapshot = ctx.snapshot(direct.clone()); - let peers_result = resolve_peers(&mut snapshot, hoist_peers_opts()); - ctx.workspace() - .record_first_walk_missing(importer_id, &peers_result.missing_names_by_pkg); + let mut snapshot = self.ctx.snapshot(self.direct.clone()); + let peers_result = { + let mut opts = self.peers_opts(); + // Re-snapshotted per peer pass because auto-hoisting + // can extend the tree and move children ownership to a + // shallower occurrence. + opts.hoist_missing_scope = + Some(Arc::new(crate::resolve_peers::HoistMissingScope { + importer_id: self.importer_id.clone(), + first_importer_by_pkg: self.ctx.workspace().first_importer_by_pkg(), + first_walk_missing_by_pkg: self.ctx.workspace().first_walk_missing_by_pkg(), + })); + resolve_peers(&mut snapshot, opts) + }; + self.ctx + .workspace() + .record_first_walk_missing(&self.importer_id, &peers_result.missing_names_by_pkg); let (missing_required, fresh_optional) = partition_missing_peers( &peers_result.peer_dependency_issues.missing, - &parent_pkg_aliases, - auto_install_peers_from_highest_match, + &self.parent_pkg_aliases, + self.auto_install_peers_from_highest_match, ); for (name, ranges) in fresh_optional { - let bucket = all_missing_optional_peers.entry(name).or_default(); + let bucket = self.all_missing_optional_peers.entry(name).or_default(); for range in ranges { if !bucket.iter().any(|existing| existing == &range) { bucket.push(range); @@ -360,8 +453,8 @@ where break; } - let workspace_root_deps = if resolve_peers_from_workspace_root { - build_workspace_root_deps(&direct, &snapshot) + let workspace_root_deps = if self.resolve_peers_from_workspace_root { + build_workspace_root_deps(&self.direct, &snapshot) } else { Vec::new() }; @@ -370,8 +463,8 @@ where missing_required.iter().map(|(n, info)| (n.clone(), info.clone())).collect(); let hoisted = hoist_peers( &HoistPeersOptions { - auto_install_peers, - all_preferred_versions: &all_preferred_versions, + auto_install_peers: self.auto_install_peers, + all_preferred_versions: &self.all_preferred_versions, workspace_root_deps: &workspace_root_deps, }, &missing_as_pairs, @@ -381,7 +474,7 @@ where } for name in hoisted.keys() { - parent_pkg_aliases.insert(name.clone()); + self.parent_pkg_aliases.insert(name.clone()); } // Hoisted required peers are installed at the importer @@ -396,21 +489,36 @@ where // threading the per-dep meta. let new_wanted: Vec = hoisted.into_iter().map(|(name, range)| (name, range, false, false)).collect(); - let new_direct = extend_tree(&ctx, resolver, new_wanted, importer_id).await?; - direct.extend(new_direct); - update_preferred_versions_with_ctx(&ctx, &mut all_preferred_versions); + let new_direct = + extend_tree(&self.ctx, resolver, new_wanted, &self.importer_id).await?; + self.direct.extend(new_direct); + update_preferred_versions_with_ctx(&self.ctx, &mut self.all_preferred_versions); } + Ok(()) + } - if all_missing_optional_peers.is_empty() { - break; + /// Hoist this round's missing optional peers; `true` when any were + /// installed (the workspace runs another round). Upstream's + /// `getHoistableOptionalPeers` arm per importer per round. + pub(crate) async fn hoist_optional_round( + &mut self, + resolver: &Chain, + ) -> Result + where + Chain: Resolver + ?Sized, + { + if self.all_missing_optional_peers.is_empty() { + return Ok(false); } - let hoisted_optional = - get_hoistable_optional_peers(&all_missing_optional_peers, &all_preferred_versions); + let hoisted_optional = get_hoistable_optional_peers( + &self.all_missing_optional_peers, + &self.all_preferred_versions, + ); if hoisted_optional.is_empty() { - break; + return Ok(false); } for name in hoisted_optional.keys() { - parent_pkg_aliases.insert(name.clone()); + self.parent_pkg_aliases.insert(name.clone()); } // Optional peers picked up via `getHoistableOptionalPeers` are // also installed at the importer level — the picker already @@ -419,15 +527,27 @@ where // also defaults to `false` for the same reason. let new_wanted: Vec = hoisted_optional.into_iter().map(|(name, range)| (name, range, false, false)).collect(); - let new_direct = extend_tree(&ctx, resolver, new_wanted, importer_id).await?; - direct.extend(new_direct); - update_preferred_versions_with_ctx(&ctx, &mut all_preferred_versions); - all_missing_optional_peers.clear(); + let new_direct = extend_tree(&self.ctx, resolver, new_wanted, &self.importer_id).await?; + self.direct.extend(new_direct); + update_preferred_versions_with_ctx(&self.ctx, &mut self.all_preferred_versions); + Ok(true) } - let mut resolved_tree = ctx.into_resolved_tree(direct); - let peers_result = resolve_peers(&mut resolved_tree, peers_opts()); - Ok(ResolveImporterResult { resolved_tree, peers_result }) + /// The importer's direct-dep envelopes for the workspace-wide peer + /// pass (which recomputes peers across importers; the per-importer + /// pass would be discarded). + pub(crate) fn into_direct(self) -> Vec { + self.direct + } + + /// Run the final per-importer peer pass and emit the result. Used + /// by the single-importer entry points. + fn into_result(self) -> ResolveImporterResult { + let peers_opts = self.peers_opts(); + let mut resolved_tree = self.ctx.into_resolved_tree(self.direct); + let peers_result = resolve_peers(&mut resolved_tree, peers_opts); + ResolveImporterResult { resolved_tree, peers_result } + } } /// Split the missing-peer report into the inputs the inner and outer diff --git a/pacquet/crates/resolving-deps-resolver/src/resolve_workspace.rs b/pacquet/crates/resolving-deps-resolver/src/resolve_workspace.rs index 54a88a4c2c..4c7ac1e8f7 100644 --- a/pacquet/crates/resolving-deps-resolver/src/resolve_workspace.rs +++ b/pacquet/crates/resolving-deps-resolver/src/resolve_workspace.rs @@ -21,10 +21,7 @@ use crate::{ resolve_dependency_tree::{ ManifestHook, UpdateReuseScope, WorkspaceTreeCtx, importer_direct_wanted_specs, }, - resolve_importer::{ - ResolveImporterError, ResolveImporterOptions, ResolveImporterResult, - resolve_importer_with_workspace, - }, + resolve_importer::{ImporterHoistState, ResolveImporterError, ResolveImporterOptions}, resolve_peers::{ ImporterPeerInput, ResolvePeersOptions, WorkspaceResolvePeersResult, resolve_peers_workspace, @@ -212,38 +209,61 @@ where maximum_published_by }; - let mut per_importer_inputs: Vec = Vec::with_capacity(importers.len()); + // Phase 1: every importer's initial wave resolves before any peer + // hoist runs, then hoist rounds repeat across all importers until + // none hoists — upstream's `resolveRootDependencies` barrier, so + // an optional-peer pick sees every importer's resolved versions. + let mut states = Vec::with_capacity(importers.len()); + let mut input_dirs = Vec::with_capacity(importers.len()); for (importer_order, (importer, mut importer_opts)) in importers.iter().zip(importer_opts).enumerate() { importer_opts.pick_lowest_direct = pick_lowest_direct; importer_opts.subdep_published_by = subdep_published_by; - 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.id, - importer_order, - importer.manifest, - dependency_groups.iter().copied(), - importer_opts, - Arc::clone(&workspace), - ) - .await?; - let direct = resolved_tree.direct; + input_dirs + .push((importer_opts.base_opts.project_dir.clone(), importer_opts.modules_dir.clone())); + states.push( + ImporterHoistState::init( + resolver, + &importer.id, + importer_order, + importer.manifest, + dependency_groups.iter().copied(), + importer_opts, + Arc::clone(&workspace), + ) + .await?, + ); + } + loop { + for state in &mut states { + state.run_required_round(resolver).await?; + } + let mut any_hoisted = false; + for state in &mut states { + any_hoisted |= state.hoist_optional_round(resolver).await?; + } + if !any_hoisted { + break; + } + } + let mut per_importer_inputs: Vec = Vec::with_capacity(importers.len()); + for ((importer, state), (project_dir, modules_dir)) in + importers.iter().zip(states).zip(input_dirs) + { per_importer_inputs.push(ImporterPeerInput { id: importer.id.clone(), - direct, + direct: state.into_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. + // Reclaim the workspace ctx now that every importer's state 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()), diff --git a/pacquet/crates/resolving-deps-resolver/src/tests.rs b/pacquet/crates/resolving-deps-resolver/src/tests.rs index b6086d3c55..b551633dc8 100644 --- a/pacquet/crates/resolving-deps-resolver/src/tests.rs +++ b/pacquet/crates/resolving-deps-resolver/src/tests.rs @@ -2863,3 +2863,200 @@ mod peer_own_dep_shadowing { assert_eq!(optional_peer.version, "*"); } } + +mod level_preferred_versions { + use super::{HashMap, Mutex, fake_manifest, fake_result}; + use crate::resolve_dependency_tree::{ResolveDependencyTreeOptions, resolve_dependency_tree}; + use pacquet_package_manifest::DependencyGroup; + use pacquet_resolving_resolver_base::{ + LatestQuery, ResolveError, ResolveFuture, ResolveLatestFuture, ResolveOptions, + ResolveResult, Resolver, WantedDependency, + }; + use pretty_assertions::assert_eq; + + /// Stub that records, per `(alias, range)`, the overlay's preferred + /// versions for the probe package `pinned` at resolve time — the + /// name a real picker would merge the overlay under. + struct OverlayRecordingResolver { + table: HashMap<(String, String), ResolveResult>, + seen_overlay: Mutex>>, + } + + impl Resolver for OverlayRecordingResolver { + fn resolve<'a>( + &'a self, + wanted: &'a WantedDependency, + opts: &'a ResolveOptions, + ) -> ResolveFuture<'a> { + let name = wanted.alias.clone().unwrap_or_default(); + let range = wanted.bare_specifier.clone().unwrap_or_default(); + let overlay_view: Vec = opts + .preferred_versions_overlay + .as_ref() + .map(|overlay| { + overlay.versions_for("pinned").into_iter().map(str::to_string).collect() + }) + .unwrap_or_default(); + self.seen_overlay.lock().unwrap().insert((name.clone(), range.clone()), overlay_view); + let result = self.table.get(&(name, range)).cloned(); + Box::pin(async move { Ok::<_, ResolveError>(result) }) + } + + fn resolve_latest<'a>( + &'a self, + _query: &'a LatestQuery, + _opts: &'a ResolveOptions, + ) -> ResolveLatestFuture<'a> { + Box::pin(async { Ok(None) }) + } + } + + /// The varint shape: `parent` pins `pinned@5.0.0` next to + /// `consumer`, whose own `pinned: ~5.0.0` child must see the + /// sibling-resolved `5.0.0` among its preferred versions — + /// upstream's per-level fold + /// (resolveDependencies.ts#L717-L746). The importer's own direct + /// deps resolve with no overlay. + #[tokio::test] + async fn child_resolution_prefers_parent_level_sibling_versions() { + let mut table = HashMap::new(); + table.insert( + ("parent".to_string(), "1.0.0".to_string()), + fake_result( + "parent", + "1.0.0", + serde_json::json!({ + "name": "parent", + "version": "1.0.0", + "dependencies": { "consumer": "1.0.0", "pinned": "5.0.0" }, + }), + ), + ); + table.insert( + ("consumer".to_string(), "1.0.0".to_string()), + fake_result( + "consumer", + "1.0.0", + serde_json::json!({ + "name": "consumer", + "version": "1.0.0", + "dependencies": { "pinned": "~5.0.0" }, + }), + ), + ); + for range in ["5.0.0", "~5.0.0"] { + table.insert( + ("pinned".to_string(), range.to_string()), + fake_result( + "pinned", + "5.0.0", + serde_json::json!({ "name": "pinned", "version": "5.0.0" }), + ), + ); + } + let resolver = OverlayRecordingResolver { table, seen_overlay: Mutex::new(HashMap::new()) }; + let (_tmp, manifest) = fake_manifest(serde_json::json!({ "parent": "1.0.0" })); + + resolve_dependency_tree( + &resolver, + &manifest, + [DependencyGroup::Prod], + ResolveDependencyTreeOptions { + base_opts: ResolveOptions::default(), + patched_dependencies: None, + manifest_hook: None, + pnpmfile_hook: None, + read_package_log: None, + auto_install_peers: false, + }, + ) + .await + .unwrap(); + + let seen = resolver.seen_overlay.lock().unwrap(); + assert_eq!( + seen.get(&("parent".to_string(), "1.0.0".to_string())), + Some(&Vec::new()), + "importer-level direct deps resolve with no overlay", + ); + assert_eq!( + seen.get(&("pinned".to_string(), "~5.0.0".to_string())), + Some(&vec!["5.0.0".to_string()]), + "the consumer's child sees its parent-level sibling's resolved version", + ); + } + + /// An `npm:` alias consults the overlay under its *inner* target + /// name — the name the npm picker resolves (`spec.name`), not the + /// outer alias — mirroring upstream, where the per-level fold keys + /// entries by the resolved package name. + #[tokio::test] + async fn npm_alias_child_consults_overlay_by_inner_name() { + let mut table = HashMap::new(); + table.insert( + ("parent".to_string(), "1.0.0".to_string()), + fake_result( + "parent", + "1.0.0", + serde_json::json!({ + "name": "parent", + "version": "1.0.0", + "dependencies": { "consumer": "1.0.0", "pinned": "5.0.0" }, + }), + ), + ); + table.insert( + ("consumer".to_string(), "1.0.0".to_string()), + fake_result( + "consumer", + "1.0.0", + serde_json::json!({ + "name": "consumer", + "version": "1.0.0", + "dependencies": { "renamed": "npm:pinned@~5.0.0" }, + }), + ), + ); + table.insert( + ("pinned".to_string(), "5.0.0".to_string()), + fake_result( + "pinned", + "5.0.0", + serde_json::json!({ "name": "pinned", "version": "5.0.0" }), + ), + ); + table.insert( + ("renamed".to_string(), "npm:pinned@~5.0.0".to_string()), + fake_result( + "pinned", + "5.0.0", + serde_json::json!({ "name": "pinned", "version": "5.0.0" }), + ), + ); + let resolver = OverlayRecordingResolver { table, seen_overlay: Mutex::new(HashMap::new()) }; + let (_tmp, manifest) = fake_manifest(serde_json::json!({ "parent": "1.0.0" })); + + resolve_dependency_tree( + &resolver, + &manifest, + [DependencyGroup::Prod], + ResolveDependencyTreeOptions { + base_opts: ResolveOptions::default(), + patched_dependencies: None, + manifest_hook: None, + pnpmfile_hook: None, + read_package_log: None, + auto_install_peers: false, + }, + ) + .await + .unwrap(); + + let seen = resolver.seen_overlay.lock().unwrap(); + assert_eq!( + seen.get(&("renamed".to_string(), "npm:pinned@~5.0.0".to_string())), + Some(&vec!["5.0.0".to_string()]), + "the npm: alias edge must carry the overlay so the picker can merge by the inner name", + ); + } +} diff --git a/pacquet/crates/resolving-npm-resolver/src/lib.rs b/pacquet/crates/resolving-npm-resolver/src/lib.rs index 657995156b..944a9b4e16 100644 --- a/pacquet/crates/resolving-npm-resolver/src/lib.rs +++ b/pacquet/crates/resolving-npm-resolver/src/lib.rs @@ -31,6 +31,7 @@ mod npm_resolver; mod parse_bare_specifier; mod pick_package; mod pick_package_from_meta; +mod preferred_overlay; mod registry_url; mod resolve_from_workspace; mod trust_checks; diff --git a/pacquet/crates/resolving-npm-resolver/src/named_registry_resolver.rs b/pacquet/crates/resolving-npm-resolver/src/named_registry_resolver.rs index b355bffc62..835db12b99 100644 --- a/pacquet/crates/resolving-npm-resolver/src/named_registry_resolver.rs +++ b/pacquet/crates/resolving-npm-resolver/src/named_registry_resolver.rs @@ -199,9 +199,13 @@ impl NamedRegistryResolver { opts: &ResolveOptions, optional: bool, ) -> Result, ResolveError> { + let overlay_selectors = + crate::preferred_overlay::overlay_merged_selectors(opts, &spec.name); let pick_opts = PickPackageOptions { registry, - preferred_version_selectors: opts.preferred_versions.get(&spec.name), + preferred_version_selectors: overlay_selectors + .as_ref() + .or_else(|| opts.preferred_versions.get(&spec.name)), published_by: opts.published_by, published_by_exclude: opts.published_by_exclude.as_ref(), pick_lowest_version: opts.pick_lowest_version, diff --git a/pacquet/crates/resolving-npm-resolver/src/npm_resolver.rs b/pacquet/crates/resolving-npm-resolver/src/npm_resolver.rs index 18167ce036..c0557fea07 100644 --- a/pacquet/crates/resolving-npm-resolver/src/npm_resolver.rs +++ b/pacquet/crates/resolving-npm-resolver/src/npm_resolver.rs @@ -365,9 +365,13 @@ impl NpmResolver { opts: &ResolveOptions, optional: bool, ) -> Result, ResolveError> { + let overlay_selectors = + crate::preferred_overlay::overlay_merged_selectors(opts, &spec.name); let pick_opts = PickPackageOptions { registry, - preferred_version_selectors: opts.preferred_versions.get(&spec.name), + preferred_version_selectors: overlay_selectors + .as_ref() + .or_else(|| opts.preferred_versions.get(&spec.name)), published_by: opts.published_by, published_by_exclude: opts.published_by_exclude.as_ref(), pick_lowest_version: opts.pick_lowest_version, diff --git a/pacquet/crates/resolving-npm-resolver/src/preferred_overlay.rs b/pacquet/crates/resolving-npm-resolver/src/preferred_overlay.rs new file mode 100644 index 0000000000..77ba89a1d1 --- /dev/null +++ b/pacquet/crates/resolving-npm-resolver/src/preferred_overlay.rs @@ -0,0 +1,27 @@ +use pacquet_resolving_resolver_base::{ + ResolveOptions, VersionSelectorEntry, VersionSelectorType, VersionSelectors, +}; + +/// The picker's preferred selectors for `name` with the per-level +/// overlay folded in: each overlay version joins as a plain `version` +/// selector — the shape upstream's per-level fold writes +/// ([`resolveDependencies.ts#L731-L733`](https://github.com/pnpm/pnpm/blob/ce9c096e8e/installing/deps-resolver/src/resolveDependencies.ts#L731-L733)). +/// `None` when no level resolved this name; callers then borrow the +/// static map directly, so the owned merge allocates only on the rare +/// overlay hit. +pub(crate) fn overlay_merged_selectors( + opts: &ResolveOptions, + name: &str, +) -> Option { + let versions = opts.preferred_versions_overlay.as_ref()?.versions_for(name); + if versions.is_empty() { + return None; + } + let mut selectors = opts.preferred_versions.get(name).cloned().unwrap_or_default(); + for version in versions { + selectors + .entry(version.to_string()) + .or_insert(VersionSelectorEntry::Plain(VersionSelectorType::Version)); + } + Some(selectors) +} diff --git a/pacquet/crates/resolving-resolver-base/src/lib.rs b/pacquet/crates/resolving-resolver-base/src/lib.rs index 1f17a8b317..2a6cf5bfcf 100644 --- a/pacquet/crates/resolving-resolver-base/src/lib.rs +++ b/pacquet/crates/resolving-resolver-base/src/lib.rs @@ -29,11 +29,11 @@ mod verifier; pub use publish_time::parse_packument_timestamp; pub use resolve::{ CurrentPkg, DIRECT_DEP_SELECTOR_WEIGHT, DependencyManifest, EXISTING_VERSION_SELECTOR_WEIGHT, - LatestInfo, LatestQuery, PkgResolutionId, PreferredVersions, ResolveError, ResolveFuture, - ResolveLatestFuture, ResolveOptions, ResolveResult, Resolver, SharedDependencyManifest, - UpdateBehavior, VersionSelectorEntry, VersionSelectorType, VersionSelectorWithWeight, - VersionSelectors, WantedDependency, WorkspacePackage, WorkspacePackages, - WorkspacePackagesByVersion, + LatestInfo, LatestQuery, PkgResolutionId, PreferredVersions, PreferredVersionsOverlay, + ResolveError, ResolveFuture, ResolveLatestFuture, ResolveOptions, ResolveResult, Resolver, + SharedDependencyManifest, UpdateBehavior, VersionSelectorEntry, VersionSelectorType, + VersionSelectorWithWeight, VersionSelectors, WantedDependency, WorkspacePackage, + WorkspacePackages, WorkspacePackagesByVersion, }; pub use verifier::{ ResolutionPolicyViolation, ResolutionVerification, ResolutionVerifier, VerifyCtx, VerifyFuture, diff --git a/pacquet/crates/resolving-resolver-base/src/resolve.rs b/pacquet/crates/resolving-resolver-base/src/resolve.rs index 9afbf0a757..f8e88746d5 100644 --- a/pacquet/crates/resolving-resolver-base/src/resolve.rs +++ b/pacquet/crates/resolving-resolver-base/src/resolve.rs @@ -145,6 +145,55 @@ pub enum VersionSelectorEntry { Weighted(VersionSelectorWithWeight), } +/// One resolution level's preferred-version additions, layered over a +/// parent level. Mirrors the prototype chain upstream builds per level +/// with +/// [`Object.create(preferredVersions)`](https://github.com/pnpm/pnpm/blob/ce9c096e8e/installing/deps-resolver/src/resolveDependencies.ts#L717-L746): +/// after a package's direct dependencies resolve, their `(name, +/// version)` pairs become plain `version` selectors for the children's +/// subtree resolutions, so a child's range prefers a version one of +/// its parent-level siblings pinned. Layering is O(1); lookups walk +/// the chain for one name. +#[derive(Debug)] +pub struct PreferredVersionsOverlay { + entries: BTreeMap>, + parent: Option>, +} + +impl PreferredVersionsOverlay { + /// Layer `entries` over `parent`. An empty layer collapses to the + /// parent so chains only grow when a level resolved something. + #[must_use] + pub fn layer( + parent: Option>, + entries: BTreeMap>, + ) -> Option> { + if entries.is_empty() { + return parent; + } + Some(Arc::new(PreferredVersionsOverlay { entries, parent })) + } + + /// Every version the chain prefers for `name`, nearest level + /// first. Empty for names no level resolved. + #[must_use] + pub fn versions_for(&self, name: &str) -> Vec<&str> { + let mut versions: Vec<&str> = Vec::new(); + let mut layer = Some(self); + while let Some(current) = layer { + if let Some(found) = current.entries.get(name) { + for version in found { + if !versions.contains(&version.as_str()) { + versions.push(version); + } + } + } + layer = current.parent.as_deref(); + } + versions + } +} + /// Selector weight applied to direct dependencies. Mirrors pnpm's /// [`DIRECT_DEP_SELECTOR_WEIGHT`](https://github.com/pnpm/pnpm/blob/3687b0e180/resolving/resolver-base/src/index.ts#L250). pub const DIRECT_DEP_SELECTOR_WEIGHT: u32 = 1_000; @@ -224,6 +273,10 @@ pub struct ResolveOptions { /// per importer — sharing the (potentially large) map keeps those /// clones to a refcount bump. pub preferred_versions: Arc, + /// Per-level preferred-version additions from the tree walk. See + /// [`PreferredVersionsOverlay`]. `None` outside the walk (importer + /// direct deps resolve against [`Self::preferred_versions`] only). + pub preferred_versions_overlay: Option>, pub workspace_packages: Option, pub default_tag: Option, pub pick_lowest_version: bool,