Display formatted resolver errors

commit-id:27ea514b
software-mansion · Jul 17, 2024 · 30f679d · 30f679d
1 parent db09a5b
commit 30f679d
Show file tree

Hide file tree

Showing 9 changed files with 99 additions and 40 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -104,7 +104,7 @@ redb = "2.1.1"
 reqwest = { version = "0.11", features = ["gzip", "brotli", "deflate", "json", "stream"], default-features = false }
 salsa = "0.16.1"
 semver = { version = "1", features = ["serde"] }
-semver-pubgrub = { git = "https://github.com/pubgrub-rs/semver-pubgrub.git" }
+semver-pubgrub = { git = "https://github.com/maciektr/semver-pubgrub.git" }
 serde = { version = "1", features = ["serde_derive"] }
 serde-untagged = "0.1"
 serde-value = "0.7"

diff --git a/scarb/src/resolver/algorithm/mod.rs b/scarb/src/resolver/algorithm/mod.rs
@@ -1,11 +1,15 @@
 use crate::core::lockfile::Lockfile;
 use crate::core::registry::Registry;
 use crate::core::{PackageId, PackageName, Resolve, Summary};
-use crate::resolver::algorithm::provider::{PubGrubDependencyProvider, PubGrubPackage};
+use crate::resolver::algorithm::provider::{
+    DependencyProviderError, PubGrubDependencyProvider, PubGrubPackage,
+};
 use crate::resolver::algorithm::solution::build_resolve;
 use anyhow::bail;
 use indoc::indoc;
 use itertools::Itertools;
+use pubgrub::error::PubGrubError;
+use pubgrub::report::{DefaultStringReporter, Reporter};
 use pubgrub::type_aliases::SelectedDependencies;
 use pubgrub::{Incompatibility, State};
 use std::collections::{HashMap, HashSet};
@@ -53,8 +57,8 @@ pub async fn resolve<'c>(
         }
 
         // Resolve requirements
-        let solution = pubgrub::solver::resolve_state(&provider, &mut state, package)
-            .map_err(|err| anyhow::format_err!("failed to resolve: {:?}", err))?;
+        let solution =
+            pubgrub::solver::resolve_state(&provider, &mut state, package).map_err(format_error)?;
 
         dbg!(&solution);
 
@@ -63,6 +67,39 @@ pub async fn resolve<'c>(
     })
 }
 
+fn format_error(err: PubGrubError<PubGrubDependencyProvider<'_, '_>>) -> anyhow::Error {
+    match err {
+        PubGrubError::NoSolution(derivation_tree) => {
+            anyhow::format_err!(
+                "version solving failed:\n{}\n",
+                DefaultStringReporter::report(&derivation_tree)
+            )
+        }
+        PubGrubError::ErrorChoosingPackageVersion(DependencyProviderError::PackageNotFound {
+            name,
+            version,
+        }) => {
+            anyhow::format_err!("cannot find package `{name} {version}`")
+        }
+        PubGrubError::ErrorChoosingPackageVersion(DependencyProviderError::PackageQueryFailed(
+            err,
+        )) => anyhow::format_err!("{}", err).context("dependency query failed"),
+        PubGrubError::ErrorRetrievingDependencies {
+            package,
+            version,
+            source,
+        } => anyhow::Error::from(source)
+            .context(format!("cannot get dependencies of `{package}@{version}`")),
+        PubGrubError::SelfDependency { package, version } => {
+            anyhow::format_err!("self dependency found: `{}@{}`", package, version)
+        }
+        PubGrubError::ErrorInShouldCancel(err) => {
+            anyhow::format_err!("{}", err).context("should cancel failed")
+        }
+        PubGrubError::Failure(msg) => anyhow::format_err!("{}", msg).context("resolver failure"),
+    }
+}
+
 fn validate_solution(
     solution: &SelectedDependencies<PubGrubDependencyProvider<'_, '_>>,
 ) -> anyhow::Result<()> {

diff --git a/scarb/src/resolver/algorithm/provider.rs b/scarb/src/resolver/algorithm/provider.rs
@@ -138,8 +138,9 @@ impl<'a, 'c> PubGrubDependencyProvider<'a, 'c> {
                 write_lock.insert(summary.package_id, summary.clone());
                 write_lock.insert(package_id, summary.clone());
             }
-            summary.ok_or_else(|| {
-                DependencyProviderError::PackageNotFound(dependency.name.clone().to_string())
+            summary.ok_or_else(|| DependencyProviderError::PackageNotFound {
+                name: dependency.name.clone().to_string(),
+                version: dependency.version_req.clone(),
             })
         })
     }
