Remove some doc collisions.

There are some cases where `cargo doc` will try to document two things
with the same crate_name. This attempts to automatically remove some of
those duplicates based on some rules:

- Prefers dependencies for the target over dependencies for the host
  (such as proc-macros).
- Prefers the "newest" version if it comes from the same source.

There are still plenty of situations where there can be collisions, but
I'm uncertain on the best way to handle those.

Author: ehuss

src/cargo/ops/cargo_compile.rs

@@ -36,10 +36,11 @@ use crate::core::profiles::{Profiles, UnitFor};
 use crate::core::resolver::features::{self, FeaturesFor, RequestedFeatures};
 use crate::core::resolver::{HasDevUnits, Resolve, ResolveOpts};
 use crate::core::{FeatureValue, Package, PackageSet, Shell, Summary, Target};
-use crate::core::{PackageId, PackageIdSpec, TargetKind, Workspace};
+use crate::core::{PackageId, PackageIdSpec, SourceId, TargetKind, Workspace};
 use crate::ops;
 use crate::ops::resolve::WorkspaceResolve;
 use crate::util::config::Config;
+use crate::util::interning::InternedString;
 use crate::util::restricted_names::is_glob_pattern;
 use crate::util::{closest_msg, profile, CargoResult, StableHasher};
 
@@ -503,6 +504,12 @@ pub fn create_bcx<'a, 'cfg>(
         interner,
     )?;
 
+    // TODO: In theory, Cargo should also dedupe the roots, but I'm uncertain
+    // what heuristics to use in that case.
+    if build_config.mode == (CompileMode::Doc { deps: true }) {
+        remove_duplicate_doc(build_config, &mut unit_graph);
+    }
+
     if build_config
         .requested_kinds
         .iter()
@@ -1455,3 +1462,108 @@ fn opt_patterns_and_names(
     }
     Ok((opt_patterns, opt_names))
 }
+
+/// Removes duplicate CompileMode::Doc units that would cause problems with
+/// filename collisions.
+///
+/// Rustdoc only separates units by crate name in the file directory
+/// structure. If any two units with the same crate name exist, this would
+/// cause a filename collision, causing different rustdoc invocations to stomp
+/// on one another's files.
+///
+/// Unfortunately this does not remove all duplicates, as some of them are
+/// either user error, or difficult to remove. Cases that I can think of:
+///
+/// - Same target name in different packages. See the `collision_doc` test.
+/// - Different sources. See `collision_doc_sources` test.
+///
+/// Ideally this would not be necessary.
+fn remove_duplicate_doc(build_config: &BuildConfig, unit_graph: &mut UnitGraph) {
+    // NOTE: There is some risk that this can introduce problems because it
+    // may create orphans in the unit graph (parts of the tree get detached
+    // from the roots). I currently can't think of any ways this will cause a
+    // problem because all other parts of Cargo traverse the graph starting
+    // from the roots. Perhaps this should scan for detached units and remove
+    // them too?
+    //
+    // First, create a mapping of crate_name -> Unit so we can see where the
+    // duplicates are.
+    let mut all_docs: HashMap<String, Vec<Unit>> = HashMap::new();
+    for unit in unit_graph.keys() {
+        if unit.mode.is_doc() {
+            all_docs
+                .entry(unit.target.crate_name())
+                .or_default()
+                .push(unit.clone());
+        }
+    }
+    let mut remove = |units: Vec<Unit>, reason: &str| {
+        for unit in &units {
+            log::debug!(
+                "removing duplicate doc due to {} for package {} target `{}`",
+                reason,
+                unit.pkg,
+                unit.target.name()
+            );
+            unit_graph.remove(unit);
+        }
+        for unit_deps in unit_graph.values_mut() {
+            unit_deps.retain(|unit_dep| !units.iter().any(|unit| *unit == unit_dep.unit));
+        }
+    };
+    // Iterate over the duplicates and try to remove them from unit_graph.
+    for (_crate_name, mut units) in all_docs {
+        if units.len() == 1 {
+            continue;
+        }
+        // Prefer target over host if --target was not specified.
+        if build_config
+            .requested_kinds
+            .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;
+            if units.len() == 1 {
+                continue;
+            }
+        }
+        // Prefer newer versions over older.
+        let mut source_map: HashMap<(InternedString, SourceId, CompileKind), Vec<Unit>> =
+            HashMap::new();
+        for unit in units {
+            let pkg_id = unit.pkg.package_id();
+            // Note, this does not detect duplicates from different sources.
+            source_map
+                .entry((pkg_id.name(), pkg_id.source_id(), unit.kind))
+                .or_default()
+                .push(unit);
+        }
+        let mut remaining_units = Vec::new();
+        for (_key, mut units) in source_map {
+            if units.len() > 1 {
+                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");
+                remaining_units.extend(keep_units);
+            } else {
+                remaining_units.extend(units);
+            }
+        }
+        if remaining_units.len() == 1 {
+            continue;
+        }
+        // Are there other heuristics to remove duplicates that would make
+        // sense? Maybe prefer path sources over all others?
+    }
+}