Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Display formatted resolver errors #1455

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
43 changes: 40 additions & 3 deletions scarb/src/resolver/algorithm/mod.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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);

Expand All @@ -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<()> {
Expand Down
21 changes: 14 additions & 7 deletions scarb/src/resolver/algorithm/provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
})
})
}
Expand Down Expand Up @@ -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
Expand All @@ -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),
}
22 changes: 10 additions & 12 deletions scarb/src/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
"}),
)
}
Expand All @@ -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`"#),
)
}

Expand All @@ -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]`"#),
)
}

Expand All @@ -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`"#),
)
}

Expand All @@ -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]`"#),
)
}

Expand All @@ -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]`"#),
)
}

Expand All @@ -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]`"#),
)
}

Expand Down
5 changes: 4 additions & 1 deletion scarb/tests/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
12 changes: 8 additions & 4 deletions scarb/tests/git_source_network.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
"#});
});
}
Expand Down Expand Up @@ -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
"#});
});
}
14 changes: 9 additions & 5 deletions scarb/tests/http_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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![["
Expand Down Expand Up @@ -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![["
Expand Down
18 changes: 12 additions & 6 deletions scarb/tests/local_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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://[..])
"#});
}

Expand All @@ -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://[..])
"#});
}

Expand All @@ -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.
Expand Down
Loading