refactor: give consistent error messages for solver errors

Signed-off-by: Andrius Pukšta <andrius.puksta@sensmetry.com>

Author: andrius-puksta-sensmetry

core/src/solve/pubgrub.rs

@@ -2,7 +2,7 @@
 // SPDX-License-Identifier: MIT OR Apache-2.0
 
 use fluent_uri::Iri;
-use pubgrub::{DependencyProvider, VersionSet};
+use pubgrub::{DefaultStringReporter, DependencyProvider, Reporter, VersionSet};
 
 use std::{
     cell::RefCell,
@@ -307,23 +307,69 @@ fn compute_deps<R: ResolveRead>(
     Ok(pubgrub::Dependencies::Available(depmap))
 }
 
-#[derive(Error, Debug)]
-#[error("{inner}")]
+#[derive(Debug)]
 pub struct SolverError<R: ResolveRead + fmt::Debug + 'static> {
-    #[from]
     pub inner: Box<pubgrub::PubGrubError<ProjectSolver<R>>>,
 }
 
+impl<R: ResolveRead + fmt::Debug + 'static> From<Box<pubgrub::PubGrubError<ProjectSolver<R>>>>
+    for SolverError<R>
+{
+    fn from(mut value: Box<pubgrub::PubGrubError<ProjectSolver<R>>>) -> Self {
+        if let pubgrub::PubGrubError::NoSolution(ref mut derivation_tree) = *value {
+            derivation_tree.collapse_no_versions();
+        }
+        Self { inner: value }
+    }
+}
+
 impl<R: ResolveRead + fmt::Debug + 'static> From<pubgrub::PubGrubError<ProjectSolver<R>>>
     for SolverError<R>
 {
     fn from(value: pubgrub::PubGrubError<ProjectSolver<R>>) -> Self {
-        Self {
-            inner: Box::new(value),
+        Self::from(Box::new(value))
+    }
+}
+
+impl<R: ResolveRead + fmt::Debug + 'static> Display for SolverError<R> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self.inner.as_ref() {
+            pubgrub::PubGrubError::NoSolution(derivation_tree) => {
+                writeln!(
+                    f,
+                    "failed to satisfy usage constraints:\n{}",
+                    DefaultStringReporter::report(derivation_tree)
+                )
+            }
+            pubgrub::PubGrubError::ErrorRetrievingDependencies {
+                package, source, ..
+            } => match package {
+                DependencyIdentifier::Requested(_) => {
+                    write!(f, "failed to retrieve project(s): {source}")
+                }
+                DependencyIdentifier::Remote(iri) => {
+                    write!(f, "failed to retrieve `{iri}`: {source}")
+                }
+            },
+            pubgrub::PubGrubError::ErrorChoosingVersion { package, source } => match package {
+                DependencyIdentifier::Requested(_) => {
+                    // `fn choose_version()` is infallible in this path
+                    unreachable!();
+                }
+                DependencyIdentifier::Remote(iri) => {
+                    write!(f, "unable to select version of `{iri}`: {source}")
+                }
+            },
+            pubgrub::PubGrubError::ErrorInShouldCancel(_) => {
+                // ProjectSolver doesn't implement this and default impl does nothing
+                unreachable!();
+            }
         }
     }
 }
 
