Skip to content

Commit

Permalink
Avoid showing dependency group annotations on workspace members in tr…
Browse files Browse the repository at this point in the history 
…ee (#8730)

## Summary

By default, `uv tree` shows the full workspace, not _just_ the root. If
the root depended on a workspace member as a dev dependency, then we'd
still show it as `(group: dev)` in `uv tree` even if you passed
`--no-dev`, because we weren't filtering the edges in the right place.

This is still somewhat confusing, because if `root` depends on workspace
member `child` as a dev dependency, `uv tree --no-dev` still shows both.

Closes #8719.
  • Loading branch information
@charliermarsh
charliermarsh authored Oct 31, 2024
1 parent f326458 commit 24ad43e
Show file tree
Hide file tree
Showing 2 changed files with 149 additions and 101 deletions.
170 changes: 70 additions & 100 deletions crates/uv-resolver/src/lock/tree.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,43 +81,22 @@ impl<'env> TreeDisplay<'env> {
continue;
}

for dependency in &package.dependencies {
// Insert the package into the graph.
let package_node = if let Some(index) = inverse.get(&package.id) {
*index
} else {
let index = graph.add_node(&package.id);
inverse.insert(&package.id, index);
index
};

// Insert the dependency into the graph.
let dependency_node = if let Some(index) = inverse.get(&dependency.package_id) {
*index
} else {
let index = graph.add_node(&dependency.package_id);
inverse.insert(&dependency.package_id, index);
index
};

// Add an edge between the package and the dependency.
graph.add_edge(
package_node,
dependency_node,
Edge::Prod(Cow::Borrowed(dependency)),
);
}
// Insert the package into the graph.
let package_node = if let Some(index) = inverse.get(&package.id) {
*index
} else {
let index = graph.add_node(&package.id);
inverse.insert(&package.id, index);
index
};

for (extra, dependencies) in &package.optional_dependencies {
for dependency in dependencies {
// Insert the package into the graph.
let package_node = if let Some(index) = inverse.get(&package.id) {
*index
} else {
let index = graph.add_node(&package.id);
inverse.insert(&package.id, index);
index
};
if dev.prod() {
for dependency in &package.dependencies {
if markers.is_some_and(|markers| {
!dependency.complexified_marker.evaluate(markers, &[])
}) {
continue;
}

// Insert the dependency into the graph.
let dependency_node = if let Some(index) = inverse.get(&dependency.package_id) {
Expand All @@ -132,89 +111,81 @@ impl<'env> TreeDisplay<'env> {
graph.add_edge(
package_node,
dependency_node,
Edge::Optional(extra, Cow::Borrowed(dependency)),
Edge::Prod(Cow::Borrowed(dependency)),
);
}
}

for (group, dependencies) in &package.dependency_groups {
for dependency in dependencies {
// Insert the package into the graph.
let package_node = if let Some(index) = inverse.get(&package.id) {
*index
} else {
let index = graph.add_node(&package.id);
inverse.insert(&package.id, index);
index
};

// Insert the dependency into the graph.
let dependency_node = if let Some(index) = inverse.get(&dependency.package_id) {
*index
} else {
let index = graph.add_node(&dependency.package_id);
inverse.insert(&dependency.package_id, index);
index
};
if dev.prod() {
for (extra, dependencies) in &package.optional_dependencies {
for dependency in dependencies {
if markers.is_some_and(|markers| {
!dependency.complexified_marker.evaluate(markers, &[])
}) {
continue;
}

// Add an edge between the package and the dependency.
graph.add_edge(
package_node,
dependency_node,
Edge::Dev(group, Cow::Borrowed(dependency)),
);
// Insert the dependency into the graph.
let dependency_node =
if let Some(index) = inverse.get(&dependency.package_id) {
*index
} else {
let index = graph.add_node(&dependency.package_id);
inverse.insert(&dependency.package_id, index);
index
};

// Add an edge between the package and the dependency.
graph.add_edge(
package_node,
dependency_node,
Edge::Optional(extra, Cow::Borrowed(dependency)),
);
}
}
}
}

// Step 1: Filter out packages that aren't reachable on this platform.
if let Some(environment_markers) = markers {
// Perform a DFS from the root nodes to find the reachable nodes, following only the
// production edges.
let mut reachable = graph
.node_indices()
.filter(|index| members.contains(graph[*index]))
.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 edge
.weight()
.dependency()
.complexified_marker
.evaluate(environment_markers, &[])
{
if reachable.insert(edge.target()) {
stack.push_back(edge.target());
for (group, dependencies) in &package.dependency_groups {
if dev.iter().contains(group) {
for dependency in dependencies {
if markers.is_some_and(|markers| {
!dependency.complexified_marker.evaluate(markers, &[])
}) {
continue;
}

// Insert the dependency into the graph.
let dependency_node =
if let Some(index) = inverse.get(&dependency.package_id) {
*index
} else {
let index = graph.add_node(&dependency.package_id);
inverse.insert(&dependency.package_id, index);
index
};

// Add an edge between the package and the dependency.
graph.add_edge(
package_node,
dependency_node,
Edge::Dev(group, Cow::Borrowed(dependency)),
);
}
}
}

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

// Step 2: Filter the graph to those that are reachable in production or development.
// Filter the graph to remove any unreachable nodes.
{
// Perform a DFS from the root nodes to find the reachable nodes, following only the
// production edges.
let mut reachable = graph
.node_indices()
.filter(|index| members.contains(graph[*index]))
.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) {
let include = match edge.weight() {
Edge::Prod(_) => dev.prod(),
Edge::Optional(_, _) => dev.prod(),
Edge::Dev(group, _) => dev.iter().contains(*group),
};
if include {
if reachable.insert(edge.target()) {
stack.push_back(edge.target());
}
if reachable.insert(edge.target()) {
stack.push_back(edge.target());
}
}
}
Expand All @@ -223,14 +194,13 @@ impl<'env> TreeDisplay<'env> {
graph.retain_nodes(|_, index| reachable.contains(&index));
}

// Step 3: Reverse the graph.
// Reverse the graph.
if invert {
graph.reverse();
}

// Step 4: Filter the graph to those nodes reachable from the target packages.
// Filter the graph to those nodes reachable from the target packages.
if !packages.is_empty() {
// Perform a DFS from the root nodes to find the reachable nodes.
let mut reachable = graph
.node_indices()
.filter(|index| packages.contains(&graph[*index].name))
Expand Down
80 changes: 79 additions & 1 deletion crates/uv/tests/it/tree.rs
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
use crate::common::{uv_snapshot, TestContext};
use anyhow::Result;
use assert_cmd::assert::OutputAssertExt;
use assert_fs::prelude::*;
use indoc::formatdoc;
use url::Url;

use crate::common::{uv_snapshot, TestContext};

#[test]
fn nested_dependencies() -> Result<()> {
let context = TestContext::new("3.12");
Expand Down Expand Up @@ -921,3 +922,80 @@ fn cycle() -> Result<()> {

Ok(())
}

#[test]
fn workspace_dev() -> 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 = ["anyio"]
[dependency-groups]
dev = ["child"]
[tool.uv.workspace]
members = ["child"]
[tool.uv.sources]
child = { workspace = true }
"#,
)?;

let child = context.temp_dir.child("child");
let pyproject_toml = child.child("pyproject.toml");
pyproject_toml.write_str(
r#"
[project]
name = "child"
version = "0.1.0"
requires-python = ">=3.12"
dependencies = ["iniconfig"]
"#,
)?;

uv_snapshot!(context.filters(), context.tree().arg("--universal"), @r###"
success: true
exit_code: 0
----- stdout -----
project v0.1.0
├── anyio v4.3.0
│ ├── idna v3.6
│ └── sniffio v1.3.1
└── child v0.1.0 (group: dev)
└── iniconfig v2.0.0
----- stderr -----
Resolved 6 packages in [TIME]
"###
);

// Under `--no-dev`, the member should still be included, since we show the entire workspace.
// But it shouldn't be considered a dependency of the root.
uv_snapshot!(context.filters(), context.tree().arg("--universal").arg("--no-dev"), @r###"
success: true
exit_code: 0
----- stdout -----
child v0.1.0
└── iniconfig v2.0.0
project v0.1.0
└── anyio v4.3.0
├── idna v3.6
└── sniffio v1.3.1
----- stderr -----
Resolved 6 packages in [TIME]
"###
);

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

Ok(())
}

0 comments on commit 24ad43e

Please sign in to comment.