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

Allow returning "unknown dependencies" in get_dependencies #17

Merged
merged 1 commit into from
Jan 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
17 changes: 14 additions & 3 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ pub struct Candidates {
/// no solution could be found otherwise.
///
/// The same behavior can be achieved by sorting this candidate to the top using the
/// [`DependencyProvider::sort_candidates`] function but using this method providers better
/// [`DependencyProvider::sort_candidates`] function but using this method provides better
/// error messages to the user.
pub favored: Option<SolvableId>,

Expand All @@ -115,14 +115,25 @@ pub struct Candidates {
}

/// Holds information about the dependencies of a package.
pub enum Dependencies {
/// The dependencies are known.
Known(KnownDependencies),
/// The dependencies are unknown, so the parent solvable should be excluded from the solution.
///
/// The string provides more information about why the dependencies are unknown (e.g. an error
/// message).
Unknown(StringId),
}

/// Holds information about the dependencies of a package when they are known.
#[derive(Default, Clone, Debug)]
pub struct Dependencies {
pub struct KnownDependencies {
/// Defines which packages should be installed alongside the depending package and the
/// constraints applied to the package.
pub requirements: Vec<VersionSetId>,

/// Defines additional constraints on packages that may or may not be part of the solution.
/// Different from `requirements` packages in this set are not necessarily included in the
/// Different from `requirements`, packages in this set are not necessarily included in the
/// solution. Only when one or more packages list the package in their `requirements` is the
/// package also added to the solution.
///
Expand Down
9 changes: 7 additions & 2 deletions src/solver/clause.rs
Original file line number Diff line number Diff line change
Expand Up @@ -574,8 +574,13 @@ impl<VS: VersionSet, N: PackageName + Display> Debug for ClauseDebug<'_, VS, N>
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self.kind {
Clause::InstallRoot => write!(f, "install root"),
Clause::Excluded(_, reason) => {
write!(f, "excluded because {}", self.pool.resolve_string(reason))
Clause::Excluded(solvable_id, reason) => {
write!(
f,
"{} excluded because: {}",
solvable_id.display(self.pool),
self.pool.resolve_string(reason)
)
}
Clause::Learnt(learnt_id) => write!(f, "learnt clause {learnt_id:?}"),
Clause::Requires(solvable_id, match_spec_id) => {
Expand Down
28 changes: 26 additions & 2 deletions src/solver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::{
pool::Pool,
problem::Problem,
solvable::SolvableInner,
DependencyProvider, PackageName, VersionSet, VersionSetId,
Dependencies, DependencyProvider, PackageName, VersionSet, VersionSetId,
};
use std::collections::HashSet;
use std::fmt::Display;
Expand Down Expand Up @@ -170,7 +170,31 @@ impl<VS: VersionSet, N: PackageName + Display, D: DependencyProvider<VS, N>> Sol
SolvableInner::Root => (self.root_requirements.clone(), Vec::new()),
SolvableInner::Package(_) => {
let deps = self.cache.get_or_cache_dependencies(solvable_id);
(deps.requirements.clone(), deps.constrains.clone())
match deps {
Dependencies::Known(deps) => {
(deps.requirements.clone(), deps.constrains.clone())
}
Dependencies::Unknown(reason) => {
// There is no information about the solvable's dependencies, so we add
// an exclusion clause for it
let clause_id = self
.clauses
.alloc(ClauseState::exclude(solvable_id, *reason));

// Exclusions are negative assertions, tracked outside of the watcher system
self.negative_assertions.push((solvable_id, clause_id));

// There should always be a conflict here
debug_assert!(
self.decision_tracker.assigned_value(solvable_id) == Some(true)
);

// The new assertion should be kept (it is returned in the lhs of the
// tuple), but it should also be marked as the source of a conflict (rhs
// of the tuple)
return (vec![clause_id], vec![clause_id]);
}
}
}
};

Expand Down
126 changes: 98 additions & 28 deletions tests/solver.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use indexmap::IndexMap;
use itertools::Itertools;
use resolvo::{
range::Range, Candidates, DefaultSolvableDisplay, Dependencies, DependencyProvider, NameId,
Pool, SolvableId, Solver, SolverCache, VersionSet, VersionSetId,
range::Range, Candidates, DefaultSolvableDisplay, Dependencies, DependencyProvider,
KnownDependencies, NameId, Pool, SolvableId, Solver, SolverCache, VersionSet, VersionSetId,
};
use std::{
collections::HashMap,
Expand All @@ -29,32 +29,56 @@ use tracing_test::traced_test;

/// This is `Pack` which is a unique version and name in our bespoke packaging system
#[derive(Debug, Ord, PartialOrd, Eq, PartialEq, Copy, Clone, Hash)]
#[repr(transparent)]
struct Pack(u32);
struct Pack {
version: u32,
unknown_deps: bool,
}

impl Pack {
fn with_version(version: u32) -> Pack {
Pack {
version,
unknown_deps: false,
}
}

fn offset(&self, version_offset: i32) -> Pack {
Pack {
version: self.version.wrapping_add_signed(version_offset),
unknown_deps: self.unknown_deps,
}
}
}

impl From<u32> for Pack {
fn from(value: u32) -> Self {
Pack(value)
Pack {
version: value,
unknown_deps: false,
}
}
}

impl From<i32> for Pack {
fn from(value: i32) -> Self {
Pack(value as u32)
Pack {
version: value as u32,
unknown_deps: false,
}
}
}

impl Display for Pack {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
write!(f, "{}", self.0)
write!(f, "{}", self.version)
}
}

impl FromStr for Pack {
type Err = ParseIntError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
u32::from_str(s).map(Self)
u32::from_str(s).map(Pack::with_version)
}
}

Expand Down Expand Up @@ -91,7 +115,7 @@ impl FromStr for Spec {
.map(FromStr::from_str)
.transpose()
.unwrap()
.unwrap_or(Pack(start.0 + 1));
.unwrap_or(start.offset(1));
Range::between(start, end)
} else {
Range::full()
Expand Down Expand Up @@ -139,24 +163,26 @@ impl BundleBoxProvider {
pub fn from_packages(packages: &[(&str, u32, Vec<&str>)]) -> Self {
let mut result = Self::new();
for (name, version, deps) in packages {
result.add_package(name, Pack(*version), deps, &[]);
result.add_package(name, Pack::with_version(*version), deps, &[]);
}
result
}

pub fn set_favored(&mut self, package_name: &str, version: u32) {
self.favored.insert(package_name.to_owned(), Pack(version));
self.favored
.insert(package_name.to_owned(), Pack::with_version(version));
}

pub fn exclude(&mut self, package_name: &str, version: u32, reason: impl Into<String>) {
self.excluded
.entry(package_name.to_owned())
.or_default()
.insert(Pack(version), reason.into());
.insert(Pack::with_version(version), reason.into());
}

pub fn set_locked(&mut self, package_name: &str, version: u32) {
self.locked.insert(package_name.to_owned(), Pack(version));
self.locked
.insert(package_name.to_owned(), Pack::with_version(version));
}

pub fn add_package(
Expand Down Expand Up @@ -205,7 +231,7 @@ impl DependencyProvider<Range<Pack>> for BundleBoxProvider {
let a = self.pool.resolve_solvable(*a).inner();
let b = self.pool.resolve_solvable(*b).inner();
// We want to sort with highest version on top
b.0.cmp(&a.0)
b.version.cmp(&a.version)
});
}

Expand Down Expand Up @@ -243,11 +269,17 @@ impl DependencyProvider<Range<Pack>> for BundleBoxProvider {
let candidate = self.pool.resolve_solvable(solvable);
let package_name = self.pool.resolve_package_name(candidate.name_id());
let pack = candidate.inner();

if pack.unknown_deps {
let reason = self.pool.intern_string("could not retrieve deps");
return Dependencies::Unknown(reason);
}

let Some(deps) = self.packages.get(package_name).and_then(|v| v.get(pack)) else {
return Default::default();
return Dependencies::Known(Default::default());
};

let mut result = Dependencies {
let mut result = KnownDependencies {
requirements: Vec::with_capacity(deps.dependencies.len()),
constrains: Vec::with_capacity(deps.constrains.len()),
};
Expand All @@ -263,7 +295,7 @@ impl DependencyProvider<Range<Pack>> for BundleBoxProvider {
result.constrains.push(dep_spec);
}

result
Dependencies::Known(result)
}
}

Expand Down Expand Up @@ -342,7 +374,7 @@ fn test_unit_propagation_1() {
solver.pool().resolve_package_name(solvable.name_id()),
"asdf"
);
assert_eq!(solvable.inner().0, 1);
assert_eq!(solvable.inner().version, 1);
}

/// Test if we can also select a nested version
Expand All @@ -365,15 +397,15 @@ fn test_unit_propagation_nested() {
solver.pool().resolve_package_name(solvable.name_id()),
"asdf"
);
assert_eq!(solvable.inner().0, 1);
assert_eq!(solvable.inner().version, 1);

let solvable = solver.pool().resolve_solvable(solved[1]);

assert_eq!(
solver.pool().resolve_package_name(solvable.name_id()),
"efgh"
);
assert_eq!(solvable.inner().0, 4);
assert_eq!(solvable.inner().version, 4);
}

/// Test if we can resolve multiple versions at once
Expand All @@ -397,15 +429,15 @@ fn test_resolve_multiple() {
solver.pool().resolve_package_name(solvable.name_id()),
"asdf"
);
assert_eq!(solvable.inner().0, 2);
assert_eq!(solvable.inner().version, 2);

let solvable = solver.pool().resolve_solvable(solved[1]);

assert_eq!(
solver.pool().resolve_package_name(solvable.name_id()),
"efgh"
);
assert_eq!(solvable.inner().0, 5);
assert_eq!(solvable.inner().version, 5);
}

/// In case of a conflict the version should not be selected with the conflict
Expand Down Expand Up @@ -453,7 +485,7 @@ fn test_resolve_with_nonexisting() {
solver.pool().resolve_package_name(solvable.name_id()),
"asdf"
);
assert_eq!(solvable.inner().0, 3);
assert_eq!(solvable.inner().version, 3);
}

#[test]
Expand Down Expand Up @@ -489,7 +521,44 @@ fn test_resolve_with_nested_deps() {
solver.pool().resolve_package_name(solvable.name_id()),
"apache-airflow"
);
assert_eq!(solvable.inner().0, 1);
assert_eq!(solvable.inner().version, 1);
}

