pioduino/uv
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fix: uv tree orphaned roots and premature deduplication (#17212)

Browse Source 
<!--
Thank you for contributing to uv! To help us out with reviewing, please
consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->

## Summary

will close https://github.com/astral-sh/uv/issues/17160

Basically, old code using nodes with no incoming edges included
transitive deps which resulted in orphaned roots. We didnt actually need
that code as well, infinite cycle handling was done in `fn visit`
correctly so just using root node directly solves the issue. I also
found another bug during the process where packages were marked as
"visited" prematurely resulting in not even expanding them and not
showing them at the tree.

## Test Plan

I added two tests with snapshots.

---------

Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
This commit is contained in:
Denizhan Dakılır
2026-03-03 20:37:54 +03:00
committed by GitHub
parent 36955f7527
commit 810072dd62
2 changed files with 291 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/uv-resolver/src/lock/tree.rs
+32 -41
View File
@@ -379,39 +379,27 @@ impl<'env> TreeDisplay<'env> {
roots
} else {
let mut edges = vec![];
let mut roots = if invert {
// For inverted trees, find leaf packages (nodes with no incoming
// edges).
graph
.node_indices()
.filter(|index| {
graph
.edges_directed(*index, Direction::Incoming)
.next()
.is_none()
})
.collect::<Vec<_>>()
} else {
// For non-inverted trees, use the root node directly.
graph
.node_indices()
.filter(|index| matches!(graph[*index], Node::Root))
.collect::<Vec<_>>()
};
// Remove any cycles.
let feedback_set: Vec<EdgeIndex> = petgraph::algo::greedy_feedback_arc_set(&graph)
.map(|e| e.id())
.collect();
for edge_id in feedback_set {
if let Some((source, target)) = graph.edge_endpoints(edge_id) {
if let Some(weight) = graph.remove_edge(edge_id) {
edges.push((source, target, weight));
}
}
}
// Find the root nodes: nodes with no incoming edges, or only an edge from the proxy.
let mut roots = graph
.node_indices()
.filter(|index| {
graph
.edges_directed(*index, Direction::Incoming)
.next()
.is_none()
})
.collect::<Vec<_>>();
// Sort the roots.
roots.sort_by_key(|index| &graph[*index]);
// Re-add the removed edges.
for (source, target, weight) in edges {
graph.add_edge(source, target, weight);
}
roots
}
};
@@ -527,16 +515,19 @@ impl<'env> TreeDisplay<'env> {
let mut lines = vec![line];
// Keep track of the dependency path to avoid cycles.
visited.insert(
package_id,
dependencies
.iter()
.filter_map(|node| match self.graph[node.node()] {
Node::Package(package_id) => Some(package_id),
Node::Root => None,
})
.collect(),
);
// Only mark as visited if we're going to expand children (not at depth limit).
if path.len() < self.depth {
visited.insert(
package_id,
dependencies
.iter()
.filter_map(|node| match self.graph[node.node()] {
Node::Package(package_id) => Some(package_id),
Node::Root => None,
})
.collect(),
);
}
path.push(package_id);
for (index, dep) in dependencies.iter().enumerate() {
crates/uv/tests/it/tree.rs
+259
View File
@@ -1004,6 +1004,265 @@ fn cycle() -> Result<()> {
Ok(())
}
#[test]
fn cycle_no_orphaned_roots() -> Result<()> {
let context = uv_test::test_context!("3.12");
let pyproject_toml = context.temp_dir.child("pyproject.toml");
pyproject_toml.write_str(
r#"
[project]
name = "project"
version = "0.1.0"
requires-python = ">=3.12"
dependencies = ["testtools==2.3.0", "fixtures==3.0.0"]
"#,
)?;
// With --depth 1, only "project" should appear as a root — transitive deps
// involved in cycles (e.g. testtools <-> fixtures) must not be promoted to roots.
uv_snapshot!(context.filters(), context.tree().arg("--universal").arg("--depth").arg("1"), @r###"
success: true
exit_code: 0
----- stdout -----
project v0.1.0
├── fixtures v3.0.0
└── testtools v2.3.0
----- stderr -----
Resolved 11 packages in [TIME]
"###);
Ok(())
}
#[test]
fn cycle_no_infinite_loop() -> Result<()> {
let context = uv_test::test_context!("3.12");
let pyproject_toml = context.temp_dir.child("pyproject.toml");
pyproject_toml.write_str(
r#"
[project]
name = "project"
version = "0.1.0"
requires-python = ">=3.12"
dependencies = ["testtools==2.3.0", "fixtures==3.0.0"]
"#,
)?;
// This should complete without hanging, and cycles should be marked with (*)
uv_snapshot!(context.filters(), context.tree().arg("--universal").arg("--depth").arg("2"), @r###"
success: true
exit_code: 0
----- stdout -----
project v0.1.0
├── fixtures v3.0.0
│ ├── pbr v6.0.0
│ ├── six v1.16.0
│ └── testtools v2.3.0
└── testtools v2.3.0
├── extras v1.0.0
├── fixtures v3.0.0 (*)
├── pbr v6.0.0
├── python-mimeparse v1.6.0
├── six v1.16.0
├── traceback2 v1.4.0
└── unittest2 v1.1.0
(*) Package tree already displayed
----- stderr -----
Resolved 11 packages in [TIME]
"###
);
Ok(())
}
#[test]
fn cycle_invert() -> Result<()> {
let context = uv_test::test_context!("3.12");
let pyproject_toml = context.temp_dir.child("pyproject.toml");
pyproject_toml.write_str(
r#"
[project]
name = "project"
version = "0.1.0"
requires-python = ">=3.12"
dependencies = ["testtools==2.3.0", "fixtures==3.0.0"]
"#,
)?;
// With --invert, leaf packages should be roots and the tree should show
// reverse dependencies without orphaned roots from cycle-breaking.
uv_snapshot!(context.filters(), context.tree().arg("--universal").arg("--invert").arg("--depth").arg("1"), @r###"
success: true
exit_code: 0
----- stdout -----
argparse v1.4.0
└── unittest2 v1.1.0
extras v1.0.0
└── testtools v2.3.0
linecache2 v1.0.0
└── traceback2 v1.4.0
pbr v6.0.0
├── fixtures v3.0.0
└── testtools v2.3.0
python-mimeparse v1.6.0
└── testtools v2.3.0
six v1.16.0
├── fixtures v3.0.0
├── testtools v2.3.0
└── unittest2 v1.1.0
----- stderr -----
Resolved 11 packages in [TIME]
"###);
Ok(())
}
#[test]
fn cycle_depth_boundary_no_premature_dedupe() -> Result<()> {
let context = uv_test::test_context!("3.12");
let pyproject_toml = context.temp_dir.child("pyproject.toml");
pyproject_toml.write_str(
r#"
[project]
name = "project"
version = "0.1.0"
requires-python = ">=3.12"
dependencies = ["testtools==2.3.0", "fixtures==3.0.0"]
"#,
)?;
// With --depth 3, packages at the depth boundary (depth 3) are shown but not
// marked as visited. Packages below the boundary (e.g., `fixtures` at depth 1)
// are correctly marked visited and show (*) on later appearances. Leaf packages
// like `pbr` (no children in this graph) appear without (*) even when visited,
// since there is nothing to deduplicate.
uv_snapshot!(context.filters(), context.tree().arg("--universal").arg("--depth").arg("3"), @r"
success: true
exit_code: 0
----- stdout -----
project v0.1.0
├── fixtures v3.0.0
│ ├── pbr v6.0.0
│ ├── six v1.16.0
│ └── testtools v2.3.0
│ ├── extras v1.0.0
│ ├── fixtures v3.0.0 (*)
│ ├── pbr v6.0.0
│ ├── python-mimeparse v1.6.0
│ ├── six v1.16.0
│ ├── traceback2 v1.4.0
│ └── unittest2 v1.1.0
└── testtools v2.3.0 (*)
(*) Package tree already displayed
----- stderr -----
Resolved 11 packages in [TIME]
");
Ok(())
}
#[test]
fn cycle_invert_deep() -> Result<()> {
let context = uv_test::test_context!("3.12");
let pyproject_toml = context.temp_dir.child("pyproject.toml");
pyproject_toml.write_str(
r#"
[project]
name = "project"
version = "0.1.0"
requires-python = ">=3.12"
dependencies = ["testtools==2.3.0", "fixtures==3.0.0"]
"#,
)?;
// With --invert and --depth 2, cycles in the reversed graph should be
// detected and marked with (*) without causing infinite loops.
uv_snapshot!(context.filters(), context.tree().arg("--universal").arg("--invert").arg("--depth").arg("2"), @r"
success: true
exit_code: 0
----- stdout -----
argparse v1.4.0
└── unittest2 v1.1.0
└── testtools v2.3.0
extras v1.0.0
└── testtools v2.3.0
├── fixtures v3.0.0
└── project v0.1.0
linecache2 v1.0.0
└── traceback2 v1.4.0
├── testtools v2.3.0 (*)
└── unittest2 v1.1.0 (*)
pbr v6.0.0
├── fixtures v3.0.0
│ ├── project v0.1.0
│ └── testtools v2.3.0 (*)
└── testtools v2.3.0 (*)
python-mimeparse v1.6.0
└── testtools v2.3.0 (*)
six v1.16.0
├── fixtures v3.0.0 (*)
├── testtools v2.3.0 (*)
└── unittest2 v1.1.0 (*)
(*) Package tree already displayed
----- stderr -----
Resolved 11 packages in [TIME]
");
Ok(())
}
#[test]
fn cycle_depth_no_dedupe() -> Result<()> {
let context = uv_test::test_context!("3.12");
let pyproject_toml = context.temp_dir.child("pyproject.toml");
pyproject_toml.write_str(
r#"
[project]
name = "project"
version = "0.1.0"
requires-python = ">=3.12"
dependencies = ["testtools==2.3.0", "fixtures==3.0.0"]
"#,
)?;
// With --no-dedupe and --depth 2, packages should be expanded each time they
// appear (up to the depth limit), and cycles should still be marked with (*).
uv_snapshot!(context.filters(), context.tree().arg("--universal").arg("--no-dedupe").arg("--depth").arg("2"), @r###"
success: true
exit_code: 0
----- stdout -----
project v0.1.0
├── fixtures v3.0.0
│ ├── pbr v6.0.0
│ ├── six v1.16.0
│ └── testtools v2.3.0
└── testtools v2.3.0
├── extras v1.0.0
├── fixtures v3.0.0
├── pbr v6.0.0
├── python-mimeparse v1.6.0
├── six v1.16.0
├── traceback2 v1.4.0
└── unittest2 v1.1.0
----- stderr -----
Resolved 11 packages in [TIME]
"###);
Ok(())
}
#[test]
fn workspace_dev() -> Result<()> {
let context = uv_test::test_context!("3.12");