diff --git a/src/lib.rs b/src/lib.rs index a2f8211..f5f3bb0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -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, @@ -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, /// 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. /// diff --git a/src/solver/clause.rs b/src/solver/clause.rs index 087dead..7644b6c 100644 --- a/src/solver/clause.rs +++ b/src/solver/clause.rs @@ -574,8 +574,13 @@ impl 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) => { diff --git a/src/solver/mod.rs b/src/solver/mod.rs index 16b3a75..697b07c 100644 --- a/src/solver/mod.rs +++ b/src/solver/mod.rs @@ -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; @@ -170,7 +170,31 @@ impl> 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]); + } + } } }; diff --git a/tests/solver.rs b/tests/solver.rs index a6115bf..a0f0a01 100644 --- a/tests/solver.rs +++ b/tests/solver.rs @@ -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, @@ -29,24 +29,48 @@ 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 for Pack { fn from(value: u32) -> Self { - Pack(value) + Pack { + version: value, + unknown_deps: false, + } } } impl From 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) } } @@ -54,7 +78,7 @@ impl FromStr for Pack { type Err = ParseIntError; fn from_str(s: &str) -> Result { - u32::from_str(s).map(Self) + u32::from_str(s).map(Pack::with_version) } } @@ -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() @@ -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) { 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( @@ -205,7 +231,7 @@ impl DependencyProvider> 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) }); } @@ -243,11 +269,17 @@ impl DependencyProvider> 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()), }; @@ -263,7 +295,7 @@ impl DependencyProvider> for BundleBoxProvider { result.constrains.push(dep_spec); } - result + Dependencies::Known(result) } } @@ -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 @@ -365,7 +397,7 @@ 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]); @@ -373,7 +405,7 @@ fn test_unit_propagation_nested() { 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 @@ -397,7 +429,7 @@ 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]); @@ -405,7 +437,7 @@ fn test_resolve_multiple() { 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 @@ -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] @@ -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 @@ -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 @@ -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 @@ -567,7 +639,6 @@ fn test_resolve_favor_without_conflict() { "###); } -// #[test] fn test_resolve_favor_with_conflict() { let mut provider = BundleBoxProvider::from_packages(&[ @@ -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(&[