mirror of
https://github.com/astral-sh/uv.git
synced 2025-10-29 03:02:55 +00:00
Deduplicate marker-specific dependencies in uv pip tree output (#16078)
Resolves https://github.com/astral-sh/uv/issues/16067 When we build the dependency graph we add an edge per `Requires-Dist`. If a package publishes multiple marker-guarded requirements (like pylint’s `dill>=…` for different Python versions), more than one marker can evaluate to true at runtime, which gives us several edges pointing to the same installed package node. To avoid printing the package multiple times, we gather all edges targeting that node, pass them through a `Cursor`, and aggregate their requirement details before we print the dependency line. That aggregation does two things: 1. If any edge carries a URL, we return that URL immediately. 2. Otherwise we merge all version specifiers into one canonical PEP 440 string. I've added an integration test, namely `cargo test -p uv --test it --features pypi -- no_duplicate_dependencies_with_markers` exercises the new snapshot that installs pylint (with the real dill duplication scenario present in the original issue) and asserts the tree output, both with and without `--show-version-specifiers`, now shows a single dill entry with the merged constraint.
This commit is contained in:
liam
GitHub
parent
8b86bd530e
commit
d51a1e7456
2 changed files with 353 additions and 41 deletions
|
|
@ -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<PackageName>>,
|
||||
path: &mut Vec<&'env PackageName>,
|
||||
) -> Vec<String> {
|
||||
|
|
@ -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::<Vec<_>>();
|
||||
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<String> {
|
||||
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<VersionSpecifier>) -> 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::<Vec<_>>();
|
||||
|
||||
let mut seen = FxHashSet::default();
|
||||
|
||||
merged.retain(|spec| seen.insert(spec.to_string()));
|
||||
|
||||
VersionSpecifiers::from_iter(merged)
|
||||
}
|
||||
|
||||
fn prefer_lower(
|
||||
current: Option<VersionSpecifier>,
|
||||
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<VersionSpecifier>,
|
||||
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<String> {
|
||||
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<EdgeIndex>);
|
||||
#[derive(Debug, Clone)]
|
||||
struct Cursor {
|
||||
node: NodeIndex,
|
||||
edges: Vec<EdgeIndex>,
|
||||
}
|
||||
|
||||
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<EdgeIndex>) -> 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<EdgeIndex> {
|
||||
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"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 -----
|
||||
"###
|
||||
);
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue