diff --git a/src/cargo/core/compiler/compilation.rs b/src/cargo/core/compiler/compilation.rs index 83cdda712..e0b9282af 100644 --- a/src/cargo/core/compiler/compilation.rs +++ b/src/cargo/core/compiler/compilation.rs @@ -51,6 +51,9 @@ pub struct Compilation<'cfg> { /// An array of all cdylibs created. pub cdylibs: Vec, + /// The crate names of the root units specified on the command-line. + pub root_crate_names: Vec, + /// All directories for the output of native build commands. /// /// This is currently used to drive some entries which are added to the @@ -136,6 +139,7 @@ impl<'cfg> Compilation<'cfg> { tests: Vec::new(), binaries: Vec::new(), cdylibs: Vec::new(), + root_crate_names: Vec::new(), extra_env: HashMap::new(), to_doc_test: Vec::new(), config: bcx.config, diff --git a/src/cargo/core/compiler/context/mod.rs b/src/cargo/core/compiler/context/mod.rs index 1b3e97aa8..cea171c7d 100644 --- a/src/cargo/core/compiler/context/mod.rs +++ b/src/cargo/core/compiler/context/mod.rs @@ -2,7 +2,7 @@ use std::collections::{BTreeSet, HashMap, HashSet}; use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex}; -use anyhow::Context as _; +use anyhow::{bail, Context as _}; use filetime::FileTime; use jobserver::Client; @@ -309,6 +309,9 @@ impl<'a, 'cfg> Context<'a, 'cfg> { } self.primary_packages .extend(self.bcx.roots.iter().map(|u| u.pkg.package_id())); + self.compilation + .root_crate_names + .extend(self.bcx.roots.iter().map(|u| u.target.crate_name())); self.record_units_requiring_metadata(); @@ -480,6 +483,22 @@ impl<'a, 'cfg> Context<'a, 'cfg> { } }; + fn doc_collision_error(unit: &Unit, other_unit: &Unit) -> CargoResult<()> { + bail!( + "document output filename collision\n\ + The {} `{}` in package `{}` has the same name as the {} `{}` in package `{}`.\n\ + Only one may be documented at once since they output to the same path.\n\ + Consider documenting only one, renaming one, \ + or marking one with `doc = false` in Cargo.toml.", + unit.target.kind().description(), + unit.target.name(), + unit.pkg, + other_unit.target.kind().description(), + other_unit.target.name(), + other_unit.pkg, + ); + } + let mut keys = self .bcx .unit_graph @@ -494,6 +513,11 @@ impl<'a, 'cfg> Context<'a, 'cfg> { if unit.mode.is_doc() { // See https://github.com/rust-lang/rust/issues/56169 // and https://github.com/rust-lang/rust/issues/61378 + if self.is_primary_package(unit) { + // This has been an error since before 1.0, so it + // is not a warning like the other situations. + doc_collision_error(unit, other_unit)?; + } report_collision(unit, other_unit, &output.path, rustdoc_suggestion)?; } else { report_collision(unit, other_unit, &output.path, suggestion)?; diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 7249af86a..f4700c7c4 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -1092,6 +1092,9 @@ fn generate_targets( // It is computed by the set of enabled features for the package plus // every enabled feature of every enabled dependency. let mut features_map = HashMap::new(); + // This needs to be a set to de-duplicate units. Due to the way the + // targets are filtered, it is possible to have duplicate proposals for + // the same thing. let mut units = HashSet::new(); for Proposal { pkg, @@ -1136,7 +1139,10 @@ fn generate_targets( } // else, silently skip target. } - Ok(units.into_iter().collect()) + let mut units: Vec<_> = units.into_iter().collect(); + // Keep the roots in a consistent order, which helps with checking test output. + units.sort_unstable(); + Ok(units) } /// Warns if a target's required-features references a feature that doesn't exist. diff --git a/src/cargo/ops/cargo_doc.rs b/src/cargo/ops/cargo_doc.rs index 6477a4d6f..640f2d9e3 100644 --- a/src/cargo/ops/cargo_doc.rs +++ b/src/cargo/ops/cargo_doc.rs @@ -1,12 +1,11 @@ -use crate::core::resolver::HasDevUnits; use crate::core::{Shell, Workspace}; use crate::ops; +use crate::util::config::PathAndArgs; use crate::util::CargoResult; -use crate::{core::compiler::RustcTargetData, util::config::PathAndArgs}; use serde::Deserialize; use std::path::Path; +use std::path::PathBuf; use std::process::Command; -use std::{collections::HashMap, path::PathBuf}; /// Strongly typed options for the `cargo doc` command. #[derive(Debug)] @@ -26,64 +25,11 @@ struct CargoDocConfig { /// Main method for `cargo doc`. pub fn doc(ws: &Workspace<'_>, options: &DocOptions) -> CargoResult<()> { - let specs = options.compile_opts.spec.to_package_id_specs(ws)?; - let target_data = RustcTargetData::new(ws, &options.compile_opts.build_config.requested_kinds)?; - let ws_resolve = ops::resolve_ws_with_opts( - ws, - &target_data, - &options.compile_opts.build_config.requested_kinds, - &options.compile_opts.cli_features, - &specs, - HasDevUnits::No, - crate::core::resolver::features::ForceAllTargets::No, - )?; - - let ids = ws_resolve.targeted_resolve.specs_to_ids(&specs)?; - let pkgs = ws_resolve.pkg_set.get_many(ids)?; - - let mut lib_names = HashMap::new(); - let mut bin_names = HashMap::new(); - let mut names = Vec::new(); - for package in &pkgs { - for target in package.targets().iter().filter(|t| t.documented()) { - if target.is_lib() { - if let Some(prev) = lib_names.insert(target.crate_name(), package) { - anyhow::bail!( - "The library `{}` is specified by packages `{}` and \ - `{}` but can only be documented once. Consider renaming \ - or marking one of the targets as `doc = false`.", - target.crate_name(), - prev, - package - ); - } - } else if let Some(prev) = bin_names.insert(target.crate_name(), package) { - anyhow::bail!( - "The binary `{}` is specified by packages `{}` and \ - `{}` but can be documented only once. Consider renaming \ - or marking one of the targets as `doc = false`.", - target.crate_name(), - prev, - package - ); - } - names.push(target.crate_name()); - } - } - - let open_kind = if options.open_result { - Some(options.compile_opts.build_config.single_requested_kind()?) - } else { - None - }; - let compilation = ops::compile(ws, &options.compile_opts)?; - if let Some(kind) = open_kind { - let name = match names.first() { - Some(s) => s.to_string(), - None => return Ok(()), - }; + if options.open_result { + let name = &compilation.root_crate_names[0]; + let kind = options.compile_opts.build_config.single_requested_kind()?; let path = compilation.root_output[&kind] .with_file_name("doc") .join(&name) diff --git a/tests/testsuite/doc.rs b/tests/testsuite/doc.rs index 5b039446f..1dcf7735b 100644 --- a/tests/testsuite/doc.rs +++ b/tests/testsuite/doc.rs @@ -223,9 +223,15 @@ fn doc_multiple_targets_same_name_lib() { p.cargo("doc --workspace") .with_status(101) - .with_stderr_contains("[..] library `foo_lib` is specified [..]") - .with_stderr_contains("[..] `foo v0.1.0[..]` [..]") - .with_stderr_contains("[..] `bar v0.1.0[..]` [..]") + .with_stderr( + "\ +error: document output filename collision +The lib `foo_lib` in package `foo v0.1.0 ([ROOT]/foo/foo)` has the same name as \ +the lib `foo_lib` in package `bar v0.1.0 ([ROOT]/foo/bar)`. +Only one may be documented at once since they output to the same path. +Consider documenting only one, renaming one, or marking one with `doc = false` in Cargo.toml. +", + ) .run(); } @@ -265,13 +271,17 @@ fn doc_multiple_targets_same_name() { .build(); p.cargo("doc --workspace") - .with_stderr_contains("[DOCUMENTING] foo v0.1.0 ([CWD]/foo)") - .with_stderr_contains("[DOCUMENTING] bar v0.1.0 ([CWD]/bar)") - .with_stderr_contains("[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]") + .with_status(101) + .with_stderr( + "\ +error: document output filename collision +The bin `foo_lib` in package `foo v0.1.0 ([ROOT]/foo/foo)` has the same name as \ +the lib `foo_lib` in package `bar v0.1.0 ([ROOT]/foo/bar)`. +Only one may be documented at once since they output to the same path. +Consider documenting only one, renaming one, or marking one with `doc = false` in Cargo.toml. +", + ) .run(); - assert!(p.root().join("target/doc").is_dir()); - let doc_file = p.root().join("target/doc/foo_lib/index.html"); - assert!(doc_file.is_file()); } #[cargo_test] @@ -290,29 +300,31 @@ fn doc_multiple_targets_same_name_bin() { [package] name = "foo" version = "0.1.0" - [[bin]] - name = "foo-cli" "#, ) - .file("foo/src/foo-cli.rs", "") + .file("foo/src/bin/foo-cli.rs", "") .file( "bar/Cargo.toml", r#" [package] name = "bar" version = "0.1.0" - [[bin]] - name = "foo-cli" "#, ) - .file("bar/src/foo-cli.rs", "") + .file("bar/src/bin/foo-cli.rs", "") .build(); p.cargo("doc --workspace") .with_status(101) - .with_stderr_contains("[..] binary `foo_cli` is specified [..]") - .with_stderr_contains("[..] `foo v0.1.0[..]` [..]") - .with_stderr_contains("[..] `bar v0.1.0[..]` [..]") + .with_stderr( + "\ +error: document output filename collision +The bin `foo-cli` in package `foo v0.1.0 ([ROOT]/foo/foo)` has the same name as \ +the bin `foo-cli` in package `bar v0.1.0 ([ROOT]/foo/bar)`. +Only one may be documented at once since they output to the same path. +Consider documenting only one, renaming one, or marking one with `doc = false` in Cargo.toml. +", + ) .run(); } @@ -1152,7 +1164,7 @@ fn doc_workspace_open_help_message() { .env("BROWSER", "echo") .with_stderr_contains("[..] Documenting bar v0.1.0 ([..])") .with_stderr_contains("[..] Documenting foo v0.1.0 ([..])") - .with_stderr_contains("[..] Opening [..]/foo/index.html") + .with_stderr_contains("[..] Opening [..]/bar/index.html") .run(); } @@ -1378,7 +1390,7 @@ fn doc_private_ws() { .file("a/src/lib.rs", "fn p() {}") .file("b/Cargo.toml", &basic_manifest("b", "0.0.1")) .file("b/src/lib.rs", "fn p2() {}") - .file("b/src/main.rs", "fn main() {}") + .file("b/src/bin/b-cli.rs", "fn main() {}") .build(); p.cargo("doc --workspace --bins --lib --document-private-items -v") .with_stderr_contains( @@ -1388,7 +1400,7 @@ fn doc_private_ws() { "[RUNNING] `rustdoc [..] b/src/lib.rs [..]--document-private-items[..]", ) .with_stderr_contains( - "[RUNNING] `rustdoc [..] b/src/main.rs [..]--document-private-items[..]", + "[RUNNING] `rustdoc [..] b/src/bin/b-cli.rs [..]--document-private-items[..]", ) .run(); }