Simplify ranges in the derivation tree before reporting (#9897)

An internal refactor to apply simplifications at the tree-level instead
of in the report formatter.
This commit is contained in:
Zanie Blue 2024-12-16 19:42:05 -06:00 committed by GitHub
parent d257bea720
commit 5c3dafc1a5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 97 additions and 46 deletions

View file

@ -336,6 +336,8 @@ impl std::fmt::Display for NoSolutionError {
collapse_redundant_no_versions(&mut tree);
collapse_redundant_depends_on_no_versions(&mut tree);
simplify_derivation_tree_ranges(&mut tree, &self.available_versions);
if should_display_tree {
display_tree(&tree, "Resolver derivation tree after reduction");
}
@ -1001,3 +1003,83 @@ impl std::fmt::Display for NoSolutionHeader {
}
}
}
/// Given a [`DerivationTree`], simplify version ranges using the available versions for each
/// package.
fn simplify_derivation_tree_ranges(
tree: &mut DerivationTree<PubGrubPackage, Range<Version>, UnavailableReason>,
available_versions: &FxHashMap<PackageName, BTreeSet<Version>>,
) {
match tree {
DerivationTree::External(external) => match external {
External::FromDependencyOf(package1, versions1, package2, versions2) => {
if let Some(simplified) = simplify_range(versions1, package1, available_versions) {
*versions1 = simplified;
}
if let Some(simplified) = simplify_range(versions2, package2, available_versions) {
*versions2 = simplified;
}
}
External::NoVersions(package, versions) => {
if let Some(simplified) = simplify_range(versions, package, available_versions) {
*versions = simplified;
}
}
External::Custom(package, versions, _) => {
if let Some(simplified) = simplify_range(versions, package, available_versions) {
*versions = simplified;
}
}
External::NotRoot(..) => (),
},
DerivationTree::Derived(derived) => {
// Recursively simplify both sides of the tree
simplify_derivation_tree_ranges(Arc::make_mut(&mut derived.cause1), available_versions);
simplify_derivation_tree_ranges(Arc::make_mut(&mut derived.cause2), available_versions);
// Simplify the terms
derived.terms = std::mem::take(&mut derived.terms)
.into_iter()
.map(|(pkg, term)| {
let term = match term {
Term::Positive(versions) => Term::Positive(
simplify_range(&versions, &pkg, available_versions).unwrap_or(versions),
),
Term::Negative(versions) => Term::Negative(
simplify_range(&versions, &pkg, available_versions).unwrap_or(versions),
),
};
(pkg, term)
})
.collect();
}
}
}
/// Helper function to simplify a version range using available versions for a package.
///
/// If the range cannot be simplified, `None` is returned.
fn simplify_range(
range: &Range<Version>,
package: &PubGrubPackage,
available_versions: &FxHashMap<PackageName, BTreeSet<Version>>,
) -> Option<Range<Version>> {
// If there's not a package name or available versions, we can't simplify anything
let name = package.name()?;
let versions = available_versions.get(name)?;
// If this is a full range, there's nothing to simplify
if range == &Range::full() {
return None;
}
// If there's only one version available and it's in the range, return just that version
if let Some(version) = versions.iter().next() {
if versions.len() == 1 && range.contains(version) {
return Some(Range::singleton(version.clone()));
}
}
// Simplify the range, as implemented in PubGrub
Some(range.simplify(versions.iter()))
}

View file