#[test]
#[traced_test]
fn test_resolve_with_unknown_deps() {
let mut provider = BundleBoxProvider::new();
provider.add_package(
"opentelemetry-api",
Pack {
version: 3,
unknown_deps: true,
},
&[],
&[],
);
provider.add_package(
"opentelemetry-api",
Pack {
version: 2,
unknown_deps: false,
},
&[],
&[],
);
let requirements = provider.requirements(&["opentelemetry-api"]);
let mut solver = Solver::new(provider);
let solved = solver.solve(requirements).unwrap();

assert_eq!(solved.len(), 1);

let solvable = solver.pool().resolve_solvable(solved[0]);

assert_eq!(
solver.pool().resolve_package_name(solvable.name_id()),
"opentelemetry-api"
);
assert_eq!(solvable.inner().version, 2);
}

/// Locking a specific package version in this case a lower version namely `3` should result
Expand All @@ -507,7 +576,10 @@ fn test_resolve_locked_top_level() {

assert_eq!(solved.len(), 1);
let solvable_id = solved[0];
assert_eq!(solver.pool().resolve_solvable(solvable_id).inner().0, 3);
assert_eq!(
solver.pool().resolve_solvable(solvable_id).inner().version,
3
);
}

/// Should ignore lock when it is not a top level package and a newer version exists without it
Expand All @@ -532,7 +604,7 @@ fn test_resolve_ignored_locked_top_level() {
solver.pool().resolve_package_name(solvable.name_id()),
"asdf"
);
assert_eq!(solvable.inner().0, 4);
assert_eq!(solvable.inner().version, 4);
}

/// Test checks if favoring without a conflict results in a package upgrade
Expand Down Expand Up @@ -567,7 +639,6 @@ fn test_resolve_favor_without_conflict() {
"###);
}

//
#[test]
fn test_resolve_favor_with_conflict() {
let mut provider = BundleBoxProvider::from_packages(&[
Expand Down Expand Up @@ -742,7 +813,6 @@ fn test_unsat_applies_graph_compression() {
insta::assert_snapshot!(error);
}

//
#[test]
fn test_unsat_constrains() {
let mut provider = BundleBoxProvider::from_packages(&[
Expand Down