Auto merge of #9404 - ehuss:rustdoc-fingerprint-remove-dir, r=alexcrichton

Some changes to rustdoc fingerprint checking.

#8640 introduced a check which deletes the `doc` directory if cargo detects it has stale contents from a different toolchain version. Rustdoc has some shared files (js and css for example) that can get corrupted between versions. Unfortunately that caused some problems with rustbuild which does a few unusual things. Rustbuild will:

* Create the `doc` directory before running `cargo doc` and places a `.stamp` file inside it.
* Creates symlinks of the `doc` directory so that they can be shared across different target directories (in particular, between rustc and rustdoc).

In order to address these issues, this PR does several things:

* Adds `-Z skip-rustdoc-fingerprint` to disable the `doc` clearing behavior.
* Don't delete the `doc` directory if the rustdoc fingerprint is missing. This is intended to help with the scenario where the user creates a `doc` directory ahead of time with pre-existing contents before the first build. The downside is that cargo will not be able to protect against switching from pre-1.53 to post-1.53.
* Don't delete the `doc` directory itself (just its contents). This should help if the user created the `doc` directory as a symlink to somewhere else.
* Don't delete hidden files in the `doc` directory. This isn't something that rustdoc creates.

Only the `-Z` change is needed for rustbuild. The others I figured I'd include just to be on the safe side in case there are other users doing unusual things (and I had already written them thinking they would work for rustbuild). Hopefully the rustbuild `.stamp` mechanism will be enough protection there.

Fixes #9336

Author: bors

src/cargo/core/compiler/build_context/target_info.rs

