Omit (*) in uv pip tree for empty packages (#4673)

## Summary

Closes https://github.com/astral-sh/uv/issues/4665.

crates/uv/src/commands/pip/tree.rs

@@ -1,12 +1,15 @@
-use distribution_types::{Diagnostic, InstalledDist, Name};
-use owo_colors::OwoColorize;
-use pep508_rs::MarkerEnvironment;
-use pypi_types::VerbatimParsedUrl;
 use std::collections::{HashMap, HashSet};
 use std::fmt::Write;
+
+use anyhow::Result;
+use owo_colors::OwoColorize;
+use rustc_hash::FxHashMap;
 use tracing::debug;
+
+use distribution_types::{Diagnostic, InstalledDist, Name};
+use pep508_rs::{MarkerEnvironment, Requirement};
+use pypi_types::VerbatimParsedUrl;
 use uv_cache::Cache;
-use uv_configuration::PreviewMode;
 use uv_fs::Simplified;
 use uv_installer::SitePackages;
 use uv_normalize::PackageName;
@@ -25,10 +28,9 @@ pub(crate) fn pip_tree(
     strict: bool,
     python: Option<&str>,
     system: bool,
-    _preview: PreviewMode,
     cache: &Cache,
     printer: Printer,
-) -> anyhow::Result<ExitStatus> {
+) -> Result<ExitStatus> {
     // Detect the current Python interpreter.
     let environment = PythonEnvironment::find(
         &python.map(ToolchainRequest::parse).unwrap_or_default(),
@@ -51,10 +53,12 @@ pub(crate) fn pip_tree(
         prune,
         no_dedupe,
         environment.interpreter().markers(),
-    )
+    )?
-    .render()
+    .render()?
     .join("\n");
-    writeln!(printer.stdout(), "{rendered_tree}").unwrap();
+
+    writeln!(printer.stdout(), "{rendered_tree}")?;
+
     if rendered_tree.contains('*') {
         let message = if no_dedupe {
             "(*) Package tree is a cycle and cannot be shown".italic()
@@ -76,6 +80,7 @@ pub(crate) fn pip_tree(
             )?;
         }
     }
+
     Ok(ExitStatus::Success)
 }
 
@@ -85,12 +90,12 @@ pub(crate) fn pip_tree(
 /// For example, `requests==2.32.3` requires `charset-normalizer`, `idna`, `urllib`, and `certifi` at
 /// all times, `PySocks` on `socks` extra and `chardet` on `use_chardet_on_py3` extra.
 /// This function will return `["charset-normalizer", "idna", "urllib", "certifi"]` for `requests`.
-fn required_with_no_extra(
+fn filtered_requirements(
     dist: &InstalledDist,
     markers: &MarkerEnvironment,
-) -> Vec<pep508_rs::Requirement<VerbatimParsedUrl>> {
+) -> Result<Vec<Requirement<VerbatimParsedUrl>>> {
-    let metadata = dist.metadata().unwrap();
+    Ok(dist
-    return metadata
+        .metadata()?
         .requires_dist
         .into_iter()
         .filter(|requirement| {
@@ -99,7 +104,7 @@ fn required_with_no_extra(
                 .as_ref()
                 .map_or(true, |m| m.evaluate(markers, &[]))
         })
-        .collect::<Vec<_>>();
+        .collect::<Vec<_>>())
 }
 
 #[derive(Debug)]
@@ -129,19 +134,19 @@ impl<'a> DisplayDependencyGraph<'a> {
         prune: Vec<PackageName>,
         no_dedupe: bool,
         markers: &'a MarkerEnvironment,
-    ) -> DisplayDependencyGraph<'a> {
+    ) -> Result<DisplayDependencyGraph<'a>> {
         let mut dist_by_package_name = HashMap::new();
         let mut required_packages = HashSet::new();
         for site_package in site_packages.iter() {
             dist_by_package_name.insert(site_package.name(), site_package);
         }
         for site_package in site_packages.iter() {
-            for required in required_with_no_extra(site_package, markers) {
+            for required in filtered_requirements(site_package, markers)? {
                 required_packages.insert(required.name.clone());
             }
         }
 
-        Self {
+        Ok(Self {
             site_packages,
             dist_by_package_name,
             required_packages,
@@ -149,46 +154,49 @@ impl<'a> DisplayDependencyGraph<'a> {
             prune,
             no_dedupe,
             markers,
-        }
+        })
     }
 
     /// Perform a depth-first traversal of the given distribution and its dependencies.
     fn visit(
         &self,
         installed_dist: &InstalledDist,
-        visited: &mut HashSet<String>,
+        visited: &mut FxHashMap<PackageName, Vec<Requirement<VerbatimParsedUrl>>>,
-        path: &mut Vec<String>,
+        path: &mut Vec<PackageName>,
-    ) -> Vec<String> {
+    ) -> Result<Vec<String>> {
         // Short-circuit if the current path is longer than the provided depth.
         if path.len() > self.depth {
-            return Vec::new();
+            return Ok(Vec::new());
         }
 
-        let package_name = installed_dist.name().to_string();
+        let package_name = installed_dist.name();
-        let is_visited = visited.contains(&package_name);
         let line = format!("{} v{}", package_name, installed_dist.version());
 
-        // Skip the traversal if
+        // Skip the traversal if:
-        // 1. the package is in the current traversal path (i.e. a dependency cycle)
+        // 1. The package is in the current traversal path (i.e., a dependency cycle).
-        // 2. if the package has been visited and de-duplication is enabled (default)
+        // 2. The package has been visited and de-duplication is enabled (default).
-        if path.contains(&package_name) || (is_visited && !self.no_dedupe) {
+        if let Some(requirements) = visited.get(package_name) {
-            return vec![format!("{} (*)", line)];
+            if !self.no_dedupe || path.contains(package_name) {
+                return Ok(if requirements.is_empty() {
+                    vec![line]
+                } else {
+                    vec![format!("{} (*)", line)]
+                });
             }
+        }
+
+        let requirements = filtered_requirements(installed_dist, self.markers)?
+            .into_iter()
+            .filter(|req| !self.prune.contains(&req.name))
+            .collect::<Vec<_>>();
 
         let mut lines = vec![line];
 
+        visited.insert(package_name.clone(), requirements.clone());
         path.push(package_name.clone());
-        visited.insert(package_name.clone());
+        for (index, req) in requirements.iter().enumerate() {
-        let required_packages = required_with_no_extra(installed_dist, self.markers)
-            .into_iter()
-            .filter(|p| !self.prune.contains(&p.name))
-            .collect::<Vec<_>>();
-        for (index, required_package) in required_packages.iter().enumerate() {
             // Skip if the current package is not one of the installed distributions.
-            if !self
+            if !self.dist_by_package_name.contains_key(&req.name) {
-                .dist_by_package_name
-                .contains_key(&required_package.name)
-            {
                 continue;
             }
 
@@ -211,7 +219,7 @@ impl<'a> DisplayDependencyGraph<'a> {
             // those in Group 3 have `└── ` at the top and `    ` at the rest.
             // This observation is true recursively even when looking at the subtree rooted
             // at `level_1_0`.
-            let (prefix_top, prefix_rest) = if required_packages.len() - 1 == index {
+            let (prefix_top, prefix_rest) = if requirements.len() - 1 == index {
                 ("└── ", "    ")
             } else {
                 ("├── ", "│   ")
@@ -219,11 +227,7 @@ impl<'a> DisplayDependencyGraph<'a> {
 
             let mut prefixed_lines = Vec::new();
             for (visited_index, visited_line) in self
-                .visit(
+                .visit(self.dist_by_package_name[&req.name], visited, path)?
-                    self.dist_by_package_name[&required_package.name],
-                    visited,
-                    path,
-                )
                 .iter()
                 .enumerate()
             {
@@ -240,21 +244,26 @@ impl<'a> DisplayDependencyGraph<'a> {
             lines.extend(prefixed_lines);
         }
         path.pop();
-        lines
+
+        Ok(lines)
     }
 
-    // Depth-first traverse the nodes to render the tree.
+    /// Depth-first traverse the nodes to render the tree.
-    // The starting nodes are the ones without incoming edges.
+    fn render(&self) -> Result<Vec<String>> {
-    fn render(&self) -> Vec<String> {
+        let mut visited: FxHashMap<PackageName, Vec<Requirement<VerbatimParsedUrl>>> =
-        let mut visited: HashSet<String> = HashSet::new();
+            FxHashMap::default();
+        let mut path: Vec<PackageName> = Vec::new();
         let mut lines: Vec<String> = Vec::new();
+
+        // The starting nodes are the ones without incoming edges.
         for site_package in self.site_packages.iter() {
             // If the current package is not required by any other package, start the traversal
             // with the current package as the root.
             if !self.required_packages.contains(site_package.name()) {
-                lines.extend(self.visit(site_package, &mut visited, &mut Vec::new()));
+                lines.extend(self.visit(site_package, &mut visited, &mut path)?);
             }
         }
-        lines
+
+        Ok(lines)
     }
 }

crates/uv/src/main.rs

@@ -554,7 +554,6 @@ async fn run() -> Result<ExitStatus> {
                 args.shared.strict,
                 args.shared.python.as_deref(),
                 args.shared.system,
-                globals.preview,
                 &cache,
                 printer,
             )

crates/uv/tests/pip_tree.rs

@@ -203,10 +203,9 @@ fn nested_dependencies() {
     scikit-learn v1.4.1.post1
     ├── numpy v1.26.4
     ├── scipy v1.12.0
-    │   └── numpy v1.26.4 (*)
+    │   └── numpy v1.26.4
     ├── joblib v1.3.2
     └── threadpoolctl v3.4.0
-    (*) Package tree already displayed
 
     ----- stderr -----
     "###
@@ -302,13 +301,11 @@ fn depth() {
     scikit-learn v1.4.1.post1
     ├── numpy v1.26.4
     ├── scipy v1.12.0
-    │   └── numpy v1.26.4 (*)
+    │   └── numpy v1.26.4
     ├── joblib v1.3.2
     └── threadpoolctl v3.4.0
-    (*) Package tree already displayed
 
     ----- stderr -----
-
     "###
     );
 }
@@ -495,20 +492,20 @@ fn nested_dependencies_more_complex() {
         │   └── certifi v2024.2.2
         ├── requests-toolbelt v1.0.0
         │   └── requests v2.31.0 (*)
-        ├── urllib3 v2.2.1 (*)
+        ├── urllib3 v2.2.1
         ├── importlib-metadata v7.1.0
         │   └── zipp v3.18.1
         ├── keyring v25.0.0
         │   ├── jaraco-classes v3.3.1
         │   │   └── more-itertools v10.2.0
         │   ├── jaraco-functools v4.0.0
-        │   │   └── more-itertools v10.2.0 (*)
+        │   │   └── more-itertools v10.2.0
         │   └── jaraco-context v4.3.0
         ├── rfc3986 v2.0.0
         └── rich v13.7.1
             ├── markdown-it-py v3.0.0
             │   └── mdurl v0.1.2
-            └── pygments v2.17.2 (*)
+            └── pygments v2.17.2
     (*) Package tree already displayed
 
     ----- stderr -----
@@ -601,20 +598,20 @@ fn prune_big_tree() {
         │   └── certifi v2024.2.2
         ├── requests-toolbelt v1.0.0
         │   └── requests v2.31.0 (*)
-        ├── urllib3 v2.2.1 (*)
+        ├── urllib3 v2.2.1
         ├── importlib-metadata v7.1.0
         │   └── zipp v3.18.1
         ├── keyring v25.0.0
         │   ├── jaraco-classes v3.3.1
         │   │   └── more-itertools v10.2.0
         │   ├── jaraco-functools v4.0.0
-        │   │   └── more-itertools v10.2.0 (*)
+        │   │   └── more-itertools v10.2.0
         │   └── jaraco-context v4.3.0
         ├── rfc3986 v2.0.0
         └── rich v13.7.1
             ├── markdown-it-py v3.0.0
             │   └── mdurl v0.1.2
-            └── pygments v2.17.2 (*)
+            └── pygments v2.17.2
     (*) Package tree already displayed
 
     ----- stderr -----
@@ -840,7 +837,7 @@ fn multiple_packages_shared_descendant() {
     │   ├── python-dateutil v2.9.0.post0
     │   │   └── six v1.16.0
     │   └── urllib3 v2.2.1
-    ├── jmespath v1.0.1 (*)
+    ├── jmespath v1.0.1
     └── s3transfer v0.10.1
         └── botocore v1.34.69 (*)
     pendulum v3.0.0