From ab6d033b7223b6a491806dc4d0ad68cae82739da Mon Sep 17 00:00:00 2001 From: Bas Zalmstra Date: Fri, 8 Sep 2023 11:04:58 +0200 Subject: [PATCH 1/2] fix: add N generic everywhere --- crates/rattler_libsolv_rs/src/lib.rs | 8 +++- crates/rattler_libsolv_rs/src/pool.rs | 31 ++++++++------- crates/rattler_libsolv_rs/src/problem.rs | 39 ++++++++++++------- .../rattler_libsolv_rs/src/solver/clause.rs | 24 +++++++----- crates/rattler_libsolv_rs/src/solver/mod.rs | 15 ++++--- 5 files changed, 70 insertions(+), 47 deletions(-) diff --git a/crates/rattler_libsolv_rs/src/lib.rs b/crates/rattler_libsolv_rs/src/lib.rs index d5b505001..5abf799b3 100644 --- a/crates/rattler_libsolv_rs/src/lib.rs +++ b/crates/rattler_libsolv_rs/src/lib.rs @@ -32,6 +32,10 @@ pub use transaction::Transaction; pub use mapping::Mapping; +/// Blanked trait implementation for something that we consider a package name. +pub trait PackageName: Eq + Hash {} +impl PackageName for N {} + /// Version is a name and a version specification. pub trait VersionTrait: Display { /// The version associated with this record. @@ -52,11 +56,11 @@ pub trait VersionSet: Debug + Display + Clone + Eq + Hash { } /// Bla -pub trait DependencyProvider { +pub trait DependencyProvider { /// Sort the specified solvables based on which solvable to try first. fn sort_candidates( &mut self, - pool: &Pool, + pool: &Pool, solvables: &mut [SolvableId], match_spec_to_candidates: &Mapping>>, ); diff --git a/crates/rattler_libsolv_rs/src/pool.rs b/crates/rattler_libsolv_rs/src/pool.rs index b3248d447..0d6ff6a2d 100644 --- a/crates/rattler_libsolv_rs/src/pool.rs +++ b/crates/rattler_libsolv_rs/src/pool.rs @@ -1,19 +1,18 @@ use std::collections::hash_map::Entry; use std::collections::HashMap; use std::fmt::{Display, Formatter}; -use std::hash::Hash; use crate::arena::Arena; use crate::id::{NameId, RepoId, SolvableId, VersionSetId}; use crate::mapping::Mapping; use crate::solvable::{PackageSolvable, Solvable}; -use crate::VersionSet; +use crate::{PackageName, VersionSet}; /// A pool that stores data related to the available packages /// /// Because it stores solvables, it contains references to `PackageRecord`s (the `'a` lifetime comes /// from the original `PackageRecord`s) -pub struct Pool { +pub struct Pool { /// All the solvables that have been registered pub(crate) solvables: Arena>, @@ -21,10 +20,10 @@ pub struct Pool { total_repos: u32, /// Interned package names - package_names: Arena, + package_names: Arena, /// Map from package names to the id of their interned counterpart - pub(crate) names_to_ids: HashMap, + pub(crate) names_to_ids: HashMap, /// Map from interned package names to the solvables that have that name pub(crate) packages_by_name: Mapping>, @@ -42,7 +41,7 @@ pub struct Pool { pub(crate) match_spec_to_forbidden: Mapping>, } -impl Default for Pool { +impl Default for Pool { fn default() -> Self { let mut solvables = Arena::new(); solvables.alloc(Solvable::new_root()); @@ -63,7 +62,7 @@ impl Default for Pool { } } -impl Pool { +impl Pool { /// Creates a new [`Pool`] pub fn new() -> Self { Self::default() @@ -136,7 +135,8 @@ impl Pool { /// Interns a package name into the `Pool`, returning its `NameId` pub fn intern_package_name(&mut self, name: NValue) -> NameId where - NValue: Into, + NValue: Into, + N: Clone, { match self.names_to_ids.entry(name.into()) { Entry::Occupied(e) => *e.get(), @@ -153,14 +153,14 @@ impl Pool { } /// Lookup the package name id associated to the provided name - pub fn lookup_package_name(&self, name: &Name) -> Option { + pub fn lookup_package_name(&self, name: &N) -> Option { self.names_to_ids.get(name).copied() } /// Returns the package name associated to the provided id /// /// Panics if the package name is not found in the pool - pub fn resolve_package_name(&self, name_id: NameId) -> &Name { + pub fn resolve_package_name(&self, name_id: NameId) -> &N { &self.package_names[name_id] } @@ -207,12 +207,12 @@ impl Pool { } /// A helper struct to visualize a name. -pub struct NameDisplay<'pool, VS: VersionSet> { +pub struct NameDisplay<'pool, VS: VersionSet, N: PackageName> { id: NameId, - pool: &'pool Pool, + pool: &'pool Pool, } -impl<'pool, VS: VersionSet> Display for NameDisplay<'pool, VS> { +impl<'pool, VS: VersionSet, N: PackageName + Display> Display for NameDisplay<'pool, VS, N> { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { let name = self.pool.resolve_package_name(self.id); write!(f, "{}", name) @@ -221,7 +221,10 @@ impl<'pool, VS: VersionSet> Display for NameDisplay<'pool, VS> { impl NameId { /// Returns an object that can be used to format the name. - pub fn display(self, pool: &Pool) -> NameDisplay<'_, VS> { + pub fn display( + self, + pool: &Pool, + ) -> NameDisplay<'_, VS, N> { NameDisplay { id: self, pool } } } diff --git a/crates/rattler_libsolv_rs/src/problem.rs b/crates/rattler_libsolv_rs/src/problem.rs index 286e3e218..d9df46e73 100644 --- a/crates/rattler_libsolv_rs/src/problem.rs +++ b/crates/rattler_libsolv_rs/src/problem.rs @@ -3,7 +3,8 @@ use std::collections::{HashMap, HashSet}; use std::fmt; -use std::fmt::Formatter; +use std::fmt::{Display, Formatter}; +use std::hash::Hash; use std::rc::Rc; use itertools::Itertools; @@ -15,7 +16,7 @@ use crate::id::{ClauseId, SolvableId, VersionSetId}; use crate::pool::Pool; use crate::solver::clause::Clause; use crate::solver::Solver; -use crate::{DependencyProvider, VersionSet, VersionTrait}; +use crate::{DependencyProvider, PackageName, VersionSet, VersionTrait}; /// Represents the cause of the solver being unable to find a solution #[derive(Debug)] @@ -38,9 +39,9 @@ impl Problem { } /// Generates a graph representation of the problem (see [`ProblemGraph`] for details) - pub fn graph>( + pub fn graph>( &self, - solver: &Solver, + solver: &Solver, ) -> ProblemGraph { let mut graph = DiGraph::::default(); let mut nodes: HashMap = HashMap::default(); @@ -142,10 +143,15 @@ impl Problem { } /// Display a user-friendly error explaining the problem - pub fn display_user_friendly<'a, VS: VersionSet, D: DependencyProvider>( + pub fn display_user_friendly< + 'a, + VS: VersionSet, + N: PackageName + Display, + D: DependencyProvider, + >( &self, - solver: &'a Solver, - ) -> DisplayUnsat<'a, VS> { + solver: &'a Solver, + ) -> DisplayUnsat<'a, VS, N> { let graph = self.graph(solver); DisplayUnsat::new(graph, solver.pool()) } @@ -311,9 +317,9 @@ impl ProblemGraph { write!(f, "}}") } - fn simplify( + fn simplify( &self, - pool: &Pool, + pool: &Pool, ) -> HashMap> { let graph = &self.graph; @@ -471,16 +477,16 @@ impl ProblemGraph { /// A struct implementing [`fmt::Display`] that generates a user-friendly representation of a /// problem graph -pub struct DisplayUnsat<'pool, VS: VersionSet> { +pub struct DisplayUnsat<'pool, VS: VersionSet, N: PackageName + Display> { graph: ProblemGraph, merged_candidates: HashMap>, installable_set: HashSet, missing_set: HashSet, - pool: &'pool Pool, + pool: &'pool Pool, } -impl<'pool, VS: VersionSet> DisplayUnsat<'pool, VS> { - pub(crate) fn new(graph: ProblemGraph, pool: &'pool Pool) -> Self { +impl<'pool, VS: VersionSet, N: PackageName + Display> DisplayUnsat<'pool, VS, N> { + pub(crate) fn new(graph: ProblemGraph, pool: &'pool Pool) -> Self { let merged_candidates = graph.simplify(pool); let installable_set = graph.get_installable_set(); let missing_set = graph.get_missing_set(); @@ -512,7 +518,10 @@ impl<'pool, VS: VersionSet> DisplayUnsat<'pool, VS> { f: &mut Formatter<'_>, top_level_edges: &[EdgeReference], top_level_indent: bool, - ) -> fmt::Result { + ) -> fmt::Result + where + N: Display, + { pub enum DisplayOp { Requirement(VersionSetId, Vec), Candidate(NodeIndex), @@ -695,7 +704,7 @@ impl<'pool, VS: VersionSet> DisplayUnsat<'pool, VS> { } } -impl fmt::Display for DisplayUnsat<'_, VS> { +impl fmt::Display for DisplayUnsat<'_, VS, N> { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { let (top_level_missing, top_level_conflicts): (Vec<_>, _) = self .graph diff --git a/crates/rattler_libsolv_rs/src/solver/clause.rs b/crates/rattler_libsolv_rs/src/solver/clause.rs index 3866f79ea..656c5bd7e 100644 --- a/crates/rattler_libsolv_rs/src/solver/clause.rs +++ b/crates/rattler_libsolv_rs/src/solver/clause.rs @@ -4,10 +4,11 @@ use crate::{ mapping::Mapping, pool::Pool, solver::decision_map::DecisionMap, - VersionSet, + PackageName, VersionSet, }; -use std::fmt::{Debug, Formatter}; +use std::fmt::{Debug, Display, Formatter}; +use std::hash::Hash; /// Represents a single clause in the SAT problem /// @@ -113,10 +114,10 @@ impl Clause { } /// Visits each literal in the clause - pub fn visit_literals( + pub fn visit_literals( &self, learnt_clauses: &Arena>, - pool: &Pool, + pool: &Pool, mut visit: impl FnMut(Literal), ) { match *self { @@ -203,7 +204,10 @@ impl ClauseState { clause } - pub fn debug<'a, VS: VersionSet>(&self, pool: &'a Pool) -> ClauseDebug<'a, VS> { + pub fn debug<'a, VS: VersionSet, N: PackageName>( + &self, + pool: &'a Pool, + ) -> ClauseDebug<'a, VS, N> { ClauseDebug { kind: self.kind, pool, @@ -315,9 +319,9 @@ impl ClauseState { } } - pub fn next_unwatched_variable( + pub fn next_unwatched_variable( &self, - pool: &Pool, + pool: &Pool, learnt_clauses: &Arena>, decision_map: &DecisionMap, ) -> Option { @@ -395,12 +399,12 @@ impl Literal { } /// A representation of a clause that implements [`Debug`] -pub(crate) struct ClauseDebug<'pool, VS: VersionSet> { +pub(crate) struct ClauseDebug<'pool, VS: VersionSet, N: PackageName> { kind: Clause, - pool: &'pool Pool, + pool: &'pool Pool, } -impl Debug for ClauseDebug<'_, VS> { +impl Debug for ClauseDebug<'_, VS, N> { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self.kind { Clause::InstallRoot => write!(f, "install root"), diff --git a/crates/rattler_libsolv_rs/src/solver/mod.rs b/crates/rattler_libsolv_rs/src/solver/mod.rs index 8d581b62d..bce95a054 100644 --- a/crates/rattler_libsolv_rs/src/solver/mod.rs +++ b/crates/rattler_libsolv_rs/src/solver/mod.rs @@ -11,8 +11,9 @@ use std::cell::OnceCell; use itertools::Itertools; use std::collections::{HashMap, HashSet}; +use std::fmt::Display; -use crate::{DependencyProvider, VersionSet, VersionSetId}; +use crate::{DependencyProvider, PackageName, VersionSet, VersionSetId}; use clause::{Clause, ClauseState, Literal}; use decision::Decision; use decision_tracker::DecisionTracker; @@ -28,9 +29,9 @@ mod watch_map; /// /// Keeps solvables in a `Pool`, which contains references to `PackageRecord`s (the `'a` lifetime /// comes from the original `PackageRecord`s) -pub struct Solver> { +pub struct Solver> { provider: D, - pool: Pool, + pool: Pool, pub(crate) clauses: Vec, watches: WatchMap, @@ -42,9 +43,9 @@ pub struct Solver> { decision_tracker: DecisionTracker, } -impl> Solver { +impl> Solver { /// Create a solver, using the provided pool - pub fn new(pool: Pool, provider: D) -> Self { + pub fn new(pool: Pool, provider: D) -> Self { Self { clauses: Vec::new(), watches: WatchMap::new(), @@ -58,10 +59,12 @@ impl> Solver { } /// Returns a reference to the pool used by the solver - pub fn pool(&self) -> &Pool { + pub fn pool(&self) -> &Pool { &self.pool } +} +impl> Solver { /// Solves the provided `jobs` and returns a transaction from the found solution /// /// Returns a [`Problem`] if no solution was found, which provides ways to inspect the causes From 13f6f948d81ed9c6778db39de1a9ae2d3df89ff4 Mon Sep 17 00:00:00 2001 From: Bas Zalmstra Date: Fri, 8 Sep 2023 11:08:54 +0200 Subject: [PATCH 2/2] Update crates/rattler_libsolv_rs/src/lib.rs Co-authored-by: Tim de Jager --- crates/rattler_libsolv_rs/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/rattler_libsolv_rs/src/lib.rs b/crates/rattler_libsolv_rs/src/lib.rs index 5abf799b3..5869ada4c 100644 --- a/crates/rattler_libsolv_rs/src/lib.rs +++ b/crates/rattler_libsolv_rs/src/lib.rs @@ -32,7 +32,7 @@ pub use transaction::Transaction; pub use mapping::Mapping; -/// Blanked trait implementation for something that we consider a package name. +/// Blanket trait implementation for something that we consider a package name. pub trait PackageName: Eq + Hash {} impl PackageName for N {}