Skip to content

Commit b20d655

Browse files
charliermarshcharliermarsh
committed
Support cyclic dependencies in uv tree
This reverts commit 7045db232d8f8aaf3f898581af51c7bd686f4240.
1 parent 58b5fd4 commit b20d655

File tree

2 files changed

+189
-54
lines changed

2 files changed

+189
-54
lines changed

‎crates/uv-resolver/src/lock/tree.rs‎

Lines changed: 103 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,18 @@
11
use std::borrow::Cow;
22
use std::collections::{BTreeSet, VecDeque};
3+
use std::path::Path;
34

45
use itertools::Itertools;
6+
use petgraph::graph::{EdgeIndex, NodeIndex};
57
use petgraph::prelude::EdgeRef;
6-
use petgraph::visit::Dfs;
78
use petgraph::Direction;
89
use rustc_hash::{FxHashMap, FxHashSet};
910

1011
use uv_configuration::DevMode;
1112
use uv_normalize::{ExtraName, GroupName, PackageName};
1213
use uv_pypi_types::ResolverMarkerEnvironment;
1314

14-
use crate::lock::{Dependency, PackageId};
15+
use crate::lock::{Dependency, PackageId, Source};
1516
use crate::Lock;
1617

1718
#[derive(Debug)]
@@ -24,6 +25,8 @@ pub struct TreeDisplay<'env> {
2425
depth: usize,
2526
/// Whether to de-duplicate the displayed dependencies.
2627
no_dedupe: bool,
28+
/// The packages considered as roots of the dependency tree.
29+
roots: Vec<NodeIndex>,
2730
}
2831

