Skip to content

Commit

Permalink
Implement --show-version-specifiers for tree (astral-sh#5240)
Browse files Browse the repository at this point in the history
## Summary
resolves astral-sh#5217

## Test Plan
existing tests pass (should be perfectly backwards compatible) + added a
few tests to cover the new functionality. in particular, in addition to
the simple use of `--show-version-specifiers`, its interaction with
`--invert` and `--package` flags are tested.
  • Loading branch information
ChannyClaus authored Jul 20, 2024
1 parent 1b09cb2 commit 12518a0
Show file tree
Hide file tree
Showing 8 changed files with 389 additions and 17 deletions.
18 changes: 18 additions & 0 deletions crates/pep508-rs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,15 @@ pub enum VersionOrUrl<T: Pep508Url = VerbatimUrl> {
Url(T),
}

impl<T: Pep508Url> Display for VersionOrUrl<T> {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
Self::VersionSpecifier(version_specifier) => Display::fmt(version_specifier, f),
Self::Url(url) => Display::fmt(url, f),
}
}
}

/// Unowned version specifier or URL to install.
#[derive(Debug, Clone, Copy, Eq, Hash, PartialEq)]
pub enum VersionOrUrlRef<'a, T: Pep508Url = VerbatimUrl> {
Expand All @@ -576,6 +585,15 @@ pub enum VersionOrUrlRef<'a, T: Pep508Url = VerbatimUrl> {
Url(&'a T),
}

impl<T: Pep508Url> Display for VersionOrUrlRef<'_, T> {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
Self::VersionSpecifier(version_specifier) => Display::fmt(version_specifier, f),
Self::Url(url) => Display::fmt(url, f),
}
}
}

