Auto merge of #8640 - CPerezz:doc_versioning, r=ehuss

Implement Rustdoc versioning checks

Before compiling, we need to make sure that if there were any previous docs already compiled, they were compiled with the same Rustc version that we're currently using. Otherways we must remove the `doc/` folder and compile again.

This is important because as stated in #8461 the .js/.html&.css files that are generated by Rustc don't have any versioning. Therefore, we can fall in weird bugs and behaviours if we mix different compiler versions of these js/.html&.css files.

Closes #8461

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

@@ -11,7 +11,9 @@ use std::collections::{HashMap, HashSet};
 use std::path::PathBuf;
 
 mod target_info;
-pub use self::target_info::{FileFlavor, FileType, RustcTargetData, TargetInfo};
+pub use self::target_info::{
+    FileFlavor, FileType, RustDocFingerprint, RustcTargetData, TargetInfo,
+};
 
 /// The build context, containing all information about a build task.
 ///

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

@@ -1,12 +1,15 @@
-use crate::core::compiler::{BuildOutput, CompileKind, CompileMode, CompileTarget, CrateType};
+use crate::core::compiler::{
+    BuildOutput, CompileKind, CompileMode, CompileTarget, Context, CrateType,
+};
 use crate::core::{Dependency, Target, TargetKind, Workspace};
 use crate::util::config::{Config, StringList, TargetConfig};
-use crate::util::{CargoResult, CargoResultExt, ProcessBuilder, Rustc};
+use crate::util::{paths, CargoResult, CargoResultExt, ProcessBuilder, Rustc};
 use cargo_platform::{Cfg, CfgExpr};
+use serde::{Deserialize, Serialize};
 use std::cell::RefCell;
 use std::collections::hash_map::{Entry, HashMap};
 use std::env;
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 use std::str::{self, FromStr};
 
 /// Information about the platform target gleaned from querying rustc.
@@ -754,3 +757,77 @@ impl RustcTargetData {
         self.target_config(kind).links_overrides.get(lib_name)
     }
 }
+
+/// Structure used to deal with Rustdoc fingerprinting
+#[derive(Debug, Serialize, Deserialize)]
+pub struct RustDocFingerprint {
+    pub rustc_vv: String,
+}
+
+impl RustDocFingerprint {
+    /// Read the `RustDocFingerprint` info from the fingerprint file.
+    fn read<'a, 'cfg>(cx: &Context<'a, 'cfg>) -> 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: &Vec<&Path>) -> CargoResult<()> {
+        doc_dirs
+            .iter()
+            .filter(|path| path.exists())
+            .map(|path| paths::remove_dir_all(&path))
+            .collect::<CargoResult<()>>()
+    }
+
+    /// 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.
+    ///
+    /// In case it's not, it takes care of removing the `doc/` folder as well as overwriting
+    /// the rustdoc fingerprint info in order to guarantee that we won't end up with mixed
+    /// versions of the `js/html/css` files that `rustdoc` autogenerates which do not have
+    /// any versioning.
+    pub fn check_rustdoc_fingerprint<'a, 'cfg>(cx: &Context<'a, 'cfg>) -> CargoResult<()> {
+        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
+            .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)?
+                }
+            }
+            // 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(())
+    }
+}

src/cargo/core/compiler/context/mod.rs

@@ -18,7 +18,9 @@ use super::job_queue::JobQueue;
 use super::layout::Layout;
 use super::lto::Lto;
 use super::unit_graph::UnitDep;
-use super::{BuildContext, Compilation, CompileKind, CompileMode, Executor, FileFlavor};
+use super::{
+    BuildContext, Compilation, CompileKind, CompileMode, Executor, FileFlavor, RustDocFingerprint,
+};
 
 mod compilation_files;
 use self::compilation_files::CompilationFiles;
@@ -133,6 +135,18 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
         custom_build::build_map(&mut self)?;
         self.check_collisions()?;
 
