Skip to content

Commit

Permalink
Rename dependencies type to available and unavailable (pubgrub-rs#216)
Browse files Browse the repository at this point in the history
In uv, dependencies are either available or unavailable. They are not unknown, but rather missing due to some brokenness: We're offline but the dep is not cached, the version list failed to deserialize, etc. (https://github.com/astral-sh/uv/blob/0b84eb01408eb0e90b5720b027359aac10708665/crates/uv-resolver/src/resolver/mod.rs#L945-L996). This change is a rename of the variants of `Dependencies` to reflect that, upstreaming the change from uv.
  • Loading branch information
konstin authored May 7, 2024
1 parent f3dbcda commit fce8e02
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 29 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
3 changes: 1 addition & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@
//!
//! The third method [get_dependencies](crate::solver::DependencyProvider::get_dependencies)
//! aims at retrieving the dependencies of a given package at a given version.
//! Returns [None] if dependencies are unknown.
//!
//! In a real scenario, these two methods may involve reading the file system
//! or doing network request, so you may want to hold a cache in your
Expand Down Expand Up @@ -151,7 +150,7 @@
//! External incompatibilities have reasons that are independent
//! of the way this algorithm is implemented such as
//! - dependencies: "package_a" at version 1 depends on "package_b" at version 4
//! - missing dependencies: dependencies of "package_a" are unknown
//! - missing dependencies: dependencies of "package_a" are unavailable
//! - absence of version: there is no version of "package_a" in the range [3.1.0 4.0.0[
//!
//! Derived incompatibilities are obtained during the algorithm execution by deduction,
Expand Down
27 changes: 13 additions & 14 deletions src/solver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,30 +149,27 @@ pub fn resolve<DP: DependencyProvider>(
}
})?;

let known_dependencies = match dependencies {
Dependencies::Unknown(reason) => {
let dependencies = match dependencies {
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.
let dep_incompats = state.add_incompatibility_from_dependencies(
p.clone(),
v.clone(),
&known_dependencies,
);
let dep_incompats =
state.add_incompatibility_from_dependencies(p.clone(), v.clone(), &dependencies);

state.partial_solution.add_version(
p.clone(),
Expand All @@ -194,9 +191,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 +274,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 unavailable.
#[allow(clippy::type_complexity)]
fn get_dependencies(
&self,
Expand Down Expand Up @@ -398,8 +395,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 fce8e02

Please sign in to comment.