Skip to content

Commit

Permalink
Include extras and dependency groups in derivation chains (#9113)
Browse files Browse the repository at this point in the history 
## Summary

Displays extras and dependency groups when explaining inclusion.
  • Loading branch information
@charliermarsh
charliermarsh authored Nov 15, 2024
1 parent 8dd095c commit 5bff2ba
Show file tree
Hide file tree
Showing 5 changed files with 128 additions and 70 deletions.
16 changes: 14 additions & 2 deletions crates/uv-distribution-types/src/derivation.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use uv_normalize::PackageName;
use uv_normalize::{ExtraName, GroupName, PackageName};
use uv_pep440::Version;
use version_ranges::Ranges;

Expand Down Expand Up @@ -53,6 +53,10 @@ impl IntoIterator for DerivationChain {
pub struct DerivationStep {
/// The name of the package.
pub name: PackageName,
/// The enabled extra of the package, if any.
pub extra: Option<ExtraName>,
/// The enabled dependency group of the package, if any.
pub group: Option<GroupName>,
/// The version of the package.
pub version: Version,
/// The constraints applied to the subsequent package in the chain.
Expand All @@ -61,9 +65,17 @@ pub struct DerivationStep {

impl DerivationStep {
/// Create a [`DerivationStep`] from a package name and version.
pub fn new(name: PackageName, version: Version, range: Ranges<Version>) -> Self {
pub fn new(
name: PackageName,
extra: Option<ExtraName>,
group: Option<GroupName>,
version: Version,
range: Ranges<Version>,
) -> Self {
Self {
name,
extra,
group,
version,
range,
}
Expand Down
38 changes: 25 additions & 13 deletions crates/uv-resolver/src/resolver/derivation.rs
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use std::collections::VecDeque;

use petgraph::visit::EdgeRef;
use petgraph::Direction;
use pubgrub::{Kind, Range, SelectedDependencies, State};
use rustc_hash::FxHashSet;

use uv_distribution_types::{
DerivationChain, DerivationStep, DistRef, Name, Node, Resolution, ResolvedDist,
DerivationChain, DerivationStep, DistRef, Edge, Name, Node, Resolution, ResolvedDist,
};
use uv_pep440::Version;

Expand Down Expand Up @@ -40,11 +41,11 @@ impl DerivationChainBuilder {

// Perform a BFS to find the shortest path to the root.
let mut queue = VecDeque::new();
queue.push_back((target, Vec::new()));
queue.push_back((target, None, None, Vec::new()));

// TODO(charlie): Consider respecting markers here.
let mut seen = FxHashSet::default();
while let Some((node, mut path)) = queue.pop_front() {
while let Some((node, extra, group, mut path)) = queue.pop_front() {
if !seen.insert(node) {
continue;
}
Expand All @@ -55,16 +56,25 @@ impl DerivationChainBuilder {
return Some(DerivationChain::from_iter(path));
}
Node::Dist { dist, .. } => {
path.push(DerivationStep::new(
dist.name().clone(),
dist.version().clone(),
Range::empty(),
));
for neighbor in resolution
.graph()
.neighbors_directed(node, Direction::Incoming)
{
queue.push_back((neighbor, path.clone()));
for edge in resolution.graph().edges_directed(node, Direction::Incoming) {
let mut path = path.clone();
path.push(DerivationStep::new(
dist.name().clone(),
extra.clone(),
group.clone(),
dist.version().clone(),
Range::empty(),
));
let target = edge.source();
let extra = match edge.weight() {
Edge::Optional(extra, ..) => Some(extra.clone()),
_ => None,
};
let group = match edge.weight() {
Edge::Dev(group, ..) => Some(group.clone()),
_ => None,
};
queue.push_back((target, extra, group, path));
}
}
}
Expand Down Expand Up @@ -109,6 +119,8 @@ impl DerivationChainBuilder {
// Add to the current path.
path.push(DerivationStep::new(
name.clone(),
p1.extra().cloned(),
p1.dev().cloned(),
version.clone(),
v2.clone(),
));
Expand Down
132 changes: 83 additions & 49 deletions crates/uv/src/commands/diagnostics.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use owo_colors::OwoColorize;
use rustc_hash::FxHashMap;
use version_ranges::Ranges;

use uv_distribution_types::{BuiltDist, DerivationChain, Name, SourceDist};
use uv_distribution_types::{BuiltDist, DerivationChain, DerivationStep, Name, SourceDist};
use uv_normalize::PackageName;
use uv_pep440::Version;
use uv_resolver::SentinelRange;
Expand Down Expand Up @@ -158,54 +158,6 @@ impl OperationDiagnostic {
}
}

/// Format a [`DerivationChain`] as a human-readable error message.
fn format_chain(name: &PackageName, version: Option<&Version>, chain: &DerivationChain) -> String {
let mut message = if let Some(version) = version {
format!(
"`{}` (v{}) was included because",
name.cyan(),
version.cyan()
)
} else {
format!("`{}` was included because", name.cyan())
};
let mut range: Option<Ranges<Version>> = None;
for (i, step) in chain.iter().enumerate() {
if i > 0 {
if let Some(range) =
range.filter(|range| *range != Ranges::empty() && *range != Ranges::full())
{
message = format!(
"{message} `{}{}` (v{}) which depends on",
step.name.cyan(),
range.cyan(),
step.version.cyan()
);
} else {
message = format!(
"{message} `{}` (v{}) which depends on",
step.name.cyan(),
step.version.cyan()
);
}
} else {
message = format!(
"{message} `{}` (v{}) depends on",
step.name.cyan(),
step.version.cyan()
);
}
range = Some(SentinelRange::from(&step.range).strip());
}
if let Some(range) = range.filter(|range| *range != Ranges::empty() && *range != Ranges::full())
{
message = format!("{message} `{}{}`", name.cyan(), range.cyan());
} else {
message = format!("{message} `{}`", name.cyan());
}
message
}

/// Render a remote source distribution build failure with a help message.
pub(crate) fn download_and_build(sdist: Box<SourceDist>, chain: &DerivationChain, cause: Error) {
#[derive(Debug, miette::Diagnostic, thiserror::Error)]
Expand Down Expand Up @@ -351,3 +303,85 @@ pub(crate) fn no_solution_hint(err: uv_resolver::NoSolutionError, help: String)
let report = miette::Report::new(Error { header, err, help });
anstream::eprint!("{report:?}");
}

/// Format a [`DerivationChain`] as a human-readable error message.
fn format_chain(name: &PackageName, version: Option<&Version>, chain: &DerivationChain) -> String {
/// Format a step in the [`DerivationChain`] as a human-readable error message.
fn format_step(step: &DerivationStep, range: Option<Ranges<Version>>) -> String {
if let Some(visual) =
range.filter(|range| *range != Ranges::empty() && *range != Ranges::full())
{
if let Some(extra) = &step.extra {
// Ex) `flask[dotenv]>=1.0.0` (v1.2.3)`
format!(
"`{}{}` (v{})",
format!("{}[{}]", step.name, extra).cyan(),
visual.cyan(),
step.version.cyan()
)
} else if let Some(group) = &step.group {
// Ex) `flask:dev>=1.0.0` (v1.2.3)`
format!(
"`{}{}` (v{})",
format!("{}:{}", step.name, group).cyan(),
visual.cyan(),
step.version.cyan()
)
} else {
// Ex) `flask>=1.0.0` (v1.2.3)`
format!(
"`{}{}` (v{})",
step.name.cyan(),
visual.cyan(),
step.version.cyan()
)
}
} else {
if let Some(extra) = &step.extra {
// Ex) `flask[dotenv]` (v1.2.3)`
format!(
"`{}` (v{})",
format!("{}[{}]", step.name, extra).cyan(),
step.version.cyan()
)
} else if let Some(group) = &step.group {
// Ex) `flask:dev` (v1.2.3)`
format!(
"`{}` (v{})",
format!("{}:{}", step.name, group).cyan(),
step.version.cyan()
)
} else {
// Ex) `flask` (v1.2.3)`
format!("`{}` (v{})", step.name.cyan(), step.version.cyan())
}
}
}

let mut message = if let Some(version) = version {
format!(
"`{}` (v{}) was included because",
name.cyan(),
version.cyan()
)
} else {
format!("`{}` was included because", name.cyan())
};
let mut range: Option<Ranges<Version>> = None;
for (i, step) in chain.iter().enumerate() {
if i > 0 {
message = format!("{message} {} which depends on", format_step(step, range));
} else {
message = format!("{message} {} depends on", format_step(step, range));
}
range = Some(SentinelRange::from(&step.range).strip());
}
if let Some(visual) =
range.filter(|range| *range != Ranges::empty() && *range != Ranges::full())
{
message = format!("{message} `{}{}`", name.cyan(), visual.cyan());
} else {
message = format!("{message} `{}`", name.cyan());
}
message
}
4 changes: 2 additions & 2 deletions crates/uv/tests/it/lock.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19940,7 +19940,7 @@ fn lock_derivation_chain_extra() -> Result<()> {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
SyntaxError: Missing parentheses in call to 'print'. Did you mean print(...)?

help: `wsgiref` (v0.1.2) was included because `project` (v0.1.0) depends on `wsgiref>=0.1`
help: `wsgiref` (v0.1.2) was included because `project[wsgi]` (v0.1.0) depends on `wsgiref>=0.1`
"###);

Ok(())
Expand Down Expand Up @@ -20000,7 +20000,7 @@ fn lock_derivation_chain_group() -> Result<()> {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
SyntaxError: Missing parentheses in call to 'print'. Did you mean print(...)?

help: `wsgiref` (v0.1.2) was included because `project` (v0.1.0) depends on `wsgiref`
help: `wsgiref` (v0.1.2) was included because `project:wsgi` (v0.1.0) depends on `wsgiref`
"###);

Ok(())
Expand Down
8 changes: 4 additions & 4 deletions crates/uv/tests/it/sync.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -792,7 +792,7 @@ fn sync_build_isolation_extra() -> Result<()> {
File "<string>", line 8, in <module>
ModuleNotFoundError: No module named 'hatchling'
help: `source-distribution` was included because `project` (v0.1.0) depends on `source-distribution`
help: `source-distribution` was included because `project[compile]` (v0.1.0) depends on `source-distribution`
"###);

// Running `uv sync` with `--all-extras` should also fail.
Expand All @@ -811,7 +811,7 @@ fn sync_build_isolation_extra() -> Result<()> {
File "<string>", line 8, in <module>
ModuleNotFoundError: No module named 'hatchling'
help: `source-distribution` was included because `project` (v0.1.0) depends on `source-distribution`
help: `source-distribution` was included because `project[compile]` (v0.1.0) depends on `source-distribution`
"###);

// Install the build dependencies.
Expand Down Expand Up @@ -4398,7 +4398,7 @@ fn sync_derivation_chain_extra() -> Result<()> {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
SyntaxError: Missing parentheses in call to 'print'. Did you mean print(...)?
help: `wsgiref` (v0.1.2) was included because `project` (v0.1.0) depends on `wsgiref`
help: `wsgiref` (v0.1.2) was included because `project[wsgi]` (v0.1.0) depends on `wsgiref`
"###);

Ok(())
Expand Down Expand Up @@ -4464,7 +4464,7 @@ fn sync_derivation_chain_group() -> Result<()> {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
SyntaxError: Missing parentheses in call to 'print'. Did you mean print(...)?
help: `wsgiref` (v0.1.2) was included because `project` (v0.1.0) depends on `wsgiref`
help: `wsgiref` (v0.1.2) was included because `project:wsgi` (v0.1.0) depends on `wsgiref`
"###);

Ok(())
Expand Down

0 comments on commit 5bff2ba

Please sign in to comment.