diff --git a/pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs b/pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs index d28a98ba74..026b63a401 100644 --- a/pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs +++ b/pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs @@ -78,11 +78,17 @@ pub struct ResolveImporterOptions { /// Seed for the preferred-versions tie-break table. The /// orchestrator extends this in place as packages are walked — /// each newly-resolved `name@version` lands as a plain - /// [`VersionSelectorType::Version`] entry so [`hoist_peers`] and - /// [`get_hoistable_optional_peers`] can reuse it. Pass the result - /// of `get_preferred_versions_from_lockfile_and_manifests` from - /// the `lockfile-preferred-versions` crate, or an empty map when - /// no lockfile + manifest seeding is available. + /// [`VersionSelectorType::Version`] entry so the [`hoist_peers`] + /// (required-peer) picker can reuse a version a sibling already + /// brought. The [`get_hoistable_optional_peers`] picker instead + /// reads a snapshot taken *before* any run-resolved version is + /// folded in — mirroring upstream's static + /// [`ctx.allPreferredVersions`](https://github.com/pnpm/pnpm/blob/894ea6af2c/installing/deps-resolver/src/resolveDependencies.ts#L340-L342) + /// — so an optional peer is never hoisted against a deep-tree + /// provider pnpm can't see. Pass the result of + /// `get_preferred_versions_from_lockfile_and_manifests` from the + /// `lockfile-preferred-versions` crate, or an empty map when no + /// lockfile + manifest seeding is available. pub all_preferred_versions: PreferredVersions, /// Configured `patchedDependencies`, grouped by package name. The @@ -292,6 +298,17 @@ where 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?; + // The optional-peer hoist must only consider versions that were + // already in scope before this run — the wanted lockfile + manifests + // — mirroring upstream's static + // [`ctx.allPreferredVersions`](https://github.com/pnpm/pnpm/blob/894ea6af2c/installing/deps-resolver/src/resolveDependencies.ts#L340-L342). + // Feeding freshly-resolved transitive versions into that decision + // would hoist an optional peer (e.g. `debug`'s `supports-color`) + // against a deep-tree provider pnpm never sees, resolving the peer + // where pnpm leaves it bare. The required-peer hoist keeps using the + // run-extended map below: a required peer is auto-installed either + // way, and reusing an in-tree version matches pnpm's picker dedup. + let optional_hoist_preferred_versions = all_preferred_versions.clone(); update_preferred_versions_with_ctx(&ctx, &mut all_preferred_versions); let mut parent_pkg_aliases: HashSet = @@ -365,8 +382,10 @@ where if all_missing_optional_peers.is_empty() { break; } - let hoisted_optional = - get_hoistable_optional_peers(&all_missing_optional_peers, &all_preferred_versions); + let hoisted_optional = get_hoistable_optional_peers( + &all_missing_optional_peers, + &optional_hoist_preferred_versions, + ); if hoisted_optional.is_empty() { break; } diff --git a/pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs b/pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs index f272b07e0b..f77923bc59 100644 --- a/pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs +++ b/pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs @@ -715,6 +715,71 @@ async fn auto_install_does_not_hoist_when_root_already_has_dep() { ); } +/// An *optional* peer that is only available deep in the freshly +/// resolved tree (brought by a sibling, not pinned in the wanted +/// lockfile) must NOT be hoisted, so the consumer's depPath stays bare. +/// `getHoistableOptionalPeers` reads upstream's static +/// `ctx.allPreferredVersions` (lockfile + manifests, empty on a fresh +/// install) — never the run-resolved versions. Feeding the latter in +/// would resolve the optional peer where pnpm leaves it unresolved, and +/// non-deterministically at that (the result would depend on resolution +/// order). Regression test for . +#[tokio::test] +async fn optional_peer_only_in_resolved_tree_is_not_hoisted() { + let mut table = HashMap::new(); + table.insert( + ("needs-opt".to_string(), "1.0.0".to_string()), + fake_result( + "needs-opt", + "1.0.0", + serde_json::json!({ + "name": "needs-opt", + "version": "1.0.0", + "peerDependencies": { "opt": "^1.0.0" }, + "peerDependenciesMeta": { "opt": { "optional": true } }, + }), + ), + ); + // `provider` pulls `opt@1.0.0` deep in the tree — a sibling of + // `needs-opt`, not an ancestor, so `opt` is out of `needs-opt`'s + // resolution scope. + table.insert( + ("provider".to_string(), "1.0.0".to_string()), + fake_result( + "provider", + "1.0.0", + serde_json::json!({ + "name": "provider", + "version": "1.0.0", + "dependencies": { "opt": "1.0.0" }, + }), + ), + ); + table.insert( + ("opt".to_string(), "1.0.0".to_string()), + fake_result("opt", "1.0.0", serde_json::json!({ "name": "opt", "version": "1.0.0" })), + ); + let resolver = StubResolver { table, calls: Mutex::new(Vec::new()) }; + let (_tmp, manifest) = fake_manifest(serde_json::json!({ + "needs-opt": "1.0.0", + "provider": "1.0.0", + })); + + let result = resolve_importer(&resolver, &manifest, [DependencyGroup::Prod], default_opts()) + .await + .unwrap(); + + // `opt` must not be hoisted to the importer, and `needs-opt`'s + // optional peer must stay unresolved — bare `needs-opt@1.0.0`. + let direct: Vec<&str> = + result.peers_result.direct_dependencies_by_alias.keys().map(String::as_str).collect(); + assert!(!direct.contains(&"opt"), "optional peer `opt` must not be hoisted: {direct:?}"); + assert_eq!( + result.peers_result.direct_dependencies_by_alias.get("needs-opt"), + Some(&DepPath::from("needs-opt@1.0.0".to_string())), + ); +} + /// Port of "don't install the same missing peer dependency twice": a /// transitive chain where each layer adds the same peer must produce a /// single hoisted entry. diff --git a/pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs b/pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs index b6db04707d..be32a6b64f 100644 --- a/pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs +++ b/pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs @@ -207,6 +207,7 @@ pub fn resolve_peers(tree: &mut ResolvedTree, opts: ResolvePeersOptions) -> Reso pure_pkgs: HashSet::new(), peers_cache: HashMap::new(), parent_pkgs_of_node: HashMap::new(), + node_records: HashMap::new(), }; walker.walk() } @@ -242,6 +243,7 @@ pub fn resolve_peers_workspace( pure_pkgs: HashSet::new(), peers_cache: HashMap::new(), parent_pkgs_of_node: HashMap::new(), + node_records: HashMap::new(), }; let mut direct_dependencies_by_importer: BTreeMap> = @@ -257,20 +259,30 @@ pub fn resolve_peers_workspace( 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); + walker.resolve_node(dep.node_id.clone(), &importer_parents, &parent_chain_names, 0); } - 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; + // Recompute depPaths with full peer suffixes once, after every + // importer is walked, then rebuild the graph and re-key each + // importer's direct deps. + let final_dep_paths = walker.build_final_dep_paths(); + for importer in importers { + let direct_by_alias: BTreeMap = importer + .direct + .iter() + .map(|dep| { + (dep.alias.clone(), walker.final_dep_path_of(&dep.node_id, &final_dep_paths)) + }) + .collect(); + direct_dependencies_by_importer.insert(importer.id.clone(), direct_by_alias); + } + let mut graph = walker.build_final_graph(&final_dep_paths); if dedupe_injected_deps_enabled { dedupe_injected_deps( @@ -401,6 +413,10 @@ struct Walker<'tree> { /// [`parentPkgsOfNode`](https://github.com/pnpm/pnpm/blob/c86c423bdc/installing/deps-resolver/src/resolvePeers.ts#L356) /// + [`parentPackagesMatch`](https://github.com/pnpm/pnpm/blob/c86c423bdc/installing/deps-resolver/src/resolvePeers.ts#L701-L731). parent_pkgs_of_node: HashMap>, + /// Per-`NodeId` snapshot captured at graph-insert time, consumed by + /// the post-walk [`Walker::build_final_dep_paths`] / + /// [`Walker::build_final_graph`] pass. See [`NodeRecord`]. + node_records: HashMap, } /// Per-peer-name snapshot stored on [`Walker::parent_pkgs_of_node`]. @@ -443,6 +459,30 @@ struct PendingPeerEdge { peer_node_id: NodeId, } +/// Per-`NodeId` data captured during the walk so the post-walk +/// [`Walker::build_final_dep_paths`] pass can recompute each node's +/// depPath with its resolved peers' *full* suffixes — matching pnpm's +/// deferred [`calculateDepPath`](https://github.com/pnpm/pnpm/blob/894ea6af2c/installing/deps-resolver/src/resolvePeers.ts#L629), +/// which awaits each pending peer's depPath and collapses to +/// `name@version` only for genuinely detected cycles. +/// +/// The walk itself is left untouched: it still computes the provisional +/// depPaths that [`Walker::find_hit`] reads, so peer-resolution and +/// cache decisions are byte-for-byte identical. Only the rendered +/// depPaths change, which is why a node whose suffix was previously +/// collapsed by the cycle fallback now splits into its own graph entry. +struct NodeRecord { + /// `alias → child/peer NodeId` edges, in the same shape the inline + /// `graph_children` map carries (children overlaid with resolved-peer + /// edges) but holding NodeIds, so the rebuild can map each edge to + /// its final depPath. + edges: BTreeMap, + transitive_peer_dependencies: HashSet, + depth: i32, + installable: bool, + is_pure: bool, +} + /// Sentinel for "this node's subtree is still missing peer `X`". The /// `range` + `optional` payload mirrors upstream's `MissingPeers` map /// value; pacquet records them for upstream parity (and for the @@ -479,13 +519,21 @@ impl<'tree> Walker<'tree> { // `self.tree.direct`. let direct: Vec = self.tree.direct.clone(); for dep in &direct { - let output = - self.resolve_node(dep.node_id.clone(), &importer_parents, &parent_chain_names, 0); - direct_by_alias.insert(dep.alias.clone(), output.dep_path); + self.resolve_node(dep.node_id.clone(), &importer_parents, &parent_chain_names, 0); } self.patch_pending_peer_edges(); + // Recompute depPaths so each resolved peer carries its full + // suffix (the cycle fallback during the walk collapses peers + // that are walk-ancestors), then rebuild the graph from the + // per-node records keyed by the corrected depPaths. + let final_dep_paths = self.build_final_dep_paths(); + for dep in &direct { + direct_by_alias + .insert(dep.alias.clone(), self.final_dep_path_of(&dep.node_id, &final_dep_paths)); + } + let graph = self.build_final_graph(&final_dep_paths); ResolvePeersResult { - graph: self.graph, + graph, direct_dependencies_by_alias: direct_by_alias, peer_dependency_issues: self.issues, } @@ -889,6 +937,32 @@ impl<'tree> Walker<'tree> { let is_pure = all_resolved_peers.is_empty() && all_missing_peers.is_empty(); + // Capture this node's NodeId-level edges + metadata for the + // post-walk [`Walker::build_final_dep_paths`] rebuild. Edges are + // the node's regular children overlaid with its *own* resolved + // peers — mirroring upstream's + // [`childrenNodeIds: { ...children, ...resolvedPeers }`](https://github.com/pnpm/pnpm/blob/894ea6af2c/installing/deps-resolver/src/resolvePeers.ts#L700-L705), + // where `resolvedPeers` is this node's own peer resolution, not + // the descendants' peers bubbled up for the suffix. A peer a + // descendant resolved (e.g. `debug`'s optional `supports-color`) + // is symlinked at the descendant that declares it, so it must + // not appear in this node's dependencies. Carries NodeIds so the + // rebuild can resolve each to its corrected final depPath. + let mut record_edges: BTreeMap = children_map.clone(); + for (peer_alias, peer_node_id) in &own_resolved_peers { + record_edges.insert(peer_alias.clone(), peer_node_id.clone()); + } + self.node_records.insert( + node_id.clone(), + NodeRecord { + edges: record_edges, + transitive_peer_dependencies: transitive_peer_dependencies.clone(), + depth: tree_node.depth, + installable: tree_node.installable, + is_pure, + }, + ); + // Record this walk's outcome in the per-`pkgIdWithPatchHash` // caches. Pure subtrees go in [`Self::pure_pkgs`] for the // fast-path early return at the top of [`resolve_node`]; @@ -1047,6 +1121,261 @@ impl<'tree> Walker<'tree> { PeerId::Pair { name, version } } + /// Resolve `node_id` to the depPath the rebuilt graph should key / + /// reference it by. Prefers the corrected `final_dep_paths` entry, + /// falls back to the provisional `node_dep_paths` value (correct for + /// peerless nodes, whose depPath is just their `pkgIdWithPatchHash`), + /// then to a bare `link:` id, then to the package id. + fn final_dep_path_of( + &self, + node_id: &NodeId, + final_dep_paths: &HashMap, + ) -> DepPath { + if let Some(dep_path) = final_dep_paths.get(node_id) { + return dep_path.clone(); + } + if let Some(dep_path) = self.node_dep_paths.get(node_id) { + return dep_path.clone(); + } + if let Some(dep_path) = link_node_id_as_dep_path(node_id) { + return dep_path; + } + let pkg_id = &self.tree.dependencies_tree[node_id].resolved_package_id; + DepPath::from(self.tree.packages[pkg_id].id.clone()) + } + + /// One resolved-peer slot of `node_id`'s suffix, computed against the + /// already-finalized depPaths. Mirrors [`Self::build_peer_id`] but + /// substitutes the provisional cycle fallback with a strongly- + /// connected-component test: a peer is collapsed to `name@version` + /// only when it shares a peer-graph SCC with `node_id` (a genuine + /// cycle, matching pnpm's `cyclicPeerAliases` branch). Non-cyclic + /// peers carry their full depPath, which is the parity fix. + fn final_peer_id( + &self, + peer_alias: &str, + peer_node_id: &NodeId, + node_scc: usize, + scc_of: &HashMap, + final_dep_paths: &HashMap, + ) -> PeerId { + if let NodeId::Leaf(id) = peer_node_id + && let Some(rel) = id.strip_prefix("link:") + { + return PeerId::Pair { + name: peer_alias.to_string(), + version: link_path_to_peer_version(rel), + }; + } + let pair = || { + let tree_node = &self.tree.dependencies_tree[peer_node_id]; + let pkg = &self.tree.packages[&tree_node.resolved_package_id]; + let (name, version) = pkg_name_version(&pkg.result); + PeerId::Pair { name, version } + }; + if self.opts.dedupe_peers && self.tree.dependencies_tree.contains_key(peer_node_id) { + return pair(); + } + if scc_of.get(peer_node_id) == Some(&node_scc) { + return pair(); + } + PeerId::DepPath(self.final_dep_path_of(peer_node_id, final_dep_paths)) + } + + /// Recompute every node's depPath with its resolved peers' *full* + /// suffixes. Genuine peer cycles (detected as multi-node peer-graph + /// SCCs, or self-loops) keep the `name@version` collapse; every other + /// peer slot carries the peer's own depPath. This is the synchronous + /// equivalent of pnpm's deferred `calculateDepPath` + `analyzeGraph` + /// cycle detection. + fn build_final_dep_paths(&self) -> HashMap { + let (sccs, scc_of) = self.peer_sccs(); + let mut final_dep_paths: HashMap = HashMap::new(); + // SCCs come out of Tarjan in reverse-topological order, so a + // node's cross-SCC peers are already finalized when we reach it. + for (scc_index, scc) in sccs.iter().enumerate() { + for node_id in scc { + let Some(peers) = self.node_external_peers.get(node_id) else { continue }; + if peers.is_empty() { + continue; + } + let mut peer_ids: Vec = peers + .iter() + .map(|(peer_alias, peer_node_id)| { + self.final_peer_id( + peer_alias, + peer_node_id, + scc_index, + &scc_of, + &final_dep_paths, + ) + }) + .collect(); + peer_ids.sort_by_key(PeerId::as_segment); + peer_ids.dedup_by_key(|peer_id| peer_id.as_segment()); + let suffix = + create_peer_dep_graph_hash(&peer_ids, self.opts.peers_suffix_max_length); + let pkg_id = &self.tree.dependencies_tree[node_id].resolved_package_id; + let dep_path = + DepPath::from(format!("{}{}", self.tree.packages[pkg_id].id, suffix)); + final_dep_paths.insert(node_id.clone(), dep_path); + } + } + final_dep_paths + } + + /// Strongly-connected components of the peer graph (node → resolved + /// peers, restricted to peers that themselves carry peers — peerless + /// peers can't close a cycle). Iterative Tarjan, returning the SCCs + /// in reverse-topological order plus a `NodeId → SCC index` map. + fn peer_sccs(&self) -> (Vec>, HashMap) { + let participants: HashSet<&NodeId> = self + .node_external_peers + .iter() + .filter(|(_, peers)| !peers.is_empty()) + .map(|(node_id, _)| node_id) + .collect(); + let neighbors = |node_id: &NodeId| -> Vec { + self.node_external_peers + .get(node_id) + .into_iter() + .flat_map(|peers| peers.values()) + .filter(|peer| participants.contains(*peer)) + .cloned() + .collect() + }; + + let mut index_of: HashMap = HashMap::new(); + let mut low_of: HashMap = HashMap::new(); + let mut on_stack: HashSet = HashSet::new(); + let mut tarjan_stack: Vec = Vec::new(); + let mut sccs: Vec> = Vec::new(); + let mut scc_of: HashMap = HashMap::new(); + let mut next_index: u32 = 0; + + // Explicit DFS stack of (node, neighbors, cursor) so deep peer + // graphs don't overflow the call stack. + for root in &participants { + if index_of.contains_key(*root) { + continue; + } + let mut work: Vec<(NodeId, Vec, usize)> = + vec![((*root).clone(), neighbors(root), 0)]; + while let Some((node_id, succ, cursor)) = work.last_mut() { + if *cursor == 0 { + index_of.insert(node_id.clone(), next_index); + low_of.insert(node_id.clone(), next_index); + next_index += 1; + on_stack.insert(node_id.clone()); + tarjan_stack.push(node_id.clone()); + } + if *cursor < succ.len() { + let child = succ[*cursor].clone(); + *cursor += 1; + if !index_of.contains_key(&child) { + let child_succ = neighbors(&child); + work.push((child, child_succ, 0)); + } else if on_stack.contains(&child) { + let node_low = low_of[node_id]; + let child_index = index_of[&child]; + low_of.insert(node_id.clone(), node_low.min(child_index)); + } + continue; + } + // All successors visited — close this node. + let node_id = node_id.clone(); + if low_of[&node_id] == index_of[&node_id] { + let scc_index = sccs.len(); + let mut component = Vec::new(); + while let Some(member) = tarjan_stack.pop() { + on_stack.remove(&member); + scc_of.insert(member.clone(), scc_index); + let is_root = member == node_id; + component.push(member); + if is_root { + break; + } + } + sccs.push(component); + } + work.pop(); + if let Some((parent, _, _)) = work.last() { + let parent_low = low_of[parent]; + let node_low = low_of[&node_id]; + low_of.insert(parent.clone(), parent_low.min(node_low)); + } + } + } + (sccs, scc_of) + } + + /// Rebuild the depPath-keyed graph from the per-`NodeId` + /// [`NodeRecord`]s using the corrected `final_dep_paths`. Nodes that + /// resolve to the same final depPath merge (taking the smallest + /// `depth`, like the inline build); nodes whose suffix was + /// previously collapsed by the cycle fallback now split into + /// distinct entries. + fn build_final_graph(&self, final_dep_paths: &HashMap) -> DependenciesGraph { + // Minimum tree depth across *every* occurrence that resolves to a + // given final depPath. `pure_pkgs` / `find_hit` revisits + // short-circuit before a [`NodeRecord`] is created, so iterating + // `node_records` alone would restore the first (possibly deeper) + // walk's depth and miss a later shallower revisit. `node_dep_paths` + // carries every walked NodeId, so recompute the `Math.min` depth + // tie-break here — the inline build threaded it through `self.graph`, + // which this rebuild discards. + let mut min_depth: HashMap = HashMap::new(); + for node_id in self.node_dep_paths.keys() { + let Some(tree_node) = self.tree.dependencies_tree.get(node_id) else { continue }; + let dep_path = self.final_dep_path_of(node_id, final_dep_paths); + min_depth + .entry(dep_path) + .and_modify(|depth| *depth = (*depth).min(tree_node.depth)) + .or_insert(tree_node.depth); + } + + let mut graph = DependenciesGraph::new(); + for (node_id, record) in &self.node_records { + let dep_path = self.final_dep_path_of(node_id, final_dep_paths); + let depth = min_depth.get(&dep_path).copied().unwrap_or(record.depth); + let pkg_id = self.tree.dependencies_tree[node_id].resolved_package_id.clone(); + let pkg = &self.tree.packages[&pkg_id]; + let children: BTreeMap = record + .edges + .iter() + .map(|(alias, edge_node_id)| { + (alias.clone(), self.final_dep_path_of(edge_node_id, final_dep_paths)) + }) + .collect(); + let resolved_peer_names: HashSet = self + .node_external_peers + .get(node_id) + .map(|peers| peers.keys().cloned().collect()) + .unwrap_or_default(); + graph + .entry(dep_path.clone()) + .and_modify(|node| { + if node.depth > depth { + node.depth = depth; + } + }) + .or_insert(DependenciesGraphNode { + dep_path, + resolved_package_id: pkg_id.clone(), + resolve_result: Arc::clone(&pkg.result), + children, + peer_dependencies: pkg.peer_dependencies.clone(), + transitive_peer_dependencies: record.transitive_peer_dependencies.clone(), + resolved_peer_names, + depth, + installable: record.installable, + is_pure: record.is_pure, + optional: pkg.optional, + }); + } + graph + } + /// Realize the `(alias → NodeId)` children of `node_id` if it's /// currently a [`TreeChildren::Lazy`] entry; return the realized /// map (cloned for the caller). On a [`TreeChildren::Realized`] diff --git a/pacquet/crates/resolving-deps-resolver/src/tests.rs b/pacquet/crates/resolving-deps-resolver/src/tests.rs index 34bec62c67..1e86ff4895 100644 --- a/pacquet/crates/resolving-deps-resolver/src/tests.rs +++ b/pacquet/crates/resolving-deps-resolver/src/tests.rs @@ -921,6 +921,283 @@ mod peers { ); } + /// A package's graph-node `depth` is the minimum across all + /// occurrences, even when the shallower one short-circuits through the + /// `pure_pkgs` fast path (which has no `NodeRecord`). `p` is reached at + /// depth 2 via `a → b → p` (walked first, so its record carries depth + /// 2) and at depth 1 via `c → p` (a pure-pkgs revisit). The rebuilt + /// graph must record depth 1. Regression guard for the `build_final_graph` + /// depth tie-break. + #[tokio::test] + async fn shallower_pure_pkgs_revisit_lowers_graph_depth() { + let mut table = HashMap::new(); + table.insert( + ("a".to_string(), "1.0.0".to_string()), + fake_result( + "a", + "1.0.0", + serde_json::json!({ + "name": "a", + "version": "1.0.0", + "dependencies": { "b": "1.0.0" } + }), + ), + ); + table.insert( + ("b".to_string(), "1.0.0".to_string()), + fake_result( + "b", + "1.0.0", + serde_json::json!({ + "name": "b", + "version": "1.0.0", + "dependencies": { "p": "1.0.0" } + }), + ), + ); + table.insert( + ("c".to_string(), "1.0.0".to_string()), + fake_result( + "c", + "1.0.0", + serde_json::json!({ + "name": "c", + "version": "1.0.0", + "dependencies": { "p": "1.0.0" } + }), + ), + ); + // `p` has a dep `q` so it gets per-occurrence NodeIds (a shared + // leaf would already carry its minimum depth in the tree). Its + // whole subtree is peer-free, so the second, shallower occurrence + // under `c` takes the `pure_pkgs` fast path — which records no + // `NodeRecord`, the case the rebuild must still account for. + table.insert( + ("p".to_string(), "1.0.0".to_string()), + fake_result( + "p", + "1.0.0", + serde_json::json!({ + "name": "p", + "version": "1.0.0", + "dependencies": { "q": "1.0.0" } + }), + ), + ); + table.insert( + ("q".to_string(), "1.0.0".to_string()), + fake_result("q", "1.0.0", serde_json::json!({ "name": "q", "version": "1.0.0" })), + ); + let resolver = StubResolver { table, calls: Mutex::new(Vec::new()) }; + // `a` precedes `c` so `p` is first walked at depth 2 (`a → b → p`), + // then revisited at depth 1 (`c → p`). + let (_tmp, manifest) = fake_manifest(serde_json::json!({ "a": "1.0.0", "c": "1.0.0" })); + let mut tree = 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, + }, + ) + .await + .unwrap(); + + let result = resolve_peers(&mut tree, ResolvePeersOptions::default()); + let p_node = &result.graph[&DepPath::from("p@1.0.0".to_string())]; + assert_eq!(p_node.depth, 1, "p's graph depth must be the minimum (1), not 2"); + } + + /// Port of upstream's + /// [`"a peer's own peer is shared with a sibling that peer-depends both"`](https://github.com/pnpm/pnpm/blob/894ea6af2c/installing/deps-resolver/test/resolvePeers.ts#L1207). + /// `plugin` peer-depends both `parser` and `typescript`; `parser` + /// peer-depends `typescript`. `plugin` and `parser` live under + /// `umbrella` (under `app`, which also brings `typescript@1.0.0`), while + /// the importer also has a top-level `typescript@2.0.0` + `parser@1.0.0`. + /// `plugin`'s `parser` must resolve `typescript@1.0.0` — the version + /// `plugin` itself uses — not be shadowed by the top-level `parser` that + /// resolved `typescript@2.0.0`. Exercises the depPath suffix machinery. + #[tokio::test] + async fn peers_own_peer_shared_with_sibling_that_peer_depends_both() { + let mut table = HashMap::new(); + for version in ["1.0.0", "2.0.0"] { + table.insert( + ("typescript".to_string(), version.to_string()), + fake_result( + "typescript", + version, + serde_json::json!({ "name": "typescript", "version": version }), + ), + ); + } + table.insert( + ("parser".to_string(), "1.0.0".to_string()), + fake_result( + "parser", + "1.0.0", + serde_json::json!({ + "name": "parser", + "version": "1.0.0", + "peerDependencies": { "typescript": "*" } + }), + ), + ); + table.insert( + ("plugin".to_string(), "1.0.0".to_string()), + fake_result( + "plugin", + "1.0.0", + serde_json::json!({ + "name": "plugin", + "version": "1.0.0", + "peerDependencies": { "parser": "*", "typescript": "*" } + }), + ), + ); + table.insert( + ("umbrella".to_string(), "1.0.0".to_string()), + fake_result( + "umbrella", + "1.0.0", + serde_json::json!({ + "name": "umbrella", + "version": "1.0.0", + "dependencies": { "plugin": "1.0.0", "parser": "1.0.0" }, + "peerDependencies": { "typescript": "*" } + }), + ), + ); + table.insert( + ("app".to_string(), "1.0.0".to_string()), + fake_result( + "app", + "1.0.0", + serde_json::json!({ + "name": "app", + "version": "1.0.0", + "dependencies": { "typescript": "1.0.0", "umbrella": "1.0.0" } + }), + ), + ); + let resolver = StubResolver { table, calls: Mutex::new(Vec::new()) }; + let (_tmp, manifest) = fake_manifest(serde_json::json!({ + "typescript": "2.0.0", + "parser": "1.0.0", + "app": "1.0.0", + })); + let mut tree = 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, + }, + ) + .await + .unwrap(); + + let result = resolve_peers(&mut tree, ResolvePeersOptions::default()); + let keys: Vec = result.graph.keys().map(|dp| dp.as_str().to_string()).collect(); + assert!( + keys.contains( + &"plugin@1.0.0(parser@1.0.0(typescript@1.0.0))(typescript@1.0.0)".to_string() + ), + "plugin's parser must resolve typescript@1.0.0; got: {keys:?}", + ); + assert!( + !keys.contains( + &"plugin@1.0.0(parser@1.0.0(typescript@2.0.0))(typescript@1.0.0)".to_string() + ), + "plugin's parser must not be shadowed by the top-level typescript@2.0.0: {keys:?}", + ); + } + + /// A peer that is a walk-ancestor must still carry its own peer + /// suffix in the dependent's depPath. `a` is a direct dep with peer + /// `c`; its child `b` peer-depends on `a`. While `b`'s suffix is + /// being built, `a` is mid-walk (in-progress) so its depPath isn't + /// finalized yet. The post-walk [`build_final_dep_paths`] pass must + /// resolve `a` to its full `a@1.0.0(c@1.0.0)` — not the collapsed + /// `a@1.0.0` the cycle fallback would emit (pnpm only collapses + /// genuine cycles, and `a→b→a` here resolves because `a` and `b` + /// don't form a peer-graph SCC). Regression test for + /// . + #[tokio::test] + async fn ancestor_peer_carries_its_own_suffix() { + let mut table = HashMap::new(); + table.insert( + ("a".to_string(), "1.0.0".to_string()), + fake_result( + "a", + "1.0.0", + serde_json::json!({ + "name": "a", + "version": "1.0.0", + "dependencies": { "b": "1.0.0" }, + "peerDependencies": { "c": "*" } + }), + ), + ); + table.insert( + ("b".to_string(), "1.0.0".to_string()), + fake_result( + "b", + "1.0.0", + serde_json::json!({ + "name": "b", + "version": "1.0.0", + "peerDependencies": { "a": "*" } + }), + ), + ); + table.insert( + ("c".to_string(), "1.0.0".to_string()), + fake_result("c", "1.0.0", serde_json::json!({ "name": "c", "version": "1.0.0" })), + ); + let resolver = StubResolver { table, calls: Mutex::new(Vec::new()) }; + let (_tmp, manifest) = fake_manifest(serde_json::json!({ "a": "1.0.0", "c": "1.0.0" })); + let mut tree = 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, + }, + ) + .await + .unwrap(); + + let result = resolve_peers(&mut tree, ResolvePeersOptions::default()); + let mut keys: Vec = result.graph.keys().map(|dp| dp.as_str().to_string()).collect(); + keys.sort(); + assert_eq!( + keys, + vec![ + "a@1.0.0(c@1.0.0)".to_string(), + "b@1.0.0(a@1.0.0(c@1.0.0))".to_string(), + "c@1.0.0".to_string(), + ], + ); + + // `c` is `a`'s peer, not `b`'s — it must not leak into `b`'s + // dependencies (only `b`'s own peer `a` is a child of `b`). + let b_node = &result.graph[&DepPath::from("b@1.0.0(a@1.0.0(c@1.0.0))".to_string())]; + let b_children: Vec<&str> = b_node.children.keys().map(String::as_str).collect(); + assert_eq!(b_children, vec!["a"]); + } + /// Shared fixture for the `dedupe_peers_*` pair: react@18 plus /// `@emotion/react@11` (peer: react) plus `@emotion/styled@11` /// (peers: react, @emotion/react). Mirrors upstream's diff --git a/pacquet/plans/TEST_PORTING.md b/pacquet/plans/TEST_PORTING.md index f2b10a7eba..1edc3dbc59 100644 --- a/pacquet/plans/TEST_PORTING.md +++ b/pacquet/plans/TEST_PORTING.md @@ -836,3 +836,32 @@ Tracks pnpm/pnpm#12196. The `catalogMode` mismatch gate landed earlier (pnpm#117 - Settings drift comparison is field-by-field on `WorkspaceStateSettings::PartialEq` rather than the upstream `Object.entries` walk. Equivalent in behavior: any field present in the cached state but `None` in today's `current_settings` (or vice versa) trips the check. - The settings construction is shared between the writer (`build_workspace_state` in `install.rs`) and the reader (`current_settings` in `optimistic_repeat_install.rs`) so adding a tracked field on one side automatically updates the other. + +## Peer Resolution (`installing/deps-resolver/test/resolvePeers.ts`, `hoistPeers.test.ts`) + +Status of the upstream peer-resolution suites, audited while landing the +lockfile-parity peer fixes (pnpm/pnpm#12266, pnpm/pnpm#12267). + +### `hoistPeers.test.ts` — fully ported + +- [x] All 11 cases (`hoistPeers` × 8 + `getHoistableOptionalPeers` × 3) are ported in `hoist_peers/tests.rs`, plus two prerelease siblings pacquet adds. + +### `resolvePeers.ts` — ported / covered + +- [x] `transitive peers use version-only suffixes` — `dedupe_peers_propagates_transitive_peer_to_parent`. +- [x] `uses version-only peer suffixes without nested dep paths` — `dedupe_peers_collapses_nested_peer_suffixes` / `no_dedupe_peers_keeps_nested_peer_suffixes`. +- [x] `resolve peer dependencies of cyclic dependencies` — `cyclic_peer_dependencies_resolve_cleanly`. +- [x] `when a package is referenced twice … still try to resolve it in the other occurrence` — `revisit_resolves_peer_in_one_occurrence_misses_in_other`. +- [x] `should return from where the bad peer dependency is resolved` — `bad_peer_inside_subtree_records_resolved_from_parent`. +- [x] `should find peer dependency conflicts` — covered by `bad_peer_version_is_reported`. +- [x] `a peer's own peer is shared with a sibling that peer-depends both` — ported as `peers_own_peer_shared_with_sibling_that_peer_depends_both`. +- [x] Walk-ancestor suffix propagation (no direct upstream case — pacquet-specific manifestation of the deferred `calculateDepPath`) — `ancestor_peer_carries_its_own_suffix`. +- [x] Optional peer not hoisted from the run-resolved tree (resolveRootDependencies behavior behind `getHoistableOptionalPeers`) — `optional_peer_only_in_resolved_tree_is_not_hoisted`. +- [x] `build_final_graph` min-depth tie-break across `pure_pkgs`/`find_hit` revisits (pacquet-specific) — `shallower_pure_pkgs_revisit_lowers_graph_depth`. + +### `resolvePeers.ts` — not yet ported + +- [ ] `multi-project: different peer versions produce different instances` — needs the multi-importer `resolve_peers_workspace` harness; general workspace peer-separation, partially covered by `dedupes_when_the_same_package_appears_in_two_subtrees`. +- [ ] `resolve peer dependencies with npm aliases` — npm-alias peer suffixes. +- [ ] `should find peer dependency conflicts when the peer is an optional peer of one of the dependencies`, `should ignore conflicts between missing optional peer dependencies`, `should pick the single wanted peer dependency range`, `should return the intersection of two compatible ranges`, the two prerelease-warning cases — peer-issue reporting edge cases. +- [ ] The `lockedPeerContext` / `resolvedPeerProviderPaths` series (`prefers a compatible locked provider …`, the six `does not replace …` cases, `does not reuse a locked provider outside the current peer range`) — pacquet hasn't ported `lockedPeerContext`/`resolvedPeerProviderPaths`, so these gate on that feature, not on the lockfile-parity peer fixes.