Use a custom pubgrub report formatter (#521)

Uses https://github.com/zanieb/pubgrub/pull/10 to drastically simplify
our reporter implementation. This will allow us to make use of upstream
improvements to the reporter e.g.
https://github.com/zanieb/pubgrub/pull/8 without multiple duplicative
pull requests.
This commit is contained in:
Zanie Blue 2023-12-01 13:36:12 -06:00 committed by GitHub
parent efcc4f1409
commit 2a8544df9e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 74 additions and 419 deletions

View file

@ -1,7 +1,7 @@
use std::fmt::Formatter;
use pubgrub::range::Range;
use pubgrub::report::Reporter;
use pubgrub::report::{DefaultStringReporter, Reporter};
use thiserror::Error;
use url::Url;
@ -11,7 +11,7 @@ use puffin_distribution::DistributionDatabaseError;
use puffin_normalize::PackageName;
use crate::pubgrub::{PubGrubPackage, PubGrubVersion};
use crate::ResolutionFailureReporter;
use crate::PubGrubReportFormatter;
#[derive(Error, Debug)]
pub enum ResolveError {
@ -78,7 +78,8 @@ impl std::error::Error for RichPubGrubError {}
impl std::fmt::Display for RichPubGrubError {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
if let pubgrub::error::PubGrubError::NoSolution(derivation_tree) = &self.source {
let report = ResolutionFailureReporter::report(derivation_tree);
let formatter = PubGrubReportFormatter;
let report = DefaultStringReporter::report_with_formatter(derivation_tree, &formatter);
write!(f, "{report}")
} else {
write!(f, "{}", self.source)

View file

@ -2,7 +2,7 @@ pub use error::ResolveError;
pub use finder::{DistFinder, Reporter as FinderReporter};
pub use manifest::Manifest;
pub use prerelease_mode::PreReleaseMode;
pub use pubgrub::ResolutionFailureReporter;
pub use pubgrub::PubGrubReportFormatter;
pub use resolution::Graph;
pub use resolution_mode::ResolutionMode;
pub use resolution_options::ResolutionOptions;

View file

@ -1,6 +1,6 @@
pub(crate) use crate::pubgrub::package::PubGrubPackage;
pub(crate) use crate::pubgrub::priority::{PubGrubPriorities, PubGrubPriority};
pub use crate::pubgrub::report::ResolutionFailureReporter;
pub use crate::pubgrub::report::PubGrubReportFormatter;
pub(crate) use crate::pubgrub::version::{PubGrubVersion, MIN_VERSION};

View file

@ -1,271 +1,83 @@
use std::fmt;
use pubgrub::range::Range;
use pubgrub::report::{DerivationTree, Derived, External, Reporter};
use pubgrub::report::{External, ReportFormatter};
use pubgrub::term::Term;
use pubgrub::type_aliases::Map;
use super::{PubGrubPackage, PubGrubVersion};
/// Puffin derivative of [`pubgrub::report::DefaultStringReporter`] for customized display
/// of package resolution errors.
pub struct ResolutionFailureReporter {
/// Number of explanations already with a line reference.
ref_count: usize,
/// Shared nodes that have already been marked with a line reference.
/// The incompatibility ids are the keys, and the line references are the values.
shared_with_ref: Map<usize, usize>,
/// Accumulated lines of the report already generated.
lines: Vec<String>,
}
#[derive(Debug, Default)]
pub struct PubGrubReportFormatter;
impl ResolutionFailureReporter {
/// Initialize the reporter.
fn new() -> Self {
Self {
ref_count: 0,
shared_with_ref: Map::default(),
lines: Vec::new(),
}
}
impl ReportFormatter<PubGrubPackage, Range<PubGrubVersion>> for PubGrubReportFormatter {
type Output = String;
fn build_recursive(&mut self, derived: &Derived<PubGrubPackage, Range<PubGrubVersion>>) {
self.build_recursive_helper(derived);
if let Some(id) = derived.shared_id {
if self.shared_with_ref.get(&id).is_none() {
self.add_line_ref();
self.shared_with_ref.insert(id, self.ref_count);
fn format_external(
&self,
external: &External<PubGrubPackage, Range<PubGrubVersion>>,
) -> Self::Output {
match external {
External::NotRoot(package, version) => {
format!("we are solving dependencies of {package} {version}")
}
};
}
fn build_recursive_helper(&mut self, current: &Derived<PubGrubPackage, Range<PubGrubVersion>>) {
match (&*current.cause1, &*current.cause2) {
(DerivationTree::External(external1), DerivationTree::External(external2)) => {
// Simplest case, we just combine two external incompatibilities.
self.lines.push(Self::explain_both_external(
external1,
external2,
&current.terms,
));
External::NoVersions(package, set) => {
if set == &Range::full() {
format!("there is no available version for {package}")
} else {
format!("there is no version of {package} available matching {set}")
}
}
(DerivationTree::Derived(derived), DerivationTree::External(external)) => {
// One cause is derived, so we explain this first
// then we add the one-line external part
// and finally conclude with the current incompatibility.
self.report_one_each(derived, external, &current.terms);
External::UnavailableDependencies(package, set) => {
if set == &Range::full() {
format!("dependencies of {package} are unavailable")
} else {
format!("dependencies of {package} at version {set} are unavailable")
}
}
(DerivationTree::External(external), DerivationTree::Derived(derived)) => {
self.report_one_each(derived, external, &current.terms);
}
(DerivationTree::Derived(derived1), DerivationTree::Derived(derived2)) => {
// This is the most complex case since both causes are also derived.
match (
self.line_ref_of(derived1.shared_id),
self.line_ref_of(derived2.shared_id),
) {
// If both causes already have been referenced (shared_id),
// the explanation simply uses those references.
(Some(ref1), Some(ref2)) => self.lines.push(Self::explain_both_ref(
ref1,
derived1,
ref2,
derived2,
&current.terms,
)),
// Otherwise, if one only has a line number reference,
// we recursively call the one without reference and then
// add the one with reference to conclude.
(Some(ref1), None) => {
self.build_recursive(derived2);
self.lines
.push(Self::and_explain_ref(ref1, derived1, &current.terms));
}
(None, Some(ref2)) => {
self.build_recursive(derived1);
self.lines
.push(Self::and_explain_ref(ref2, derived2, &current.terms));
}
// Finally, if no line reference exists yet,
// we call recursively the first one and then,
// - if this was a shared node, it will get a line ref
// and we can simply recall this with the current node.
// - otherwise, we add a line reference to it,
// recursively call on the second node,
// and finally conclude.
(None, None) => {
self.build_recursive(derived1);
if derived1.shared_id.is_some() {
self.lines.push(String::new());
self.build_recursive(current);
External::UnusableDependencies(package, set, reason) => {
if let Some(reason) = reason {
if matches!(package, PubGrubPackage::Root(_)) {
format!("{package} dependencies are unusable: {reason}")
} else {
if set == &Range::full() {
format!("dependencies of {package} are unusable: {reason}")
} else {
self.add_line_ref();
let ref1 = self.ref_count;
self.lines.push(String::new());
self.build_recursive(derived2);
self.lines
.push(Self::and_explain_ref(ref1, derived1, &current.terms));
format!("dependencies of {package}{set} are unusable: {reason}",)
}
}
} else {
if set == &Range::full() {
format!("dependencies of {package} are unusable")
} else {
format!("dependencies of {package}{set} are unusable")
}
}
}
External::FromDependencyOf(package, package_set, dependency, dependency_set) => {
if package_set == &Range::full() && dependency_set == &Range::full() {
format!("{package} depends on {dependency}")
} else if package_set == &Range::full() {
format!("{package} depends on {dependency}{dependency_set}")
} else if dependency_set == &Range::full() {
if matches!(package, PubGrubPackage::Root(_)) {
// Exclude the dummy version for root packages
format!("{package} depends on {dependency}")
} else {
format!("{package}{package_set} depends on {dependency}")
}
} else {
if matches!(package, PubGrubPackage::Root(_)) {
// Exclude the dummy version for root packages
format!("{package} depends on {dependency}{dependency_set}")
} else {
format!("{package}{package_set} depends on {dependency}{dependency_set}")
}
}
}
}
}
/// Report a derived and an external incompatibility.
///
/// The result will depend on the fact that the derived incompatibility
/// has already been explained or not.
fn report_one_each(
&mut self,
derived: &Derived<PubGrubPackage, Range<PubGrubVersion>>,
external: &External<PubGrubPackage, Range<PubGrubVersion>>,
current_terms: &Map<PubGrubPackage, Term<Range<PubGrubVersion>>>,
) {
match self.line_ref_of(derived.shared_id) {
Some(ref_id) => self.lines.push(Self::explain_ref_and_external(
ref_id,
derived,
external,
current_terms,
)),
None => self.report_recurse_one_each(derived, external, current_terms),
}
}
/// Report one derived (without a line ref yet) and one external.
fn report_recurse_one_each(
&mut self,
derived: &Derived<PubGrubPackage, Range<PubGrubVersion>>,
external: &External<PubGrubPackage, Range<PubGrubVersion>>,
current_terms: &Map<PubGrubPackage, Term<Range<PubGrubVersion>>>,
) {
match (&*derived.cause1, &*derived.cause2) {
// If the derived cause has itself one external prior cause,
// we can chain the external explanations.
(DerivationTree::Derived(prior_derived), DerivationTree::External(prior_external)) => {
self.build_recursive(prior_derived);
self.lines.push(Self::and_explain_prior_and_external(
prior_external,
external,
current_terms,
));
}
// If the derived cause has itself one external prior cause,
// we can chain the external explanations.
(DerivationTree::External(prior_external), DerivationTree::Derived(prior_derived)) => {
self.build_recursive(prior_derived);
self.lines.push(Self::and_explain_prior_and_external(
prior_external,
external,
current_terms,
));
}
_ => {
self.build_recursive(derived);
self.lines
.push(Self::and_explain_external(external, current_terms));
}
}
}
// String explanations #####################################################
/// Simplest case, we just combine two external incompatibilities.
fn explain_both_external(
external1: &External<PubGrubPackage, Range<PubGrubVersion>>,
external2: &External<PubGrubPackage, Range<PubGrubVersion>>,
current_terms: &Map<PubGrubPackage, Term<Range<PubGrubVersion>>>,
) -> String {
// TODO: order should be chosen to make it more logical.
format!(
"Because {} and {}, {}.",
PuffinExternal::from_pubgrub(external1.clone()),
PuffinExternal::from_pubgrub(external2.clone()),
Self::string_terms(current_terms)
)
}
/// Both causes have already been explained so we use their refs.
fn explain_both_ref(
ref_id1: usize,
derived1: &Derived<PubGrubPackage, Range<PubGrubVersion>>,
ref_id2: usize,
derived2: &Derived<PubGrubPackage, Range<PubGrubVersion>>,
current_terms: &Map<PubGrubPackage, Term<Range<PubGrubVersion>>>,
) -> String {
// TODO: order should be chosen to make it more logical.
format!(
"Because {} ({}) and {} ({}), {}.",
Self::string_terms(&derived1.terms),
ref_id1,
Self::string_terms(&derived2.terms),
ref_id2,
Self::string_terms(current_terms)
)
}
/// One cause is derived (already explained so one-line),
/// the other is a one-line external cause,
/// and finally we conclude with the current incompatibility.
fn explain_ref_and_external(
ref_id: usize,
derived: &Derived<PubGrubPackage, Range<PubGrubVersion>>,
external: &External<PubGrubPackage, Range<PubGrubVersion>>,
current_terms: &Map<PubGrubPackage, Term<Range<PubGrubVersion>>>,
) -> String {
// TODO: order should be chosen to make it more logical.
format!(
"Because {} ({}) and {}, {}.",
Self::string_terms(&derived.terms),
ref_id,
PuffinExternal::from_pubgrub(external.clone()),
Self::string_terms(current_terms)
)
}
/// Add an external cause to the chain of explanations.
fn and_explain_external(
external: &External<PubGrubPackage, Range<PubGrubVersion>>,
current_terms: &Map<PubGrubPackage, Term<Range<PubGrubVersion>>>,
) -> String {
format!(
"And because {}, {}.",
PuffinExternal::from_pubgrub(external.clone()),
Self::string_terms(current_terms)
)
}
/// Add an already explained incompat to the chain of explanations.
fn and_explain_ref(
ref_id: usize,
derived: &Derived<PubGrubPackage, Range<PubGrubVersion>>,
current_terms: &Map<PubGrubPackage, Term<Range<PubGrubVersion>>>,
) -> String {
format!(
"And because {} ({}), {}.",
Self::string_terms(&derived.terms),
ref_id,
Self::string_terms(current_terms)
)
}
/// Add an already explained incompat to the chain of explanations.
fn and_explain_prior_and_external(
prior_external: &External<PubGrubPackage, Range<PubGrubVersion>>,
external: &External<PubGrubPackage, Range<PubGrubVersion>>,
current_terms: &Map<PubGrubPackage, Term<Range<PubGrubVersion>>>,
) -> String {
format!(
"And because {} and {}, {}.",
prior_external,
external,
Self::string_terms(current_terms)
)
}
/// Try to print terms of an incompatibility in a human-readable way.
pub fn string_terms(terms: &Map<PubGrubPackage, Term<Range<PubGrubVersion>>>) -> String {
fn format_terms(&self, terms: &Map<PubGrubPackage, Term<Range<PubGrubVersion>>>) -> String {
let terms_vec: Vec<_> = terms.iter().collect();
match terms_vec.as_slice() {
[] | [(PubGrubPackage::Root(_), _)] => "version solving failed".into(),
@ -275,174 +87,16 @@ impl ResolutionFailureReporter {
[(package @ PubGrubPackage::Package(..), Term::Negative(range))] => {
format!("{package}{range} is mandatory")
}
[(p1, Term::Positive(r1)), (p2, Term::Negative(r2))] => {
PuffinExternal::FromDependencyOf(
(*p1).clone(),
r1.clone(),
(*p2).clone(),
r2.clone(),
)
.to_string()
}
[(p1, Term::Negative(r1)), (p2, Term::Positive(r2))] => {
PuffinExternal::FromDependencyOf(
(*p2).clone(),
r2.clone(),
(*p1).clone(),
r1.clone(),
)
.to_string()
}
[(p1, Term::Positive(r1)), (p2, Term::Negative(r2))] => self.format_external(
&External::FromDependencyOf((*p1).clone(), r1.clone(), (*p2).clone(), r2.clone()),
),
[(p1, Term::Negative(r1)), (p2, Term::Positive(r2))] => self.format_external(
&External::FromDependencyOf((*p2).clone(), r2.clone(), (*p1).clone(), r1.clone()),
),
slice => {
let str_terms: Vec<_> = slice.iter().map(|(p, t)| format!("{p} {t}")).collect();
str_terms.join(", ") + " are incompatible"
}
}
}
// Helper functions ########################################################
fn add_line_ref(&mut self) {
let new_count = self.ref_count + 1;
self.ref_count = new_count;
if let Some(line) = self.lines.last_mut() {
*line = format!("{line} ({new_count})");
}
}
fn line_ref_of(&self, shared_id: Option<usize>) -> Option<usize> {
shared_id.and_then(|id| self.shared_with_ref.get(&id).copied())
}
}
impl Reporter<PubGrubPackage, Range<PubGrubVersion>> for ResolutionFailureReporter {
type Output = String;
fn report(
derivation_tree: &DerivationTree<PubGrubPackage, Range<PubGrubVersion>>,
) -> Self::Output {
match derivation_tree {
DerivationTree::External(external) => {
PuffinExternal::from_pubgrub(external.clone()).to_string()
}
DerivationTree::Derived(derived) => {
let mut reporter = Self::new();
reporter.build_recursive(derived);
reporter.lines.join("\n")
}
}
}
}
/// Puffin derivative of [`pubgrub::report::External`] for customized display
/// for Puffin internal [`PubGrubPackage`].
#[allow(clippy::large_enum_variant)]
#[derive(Debug, Clone)]
enum PuffinExternal {
/// Initial incompatibility aiming at picking the root package for the first decision.
NotRoot(PubGrubPackage, PubGrubVersion),
/// There are no versions in the given set for this package.
NoVersions(PubGrubPackage, Range<PubGrubVersion>),
/// Dependencies of the package are unavailable for versions in that set.
UnavailableDependencies(PubGrubPackage, Range<PubGrubVersion>),
/// Dependencies of the package are unusable for versions in that set.
UnusableDependencies(PubGrubPackage, Range<PubGrubVersion>, Option<String>),
/// Incompatibility coming from the dependencies of a given package.
FromDependencyOf(
PubGrubPackage,
Range<PubGrubVersion>,
PubGrubPackage,
Range<PubGrubVersion>,
),
}
impl PuffinExternal {
fn from_pubgrub(external: External<PubGrubPackage, Range<PubGrubVersion>>) -> Self {
match external {
External::NotRoot(p, v) => PuffinExternal::NotRoot(p, v),
External::NoVersions(p, vs) => PuffinExternal::NoVersions(p, vs),
External::UnavailableDependencies(p, vs) => {
PuffinExternal::UnavailableDependencies(p, vs)
}
External::UnusableDependencies(p, vs, reason) => {
PuffinExternal::UnusableDependencies(p, vs, reason)
}
External::FromDependencyOf(p, vs, p_dep, vs_dep) => {
PuffinExternal::FromDependencyOf(p, vs, p_dep, vs_dep)
}
}
}
}
impl fmt::Display for PuffinExternal {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::NotRoot(package, version) => {
write!(f, "we are solving dependencies of {package} {version}")
}
Self::NoVersions(package, set) => {
if set == &Range::full() {
write!(f, "there is no available version for {package}")
} else {
write!(
f,
"there is no version of {package} available matching {set}"
)
}
}
Self::UnavailableDependencies(package, set) => {
if set == &Range::full() {
write!(f, "dependencies of {package} are unavailable")
} else {
write!(
f,
"dependencies of {package} at version {set} are unavailable"
)
}
}
Self::UnusableDependencies(package, set, reason) => {
if let Some(reason) = reason {
if matches!(package, PubGrubPackage::Root(_)) {
write!(f, "{package} dependencies are unusable: {reason}")
} else {
if set == &Range::full() {
write!(f, "dependencies of {package} are unusable: {reason}")
} else {
write!(f, "dependencies of {package}{set} are unusable: {reason}",)
}
}
} else {
if set == &Range::full() {
write!(f, "dependencies of {package} are unusable")
} else {
write!(f, "dependencies of {package}{set} are unusable")
}
}
}
Self::FromDependencyOf(package, package_set, dependency, dependency_set) => {
if package_set == &Range::full() && dependency_set == &Range::full() {
write!(f, "{package} depends on {dependency}")
} else if package_set == &Range::full() {
write!(f, "{package} depends on {dependency}{dependency_set}")
} else if dependency_set == &Range::full() {
if matches!(package, PubGrubPackage::Root(_)) {
// Exclude the dummy version for root packages
write!(f, "{package} depends on {dependency}")
} else {
write!(f, "{package}{package_set} depends on {dependency}")
}
} else {
if matches!(package, PubGrubPackage::Root(_)) {
// Exclude the dummy version for root packages
write!(f, "{package} depends on {dependency}{dependency_set}")
} else {
write!(
f,
"{package}{package_set} depends on {dependency}{dependency_set}"
)
}
}
}
}
}
}