+impl<R: ResolveRead + fmt::Debug + 'static> std::error::Error for SolverError<R> {}
+
 #[derive(Error, Debug)]
 pub enum InternalSolverError<R: ResolveRead> {
     #[error("resolution error: {0}")]

sysand/src/commands/add.rs

@@ -14,11 +14,7 @@ use sysand_core::{
 };
 
 use crate::{
-    CliError,
-    cli::ResolutionOptions,
-    command_sync,
-    commands::lock::{create_resolver, handle_lock_error},
-    read_lockfile,
+    CliError, cli::ResolutionOptions, command_sync, commands::lock::create_resolver, read_lockfile,
 };
 
 // TODO: Collect common arguments
@@ -74,8 +70,7 @@ pub fn command_add<S: AsRef<str>>(
         // Lock before adding to check if resource (and its required version)
         // can be found
         let usage = usage_raw.validate()?;
-        let LockOutcome { lock, .. } =
-            do_lock_extend(current_lock, [usage], resolver).map_err(handle_lock_error)?;
+        let LockOutcome { lock, .. } = do_lock_extend(current_lock, [usage], resolver)?;
 
         do_add(&mut current_project, usage_raw)?;
 

sysand/src/commands/lock.rs

@@ -5,22 +5,17 @@ use std::collections::HashMap;
 use std::path::Path;
 use std::sync::Arc;
 
-use anyhow::{Result, anyhow};
-use pubgrub::Reporter as _;
+use anyhow::Result;
 
 use sysand_core::{
-    commands::lock::{
-        DEFAULT_LOCKFILE_NAME, LockError, LockOutcome, LockProjectError, do_lock_local_editable,
-    },
+    commands::lock::{DEFAULT_LOCKFILE_NAME, LockOutcome, do_lock_local_editable},
     config::Config,
-    project::{ProjectRead, memory::InMemoryProject, utils::wrapfs},
+    project::{memory::InMemoryProject, utils::wrapfs},
     resolve::{
-        ResolveRead,
         memory::{AcceptAll, MemoryResolver},
         priority::PriorityResolver,
         standard::{StandardResolver, standard_resolver},
     },
-    solve::pubgrub::DependencyIdentifier,
     stdlib::known_std_libs,
 };
 
@@ -56,13 +51,7 @@ pub fn command_lock<P: AsRef<Path>>(
     let LockOutcome {
         lock,
         dependencies: _dependencies,
-    } = match do_lock_local_editable(&path, wrapped_resolver) {
-        Ok(lock_outcome) => lock_outcome,
-        Err(LockProjectError::LockError(lock_error)) => {
-            return Err(handle_lock_error(lock_error));
-        }
-        Err(err) => Err(err)?,
-    };
+    } = do_lock_local_editable(&path, wrapped_resolver)?;
 
     let canonical = lock.canonicalize();
     wrapfs::write(
@@ -128,45 +117,3 @@ pub fn create_resolver<P: AsRef<Path>>(
     );
     Ok(wrapped_resolver)
 }
-
-pub fn handle_lock_error<PR: ProjectRead, RR: ResolveRead + std::fmt::Debug>(
-    lock_error: LockError<PR, RR>,
-) -> anyhow::Error {
-    if let LockError::Solver(solver_error) = lock_error {
-        match *solver_error.inner {
-            pubgrub::PubGrubError::NoSolution(mut derivation_tree) => {
-                derivation_tree.collapse_no_versions();
-
-                anyhow!(
-                    "failed to satisfy usage constraints:\n{}",
-                    pubgrub::DefaultStringReporter::report(&derivation_tree)
-                )
-            }
-            pubgrub::PubGrubError::ErrorRetrievingDependencies {
-                package, source, ..
-            } => match package {
-                DependencyIdentifier::Requested(_) => {
-                    anyhow!("failed to retrieve project(s): {source}")
-                }
-                DependencyIdentifier::Remote(iri) => {
-                    anyhow!("failed to retrieve `{iri}`: {source}")
-                }
-            },
-            pubgrub::PubGrubError::ErrorChoosingVersion { package, source } => match package {
-                DependencyIdentifier::Requested(_) => {
-                    // `fn choose_version()` is infallible in this path
-                    unreachable!();
-                }
-                DependencyIdentifier::Remote(iri) => {
-                    anyhow!("unable to select version of `{iri}`: {source}")
-                }
-            },
-            pubgrub::PubGrubError::ErrorInShouldCancel(_) => {
-                // ProjectSolver doesn't implement this and default impl does nothing
-                unreachable!();
-            }
-        }
-    } else {
-        anyhow!("{lock_error}")
-    }
-}