Auto merge of #10172 - ehuss:doc-lib-before-bin, r=alexcrichton

Document lib before bin.

This changes it so that documenting a library is required to finish before documenting a binary. The issue is that the binary may have intra-doc links to the library. If they are documented concurrently, then the links will sometimes fail (since it is a race).   Or, if doing `cargo doc --bins`, then the library docs wouldn't exist at all.

Note that in the tests this introduces some more name collisions if you just run `cargo doc --bins` and there is a colliding library/binary name. There is some risk that someone might be trying to run the commands separately to get around the collision error, but I think it is unlikely.

Author: bors

src/cargo/core/compiler/unit_dependencies.rs

@@ -326,7 +326,7 @@ fn compute_deps(
     if unit.target.is_lib() && unit.mode != CompileMode::Doctest {
         return Ok(ret);
     }
-    ret.extend(maybe_lib(unit, state, unit_for)?);
+    ret.extend(maybe_lib(unit, state, unit_for, None)?);
 
     // If any integration tests/benches are being run, make sure that
     // binaries are built as well.
@@ -469,7 +469,10 @@ fn compute_deps_doc(
 
     // If we document a binary/example, we need the library available.
     if unit.target.is_bin() || unit.target.is_example() {
-        ret.extend(maybe_lib(unit, state, unit_for)?);
+        // build the lib
+        ret.extend(maybe_lib(unit, state, unit_for, None)?);
+        // and also the lib docs for intra-doc links
+        ret.extend(maybe_lib(unit, state, unit_for, Some(unit.mode))?);
     }
 
     // Add all units being scraped for examples as a dependency of Doc units.
@@ -497,13 +500,14 @@ fn maybe_lib(
     unit: &Unit,
     state: &mut State<'_, '_>,
     unit_for: UnitFor,
+    force_mode: Option<CompileMode>,
 ) -> CargoResult<Option<UnitDep>> {
     unit.pkg
         .targets()
         .iter()
         .find(|t| t.is_linkable())
         .map(|t| {
-            let mode = check_or_build_mode(unit.mode, t);
+            let mode = force_mode.unwrap_or_else(|| check_or_build_mode(unit.mode, t));
             let dep_unit_for = unit_for.with_dependency(unit, t);
             new_unit_dep(
                 state,

tests/testsuite/doc.rs

@@ -464,8 +464,17 @@ fn doc_lib_bin_same_name_documents_named_bin_when_requested() {
         .build();
 
     p.cargo("doc --bin foo")
-        .with_stderr(
+        // The checking/documenting lines are sometimes swapped since they run
+        // concurrently.
+        .with_stderr_unordered(
             "\
+warning: output filename collision.
+The bin target `foo` in package `foo v0.0.1 ([ROOT]/foo)` \
+has the same output filename as the lib target `foo` in package `foo v0.0.1 ([ROOT]/foo)`.
+Colliding filename is: [ROOT]/foo/target/doc/foo/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 v0.0.1 ([CWD])
 [DOCUMENTING] foo v0.0.1 ([CWD])
 [FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
@@ -500,8 +509,17 @@ fn doc_lib_bin_same_name_documents_bins_when_requested() {
         .build();
 
     p.cargo("doc --bins")
-        .with_stderr(
+        // The checking/documenting lines are sometimes swapped since they run
+        // concurrently.
+        .with_stderr_unordered(
             "\
+warning: output filename collision.
+The bin target `foo` in package `foo v0.0.1 ([ROOT]/foo)` \
+has the same output filename as the lib target `foo` in package `foo v0.0.1 ([ROOT]/foo)`.
+Colliding filename is: [ROOT]/foo/target/doc/foo/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 v0.0.1 ([CWD])
 [DOCUMENTING] foo v0.0.1 ([CWD])
 [FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
@@ -543,7 +561,9 @@ fn doc_lib_bin_example_same_name_documents_named_example_when_requested() {
         .build();
 
     p.cargo("doc --example ex1")
-        .with_stderr(
+        // The checking/documenting lines are sometimes swapped since they run
+        // concurrently.
+        .with_stderr_unordered(
             "\
 [CHECKING] foo v0.0.1 ([CWD])
 [DOCUMENTING] foo v0.0.1 ([CWD])
@@ -594,7 +614,9 @@ fn doc_lib_bin_example_same_name_documents_examples_when_requested() {
         .build();
 
     p.cargo("doc --examples")
-        .with_stderr(
+        // The checking/documenting lines are sometimes swapped since they run
+        // concurrently.
+        .with_stderr_unordered(
             "\
 [CHECKING] foo v0.0.1 ([CWD])
 [DOCUMENTING] foo v0.0.1 ([CWD])
@@ -2365,3 +2387,46 @@ fn scrape_examples_missing_flag() {
         .with_stderr("error: -Z rustdoc-scrape-examples must take [..] an argument")
         .run();
 }
+
+#[cargo_test]
+fn lib_before_bin() {
+    // Checks that the library is documented before the binary.
+    // Previously they were built concurrently, which can cause issues
+    // if the bin has intra-doc links to the lib.
+    let p = project()
+        .file(
+            "src/lib.rs",
+            r#"
+                /// Hi
+                pub fn abc() {}
+            "#,
+        )
+        .file(
+            "src/bin/somebin.rs",
+            r#"
+                //! See [`foo::abc`]
+                fn main() {}
+            "#,
+        )
+        .build();
+
+    // Run check first. This just helps ensure that the test clearly shows the
+    // order of the rustdoc commands.
+    p.cargo("check").run();
+
+    // The order of output here should be deterministic.
+    p.cargo("doc -v")
+        .with_stderr(
+            "\
+[DOCUMENTING] foo [..]
+[RUNNING] `rustdoc --crate-type lib --crate-name foo src/lib.rs [..]
+[RUNNING] `rustdoc --crate-type bin --crate-name somebin src/bin/somebin.rs [..]
+[FINISHED] [..]
+",
+        )
+        .run();
+
+    // And the link should exist.
+    let bin_html = p.read_file("target/doc/somebin/index.html");
+    assert!(bin_html.contains("../foo/fn.abc.html"));
+}