2932
impl<'env> TreeDisplay<'env> {
@@ -38,6 +41,38 @@ impl<'env> TreeDisplay<'env> {
3841
no_dedupe: bool,
3942
invert: bool,
4043
) -> Self {
44+
// Identify the workspace members.
45+
//
46+
// The members are encoded directly in the lockfile, unless the workspace contains a
47+
// single member at the root, in which case, we identify it by its source.
48+
let members: FxHashSet<&PackageId> = if lock.members().is_empty() {
49+
lock.packages
50+
.iter()
51+
.filter_map(|package| {
52+
let (Source::Editable(path) | Source::Virtual(path)) = &package.id.source
53+
else {
54+
return None;
55+
};
56+
if path == Path::new("") {
57+
Some(&package.id)
58+
} else {
59+
None
60+
}
61+
})
62+
.collect()
63+
} else {
64+
lock.packages
65+
.iter()
66+
.filter_map(|package| {
67+
if lock.members().contains(&package.id.name) {
68+
Some(&package.id)
69+
} else {
70+
None
71+
}
72+
})
73+
.collect()
74+
};
75+
4176
// Create a graph.
4277
let mut graph = petgraph::graph::Graph::<&PackageId, Edge, petgraph::Directed>::new();
4378

@@ -138,29 +173,24 @@ impl<'env> TreeDisplay<'env> {
138173

139174
// Step 1: Filter out packages that aren't reachable on this platform.
140175
if let Some(environment_markers) = markers {
141-
let mut reachable = FxHashSet::default();
142-
143176
// Perform a DFS from the root nodes to find the reachable nodes, following only the
144177
// production edges.
145-
let mut stack = graph
178+
let mut reachable = graph
146179
.node_indices()
147-
.filter(|index| {
148-
graph
149-
.edges_directed(*index, Direction::Incoming)
150-
.next()
151-
.is_none()
152-
})
153-
.collect::<VecDeque<_>>();
180+
.filter(|index| members.contains(graph[*index]))
181+
.collect::<FxHashSet<_>>();
182+
let mut stack = reachable.iter().copied().collect::<VecDeque<_>>();
154183
while let Some(node) = stack.pop_front() {
155-
reachable.insert(node);
156184
for edge in graph.edges_directed(node, Direction::Outgoing) {
157185
if edge
158186
.weight()
159187
.dependency()
160188
.complexified_marker
161189
.evaluate(environment_markers, &[])
162190
{
163-
stack.push_back(edge.target());
191+
if reachable.insert(edge.target()) {
192+
stack.push_back(edge.target());
193+
}
164194
}
165195
}
166196
}
@@ -173,24 +203,19 @@ impl<'env> TreeDisplay<'env> {
173203
// Step 2: Filter the graph to those that are reachable in production or development, if
174204
// `--no-dev` or `--only-dev` were specified, respectively.
175205
if dev != DevMode::Include {
176-
let mut reachable = FxHashSet::default();
177-
178206
// Perform a DFS from the root nodes to find the reachable nodes, following only the
179207
// production edges.
180-
let mut stack = graph
208+
let mut reachable = graph
181209
.node_indices()
182-
.filter(|index| {
183-
graph
184-
.edges_directed(*index, Direction::Incoming)
185-
.next()
186-
.is_none()
187-
})
188-
.collect::<VecDeque<_>>();
210+
.filter(|index| members.contains(graph[*index]))
211+
.collect::<FxHashSet<_>>();
212+
let mut stack = reachable.iter().copied().collect::<VecDeque<_>>();
189213
while let Some(node) = stack.pop_front() {
190-
reachable.insert(node);
191214
for edge in graph.edges_directed(node, Direction::Outgoing) {
192215
if matches!(edge.weight(), Edge::Prod(_) | Edge::Optional(_, _)) {
193-
stack.push_back(edge.target());
216+
if reachable.insert(edge.target()) {
217+
stack.push_back(edge.target());
218+
}
194219
}
195220
}
196221
}
@@ -214,25 +239,63 @@ impl<'env> TreeDisplay<'env> {
214239

215240
// Step 4: Filter the graph to those nodes reachable from the target packages.
216241
if !packages.is_empty() {
217-
let mut reachable = FxHashSet::default();
218-
219242
// Perform a DFS from the root nodes to find the reachable nodes.
220-
let mut dfs = Dfs {
221-
stack: graph
222-
.node_indices()
223-
.filter(|index| packages.contains(&graph[*index].name))
224-
.collect::<Vec<_>>(),
225-
..Dfs::empty(&graph)
226-
};
227-
while let Some(node) = dfs.next(&graph) {
228-
reachable.insert(node);
243+
let mut reachable = graph
244+
.node_indices()
245+
.filter(|index| packages.contains(&graph[*index].name))
246+
.collect::<FxHashSet<_>>();
247+
let mut stack = reachable.iter().copied().collect::<VecDeque<_>>();
248+
while let Some(node) = stack.pop_front() {
249+
for edge in graph.edges_directed(node, Direction::Outgoing) {
250+
if reachable.insert(edge.target()) {
251+
stack.push_back(edge.target());
252+
}
253+
}
229254
}
230255

231256
// Remove the unreachable nodes from the graph.
232257
graph.retain_nodes(|_, index| reachable.contains(&index));
233258
modified = true;
234259
}
235260

261+
// Compute the list of roots.
262+
let roots = {
263+
let mut edges = vec![];
264+
265+
// Remove any cycles.
266+
let feedback_set: Vec<EdgeIndex> = petgraph::algo::greedy_feedback_arc_set(&graph)
267+
.map(|e| e.id())
268+
.collect();
269+
for edge_id in feedback_set {
270+
if let Some((source, target)) = graph.edge_endpoints(edge_id) {
271+
if let Some(weight) = graph.remove_edge(edge_id) {
272+
edges.push((source, target, weight));
273+
}
274+
}
275+
}
276+
277+
// Find the root nodes.
278+
let mut roots = graph
279+
.node_indices()
280+
.filter(|index| {
281+
graph
282+
.edges_directed(*index, Direction::Incoming)
283+
.next()
284+
.is_none()
285+
})
286+
.collect::<Vec<_>>();
287+
288+
// Sort the roots.
289+
roots.sort_by_key(|index| &graph[*index]);
290+
291+
// Re-add the removed edges.
292+
for (source, target, weight) in edges {
293+
graph.add_edge(source, target, weight);
294+
}
295+
296+
roots
297+
};
298+
236299
// If the graph was modified, re-create the inverse map.
237300
if modified {
238301
inverse.clear();
@@ -246,6 +309,7 @@ impl<'env> TreeDisplay<'env> {
246309
inverse,
247310
depth,
248311
no_dedupe,
312+
roots,
249313
}
250314
}
251315

@@ -362,24 +426,9 @@ impl<'env> TreeDisplay<'env> {
362426
let mut path = Vec::new();
363427
let mut lines = Vec::new();
364428

365-
let roots = {
366-
let mut roots = self
367-
.graph
368-
.node_indices()
369-
.filter(|index| {
370-
self.graph
371-
.edges_directed(*index, petgraph::Direction::Incoming)
372-
.next()
373-
.is_none()
374-
})
375-
.collect::<Vec<_>>();
376-
roots.sort_by_key(|index| &self.graph[*index]);
377-
roots
378-
};
379-
380-
for node in roots {
429+
for node in &self.roots {
381430
path.clear();
382-
lines.extend(self.visit(Node::Root(self.graph[node]), &mut visited, &mut path));
431+
lines.extend(self.visit(Node::Root(self.graph[*node]), &mut visited, &mut path));
383432
}
384433

385434
lines

‎crates/uv/tests/it/tree.rs‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -755,3 +755,89 @@ fn package() -> Result<()> {
755755

756756
Ok(())
757757
}
758+
759+
#[test]
760+
fn cycle() -> Result<()> {
761+
let context = TestContext::new("3.12");
762+
763+
let pyproject_toml = context.temp_dir.child("pyproject.toml");
764+
pyproject_toml.write_str(
765+
r#"
766+
[project]
767+
name = "project"
768+
version = "0.1.0"
769+
requires-python = ">=3.12"
770+
dependencies = ["testtools==2.3.0", "fixtures==3.0.0"]
771+
"#,
772+
)?;
773+
774+
uv_snapshot!(context.filters(), context.tree().arg("--universal"), @r###"
775+
success: true
776+
exit_code: 0
777+
----- stdout -----
778+
project v0.1.0
779+
├── fixtures v3.0.0
780+
│ ├── pbr v6.0.0
781+
│ ├── six v1.16.0
782+
│ └── testtools v2.3.0
783+
│ ├── extras v1.0.0
784+
│ ├── fixtures v3.0.0 (*)
785+
│ ├── pbr v6.0.0
786+
│ ├── python-mimeparse v1.6.0
787+
│ ├── six v1.16.0
788+
│ ├── traceback2 v1.4.0
789+
│ │ └── linecache2 v1.0.0
790+
│ └── unittest2 v1.1.0
791+
│ ├── argparse v1.4.0
792+
│ ├── six v1.16.0
793+
│ └── traceback2 v1.4.0 (*)
794+
└── testtools v2.3.0 (*)
795+
(*) Package tree already displayed
796+
797+
----- stderr -----
798+
Resolved 11 packages in [TIME]
799+
"###
800+
);
801+
802+
uv_snapshot!(context.filters(), context.tree().arg("--package").arg("traceback2").arg("--package").arg("six"), @r###"
803+
success: true
804+
exit_code: 0
805+
----- stdout -----
806+
six v1.16.0
807+
traceback2 v1.4.0
808+
└── linecache2 v1.0.0
809+
810+
----- stderr -----
811+
Resolved 11 packages in [TIME]
812+
"###
813+
);
814+
815+
uv_snapshot!(context.filters(), context.tree().arg("--package").arg("traceback2").arg("--package").arg("six").arg("--invert"), @r###"
816+
success: true
817+
exit_code: 0
818+
----- stdout -----
819+
six v1.16.0
820+
├── fixtures v3.0.0
821+
│ ├── project v0.1.0
822+
│ └── testtools v2.3.0
823+
│ ├── fixtures v3.0.0 (*)
824+
│ └── project v0.1.0
825+
├── testtools v2.3.0 (*)
826+
└── unittest2 v1.1.0
827+
└── testtools v2.3.0 (*)
828+
traceback2 v1.4.0
829+
├── testtools v2.3.0 (*)
830+
└── unittest2 v1.1.0 (*)
831+
(*) Package tree already displayed
832+
833+
----- stderr -----
834+
Resolved 11 packages in [TIME]
835+
"###
836+
);
837+
838+
// `uv tree` should update the lockfile
839+
let lock = context.read("uv.lock");
840+
assert!(!lock.is_empty());
841+
842+
Ok(())
843+
}

0 commit comments

Comments
 (0)