diff --git a/crates/uv/src/commands/pip/tree.rs b/crates/uv/src/commands/pip/tree.rs index 622d308cd..2adcb4381 100644 --- a/crates/uv/src/commands/pip/tree.rs +++ b/crates/uv/src/commands/pip/tree.rs @@ -1,3 +1,4 @@ +use std::cmp::Ordering; use std::collections::VecDeque; use std::fmt::Write; @@ -17,7 +18,7 @@ use uv_configuration::{Concurrency, IndexStrategy, KeyringProviderType}; use uv_distribution_types::{Diagnostic, IndexCapabilities, IndexLocations, Name, RequiresPython}; use uv_installer::SitePackages; use uv_normalize::PackageName; -use uv_pep440::Version; +use uv_pep440::{Operator, Version, VersionSpecifier, VersionSpecifiers}; use uv_pep508::{Requirement, VersionOrUrl}; use uv_preview::Preview; use uv_pypi_types::{ResolutionMetadata, ResolverMarkerEnvironment, VerbatimParsedUrl}; @@ -359,7 +360,7 @@ impl<'env> DisplayDependencyGraph<'env> { /// Perform a depth-first traversal of the given distribution and its dependencies. fn visit( &self, - cursor: Cursor, + cursor: &Cursor, visited: &mut FxHashMap<&'env PackageName, Vec>, path: &mut Vec<&'env PackageName>, ) -> Vec { @@ -373,30 +374,31 @@ impl<'env> DisplayDependencyGraph<'env> { let mut line = format!("{} v{}", package_name, metadata.version); // If the current package is not top-level (i.e., it has a parent), include the specifiers. - if self.show_version_specifiers { - if let Some(edge) = cursor.edge() { - line.push(' '); + if self.show_version_specifiers && !cursor.is_root() { + line.push(' '); - let source = &self.graph[edge]; - if self.invert { - let parent = self.graph.edge_endpoints(edge).unwrap().0; - let parent = &self.graph[parent].name; - match source.version_or_url.as_ref() { - None => { - let _ = write!(line, "[requires: {parent} *]"); - } - Some(version) => { - let _ = write!(line, "[requires: {parent} {version}]"); - } + let requirement = self.aggregate_requirement(cursor); + + if self.invert { + let parent = self.graph.edge_endpoints(cursor.edge().unwrap()).unwrap().0; + + let parent = &self.graph[parent].name; + + match requirement { + None => { + let _ = write!(line, "[requires: {parent} *]"); } - } else { - match source.version_or_url.as_ref() { - None => { - let _ = write!(line, "[required: *]"); - } - Some(version) => { - let _ = write!(line, "[required: {version}]"); - } + Some(value) => { + let _ = write!(line, "[requires: {parent} {value}]"); + } + } + } else { + match requirement { + None => { + let _ = write!(line, "[required: *]"); + } + Some(value) => { + let _ = write!(line, "[required: {value}]"); } } } @@ -429,10 +431,14 @@ impl<'env> DisplayDependencyGraph<'env> { let mut dependencies = self .graph .edges_directed(cursor.node(), Direction::Outgoing) - .map(|edge| { - let node = edge.target(); - Cursor::new(node, edge.id()) + .fold(FxHashMap::default(), |mut acc, edge| { + acc.entry(edge.target()) + .or_insert_with(Vec::new) + .push(edge.id()); + acc }) + .into_iter() + .map(|(node, edges)| Cursor::with_edges(node, edges)) .collect::>(); dependencies.sort_by_key(|node| { let metadata = &self.graph[node.node()]; @@ -480,8 +486,7 @@ impl<'env> DisplayDependencyGraph<'env> { ("├── ", "│ ") }; - for (visited_index, visited_line) in self.visit(*dep, visited, path).iter().enumerate() - { + for (visited_index, visited_line) in self.visit(dep, visited, path).iter().enumerate() { let prefix = if visited_index == 0 { prefix_top } else { @@ -496,6 +501,120 @@ impl<'env> DisplayDependencyGraph<'env> { lines } + /// Aggregate the requirements associated with the incoming edges for a node. + fn aggregate_requirement(&self, cursor: &Cursor) -> Option { + let mut specifiers = Vec::new(); + + for edge_id in cursor.edge_ids() { + let requirement = &self.graph[*edge_id]; + + let Some(version_or_url) = requirement.version_or_url.as_ref() else { + continue; + }; + + match version_or_url { + VersionOrUrl::VersionSpecifier(values) => { + specifiers.extend(values.iter().cloned()); + } + VersionOrUrl::Url(value) => return Some(value.to_string()), + } + } + + if specifiers.is_empty() { + return None; + } + + let display = Self::simplify_specifiers(specifiers).to_string(); + + if display.is_empty() { + None + } else { + Some(display) + } + } + + /// Simplify a collection of specifiers into a canonical representation for display. + fn simplify_specifiers(specifiers: Vec) -> VersionSpecifiers { + let (lower, upper, others) = specifiers.into_iter().fold( + (None, None, Vec::new()), + |(lower, upper, mut rest), spec| match *spec.operator() { + Operator::GreaterThan | Operator::GreaterThanEqual => { + (Some(Self::prefer_lower(lower, spec)), upper, rest) + } + Operator::LessThan | Operator::LessThanEqual => { + (lower, Some(Self::prefer_upper(upper, spec)), rest) + } + _ => { + rest.push(spec); + (lower, upper, rest) + } + }, + ); + + let mut merged = lower + .into_iter() + .chain(upper) + .chain(others) + .collect::>(); + + let mut seen = FxHashSet::default(); + + merged.retain(|spec| seen.insert(spec.to_string())); + + VersionSpecifiers::from_iter(merged) + } + + fn prefer_lower( + current: Option, + candidate: VersionSpecifier, + ) -> VersionSpecifier { + match current { + None => candidate, + Some(existing) => match candidate.version().cmp(existing.version()) { + Ordering::Greater => candidate, + Ordering::Less => existing, + Ordering::Equal => { + let candidate_inclusive = + matches!(candidate.operator(), Operator::GreaterThanEqual); + + let existing_inclusive = + matches!(existing.operator(), Operator::GreaterThanEqual); + + if !candidate_inclusive && existing_inclusive { + candidate + } else { + existing + } + } + }, + } + } + + fn prefer_upper( + current: Option, + candidate: VersionSpecifier, + ) -> VersionSpecifier { + match current { + None => candidate, + Some(existing) => match candidate.version().cmp(existing.version()) { + Ordering::Less => candidate, + Ordering::Greater => existing, + Ordering::Equal => { + let candidate_inclusive = + matches!(candidate.operator(), Operator::LessThanEqual); + + let existing_inclusive = matches!(existing.operator(), Operator::LessThanEqual); + + if !candidate_inclusive && existing_inclusive { + candidate + } else { + existing + } + } + }, + } + } + /// Depth-first traverse the nodes to render the tree. pub(crate) fn render(&self) -> Vec { let mut path = Vec::new(); @@ -505,7 +624,8 @@ impl<'env> DisplayDependencyGraph<'env> { for node in &self.roots { path.clear(); - lines.extend(self.visit(Cursor::root(*node), &mut visited, &mut path)); + let cursor = Cursor::root(*node); + lines.extend(self.visit(&cursor, &mut visited, &mut path)); } lines @@ -513,27 +633,93 @@ impl<'env> DisplayDependencyGraph<'env> { } /// A node in the dependency graph along with the edge that led to it, or `None` for root nodes. -#[derive(Debug, Copy, Clone, PartialEq, Eq, Ord, PartialOrd)] -struct Cursor(NodeIndex, Option); +#[derive(Debug, Clone)] +struct Cursor { + node: NodeIndex, + edges: Vec, +} impl Cursor { - /// Create a [`Cursor`] representing a node in the dependency tree. - fn new(node: NodeIndex, edge: EdgeIndex) -> Self { - Self(node, Some(edge)) - } - /// Create a [`Cursor`] representing a root node in the dependency tree. fn root(node: NodeIndex) -> Self { - Self(node, None) + Self { + node, + edges: Vec::new(), + } + } + + /// Create a [`Cursor`] with the provided set of edges. + fn with_edges(node: NodeIndex, edges: Vec) -> Self { + Self { node, edges } } /// Return the [`NodeIndex`] of the node. fn node(&self) -> NodeIndex { - self.0 + self.node } - /// Return the [`EdgeIndex`] of the edge that led to the node, if any. + /// Return the [`EdgeIndex`] values that led to the node. + fn edge_ids(&self) -> &[EdgeIndex] { + &self.edges + } + + /// Return the first [`EdgeIndex`] if the node is not a root. fn edge(&self) -> Option { - self.1 + self.edges.first().copied() + } + + /// Whether this cursor represents a root node. + fn is_root(&self) -> bool { + self.edges.is_empty() + } +} + +#[cfg(test)] +mod tests { + use super::DisplayDependencyGraph; + use std::str::FromStr; + use uv_pep440::{VersionSpecifier, VersionSpecifiers}; + + fn simplify_specs(specs: &[&str]) -> VersionSpecifiers { + DisplayDependencyGraph::simplify_specifiers( + specs + .iter() + .map(|s| VersionSpecifier::from_str(s).expect("valid specifier")) + .collect(), + ) + } + + #[test] + fn prefers_highest_lower_bound() { + assert_eq!( + simplify_specs(&[">=0.3.6", ">=0.3.7"]).to_string(), + ">=0.3.7" + ); + } + + #[test] + fn prefers_strict_bound_on_tie() { + assert_eq!(simplify_specs(&[">=1", ">1"]).to_string(), ">1"); + } + + #[test] + fn retains_other_specifiers_and_dedupes() { + assert_eq!( + simplify_specs(&[">=0.3.7", "<0.4", "!=0.3.9", ">=0.3.7"]).to_string(), + ">=0.3.7, !=0.3.9, <0.4" + ); + } + + #[test] + fn prefers_lowest_upper_bound() { + assert_eq!(simplify_specs(&["<=1", "<1"]).to_string(), "<1"); + } + + #[test] + fn keeps_both_bounds_when_present() { + assert_eq!( + simplify_specs(&[">=0.3.7", "<0.4", ">=0.3.6"]).to_string(), + ">=0.3.7, <0.4" + ); } } diff --git a/crates/uv/tests/it/pip_tree.rs b/crates/uv/tests/it/pip_tree.rs index 5393c9823..83f0de711 100644 --- a/crates/uv/tests/it/pip_tree.rs +++ b/crates/uv/tests/it/pip_tree.rs @@ -1,9 +1,13 @@ #![cfg(not(windows))] +use assert_cmd::assert::OutputAssertExt; use std::process::Command; +use assert_fs::fixture::FileTouch; use assert_fs::fixture::FileWriteStr; use assert_fs::fixture::PathChild; +use assert_fs::fixture::PathCreateDir; +use indoc::indoc; use uv_static::EnvVars; @@ -1159,3 +1163,125 @@ fn outdated() { "### ); } + +/// Test that dependencies with multiple marker-specific requirements +/// are only displayed once in the tree. +#[test] +#[cfg(feature = "pypi")] +fn no_duplicate_dependencies_with_markers() { + const PY_PROJECT: &str = indoc! {r#" + [project] + name = "debug" + version = "0.1.0" + requires-python = ">=3.12.0" + dependencies = [ + "sniffio>=1.0.0; python_version >= '3.11'", + "sniffio>=1.0.1; python_version >= '3.12'", + "sniffio>=1.0.2; python_version >= '3.13'", + ] + + [build-system] + requires = ["uv_build>=0.8.22,<10000"] + build-backend = "uv_build" + "#}; + + let context = TestContext::new_with_versions(&["3.12", "3.13"]).with_filtered_counts(); + + let project = context.temp_dir.child("debug"); + + project.create_dir_all().unwrap(); + + project.child("src/debug").create_dir_all().unwrap(); + + project.child("src/debug/__init__.py").touch().unwrap(); + + project + .child("pyproject.toml") + .write_str(PY_PROJECT) + .unwrap(); + + context.reset_venv(); + + uv_snapshot!(context.filters(), context + .pip_install() + .arg(project.path()) + .arg("--strict"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved [N] packages in [TIME] + Prepared [N] packages in [TIME] + Installed [N] packages in [TIME] + + debug==0.1.0 (from file://[TEMP_DIR]/debug) + + sniffio==1.3.1 + "### + ); + + // Ensure that the dependency is only listed once, even though `debug` declares multiple + // marker-specific requirements for the same dependency. + uv_snapshot!(context.filters(), context.pip_tree(), @r###" + success: true + exit_code: 0 + ----- stdout ----- + debug v0.1.0 + └── sniffio v1.3.1 + + ----- stderr ----- + "### + ); + + uv_snapshot!( + context.filters(), + context.pip_tree().arg("--show-version-specifiers"), + @r###" + success: true + exit_code: 0 + ----- stdout ----- + debug v0.1.0 + └── sniffio v1.3.1 [required: >=1.0.1] + + ----- stderr ----- + "### + ); + + context + .venv() + .arg("--clear") + .arg("--python") + .arg("3.13") + .assert() + .success(); + + uv_snapshot!(context.filters(), context + .pip_install() + .arg(project.path()) + .arg("--strict"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved [N] packages in [TIME] + Prepared [N] packages in [TIME] + Installed [N] packages in [TIME] + + debug==0.1.0 (from file://[TEMP_DIR]/debug) + + sniffio==1.3.1 + "### + ); + + uv_snapshot!( + context.filters(), + context.pip_tree().arg("--show-version-specifiers"), + @r###" + success: true + exit_code: 0 + ----- stdout ----- + debug v0.1.0 + └── sniffio v1.3.1 [required: >=1.0.2] + + ----- stderr ----- + "### + ); +}