Auto merge of #7934 - ehuss:query-error-context, r=Eh2406

Provide extra context on a query failure. This adds error context when a query fails, primarily to tell you which parent package included the dependency that failed. For example, imagine deep within your dependency graph you have a `git` dependency, and it fails to download. The current error doesn't tell you where in the graph that `git` dependency was included. I also slightly tweaked the `failed to load source` error message. I felt like the existing wording could be misinterpreted that it was an error loading the dependency *for* the given package. I felt like there were multiple ways to interpret it, so I tried to simplify it to avoid that possibility.
rust-lang · Feb 27, 2020 · 7ba6e49 · 7ba6e49
2 parents ab2b2c0 + 1eca786
commit 7ba6e49
Show file tree

Hide file tree

Showing 14 changed files with 164 additions and 41 deletions.
diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs
@@ -256,8 +256,7 @@ impl<'cfg> PackageRegistry<'cfg> {
                 self.ensure_loaded(dep.source_id(), Kind::Normal)
                     .chain_err(|| {
                         anyhow::format_err!(
-                            "failed to load source for a dependency \
-                             on `{}`",
+                            "failed to load source for dependency `{}`",
                             dep.package_name()
                         )
                     })?;
@@ -517,8 +516,7 @@ impl<'cfg> Registry for PackageRegistry<'cfg> {
                 self.ensure_loaded(dep.source_id(), Kind::Normal)
                     .chain_err(|| {
                         anyhow::format_err!(
-                            "failed to load source for a dependency \
-                             on `{}`",
+                            "failed to load source for dependency `{}`",
                             dep.package_name()
                         )
                     })?;

diff --git a/src/cargo/core/resolver/dep_cache.rs b/src/cargo/core/resolver/dep_cache.rs
@@ -16,8 +16,10 @@ use std::rc::Rc;
 use log::debug;
 
 use crate::core::interning::InternedString;
+use crate::core::resolver::context::Context;
+use crate::core::resolver::errors::describe_path;
 use crate::core::{Dependency, FeatureValue, PackageId, PackageIdSpec, Registry, Summary};
-use crate::util::errors::CargoResult;
+use crate::util::errors::{CargoResult, CargoResultExt};
 
 use crate::core::resolver::types::{ConflictReason, DepInfo, FeaturesSet};
 use crate::core::resolver::{ActivateResult, ResolveOpts};
@@ -197,6 +199,7 @@ impl<'a> RegistryQueryer<'a> {
     /// next obvious question.
     pub fn build_deps(
         &mut self,
+        cx: &Context,
         parent: Option<PackageId>,
         candidate: &Summary,
         opts: &ResolveOpts,
@@ -220,7 +223,13 @@ impl<'a> RegistryQueryer<'a> {
         let mut deps = deps
             .into_iter()
             .map(|(dep, features)| {
-                let candidates = self.query(&dep)?;
+                let candidates = self.query(&dep).chain_err(|| {
+                    anyhow::format_err!(
+                        "failed to get `{}` as a dependency of {}",
+                        dep.package_name(),
+                        describe_path(&cx.parents.path_to_bottom(&candidate.package_id())),
+                    )
+                })?;
                 Ok((dep, candidates, features))
             })
             .collect::<CargoResult<Vec<DepInfo>>>()?;

diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs
@@ -656,7 +656,7 @@ fn activate(
 
     let now = Instant::now();
     let (used_features, deps) =
-        &*registry.build_deps(parent.map(|p| p.0.package_id()), &candidate, &opts)?;
+        &*registry.build_deps(cx, parent.map(|p| p.0.package_id()), &candidate, &opts)?;
 
     // Record what list of features is active for this package.
     if !used_features.is_empty() {

diff --git a/tests/testsuite/bad_config.rs b/tests/testsuite/bad_config.rs
@@ -368,7 +368,10 @@ fn bad_git_dependency() {
         .with_stderr(
             "\
 [UPDATING] git repository `file:///`
-[ERROR] failed to load source for a dependency on `foo`
+[ERROR] failed to get `foo` as a dependency of package `foo v0.0.0 [..]`
+
+Caused by:
+  failed to load source for dependency `foo`
 
 Caused by:
   Unable to update file:///
@@ -901,7 +904,10 @@ fn bad_source_config2() {
         .with_status(101)
         .with_stderr(
             "\
-error: failed to load source for a dependency on `bar`
+[ERROR] failed to get `bar` as a dependency of package `foo v0.0.0 [..]`
+
+Caused by:
+  failed to load source for dependency `bar`
 
 Caused by:
   Unable to update registry `https://[..]`
@@ -944,7 +950,10 @@ fn bad_source_config3() {
         .with_status(101)
         .with_stderr(
             "\
-error: failed to load source for a dependency on `bar`
+[ERROR] failed to get `bar` as a dependency of package `foo v0.0.0 [..]`
+
+Caused by:
+  failed to load source for dependency `bar`
 
 Caused by:
   Unable to update registry `https://[..]`
@@ -989,7 +998,10 @@ fn bad_source_config4() {
         .with_status(101)
         .with_stderr(
             "\
-error: failed to load source for a dependency on `bar`
+[ERROR] failed to get `bar` as a dependency of package `foo v0.0.0 ([..])`
+
+Caused by:
+  failed to load source for dependency `bar`
 
 Caused by:
   Unable to update registry `https://[..]`

diff --git a/tests/testsuite/cargo_features.rs b/tests/testsuite/cargo_features.rs
@@ -199,7 +199,10 @@ fn nightly_feature_requires_nightly_in_dep() {
         .with_status(101)
         .with_stderr(
             "\
-error: failed to load source for a dependency on `a`
+[ERROR] failed to get `a` as a dependency of package `b v0.0.1 ([..])`
+
+Caused by:
+  failed to load source for dependency `a`
 
 Caused by:
   Unable to update [..]

diff --git a/tests/testsuite/directory.rs b/tests/testsuite/directory.rs
@@ -652,7 +652,10 @@ fn git_override_requires_lockfile() {
         .with_status(101)
         .with_stderr(
             "\
-error: failed to load source for a dependency on `git`
+[ERROR] failed to get `git` as a dependency of package `foo v0.0.1 ([..])`
+
+Caused by:
+  failed to load source for dependency `git`
 
 Caused by:
   Unable to update [..]

diff --git a/tests/testsuite/git.rs b/tests/testsuite/git.rs
@@ -931,7 +931,10 @@ fn dep_with_bad_submodule() {
     let expected = format!(
         "\
 [UPDATING] git repository [..]
-[ERROR] failed to load source for a dependency on `dep1`
+[ERROR] failed to get `dep1` as a dependency of package `foo v0.5.0 [..]`
+
+Caused by:
+  failed to load source for dependency `dep1`
 
 Caused by:
   Unable to update {}
@@ -2382,20 +2385,24 @@ fn invalid_git_dependency_manifest() {
         .cargo("build")
         .with_status(101)
         .with_stderr(&format!(
-            "[UPDATING] git repository `{}`\n\
-             error: failed to load source for a dependency on `dep1`\n\
-             \n\
-             Caused by:\n  \
-             Unable to update {}\n\
-             \n\
-             Caused by:\n  \
-             failed to parse manifest at `[..]`\n\
-             \n\
-             Caused by:\n  \
-             could not parse input as TOML\n\
-             \n\
-             Caused by:\n  \
-             duplicate key: `categories` for key `project` at line 10 column 17",
+            "\
+[UPDATING] git repository `{}`
+[ERROR] failed to get `dep1` as a dependency of package `foo v0.5.0 ([..])`
+
+Caused by:
+  failed to load source for dependency `dep1`
+
+Caused by:
+  Unable to update {}
+
+Caused by:
+  failed to parse manifest at `[..]`
+
+Caused by:
+  could not parse input as TOML
+
+Caused by:
+  duplicate key: `categories` for key `project` at line 10 column 17",
             path2url(&git_root),
             path2url(&git_root),
         ))

diff --git a/tests/testsuite/git_auth.rs b/tests/testsuite/git_auth.rs
@@ -133,7 +133,10 @@ fn http_auth_offered() {
         .with_stderr_contains(&format!(
             "\
 [UPDATING] git repository `http://{addr}/foo/bar`
-[ERROR] failed to load source for a dependency on `bar`
+[ERROR] failed to get `bar` as a dependency of package `foo v0.0.1 [..]`
+
+Caused by:
+  failed to load source for dependency `bar`
 
 Caused by:
   Unable to update http://{addr}/foo/bar

diff --git a/tests/testsuite/local_registry.rs b/tests/testsuite/local_registry.rs
@@ -360,7 +360,10 @@ fn invalid_dir_bad() {
         .with_status(101)
         .with_stderr(
             "\
-[ERROR] failed to load source for a dependency on `bar`
+[ERROR] failed to get `bar` as a dependency of package `foo v0.0.1 [..]`
+
+Caused by:
+  failed to load source for dependency `bar`
 
 Caused by:
   Unable to update registry `https://[..]`

diff --git a/tests/testsuite/offline.rs b/tests/testsuite/offline.rs
@@ -270,7 +270,10 @@ fn cargo_compile_forbird_git_httpsrepo_offline() {
         .build();
 
     p.cargo("build --offline").with_status(101).with_stderr("\
-error: failed to load source for a dependency on `dep1`
+[ERROR] failed to get `dep1` as a dependency of package `foo v0.5.0 [..]`
+
+Caused by:
+  failed to load source for dependency `dep1`
 
 Caused by:
   Unable to update https://github.com/some_user/dep1.git

diff --git a/tests/testsuite/path.rs b/tests/testsuite/path.rs
@@ -521,7 +521,10 @@ fn error_message_for_missing_manifest() {
         .with_status(101)
         .with_stderr(
             "\
-[ERROR] failed to load source for a dependency on `bar`
+[ERROR] failed to get `bar` as a dependency of package `foo v0.5.0 [..]`
+
+Caused by:
+  failed to load source for dependency `bar`
 
 Caused by:
   Unable to update [CWD]/src/bar
@@ -1017,3 +1020,66 @@ fn workspace_produces_rlib() {
     assert!(p.root().join("target/debug/libtop.rlib").is_file());
     assert!(!p.root().join("target/debug/libfoo.rlib").is_file());
 }
+
+#[cargo_test]
+fn deep_path_error() {
+    // Test for an error loading a path deep in the dependency graph.
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+            [dependencies]
+            a = {path="a"}
+            "#,
+        )
+        .file("src/lib.rs", "")
+        .file(
+            "a/Cargo.toml",
+            r#"
+            [package]
+            name = "a"
+            version = "0.1.0"
+            [dependencies]
+            b = {path="../b"}
+           "#,
+        )
+        .file("a/src/lib.rs", "")
+        .file(
+            "b/Cargo.toml",
+            r#"
+            [package]
+            name = "b"
+            version = "0.1.0"
+            [dependencies]
+            c = {path="../c"}
+           "#,
+        )
+        .file("b/src/lib.rs", "")
+        .build();
+
+    p.cargo("check")
+        .with_status(101)
+        .with_stderr(
+            "\
+[ERROR] failed to get `c` as a dependency of package `b v0.1.0 [..]`
+    ... which is depended on by `a v0.1.0 [..]`
+    ... which is depended on by `foo v0.1.0 [..]`
+
+Caused by:
+  failed to load source for dependency `c`
+
+Caused by:
+  Unable to update [..]/foo/c
+
+Caused by:
+  failed to read `[..]/foo/c/Cargo.toml`
+
+Caused by:
+  [..]
+",
+        )
+        .run();
+}
diff --git a/tests/testsuite/registry.rs b/tests/testsuite/registry.rs
@@ -1558,7 +1558,10 @@ fn disallow_network() {
         .with_status(101)
         .with_stderr(
             "\
-error: failed to load source for a dependency on `foo`
+[ERROR] failed to get `foo` as a dependency of package `bar v0.5.0 [..]`
+
+Caused by:
+  failed to load source for dependency `foo`
 
 Caused by:
   Unable to update registry [..]

diff --git a/tests/testsuite/replace.rs b/tests/testsuite/replace.rs
@@ -544,7 +544,10 @@ fn override_wrong_name() {
             "\
 [UPDATING] [..] index
 [UPDATING] git repository [..]
-error: no matching package for override `[..]baz:0.1.0` found
+[ERROR] failed to get `baz` as a dependency of package `foo v0.0.1 ([..])`
+
+Caused by:
+  no matching package for override `[..]baz:0.1.0` found
 location searched: file://[..]
 version required: = 0.1.0
 ",
@@ -588,7 +591,10 @@ fn override_with_nothing() {
             "\
 [UPDATING] [..] index
 [UPDATING] git repository [..]
-[ERROR] failed to load source for a dependency on `bar`
+[ERROR] failed to get `bar` as a dependency of package `foo v0.0.1 ([..])`
+
+Caused by:
+  failed to load source for dependency `bar`
 
 Caused by:
   Unable to update file://[..]
@@ -671,7 +677,10 @@ fn multiple_specs() {
             "\
 [UPDATING] [..] index
 [UPDATING] git repository [..]
-error: overlapping replacement specifications found:
+[ERROR] failed to get `bar` as a dependency of package `foo v0.0.1 ([..])`
+
+Caused by:
+  overlapping replacement specifications found:
 
   * [..]
   * [..]

diff --git a/tests/testsuite/workspaces.rs b/tests/testsuite/workspaces.rs
@@ -2203,7 +2203,7 @@ fn ws_warn_path() {
 
 #[cargo_test]
 fn invalid_missing() {
-    // Warnings include path to manifest.
+    // Make sure errors are not suppressed with -q.
     let p = project()
         .file(
             "Cargo.toml",
@@ -2223,16 +2223,20 @@ fn invalid_missing() {
         .with_status(101)
         .with_stderr(
             "\
-error: [..]
+[ERROR] failed to get `x` as a dependency of package `foo v0.1.0 [..]`
 
 Caused by:
-  [..]
+  failed to load source for dependency `x`
 
 Caused by:
-  [..]
+  Unable to update [..]/foo/x
 
 Caused by:
-  [..]",
+  failed to read `[..]foo/x/Cargo.toml`
+
+Caused by:
+  [..]
+",
         )
         .run();
 }