@ -1,4 +1,3 @@
use std::borrow::Cow;
use std::cmp::Ordering;
use std::collections::{BTreeMap, BTreeSet};
use std::ops::Bound;
@ -74,9 +73,7 @@ impl ReportFormatter<PubGrubPackage, Range<Version>, UnavailableReason>
);
}
let set = self.simplify_set(set, package);
if set.as_ref() == &Range::full() {
if set == &Range::full() {
format!("there are no versions of {package}")
} else if set.as_singleton().is_some() {
format!("there is no version of {package}{set}")
@ -114,8 +111,6 @@ impl ReportFormatter<PubGrubPackage, Range<Version>, UnavailableReason>
} else {
match reason {
UnavailableReason::Package(reason) => {
// While there may be a term attached, this error applies to the entire
// package, so we show it for the entire package
format!(
"{}{}",
Padded::new("", &package, " "),
@ -123,8 +118,7 @@ impl ReportFormatter<PubGrubPackage, Range<Version>, UnavailableReason>
)
}
UnavailableReason::Version(reason) => {
let set = self.simplify_set(set, package);
let range = self.compatible_range(package, &set);
let range = self.compatible_range(package, set);
let reason = if range.plural() {
reason.plural_message()
} else {
@ -136,14 +130,11 @@ impl ReportFormatter<PubGrubPackage, Range<Version>, UnavailableReason>
}
}
External::FromDependencyOf(package, package_set, dependency, dependency_set) => {
let package_set = self.simplify_set(package_set, package);
let dependency_set = self.simplify_set(dependency_set, dependency);
if package.name_no_root() == dependency.name_no_root() {
if let Some(member) = self.format_workspace_member(package) {
return format!(
"{member} depends on itself at an incompatible version ({})",
PackageRange::dependency(dependency, &dependency_set, None)
PackageRange::dependency(dependency, dependency_set, None)
);
}
}
@ -151,13 +142,13 @@ impl ReportFormatter<PubGrubPackage, Range<Version>, UnavailableReason>
if let Some(root) = self.format_root_requires(package) {
return format!(
"{root} {}",
self.dependency_range(dependency, &dependency_set)
self.dependency_range(dependency, dependency_set)
);
}
format!(
"{}",
self.compatible_range(package, &package_set)
.depends_on(dependency, &dependency_set),
self.compatible_range(package, package_set)
.depends_on(dependency, dependency_set),
)
}
}
@ -178,18 +169,16 @@ impl ReportFormatter<PubGrubPackage, Range<Version>, UnavailableReason>
[(package, Term::Positive(range))]
if matches!(&**(*package), PubGrubPackageInner::Package { .. }) =>
{
let range = self.simplify_set(range, package);
if let Some(member) = self.format_workspace_member(package) {
format!("{member}'s requirements are unsatisfiable")
} else {
format!("{} cannot be used", self.compatible_range(package, &range))
format!("{} cannot be used", self.compatible_range(package, range))
}
}
[(package, Term::Negative(range))]
if matches!(&**(*package), PubGrubPackageInner::Package { .. }) =>
{
let range = self.simplify_set(range, package);
format!("{} must be used", self.compatible_range(package, &range))
format!("{} must be used", self.compatible_range(package, range))
}
[(p1, Term::Positive(r1)), (p2, Term::Negative(r2))] => self.format_external(
&External::FromDependencyOf((*p1).clone(), r1.clone(), (*p2).clone(), r2.clone()),
@ -478,11 +467,8 @@ impl PubGrubReportFormatter<'_> {
External::FromDependencyOf(package1, package_set1, dependency1, dependency_set1),
External::FromDependencyOf(package2, _, dependency2, dependency_set2),
) if package1 == package2 => {
let dependency_set1 = self.simplify_set(dependency_set1, dependency1);
let dependency1 = self.dependency_range(dependency1, &dependency_set1);
let dependency_set2 = self.simplify_set(dependency_set2, dependency2);
let dependency2 = self.dependency_range(dependency2, &dependency_set2);
let dependency1 = self.dependency_range(dependency1, dependency_set1);
let dependency2 = self.dependency_range(dependency2, dependency_set2);
if let Some(root) = self.format_root_requires(package1) {
return format!(
@ -491,13 +477,12 @@ impl PubGrubReportFormatter<'_> {
dependency2,
);
}
let package_set = self.simplify_set(package_set1, package1);
format!(
"{}",
self.compatible_range(package1, &package_set)
.depends_on(dependency1.package, &dependency_set1)
.and(dependency2.package, &dependency_set2),
self.compatible_range(package1, package_set1)
.depends_on(dependency1.package, dependency_set1)
.and(dependency2.package, dependency_set2),
)
}
(.., External::FromDependencyOf(package, _, dependency, _))
@ -525,22 +510,6 @@ impl PubGrubReportFormatter<'_> {
}
}
/// Simplify a [`Range`] of versions using the available versions for a package.
fn simplify_set<'a>(
&self,
set: &'a Range<Version>,
package: &PubGrubPackage,
) -> Cow<'a, Range<Version>> {
let Some(name) = package.name() else {
return Cow::Borrowed(set);
};
if set == &Range::full() {
Cow::Borrowed(set)
} else {
Cow::Owned(set.simplify(self.available_versions.get(name).into_iter().flatten()))
}
}
/// Generate the [`PubGrubHints`] for a derivation tree.
///
/// The [`PubGrubHints`] help users resolve errors by providing additional context or modifying
@ -685,7 +654,7 @@ impl PubGrubReportFormatter<'_> {
source: self.python_requirement.source(),
requires_python: self.python_requirement.target().clone(),
package: package.clone(),
package_set: self.simplify_set(package_set, package).into_owned(),
package_set: package_set.clone(),
package_requires_python: dependency_set.clone(),
});
}
@ -878,7 +847,7 @@ impl PubGrubReportFormatter<'_> {
if selector.prerelease_strategy().allows(name, env) != AllowPrerelease::Yes {
hints.insert(PubGrubHint::PrereleaseRequested {
package: package.clone(),
range: self.simplify_set(set, package).into_owned(),
range: set.clone(),
});
}
} else if let Some(version) = package