Skip to content

Commit ae03abc

Browse files
charliermarshcharliermarsh
committed
Support cyclic dependencies in uv tree
This reverts commit 7045db232d8f8aaf3f898581af51c7bd686f4240.
1 parent 084ad2e commit ae03abc

File tree

2 files changed

+190
-56
lines changed

2 files changed

+190
-56
lines changed

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

Lines changed: 104 additions & 56 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::DevGroupsManifest;
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

@@ -136,29 +171,24 @@ impl<'env> TreeDisplay<'env> {
136171

137172
// Step 1: Filter out packages that aren't reachable on this platform.
138173
if let Some(environment_markers) = markers {
139-
let mut reachable = FxHashSet::default();
140-
141174
// Perform a DFS from the root nodes to find the reachable nodes, following only the
142175
// production edges.
143-
let mut stack = graph
176+
let mut reachable = graph
144177
.node_indices()
145-
.filter(|index| {
146-
graph
147-
.edges_directed(*index, Direction::Incoming)
148-
.next()
149-
.is_none()
150-
})
151-
.collect::<VecDeque<_>>();
178+
.filter(|index| members.contains(graph[*index]))
179+
.collect::<FxHashSet<_>>();
180+
let mut stack = reachable.iter().copied().collect::<VecDeque<_>>();
152181
while let Some(node) = stack.pop_front() {
153-
reachable.insert(node);
154182
for edge in graph.edges_directed(node, Direction::Outgoing) {
155183
if edge
156184
.weight()
157185
.dependency()
158186
.complexified_marker
159187
.evaluate(environment_markers, &[])
160188
{
161-
stack.push_back(edge.target());
189+
if reachable.insert(edge.target()) {
190+
stack.push_back(edge.target());
191+
}
162192
}
163193
}
164194
}
@@ -167,32 +197,26 @@ impl<'env> TreeDisplay<'env> {
167197
graph.retain_nodes(|_, index| reachable.contains(&index));
168198
}
169199

170-
// Step 2: Filter the graph to those that are reachable in production or development, if
171-
// `--no-dev` or `--only-dev` were specified, respectively.
200+
// Step 2: Filter the graph to those that are reachable in production or development.
172201
{
173-
let mut reachable = FxHashSet::default();
174-
175202
// Perform a DFS from the root nodes to find the reachable nodes, following only the
176203
// production edges.
177-
let mut stack = graph
204+
let mut reachable = graph
178205
.node_indices()
179-
.filter(|index| {
180-
graph
181-
.edges_directed(*index, Direction::Incoming)
182-
.next()
183-
.is_none()
184-
})
185-
.collect::<VecDeque<_>>();
206+
.filter(|index| members.contains(graph[*index]))
207+
.collect::<FxHashSet<_>>();
208+
let mut stack = reachable.iter().copied().collect::<VecDeque<_>>();
186209
while let Some(node) = stack.pop_front() {
187-
reachable.insert(node);
188210
for edge in graph.edges_directed(node, Direction::Outgoing) {
189211
let include = match edge.weight() {
190212
Edge::Prod(_) => dev.prod(),
191213
Edge::Optional(_, _) => dev.prod(),
192214
Edge::Dev(group, _) => dev.iter().contains(*group),
193215
};
194216
if include {
195-
stack.push_back(edge.target());
217+
if reachable.insert(edge.target()) {
218+
stack.push_back(edge.target());
219+
}
196220
}
197221
}
198222
}
@@ -208,24 +232,62 @@ impl<'env> TreeDisplay<'env> {
208232

209233
// Step 4: Filter the graph to those nodes reachable from the target packages.
210234
if !packages.is_empty() {
211-
let mut reachable = FxHashSet::default();
212-
213235
// Perform a DFS from the root nodes to find the reachable nodes.
214-
let mut dfs = Dfs {
215-
stack: graph
216-
.node_indices()
217-
.filter(|index| packages.contains(&graph[*index].name))
218-
.collect::<Vec<_>>(),
219-
..Dfs::empty(&graph)
220-
};
221-
while let Some(node) = dfs.next(&graph) {
222-
reachable.insert(node);
236+
let mut reachable = graph
237+
.node_indices()
238+
.filter(|index| packages.contains(&graph[*index].name))
239+
.collect::<FxHashSet<_>>();
240+
let mut stack = reachable.iter().copied().collect::<VecDeque<_>>();
241+
while let Some(node) = stack.pop_front() {
242+
for edge in graph.edges_directed(node, Direction::Outgoing) {
243+
if reachable.insert(edge.target()) {
244+
stack.push_back(edge.target());
245+
}
246+
}
223247
}
224248

225249
// Remove the unreachable nodes from the graph.
226250
graph.retain_nodes(|_, index| reachable.contains(&index));
227251
}
228252

253+
// Compute the list of roots.
254+
let roots = {
255+
let mut edges = vec![];
256+
257+
// Remove any cycles.
258+
let feedback_set: Vec<EdgeIndex> = petgraph::algo::greedy_feedback_arc_set(&graph)
259+
.map(|e| e.id())
260+
.collect();
261+
for edge_id in feedback_set {
262+
if let Some((source, target)) = graph.edge_endpoints(edge_id) {
263+
if let Some(weight) = graph.remove_edge(edge_id) {
264+
edges.push((source, target, weight));
265+
}
266+
}
267+
}
268+
269+
// Find the root nodes.
270+
let mut roots = graph
271+
.node_indices()
272+
.filter(|index| {
273+
graph
274+
.edges_directed(*index, Direction::Incoming)
275+
.next()
276+
.is_none()
277+
})
278+
.collect::<Vec<_>>();
279+
280+
// Sort the roots.
281+
roots.sort_by_key(|index| &graph[*index]);
282+
283+
// Re-add the removed edges.
284+
for (source, target, weight) in edges {
285+
graph.add_edge(source, target, weight);
286+
}
287+
288+
roots
289+
};
290+
229291
// Re-create the inverse map.
230292
{
231293
inverse.clear();
@@ -239,6 +301,7 @@ impl<'env> TreeDisplay<'env> {
239301
inverse,
240302
depth,
241303
no_dedupe,
304+
roots,
242305
}
243306
}
244307

@@ -355,24 +418,9 @@ impl<'env> TreeDisplay<'env> {
355418
let mut path = Vec::new();
356419
let mut lines = Vec::new();
357420

358-
let roots = {
359-
let mut roots = self
360-
.graph
361-
.node_indices()
362-
.filter(|index| {
363-
self.graph
364-
.edges_directed(*index, petgraph::Direction::Incoming)
365-
.next()
366-
.is_none()
367-
})
368-
.collect::<Vec<_>>();
369-
roots.sort_by_key(|index| &self.graph[*index]);
370-
roots
371-
};
372-
373-
for node in roots {
421+
for node in &self.roots {
374422
path.clear();
375-
lines.extend(self.visit(Node::Root(self.graph[node]), &mut visited, &mut path));
423+
lines.extend(self.visit(Node::Root(self.graph[*node]), &mut visited, &mut path));
376424
}
377425

378426
lines

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

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

836836
Ok(())
837837
}
838+
839+
#[test]
840+
fn cycle() -> Result<()> {
841+
let context = TestContext::new("3.12");
842+
843+
let pyproject_toml = context.temp_dir.child("pyproject.toml");
844+
pyproject_toml.write_str(
845+
r#"
846+
[project]
847+
name = "project"
848+
version = "0.1.0"
849+
requires-python = ">=3.12"
850+
dependencies = ["testtools==2.3.0", "fixtures==3.0.0"]
851+
"#,
852+
)?;
853+
854+
uv_snapshot!(context.filters(), context.tree().arg("--universal"), @r###"
855+
success: true
856+
exit_code: 0
857+
----- stdout -----
858+
project v0.1.0
859+
├── fixtures v3.0.0
860+
│ ├── pbr v6.0.0
861+
│ ├── six v1.16.0
862+
│ └── testtools v2.3.0
863+
│ ├── extras v1.0.0
864+
│ ├── fixtures v3.0.0 (*)
865+
│ ├── pbr v6.0.0
866+
│ ├── python-mimeparse v1.6.0
867+
│ ├── six v1.16.0
868+
│ ├── traceback2 v1.4.0
869+
│ │ └── linecache2 v1.0.0
870+
│ └── unittest2 v1.1.0
871+
│ ├── argparse v1.4.0
872+
│ ├── six v1.16.0
873+
│ └── traceback2 v1.4.0 (*)
874+
└── testtools v2.3.0 (*)
875+
(*) Package tree already displayed
876+
877+
----- stderr -----
878+
Resolved 11 packages in [TIME]
879+
"###
880+
);
881+
882+
uv_snapshot!(context.filters(), context.tree().arg("--package").arg("traceback2").arg("--package").arg("six"), @r###"
883+
success: true
884+
exit_code: 0
885+
----- stdout -----
886+
six v1.16.0
887+
traceback2 v1.4.0
888+
└── linecache2 v1.0.0
889+
890+
----- stderr -----
891+
Resolved 11 packages in [TIME]
892+
"###
893+
);
894+
895+
uv_snapshot!(context.filters(), context.tree().arg("--package").arg("traceback2").arg("--package").arg("six").arg("--invert"), @r###"
896+
success: true
897+
exit_code: 0
898+
----- stdout -----
899+
six v1.16.0
900+
├── fixtures v3.0.0
901+
│ ├── project v0.1.0
902+
│ └── testtools v2.3.0
903+
│ ├── fixtures v3.0.0 (*)
904+
│ └── project v0.1.0
905+
├── testtools v2.3.0 (*)
906+
└── unittest2 v1.1.0
907+
└── testtools v2.3.0 (*)
908+
traceback2 v1.4.0
909+
├── testtools v2.3.0 (*)
910+
└── unittest2 v1.1.0 (*)
911+
(*) Package tree already displayed
912+
913+
----- stderr -----
914+
Resolved 11 packages in [TIME]
915+
"###
916+
);
917+
918+
// `uv tree` should update the lockfile
919+
let lock = context.read("uv.lock");
920+
assert!(!lock.is_empty());
921+
922+
Ok(())
923+
}

0 commit comments

Comments
 (0)