Make it possible to choose resolvo with --solver-to-run

Signed-off-by: J Robert Ray <[email protected]>

Author: jrray

Cargo.lock

@@ -38,7 +38,7 @@ where
 
 impl<Solver> PackageBuildTester<Solver>
 where
-    Solver: AbstractSolverExt + AbstractSolverMut + Default + Send,
+    Solver: AbstractSolverExt + AbstractSolverMut + Clone + Send,
 {
     pub fn new(recipe: SpecRecipe, script: String, solver: Solver) -> Self {
         let source =
@@ -109,22 +109,22 @@ where
             }
         }
 
-        self.solver.set_binary_only(true);
-        self.solver.update_options(self.options.clone());
+        // Use a clone of the solver before changing settings so
+        // `resolve_source_package` can do the same.
+        let mut solver = self.solver.clone();
+        solver.set_binary_only(true);
+        solver.update_options(self.options.clone());
         for repo in self.repos.iter().cloned() {
-            self.solver.add_repository(repo);
+            solver.add_repository(repo);
         }
         // TODO
         // solver.configure_for_build_environment(&self.recipe)?;
         for request in self.additional_requirements.drain(..) {
-            self.solver.add_request(request)
+            solver.add_request(request)
         }
 
         // let (solution, _) = self.build_resolver.solve(&solver).await?;
