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 and konstin committed May 2, 2024
1 parent e12e4d6 commit 17c6936
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 34 deletions.
8 changes: 4 additions & 4 deletions examples/caching_dependency_provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,18 @@ impl<DP: DependencyProvider<M = String>> DependencyProvider for CachingDependenc
) -> 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::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(reason)) => Ok(Dependencies::Unknown(reason)),
Ok(Dependencies::Unavailable(reason)) => Ok(Dependencies::Unavailable(reason)),
error @ Err(_) => error,
}
}
Expand Down
15 changes: 2 additions & 13 deletions src/internal/incompatibility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,17 +138,6 @@ impl<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> Incompatibilit
}
}

/// 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: M) -> Self {
let set = VS::singleton(version);
Self {
package_terms: SmallMap::One([(package.clone(), Term::Positive(set.clone()))]),
kind: Kind::Custom(package, set, reason),
}
}

/// Build an incompatibility from a given dependency.
pub fn from_dependency(package: P, versions: VS, dep: (&P, &VS)) -> Self {
let (p2, set2) = dep;
Expand Down Expand Up @@ -383,12 +372,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::<_, _, String>::FromDependencyOf("p1", Range::full(), "p2", Range::full())
kind: Kind::Custom("0", Range::full(), "foo".to_string())
});

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

let mut i3 = Map::default();
Expand Down
18 changes: 10 additions & 8 deletions src/solver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,21 +150,21 @@ pub fn resolve<DP: DependencyProvider>(
})?;

let known_dependencies = match dependencies {
Dependencies::Unknown(reason) => {
Dependencies::Unavailable(reason) => {
state.add_incompatibility(Incompatibility::custom_version(
p.clone(),
v.clone(),
reason,
));
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 @@ -194,9 +194,9 @@ pub fn resolve<DP: DependencyProvider>(
#[derive(Clone)]
pub enum Dependencies<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> {
/// Package dependencies are unavailable with the reason why they are missing.
Unknown(M),
Unavailable(M),
/// 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 @@ -277,7 +277,7 @@ pub trait DependencyProvider {
) -> Result<Option<Self::V>, Self::Err>;

/// Retrieves the package dependencies.
/// Return [Dependencies::Unknown] if its dependencies are unknown.
/// Return [Dependencies::Unavailable] if its dependencies are not available.
#[allow(clippy::type_complexity)]
fn get_dependencies(
&self,
Expand Down Expand Up @@ -398,8 +398,10 @@ impl<P: Package, VS: VersionSet> DependencyProvider for OfflineDependencyProvide
version: &VS::V,
) -> Result<Dependencies<P, VS, Self::M>, Infallible> {
Ok(match self.dependencies(package, version) {
None => Dependencies::Unknown("its dependencies could not be determined".to_string()),
Some(dependencies) => Dependencies::Known(dependencies),
None => {
Dependencies::Unavailable("its dependencies could not be determined".to_string())
}
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 @@ -17,7 +17,7 @@ pub type SelectedDependencies<DP> =

/// 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].
pub type DependencyConstraints<P, VS> = Map<P, VS>;
Expand Down
12 changes: 6 additions & 6 deletions tests/proptest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,8 +322,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 @@ -346,8 +346,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 @@ -517,8 +517,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 @@ -66,8 +66,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 17c6936

Please sign in to comment.