Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consolidate UnusableDependencies into a generic Unavailable incompatibility #20

Merged
merged 2 commits into from
Jan 25, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.
zanieb marked this conversation as resolved.
Show resolved Hide resolved
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
52 changes: 11 additions & 41 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>),
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