From cd2e4e39120138721bbefebf5329ce6afbe91a04 Mon Sep 17 00:00:00 2001
From: Jacob Finkelman <jfinkelm@amazon.com>
Date: Tue, 14 Nov 2023 20:18:43 +0000
Subject: [PATCH 1/2] add a test

---
 src/cargo/util/graph.rs | 16 ++++++++++++++--
 tests/testsuite/test.rs | 28 ++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/src/cargo/util/graph.rs b/src/cargo/util/graph.rs
index 8c4a593eafd..6b56ae595b7 100644
--- a/src/cargo/util/graph.rs
+++ b/src/cargo/util/graph.rs
@@ -165,8 +165,12 @@ impl<'s, N: Eq + Ord + Clone + 's, E: Default + Clone + 's> Graph<N, E> {
                 ));
             }
             let last = result.last().unwrap().0;
-            // fixme: this may sometimes be wrong when there are cycles.
-            if !fn_edge(&self, last).next().is_none() {
+            let set: Vec<_> = result.iter().map(|(k, _)| k).collect();
+            if !fn_edge(&self, last)
+                .filter(|(e, _)| !set.contains(&e))
+                .next()
+                .is_none()
+            {
                 self.print_for_test();
                 unreachable!("The last element in the path should not have outgoing edges");
             }
@@ -188,6 +192,14 @@ fn path_to_case() {
     );
 }
 
+#[test]
+fn path_to_self() {
+    // Extracted from #12941
+    let mut new: Graph<i32, ()> = Graph::new();
+    new.link(0, 0);
+    assert_eq!(new.path_to_bottom(&0), vec![(&0, None)]);
+}
+
 impl<N: Eq + Ord + Clone, E: Default + Clone> Default for Graph<N, E> {
     fn default() -> Graph<N, E> {
         Graph::new()
diff --git a/tests/testsuite/test.rs b/tests/testsuite/test.rs
index 5f6528109be..e3c87d60995 100644
--- a/tests/testsuite/test.rs
+++ b/tests/testsuite/test.rs
@@ -3555,6 +3555,34 @@ fn cyclic_dev() {
     p.cargo("test --workspace").run();
 }
 
+#[cargo_test]
+fn cyclical_dep_with_missing_feature() {
+    // Checks for error handling when a cyclical dev-dependency specify a
+    // feature that doesn't exist.
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                [package]
+                name = "foo"
+                version = "0.1.0"
+
+                [dev-dependencies]
+                foo = { path = ".", features = ["missing"] }
+            "#,
+        )
+        .file("src/lib.rs", "")
+        .build();
+    p.cargo("check")
+        .with_status(101)
+        .with_stderr(
+            "thread 'main' panicked at src/cargo/util/graph.rs:149:20:
+the only path was a cycle, no dependency graph has this shape
+note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace",
+        )
+        .run();
+}
+
 #[cargo_test]
 fn publish_a_crate_without_tests() {
     Package::new("testless", "0.1.0")

From d19273c04d77323bdc3dbbece646d05ef29edb18 Mon Sep 17 00:00:00 2001
From: Jacob Finkelman <jfinkelm@amazon.com>
Date: Tue, 14 Nov 2023 20:18:43 +0000
Subject: [PATCH 2/2] If the only path is a loop then counted as the shortest
 path.

---
 src/cargo/util/graph.rs | 18 ++++++++++--------
 tests/testsuite/test.rs | 11 ++++++++---
 2 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/src/cargo/util/graph.rs b/src/cargo/util/graph.rs
index 6b56ae595b7..eed3ad4c1fd 100644
--- a/src/cargo/util/graph.rs
+++ b/src/cargo/util/graph.rs
@@ -128,30 +128,32 @@ impl<'s, N: Eq + Ord + Clone + 's, E: Default + Clone + 's> Graph<N, E> {
     {
         let mut back_link = BTreeMap::new();
         let mut queue = VecDeque::from([pkg]);
-        let mut bottom = None;
+        let mut last = pkg;
 
         while let Some(p) = queue.pop_front() {
-            bottom = Some(p);
+            last = p;
+            let mut out_edges = true;
             for (child, edge) in fn_edge(&self, p) {
-                bottom = None;
+                out_edges = false;
                 back_link.entry(child).or_insert_with(|| {
                     queue.push_back(child);
                     (p, edge)
                 });
             }
-            if bottom.is_some() {
+            if out_edges {
                 break;
             }
         }
 
         let mut result = Vec::new();
-        let mut next =
-            bottom.expect("the only path was a cycle, no dependency graph has this shape");
+        let mut next = last;
         while let Some((p, e)) = back_link.remove(&next) {
             result.push((next, Some(e)));
             next = p;
         }
-        result.push((next, None));
+        if result.iter().all(|(n, _)| n != &next) {
+            result.push((next, None));
+        }
         result.reverse();
         #[cfg(debug_assertions)]
         {
@@ -197,7 +199,7 @@ fn path_to_self() {
     // Extracted from #12941
     let mut new: Graph<i32, ()> = Graph::new();
     new.link(0, 0);
-    assert_eq!(new.path_to_bottom(&0), vec![(&0, None)]);
+    assert_eq!(new.path_to_bottom(&0), vec![(&0, Some(&()))]);
 }
 
 impl<N: Eq + Ord + Clone, E: Default + Clone> Default for Graph<N, E> {
diff --git a/tests/testsuite/test.rs b/tests/testsuite/test.rs
index e3c87d60995..71acb26fca3 100644
--- a/tests/testsuite/test.rs
+++ b/tests/testsuite/test.rs
@@ -3576,9 +3576,14 @@ fn cyclical_dep_with_missing_feature() {
     p.cargo("check")
         .with_status(101)
         .with_stderr(
-            "thread 'main' panicked at src/cargo/util/graph.rs:149:20:
-the only path was a cycle, no dependency graph has this shape
-note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace",
+            "error: failed to select a version for `foo`.
+    ... required by package `foo v0.1.0 ([..]/foo)`
+versions that meet the requirements `*` are: 0.1.0
+
+the package `foo` depends on `foo`, with features: `missing` but `foo` does not have these features.
+
+
+failed to select a version for `foo` which could resolve this conflict",
         )
         .run();
 }