Skip to content

Commit

Permalink
Generic reason for custom incompatibility (#208)
Browse files Browse the repository at this point in the history 
* feat: Generic reason for custom incompatibility

Co-authored-by: Zanie Blue <[email protected]>

* Pacify clippy

* Document the intended usage of `DependencyProvider::M`

* Remove metadata merging check

* Improve docs

---------

Co-authored-by: Zanie Blue <[email protected]>
  • Loading branch information
@konstin @zanieb
konstin and zanieb authored May 1, 2024
1 parent 34bf75c commit d5ed801
Show file tree
Hide file tree
Showing 12 changed files with 287 additions and 161 deletions.
9 changes: 5 additions & 4 deletions examples/caching_dependency_provider.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,16 @@ impl<DP: DependencyProvider> CachingDependencyProvider<DP> {
}
}

impl<DP: DependencyProvider> DependencyProvider for CachingDependencyProvider<DP> {
impl<DP: DependencyProvider<M = String>> DependencyProvider for CachingDependencyProvider<DP> {
// Caches dependencies if they were already queried
fn get_dependencies(
&self,
package: &DP::P,
version: &DP::V,
) -> Result<Dependencies<DP::P, DP::VS>, DP::Err> {
) -> Result<Dependencies<DP::P, DP::VS, DP::M>, DP::Err> {
let mut cache = self.cached_dependencies.borrow_mut();
match cache.get_dependencies(package, version) {
Ok(Dependencies::Unknown) => {
Ok(Dependencies::Unknown(_)) => {
let dependencies = self.remote_dependencies.get_dependencies(package, version);
match dependencies {
Ok(Dependencies::Known(dependencies)) => {
Expand All @@ -43,7 +43,7 @@ impl<DP: DependencyProvider> DependencyProvider for CachingDependencyProvider<DP
);
Ok(Dependencies::Known(dependencies))
}
Ok(Dependencies::Unknown) => Ok(Dependencies::Unknown),
Ok(Dependencies::Unknown(reason)) => Ok(Dependencies::Unknown(reason)),
error @ Err(_) => error,
}
}
Expand All @@ -67,6 +67,7 @@ impl<DP: DependencyProvider> DependencyProvider for CachingDependencyProvider<DP
type P = DP::P;
type V = DP::V;
type VS = DP::VS;
type M = DP::M;
}

fn main() {
Expand Down
19 changes: 12 additions & 7 deletions examples/unsat_root_message_no_version.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ impl Display for Package {
#[derive(Debug, Default)]
struct CustomReportFormatter;

impl ReportFormatter<Package, Range<SemanticVersion>> for CustomReportFormatter {
impl ReportFormatter<Package, Range<SemanticVersion>, String> for CustomReportFormatter {
type Output = String;

fn format_terms(&self, terms: &Map<Package, Term<Range<SemanticVersion>>>) -> String {
Expand All @@ -49,10 +49,12 @@ impl ReportFormatter<Package, Range<SemanticVersion>> for CustomReportFormatter
format!("{package} {range} is mandatory")
}
[(p1, Term::Positive(r1)), (p2, Term::Negative(r2))] => {
External::FromDependencyOf(p1, r1.clone(), p2, r2.clone()).to_string()
External::<_, _, String>::FromDependencyOf(p1, r1.clone(), p2, r2.clone())
.to_string()
}
[(p1, Term::Negative(r1)), (p2, Term::Positive(r2))] => {
External::FromDependencyOf(p2, r2.clone(), p1, r1.clone()).to_string()
External::<_, _, String>::FromDependencyOf(p2, r2.clone(), p1, r1.clone())
.to_string()
}
slice => {
let str_terms: Vec<_> = slice.iter().map(|(p, t)| format!("{p} {t}")).collect();
Expand All @@ -61,7 +63,10 @@ impl ReportFormatter<Package, Range<SemanticVersion>> for CustomReportFormatter
}
}

fn format_external(&self, external: &External<Package, Range<SemanticVersion>>) -> String {
fn format_external(
&self,
external: &External<Package, Range<SemanticVersion>, String>,
) -> String {
match external {
External::NotRoot(package, version) => {
format!("we are solving dependencies of {package} {version}")
Expand All @@ -73,11 +78,11 @@ impl ReportFormatter<Package, Range<SemanticVersion>> for CustomReportFormatter
format!("there is no version of {package} in {set}")
}
}
External::UnavailableDependencies(package, set) => {
External::Custom(package, set, reason) => {
if set == &Range::full() {
format!("dependencies of {package} are unavailable")
format!("dependencies of {package} are unavailable because {reason}")
} else {
format!("dependencies of {package} at version {set} are unavailable")
format!("dependencies of {package} at version {set} are unavailable because {reason}")
}
}
External::FromDependencyOf(package, package_set, dependency, dependency_set) => {
Expand Down
2 changes: 1 addition & 1 deletion src/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ where
{
/// There is no solution for this set of dependencies.
#[error("No solution")]
NoSolution(DerivationTree<DP::P, DP::VS>),
NoSolution(DerivationTree<DP::P, DP::VS, DP::M>),

/// Error arising when the implementer of
/// [DependencyProvider]
Expand Down
9 changes: 6 additions & 3 deletions src/internal/core.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ pub struct State<DP: DependencyProvider> {
pub partial_solution: PartialSolution<DP>,

/// The store is the reference storage for all incompatibilities.
pub incompatibility_store: Arena<Incompatibility<DP::P, DP::VS>>,
pub incompatibility_store: Arena<Incompatibility<DP::P, DP::VS, DP::M>>,

/// This is a stack of work to be done in `unit_propagation`.
/// It can definitely be a local variable to that method, but
Expand Down Expand Up @@ -74,7 +74,7 @@ impl<DP: DependencyProvider> State<DP> {
}

/// Add an incompatibility to the state.
pub fn add_incompatibility(&mut self, incompat: Incompatibility<DP::P, DP::VS>) {
pub fn add_incompatibility(&mut self, incompat: Incompatibility<DP::P, DP::VS, DP::M>) {
let id = self.incompatibility_store.alloc(incompat);
self.merge_incompatibility(id);
}
Expand Down Expand Up @@ -297,7 +297,10 @@ impl<DP: DependencyProvider> State<DP> {

// Error reporting #########################################################

fn build_derivation_tree(&self, incompat: IncompDpId<DP>) -> DerivationTree<DP::P, DP::VS> {
fn build_derivation_tree(
&self,
incompat: IncompDpId<DP>,
) -> DerivationTree<DP::P, DP::VS, DP::M> {
let mut all_ids: Set<IncompDpId<DP>> = Set::default();
let mut shared_ids = Set::default();
let mut stack = vec![incompat];
Expand Down
105 changes: 74 additions & 31 deletions src/internal/incompatibility.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
//! An incompatibility is a set of terms for different packages
//! that should never be satisfied all together.
use std::fmt;
use std::fmt::{self, Debug, Display};
use std::sync::Arc;

use crate::internal::arena::{Arena, Id};
Expand Down Expand Up @@ -32,26 +32,44 @@ use crate::version_set::VersionSet;
/// during conflict resolution. More about all this in
/// [PubGrub documentation](https://github.com/dart-lang/pub/blob/master/doc/solver.md#incompatibility).
#[derive(Debug, Clone)]
pub struct Incompatibility<P: Package, VS: VersionSet> {
pub struct Incompatibility<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> {
package_terms: SmallMap<P, Term<VS>>,
kind: Kind<P, VS>,
kind: Kind<P, VS, M>,
}

/// Type alias of unique identifiers for incompatibilities.
pub type IncompId<P, VS> = Id<Incompatibility<P, VS>>;
pub type IncompId<P, VS, M> = Id<Incompatibility<P, VS, M>>;

#[derive(Debug, Clone)]
enum Kind<P: Package, VS: VersionSet> {
enum Kind<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> {
/// Initial incompatibility aiming at picking the root package for the first decision.
///
/// This incompatibility drives the resolution, it requires that we pick the (virtual) root
/// packages.
NotRoot(P, VS::V),
/// There are no versions in the given range for this package.
///
/// This incompatibility is used when we tried all versions in a range and no version
/// worked, so we have to backtrack
NoVersions(P, VS),
/// Dependencies of the package are unavailable for versions in that range.
UnavailableDependencies(P, VS),
/// Incompatibility coming from the dependencies of a given package.
///
/// If a@1 depends on b>=1,<2, we create an incompatibility with terms `{a 1, b <1,>=2}` with
/// kind `FromDependencyOf(a, 1, b, >=1,<2)`.
///
/// We can merge multiple dependents with the same version. For example, if a@1 depends on b and
/// a@2 depends on b, we can say instead a@1||2 depends on b.
FromDependencyOf(P, VS, P, VS),
/// Derived from two causes. Stores cause ids.
DerivedFrom(IncompId<P, VS>, IncompId<P, VS>),
///
/// For example, if a -> b and b -> c, we can derive a -> c.
DerivedFrom(IncompId<P, VS, M>, IncompId<P, VS, M>),
/// The package is unavailable for reasons outside pubgrub.
///
/// Examples:
/// * The version would require building the package, but builds are disabled.
/// * The package is not available in the cache, but internet access has been disabled.
Custom(P, VS, M),
}

/// A Relation describes how a set of terms can be compared to an incompatibility.
Expand All @@ -71,7 +89,7 @@ pub enum Relation<P: Package> {
Inconclusive,
}

impl<P: Package, VS: VersionSet> Incompatibility<P, VS> {
impl<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> Incompatibility<P, VS, M> {
/// Create the initial "not Root" incompatibility.
pub fn not_root(package: P, version: VS::V) -> Self {
Self {
Expand All @@ -83,8 +101,7 @@ impl<P: Package, VS: VersionSet> Incompatibility<P, VS> {
}
}

/// Create an incompatibility to remember
/// that a given set does not contain any version.
/// Create an incompatibility to remember that a given set does not contain any version.
pub fn no_versions(package: P, term: Term<VS>) -> Self {
let set = match &term {
Term::Positive(r) => r.clone(),
Expand All @@ -96,14 +113,26 @@ impl<P: Package, VS: VersionSet> Incompatibility<P, VS> {
}
}

/// Create an incompatibility to remember
/// that a package version is not selectable
/// because its list of dependencies is unavailable.
pub fn unavailable_dependencies(package: P, version: VS::V) -> Self {
/// Create an incompatibility for a reason outside pubgrub.
#[allow(dead_code)] // Used by uv
pub fn custom_term(package: P, term: Term<VS>, metadata: M) -> Self {
let set = match &term {
Term::Positive(r) => r.clone(),
Term::Negative(_) => panic!("No version should have a positive term"),
};
Self {
package_terms: SmallMap::One([(package.clone(), term)]),
kind: Kind::Custom(package, set, metadata),
}
}

/// Create an incompatibility for a reason outside pubgrub.
pub fn custom_version(package: P, version: VS::V, metadata: M) -> Self {
let set = VS::singleton(version);
let term = Term::Positive(set.clone());
Self {
package_terms: SmallMap::One([(package.clone(), Term::Positive(set.clone()))]),
kind: Kind::UnavailableDependencies(package, set),
package_terms: SmallMap::One([(package.clone(), term)]),
kind: Kind::Custom(package, set, metadata),
}
}

Expand Down Expand Up @@ -135,7 +164,7 @@ impl<P: Package, VS: VersionSet> Incompatibility<P, VS> {
/// When multiple versions of a package depend on the same range of another package,
/// we can merge the two into a single incompatibility.
/// For example, if a@1 depends on b and a@2 depends on b, we can say instead
/// a@1 and a@b depend on b.
/// a@1||2 depends on b.
///
/// It is a special case of prior cause computation where the unified package
/// is the common dependant in the two incompatibilities expressing dependencies.
Expand Down Expand Up @@ -231,8 +260,8 @@ impl<P: Package, VS: VersionSet> Incompatibility<P, VS> {
self_id: Id<Self>,
shared_ids: &Set<Id<Self>>,
store: &Arena<Self>,
precomputed: &Map<Id<Self>, Arc<DerivationTree<P, VS>>>,
) -> DerivationTree<P, VS> {
precomputed: &Map<Id<Self>, Arc<DerivationTree<P, VS, M>>>,
) -> DerivationTree<P, VS, M> {
match store[self_id].kind.clone() {
Kind::DerivedFrom(id1, id2) => {
let derived = Derived {
Expand All @@ -253,19 +282,28 @@ impl<P: Package, VS: VersionSet> Incompatibility<P, VS> {
DerivationTree::External(External::NotRoot(package, version))
}
Kind::NoVersions(package, set) => {
DerivationTree::External(External::NoVersions(package, set))
DerivationTree::External(External::NoVersions(package.clone(), set.clone()))
}
Kind::UnavailableDependencies(package, set) => {
DerivationTree::External(External::UnavailableDependencies(package, set))
Kind::FromDependencyOf(package, set, dep_package, dep_set) => {
DerivationTree::External(External::FromDependencyOf(
package.clone(),
set.clone(),
dep_package.clone(),
dep_set.clone(),
))
}
Kind::FromDependencyOf(package, set, dep_package, dep_set) => DerivationTree::External(
External::FromDependencyOf(package, set, dep_package, dep_set),
),
Kind::Custom(package, set, metadata) => DerivationTree::External(External::Custom(
package.clone(),
set.clone(),
metadata.clone(),
)),
}
}
}

impl<'a, P: Package, VS: VersionSet + 'a> Incompatibility<P, VS> {
impl<'a, P: Package, VS: VersionSet + 'a, M: Eq + Clone + Debug + Display + 'a>
Incompatibility<P, VS, M>
{
/// CF definition of Relation enum.
pub fn relation(&self, terms: impl Fn(&P) -> Option<&'a Term<VS>>) -> Relation<P> {
let mut relation = Relation::Satisfied;
Expand Down Expand Up @@ -293,12 +331,17 @@ impl<'a, P: Package, VS: VersionSet + 'a> Incompatibility<P, VS> {
}
}

impl<P: Package, VS: VersionSet> fmt::Display for Incompatibility<P, VS> {
impl<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> fmt::Display
for Incompatibility<P, VS, M>
{
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(
f,
"{}",
DefaultStringReportFormatter.format_terms(&self.package_terms.as_map())
ReportFormatter::<P, VS, M>::format_terms(
&DefaultStringReportFormatter,
&self.package_terms.as_map()
)
)
}
}
Expand Down Expand Up @@ -326,12 +369,12 @@ pub mod tests {
let mut store = Arena::new();
let i1 = store.alloc(Incompatibility {
package_terms: SmallMap::Two([("p1", t1.clone()), ("p2", t2.negate())]),
kind: Kind::UnavailableDependencies("0", Range::full())
kind: Kind::<_, _, String>::FromDependencyOf("p1", Range::full(), "p2", Range::full())
});

let i2 = store.alloc(Incompatibility {
package_terms: SmallMap::Two([("p2", t2), ("p3", t3.clone())]),
kind: Kind::UnavailableDependencies("0", Range::full())
kind: Kind::<_, _, String>::FromDependencyOf("p2", Range::full(), "p3", Range::full())
});

let mut i3 = Map::default();
Expand Down
Loading

0 comments on commit d5ed801

Please sign in to comment.