From e50292a15354a4320898f92cace5a4803f52ca65 Mon Sep 17 00:00:00 2001
From: Eric Huss <eric@huss.org>
Date: Tue, 16 Mar 2021 10:32:08 -0700
Subject: [PATCH] Fix doc duplicate removal of root units.

---
 src/cargo/ops/cargo_compile.rs | 20 +++++------
 tests/testsuite/collisions.rs  | 66 ++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+), 10 deletions(-)

diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs
index dd15a970a18..49027a01a26 100644
--- a/src/cargo/ops/cargo_compile.rs
+++ b/src/cargo/ops/cargo_compile.rs
@@ -1558,8 +1558,11 @@ fn remove_duplicate_doc(
     // Keep track of units to remove so that they can be efficiently removed
     // from the unit_deps.
     let mut removed_units: HashSet<Unit> = HashSet::new();
-    let mut remove = |units: Vec<Unit>, reason: &str| {
-        for unit in units {
+    let mut remove = |units: Vec<Unit>, reason: &str, cb: &dyn Fn(&Unit) -> bool| -> Vec<Unit> {
+        let (to_remove, remaining_units): (Vec<Unit>, Vec<Unit>) = units
+            .into_iter()
+            .partition(|unit| cb(unit) && !root_units.contains(unit));
+        for unit in to_remove {
             log::debug!(
                 "removing duplicate doc due to {} for package {} target `{}`",
                 reason,
@@ -1569,6 +1572,7 @@ fn remove_duplicate_doc(
             unit_graph.remove(&unit);
             removed_units.insert(unit);
         }
+        remaining_units
     };
     // Iterate over the duplicates and try to remove them from unit_graph.
     for (_crate_name, mut units) in all_docs {
@@ -1581,14 +1585,11 @@ fn remove_duplicate_doc(
             .iter()
             .all(CompileKind::is_host)
         {
-            let (to_remove, remaining_units): (Vec<Unit>, Vec<Unit>) =
-                units.into_iter().partition(|unit| unit.kind.is_host());
             // Note these duplicates may not be real duplicates, since they
             // might get merged in rebuild_unit_graph_shared. Either way, it
             // shouldn't hurt to remove them early (although the report in the
             // log might be confusing).
-            remove(to_remove, "host/target merger");
-            units = remaining_units;
+            units = remove(units, "host/target merger", &|unit| unit.kind.is_host());
             if units.len() == 1 {
                 continue;
             }
@@ -1610,10 +1611,9 @@ fn remove_duplicate_doc(
                 units.sort_by(|a, b| a.pkg.version().partial_cmp(b.pkg.version()).unwrap());
                 // Remove any entries with version < newest.
                 let newest_version = units.last().unwrap().pkg.version().clone();
-                let (to_remove, keep_units): (Vec<Unit>, Vec<Unit>) = units
-                    .into_iter()
-                    .partition(|unit| unit.pkg.version() < &newest_version);
-                remove(to_remove, "older version");
+                let keep_units = remove(units, "older version", &|unit| {
+                    unit.pkg.version() < &newest_version
+                });
                 remaining_units.extend(keep_units);
             } else {
                 remaining_units.extend(units);
diff --git a/tests/testsuite/collisions.rs b/tests/testsuite/collisions.rs
index d7deebc175c..2721746d6f6 100644
--- a/tests/testsuite/collisions.rs
+++ b/tests/testsuite/collisions.rs
@@ -478,3 +478,69 @@ fn collision_doc_target() {
         )
         .run();
 }
+
+#[cargo_test]
+fn collision_with_root() {
+    // Check for a doc collision between a root package and a dependency.
+    // In this case, `foo-macro` comes from both the workspace and crates.io.
+    // This checks that the duplicate correction code doesn't choke on this
+    // by removing the root unit.
+    Package::new("foo-macro", "1.0.0").publish();
+
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                [workspace]
+                members = ["abc", "foo-macro"]
+            "#,
+        )
+        .file(
+            "abc/Cargo.toml",
+            r#"
+                [package]
+                name = "abc"
+                version = "1.0.0"
+
+                [dependencies]
+                foo-macro = "1.0"
+            "#,
+        )
+        .file("abc/src/lib.rs", "")
+        .file(
+            "foo-macro/Cargo.toml",
+            r#"
+                [package]
+                name = "foo-macro"
+                version = "1.0.0"
+
+                [lib]
+                proc-macro = true
+
+                [dependencies]
+                abc = {path="../abc"}
+            "#,
+        )
+        .file("foo-macro/src/lib.rs", "")
+        .build();
+
+    p.cargo("doc")
+        .with_stderr_unordered("\
+[UPDATING] [..]
+[DOWNLOADING] crates ...
+[DOWNLOADED] foo-macro v1.0.0 [..]
+warning: output filename collision.
+The lib target `foo-macro` in package `foo-macro v1.0.0` has the same output filename as the lib target `foo-macro` in package `foo-macro v1.0.0 [..]`.
+Colliding filename is: [CWD]/target/doc/foo_macro/index.html
+The targets should have unique names.
+This is a known bug where multiple crates with the same name use
+the same path; see <https://github.com/rust-lang/cargo/issues/6313>.
+[CHECKING] foo-macro v1.0.0
+[DOCUMENTING] foo-macro v1.0.0
+[CHECKING] abc v1.0.0 [..]
+[DOCUMENTING] foo-macro v1.0.0 [..]
+[DOCUMENTING] abc v1.0.0 [..]
+[FINISHED] [..]
+")
+        .run();
+}