@@ -254,8 +255,11 @@ impl<'a, 'c> DependencyProvider for PubGrubDependencyProvider<'a, 'c> {
                 summaries
                     .into_iter()
                     .find(|summary| req.matches(&summary.package_id.version))
-                    .map(|summary| (summary, req))
-                    .ok_or_else(|| DependencyProviderError::PackageNotFound(dep_name))
+                    .map(|summary| (summary, req.clone()))
+                    .ok_or_else(|| DependencyProviderError::PackageNotFound {
+                        name: dep_name,
+                        version: DependencyVersionReq::Req(req),
+                    })
             })
             .collect::<Result<Vec<(Summary, VersionReq)>, DependencyProviderError>>()?;
         let constraints = deps
@@ -272,9 +276,12 @@ impl<'a, 'c> DependencyProvider for PubGrubDependencyProvider<'a, 'c> {
 #[non_exhaustive]
 pub enum DependencyProviderError {
     /// Package not found.
-    #[error("failed to get package `{0}`")]
-    PackageNotFound(String),
+    #[error("cannot find package `{name} {version}`")]
+    PackageNotFound {
+        name: String,
+        version: DependencyVersionReq,
+    },
     // Package query failed.
-    #[error("package query failed: {0}")]
+    #[error("{0}")]
     PackageQueryFailed(#[from] anyhow::Error),
 }
diff --git a/scarb/src/resolver/mod.rs b/scarb/src/resolver/mod.rs
@@ -320,12 +320,10 @@ mod tests {
             ],
             &[deps![("top1", "1"), ("top2", "1")]],
             Err(indoc! {"
-            Version solving failed:
-            - top2 v1.0.0 cannot use foo v1.0.0, because top2 requires foo ^2.0.0
-
-            Scarb does not have real version solving algorithm yet.
-            Perhaps in the future this conflict could be resolved, but currently,
-            please upgrade your dependencies to use latest versions of their dependencies.
+            version solving failed:
+            Because there is no version of top2 in >1.0.0, <2.0.0 and top2 1.0.0 depends on foo >=2.0.0, <3.0.0, top2 >=1.0.0, <2.0.0 depends on foo >=2.0.0, <3.0.0.
+            And because top1 1.0.0 depends on foo >=1.0.0, <2.0.0 and there is no version of top1 in >1.0.0, <2.0.0, top1 >=1.0.0, <2.0.0, top2 >=1.0.0, <2.0.0 are incompatible.
+            And because root_1 1.0.0 depends on top1 >=1.0.0, <2.0.0 and root_1 1.0.0 depends on top2 >=1.0.0, <2.0.0, root_1 1.0.0 is forbidden.
             "}),
         )
     }
@@ -335,7 +333,7 @@ mod tests {
         check(
             registry![],
             &[deps![("foo", "1.0.0")]],
-            Err(r#"MockRegistry/query: cannot find foo ^1.0.0"#),
+            Err(r#"cannot get dependencies of `root_1@1.0.0`"#),
         )
     }
 
@@ -344,7 +342,7 @@ mod tests {
         check(
             registry![("foo v2.0.0", []),],
             &[deps![("foo", "1.0.0")]],
-            Err(r#"cannot find package foo"#),
+            Err(r#"cannot get dependencies of `[email protected]`"#),
         )
     }
 
@@ -353,7 +351,7 @@ mod tests {
         check(
             registry![("foo v1.0.0", []),],
             &[deps![("foo", "1.0.0", "git+https://example.git/foo.git")]],
-            Err(r#"MockRegistry/query: cannot find foo ^1.0.0 (git+https://example.git/foo.git)"#),
+            Err(r#"cannot get dependencies of `root_1@1.0.0`"#),
         )
     }
 
@@ -369,7 +367,7 @@ mod tests {
                 ("b v3.8.14", []),
             ],
             &[deps![("a", "~3.6"), ("b", "~3.6")]],
-            Err(r#"cannot find package a"#),
+            Err(r#"cannot get dependencies of `[email protected]`"#),
         )
     }
 
@@ -389,7 +387,7 @@ mod tests {
                 ("b v3.8.5", [("d", "2.9.0")]),
             ],
             &[deps![("a", "~3.6"), ("c", "~1.1"), ("b", "~3.6")]],
-            Err(r#"cannot find package a"#),
+            Err(r#"cannot get dependencies of `[email protected]`"#),
         )
     }
 
@@ -412,7 +410,7 @@ mod tests {
                 ),
             ],
             &[deps![("e", "~1.0"), ("a", "~3.7"), ("b", "~3.7")]],
-            Err(r#"cannot find package e"#),
+            Err(r#"cannot get dependencies of `[email protected]`"#),
         )
     }
 

diff --git a/scarb/tests/add.rs b/scarb/tests/add.rs
@@ -199,7 +199,10 @@ fn runs_resolver_if_network_is_allowed() {
         "#})
         .failure()
         .stdout_matches(indoc! {r#"
-            error: cannot find package dep
+            error: cannot get dependencies of `[email protected]`
+
+            Caused by:
+                cannot find package `dep ^1.0.0`
         "#})
         .run();
 }

diff --git a/scarb/tests/git_source_network.rs b/scarb/tests/git_source_network.rs
@@ -38,10 +38,12 @@ fn https_something_happens() {
             .failure()
             .stdout_matches(indoc! {r#"
             [..] Updating git repository https://127.0.0.1:[..]/foo/bar
-            error: failed to clone into: [..]
+            error: cannot get dependencies of `[email protected]`
 
             Caused by:
-                process did not exit successfully: exit [..]: 128
+                0: failed to clone into: [..]
+                1: failed to clone into: [..]
+                2: process did not exit successfully: exit status: 128
             "#});
     });
 }
@@ -73,10 +75,12 @@ fn ssh_something_happens() {
             .failure()
             .stdout_matches(indoc! {r#"
             [..] Updating git repository ssh://127.0.0.1:[..]/foo/bar
-            error: failed to clone into: [..]
+            error: cannot get dependencies of `[email protected]`
 
             Caused by:
-                process did not exit successfully: exit [..]: 128
+                0: failed to clone into: [..]
+                1: failed to clone into: [..]
+                2: process did not exit successfully: exit status: 128
             "#});
     });
 }
diff --git a/scarb/tests/http_registry.rs b/scarb/tests/http_registry.rs
@@ -191,10 +191,12 @@ fn not_found() {
         .assert()
         .failure()
         .stdout_matches(indoc! {r#"
-        error: failed to lookup for `baz ^1 (registry+http://[..])` in registry: registry+http://[..]
+        error: cannot get dependencies of `[email protected]`
 
         Caused by:
-            package not found in registry: baz ^1 (registry+http://[..])
+            0: failed to lookup for `baz ^1 (registry+http://[..])` in registry: registry+http://[..]
+            1: failed to lookup for `baz ^1 (registry+http://[..])` in registry: registry+http://[..]
+            2: package not found in registry: baz ^1 (registry+http://[..])
         "#});
 
     let expected = expect![["
@@ -245,11 +247,13 @@ fn missing_config_json() {
         .assert()
         .failure()
         .stdout_matches(indoc! {r#"
-        error: failed to lookup for `baz ^1 (registry+http://[..])` in registry: registry+http://[..]
+        error: cannot get dependencies of `[email protected]`
 
         Caused by:
-            0: failed to fetch registry config
-            1: HTTP status client error (404 Not Found) for url (http://[..]/config.json)
+            0: failed to lookup for `baz ^1 (registry+http://[..])` in registry: registry+http://[..]
+            1: failed to lookup for `baz ^1 (registry+http://[..])` in registry: registry+http://[..]
+            2: failed to fetch registry config
+            3: HTTP status client error (404 Not Found) for url (http://[..]/config.json)
         "#});
 
     let expected = expect![["

diff --git a/scarb/tests/local_registry.rs b/scarb/tests/local_registry.rs
@@ -63,10 +63,12 @@ fn not_found() {
         .assert()
         .failure()
         .stdout_matches(indoc! {r#"
-        error: failed to lookup for `baz ^1 (registry+file://[..])` in registry: registry+file://[..]
+        error: cannot get dependencies of `[email protected]`
 
         Caused by:
-            package not found in registry: baz ^1 (registry+file://[..])
+            0: failed to lookup for `baz ^1 (registry+file://[..])` in registry: registry+file://[..]
+            1: failed to lookup for `baz ^1 (registry+file://[..])` in registry: registry+file://[..]
+            2: package not found in registry: baz ^1 (registry+file://[..])
         "#});
 }
 
@@ -90,10 +92,12 @@ fn empty_registry() {
         .assert()
         .failure()
         .stdout_matches(indoc! {r#"
-        error: failed to lookup for `baz ^1 (registry+file://[..])` in registry: registry+file://[..]
+        error: cannot get dependencies of `[email protected]`
 
         Caused by:
-            package not found in registry: baz ^1 (registry+file://[..])
+            0: failed to lookup for `baz ^1 (registry+file://[..])` in registry: registry+file://[..]
+            1: failed to lookup for `baz ^1 (registry+file://[..])` in registry: registry+file://[..]
+            2: package not found in registry: baz ^1 (registry+file://[..])
         "#});
 }
 
@@ -117,10 +121,12 @@ fn url_pointing_to_file() {
         .assert()
         .failure()
         .stdout_matches(indoc! {r#"
-        error: failed to load source: registry+file://[..]
+        error: cannot get dependencies of `[email protected]`
 
         Caused by:
-            local registry path is not a directory: [..]
+            0: failed to load source: registry+file://[..]
+            1: failed to load source: registry+file://[..]
+            2: local registry path is not a directory: [..]
         "#});
 
     // Prevent the temp directory from being deleted until this point.