impl<'a> From<&'a VersionOrUrl> for VersionOrUrlRef<'a> {
fn from(value: &'a VersionOrUrl) -> Self {
match value {
Expand Down
18 changes: 18 additions & 0 deletions crates/pypi-types/src/requirement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,24 @@ impl RequirementSource {
}
}

/// Convert the source to a version specifier or URL.
///
/// If the source is a registry and the specifier is empty, it returns `None`.
pub fn version_or_url(&self) -> Option<VersionOrUrl<VerbatimParsedUrl>> {
match self {
Self::Registry { specifier, .. } => {
if specifier.len() == 0 {
None
} else {
Some(VersionOrUrl::VersionSpecifier(specifier.clone()))
}
}
Self::Url { .. } | Self::Git { .. } | Self::Path { .. } | Self::Directory { .. } => {
Some(VersionOrUrl::Url(self.to_verbatim_parsed_url()?))
}
}
}

/// Returns `true` if the source is editable.
pub fn is_editable(&self) -> bool {
matches!(self, Self::Directory { editable: true, .. })
Expand Down
4 changes: 4 additions & 0 deletions crates/uv-cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2764,4 +2764,8 @@ pub struct DisplayTreeArgs {
/// Show the reverse dependencies for the given package. This flag will invert the tree and display the packages that depend on the given package.
#[arg(long, alias = "reverse")]
pub invert: bool,

/// Show the version constraint(s) imposed on each package.
#[arg(long)]
pub show_version_specifiers: bool,
}
112 changes: 96 additions & 16 deletions crates/uv/src/commands/pip/tree.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
use std::collections::{HashMap, HashSet};
use std::fmt::Write;

use anyhow::Result;
use indexmap::IndexMap;
use owo_colors::OwoColorize;
use rustc_hash::FxHashMap;
use rustc_hash::{FxHashMap, FxHashSet};
use tracing::debug;

use distribution_types::{Diagnostic, Name};
use pep508_rs::MarkerEnvironment;
use pypi_types::RequirementSource;
use uv_cache::Cache;
use uv_distribution::Metadata;
use uv_fs::Simplified;
Expand All @@ -29,6 +29,7 @@ pub(crate) fn pip_tree(
package: Vec<PackageName>,
no_dedupe: bool,
invert: bool,
show_version_specifiers: bool,
strict: bool,
python: Option<&str>,
system: bool,
Expand Down Expand Up @@ -66,6 +67,7 @@ pub(crate) fn pip_tree(
package,
no_dedupe,
invert,
show_version_specifiers,
environment.interpreter().markers(),
packages,
)
Expand All @@ -74,7 +76,7 @@ pub(crate) fn pip_tree(

writeln!(printer.stdout(), "{rendered_tree}")?;

if rendered_tree.contains('*') {
if rendered_tree.contains("(*)") {
let message = if no_dedupe {
"(*) Package tree is a cycle and cannot be shown".italic()
} else {
Expand Down Expand Up @@ -113,7 +115,9 @@ pub(crate) struct DisplayDependencyGraph {
/// Map from package name to its requirements.
///
/// If `--invert` is given the map is inverted.
requirements: HashMap<PackageName, Vec<PackageName>>,
requirements: FxHashMap<PackageName, Vec<PackageName>>,
/// Map from requirement package name-to-parent-to-dependency metadata.
dependencies: FxHashMap<PackageName, FxHashMap<PackageName, Dependency>>,
}

impl DisplayDependencyGraph {
Expand All @@ -124,10 +128,13 @@ impl DisplayDependencyGraph {
package: Vec<PackageName>,
no_dedupe: bool,
invert: bool,
show_version_specifiers: bool,
markers: &MarkerEnvironment,
packages: IndexMap<PackageName, Vec<Metadata>>,
) -> Self {
let mut requirements: HashMap<_, Vec<_>> = HashMap::new();
let mut requirements: FxHashMap<_, Vec<_>> = FxHashMap::default();
let mut dependencies: FxHashMap<PackageName, FxHashMap<PackageName, Dependency>> =
FxHashMap::default();

// Add all transitive requirements.
for metadata in packages.values().flatten() {
Expand All @@ -138,27 +145,41 @@ impl DisplayDependencyGraph {
.as_ref()
.map_or(true, |m| m.evaluate(markers, &[]))
}) {
if invert {
requirements
.entry(required.name.clone())
.or_default()
.push(metadata.name.clone());
let dependency = if invert {
Dependency::Inverted(
required.name.clone(),
metadata.name.clone(),
required.source.clone(),
)
} else {
requirements
.entry(metadata.name.clone())
Dependency::Normal(
metadata.name.clone(),
required.name.clone(),
required.source.clone(),
)
};

requirements
.entry(dependency.parent().clone())
.or_default()
.push(dependency.child().clone());

if show_version_specifiers {
dependencies
.entry(dependency.parent().clone())
.or_default()
.push(required.name.clone());
.insert(dependency.child().clone(), dependency);
}
}
}

Self {
packages,
depth,
prune,
package,
no_dedupe,
requirements,
dependencies,
}
}

Expand All @@ -175,7 +196,19 @@ impl DisplayDependencyGraph {
}

let package_name = &metadata.name;
let line = format!("{} v{}", package_name, metadata.version);
let mut line = format!("{} v{}", package_name, metadata.version);

// If the current package is not top-level (i.e., it has a parent), include the specifiers.
if let Some(last) = path.last().copied() {
if let Some(dependency) = self
.dependencies
.get(last)
.and_then(|deps| deps.get(package_name))
{
line.push(' ');
line.push_str(&format!("[{dependency}]"));
}
}

// Skip the traversal if:
// 1. The package is in the current traversal path (i.e., a dependency cycle).
Expand Down Expand Up @@ -261,7 +294,7 @@ impl DisplayDependencyGraph {

if self.package.is_empty() {
// The root nodes are those that are not required by any other package.
let children: HashSet<_> = self.requirements.values().flatten().collect();
let children: FxHashSet<_> = self.requirements.values().flatten().collect();
for package in self.packages.values().flatten() {
// If the current package is not required by any other package, start the traversal
// with the current package as the root.
Expand All @@ -286,3 +319,50 @@ impl DisplayDependencyGraph {
lines
}
}

#[derive(Debug)]
enum Dependency {
/// Show dependencies from parent to the child package that it requires.
Normal(PackageName, PackageName, RequirementSource),
/// Show dependencies from the child package to the parent that requires it.
Inverted(PackageName, PackageName, RequirementSource),
}

impl Dependency {
/// Return the parent in the tree.
fn parent(&self) -> &PackageName {
match self {
Self::Normal(parent, _, _) => parent,
Self::Inverted(parent, _, _) => parent,
}
}

/// Return the child in the tree.
fn child(&self) -> &PackageName {
match self {
Self::Normal(_, child, _) => child,
Self::Inverted(_, child, _) => child,
}
}
}

impl std::fmt::Display for Dependency {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::Normal(_, _, source) => {
let version = match source.version_or_url() {
None => "*".to_string(),
Some(version) => version.to_string(),
};
write!(f, "required: {version}")
}
Self::Inverted(parent, _, source) => {
let version = match source.version_or_url() {
None => "*".to_string(),
Some(version) => version.to_string(),
};
write!(f, "requires: {parent} {version}")
}
}
}
}
2 changes: 2 additions & 0 deletions crates/uv/src/commands/project/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ pub(crate) async fn tree(
package: Vec<PackageName>,
no_dedupe: bool,
invert: bool,
show_version_specifiers: bool,
python: Option<String>,
settings: ResolverSettings,
python_preference: PythonPreference,
Expand Down Expand Up @@ -94,6 +95,7 @@ pub(crate) async fn tree(
package,
no_dedupe,
invert,
show_version_specifiers,
interpreter.markers(),
packages,
)
Expand Down
2 changes: 2 additions & 0 deletions crates/uv/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,7 @@ async fn run(cli: Cli) -> Result<ExitStatus> {
args.package,
args.no_dedupe,
args.invert,
args.show_version_specifiers,
args.shared.strict,
args.shared.python.as_deref(),
args.shared.system,
Expand Down Expand Up @@ -999,6 +1000,7 @@ async fn run_project(
args.package,
args.no_dedupe,
args.invert,
args.show_version_specifiers,
args.python,
args.resolver,
globals.python_preference,
Expand Down
4 changes: 4 additions & 0 deletions crates/uv/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -725,6 +725,7 @@ pub(crate) struct TreeSettings {
pub(crate) package: Vec<PackageName>,
pub(crate) no_dedupe: bool,
pub(crate) invert: bool,
pub(crate) show_version_specifiers: bool,
pub(crate) python: Option<String>,
pub(crate) resolver: ResolverSettings,
}
Expand All @@ -749,6 +750,7 @@ impl TreeSettings {
package: tree.package,
no_dedupe: tree.no_dedupe,
invert: tree.invert,
show_version_specifiers: tree.show_version_specifiers,
python,
resolver: ResolverSettings::combine(resolver_options(resolver, build), filesystem),
}
Expand Down Expand Up @@ -1265,6 +1267,7 @@ pub(crate) struct PipTreeSettings {
pub(crate) package: Vec<PackageName>,
pub(crate) no_dedupe: bool,
pub(crate) invert: bool,
pub(crate) show_version_specifiers: bool,
// CLI-only settings.
pub(crate) shared: PipSettings,
}
Expand All @@ -1287,6 +1290,7 @@ impl PipTreeSettings {
prune: tree.prune,
no_dedupe: tree.no_dedupe,
invert: tree.invert,
show_version_specifiers: tree.show_version_specifiers,
package: tree.package,
// Shared settings.
shared: PipSettings::combine(
Expand Down
Loading

0 comments on commit 12518a0

Please sign in to comment.