From 6745a8b00aef50e29beccfc7ea576291c43aa776 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 24 Dec 2024 18:34:58 -0500 Subject: [PATCH] Show non-project dependencies in `uv tree` (#10149) ## Summary Closes #10147. --- .../uv-resolver/src/lock/requirements_txt.rs | 28 ++- crates/uv-resolver/src/lock/target.rs | 40 ++++ crates/uv-resolver/src/lock/tree.rs | 221 +++++++++++++----- crates/uv/tests/it/tree.rs | 110 +++++++++ 4 files changed, 337 insertions(+), 62 deletions(-) diff --git a/crates/uv-resolver/src/lock/requirements_txt.rs b/crates/uv-resolver/src/lock/requirements_txt.rs index 6d80ae0ddcd4..e6942bdf0026 100644 --- a/crates/uv-resolver/src/lock/requirements_txt.rs +++ b/crates/uv-resolver/src/lock/requirements_txt.rs @@ -139,16 +139,22 @@ impl<'lock> RequirementsTxtExport<'lock> { // (legacy) non-project workspace roots). let root_requirements = target .lock() - .dependency_groups() + .requirements() .iter() - .filter_map(|(group, deps)| { - if dev.contains(group) { - Some(deps) - } else { - None - } - }) - .flatten() + .chain( + target + .lock() + .dependency_groups() + .iter() + .filter_map(|(group, deps)| { + if dev.contains(group) { + Some(deps) + } else { + None + } + }) + .flatten(), + ) .filter(|dep| !prune.contains(&dep.name)) .collect::>(); @@ -185,6 +191,10 @@ impl<'lock> RequirementsTxtExport<'lock> { combined }; + if marker.is_false() { + continue; + } + // Simplify the marker. let marker = target.lock().simplify_environment(marker); diff --git a/crates/uv-resolver/src/lock/target.rs b/crates/uv-resolver/src/lock/target.rs index b399a4550eba..7a21e0ba2ff2 100644 --- a/crates/uv-resolver/src/lock/target.rs +++ b/crates/uv-resolver/src/lock/target.rs @@ -212,6 +212,46 @@ impl<'env> InstallTarget<'env> { } } + // Add any requirements that are exclusive to the workspace root (e.g., dependencies in + // PEP 723 scripts). + for dependency in self.lock().requirements() { + if !dependency.marker.evaluate(marker_env, &[]) { + continue; + } + + let root_name = &dependency.name; + let dist = self + .lock() + .find_by_markers(root_name, marker_env) + .map_err(|_| LockErrorKind::MultipleRootPackages { + name: root_name.clone(), + })? + .ok_or_else(|| LockErrorKind::MissingRootPackage { + name: root_name.clone(), + })?; + + // Add the package to the graph. + let index = petgraph.add_node(if dev.prod() { + self.package_to_node(dist, tags, build_options, install_options)? + } else { + self.non_installable_node(dist, tags)? + }); + inverse.insert(&dist.id, index); + + // Add the edge. + petgraph.add_edge(root, index, Edge::Prod(dependency.marker)); + + // Push its dependencies on the queue. + if seen.insert((&dist.id, None)) { + queue.push_back((dist, None)); + } + for extra in &dependency.extras { + if seen.insert((&dist.id, Some(extra))) { + queue.push_back((dist, Some(extra))); + } + } + } + // Add any dependency groups that are exclusive to the workspace root (e.g., dev // dependencies in (legacy) non-project workspace roots). for (group, dependency) in self diff --git a/crates/uv-resolver/src/lock/tree.rs b/crates/uv-resolver/src/lock/tree.rs index fc913844ced1..02708e6b302c 100644 --- a/crates/uv-resolver/src/lock/tree.rs +++ b/crates/uv-resolver/src/lock/tree.rs @@ -1,25 +1,25 @@ -use std::borrow::Cow; -use std::collections::VecDeque; +use std::collections::{BTreeSet, VecDeque}; use itertools::Itertools; use owo_colors::OwoColorize; use petgraph::graph::{EdgeIndex, NodeIndex}; use petgraph::prelude::EdgeRef; use petgraph::Direction; -use rustc_hash::{FxHashMap, FxHashSet}; +use rustc_hash::{FxBuildHasher, FxHashMap, FxHashSet}; use uv_configuration::DevGroupsManifest; use uv_normalize::{ExtraName, GroupName, PackageName}; use uv_pep440::Version; +use uv_pep508::MarkerTree; use uv_pypi_types::ResolverMarkerEnvironment; -use crate::lock::{Dependency, PackageId}; +use crate::lock::PackageId; use crate::{Lock, PackageMap}; #[derive(Debug)] pub struct TreeDisplay<'env> { /// The constructed dependency graph. - graph: petgraph::graph::Graph<&'env PackageId, Edge<'env>, petgraph::Directed>, + graph: petgraph::graph::Graph, Edge<'env>, petgraph::Directed>, /// The packages considered as roots of the dependency tree. roots: Vec, /// The latest known version of each package. @@ -43,24 +43,8 @@ impl<'env> TreeDisplay<'env> { no_dedupe: bool, invert: bool, ) -> Self { - // Identify the workspace members. - let members: FxHashSet<&PackageId> = if lock.members().is_empty() { - lock.root().into_iter().map(|package| &package.id).collect() - } else { - lock.packages - .iter() - .filter_map(|package| { - if lock.members().contains(&package.id.name) { - Some(&package.id) - } else { - None - } - }) - .collect() - }; - // Create a graph. - let mut graph = petgraph::graph::Graph::<&PackageId, Edge, petgraph::Directed>::new(); + let mut graph = petgraph::graph::Graph::::new(); // Create the complete graph. let mut inverse = FxHashMap::default(); @@ -73,7 +57,7 @@ impl<'env> TreeDisplay<'env> { let package_node = if let Some(index) = inverse.get(&package.id) { *index } else { - let index = graph.add_node(&package.id); + let index = graph.add_node(Node::Package(&package.id)); inverse.insert(&package.id, index); index }; @@ -90,7 +74,7 @@ impl<'env> TreeDisplay<'env> { let dependency_node = if let Some(index) = inverse.get(&dependency.package_id) { *index } else { - let index = graph.add_node(&dependency.package_id); + let index = graph.add_node(Node::Package(&dependency.package_id)); inverse.insert(&dependency.package_id, index); index }; @@ -99,7 +83,7 @@ impl<'env> TreeDisplay<'env> { graph.add_edge( package_node, dependency_node, - Edge::Prod(Cow::Borrowed(dependency)), + Edge::Prod(Some(&dependency.extra)), ); } } @@ -118,7 +102,7 @@ impl<'env> TreeDisplay<'env> { if let Some(index) = inverse.get(&dependency.package_id) { *index } else { - let index = graph.add_node(&dependency.package_id); + let index = graph.add_node(Node::Package(&dependency.package_id)); inverse.insert(&dependency.package_id, index); index }; @@ -127,7 +111,7 @@ impl<'env> TreeDisplay<'env> { graph.add_edge( package_node, dependency_node, - Edge::Optional(extra, Cow::Borrowed(dependency)), + Edge::Optional(extra, Some(&dependency.extra)), ); } } @@ -147,7 +131,7 @@ impl<'env> TreeDisplay<'env> { if let Some(index) = inverse.get(&dependency.package_id) { *index } else { - let index = graph.add_node(&dependency.package_id); + let index = graph.add_node(Node::Package(&dependency.package_id)); inverse.insert(&dependency.package_id, index); index }; @@ -156,18 +140,117 @@ impl<'env> TreeDisplay<'env> { graph.add_edge( package_node, dependency_node, - Edge::Dev(group, Cow::Borrowed(dependency)), + Edge::Dev(group, Some(&dependency.extra)), ); } } } } + // Identify any workspace members. + // + // These include: + // - The members listed in the lockfile. + // - The root package, if it's not in the list of members. (The root package is omitted from + // the list of workspace members for single-member workspaces with a `[project]` section, + // to avoid cluttering the lockfile. + let members: FxHashSet<&PackageId> = if lock.members().is_empty() { + lock.root().into_iter().map(|package| &package.id).collect() + } else { + lock.packages + .iter() + .filter_map(|package| { + if lock.members().contains(&package.id.name) { + Some(&package.id) + } else { + None + } + }) + .collect() + }; + + // Identify any packages that are connected directly to the synthetic root node, i.e., + // requirements that are attached to the workspace itself. + // + // These include + // - `[dependency-groups]` dependencies for workspaces whose roots do not include a + // `[project]` table, since those roots are not workspace members, but they _can_ define + // dependencies. + // - `dependencies` in PEP 723 scripts. + let root = graph.add_node(Node::Root); + { + // Index the lockfile by name. + let by_name: FxHashMap<_, Vec<_>> = { + lock.packages().iter().fold( + FxHashMap::with_capacity_and_hasher(lock.len(), FxBuildHasher), + |mut map, package| { + map.entry(&package.id.name).or_default().push(package); + map + }, + ) + }; + + // Identify any requirements attached to the workspace itself. + for requirement in lock.requirements() { + for package in by_name.get(&requirement.name).into_iter().flatten() { + // Determine whether this entry is "relevant" for the requirement, by intersecting + // the markers. + let marker = if package.fork_markers.is_empty() { + requirement.marker + } else { + let mut combined = MarkerTree::FALSE; + for fork_marker in &package.fork_markers { + combined.or(fork_marker.pep508()); + } + combined.and(requirement.marker); + combined + }; + if marker.is_false() { + continue; + } + if markers.is_some_and(|markers| !marker.evaluate(markers, &[])) { + continue; + } + graph.add_edge(root, inverse[&package.id], Edge::Prod(None)); + } + } + + // Identify any dependency groups attached to the workspace itself. + for (group, requirements) in lock.dependency_groups() { + for requirement in requirements { + for package in by_name.get(&requirement.name).into_iter().flatten() { + // Determine whether this entry is "relevant" for the requirement, by intersecting + // the markers. + let marker = if package.fork_markers.is_empty() { + requirement.marker + } else { + let mut combined = MarkerTree::FALSE; + for fork_marker in &package.fork_markers { + combined.or(fork_marker.pep508()); + } + combined.and(requirement.marker); + combined + }; + if marker.is_false() { + continue; + } + if markers.is_some_and(|markers| !marker.evaluate(markers, &[])) { + continue; + } + graph.add_edge(root, inverse[&package.id], Edge::Dev(group, None)); + } + } + } + } + // Filter the graph to remove any unreachable nodes. { let mut reachable = graph .node_indices() - .filter(|index| members.contains(graph[*index])) + .filter(|index| match graph[*index] { + Node::Package(package_id) => members.contains(package_id), + Node::Root => true, + }) .collect::>(); let mut stack = reachable.iter().copied().collect::>(); while let Some(node) = stack.pop_front() { @@ -191,7 +274,12 @@ impl<'env> TreeDisplay<'env> { if !packages.is_empty() { let mut reachable = graph .node_indices() - .filter(|index| packages.contains(&graph[*index].name)) + .filter(|index| { + let Node::Package(package_id) = graph[*index] else { + return false; + }; + packages.contains(&package_id.name) + }) .collect::>(); let mut stack = reachable.iter().copied().collect::>(); while let Some(node) = stack.pop_front() { @@ -222,7 +310,7 @@ impl<'env> TreeDisplay<'env> { } } - // Find the root nodes. + // Find the root nodes: nodes with no incoming edges, or only an edge from the proxy. let mut roots = graph .node_indices() .filter(|index| { @@ -265,14 +353,15 @@ impl<'env> TreeDisplay<'env> { return Vec::new(); } - let package_id = self.graph[cursor.node()]; + let Node::Package(package_id) = self.graph[cursor.node()] else { + return Vec::new(); + }; let edge = cursor.edge().map(|edge_id| &self.graph[edge_id]); let line = { let mut line = format!("{}", package_id.name); - if let Some(edge) = edge { - let extras = &edge.dependency().extra; + if let Some(extras) = edge.and_then(Edge::extras) { if !extras.is_empty() { line.push('['); line.push_str(extras.iter().join(", ").as_str()); @@ -322,18 +411,18 @@ impl<'env> TreeDisplay<'env> { let mut dependencies = self .graph .edges_directed(cursor.node(), Direction::Outgoing) - .map(|edge| { - let node = edge.target(); - Cursor::new(node, edge.id()) + .filter_map(|edge| match self.graph[edge.target()] { + Node::Root => None, + Node::Package(_) => Some(Cursor::new(edge.target(), edge.id())), }) .collect::>(); - dependencies.sort_by_key(|node| { - let package_id = self.graph[node.node()]; - let edge = node + dependencies.sort_by_key(|cursor| { + let node = &self.graph[cursor.node()]; + let edge = cursor .edge() .map(|edge_id| &self.graph[edge_id]) .map(Edge::kind); - (edge, package_id) + (edge, node) }); let mut lines = vec![line]; @@ -343,7 +432,10 @@ impl<'env> TreeDisplay<'env> { package_id, dependencies .iter() - .map(|node| self.graph[node.node()]) + .filter_map(|node| match self.graph[node.node()] { + Node::Package(package_id) => Some(package_id), + Node::Root => None, + }) .collect(), ); path.push(package_id); @@ -394,30 +486,53 @@ impl<'env> TreeDisplay<'env> { let mut path = Vec::new(); let mut lines = Vec::with_capacity(self.graph.node_count()); let mut visited = - FxHashMap::with_capacity_and_hasher(self.graph.node_count(), rustc_hash::FxBuildHasher); + FxHashMap::with_capacity_and_hasher(self.graph.node_count(), FxBuildHasher); for node in &self.roots { - path.clear(); - lines.extend(self.visit(Cursor::root(*node), &mut visited, &mut path)); + match self.graph[*node] { + Node::Root => { + for edge in self.graph.edges_directed(*node, Direction::Outgoing) { + let node = edge.target(); + path.clear(); + lines.extend(self.visit( + Cursor::new(node, edge.id()), + &mut visited, + &mut path, + )); + } + } + Node::Package(_) => { + path.clear(); + lines.extend(self.visit(Cursor::root(*node), &mut visited, &mut path)); + } + } } lines } } +#[derive(Debug, Clone, PartialEq, Eq, Ord, PartialOrd)] +enum Node<'env> { + /// The synthetic root node. + Root, + /// A package in the dependency graph. + Package(&'env PackageId), +} + #[derive(Debug, Clone, PartialEq, Eq, Ord, PartialOrd)] enum Edge<'env> { - Prod(Cow<'env, Dependency>), - Optional(&'env ExtraName, Cow<'env, Dependency>), - Dev(&'env GroupName, Cow<'env, Dependency>), + Prod(Option<&'env BTreeSet>), + Optional(&'env ExtraName, Option<&'env BTreeSet>), + Dev(&'env GroupName, Option<&'env BTreeSet>), } impl<'env> Edge<'env> { - fn dependency(&self) -> &Dependency { + fn extras(&self) -> Option<&'env BTreeSet> { match self { - Self::Prod(dependency) => dependency, - Self::Optional(_, dependency) => dependency, - Self::Dev(_, dependency) => dependency, + Self::Prod(extras) => *extras, + Self::Optional(_, extras) => *extras, + Self::Dev(_, extras) => *extras, } } diff --git a/crates/uv/tests/it/tree.rs b/crates/uv/tests/it/tree.rs index 311fccdc041c..879fe3315569 100644 --- a/crates/uv/tests/it/tree.rs +++ b/crates/uv/tests/it/tree.rs @@ -1062,3 +1062,113 @@ fn workspace_dev() -> Result<()> { Ok(()) } + +#[test] +fn non_project() -> Result<()> { + let context = TestContext::new("3.12"); + + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str( + r#" + [tool.uv.workspace] + members = [] + + [dependency-groups] + async = ["anyio"] + "#, + )?; + + uv_snapshot!(context.filters(), context.tree().arg("--universal"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + anyio v4.3.0 (group: async) + ├── idna v3.6 + └── sniffio v1.3.1 + + ----- stderr ----- + warning: No `requires-python` value found in the workspace. Defaulting to `>=3.12`. + Resolved 3 packages in [TIME] + "### + ); + + // `uv tree` should update the lockfile + let lock = context.read("uv.lock"); + assert!(!lock.is_empty()); + + Ok(()) +} + +#[test] +fn non_project_member() -> Result<()> { + let context = TestContext::new("3.12"); + + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str( + r#" + [tool.uv.workspace] + members = ["child"] + + [dependency-groups] + async = ["anyio"] + "#, + )?; + + let child = context.temp_dir.child("child"); + child.child("pyproject.toml").write_str( + r#" + [project] + name = "child" + version = "0.1.0" + requires-python = ">=3.12" + dependencies = ["iniconfig", "sniffio", "anyio"] + + [build-system] + requires = ["setuptools>=42"] + build-backend = "setuptools.build_meta" + "#, + )?; + + uv_snapshot!(context.filters(), context.tree().arg("--universal"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + anyio v4.3.0 (group: async) + ├── idna v3.6 + └── sniffio v1.3.1 + child v0.1.0 + ├── anyio v4.3.0 (*) + ├── iniconfig v2.0.0 + └── sniffio v1.3.1 + (*) Package tree already displayed + + ----- stderr ----- + Resolved 5 packages in [TIME] + "### + ); + + uv_snapshot!(context.filters(), context.tree().arg("--universal").arg("--invert"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + idna v3.6 + └── anyio v4.3.0 + └── child v0.1.0 + iniconfig v2.0.0 + └── child v0.1.0 + sniffio v1.3.1 + ├── anyio v4.3.0 (*) + └── child v0.1.0 + (*) Package tree already displayed + + ----- stderr ----- + Resolved 5 packages in [TIME] + "### + ); + + // `uv tree` should update the lockfile + let lock = context.read("uv.lock"); + assert!(!lock.is_empty()); + + Ok(()) +}