+        // We need to make sure that if there were any previous docs
+        // already compiled, they were compiled with the same Rustc version that we're currently
+        // using. Otherways we must remove the `doc/` folder and compile again forcing a rebuild.
+        //
+        // This is important because the `.js`/`.html` & `.css` files that are generated by Rustc don't have
+        // any versioning (See https://github.com/rust-lang/cargo/issues/8461).
+        // Therefore, we can end up with weird bugs and behaviours if we mix different
+        // versions of these files.
+        if self.bcx.build_config.mode.is_doc() {
+            RustDocFingerprint::check_rustdoc_fingerprint(&self)?
+        }
+
         for unit in &self.bcx.roots {
             // Build up a list of pending jobs, each of which represent
             // compiling a particular package. No actual work is executed as

src/cargo/core/compiler/mod.rs

@@ -32,7 +32,9 @@ use lazycell::LazyCell;
 use log::debug;
 
 pub use self::build_config::{BuildConfig, CompileMode, MessageFormat};
-pub use self::build_context::{BuildContext, FileFlavor, FileType, RustcTargetData, TargetInfo};
+pub use self::build_context::{
+    BuildContext, FileFlavor, FileType, RustDocFingerprint, RustcTargetData, TargetInfo,
+};
 use self::build_plan::BuildPlan;
 pub use self::compilation::{Compilation, Doctest, UnitOutput};
 pub use self::compile_kind::{CompileKind, CompileTarget};
@@ -586,7 +588,6 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Work> {
     if let CompileKind::Target(target) = unit.kind {
         rustdoc.arg("--target").arg(target.rustc_target());
     }
-
     let doc_dir = cx.files().out_dir(unit);
 
     // Create the documentation directory ahead of time as rustdoc currently has

tests/testsuite/doc.rs

@@ -1,5 +1,6 @@
 //! Tests for the `cargo doc` command.
 
+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};
@@ -1638,3 +1639,179 @@ fn crate_versions_flag_is_overridden() {
     p.cargo("rustdoc -- --crate-version 2.0.3").run();
     asserts(output_documentation());
 }
+
+#[cargo_test]
+fn doc_fingerprint_is_versioning_consistent() {
+    // Random rustc verbose version
+    let old_rustc_verbose_version = format!(
+        "\
+rustc 1.41.1 (f3e1a954d 2020-02-24)
+binary: rustc
+commit-hash: f3e1a954d2ead4e2fc197c7da7d71e6c61bad196
+commit-date: 2020-02-24
+host: {}
+release: 1.41.1
+LLVM version: 9.0
+",
+        rustc_host()
+    );
+
+    // Create the dummy project.
+    let dummy_project = project()
+        .file(
+            "Cargo.toml",
+            r#"
+            [package]
+            name = "foo"
+            version = "1.2.4"
+            authors = []
+        "#,
+        )
+        .file("src/lib.rs", "//! These are the docs!")
+        .build();
+
+    dummy_project.cargo("doc").run();
+
+    let fingerprint: RustDocFingerprint =
+        serde_json::from_str(&dummy_project.read_file("target/.rustdoc_fingerprint.json"))
+            .expect("JSON Serde fail");
+
+    // Check that the fingerprint contains the actual rustc version
+    // which has been used to compile the docs.
+    let output = std::process::Command::new("rustc")
+        .arg("-vV")
+        .output()
+        .expect("Failed to get actual rustc verbose version");
+    assert_eq!(
+        fingerprint.rustc_vv,
+        (String::from_utf8_lossy(&output.stdout).as_ref())
+    );
+
+    // As the test shows above. Now we have generated the `doc/` folder and inside
+    // the rustdoc fingerprint file is located with the correct rustc version.
+    // So we will remove it and create a new fingerprint with an old rustc version
+    // inside it. We will also place a bogus file inside of the `doc/` folder to ensure
+    // it gets removed as we expect on the next doc compilation.
+    dummy_project.change_file(
+        "target/.rustdoc_fingerprint.json",
+        &old_rustc_verbose_version,
+    );
+
+    fs::write(
+        dummy_project.build_dir().join("doc/bogus_file"),
+        String::from("This is a bogus file and should be removed!"),
+    )
+    .expect("Error writing test bogus file");
+
+    // Now if we trigger another compilation, since the fingerprint contains an old version
+    // 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").run();
+
+    assert!(!dummy_project.build_dir().join("doc/bogus_file").exists());
+
+    let fingerprint: RustDocFingerprint =
+        serde_json::from_str(&dummy_project.read_file("target/.rustdoc_fingerprint.json"))
+            .expect("JSON Serde fail");
+
+    // Check that the fingerprint contains the actual rustc version
+    // which has been used to compile the docs.
+    assert_eq!(
+        fingerprint.rustc_vv,
+        (String::from_utf8_lossy(&output.stdout).as_ref())
+    );
+}
+
+#[cfg(target_os = "linux")]
+#[cargo_test]
+fn doc_fingerprint_respects_target_paths() {
+    // Random rustc verbose version
+    let old_rustc_verbose_version = format!(
+        "\
+rustc 1.41.1 (f3e1a954d 2020-02-24)
+binary: rustc
+commit-hash: f3e1a954d2ead4e2fc197c7da7d71e6c61bad196
+commit-date: 2020-02-24
+host: {}
+release: 1.41.1
+LLVM version: 9.0
+",
+        rustc_host()
+    );
+
+    // Create the dummy project.
+    let dummy_project = project()
+        .file(
+            "Cargo.toml",
+            r#"
+            [package]
+            name = "foo"
+            version = "1.2.4"
+            authors = []
+        "#,
+        )
+        .file("src/lib.rs", "//! These are the docs!")
+        .build();
+
+    dummy_project
+        .cargo("doc --target x86_64-unknown-linux-gnu")
+        .run();
+
+    let fingerprint: RustDocFingerprint =
+        serde_json::from_str(&dummy_project.read_file("target/.rustdoc_fingerprint.json"))
+            .expect("JSON Serde fail");
+
+    // Check that the fingerprint contains the actual rustc version
+    // which has been used to compile the docs.
+    let output = std::process::Command::new("rustc")
+        .arg("-vV")
+        .output()
+        .expect("Failed to get actual rustc verbose version");
+    assert_eq!(
+        fingerprint.rustc_vv,
+        (String::from_utf8_lossy(&output.stdout).as_ref())
+    );
+
+    // As the test shows above. Now we have generated the `doc/` folder and inside
+    // the rustdoc fingerprint file is located with the correct rustc version.
+    // So we will remove it and create a new fingerprint with an old rustc version
+    // inside it. We will also place a bogus file inside of the `doc/` folder to ensure
+    // it gets removed as we expect on the next doc compilation.
+    dummy_project.change_file(
+        "target/.rustdoc_fingerprint.json",
+        &old_rustc_verbose_version,
+    );
+
+    fs::write(
+        dummy_project
+            .build_dir()
+            .join("x86_64-unknown-linux-gnu/doc/bogus_file"),
+        String::from("This is a bogus file and should be removed!"),
+    )
+    .expect("Error writing test bogus file");
+
+    // Now if we trigger another compilation, since the fingerprint contains an old version
+    // 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();
+
+    assert!(!dummy_project
+        .build_dir()
+        .join("x86_64-unknown-linux-gnu/doc/bogus_file")
+        .exists());
+
+    let fingerprint: RustDocFingerprint =
+        serde_json::from_str(&dummy_project.read_file("target/.rustdoc_fingerprint.json"))
+            .expect("JSON Serde fail");
+
+    // Check that the fingerprint contains the actual rustc version
+    // which has been used to compile the docs.
+    assert_eq!(
+        fingerprint.rustc_vv,
+        (String::from_utf8_lossy(&output.stdout).as_ref())
+    );
+}