Skip to content

Commit 8838dca

Browse files
zaniebkonstin
authored andcommitted
feat: Generic reason for custom incompatibility
Co-authored-by: Zanie Blue <contact@zanie.dev>
1 parent 97951bb commit 8838dca

12 files changed

+283
-161
lines changed

examples/caching_dependency_provider.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,16 @@ impl<DP: DependencyProvider> CachingDependencyProvider<DP> {
2323
}
2424
}
2525

26-
impl<DP: DependencyProvider> DependencyProvider for CachingDependencyProvider<DP> {
26+
impl<DP: DependencyProvider<M = String>> DependencyProvider for CachingDependencyProvider<DP> {
2727
// Caches dependencies if they were already queried
2828
fn get_dependencies(
2929
&self,
3030
package: &DP::P,
3131
version: &DP::V,
32-
) -> Result<Dependencies<DP::P, DP::VS>, DP::Err> {
32+
) -> Result<Dependencies<DP::P, DP::VS, DP::M>, DP::Err> {
3333
let mut cache = self.cached_dependencies.borrow_mut();
3434
match cache.get_dependencies(package, version) {
35-
Ok(Dependencies::Unknown) => {
35+
Ok(Dependencies::Unknown(reason)) => {
3636
let dependencies = self.remote_dependencies.get_dependencies(package, version);
3737
match dependencies {
3838
Ok(Dependencies::Known(dependencies)) => {
@@ -43,7 +43,7 @@ impl<DP: DependencyProvider> DependencyProvider for CachingDependencyProvider<DP
4343
);
4444
Ok(Dependencies::Known(dependencies))
4545
}
46-
Ok(Dependencies::Unknown) => Ok(Dependencies::Unknown),
46+
Ok(Dependencies::Unknown(reason)) => Ok(Dependencies::Unknown(reason)),
4747
error @ Err(_) => error,
4848
}
4949
}
@@ -67,6 +67,7 @@ impl<DP: DependencyProvider> DependencyProvider for CachingDependencyProvider<DP
6767
type P = DP::P;
6868
type V = DP::V;
6969
type VS = DP::VS;
70+
type M = DP::M;
7071
}
7172

7273
fn main() {

examples/unsat_root_message_no_version.rs

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ impl Display for Package {
2929
#[derive(Debug, Default)]
3030
struct CustomReportFormatter;
3131

32-
impl ReportFormatter<Package, Range<SemanticVersion>> for CustomReportFormatter {
32+
impl ReportFormatter<Package, Range<SemanticVersion>, String> for CustomReportFormatter {
3333
type Output = String;
3434

3535
fn format_terms(&self, terms: &Map<Package, Term<Range<SemanticVersion>>>) -> String {
@@ -49,10 +49,12 @@ impl ReportFormatter<Package, Range<SemanticVersion>> for CustomReportFormatter
4949
format!("{package} {range} is mandatory")
5050
}
5151
[(p1, Term::Positive(r1)), (p2, Term::Negative(r2))] => {
52-
External::FromDependencyOf(p1, r1.clone(), p2, r2.clone()).to_string()
52+
External::<_, _, String>::FromDependencyOf(p1, r1.clone(), p2, r2.clone())
53+
.to_string()
5354
}
5455
[(p1, Term::Negative(r1)), (p2, Term::Positive(r2))] => {
55-
External::FromDependencyOf(p2, r2.clone(), p1, r1.clone()).to_string()
56+
External::<_, _, String>::FromDependencyOf(p2, r2.clone(), p1, r1.clone())
57+
.to_string()
5658
}
5759
slice => {
5860
let str_terms: Vec<_> = slice.iter().map(|(p, t)| format!("{p} {t}")).collect();
@@ -61,7 +63,10 @@ impl ReportFormatter<Package, Range<SemanticVersion>> for CustomReportFormatter
6163
}
6264
}
6365

64-
fn format_external(&self, external: &External<Package, Range<SemanticVersion>>) -> String {
66+
fn format_external(
67+
&self,
68+
external: &External<Package, Range<SemanticVersion>, String>,
69+
) -> String {
6570
match external {
6671
External::NotRoot(package, version) => {
6772
format!("we are solving dependencies of {package} {version}")
@@ -73,11 +78,11 @@ impl ReportFormatter<Package, Range<SemanticVersion>> for CustomReportFormatter
7378
format!("there is no version of {package} in {set}")
7479
}
7580
}
76-
External::UnavailableDependencies(package, set) => {
81+
External::Custom(package, set, reason) => {
7782
if set == &Range::full() {
78-
format!("dependencies of {package} are unavailable")
83+
format!("dependencies of {package} are unavailable because {reason}")
7984
} else {
80-
format!("dependencies of {package} at version {set} are unavailable")
85+
format!("dependencies of {package} at version {set} are unavailable because {reason}")
8186
}
8287
}
8388
External::FromDependencyOf(package, package_set, dependency, dependency_set) => {

src/error.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ where
1515
{
1616
/// There is no solution for this set of dependencies.
1717
#[error("No solution")]
18-
NoSolution(DerivationTree<DP::P, DP::VS>),
18+
NoSolution(DerivationTree<DP::P, DP::VS, DP::M>),
1919

2020
/// Error arising when the implementer of
2121
/// [DependencyProvider]

src/internal/core.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ pub struct State<DP: DependencyProvider> {
4343
pub partial_solution: PartialSolution<DP>,
4444

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

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

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

298298
// Error reporting #########################################################
299299

300-
fn build_derivation_tree(&self, incompat: IncompDpId<DP>) -> DerivationTree<DP::P, DP::VS> {
300+
fn build_derivation_tree(
301+
&self,
302+
incompat: IncompDpId<DP>,
303+
) -> DerivationTree<DP::P, DP::VS, DP::M> {
301304
let mut all_ids: Set<IncompDpId<DP>> = Set::default();
302305
let mut shared_ids = Set::default();
303306
let mut stack = vec![incompat];

src/internal/incompatibility.rs

Lines changed: 88 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
//! An incompatibility is a set of terms for different packages
44
//! that should never be satisfied all together.
55
6-
use std::fmt;
6+
use std::fmt::{self, Debug, Display};
77
use std::sync::Arc;
88

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

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

4343
#[derive(Debug, Clone)]
44-
enum Kind<P: Package, VS: VersionSet> {
44+
enum Kind<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> {
4545
/// Initial incompatibility aiming at picking the root package for the first decision.
46+
///
47+
/// This incompatibility drives the resolution, it requires that we pick the (virtual) root
48+
/// packages.
4649
NotRoot(P, VS::V),
4750
/// There are no versions in the given range for this package.
51+
///
52+
/// This incompatibility is used when we tried all versions in a range and no version
53+
/// worked, so we have to backtrack
4854
NoVersions(P, VS),
49-
/// Dependencies of the package are unavailable for versions in that range.
50-
UnavailableDependencies(P, VS),
5155
/// Incompatibility coming from the dependencies of a given package.
56+
///
57+
/// If a@1 depends on b>=1,<2, we create an incompatibility with terms `{a 1, b <1,>=2}` with
58+
/// kind `FromDependencyOf(a, 1, b, >=1,<2)`.
59+
///
60+
/// We can merge multiple dependents with the same version. For example, if a@1 depends on b and
61+
/// a@2 depends on b, we can say instead a@1||2 depends on b.
5262
FromDependencyOf(P, VS, P, VS),
5363
/// Derived from two causes. Stores cause ids.
54-
DerivedFrom(IncompId<P, VS>, IncompId<P, VS>),
64+
///
65+
/// For example, if a -> b and b -> c, we can derive a -> c.
66+
DerivedFrom(IncompId<P, VS, M>, IncompId<P, VS, M>),
67+
/// The package is unavailable for reasons outside pubgrub.
68+
///
69+
/// Examples:
70+
/// * The version would require building the package, but builds are disabled.
71+
/// * The package is not available in the cache, but internet access has been disabled.
72+
Custom(P, VS, M),
5573
}
5674

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

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

86-
/// Create an incompatibility to remember
87-
/// that a given set does not contain any version.
104+
/// Create an incompatibility to remember that a given set does not contain any version.
88105
pub fn no_versions(package: P, term: Term<VS>) -> Self {
89106
let set = match &term {
90107
Term::Positive(r) => r.clone(),
@@ -96,14 +113,25 @@ impl<P: Package, VS: VersionSet> Incompatibility<P, VS> {
96113
}
97114
}
98115

99-
/// Create an incompatibility to remember
100-
/// that a package version is not selectable
101-
/// because its list of dependencies is unavailable.
102-
pub fn unavailable_dependencies(package: P, version: VS::V) -> Self {
116+
/// Create an incompatibility for a reason outside pubgrub.
117+
pub fn custom_term(package: P, term: Term<VS>, metadata: M) -> Self {
118+
let set = match &term {
119+
Term::Positive(r) => r.clone(),
120+
Term::Negative(_) => panic!("No version should have a positive term"),
121+
};
122+
Self {
123+
package_terms: SmallMap::One([(package.clone(), term)]),
124+
kind: Kind::Custom(package, set, metadata),
125+
}
126+
}
127+
128+
/// Create an incompatibility for a reason outside pubgrub.
129+
pub fn custom_version(package: P, version: VS::V, metadata: M) -> Self {
103130
let set = VS::singleton(version);
131+
let term = Term::Positive(set.clone());
104132
Self {
105-
package_terms: SmallMap::One([(package.clone(), Term::Positive(set.clone()))]),
106-
kind: Kind::UnavailableDependencies(package, set),
133+
package_terms: SmallMap::One([(package.clone(), term)]),
134+
kind: Kind::Custom(package, set, metadata),
107135
}
108136
}
109137

@@ -135,7 +163,7 @@ impl<P: Package, VS: VersionSet> Incompatibility<P, VS> {
135163
/// When multiple versions of a package depend on the same range of another package,
136164
/// we can merge the two into a single incompatibility.
137165
/// For example, if a@1 depends on b and a@2 depends on b, we can say instead
138-
/// a@1 and a@b depend on b.
166+
/// a@1||2 depends on b.
139167
///
140168
/// It is a special case of prior cause computation where the unified package
141169
/// is the common dependant in the two incompatibilities expressing dependencies.
@@ -155,6 +183,11 @@ impl<P: Package, VS: VersionSet> Incompatibility<P, VS> {
155183
if dep_term != other.get(p2) {
156184
return None;
157185
}
186+
// The metadata must be identical to merge
187+
let self_metadata = self.metadata();
188+
if self_metadata != other.metadata() {
189+
return None;
190+
}
158191
return Some(Self::from_dependency(
159192
p1.clone(),
160193
self.get(p1)
@@ -231,8 +264,8 @@ impl<P: Package, VS: VersionSet> Incompatibility<P, VS> {
231264
self_id: Id<Self>,
232265
shared_ids: &Set<Id<Self>>,
233266
store: &Arena<Self>,
234-
precomputed: &Map<Id<Self>, Arc<DerivationTree<P, VS>>>,
235-
) -> DerivationTree<P, VS> {
267+
precomputed: &Map<Id<Self>, Arc<DerivationTree<P, VS, M>>>,
268+
) -> DerivationTree<P, VS, M> {
236269
match store[self_id].kind.clone() {
237270
Kind::DerivedFrom(id1, id2) => {
238271
let derived = Derived {
@@ -253,19 +286,38 @@ impl<P: Package, VS: VersionSet> Incompatibility<P, VS> {
253286
DerivationTree::External(External::NotRoot(package, version))
254287
}
255288
Kind::NoVersions(package, set) => {
256-
DerivationTree::External(External::NoVersions(package, set))
289+
DerivationTree::External(External::NoVersions(package.clone(), set.clone()))
257290
}
258-
Kind::UnavailableDependencies(package, set) => {
259-
DerivationTree::External(External::UnavailableDependencies(package, set))
291+
Kind::FromDependencyOf(package, set, dep_package, dep_set) => {
292+
DerivationTree::External(External::FromDependencyOf(
293+
package.clone(),
294+
set.clone(),
295+
dep_package.clone(),
296+
dep_set.clone(),
297+
))
260298
}
261-
Kind::FromDependencyOf(package, set, dep_package, dep_set) => DerivationTree::External(
262-
External::FromDependencyOf(package, set, dep_package, dep_set),
263-
),
299+
Kind::Custom(package, set, metadata) => DerivationTree::External(External::Custom(
300+
package.clone(),
301+
set.clone(),
302+
metadata.clone(),
303+
)),
304+
}
305+
}
306+
307+
fn metadata(&self) -> Option<&M> {
308+
match &self.kind {
309+
Kind::NotRoot(_, _)
310+
| Kind::NoVersions(_, _)
311+
| Kind::FromDependencyOf(_, _, _, _)
312+
| Kind::DerivedFrom(_, _) => None,
313+
Kind::Custom(_, _, metadata) => Some(metadata),
264314
}
265315
}
266316
}
267317

268-
impl<'a, P: Package, VS: VersionSet + 'a> Incompatibility<P, VS> {
318+
impl<'a, P: Package, VS: VersionSet + 'a, M: Eq + Clone + Debug + Display + 'a>
319+
Incompatibility<P, VS, M>
320+
{
269321
/// CF definition of Relation enum.
270322
pub fn relation(&self, terms: impl Fn(&P) -> Option<&'a Term<VS>>) -> Relation<P> {
271323
let mut relation = Relation::Satisfied;
@@ -293,12 +345,17 @@ impl<'a, P: Package, VS: VersionSet + 'a> Incompatibility<P, VS> {
293345
}
294346
}
295347

296-
impl<P: Package, VS: VersionSet> fmt::Display for Incompatibility<P, VS> {
348+
impl<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> fmt::Display
349+
for Incompatibility<P, VS, M>
350+
{
297351
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
298352
write!(
299353
f,
300354
"{}",
301-
DefaultStringReportFormatter.format_terms(&self.package_terms.as_map())
355+
ReportFormatter::<P, VS, M>::format_terms(
356+
&DefaultStringReportFormatter,
357+
&self.package_terms.as_map()
358+
)
302359
)
303360
}
304361
}
@@ -326,12 +383,12 @@ pub mod tests {
326383
let mut store = Arena::new();
327384
let i1 = store.alloc(Incompatibility {
328385
package_terms: SmallMap::Two([("p1", t1.clone()), ("p2", t2.negate())]),
329-
kind: Kind::UnavailableDependencies("0", Range::full())
386+
kind: Kind::Custom("0", Range::full(), "foo".to_string())
330387
});
331388

332389
let i2 = store.alloc(Incompatibility {
333390
package_terms: SmallMap::Two([("p2", t2), ("p3", t3.clone())]),
334-
kind: Kind::UnavailableDependencies("0", Range::full())
391+
kind: Kind::Custom("0", Range::full(), "bar".to_string())
335392
});
336393

337394
let mut i3 = Map::default();

0 commit comments

Comments
 (0)