-        let solution = self
-            .solver
-            .run_and_print_resolve(&self.build_formatter)
-            .await?;
+        let solution = solver.run_and_print_resolve(&self.build_formatter).await?;
 
         for layer in resolve_runtime_layers(requires_localization, &solution).await? {
             rt.push_digest(layer);
@@ -150,7 +150,9 @@ where
     }
 
     async fn resolve_source_package(&mut self, package: &AnyIdent) -> Result<Solution> {
-        let mut solver = Solver::default();
+        // Use a clone of the solver before changing settings so `test` can do
+        // the same.
+        let mut solver = self.solver.clone();
         solver.update_options(self.options.clone());
         let local_repo: Arc<storage::RepositoryHandle> =
             Arc::new(storage::local_repository().await?.into());
@@ -180,7 +182,7 @@ where
 #[async_trait::async_trait]
 impl<Solver> Tester for PackageBuildTester<Solver>
 where
-    Solver: AbstractSolverExt + AbstractSolverMut + Default + Send,
+    Solver: AbstractSolverExt + AbstractSolverMut + Clone + Send,
 {
     async fn test(&mut self) -> Result<()> {
         PackageBuildTester::test(self).await

crates/spk-cli/cmd-test/src/test/build.rs

@@ -18,10 +18,10 @@ use crate::Result;
 /// Returns a new solution of only binary packages.
 pub async fn build_required_packages<Solver>(
     solution: &Solution,
-    _solver: Solver,
+    solver: Solver,
 ) -> Result<Solution>
 where
-    Solver: AbstractSolverExt + AbstractSolverMut + Default,
+    Solver: AbstractSolverExt + AbstractSolverMut + Clone,
 {
     let handle: storage::RepositoryHandle = storage::local_repository().await?.into();
     let local_repo = Arc::new(handle);
@@ -43,7 +43,7 @@ where
             options.format_option_map()
         );
         let (package, components) =
-            BinaryPackageBuilder::from_recipe_with_solver((**recipe).clone(), Solver::default())
+            BinaryPackageBuilder::from_recipe_with_solver((**recipe).clone(), solver.clone())
                 .with_repositories(repos.clone())
                 .build_and_publish(&options, &*local_repo)
                 .await?;

crates/spk-cli/common/src/exec.rs

@@ -36,7 +36,7 @@ use spk_schema::option_map::HOST_OPTIONS;
 use spk_schema::{Recipe, SpecFileData, SpecRecipe, Template, TestStage, VariantExt};
 #[cfg(feature = "statsd")]
 use spk_solve::{get_metrics_client, SPK_RUN_TIME_METRIC};
-use spk_solve::{AbstractSolverExt, AbstractSolverMut};
+use spk_solve::{AbstractSolverExt, AbstractSolverMut, SolverImpl};
 use spk_workspace::FindPackageTemplateResult;
 pub use variant::{Variant, VariantBuildStatus, VariantLocation};
 use {spk_solve as solve, spk_storage as storage};
@@ -264,27 +264,34 @@ impl Solver {
     pub async fn get_solver(
         &self,
         options: &Options,
-    ) -> Result<impl AbstractSolverExt + AbstractSolverMut + Clone + Default> {
+    ) -> Result<impl AbstractSolverExt + AbstractSolverMut + Clone> {
         let option_map = options.get_options()?;
 
-        //let mut solver = solve::Solver::default();
-        let mut solver = solve::CdclSolver::default();
+        let mut solver = match self.decision_formatter_settings.solver_to_run {
+            SolverToRun::Resolvo => SolverImpl::Cdcl(solve::CdclSolver::default()),
+            _ => {
+                let mut solver = solve::Solver::default();
+                // These settings are only applicable to the Og solver.
+                solver.set_initial_request_impossible_checks(
+                    self.check_impossible_initial || self.check_impossible_all,
+                );
+                solver.set_resolve_validation_impossible_checks(
+                    self.check_impossible_validation || self.check_impossible_all,
+                );
+                solver.set_build_key_impossible_checks(
+                    self.check_impossible_builds || self.check_impossible_all,
+                );
+                SolverImpl::Og(solver)
+            }
+        };
+
         solver.update_options(option_map);
 
         for (name, repo) in self.repos.get_repos_for_non_destructive_operation().await? {
             tracing::debug!(repo=%name, "using repository");
             solver.add_repository(repo);
         }
         solver.set_binary_only(!self.allow_builds);
-        //solver.set_initial_request_impossible_checks(
-        //    self.check_impossible_initial || self.check_impossible_all,
-        //);
-        //solver.set_resolve_validation_impossible_checks(
-        //    self.check_impossible_validation || self.check_impossible_all,
-        //);
-        //solver.set_build_key_impossible_checks(
-        //    self.check_impossible_builds || self.check_impossible_all,
-        //);
 
         Ok(solver)
     }
@@ -1123,17 +1130,22 @@ pub enum SolverToRun {
     Cli,
     /// Run and show output from the "impossible requests" checking solver
     Checks,
-    /// Run both solvers, showing the output from the basic solver,
-    /// unless overridden with --solver-to-run
+    /// Run both "cli" and "checks" solvers, showing the output from the "cli"
+    /// solver, unless overridden with --solver-to-run
     All,
+    /// Run the Resolvo-based SAT solver
+    Resolvo,
 }
 
 #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, ValueEnum)]
 pub enum SolverToShow {
-    /// Show output from the basic solver
+    /// Show output from the basic solver.
     Cli,
-    /// Show output from the "impossible requests" checking solver
+    /// Show output from the "impossible requests" checking solver.
     Checks,
+    /// Show output from the Resolvo SAT solver. This is only possible when
+    /// running with that solver.
+    Resolvo,
 }
 
 impl From<SolverToRun> for MultiSolverKind {
@@ -1142,6 +1154,7 @@ impl From<SolverToRun> for MultiSolverKind {
             SolverToRun::Cli => MultiSolverKind::Unchanged,
             SolverToRun::Checks => MultiSolverKind::AllImpossibleChecks,
             SolverToRun::All => MultiSolverKind::All,
+            SolverToRun::Resolvo => MultiSolverKind::Resolvo,
         }
     }
 }
@@ -1151,6 +1164,7 @@ impl From<SolverToShow> for MultiSolverKind {
         match item {
             SolverToShow::Cli => MultiSolverKind::Unchanged,
             SolverToShow::Checks => MultiSolverKind::AllImpossibleChecks,
+            SolverToShow::Resolvo => MultiSolverKind::Resolvo,
         }
     }
 }

crates/spk-cli/common/src/flags.rs

@@ -56,6 +56,7 @@ spk-solve-solution = { workspace = true }
 spk-solve-validation = { workspace = true }
 spk-storage = { workspace = true }
 statsd = { version = "0.15.0", optional = true }
+strum = { workspace = true }
 thiserror = { workspace = true }
 tokio = { workspace = true, features = ["rt"] }
 tracing = { workspace = true }

crates/spk-solve/Cargo.toml

@@ -14,6 +14,9 @@ use variantly::Variantly;
 use crate::{DecisionFormatter, Result};
 
 #[enum_dispatch(AbstractSolver, AbstractSolverExt, AbstractSolverMut)]
+// Don't derive Default. If some code is generic on AbstractSolver and is given
+// one of these, if it wants a "default" solver it needs to be given a new
+// solver of the same variety and `SolverImpl::default()` can't do that.
 #[derive(Clone, Variantly)]
 pub enum SolverImpl {
     Og(crate::Solver),

crates/spk-solve/src/abstract_solver.rs

@@ -4,7 +4,7 @@
 
 use std::cmp::max;
 use std::collections::VecDeque;
-use std::fmt::{Display, Write};
+use std::fmt::Write;
 use std::fs::{File, OpenOptions};
 use std::io::{BufWriter, ErrorKind, Write as IOWrite};
 use std::path::{Path, PathBuf};
@@ -67,6 +67,7 @@ const BY_USER: &str = "by user";
 const CLI_SOLVER: &str = "cli";
 const IMPOSSIBLE_CHECKS_SOLVER: &str = "check";
 const ALL_SOLVERS: &str = "all";
+const RESOLVO_SOLVER: &str = "resolvo";
 
 const UNABLE_TO_GET_OUTPUT_FILE_LOCK: &str = "Unable to get lock to write solver output to file";
 const UNABLE_TO_WRITE_OUTPUT_MESSAGE: &str = "Unable to write solver output message to file";
@@ -1038,14 +1039,22 @@ enum LoopOutcome {
     Success,
 }
 
-#[derive(PartialEq, Eq, Clone, Debug, Default)]
+#[derive(PartialEq, Eq, Clone, Debug, Default, strum::Display)]
 pub enum MultiSolverKind {
+    #[strum(to_string = "Unchanged")]
     Unchanged,
+    #[strum(to_string = "All Impossible Checks")]
     AllImpossibleChecks,
     // This isn't a solver on its own. It indicates: the run all the
     // solvers in parallel but show the output from the unchanged one.
+    // This runs all the solvers implemented in the original solver. At least
+    // for now, it is not possible to run both the original solver and the
+    // new solver in parallel.
     #[default]
+    #[strum(to_string = "All")]
     All,
+    #[strum(to_string = "Resolvo")]
+    Resolvo,
 }
 
 impl MultiSolverKind {
@@ -1060,6 +1069,7 @@ impl MultiSolverKind {
             MultiSolverKind::Unchanged => CLI_SOLVER,
             MultiSolverKind::AllImpossibleChecks => IMPOSSIBLE_CHECKS_SOLVER,
             MultiSolverKind::All => ALL_SOLVERS,
+            MultiSolverKind::Resolvo => RESOLVO_SOLVER,
         }
     }
 
@@ -1087,17 +1097,6 @@ impl MultiSolverKind {
     }
 }
 
-impl Display for MultiSolverKind {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        let name = match self {
-            MultiSolverKind::Unchanged => "Unchanged",
-            MultiSolverKind::AllImpossibleChecks => "All Impossible Checks",
-            MultiSolverKind::All => "All",
-        };
-        write!(f, "{name}")
-    }
-}
-
 struct SolverTaskSettings {
     solver: Solver,
     solver_kind: MultiSolverKind,
@@ -1233,6 +1232,7 @@ impl DecisionFormatter {
                     ignore_failure: false,
                 },
             ]),
+            MultiSolverKind::Resolvo => unreachable!(),
         }
     }