Improve error message for cyclic dependencies

crates/resolver-tests/tests/resolve.rs

@@ -1451,3 +1451,21 @@ fn conflict_store_more_then_one_match() {
     let reg = registry(input);
     let _ = resolve_and_validated(vec![dep("nA")], &reg, None);
 }
+
+#[test]
+fn cyclic_good_error_message() {
+    let input = vec![
+        pkg!(("A", "0.0.0") => [dep("C")]),
+        pkg!(("B", "0.0.0") => [dep("C")]),
+        pkg!(("C", "0.0.0") => [dep("A")]),
+    ];
+    let reg = registry(input);
+    let error = resolve(vec![dep("A"), dep("B")], &reg).unwrap_err();
+    println!("{}", error);
+    assert_eq!("\
+cyclic package dependency: package `A v0.0.0 (registry `https://example.com/`)` depends on itself. Cycle:
+package `A v0.0.0 (registry `https://example.com/`)`
+    ... which is depended on by `C v0.0.0 (registry `https://example.com/`)`
+    ... which is depended on by `A v0.0.0 (registry `https://example.com/`)`\
+", error.to_string());
+}

src/cargo/core/resolver/mod.rs

@@ -993,9 +993,11 @@ fn check_cycles(resolve: &Resolve) -> CargoResult<()> {
     let mut all_packages: Vec<_> = resolve.iter().collect();
     all_packages.sort_unstable();
     let mut checked = HashSet::new();
+    let mut path = Vec::new();
+    let mut visited = HashSet::new();
     for pkg in all_packages {
         if !checked.contains(&pkg) {
-            visit(resolve, pkg, &mut HashSet::new(), &mut checked)?
+            visit(resolve, pkg, &mut visited, &mut path, &mut checked)?
         }
     }
     return Ok(());
@@ -1004,14 +1006,16 @@ fn check_cycles(resolve: &Resolve) -> CargoResult<()> {
         resolve: &Resolve,
         id: PackageId,
         visited: &mut HashSet<PackageId>,
+        path: &mut Vec<PackageId>,
         checked: &mut HashSet<PackageId>,
     ) -> CargoResult<()> {
+        path.push(id);
         // See if we visited ourselves
         if !visited.insert(id) {
             failure::bail!(
                 "cyclic package dependency: package `{}` depends on itself. Cycle:\n{}",
                 id,
-                errors::describe_path(&resolve.path_to_top(&id))
+                errors::describe_path(&path.iter().rev().collect::<Vec<_>>()),
             );
         }
 
@@ -1023,23 +1027,25 @@ fn check_cycles(resolve: &Resolve) -> CargoResult<()> {
         // visitation list as we can't induce a cycle through transitive
         // dependencies.
         if checked.insert(id) {
+            let mut empty_set = HashSet::new();
+            let mut empty_vec = Vec::new();
             for (dep, listings) in resolve.deps_not_replaced(id) {
                 let is_transitive = listings.iter().any(|d| d.is_transitive());
-                let mut empty = HashSet::new();
-                let visited = if is_transitive {
-                    &mut *visited
+                let (visited, path) = if is_transitive {
+                    (&mut *visited, &mut *path)
                 } else {
-                    &mut empty
+                    (&mut empty_set, &mut empty_vec)
                 };
-                visit(resolve, dep, visited, checked)?;
+                visit(resolve, dep, visited, path, checked)?;
 
                 if let Some(id) = resolve.replacement(dep) {
-                    visit(resolve, id, visited, checked)?;
+                    visit(resolve, id, visited, path, checked)?;
                 }
             }
         }
 
         // Ok, we're done, no longer visiting our node any more
+        path.pop();
         visited.remove(&id);
         Ok(())
     }

tests/testsuite/build.rs

@@ -1487,7 +1487,8 @@ fn self_dependency() {
         .with_stderr(
             "\
 [ERROR] cyclic package dependency: package `test v0.0.0 ([CWD])` depends on itself. Cycle:
-package `test v0.0.0 ([CWD])`",
+package `test v0.0.0 ([CWD])`
+    ... which is depended on by `test v0.0.0 ([..])`",
         )
         .run();
 }
@@ -2613,7 +2614,8 @@ fn cyclic_deps_rejected() {
         .with_stderr(
 "[ERROR] cyclic package dependency: package `a v0.0.1 ([CWD]/a)` depends on itself. Cycle:
 package `a v0.0.1 ([CWD]/a)`
-    ... which is depended on by `foo v0.0.1 ([CWD])`",
+    ... which is depended on by `foo v0.0.1 ([CWD])`
+    ... which is depended on by `a v0.0.1 ([..])`",
         ).run();
 }
 

tests/testsuite/patch.rs

@@ -1187,6 +1187,7 @@ fn cycle() {
 error: cyclic package dependency: [..]
 package `[..]`
     ... which is depended on by `[..]`
+    ... which is depended on by `[..]`
 ",
         )
         .run();