Properly memoize Docscrape units, update doc tests with scraping-by-default

Update to latest scrape-test flag

Author: willcrichton

crates/cargo-test-support/src/compare.rs

@@ -117,6 +117,7 @@ fn substitute_macros(input: &str) -> String {
         ("[NOTE]", "note:"),
         ("[HELP]", "help:"),
         ("[DOCUMENTING]", " Documenting"),
+        ("[SCRAPING]", "    Scraping"),
         ("[FRESH]", "       Fresh"),
         ("[UPDATING]", "    Updating"),
         ("[ADDING]", "      Adding"),

src/cargo/core/compiler/fingerprint.rs

@@ -1276,7 +1276,7 @@ fn calculate_normal(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Finger
 
     // Afterwards calculate our own fingerprint information.
     let target_root = target_root(cx);
-    let local = if unit.mode.is_doc() {
+    let local = if unit.mode.is_doc() || unit.mode.is_doc_scrape() {
         // rustdoc does not have dep-info files.
         let fingerprint = pkg_fingerprint(cx.bcx, &unit.pkg).with_context(|| {
             format!(
@@ -1303,7 +1303,7 @@ fn calculate_normal(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Finger
     // Fill out a bunch more information that we'll be tracking typically
     // hashed to take up less space on disk as we just need to know when things
     // change.
-    let extra_flags = if unit.mode.is_doc() {
+    let extra_flags = if unit.mode.is_doc() || unit.mode.is_doc_scrape() {
         cx.bcx.rustdocflags_args(unit)
     } else {
         cx.bcx.rustflags_args(unit)
@@ -1371,7 +1371,7 @@ fn calculate_run_custom_build(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoRes
 An I/O error happened. Please make sure you can access the file.
 
 By default, if your project contains a build script, cargo scans all files in
-it to determine whether a rebuild is needed. If you don't expect to access the 
+it to determine whether a rebuild is needed. If you don't expect to access the
 file, specify `rerun-if-changed` in your build script.
 See https://doc.rust-lang.org/cargo/reference/build-scripts.html#rerun-if-changed for more information.";
             pkg_fingerprint(cx.bcx, &unit.pkg).map_err(|err| {

src/cargo/core/compiler/job_queue.rs

@@ -1204,6 +1204,8 @@ impl<'cfg> DrainState<'cfg> {
                     config.shell().status("Documenting", &unit.pkg)?;
                 } else if unit.mode.is_doc_test() {
                     // Skip doc test.
+                } else if unit.mode.is_doc_scrape() {
+                    config.shell().status("Scraping", &unit.pkg)?;
                 } else {
                     self.compiled.insert(unit.pkg.package_id());
                     if unit.mode.is_check() {

src/cargo/core/compiler/mod.rs

@@ -52,7 +52,7 @@ pub use self::lto::Lto;
 use self::output_depinfo::output_depinfo;
 use self::unit_graph::UnitDep;
 use crate::core::compiler::future_incompat::FutureIncompatReport;
-pub use crate::core::compiler::unit::{Unit, UnitInterner};
+pub use crate::core::compiler::unit::{Unit, UnitInner, UnitInterner};
 use crate::core::manifest::TargetSourcePath;
 use crate::core::profiles::{PanicStrategy, Profile, Strip};
 use crate::core::{Feature, PackageId, Target};
@@ -659,15 +659,15 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Work> {
     rustdoc.arg("-C").arg(format!("metadata={}", metadata));
 
     let scrape_output_path = |unit: &Unit| -> CargoResult<PathBuf> {
-        let output_dir = cx.files().deps_dir(unit);
-        Ok(output_dir.join(format!("{}.examples", unit.buildkey())))
+        cx.outputs(unit)
+            .map(|outputs| outputs[0].path.to_path_buf())
     };
 
     if unit.mode.is_doc_scrape() {
         debug_assert!(cx.bcx.scrape_units.contains(unit));
 
         if unit.target.is_test() {
-            rustdoc.arg("--test");
+            rustdoc.arg("--scrape-tests");
         }
 
         rustdoc.arg("-Zunstable-options");

src/cargo/core/compiler/unit.rs

@@ -101,7 +101,7 @@ impl UnitInner {
 
 impl Unit {
     pub fn buildkey(&self) -> String {
-        format!("{}-{}", self.pkg.name(), short_hash(self))
+        format!("{}-{}", self.pkg.name(), short_hash(&*self.inner))
     }
 }
 

src/cargo/core/compiler/unit_dependencies.rs

@@ -712,14 +712,15 @@ fn compute_deps_doc(
     if state.ws.unit_needs_doc_scrape(unit) {
         for scrape_unit in state.scrape_units.iter() {
             deps_of(scrape_unit, state, unit_for)?;
-            ret.push(new_unit_dep(
+            ret.push(new_unit_dep_with_profile(
                 state,
                 scrape_unit,
                 &scrape_unit.pkg,
                 &scrape_unit.target,
                 unit_for,
                 scrape_unit.kind,
                 scrape_unit.mode,
+                scrape_unit.profile.clone(),
                 IS_NO_ARTIFACT_DEP,
             )?);
         }

src/cargo/ops/cargo_compile.rs

@@ -30,7 +30,9 @@ use crate::core::compiler::unit_dependencies::{build_unit_dependencies, IsArtifa
 use crate::core::compiler::unit_graph::{self, UnitDep, UnitGraph};
 use crate::core::compiler::{standard_lib, CrateType, TargetInfo};
 use crate::core::compiler::{BuildConfig, BuildContext, Compilation, Context};
-use crate::core::compiler::{CompileKind, CompileMode, CompileTarget, RustcTargetData, Unit};
+use crate::core::compiler::{
+    CompileKind, CompileMode, CompileTarget, RustcTargetData, Unit, UnitInner,
+};
 use crate::core::compiler::{DefaultExecutor, Executor, UnitInterner};
 use crate::core::profiles::{Profiles, UnitFor};
 use crate::core::resolver::features::{self, CliFeatures, FeaturesFor};
@@ -510,7 +512,7 @@ pub fn create_bcx<'a, 'cfg>(
         override_rustc_crate_types(&mut units, args, interner)?;
     }
 
-    let mut scrape_units = {
+    let mut scrape_units = if need_reverse_dependencies {
         let to_build_ids = resolve.specs_to_ids(&resolve_specs)?;
         let to_builds = pkg_set.get_many(to_build_ids)?;
 
@@ -530,6 +532,8 @@ pub fn create_bcx<'a, 'cfg>(
             &profiles,
             interner,
         )?
+    } else {
+        Vec::new()
     };
 
     let std_roots = if let Some(crates) = &config.cli_unstable().build_std {
@@ -1574,9 +1578,12 @@ fn rebuild_unit_graph_shared(
             traverse_and_share(interner, &mut memo, &mut result, &unit_graph, root, to_host)
         })
         .collect();
+    // If no unit in the unit graph ended up having scrape units attached as dependencies,
+    // then they won't have been discovered in traverse_and_share and hence won't be in
+    // memo. So we filter out missing scrape units.
     let new_scrape_units = scrape_units
         .iter()
-        .map(|unit| memo.get(unit).unwrap().clone())
+        .filter_map(|unit| memo.get(unit).cloned())
         .collect();
     (new_roots, new_scrape_units, result)
 }
@@ -1604,7 +1611,13 @@ fn traverse_and_share(
         .map(|dep| {
             let new_dep_unit =
                 traverse_and_share(interner, memo, new_graph, unit_graph, &dep.unit, to_host);
-            new_dep_unit.hash(&mut dep_hash);
+
+            // The default hash for a unit is the interned pointer, but we need dep_hash to
+            // be stable across Cargo sessions for Doscrape units to be memoized, so we
+            // take care to hash the contents of the unit here.
+            let inner: &UnitInner = &*new_dep_unit;
+            inner.hash(&mut dep_hash);
+
             UnitDep {
                 unit: new_dep_unit,
                 ..dep.clone()

tests/testsuite/build_script.rs

@@ -1317,6 +1317,8 @@ fn testing_and_such() {
     p.cargo("doc -v")
         .with_stderr(
             "\
+[SCRAPING] foo v0.5.0 ([CWD])
+[RUNNING] `rustdoc [..]`
 [DOCUMENTING] foo v0.5.0 ([CWD])
 [RUNNING] `rustdoc [..]`
 [FINISHED] dev [unoptimized + debuginfo] target(s) in [..]

tests/testsuite/collisions.rs

@@ -199,6 +199,7 @@ fn collision_doc_multiple_versions() {
 [CHECKING] bar v1.0.0
 [DOCUMENTING] bar v2.0.0
 [FINISHED] [..]
+[SCRAPING] foo v0.1.0 [..]
 [DOCUMENTING] foo v0.1.0 [..]
 ",
         )
@@ -378,6 +379,7 @@ fn collision_doc_profile_split() {
 [CHECKING] common v1.0.0
 [DOCUMENTING] common v1.0.0
 [DOCUMENTING] pm v0.1.0 [..]
+[SCRAPING] foo v0.1.0 [..]
 [DOCUMENTING] foo v0.1.0 [..]
 [FINISHED] [..]
 ",
@@ -424,6 +426,7 @@ the same path; see <https://github.com/rust-lang/cargo/issues/6313>.
 [DOCUMENTING] bar v1.0.0 [..]
 [DOCUMENTING] bar v1.0.0
 [CHECKING] bar v1.0.0
+[SCRAPING] foo v0.1.0 [..]
 [DOCUMENTING] foo v0.1.0 [..]
 [FINISHED] [..]
 ",
@@ -538,6 +541,7 @@ 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 [..]
+[SCRAPING] foo-macro v1.0.0 [..]
 [DOCUMENTING] foo-macro v1.0.0 [..]
 [DOCUMENTING] abc v1.0.0 [..]
 [FINISHED] [..]