Skip to content

Support cyclic dependencies in uv tree #8564

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 26, 2024
Merged

Support cyclic dependencies in uv tree #8564

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
160 changes: 104 additions & 56 deletions crates/uv-resolver/src/lock/tree.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
use std::borrow::Cow;
use std::collections::{BTreeSet, VecDeque};
use std::path::Path;

use itertools::Itertools;
use petgraph::graph::{EdgeIndex, NodeIndex};
use petgraph::prelude::EdgeRef;
use petgraph::visit::Dfs;
use petgraph::Direction;
use rustc_hash::{FxHashMap, FxHashSet};

use uv_configuration::DevGroupsManifest;
use uv_normalize::{ExtraName, GroupName, PackageName};
use uv_pypi_types::ResolverMarkerEnvironment;

use crate::lock::{Dependency, PackageId};
use crate::lock::{Dependency, PackageId, Source};
use crate::Lock;

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

impl<'env> TreeDisplay<'env> {
Expand All @@ -38,6 +41,38 @@ impl<'env> TreeDisplay<'env> {
no_dedupe: bool,
invert: bool,
) -> Self {
// Identify the workspace members.
//
// The members are encoded directly in the lockfile, unless the workspace contains a
// single member at the root, in which case, we identify it by its source.
let members: FxHashSet<&PackageId> = if lock.members().is_empty() {
lock.packages
.iter()
.filter_map(|package| {
let (Source::Editable(path) | Source::Virtual(path)) = &package.id.source
else {
return None;
};
if path == Path::new("") {
Some(&package.id)
} else {
None
}
})
.collect()
} else {
lock.packages
.iter()
.filter_map(|package| {
if lock.members().contains(&package.id.name) {
Some(&package.id)
} else {
None
}
})
.collect()
};

// Create a graph.
let mut graph = petgraph::graph::Graph::<&PackageId, Edge, petgraph::Directed>::new();

Expand Down Expand Up @@ -136,29 +171,24 @@ impl<'env> TreeDisplay<'env> {

// Step 1: Filter out packages that aren't reachable on this platform.
if let Some(environment_markers) = markers {
let mut reachable = FxHashSet::default();

// Perform a DFS from the root nodes to find the reachable nodes, following only the
// production edges.
let mut stack = graph
let mut reachable = graph
.node_indices()
.filter(|index| {
graph
.edges_directed(*index, Direction::Incoming)
.next()
.is_none()
})
.collect::<VecDeque<_>>();
.filter(|index| members.contains(graph[*index]))
.collect::<FxHashSet<_>>();
let mut stack = reachable.iter().copied().collect::<VecDeque<_>>();
while let Some(node) = stack.pop_front() {
reachable.insert(node);
for edge in graph.edges_directed(node, Direction::Outgoing) {
if edge
.weight()
.dependency()
.complexified_marker
.evaluate(environment_markers, &[])
{
stack.push_back(edge.target());
if reachable.insert(edge.target()) {
stack.push_back(edge.target());
}
}
}
}
Expand All @@ -167,32 +197,26 @@ impl<'env> TreeDisplay<'env> {
graph.retain_nodes(|_, index| reachable.contains(&index));
}

// Step 2: Filter the graph to those that are reachable in production or development, if
// `--no-dev` or `--only-dev` were specified, respectively.
// Step 2: Filter the graph to those that are reachable in production or development.
{
let mut reachable = FxHashSet::default();

// Perform a DFS from the root nodes to find the reachable nodes, following only the
// production edges.
let mut stack = graph
let mut reachable = graph
.node_indices()
.filter(|index| {
graph
.edges_directed(*index, Direction::Incoming)
.next()
.is_none()
})
.collect::<VecDeque<_>>();
.filter(|index| members.contains(graph[*index]))
.collect::<FxHashSet<_>>();
let mut stack = reachable.iter().copied().collect::<VecDeque<_>>();
while let Some(node) = stack.pop_front() {
reachable.insert(node);
for edge in graph.edges_directed(node, Direction::Outgoing) {
let include = match edge.weight() {
Edge::Prod(_) => dev.prod(),
Edge::Optional(_, _) => dev.prod(),
Edge::Dev(group, _) => dev.iter().contains(*group),
};
if include {
stack.push_back(edge.target());
if reachable.insert(edge.target()) {
stack.push_back(edge.target());
}
}
}
}
Expand All @@ -208,24 +232,62 @@ impl<'env> TreeDisplay<'env> {

// Step 4: Filter the graph to those nodes reachable from the target packages.
if !packages.is_empty() {
let mut reachable = FxHashSet::default();

// Perform a DFS from the root nodes to find the reachable nodes.
let mut dfs = Dfs {
stack: graph
.node_indices()
.filter(|index| packages.contains(&graph[*index].name))
.collect::<Vec<_>>(),
..Dfs::empty(&graph)
};
while let Some(node) = dfs.next(&graph) {
reachable.insert(node);
let mut reachable = graph
.node_indices()
.filter(|index| packages.contains(&graph[*index].name))
.collect::<FxHashSet<_>>();
let mut stack = reachable.iter().copied().collect::<VecDeque<_>>();
while let Some(node) = stack.pop_front() {
for edge in graph.edges_directed(node, Direction::Outgoing) {
if reachable.insert(edge.target()) {
stack.push_back(edge.target());
}
}
}

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

// Compute the list of roots.
let roots = {
let mut edges = 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.
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
};

// Re-create the inverse map.
{
inverse.clear();
Expand All @@ -239,6 +301,7 @@ impl<'env> TreeDisplay<'env> {
inverse,
depth,
no_dedupe,
roots,
}
}

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

let roots = {
let mut roots = self
.graph
.node_indices()
.filter(|index| {
self.graph
.edges_directed(*index, petgraph::Direction::Incoming)
.next()
.is_none()
})
.collect::<Vec<_>>();
roots.sort_by_key(|index| &self.graph[*index]);
roots
};

for node in roots {
for node in &self.roots {
path.clear();
lines.extend(self.visit(Node::Root(self.graph[node]), &mut visited, &mut path));
lines.extend(self.visit(Node::Root(self.graph[*node]), &mut visited, &mut path));
}

lines
Expand Down
86 changes: 86 additions & 0 deletions crates/uv/tests/it/tree.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -835,3 +835,89 @@ fn group() -> Result<()> {

Ok(())
}

#[test]
fn cycle() -> Result<()> {
let context = TestContext::new("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"]
"#,
)?;

uv_snapshot!(context.filters(), context.tree().arg("--universal"), @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
│ │ └── linecache2 v1.0.0
│ └── unittest2 v1.1.0
│ ├── argparse v1.4.0
│ ├── six v1.16.0
│ └── traceback2 v1.4.0 (*)
└── testtools v2.3.0 (*)
(*) Package tree already displayed

----- stderr -----
Resolved 11 packages in [TIME]
"###
);

uv_snapshot!(context.filters(), context.tree().arg("--package").arg("traceback2").arg("--package").arg("six"), @r###"
success: true
exit_code: 0
----- stdout -----
six v1.16.0
traceback2 v1.4.0
└── linecache2 v1.0.0

----- stderr -----
Resolved 11 packages in [TIME]
"###
);

uv_snapshot!(context.filters(), context.tree().arg("--package").arg("traceback2").arg("--package").arg("six").arg("--invert"), @r###"
success: true
exit_code: 0
----- stdout -----
six v1.16.0
├── fixtures v3.0.0
│ ├── project v0.1.0
│ └── testtools v2.3.0
│ ├── fixtures v3.0.0 (*)
│ └── project v0.1.0
├── testtools v2.3.0 (*)
└── unittest2 v1.1.0
└── testtools v2.3.0 (*)
traceback2 v1.4.0
├── testtools v2.3.0 (*)
└── unittest2 v1.1.0 (*)
(*) Package tree already displayed

----- stderr -----
Resolved 11 packages in [TIME]
"###
);

// `uv tree` should update the lockfile
let lock = context.read("uv.lock");
assert!(!lock.is_empty());

Ok(())
}
Loading