Skip to content

Commit

Permalink
Consolidate UnusableDependencies into a generic Unavailable incom…
Browse files Browse the repository at this point in the history
…patibility (#20)

* Consolidate `UnusableDependencies` into a generic `Unavailable` incompatibility

* Docs
  • Loading branch information
zanieb authored Jan 25, 2024
1 parent f01151d commit 86447f2
Show file tree
Hide file tree
Showing 8 changed files with 46 additions and 99 deletions.
8 changes: 4 additions & 4 deletions examples/caching_dependency_provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,18 +39,18 @@ impl<P: Package, VS: VersionSet, DP: DependencyProvider<P, VS>> DependencyProvid
) -> Result<Dependencies<P, VS>, DP::Err> {
let mut cache = self.cached_dependencies.borrow_mut();
match cache.get_dependencies(package, version) {
Ok(Dependencies::Unknown) => {
Ok(Dependencies::Unavailable) => {
let dependencies = self.remote_dependencies.get_dependencies(package, version);
match dependencies {
Ok(Dependencies::Known(dependencies)) => {
Ok(Dependencies::Available(dependencies)) => {
cache.add_dependencies(
package.clone(),
version.clone(),
dependencies.clone(),
);
Ok(Dependencies::Known(dependencies))
Ok(Dependencies::Available(dependencies))
}
Ok(Dependencies::Unknown) => Ok(Dependencies::Unknown),
Ok(Dependencies::Unavailable) => Ok(Dependencies::Unavailable),
error @ Err(_) => error,
}
}
Expand Down
13 changes: 3 additions & 10 deletions examples/unsat_root_message_no_version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,18 +73,11 @@ impl ReportFormatter<Package, Range<SemanticVersion>> for CustomReportFormatter
format!("there is no version of {package} in {set}")
}
}
External::UnavailableDependencies(package, set) => {
External::Unavailable(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")
}
}
External::UnusableDependencies(package, set, ..) => {
if set == &Range::full() {
format!("dependencies of {package} are unusable")
} else {
format!("dependencies of {package} at version {set} are unusable")
format!("dependencies of {package} at version {set} are unavailable because {reason}")
}
}
External::FromDependencyOf(package, package_set, dependency, dependency_set) => {
Expand Down
33 changes: 8 additions & 25 deletions src/internal/incompatibility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,8 @@ pub enum Kind<P: Package, VS: VersionSet> {
NotRoot(P, VS::V),
/// There are no versions in the given range for this package.
NoVersions(P, VS),
/// Dependencies of the package are unavailable for versions in that range.
UnavailableDependencies(P, VS),
/// Dependencies of the package are unusable for versions in that range.
UnusableDependencies(P, VS, Option<String>),
/// The package is unavailable for versions in the range. A string reason is included.
Unavailable(P, VS, String),
/// Incompatibility coming from the dependencies of a given package.
FromDependencyOf(P, VS, P, VS),
/// Derived from two causes. Stores cause ids.
Expand Down Expand Up @@ -99,23 +97,11 @@ 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 {
pub fn unavailable(package: P, version: VS::V, reason: String) -> Self {
let set = VS::singleton(version);
Self {
package_terms: SmallMap::One([(package.clone(), Term::Positive(set.clone()))]),
kind: Kind::UnavailableDependencies(package, set),
}
}

/// Create an incompatibility to remember
/// that a package version is not selectable
/// because its dependencies are not usable.
pub fn unusable_dependencies(package: P, version: VS::V, reason: Option<String>) -> Self {
let set = VS::singleton(version);
Self {
package_terms: SmallMap::One([(package.clone(), Term::Positive(set.clone()))]),
kind: Kind::UnusableDependencies(package, set, reason),
kind: Kind::Unavailable(package, set, reason),
}
}

Expand Down Expand Up @@ -259,11 +245,8 @@ impl<P: Package, VS: VersionSet> Incompatibility<P, VS> {
Kind::NoVersions(package, set) => {
DerivationTree::External(External::NoVersions(package.clone(), set.clone()))
}
Kind::UnavailableDependencies(package, set) => DerivationTree::External(
External::UnavailableDependencies(package.clone(), set.clone()),
),
Kind::UnusableDependencies(package, set, reason) => DerivationTree::External(
External::UnusableDependencies(package.clone(), set.clone(), reason.clone()),
Kind::Unavailable(package, set, reason) => DerivationTree::External(
External::Unavailable(package.clone(), set.clone(), reason.clone()),
),
Kind::FromDependencyOf(package, set, dep_package, dep_set) => {
DerivationTree::External(External::FromDependencyOf(
Expand Down Expand Up @@ -339,12 +322,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::Unavailable("0", Range::full(), "foo".to_string())
});

let i2 = store.alloc(Incompatibility {
package_terms: SmallMap::Two([("p2", t2), ("p3", t3.clone())]),
kind: Kind::UnavailableDependencies("0", Range::full())
kind: Kind::Unavailable("0", Range::full(), "bar".to_string())
});

let mut i3 = Map::default();
Expand Down
54 changes: 12 additions & 42 deletions src/report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,8 @@ pub enum External<P: Package, VS: VersionSet> {
NotRoot(P, VS::V),
/// There are no versions in the given set for this package.
NoVersions(P, VS),
/// Dependencies of the package are unavailable for versions in that set.
UnavailableDependencies(P, VS),
/// Dependencies of the package are unusable for versions in that set.
UnusableDependencies(P, VS, Option<String>),
/// The package is unusable in the given set. A string reason is included.
Unavailable(P, VS, String),
/// Incompatibility coming from the dependencies of a given package.
FromDependencyOf(P, VS, P, VS),
}
Expand Down Expand Up @@ -85,8 +83,7 @@ impl<P: Package, VS: VersionSet> DerivationTree<P, VS> {
}
External::NoVersions(p, _)
| External::NotRoot(p, _)
| External::UnavailableDependencies(p, _)
| External::UnusableDependencies(p, ..) => {
| External::Unavailable(p, ..) => {
packages.insert(p);
}
},
Expand Down Expand Up @@ -146,16 +143,8 @@ impl<P: Package, VS: VersionSet> DerivationTree<P, VS> {
DerivationTree::External(External::NoVersions(_, r)) => Some(DerivationTree::External(
External::NoVersions(package, set.union(&r)),
)),
DerivationTree::External(External::UnavailableDependencies(_, r)) => Some(
DerivationTree::External(External::UnavailableDependencies(package, set.union(&r))),
),
DerivationTree::External(External::UnusableDependencies(_, r, reason)) => {
Some(DerivationTree::External(External::UnusableDependencies(
package,
set.union(&r),
reason,
)))
}
// Cannot be merged because the reason may not match
DerivationTree::External(External::Unavailable(_, _, _)) => None,
DerivationTree::External(External::FromDependencyOf(p1, r1, p2, r2)) => {
if p1 == package {
Some(DerivationTree::External(External::FromDependencyOf(
Expand Down Expand Up @@ -190,40 +179,21 @@ impl<P: Package, VS: VersionSet> fmt::Display for External<P, VS> {
write!(f, "there is no version of {} in {}", package, set)
}
}
Self::UnavailableDependencies(package, set) => {
Self::Unavailable(package, set, reason) => {
if set == &VS::full() {
write!(f, "dependencies of {} are unavailable", package)
write!(
f,
"dependencies of {} are unavailable because {reason}",
package
)
} else {
write!(
f,
"dependencies of {} at version {} are unavailable",
"dependencies of {} at version {} are unavailable because {reason}",
package, set
)
}
}
Self::UnusableDependencies(package, set, reason) => {
if let Some(reason) = reason {
if set == &VS::full() {
write!(f, "dependencies of {} are unusable: {reason}", package)
} else {
write!(
f,
"dependencies of {} at version {} are unusable: {reason}",
package, set
)
}
} else {
if set == &VS::full() {
write!(f, "dependencies of {} are unusable", package)
} else {
write!(
f,
"dependencies of {} at version {} are unusable",
package, set
)
}
}
}
Self::FromDependencyOf(p, set_p, dep, set_dep) => {
if set_p == &VS::full() && set_dep == &VS::full() {
write!(f, "{} depends on {}", p, dep)
Expand Down
19 changes: 10 additions & 9 deletions src/solver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,20 +158,21 @@ pub fn resolve<P: Package, VS: VersionSet, DP: DependencyProvider<P, VS>>(
})?;

let known_dependencies = match dependencies {
Dependencies::Unknown => {
state.add_incompatibility(Incompatibility::unavailable_dependencies(
Dependencies::Unavailable => {
state.add_incompatibility(Incompatibility::unavailable(
p.clone(),
v.clone(),
"its dependencies could not be determined".to_string(),
));
continue;
}
Dependencies::Known(x) if x.contains_key(p) => {
Dependencies::Available(x) if x.contains_key(p) => {
return Err(PubGrubError::SelfDependency {
package: p.clone(),
version: v,
});
}
Dependencies::Known(x) => x,
Dependencies::Available(x) => x,
};

// Add that package and version if the dependencies are not problematic.
Expand Down Expand Up @@ -201,9 +202,9 @@ pub fn resolve<P: Package, VS: VersionSet, DP: DependencyProvider<P, VS>>(
#[derive(Clone)]
pub enum Dependencies<P: Package, VS: VersionSet> {
/// Package dependencies are unavailable.
Unknown,
Unavailable,
/// Container for all available package versions.
Known(DependencyConstraints<P, VS>),
Available(DependencyConstraints<P, VS>),
}

/// Trait that allows the algorithm to retrieve available packages and their dependencies.
Expand Down Expand Up @@ -254,7 +255,7 @@ pub trait DependencyProvider<P: Package, VS: VersionSet> {
fn choose_version(&self, package: &P, range: &VS) -> Result<Option<VS::V>, Self::Err>;

/// Retrieves the package dependencies.
/// Return [Dependencies::Unknown] if its dependencies are unknown.
/// Return [Dependencies::Unavailable] if its dependencies are not available.
fn get_dependencies(
&self,
package: &P,
Expand Down Expand Up @@ -369,8 +370,8 @@ impl<P: Package, VS: VersionSet> DependencyProvider<P, VS> for OfflineDependency
version: &VS::V,
) -> Result<Dependencies<P, VS>, Infallible> {
Ok(match self.dependencies(package, version) {
None => Dependencies::Unknown,
Some(dependencies) => Dependencies::Known(dependencies),
None => Dependencies::Unavailable,
Some(dependencies) => Dependencies::Available(dependencies),
})
}
}
2 changes: 1 addition & 1 deletion src/type_aliases.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ pub type SelectedDependencies<P, V> = Map<P, V>;

/// Holds information about all possible versions a given package can accept.
/// There is a difference in semantics between an empty map
/// inside [DependencyConstraints] and [Dependencies::Unknown](crate::solver::Dependencies::Unknown):
/// inside [DependencyConstraints] and [Dependencies::Unavailable](crate::solver::Dependencies::Unavailable):
/// the former means the package has no dependency and it is a known fact,
/// while the latter means they could not be fetched by the [DependencyProvider](crate::solver::DependencyProvider).
pub type DependencyConstraints<P, VS> = Map<P, VS>;
12 changes: 6 additions & 6 deletions tests/proptest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,8 +309,8 @@ fn retain_versions<N: Package + Ord, VS: VersionSet>(
continue;
}
let deps = match dependency_provider.get_dependencies(n, v).unwrap() {
Dependencies::Unknown => panic!(),
Dependencies::Known(deps) => deps,
Dependencies::Unavailable => panic!(),
Dependencies::Available(deps) => deps,
};
smaller_dependency_provider.add_dependencies(n.clone(), v.clone(), deps)
}
Expand All @@ -333,8 +333,8 @@ fn retain_dependencies<N: Package + Ord, VS: VersionSet>(
for n in dependency_provider.packages() {
for v in dependency_provider.versions(n).unwrap() {
let deps = match dependency_provider.get_dependencies(n, v).unwrap() {
Dependencies::Unknown => panic!(),
Dependencies::Known(deps) => deps,
Dependencies::Unavailable => panic!(),
Dependencies::Available(deps) => deps,
};
smaller_dependency_provider.add_dependencies(
n.clone(),
Expand Down Expand Up @@ -504,8 +504,8 @@ proptest! {
.get_dependencies(package, version)
.unwrap()
{
Dependencies::Unknown => panic!(),
Dependencies::Known(d) => d.into_iter().collect(),
Dependencies::Unavailable => panic!(),
Dependencies::Available(d) => d.into_iter().collect(),
};
if !dependencies.is_empty() {
to_remove.insert((package, **version, dep_idx.get(&dependencies).0));
Expand Down
4 changes: 2 additions & 2 deletions tests/sat_dependency_provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ impl<P: Package, VS: VersionSet> SatResolve<P, VS> {
// active packages need each of there `deps` to be satisfied
for (p, v, var) in &all_versions {
let deps = match dp.get_dependencies(p, v).unwrap() {
Dependencies::Unknown => panic!(),
Dependencies::Known(d) => d,
Dependencies::Unavailable => panic!(),
Dependencies::Available(d) => d,
};
for (p1, range) in &deps {
let empty_vec = vec![];
Expand Down

0 comments on commit 86447f2

Please sign in to comment.