From 24ad43e79c34df85fd7b1d06f588614af2ab730b Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 31 Oct 2024 16:09:36 -0400 Subject: [PATCH] Avoid showing dependency group annotations on workspace members in tree (#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 https://github.com/astral-sh/uv/issues/8719. --- crates/uv-resolver/src/lock/tree.rs | 176 ++++++++++++---------------- crates/uv/tests/it/tree.rs | 80 ++++++++++++- 2 files changed, 152 insertions(+), 104 deletions(-) diff --git a/crates/uv-resolver/src/lock/tree.rs b/crates/uv-resolver/src/lock/tree.rs index a888dd9a2..89191b3d6 100644 --- a/crates/uv-resolver/src/lock/tree.rs +++ b/crates/uv-resolver/src/lock/tree.rs @@ -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 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)), - ); - } - - 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) { @@ -132,73 +111,72 @@ impl<'env> TreeDisplay<'env> { graph.add_edge( package_node, dependency_node, - Edge::Optional(extra, Cow::Borrowed(dependency)), + Edge::Prod(Cow::Borrowed(dependency)), ); } } + 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; + } + + // 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)), + ); + } + } + } + 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 - }; - - // Add an edge between the package and the dependency. - graph.add_edge( - package_node, - dependency_node, - Edge::Dev(group, 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::>(); - let mut stack = reachable.iter().copied().collect::>(); - 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()); + 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])) @@ -206,15 +184,8 @@ impl<'env> TreeDisplay<'env> { let mut stack = reachable.iter().copied().collect::>(); 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()); } } } @@ -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)) diff --git a/crates/uv/tests/it/tree.rs b/crates/uv/tests/it/tree.rs index 9cb9c6edb..c375b11a0 100644 --- a/crates/uv/tests/it/tree.rs +++ b/crates/uv/tests/it/tree.rs @@ -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"); @@ -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(()) +}