@@ -771,27 +771,6 @@ pub struct RustDocFingerprint {
 }
 
 impl RustDocFingerprint {
-    /// Read the `RustDocFingerprint` info from the fingerprint file.
-    fn read(cx: &Context<'_, '_>) -> CargoResult<Self> {
-        let rustdoc_data = paths::read(&cx.files().host_root().join(".rustdoc_fingerprint.json"))?;
-        serde_json::from_str(&rustdoc_data).map_err(|e| anyhow::anyhow!("{:?}", e))
-    }
-
-    /// Write the `RustDocFingerprint` info into the fingerprint file.
-    fn write<'a, 'cfg>(&self, cx: &Context<'a, 'cfg>) -> CargoResult<()> {
-        paths::write(
-            &cx.files().host_root().join(".rustdoc_fingerprint.json"),
-            serde_json::to_string(&self)?.as_bytes(),
-        )
-    }
-
-    fn remove_doc_dirs(doc_dirs: &[&Path]) -> CargoResult<()> {
-        doc_dirs
-            .iter()
-            .filter(|path| path.exists())
-            .try_for_each(|path| paths::remove_dir_all(&path))
-    }
-
     /// This function checks whether the latest version of `Rustc` used to compile this
     /// `Workspace`'s docs was the same as the one is currently being used in this `cargo doc`
     /// call.
@@ -801,38 +780,81 @@ impl RustDocFingerprint {
     /// versions of the `js/html/css` files that `rustdoc` autogenerates which do not have
     /// any versioning.
     pub fn check_rustdoc_fingerprint(cx: &Context<'_, '_>) -> CargoResult<()> {
+        if cx.bcx.config.cli_unstable().skip_rustdoc_fingerprint {
+            return Ok(());
+        }
         let actual_rustdoc_target_data = RustDocFingerprint {
             rustc_vv: cx.bcx.rustc().verbose_version.clone(),
         };
 
-        // Collect all of the target doc paths for which the docs need to be compiled for.
-        let doc_dirs: Vec<&Path> = cx
-            .bcx
+        let fingerprint_path = cx.files().host_root().join(".rustdoc_fingerprint.json");
+        let write_fingerprint = || -> CargoResult<()> {
+            paths::write(
+                &fingerprint_path,
+                serde_json::to_string(&actual_rustdoc_target_data)?,
+            )
+        };
+        let rustdoc_data = match paths::read(&fingerprint_path) {
+            Ok(rustdoc_data) => rustdoc_data,
+            // If the fingerprint does not exist, do not clear out the doc
+            // directories. Otherwise this ran into problems where projects
+            // like rustbuild were creating the doc directory before running
+            // `cargo doc` in a way that deleting it would break it.
+            Err(_) => return write_fingerprint(),
+        };
+        match serde_json::from_str::<RustDocFingerprint>(&rustdoc_data) {
+            Ok(fingerprint) => {
+                if fingerprint.rustc_vv == actual_rustdoc_target_data.rustc_vv {
+                    return Ok(());
+                } else {
+                    log::debug!(
+                        "doc fingerprint changed:\noriginal:\n{}\nnew:\n{}",
+                        fingerprint.rustc_vv,
+                        actual_rustdoc_target_data.rustc_vv
+                    );
+                }
+            }
+            Err(e) => {
+                log::debug!("could not deserialize {:?}: {}", fingerprint_path, e);
+            }
+        };
+        // Fingerprint does not match, delete the doc directories and write a new fingerprint.
+        log::debug!(
+            "fingerprint {:?} mismatch, clearing doc directories",
+            fingerprint_path
+        );
+        cx.bcx
             .all_kinds
             .iter()
             .map(|kind| cx.files().layout(*kind).doc())
-            .collect();
-
-        // Check wether `.rustdoc_fingerprint.json` exists
-        match Self::read(cx) {
-            Ok(fingerprint) => {
-                // Check if rustc_version matches the one we just used. Otherways,
-                // remove the `doc` folder to trigger a re-compilation of the docs.
-                if fingerprint.rustc_vv != actual_rustdoc_target_data.rustc_vv {
-                    Self::remove_doc_dirs(&doc_dirs)?;
-                    actual_rustdoc_target_data.write(cx)?
+            .filter(|path| path.exists())
+            .try_for_each(|path| clean_doc(path))?;
+        write_fingerprint()?;
+        return Ok(());
+
+        fn clean_doc(path: &Path) -> CargoResult<()> {
+            let entries = path
+                .read_dir()
+                .with_context(|| format!("failed to read directory `{}`", path.display()))?;
+            for entry in entries {
+                let entry = entry?;
+                // Don't remove hidden files. Rustdoc does not create them,
+                // but the user might have.
+                if entry
+                    .file_name()
+                    .to_str()
+                    .map_or(false, |name| name.starts_with('.'))
+                {
+                    continue;
+                }
+                let path = entry.path();
+                if entry.file_type()?.is_dir() {
+                    paths::remove_dir_all(path)?;
+                } else {
+                    paths::remove_file(path)?;
                 }
             }
-            // If the file does not exist, then we cannot assume that the docs were compiled
-            // with the actual Rustc instance version. Therefore, we try to remove the
-            // `doc` directory forcing the recompilation of the docs. If the directory doesn't
-            // exists neither, we simply do nothing and continue.
-            Err(_) => {
-                // We don't care if this succeeds as explained above.
-                let _ = Self::remove_doc_dirs(&doc_dirs);
-                actual_rustdoc_target_data.write(cx)?
-            }
+            Ok(())
         }
-        Ok(())
     }
 }

src/cargo/core/features.rs

@@ -542,8 +542,8 @@ macro_rules! unstable_cli_options {
     (
         $(
             $(#[$meta:meta])?
-            $element: ident: $ty: ty = ($help: expr )
-        ),*
+            $element: ident: $ty: ty = ($help: expr ),
+        )*
     ) => {
         /// A parsed representation of all unstable flags that Cargo accepts.
         ///
@@ -603,7 +603,8 @@ unstable_cli_options!(
     terminal_width: Option<Option<usize>>  = ("Provide a terminal width to rustc for error truncation"),
     timings: Option<Vec<String>>  = ("Display concurrency information"),
     unstable_options: bool = ("Allow the usage of unstable options"),
-    weak_dep_features: bool = ("Allow `dep_name?/feature` feature syntax")
+    weak_dep_features: bool = ("Allow `dep_name?/feature` feature syntax"),
+    skip_rustdoc_fingerprint: bool = (HIDDEN),
 );
 
 const STABILIZED_COMPILE_PROGRESS: &str = "The progress bar is now always \
@@ -813,6 +814,7 @@ impl CliUnstable {
             "weak-dep-features" => self.weak_dep_features = parse_empty(k, v)?,
             "extra-link-arg" => self.extra_link_arg = parse_empty(k, v)?,
             "credential-process" => self.credential_process = parse_empty(k, v)?,
+            "skip-rustdoc-fingerprint" => self.skip_rustdoc_fingerprint = parse_empty(k, v)?,
             "compile-progress" => stabilized_warn(k, "1.30", STABILIZED_COMPILE_PROGRESS),
             "offline" => stabilized_err(k, "1.36", STABILIZED_OFFLINE)?,
             "cache-messages" => stabilized_warn(k, "1.40", STABILIZED_CACHE_MESSAGES),

tests/testsuite/doc.rs

@@ -4,7 +4,7 @@ use cargo::core::compiler::RustDocFingerprint;
 use cargo_test_support::paths::CargoPathExt;
 use cargo_test_support::registry::Package;
 use cargo_test_support::{basic_lib_manifest, basic_manifest, git, project};
-use cargo_test_support::{is_nightly, rustc_host};
+use cargo_test_support::{is_nightly, rustc_host, symlink_supported};
 use std::fs;
 use std::str;
 
@@ -1800,7 +1800,6 @@ LLVM version: 9.0
     );
 }
 
-#[cfg(target_os = "linux")]
 #[cargo_test]
 fn doc_fingerprint_respects_target_paths() {
     // Random rustc verbose version
@@ -1831,9 +1830,7 @@ LLVM version: 9.0
         .file("src/lib.rs", "//! These are the docs!")
         .build();
 
-    dummy_project
-        .cargo("doc --target x86_64-unknown-linux-gnu")
-        .run();
+    dummy_project.cargo("doc --target").arg(rustc_host()).run();
 
     let fingerprint: RustDocFingerprint =
         serde_json::from_str(&dummy_project.read_file("target/.rustdoc_fingerprint.json"))
@@ -1863,7 +1860,8 @@ LLVM version: 9.0
     fs::write(
         dummy_project
             .build_dir()
-            .join("x86_64-unknown-linux-gnu/doc/bogus_file"),
+            .join(rustc_host())
+            .join("doc/bogus_file"),
         String::from("This is a bogus file and should be removed!"),
     )
     .expect("Error writing test bogus file");
@@ -1872,13 +1870,12 @@ LLVM version: 9.0
     // of rustc, cargo should remove the entire `/doc` folder (including the fingerprint)
     // and generating another one with the actual version.
     // It should also remove the bogus file we created above.
-    dummy_project
-        .cargo("doc --target x86_64-unknown-linux-gnu")
-        .run();
+    dummy_project.cargo("doc --target").arg(rustc_host()).run();
 
     assert!(!dummy_project
         .build_dir()
-        .join("x86_64-unknown-linux-gnu/doc/bogus_file")
+        .join(rustc_host())
+        .join("doc/bogus_file")
         .exists());
 
     let fingerprint: RustDocFingerprint =
@@ -1892,3 +1889,61 @@ LLVM version: 9.0
         (String::from_utf8_lossy(&output.stdout).as_ref())
     );
 }
+
+#[cargo_test]
+fn doc_fingerprint_unusual_behavior() {
+    // Checks for some unusual circumstances with clearing the doc directory.
+    if !symlink_supported() {
+        return;
+    }
+    let p = project().file("src/lib.rs", "").build();
+    p.build_dir().mkdir_p();
+    let real_doc = p.root().join("doc");
+    real_doc.mkdir_p();
+    let build_doc = p.build_dir().join("doc");
+    p.symlink(&real_doc, &build_doc);
+    fs::write(real_doc.join("somefile"), "test").unwrap();
+    fs::write(real_doc.join(".hidden"), "test").unwrap();
+    p.cargo("doc").run();
+    // Make sure for the first run, it does not delete any files and does not
+    // break the symlink.
+    assert!(build_doc.join("somefile").exists());
+    assert!(real_doc.join("somefile").exists());
+    assert!(real_doc.join(".hidden").exists());
+    assert!(real_doc.join("foo/index.html").exists());
+    // Pretend that the last build was generated by an older version.
+    p.change_file(
+        "target/.rustdoc_fingerprint.json",
+        "{\"rustc_vv\": \"I am old\"}",
+    );
+    // Change file to trigger a new build.
+    p.change_file("src/lib.rs", "// changed");
+    p.cargo("doc")
+        .with_stderr(
+            "[DOCUMENTING] foo [..]\n\
+             [FINISHED] [..]",
+        )
+        .run();
+    // This will delete somefile, but not .hidden.
+    assert!(!real_doc.join("somefile").exists());
+    assert!(real_doc.join(".hidden").exists());
+    assert!(real_doc.join("foo/index.html").exists());
+    // And also check the -Z flag behavior.
+    p.change_file(
+        "target/.rustdoc_fingerprint.json",
+        "{\"rustc_vv\": \"I am old\"}",
+    );
+    // Change file to trigger a new build.
+    p.change_file("src/lib.rs", "// changed2");
+    fs::write(real_doc.join("somefile"), "test").unwrap();
+    p.cargo("doc -Z skip-rustdoc-fingerprint")
+        .masquerade_as_nightly_cargo()
+        .with_stderr(
+            "[DOCUMENTING] foo [..]\n\
+             [FINISHED] [..]",
+        )
+        .run();
+    // Should not have deleted anything.
+    assert!(build_doc.join("somefile").exists());
+    assert!(real_doc.join("somefile").exists());
+}