From 5035321021c8aabf2c81b038c46c3e3dc385100e Mon Sep 17 00:00:00 2001 From: CPerezz Date: Sat, 22 Aug 2020 09:12:49 +0200 Subject: [PATCH 01/22] Implement `RustDocTargetData` This intends to be a helper structure similar to `RustTargetData` which includes helper functions which are useful for interacting with the rustdoc fingerprint files among other things. --- src/cargo/ops/cargo_doc.rs | 39 +++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/src/cargo/ops/cargo_doc.rs b/src/cargo/ops/cargo_doc.rs index 78e2f66f7dc..b03de1a3066 100644 --- a/src/cargo/ops/cargo_doc.rs +++ b/src/cargo/ops/cargo_doc.rs @@ -3,8 +3,12 @@ use crate::core::resolver::{HasDevUnits, ResolveOpts}; use crate::core::{Shell, Workspace}; use crate::ops; use crate::util::CargoResult; +use serde::{Deserialize, Serialize}; +use serde_json::{from_str, to_writer}; use std::collections::HashMap; -use std::path::Path; +use std::fs::File; +use std::io::Read; +use std::path::{Path, PathBuf}; use std::process::Command; /// Strongly typed options for the `cargo doc` command. @@ -75,6 +79,15 @@ pub fn doc(ws: &Workspace<'_>, options: &DocOptions) -> CargoResult<()> { None }; + // 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 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. + check_rustdoc_appropiate_versioning(ws, &target_data)?; + let compilation = ops::compile(ws, &options.compile_opts)?; if let Some(kind) = open_kind { @@ -117,3 +130,27 @@ fn open_docs(path: &Path, shell: &mut Shell) -> CargoResult<()> { Ok(()) } + +#[derive(Debug, Serialize, Deserialize)] +pub struct RustDocTargetData { + rustc_verbose_version: String, +} + +impl RustDocTargetData { + pub fn with_actual_rustc_version(rustc_verbose_version: &str) -> Self { + Self { + rustc_verbose_version: rustc_verbose_version.to_string(), + } + } + + pub fn path_to_fingerprint(ws: &Workspace<'_>) -> PathBuf { + ws.root().join("target").join(".rustdoc_fingerprint.json") + } + + pub fn from_file(file: &mut File) -> anyhow::Result { + let mut contents = String::new(); + file.read_to_string(&mut contents)?; + let rustdoc_target_data: Self = from_str(&contents)?; + Ok(rustdoc_target_data) + } +} From c70c2cc8aca2d8f244ad358c4df83597d1ce7aea Mon Sep 17 00:00:00 2001 From: CPerezz Date: Sat, 22 Aug 2020 09:14:25 +0200 Subject: [PATCH 02/22] Impl remove_doc_directory function Sometimes we need to remove the `doc/` directory (if exists) in order to not mix different versions of the js/html/css files that Rustc autogenerates and lack versioning. --- src/cargo/ops/cargo_doc.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/cargo/ops/cargo_doc.rs b/src/cargo/ops/cargo_doc.rs index b03de1a3066..73dcd4c5510 100644 --- a/src/cargo/ops/cargo_doc.rs +++ b/src/cargo/ops/cargo_doc.rs @@ -131,6 +131,19 @@ fn open_docs(path: &Path, shell: &mut Shell) -> CargoResult<()> { Ok(()) } +/// Tries to remove the Workspace's `target/doc/` directory wit all of it's contents +/// returning an error if it's not possible. +fn remove_doc_directory(ws: &Workspace<'_>) -> std::io::Result<()> { + let doc_path = &ws.root().join("target").join("doc"); + if doc_path.exists() { + // Try to remove the `doc/` folder if exists. + // XXX: What should we do if the op fails? + std::fs::remove_dir_all(doc_path) + } else { + Ok(()) + } +} + #[derive(Debug, Serialize, Deserialize)] pub struct RustDocTargetData { rustc_verbose_version: String, From 51d3050a9c4f6f337ffcdf4cfd1d32669cae65e1 Mon Sep 17 00:00:00 2001 From: CPerezz Date: Sat, 22 Aug 2020 09:15:49 +0200 Subject: [PATCH 03/22] 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. --- src/cargo/ops/cargo_doc.rs | 48 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/src/cargo/ops/cargo_doc.rs b/src/cargo/ops/cargo_doc.rs index 73dcd4c5510..cd4d197ab75 100644 --- a/src/cargo/ops/cargo_doc.rs +++ b/src/cargo/ops/cargo_doc.rs @@ -131,6 +131,54 @@ fn open_docs(path: &Path, shell: &mut Shell) -> CargoResult<()> { Ok(()) } +/// This function checks whether the latest version of `Rustc` used to compile this +/// `Workspace`'s docs was the same as the one is being used in this `cargo doc` call. +/// +/// In case it's not, it takes care of removig 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 `Rustc` autogenerates (which btw, do not have) +/// any versioning. +fn check_rustdoc_appropiate_versioning( + ws: &Workspace<'_>, + target_data: &RustcTargetData, +) -> anyhow::Result<()> { + let actual_rustdoc_target_data = + RustDocTargetData::with_actual_rustc_version(&target_data.rustc.verbose_version); + let mut rustdoc_fingerprint_file = match File::open(RustDocTargetData::path_to_fingerprint(ws)) + { + Ok(file) => file, + // If the file does not exist, create it with the actual `Rustc` version + // and try to delete the `doc/` folder. + // + // This is weird, but if someone deletes the fingerprint and compiled the docs + // of the target workspace with a previous `Rustc` version, we would fall under + // the same problem. + Err(_) => { + // Check if `doc/` folder exists and remove it. Otherways, simply create + // `.rustdoc_fingerprint.json` file. + remove_doc_directory(ws)?; + let rustdoc_fingerprint_file = + File::create(RustDocTargetData::path_to_fingerprint(ws))?; + // We write the actual `Rustc` version to it so that we just need to compile it straight + // since there's no `doc/` folder to remove. + to_writer(&rustdoc_fingerprint_file, &actual_rustdoc_target_data)?; + rustdoc_fingerprint_file + } + }; + + let previous_rustdoc_target_data = RustDocTargetData::from_file(&mut rustdoc_fingerprint_file)?; + // Check if rustc_version matches the one we just used. Otherways, + // re-write it and delete the ´doc/` folder (if exists). + if previous_rustdoc_target_data.rustc_verbose_version != target_data.rustc.verbose_version { + remove_doc_directory(ws)?; + // Remove `rustdoc_fingerprint_file` contents and write the new version used + // to compile the docs. + rustdoc_fingerprint_file.set_len(0)?; + to_writer(&rustdoc_fingerprint_file, &actual_rustdoc_target_data)?; + }; + Ok(()) +} + /// Tries to remove the Workspace's `target/doc/` directory wit all of it's contents /// returning an error if it's not possible. fn remove_doc_directory(ws: &Workspace<'_>) -> std::io::Result<()> { From e38c3ac3641d4d539c565091f1a2b8c82d110c92 Mon Sep 17 00:00:00 2001 From: CPerezz Date: Sun, 20 Sep 2020 09:57:44 +0200 Subject: [PATCH 04/22] Fail when dealing with io fails Is the user does not have permissions or a folder/file can't be created or deleted, Cargo will fail and return an Error indicating the problem. --- src/cargo/ops/cargo_doc.rs | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/cargo/ops/cargo_doc.rs b/src/cargo/ops/cargo_doc.rs index cd4d197ab75..204d6893acc 100644 --- a/src/cargo/ops/cargo_doc.rs +++ b/src/cargo/ops/cargo_doc.rs @@ -83,9 +83,9 @@ pub fn doc(ws: &Workspace<'_>, options: &DocOptions) -> CargoResult<()> { // 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 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. + // This is important because the `.js`/`.html` & `.css` files that are generated by Rustc don't have + // any versioning. Therefore, we can end up with weird bugs and behaviours if we mix different + // compiler versions of these files. check_rustdoc_appropiate_versioning(ws, &target_data)?; let compilation = ops::compile(ws, &options.compile_opts)?; @@ -156,7 +156,11 @@ fn check_rustdoc_appropiate_versioning( Err(_) => { // Check if `doc/` folder exists and remove it. Otherways, simply create // `.rustdoc_fingerprint.json` file. - remove_doc_directory(ws)?; + match remove_doc_directory(ws) { + // If the folder does not exist, we don't care since our goal was to + // delete it. + _ => (), + } let rustdoc_fingerprint_file = File::create(RustDocTargetData::path_to_fingerprint(ws))?; // We write the actual `Rustc` version to it so that we just need to compile it straight @@ -173,23 +177,17 @@ fn check_rustdoc_appropiate_versioning( remove_doc_directory(ws)?; // Remove `rustdoc_fingerprint_file` contents and write the new version used // to compile the docs. - rustdoc_fingerprint_file.set_len(0)?; + //rustdoc_fingerprint_file.set_len(0)?; to_writer(&rustdoc_fingerprint_file, &actual_rustdoc_target_data)?; }; Ok(()) } -/// Tries to remove the Workspace's `target/doc/` directory wit all of it's contents +/// Tries to remove the Workspace's `target/doc/` directory with all of it's contents /// returning an error if it's not possible. fn remove_doc_directory(ws: &Workspace<'_>) -> std::io::Result<()> { - let doc_path = &ws.root().join("target").join("doc"); - if doc_path.exists() { - // Try to remove the `doc/` folder if exists. - // XXX: What should we do if the op fails? - std::fs::remove_dir_all(doc_path) - } else { - Ok(()) - } + // Try to remove the `doc/` folder if exists. + std::fs::remove_dir_all(&ws.root().join("target").join("doc")).into() } #[derive(Debug, Serialize, Deserialize)] From 7a5f7844aa77734977cff530900815c13f3e2d26 Mon Sep 17 00:00:00 2001 From: CPerezz Date: Sun, 20 Sep 2020 10:28:46 +0200 Subject: [PATCH 05/22] Add skeleton for testing doc-fingerprint --- tests/testsuite/doc.rs | 104 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/tests/testsuite/doc.rs b/tests/testsuite/doc.rs index 0a4a914ef67..805c1fc893b 100644 --- a/tests/testsuite/doc.rs +++ b/tests/testsuite/doc.rs @@ -5,6 +5,7 @@ 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 std::fs; +use std::io::Read; use std::str; #[cargo_test] @@ -1551,3 +1552,106 @@ fn crate_versions_flag_is_overridden() { .run(); asserts(output_documentation()); } + +#[cargo_test] +fn doc_versioning_works() { + // Test that using different Rustc versions forces a + // doc re-compilation producing new html, css & js files. + + // Create separate rustc binaries to emulate running different toolchains. + let nightly1 = format!( + "\ +rustc 1.44.0-nightly (38114ff16 2020-03-21) +binary: rustc +commit-hash: 38114ff16e7856f98b2b4be7ab4cd29b38bed59a +commit-date: 2020-03-21 +host: {} +release: 1.44.0-nightly +LLVM version: 9.0 +", + rustc_host() + ); + + let nightly2 = format!( + "\ +rustc 1.44.0-nightly (a5b09d354 2020-03-31) +binary: rustc +commit-hash: a5b09d35473615e7142f5570f5c5fad0caf68bd2 +commit-date: 2020-03-31 +host: {} +release: 1.44.0-nightly +LLVM version: 9.0 +", + rustc_host() + ); + + let compiler = project() + .at("compiler") + .file("Cargo.toml", &basic_manifest("compiler", "0.1.0")) + .file( + "src/main.rs", + r#" + fn main() { + if std::env::args_os().any(|a| a == "-vV") { + print!("{}", env!("FUNKY_VERSION_TEST")); + return; + } + let mut cmd = std::process::Command::new("rustc"); + cmd.args(std::env::args_os().skip(1)); + assert!(cmd.status().unwrap().success()); + } + "#, + ) + .build(); + + let makeit = |version, vv| { + // Force a rebuild. + compiler.target_debug_dir().join("deps").rm_rf(); + compiler.cargo("build").env("FUNKY_VERSION_TEST", vv).run(); + fs::rename(compiler.bin("compiler"), compiler.bin(version)).unwrap(); + }; + makeit("nightly1", nightly1); + + let get_fingerprint_v = |target_path: &std::path::Path| -> String { + let mut contents = Vec::new(); + let mut fingerp_file = std::fs::File::open(target_path).unwrap(); + fingerp_file.read(&mut contents).unwrap(); + String::from_utf8(contents).unwrap() + }; + + // Create the dummy project and compile it in "nightly1" + 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(); + + // Build should have created the `.rustdoc_fingerprint.json` under `target/` + assert!(fs::File::open(dummy_project.build_dir().join(".rustdoc_fingerprint.json")).is_ok()); + // The file should contain the `nightly1` version of Rustc + assert_eq!(get_fingerprint_v(&dummy_project.build_dir()), "nightly1"); + // Compiling the project with "nightly2" should remove the /doc folder and re-build + // the docs. Therefore, we place a bogus file to check that this happens. + makeit("nightly2", nightly2); + // Add a bogus_file to check that indeed the /doc folder will be removed due to + // the change of toolchain version. + fs::File::create(dummy_project.build_dir().join("doc/bogus_file.json")).unwrap(); + // Re-document the crate + dummy_project + .cargo("doc --no-deps") + .exec_with_output() + .unwrap(); + // Build should have removed /doc and therefore bogus_file.json + assert!(fs::File::open(dummy_project.target_debug_dir().join("doc/bogus_file.json")).is_err()); + // Build should have created the `.rustdoc_fingerprint.json` under `target/` + assert!(fs::File::open(dummy_project.build_dir().join(".rustdoc_fingerprint.json")).is_ok()); + // The file should contain the `nightly2` version of Rustc + assert_eq!(get_fingerprint_v(&dummy_project.build_dir()), "nightly2"); +} From 87c1f6c97e94ec5cff116512e893e027f3dae7f8 Mon Sep 17 00:00:00 2001 From: CPerezz Date: Wed, 16 Dec 2020 09:21:20 +0100 Subject: [PATCH 06/22] Remove previous solution --- src/cargo/ops/cargo_doc.rs | 98 +------------------------------------- 1 file changed, 1 insertion(+), 97 deletions(-) diff --git a/src/cargo/ops/cargo_doc.rs b/src/cargo/ops/cargo_doc.rs index 204d6893acc..78e2f66f7dc 100644 --- a/src/cargo/ops/cargo_doc.rs +++ b/src/cargo/ops/cargo_doc.rs @@ -3,12 +3,8 @@ use crate::core::resolver::{HasDevUnits, ResolveOpts}; use crate::core::{Shell, Workspace}; use crate::ops; use crate::util::CargoResult; -use serde::{Deserialize, Serialize}; -use serde_json::{from_str, to_writer}; use std::collections::HashMap; -use std::fs::File; -use std::io::Read; -use std::path::{Path, PathBuf}; +use std::path::Path; use std::process::Command; /// Strongly typed options for the `cargo doc` command. @@ -79,15 +75,6 @@ pub fn doc(ws: &Workspace<'_>, options: &DocOptions) -> CargoResult<()> { None }; - // 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 the `.js`/`.html` & `.css` files that are generated by Rustc don't have - // any versioning. Therefore, we can end up with weird bugs and behaviours if we mix different - // compiler versions of these files. - check_rustdoc_appropiate_versioning(ws, &target_data)?; - let compilation = ops::compile(ws, &options.compile_opts)?; if let Some(kind) = open_kind { @@ -130,86 +117,3 @@ fn open_docs(path: &Path, shell: &mut Shell) -> CargoResult<()> { Ok(()) } - -/// This function checks whether the latest version of `Rustc` used to compile this -/// `Workspace`'s docs was the same as the one is being used in this `cargo doc` call. -/// -/// In case it's not, it takes care of removig 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 `Rustc` autogenerates (which btw, do not have) -/// any versioning. -fn check_rustdoc_appropiate_versioning( - ws: &Workspace<'_>, - target_data: &RustcTargetData, -) -> anyhow::Result<()> { - let actual_rustdoc_target_data = - RustDocTargetData::with_actual_rustc_version(&target_data.rustc.verbose_version); - let mut rustdoc_fingerprint_file = match File::open(RustDocTargetData::path_to_fingerprint(ws)) - { - Ok(file) => file, - // If the file does not exist, create it with the actual `Rustc` version - // and try to delete the `doc/` folder. - // - // This is weird, but if someone deletes the fingerprint and compiled the docs - // of the target workspace with a previous `Rustc` version, we would fall under - // the same problem. - Err(_) => { - // Check if `doc/` folder exists and remove it. Otherways, simply create - // `.rustdoc_fingerprint.json` file. - match remove_doc_directory(ws) { - // If the folder does not exist, we don't care since our goal was to - // delete it. - _ => (), - } - let rustdoc_fingerprint_file = - File::create(RustDocTargetData::path_to_fingerprint(ws))?; - // We write the actual `Rustc` version to it so that we just need to compile it straight - // since there's no `doc/` folder to remove. - to_writer(&rustdoc_fingerprint_file, &actual_rustdoc_target_data)?; - rustdoc_fingerprint_file - } - }; - - let previous_rustdoc_target_data = RustDocTargetData::from_file(&mut rustdoc_fingerprint_file)?; - // Check if rustc_version matches the one we just used. Otherways, - // re-write it and delete the ´doc/` folder (if exists). - if previous_rustdoc_target_data.rustc_verbose_version != target_data.rustc.verbose_version { - remove_doc_directory(ws)?; - // Remove `rustdoc_fingerprint_file` contents and write the new version used - // to compile the docs. - //rustdoc_fingerprint_file.set_len(0)?; - to_writer(&rustdoc_fingerprint_file, &actual_rustdoc_target_data)?; - }; - Ok(()) -} - -/// Tries to remove the Workspace's `target/doc/` directory with all of it's contents -/// returning an error if it's not possible. -fn remove_doc_directory(ws: &Workspace<'_>) -> std::io::Result<()> { - // Try to remove the `doc/` folder if exists. - std::fs::remove_dir_all(&ws.root().join("target").join("doc")).into() -} - -#[derive(Debug, Serialize, Deserialize)] -pub struct RustDocTargetData { - rustc_verbose_version: String, -} - -impl RustDocTargetData { - pub fn with_actual_rustc_version(rustc_verbose_version: &str) -> Self { - Self { - rustc_verbose_version: rustc_verbose_version.to_string(), - } - } - - pub fn path_to_fingerprint(ws: &Workspace<'_>) -> PathBuf { - ws.root().join("target").join(".rustdoc_fingerprint.json") - } - - pub fn from_file(file: &mut File) -> anyhow::Result { - let mut contents = String::new(); - file.read_to_string(&mut contents)?; - let rustdoc_target_data: Self = from_str(&contents)?; - Ok(rustdoc_target_data) - } -} From 65f17e1ca669a05a60ad1aa108df72443aebe6db Mon Sep 17 00:00:00 2001 From: CPerezz Date: Wed, 16 Dec 2020 09:21:56 +0100 Subject: [PATCH 07/22] Implement rustdoc versioning After refactoring what was done on the latest approach this time we included the rustdoc versioning logic inside the `compiler::rustdoc` fn. - Created `RustDocFingerprint` struct so that is easier to manage all of the logic & functions related to rustdoc versioning as well as scaling it in the future if needed. --- src/cargo/core/compiler/build_context/mod.rs | 4 +- .../compiler/build_context/target_info.rs | 82 ++++++++++++++++++- src/cargo/core/compiler/context/mod.rs | 9 +- src/cargo/core/compiler/mod.rs | 23 +++++- 4 files changed, 108 insertions(+), 10 deletions(-) diff --git a/src/cargo/core/compiler/build_context/mod.rs b/src/cargo/core/compiler/build_context/mod.rs index 47efdc77de8..455a7e622d8 100644 --- a/src/cargo/core/compiler/build_context/mod.rs +++ b/src/cargo/core/compiler/build_context/mod.rs @@ -11,7 +11,9 @@ use std::collections::HashMap; 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. /// diff --git a/src/cargo/core/compiler/build_context/target_info.rs b/src/cargo/core/compiler/build_context/target_info.rs index b2d0b0ffeb5..4612a3c8807 100644 --- a/src/cargo/core/compiler/build_context/target_info.rs +++ b/src/cargo/core/compiler/build_context/target_info.rs @@ -3,10 +3,14 @@ use crate::core::{Dependency, Target, TargetKind, Workspace}; use crate::util::config::{Config, StringList, TargetConfig}; use crate::util::{CargoResult, CargoResultExt, ProcessBuilder, Rustc}; use cargo_platform::{Cfg, CfgExpr}; +use serde::{Deserialize, Serialize}; +use serde_json::{from_str, to_writer}; use std::cell::RefCell; use std::collections::hash_map::{Entry, HashMap}; use std::env; -use std::path::PathBuf; +use std::fs::File; +use std::io::Read; +use std::path::{Path, PathBuf}; use std::str::{self, FromStr}; /// Information about the platform target gleaned from querying rustc. @@ -753,3 +757,79 @@ 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 { + _rustc_verbose_version: String, +} + +impl RustDocFingerprint { + /// Returns the version contained by this `RustDocFingerprint` instance. + fn version(&self) -> &str { + self._rustc_verbose_version.as_str() + } + + /// Given the `doc` dir, returns the path to the rustdoc fingerprint file. + fn path_to_fingerprint(doc_dir: &Path) -> PathBuf { + doc_dir.to_path_buf().join(".rustdoc_fingerprint.json") + } + + /// Generate an instance of `RustDocFingerprint` from the fingerprint file. + fn from_file(file: &mut File) -> anyhow::Result { + let mut contents = String::new(); + file.read_to_string(&mut contents)?; + let rustdoc_target_data: Self = from_str(&contents)?; + Ok(rustdoc_target_data) + } + + /// Write the `RustDocFingerprint` info into the fingerprint file. + pub fn write(&self, doc_dir: &Path) -> std::io::Result<()> { + let rustdoc_fingerprint_file = + File::create(RustDocFingerprint::path_to_fingerprint(doc_dir))?; + // We write the actual `Rustc` version to it so that we just need to compile it straight + // since there's no `doc/` folder to remove. + to_writer(&rustdoc_fingerprint_file, &self) + .map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, format!("{:?}", e))) + } + + /// 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, returning `(bool, Self)` where the `bool` says if the `doc` folder was removed. + /// + /// In case it's not, it takes care of removig 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 `Rustc` autogenerates which do not have + /// any versioning. + pub fn check_rustdoc_fingerprint( + doc_dir: &Path, + rustc_verbose_ver: &str, + ) -> anyhow::Result<(Self, bool)> { + let actual_rustdoc_target_data = RustDocFingerprint { + _rustc_verbose_version: rustc_verbose_ver.to_string(), + }; + + // Check wether `.rustdoc_fingerprint.json exists + let mut fingerprint = match File::open(RustDocFingerprint::path_to_fingerprint(doc_dir)) { + Ok(file) => file, + // If the file does not exist, then we cannot assume that the docs were compiled + // with the actual Rustc instace 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 suceeds as explained above. + let _ = std::fs::remove_dir_all(doc_dir); + return Ok((actual_rustdoc_target_data, true)); + } + }; + + let previous_rustdoc_target_data = RustDocFingerprint::from_file(&mut 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 previous_rustdoc_target_data.version() != actual_rustdoc_target_data.version() { + std::fs::remove_dir_all(doc_dir)?; + return Ok((actual_rustdoc_target_data, true)); + }; + Ok((actual_rustdoc_target_data, false)) + } +} diff --git a/src/cargo/core/compiler/context/mod.rs b/src/cargo/core/compiler/context/mod.rs index 52b954dd12d..31a8f3cd028 100644 --- a/src/cargo/core/compiler/context/mod.rs +++ b/src/cargo/core/compiler/context/mod.rs @@ -5,11 +5,6 @@ use std::sync::{Arc, Mutex}; use filetime::FileTime; use jobserver::Client; -use crate::core::compiler::{self, compilation, Unit}; -use crate::core::PackageId; -use crate::util::errors::{CargoResult, CargoResultExt}; -use crate::util::profile; - use super::build_plan::BuildPlan; use super::custom_build::{self, BuildDeps, BuildScriptOutputs, BuildScripts}; use super::fingerprint::Fingerprint; @@ -18,6 +13,10 @@ use super::layout::Layout; use super::lto::Lto; use super::unit_graph::UnitDep; use super::{BuildContext, Compilation, CompileKind, CompileMode, Executor, FileFlavor}; +use crate::core::compiler::{self, compilation, Unit}; +use crate::core::PackageId; +use crate::util::errors::{CargoResult, CargoResultExt}; +use crate::util::profile; mod compilation_files; use self::compilation_files::CompilationFiles; diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 91d444cce05..f2ac04f1de0 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/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}; pub use self::compile_kind::{CompileKind, CompileTarget}; @@ -554,15 +556,26 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { if let CompileKind::Target(target) = unit.kind { rustdoc.arg("--target").arg(target.rustc_target()); } - let doc_dir = cx.files().out_dir(unit); + // 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 + // compiler versions of these files. + let (fingerprint, doc_dir_removed) = RustDocFingerprint::check_rustdoc_fingerprint( + &doc_dir, + cx.bcx.rustc().verbose_version.as_str(), + )?; // Create the documentation directory ahead of time as rustdoc currently has // a bug where concurrent invocations will race to create this directory if // it doesn't already exist. paths::create_dir_all(&doc_dir)?; - rustdoc.arg("-o").arg(doc_dir); + rustdoc.arg("-o").arg(doc_dir.clone()); for feat in &unit.features { rustdoc.arg("--cfg").arg(&format!("feature=\"{}\"", feat)); @@ -613,6 +626,10 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { false, ) .chain_err(|| format!("Could not document `{}`.", name))?; + + if doc_dir_removed { + fingerprint.write(&doc_dir)? + }; Ok(()) })) } From 2fea14e449eb1380bdc1f434efacd709e8d7b78d Mon Sep 17 00:00:00 2001 From: CPerezz Date: Wed, 16 Dec 2020 09:24:59 +0100 Subject: [PATCH 08/22] Add tests for rustdoc fingerprint impl - Check wether the fingerprint is created with the correct data after a `rustdoc` execution. - Check wether the previous docs are removed if the version used to compile them reported in the fingerprint is different from the actual one being used by the compiler. --- tests/testsuite/doc.rs | 180 ++++++++++++++++++++++++----------------- 1 file changed, 104 insertions(+), 76 deletions(-) diff --git a/tests/testsuite/doc.rs b/tests/testsuite/doc.rs index 805c1fc893b..eaa21672b51 100644 --- a/tests/testsuite/doc.rs +++ b/tests/testsuite/doc.rs @@ -1554,72 +1554,65 @@ fn crate_versions_flag_is_overridden() { } #[cargo_test] -fn doc_versioning_works() { +fn rustc_adds_doc_fingerprint() { + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + authors = [] + "#, + ) + .file("src/lib.rs", "//! Hello") + .build(); + + p.cargo("doc").run(); + + // Read fingerprint rustc version and check it matches the expected one + let mut fingerprint = fs::File::open( + p.root() + .join("target") + .join("doc") + .join(".rustdoc_fingerprint.json"), + ) + .expect("Failed to read fingerprint"); + + let mut rustc_verbose_version = String::new(); + fingerprint + .read_to_string(&mut rustc_verbose_version) + .expect("Error reading the fingerprint contents"); + let output = std::process::Command::new("rustc") + .arg("-vV") + .stdout(std::process::Stdio::inherit()) + .output() + .expect("Failed to get actual rustc verbose version"); + assert!(rustc_verbose_version + .as_str() + .contains(String::from_utf8_lossy(&output.stdout).as_ref())); +} + +#[cargo_test] +fn doc_fingerprint_versioning_consistent() { // Test that using different Rustc versions forces a // doc re-compilation producing new html, css & js files. - // Create separate rustc binaries to emulate running different toolchains. - let nightly1 = format!( - "\ -rustc 1.44.0-nightly (38114ff16 2020-03-21) -binary: rustc -commit-hash: 38114ff16e7856f98b2b4be7ab4cd29b38bed59a -commit-date: 2020-03-21 -host: {} -release: 1.44.0-nightly -LLVM version: 9.0 -", - rustc_host() - ); - - let nightly2 = format!( + // Random rustc verbose version + let stable_rustc_verbose_version = format!( "\ -rustc 1.44.0-nightly (a5b09d354 2020-03-31) +rustc 1.41.1 (f3e1a954d 2020-02-24) binary: rustc -commit-hash: a5b09d35473615e7142f5570f5c5fad0caf68bd2 -commit-date: 2020-03-31 +commit-hash: f3e1a954d2ead4e2fc197c7da7d71e6c61bad196 +commit-date: 2020-02-24 host: {} -release: 1.44.0-nightly +release: 1.41.1 LLVM version: 9.0 ", rustc_host() ); - let compiler = project() - .at("compiler") - .file("Cargo.toml", &basic_manifest("compiler", "0.1.0")) - .file( - "src/main.rs", - r#" - fn main() { - if std::env::args_os().any(|a| a == "-vV") { - print!("{}", env!("FUNKY_VERSION_TEST")); - return; - } - let mut cmd = std::process::Command::new("rustc"); - cmd.args(std::env::args_os().skip(1)); - assert!(cmd.status().unwrap().success()); - } - "#, - ) - .build(); - - let makeit = |version, vv| { - // Force a rebuild. - compiler.target_debug_dir().join("deps").rm_rf(); - compiler.cargo("build").env("FUNKY_VERSION_TEST", vv).run(); - fs::rename(compiler.bin("compiler"), compiler.bin(version)).unwrap(); - }; - makeit("nightly1", nightly1); - - let get_fingerprint_v = |target_path: &std::path::Path| -> String { - let mut contents = Vec::new(); - let mut fingerp_file = std::fs::File::open(target_path).unwrap(); - fingerp_file.read(&mut contents).unwrap(); - String::from_utf8(contents).unwrap() - }; - - // Create the dummy project and compile it in "nightly1" + // Create the dummy project. let dummy_project = project() .file( "Cargo.toml", @@ -1633,25 +1626,60 @@ LLVM version: 9.0 .file("src/lib.rs", "//! These are the docs!") .build(); - // Build should have created the `.rustdoc_fingerprint.json` under `target/` - assert!(fs::File::open(dummy_project.build_dir().join(".rustdoc_fingerprint.json")).is_ok()); - // The file should contain the `nightly1` version of Rustc - assert_eq!(get_fingerprint_v(&dummy_project.build_dir()), "nightly1"); - // Compiling the project with "nightly2" should remove the /doc folder and re-build - // the docs. Therefore, we place a bogus file to check that this happens. - makeit("nightly2", nightly2); - // Add a bogus_file to check that indeed the /doc folder will be removed due to - // the change of toolchain version. - fs::File::create(dummy_project.build_dir().join("doc/bogus_file.json")).unwrap(); - // Re-document the crate - dummy_project - .cargo("doc --no-deps") - .exec_with_output() - .unwrap(); - // Build should have removed /doc and therefore bogus_file.json - assert!(fs::File::open(dummy_project.target_debug_dir().join("doc/bogus_file.json")).is_err()); - // Build should have created the `.rustdoc_fingerprint.json` under `target/` - assert!(fs::File::open(dummy_project.build_dir().join(".rustdoc_fingerprint.json")).is_ok()); - // The file should contain the `nightly2` version of Rustc - assert_eq!(get_fingerprint_v(&dummy_project.build_dir()), "nightly2"); + dummy_project.cargo("doc").run(); + + // As the test shows avobe. Now we have generated the `doc/` folder and inside + // the rustdoc fingerprint file is located. + // So we will remove it and create a new fingerprint with an old rustc version + // inside it. + fs::remove_file( + dummy_project + .root() + .join("target") + .join("doc") + .join(".rustdoc_fingerprint.json"), + ) + .expect("Failed to read fingerprint on tests"); + + fs::write( + dummy_project + .root() + .join("target") + .join("doc") + .join(".rustdoc_fingerprint.json"), + stable_rustc_verbose_version, + ) + .expect("Error writing old rustc version"); + + // 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. + dummy_project.cargo("doc").run(); + + // Edit the fingerprint rustc version. + // Read fingerprint rustc version and return it as String + let get_fingerprint_version = |root_path: std::path::PathBuf| -> String { + let mut fingerprint = fs::File::open( + root_path + .join("target") + .join("doc") + .join(".rustdoc_fingerprint.json"), + ) + .expect("Failed to read fingerprint on tests"); + + let mut rustc_verbose_version = String::new(); + fingerprint + .read_to_string(&mut rustc_verbose_version) + .expect("Error reading the fingerprint contents"); + rustc_verbose_version + }; + + let output = std::process::Command::new("rustc") + .arg("-vV") + .stdout(std::process::Stdio::inherit()) + .output() + .expect("Failed to get actual rustc verbose version"); + assert!(get_fingerprint_version(dummy_project.root()) + .as_str() + .contains(String::from_utf8_lossy(&output.stdout).as_ref())); } From ebd0d58c1c11f98e804c80b3378d00f9e1000a95 Mon Sep 17 00:00:00 2001 From: CPerezz Date: Wed, 16 Dec 2020 09:39:51 +0100 Subject: [PATCH 09/22] Apply rustfmt --- tests/testsuite/doc.rs | 54 +++++++++++++++++++++--------------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/tests/testsuite/doc.rs b/tests/testsuite/doc.rs index 830b441d230..509683b1f1e 100644 --- a/tests/testsuite/doc.rs +++ b/tests/testsuite/doc.rs @@ -1770,57 +1770,57 @@ LLVM version: 9.0 .as_str() .contains(String::from_utf8_lossy(&output.stdout).as_ref())); } - + #[cargo_test] fn doc_test_in_workspace() { - let p = project() - .file( - "Cargo.toml", - r#" + let p = project() + .file( + "Cargo.toml", + r#" [workspace] members = [ "crate-a", "crate-b", ] "#, - ) - .file( - "crate-a/Cargo.toml", - r#" + ) + .file( + "crate-a/Cargo.toml", + r#" [project] name = "crate-a" version = "0.1.0" "#, - ) - .file( - "crate-a/src/lib.rs", - "\ + ) + .file( + "crate-a/src/lib.rs", + "\ //! ``` //! assert_eq!(1, 1); //! ``` ", - ) - .file( - "crate-b/Cargo.toml", - r#" + ) + .file( + "crate-b/Cargo.toml", + r#" [project] name = "crate-b" version = "0.1.0" "#, - ) - .file( - "crate-b/src/lib.rs", - "\ + ) + .file( + "crate-b/src/lib.rs", + "\ //! ``` //! assert_eq!(1, 1); //! ``` ", - ) - .build(); - p.cargo("test --doc -vv") - .with_stderr_contains("[DOCTEST] crate-a") - .with_stdout_contains( - " + ) + .build(); + p.cargo("test --doc -vv") + .with_stderr_contains("[DOCTEST] crate-a") + .with_stdout_contains( + " running 1 test test crate-a/src/lib.rs - (line 1) ... ok From 994ccec1367e3e22455e4cb78a98157848d26fb7 Mon Sep 17 00:00:00 2001 From: CPerezz Date: Thu, 17 Dec 2020 02:05:25 +0100 Subject: [PATCH 10/22] Address @ehuss suggestions/nits --- .../compiler/build_context/target_info.rs | 55 +++-- src/cargo/core/compiler/mod.rs | 2 +- tests/testsuite/doc.rs | 200 ++++++++++-------- 3 files changed, 135 insertions(+), 122 deletions(-) diff --git a/src/cargo/core/compiler/build_context/target_info.rs b/src/cargo/core/compiler/build_context/target_info.rs index 5dda0b8d6ee..cf4ea9b7273 100644 --- a/src/cargo/core/compiler/build_context/target_info.rs +++ b/src/cargo/core/compiler/build_context/target_info.rs @@ -4,12 +4,10 @@ use crate::util::config::{Config, StringList, TargetConfig}; use crate::util::{CargoResult, CargoResultExt, ProcessBuilder, Rustc}; use cargo_platform::{Cfg, CfgExpr}; use serde::{Deserialize, Serialize}; -use serde_json::{from_str, to_writer}; use std::cell::RefCell; use std::collections::hash_map::{Entry, HashMap}; use std::env; use std::fs::File; -use std::io::Read; use std::path::{Path, PathBuf}; use std::str::{self, FromStr}; @@ -762,13 +760,13 @@ impl RustcTargetData { /// Structure used to deal with Rustdoc fingerprinting #[derive(Debug, Serialize, Deserialize)] pub struct RustDocFingerprint { - _rustc_verbose_version: String, + rustc_verbose_version: String, } impl RustDocFingerprint { /// Returns the version contained by this `RustDocFingerprint` instance. fn version(&self) -> &str { - self._rustc_verbose_version.as_str() + self.rustc_verbose_version.as_str() } /// Given the `doc` dir, returns the path to the rustdoc fingerprint file. @@ -776,27 +774,19 @@ impl RustDocFingerprint { doc_dir.to_path_buf().join(".rustdoc_fingerprint.json") } - /// Generate an instance of `RustDocFingerprint` from the fingerprint file. - fn from_file(file: &mut File) -> anyhow::Result { - let mut contents = String::new(); - file.read_to_string(&mut contents)?; - let rustdoc_target_data: Self = from_str(&contents)?; - Ok(rustdoc_target_data) - } - /// Write the `RustDocFingerprint` info into the fingerprint file. pub fn write(&self, doc_dir: &Path) -> std::io::Result<()> { let rustdoc_fingerprint_file = File::create(RustDocFingerprint::path_to_fingerprint(doc_dir))?; // We write the actual `Rustc` version to it so that we just need to compile it straight // since there's no `doc/` folder to remove. - to_writer(&rustdoc_fingerprint_file, &self) + serde_json::to_writer(&rustdoc_fingerprint_file, &self) .map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, format!("{:?}", e))) } /// 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, returning `(bool, Self)` where the `bool` says if the `doc` folder was removed. + /// call, returning `(Self, bool)` where the `bool` says if the `doc` folder was removed. /// /// In case it's not, it takes care of removig the `doc/` folder as well as overwriting /// the rustdoc fingerprint info in order to guarantee that we won't end up with mixed @@ -807,30 +797,37 @@ impl RustDocFingerprint { rustc_verbose_ver: &str, ) -> anyhow::Result<(Self, bool)> { let actual_rustdoc_target_data = RustDocFingerprint { - _rustc_verbose_version: rustc_verbose_ver.to_string(), + rustc_verbose_version: rustc_verbose_ver.to_string(), }; // Check wether `.rustdoc_fingerprint.json exists - let mut fingerprint = match File::open(RustDocFingerprint::path_to_fingerprint(doc_dir)) { - Ok(file) => file, + match std::fs::read_to_string(Self::path_to_fingerprint(doc_dir)) { + Ok(rustdoc_data) => { + let rustdoc_fingerprint: Self = match serde_json::from_str(&rustdoc_data) { + Ok(res) => res, + Err(_) => { + crate::util::paths::remove_dir_all(doc_dir)?; + return Ok((actual_rustdoc_target_data, true)); + } + }; + // Check if rustc_version matches the one we just used. Otherways, + // remove the `doc` folder to trigger a re-compilation of the docs. + if rustdoc_fingerprint.version() != actual_rustdoc_target_data.version() { + crate::util::paths::remove_dir_all(doc_dir)?; + Ok((actual_rustdoc_target_data, true)) + } else { + Ok((actual_rustdoc_target_data, false)) + } + } // If the file does not exist, then we cannot assume that the docs were compiled // with the actual Rustc instace 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 suceeds as explained above. - let _ = std::fs::remove_dir_all(doc_dir); - return Ok((actual_rustdoc_target_data, true)); + let _ = crate::util::paths::remove_dir_all(doc_dir); + Ok((actual_rustdoc_target_data, true)) } - }; - - let previous_rustdoc_target_data = RustDocFingerprint::from_file(&mut 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 previous_rustdoc_target_data.version() != actual_rustdoc_target_data.version() { - std::fs::remove_dir_all(doc_dir)?; - return Ok((actual_rustdoc_target_data, true)); - }; - Ok((actual_rustdoc_target_data, false)) + } } } diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 5e0dc595373..335a198eac8 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -662,7 +662,7 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { &mut |line| on_stderr_line(state, line, package_id, &target, &mut output_options), false, ) - .chain_err(|| format!("Could not document `{}`.", name))?; + .chain_err(|| format!("could not document `{}`.", name))?; if doc_dir_removed { fingerprint.write(&doc_dir)? diff --git a/tests/testsuite/doc.rs b/tests/testsuite/doc.rs index 509683b1f1e..a8698ed3b90 100644 --- a/tests/testsuite/doc.rs +++ b/tests/testsuite/doc.rs @@ -4,8 +4,8 @@ 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 serde::{Deserialize, Serialize}; use std::fs; -use std::io::Read; use std::str; #[cargo_test] @@ -1607,6 +1607,11 @@ fn crate_versions() { #[cargo_test] fn crate_versions_flag_is_overridden() { + #[derive(Debug, Serialize, Deserialize)] + pub struct RustDocFingerprint { + rustc_verbose_version: String, + } + let p = project() .file( "Cargo.toml", @@ -1640,48 +1645,13 @@ fn crate_versions_flag_is_overridden() { asserts(output_documentation()); } -#[cargo_test] -fn rustc_adds_doc_fingerprint() { - let p = project() - .file( - "Cargo.toml", - r#" - [package] - name = "foo" - version = "0.1.0" - authors = [] - "#, - ) - .file("src/lib.rs", "//! Hello") - .build(); - - p.cargo("doc").run(); - - // Read fingerprint rustc version and check it matches the expected one - let mut fingerprint = fs::File::open( - p.root() - .join("target") - .join("doc") - .join(".rustdoc_fingerprint.json"), - ) - .expect("Failed to read fingerprint"); - - let mut rustc_verbose_version = String::new(); - fingerprint - .read_to_string(&mut rustc_verbose_version) - .expect("Error reading the fingerprint contents"); - let output = std::process::Command::new("rustc") - .arg("-vV") - .stdout(std::process::Stdio::inherit()) - .output() - .expect("Failed to get actual rustc verbose version"); - assert!(rustc_verbose_version - .as_str() - .contains(String::from_utf8_lossy(&output.stdout).as_ref())); -} - #[cargo_test] fn doc_fingerprint_versioning_consistent() { + #[derive(Debug, Serialize, Deserialize)] + pub struct RustDocFingerprint { + pub rustc_verbose_version: String, + } + // Test that using different Rustc versions forces a // doc re-compilation producing new html, css & js files. @@ -1704,21 +1674,47 @@ LLVM version: 9.0 .file( "Cargo.toml", r#" - [package] - name = "foo" - version = "1.2.4" - authors = [] - "#, + [package] + name = "foo" + version = "1.2.4" + authors = [] + "#, ) .file("src/lib.rs", "//! These are the docs!") .build(); dummy_project.cargo("doc").run(); - // As the test shows avobe. Now we have generated the `doc/` folder and inside - // the rustdoc fingerprint file is located. + let fingerprint: RustDocFingerprint = serde_json::from_str( + dummy_project + .read_file( + std::path::PathBuf::from("target") + .join("doc") + .join(".rustdoc_fingerprint.json") + .to_str() + .expect("Malformed path"), + ) + .as_str(), + ) + .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") + .stdout(std::process::Stdio::piped()) + .output() + .expect("Failed to get actual rustc verbose version"); + assert_eq!( + fingerprint.rustc_verbose_version, + (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. + // 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. fs::remove_file( dummy_project .root() @@ -1738,37 +1734,57 @@ LLVM version: 9.0 ) .expect("Error writing old rustc version"); + fs::write( + dummy_project + .root() + .join("target") + .join("doc") + .join("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.change_file("src/lib.rs", "//! These are the docs 2!"); dummy_project.cargo("doc").run(); - // Edit the fingerprint rustc version. - // Read fingerprint rustc version and return it as String - let get_fingerprint_version = |root_path: std::path::PathBuf| -> String { - let mut fingerprint = fs::File::open( - root_path - .join("target") - .join("doc") - .join(".rustdoc_fingerprint.json"), - ) - .expect("Failed to read fingerprint on tests"); - - let mut rustc_verbose_version = String::new(); - fingerprint - .read_to_string(&mut rustc_verbose_version) - .expect("Error reading the fingerprint contents"); - rustc_verbose_version - }; + assert!(fs::File::open( + dummy_project + .root() + .join("target") + .join("doc") + .join("bogus_file") + ) + .is_err()); + + let fingerprint: RustDocFingerprint = serde_json::from_str( + dummy_project + .read_file( + std::path::PathBuf::from("target") + .join("doc") + .join(".rustdoc_fingerprint.json") + .to_str() + .expect("Malformed path"), + ) + .as_str(), + ) + .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") - .stdout(std::process::Stdio::inherit()) + .stdout(std::process::Stdio::piped()) .output() .expect("Failed to get actual rustc verbose version"); - assert!(get_fingerprint_version(dummy_project.root()) - .as_str() - .contains(String::from_utf8_lossy(&output.stdout).as_ref())); + + assert_eq!( + fingerprint.rustc_verbose_version, + (String::from_utf8_lossy(&output.stdout).as_ref()) + ); } #[cargo_test] @@ -1777,44 +1793,44 @@ fn doc_test_in_workspace() { .file( "Cargo.toml", r#" - [workspace] - members = [ - "crate-a", - "crate-b", - ] - "#, + [workspace] + members = [ + "crate-a", + "crate-b", + ] + "#, ) .file( "crate-a/Cargo.toml", r#" - [project] - name = "crate-a" - version = "0.1.0" - "#, + [project] + name = "crate-a" + version = "0.1.0" + "#, ) .file( "crate-a/src/lib.rs", "\ - //! ``` - //! assert_eq!(1, 1); - //! ``` - ", + //! ``` + //! assert_eq!(1, 1); + //! ``` + ", ) .file( "crate-b/Cargo.toml", r#" - [project] - name = "crate-b" - version = "0.1.0" - "#, + [project] + name = "crate-b" + version = "0.1.0" + "#, ) .file( "crate-b/src/lib.rs", "\ - //! ``` - //! assert_eq!(1, 1); - //! ``` - ", + //! ``` + //! assert_eq!(1, 1); + //! ``` + ", ) .build(); p.cargo("test --doc -vv") From 33a524811a0e21a69d5a36a7e08ca588018939a6 Mon Sep 17 00:00:00 2001 From: CPerezz Date: Tue, 26 Jan 2021 00:00:24 +0100 Subject: [PATCH 11/22] Move rustdoc fingerprint checks earlier As @ehuss suggested, we should only execute once the rustdoc fingerprint check per build, not once per dep. Therefore the checks have been moved to an earlier stage. --- src/cargo/core/compiler/context/mod.rs | 21 ++++++++++++++++++++- src/cargo/core/compiler/mod.rs | 17 +---------------- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/cargo/core/compiler/context/mod.rs b/src/cargo/core/compiler/context/mod.rs index 506e3932332..16166bebf8d 100644 --- a/src/cargo/core/compiler/context/mod.rs +++ b/src/cargo/core/compiler/context/mod.rs @@ -12,7 +12,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, +}; use crate::core::compiler::{self, compilation, Unit}; use crate::core::PackageId; use crate::util::errors::{CargoResult, CargoResultExt}; @@ -131,6 +133,23 @@ impl<'a, 'cfg> Context<'a, 'cfg> { custom_build::build_map(&mut self)?; self.check_collistions()?; + // 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 + // compiler versions of these files. + let doc_dir = self.files().host_root().join("doc"); + let (fingerprint, doc_dir_removed) = RustDocFingerprint::check_rustdoc_fingerprint( + &doc_dir, + self.bcx.rustc().verbose_version.as_str(), + )?; + if doc_dir_removed { + fingerprint.write(&doc_dir)? + }; + 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 diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 335a198eac8..d9a43d449d0 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -592,18 +592,6 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { rustdoc.arg("--target").arg(target.rustc_target()); } let doc_dir = cx.files().out_dir(unit); - // 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 - // compiler versions of these files. - let (fingerprint, doc_dir_removed) = RustDocFingerprint::check_rustdoc_fingerprint( - &doc_dir, - cx.bcx.rustc().verbose_version.as_str(), - )?; // Create the documentation directory ahead of time as rustdoc currently has // a bug where concurrent invocations will race to create this directory if @@ -662,11 +650,8 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { &mut |line| on_stderr_line(state, line, package_id, &target, &mut output_options), false, ) - .chain_err(|| format!("could not document `{}`.", name))?; + .chain_err(|| format!("could not document `{}`", name))?; - if doc_dir_removed { - fingerprint.write(&doc_dir)? - }; Ok(()) })) } From 94519f2649cb843ad0162645858054ae71b5f663 Mon Sep 17 00:00:00 2001 From: CPerezz Date: Tue, 26 Jan 2021 00:01:59 +0100 Subject: [PATCH 12/22] Create dirs if needed before f_p write call Once `RustDocFingerprint::check_rustdoc_fingerprint()` is executed it might happen that the `doc/` dir is removed. This means that when we call `fingerprint.write()` we need to create the `doc` directory again. --- .../compiler/build_context/target_info.rs | 2 + src/cargo/core/compiler/mod.rs | 7 +- tests/testsuite/doc.rs | 85 ++----------------- 3 files changed, 9 insertions(+), 85 deletions(-) diff --git a/src/cargo/core/compiler/build_context/target_info.rs b/src/cargo/core/compiler/build_context/target_info.rs index cf4ea9b7273..0c8cb5d4b23 100644 --- a/src/cargo/core/compiler/build_context/target_info.rs +++ b/src/cargo/core/compiler/build_context/target_info.rs @@ -776,6 +776,8 @@ impl RustDocFingerprint { /// Write the `RustDocFingerprint` info into the fingerprint file. pub fn write(&self, doc_dir: &Path) -> std::io::Result<()> { + crate::util::paths::create_dir_all(doc_dir) + .map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, format!("{:?}", e)))?; let rustdoc_fingerprint_file = File::create(RustDocFingerprint::path_to_fingerprint(doc_dir))?; // We write the actual `Rustc` version to it so that we just need to compile it straight diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index d9a43d449d0..a1a443f5308 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -32,9 +32,7 @@ use lazycell::LazyCell; use log::debug; pub use self::build_config::{BuildConfig, CompileMode, MessageFormat}; -pub use self::build_context::{ - BuildContext, FileFlavor, FileType, RustDocFingerprint, RustcTargetData, TargetInfo, -}; +pub use self::build_context::{BuildContext, FileFlavor, FileType, TargetInfo}; use self::build_plan::BuildPlan; pub use self::compilation::{Compilation, Doctest}; pub use self::compile_kind::{CompileKind, CompileTarget}; @@ -598,7 +596,7 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { // it doesn't already exist. paths::create_dir_all(&doc_dir)?; - rustdoc.arg("-o").arg(doc_dir.clone()); + rustdoc.arg("-o").arg(doc_dir); for feat in &unit.features { rustdoc.arg("--cfg").arg(&format!("feature=\"{}\"", feat)); @@ -651,7 +649,6 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { false, ) .chain_err(|| format!("could not document `{}`", name))?; - Ok(()) })) } diff --git a/tests/testsuite/doc.rs b/tests/testsuite/doc.rs index a8698ed3b90..16de4a0b9ca 100644 --- a/tests/testsuite/doc.rs +++ b/tests/testsuite/doc.rs @@ -1607,11 +1607,6 @@ fn crate_versions() { #[cargo_test] fn crate_versions_flag_is_overridden() { - #[derive(Debug, Serialize, Deserialize)] - pub struct RustDocFingerprint { - rustc_verbose_version: String, - } - let p = project() .file( "Cargo.toml", @@ -1674,11 +1669,11 @@ LLVM version: 9.0 .file( "Cargo.toml", r#" - [package] - name = "foo" - version = "1.2.4" - authors = [] - "#, + [package] + name = "foo" + version = "1.2.4" + authors = [] + "#, ) .file("src/lib.rs", "//! These are the docs!") .build(); @@ -1786,73 +1781,3 @@ LLVM version: 9.0 (String::from_utf8_lossy(&output.stdout).as_ref()) ); } - -#[cargo_test] -fn doc_test_in_workspace() { - let p = project() - .file( - "Cargo.toml", - r#" - [workspace] - members = [ - "crate-a", - "crate-b", - ] - "#, - ) - .file( - "crate-a/Cargo.toml", - r#" - [project] - name = "crate-a" - version = "0.1.0" - "#, - ) - .file( - "crate-a/src/lib.rs", - "\ - //! ``` - //! assert_eq!(1, 1); - //! ``` - ", - ) - .file( - "crate-b/Cargo.toml", - r#" - [project] - name = "crate-b" - version = "0.1.0" - "#, - ) - .file( - "crate-b/src/lib.rs", - "\ - //! ``` - //! assert_eq!(1, 1); - //! ``` - ", - ) - .build(); - p.cargo("test --doc -vv") - .with_stderr_contains("[DOCTEST] crate-a") - .with_stdout_contains( - " -running 1 test -test crate-a/src/lib.rs - (line 1) ... ok - -test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out[..] - -", - ) - .with_stderr_contains("[DOCTEST] crate-b") - .with_stdout_contains( - " -running 1 test -test crate-b/src/lib.rs - (line 1) ... ok - -test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out[..] - -", - ) - .run(); -} From 27987b670640fb8c79ee148575eaa384d3f54e44 Mon Sep 17 00:00:00 2001 From: CPerezz Date: Tue, 26 Jan 2021 00:52:07 +0100 Subject: [PATCH 13/22] Add rustdoc fingerprint exception for clean --- tests/testsuite/clean.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testsuite/clean.rs b/tests/testsuite/clean.rs index 49986070b85..dae255433d5 100644 --- a/tests/testsuite/clean.rs +++ b/tests/testsuite/clean.rs @@ -432,7 +432,7 @@ fn assert_all_clean(build_dir: &Path) { }) { let entry = entry.unwrap(); let path = entry.path(); - if let ".rustc_info.json" | ".cargo-lock" | "CACHEDIR.TAG" = + if let ".rustc_info.json" | ".cargo-lock" | "CACHEDIR.TAG" | ".rustdoc_fingerprint.json" = path.file_name().unwrap().to_str().unwrap() { continue; From 8ddc56f14d26ba1612b9e425b25b02bda2afb02c Mon Sep 17 00:00:00 2001 From: CPerezz Date: Tue, 26 Jan 2021 01:26:39 +0100 Subject: [PATCH 14/22] Export `RustDocFingerprint` from `core/compiler` --- src/cargo/core/compiler/mod.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index e700bef94fc..51938dfcd46 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/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, TargetInfo}; +pub use self::build_context::{ + BuildContext, FileFlavor, FileType, RustDocFingerprint, RustcTargetData, TargetInfo, +}; use self::build_plan::BuildPlan; pub use self::compilation::{Compilation, Doctest}; pub use self::compile_kind::{CompileKind, CompileTarget}; From d2572a2800bead998a86cd8409385094f697f35e Mon Sep 17 00:00:00 2001 From: CPerezz Date: Wed, 27 Jan 2021 01:14:31 +0100 Subject: [PATCH 15/22] Address latest reivew suggestions - Instead of `fs` we use the `utils::paths` functions to interact with the filesystem. - The doc fingerprint is now stored under `target/` instead of `target/doc/`. - The code in `compile` has been reduced to a single function call. --- .../compiler/build_context/target_info.rs | 82 ++++++++--------- src/cargo/core/compiler/context/mod.rs | 13 +-- tests/testsuite/clean.rs | 2 +- tests/testsuite/doc.rs | 87 ++++--------------- 4 files changed, 59 insertions(+), 125 deletions(-) diff --git a/src/cargo/core/compiler/build_context/target_info.rs b/src/cargo/core/compiler/build_context/target_info.rs index 4bfcaf6d080..a3c59f43f8c 100644 --- a/src/cargo/core/compiler/build_context/target_info.rs +++ b/src/cargo/core/compiler/build_context/target_info.rs @@ -1,14 +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::fs::File; -use std::path::{Path, PathBuf}; +use std::path::PathBuf; use std::str::{self, FromStr}; /// Information about the platform target gleaned from querying rustc. @@ -754,65 +755,53 @@ impl RustcTargetData { /// Structure used to deal with Rustdoc fingerprinting #[derive(Debug, Serialize, Deserialize)] pub struct RustDocFingerprint { - rustc_verbose_version: String, + rustc_vv: String, } impl RustDocFingerprint { - /// Returns the version contained by this `RustDocFingerprint` instance. - fn version(&self) -> &str { - self.rustc_verbose_version.as_str() - } - - /// Given the `doc` dir, returns the path to the rustdoc fingerprint file. - fn path_to_fingerprint(doc_dir: &Path) -> PathBuf { - doc_dir.to_path_buf().join(".rustdoc_fingerprint.json") + /// Read the `RustDocFingerprint` info from the fingerprint file. + fn read<'a, 'cfg>(cx: &Context<'a, 'cfg>) -> CargoResult { + let rustdoc_data = paths::read( + &cx.files() + .host_root() + .join(".rustdoc_fingerprint") + .with_extension("json"), + )?; + serde_json::from_str(&rustdoc_data).map_err(|e| anyhow::anyhow!("{:?}", e)) } /// Write the `RustDocFingerprint` info into the fingerprint file. - pub fn write(&self, doc_dir: &Path) -> std::io::Result<()> { - crate::util::paths::create_dir_all(doc_dir) - .map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, format!("{:?}", e)))?; - let rustdoc_fingerprint_file = - File::create(RustDocFingerprint::path_to_fingerprint(doc_dir))?; - // We write the actual `Rustc` version to it so that we just need to compile it straight - // since there's no `doc/` folder to remove. - serde_json::to_writer(&rustdoc_fingerprint_file, &self) - .map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, format!("{:?}", e))) + fn write<'a, 'cfg>(&self, cx: &Context<'a, 'cfg>) -> CargoResult<()> { + paths::write( + &cx.files() + .host_root() + .join(".rustdoc_fingerprint.json") + .with_extension("json"), + serde_json::to_string(&self)?.as_bytes(), + ) } /// 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, returning `(Self, bool)` where the `bool` says if the `doc` folder was removed. + /// call. /// /// In case it's not, it takes care of removig 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 `Rustc` autogenerates which do not have /// any versioning. - pub fn check_rustdoc_fingerprint( - doc_dir: &Path, - rustc_verbose_ver: &str, - ) -> anyhow::Result<(Self, bool)> { + pub fn check_rustdoc_fingerprint<'a, 'cfg>(cx: &Context<'a, 'cfg>) -> CargoResult<()> { let actual_rustdoc_target_data = RustDocFingerprint { - rustc_verbose_version: rustc_verbose_ver.to_string(), + rustc_vv: cx.bcx.rustc().verbose_version.clone(), }; - - // Check wether `.rustdoc_fingerprint.json exists - match std::fs::read_to_string(Self::path_to_fingerprint(doc_dir)) { - Ok(rustdoc_data) => { - let rustdoc_fingerprint: Self = match serde_json::from_str(&rustdoc_data) { - Ok(res) => res, - Err(_) => { - crate::util::paths::remove_dir_all(doc_dir)?; - return Ok((actual_rustdoc_target_data, true)); - } - }; + let doc_dir = cx.files().host_root().join("doc"); + // 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 rustdoc_fingerprint.version() != actual_rustdoc_target_data.version() { - crate::util::paths::remove_dir_all(doc_dir)?; - Ok((actual_rustdoc_target_data, true)) - } else { - Ok((actual_rustdoc_target_data, false)) + if fingerprint.rustc_vv != actual_rustdoc_target_data.rustc_vv { + paths::remove_dir_all(&doc_dir)?; + actual_rustdoc_target_data.write(cx)? } } // If the file does not exist, then we cannot assume that the docs were compiled @@ -821,9 +810,10 @@ impl RustDocFingerprint { // exists neither, we simply do nothing and continue. Err(_) => { // We don't care if this suceeds as explained above. - let _ = crate::util::paths::remove_dir_all(doc_dir); - Ok((actual_rustdoc_target_data, true)) + let _ = paths::remove_dir_all(doc_dir); + actual_rustdoc_target_data.write(cx)? } } + Ok(()) } } diff --git a/src/cargo/core/compiler/context/mod.rs b/src/cargo/core/compiler/context/mod.rs index df8c7bc0ff0..6def6729e23 100644 --- a/src/cargo/core/compiler/context/mod.rs +++ b/src/cargo/core/compiler/context/mod.rs @@ -140,15 +140,10 @@ impl<'a, 'cfg> Context<'a, 'cfg> { // 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 - // compiler versions of these files. - let doc_dir = self.files().host_root().join("doc"); - let (fingerprint, doc_dir_removed) = RustDocFingerprint::check_rustdoc_fingerprint( - &doc_dir, - self.bcx.rustc().verbose_version.as_str(), - )?; - if doc_dir_removed { - fingerprint.write(&doc_dir)? - }; + // 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 diff --git a/tests/testsuite/clean.rs b/tests/testsuite/clean.rs index 8da5ba59120..f79619b15d2 100644 --- a/tests/testsuite/clean.rs +++ b/tests/testsuite/clean.rs @@ -432,7 +432,7 @@ fn assert_all_clean(build_dir: &Path) { }) { let entry = entry.unwrap(); let path = entry.path(); - if let ".rustc_info.json" | ".cargo-lock" | "CACHEDIR.TAG" | ".rustdoc_fingerprint.json" = + if let ".rustc_info.json" | ".cargo-lock" | "CACHEDIR.TAG" = path.file_name().unwrap().to_str().unwrap() { continue; diff --git a/tests/testsuite/doc.rs b/tests/testsuite/doc.rs index 16de4a0b9ca..daeda24dcac 100644 --- a/tests/testsuite/doc.rs +++ b/tests/testsuite/doc.rs @@ -1644,14 +1644,14 @@ fn crate_versions_flag_is_overridden() { fn doc_fingerprint_versioning_consistent() { #[derive(Debug, Serialize, Deserialize)] pub struct RustDocFingerprint { - pub rustc_verbose_version: String, + rustc_vv: String, } // Test that using different Rustc versions forces a // doc re-compilation producing new html, css & js files. // Random rustc verbose version - let stable_rustc_verbose_version = format!( + let old_rustc_verbose_version = format!( "\ rustc 1.41.1 (f3e1a954d 2020-02-24) binary: rustc @@ -1680,28 +1680,18 @@ LLVM version: 9.0 dummy_project.cargo("doc").run(); - let fingerprint: RustDocFingerprint = serde_json::from_str( - dummy_project - .read_file( - std::path::PathBuf::from("target") - .join("doc") - .join(".rustdoc_fingerprint.json") - .to_str() - .expect("Malformed path"), - ) - .as_str(), - ) - .expect("JSON Serde fail"); + 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") - .stdout(std::process::Stdio::piped()) .output() .expect("Failed to get actual rustc verbose version"); assert_eq!( - fingerprint.rustc_verbose_version, + fingerprint.rustc_vv, (String::from_utf8_lossy(&output.stdout).as_ref()) ); @@ -1710,31 +1700,13 @@ LLVM version: 9.0 // 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. - fs::remove_file( - dummy_project - .root() - .join("target") - .join("doc") - .join(".rustdoc_fingerprint.json"), - ) - .expect("Failed to read fingerprint on tests"); - - fs::write( - dummy_project - .root() - .join("target") - .join("doc") - .join(".rustdoc_fingerprint.json"), - stable_rustc_verbose_version, - ) - .expect("Error writing old rustc version"); + dummy_project.change_file( + "target/.rustdoc_fingerprint.json", + &old_rustc_verbose_version, + ); fs::write( - dummy_project - .root() - .join("target") - .join("doc") - .join("bogus_file"), + 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"); @@ -1742,42 +1714,19 @@ LLVM version: 9.0 // 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.change_file("src/lib.rs", "//! These are the docs 2!"); + // It should also remove the bogus file we created above. dummy_project.cargo("doc").run(); - assert!(fs::File::open( - dummy_project - .root() - .join("target") - .join("doc") - .join("bogus_file") - ) - .is_err()); - - let fingerprint: RustDocFingerprint = serde_json::from_str( - dummy_project - .read_file( - std::path::PathBuf::from("target") - .join("doc") - .join(".rustdoc_fingerprint.json") - .to_str() - .expect("Malformed path"), - ) - .as_str(), - ) - .expect("JSON Serde fail"); + 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. - let output = std::process::Command::new("rustc") - .arg("-vV") - .stdout(std::process::Stdio::piped()) - .output() - .expect("Failed to get actual rustc verbose version"); - assert_eq!( - fingerprint.rustc_verbose_version, + fingerprint.rustc_vv, (String::from_utf8_lossy(&output.stdout).as_ref()) ); } From 7c450213286e491cb8809606e78159e0470135fc Mon Sep 17 00:00:00 2001 From: CPerezz Date: Sun, 31 Jan 2021 23:43:27 +0100 Subject: [PATCH 16/22] Check target-dependant doc folders When checking the fingerprint for rustdoc and applying the corresponding logic, we don't only need to consider the `target/doc` folder (Host target) but also triple targets. So now the actual compilation targets are checked during the rustdoc_fingerprint processing and they're treated as the Host/doc folder. --- .../compiler/build_context/target_info.rs | 38 ++++--- tests/testsuite/doc.rs | 104 ++++++++++++++++-- 2 files changed, 117 insertions(+), 25 deletions(-) diff --git a/src/cargo/core/compiler/build_context/target_info.rs b/src/cargo/core/compiler/build_context/target_info.rs index a3c59f43f8c..f98bf903466 100644 --- a/src/cargo/core/compiler/build_context/target_info.rs +++ b/src/cargo/core/compiler/build_context/target_info.rs @@ -9,7 +9,7 @@ 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. @@ -755,28 +755,20 @@ impl RustcTargetData { /// Structure used to deal with Rustdoc fingerprinting #[derive(Debug, Serialize, Deserialize)] pub struct RustDocFingerprint { - rustc_vv: String, + pub rustc_vv: String, } impl RustDocFingerprint { /// Read the `RustDocFingerprint` info from the fingerprint file. fn read<'a, 'cfg>(cx: &Context<'a, 'cfg>) -> CargoResult { - let rustdoc_data = paths::read( - &cx.files() - .host_root() - .join(".rustdoc_fingerprint") - .with_extension("json"), - )?; + 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") - .with_extension("json"), + &cx.files().host_root().join(".rustdoc_fingerprint.json"), serde_json::to_string(&self)?.as_bytes(), ) } @@ -793,14 +785,28 @@ impl RustDocFingerprint { let actual_rustdoc_target_data = RustDocFingerprint { rustc_vv: cx.bcx.rustc().verbose_version.clone(), }; - let doc_dir = cx.files().host_root().join("doc"); + + // Collect all of the target doc paths for which the docs need to be compiled for. + let doc_dirs: Vec = cx + .compilation + .root_output + .iter() + .map(|(ck, _)| match ck { + CompileKind::Host => cx.files().host_root().to_path_buf(), + CompileKind::Target(t) => cx.files().host_root().join(Path::new(t.rustc_target())), + }) + .map(|path| path.join("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 { - paths::remove_dir_all(&doc_dir)?; + doc_dirs + .iter() + .try_for_each(|path| paths::remove_dir_all(&path))?; actual_rustdoc_target_data.write(cx)? } } @@ -810,7 +816,9 @@ impl RustDocFingerprint { // exists neither, we simply do nothing and continue. Err(_) => { // We don't care if this suceeds as explained above. - let _ = paths::remove_dir_all(doc_dir); + let _ = doc_dirs + .iter() + .try_for_each(|path| paths::remove_dir_all(&path)); actual_rustdoc_target_data.write(cx)? } } diff --git a/tests/testsuite/doc.rs b/tests/testsuite/doc.rs index daeda24dcac..610a57c001d 100644 --- a/tests/testsuite/doc.rs +++ b/tests/testsuite/doc.rs @@ -1,10 +1,10 @@ //! 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}; use cargo_test_support::{is_nightly, rustc_host}; -use serde::{Deserialize, Serialize}; use std::fs; use std::str; @@ -1641,15 +1641,7 @@ fn crate_versions_flag_is_overridden() { } #[cargo_test] -fn doc_fingerprint_versioning_consistent() { - #[derive(Debug, Serialize, Deserialize)] - pub struct RustDocFingerprint { - rustc_vv: String, - } - - // Test that using different Rustc versions forces a - // doc re-compilation producing new html, css & js files. - +fn doc_fingerprint_is_versioning_consistent() { // Random rustc verbose version let old_rustc_verbose_version = format!( "\ @@ -1730,3 +1722,95 @@ LLVM version: 9.0 (String::from_utf8_lossy(&output.stdout).as_ref()) ); } + +#[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()) + ); +} From cb21e6424b51427eff2992e3559b3b1609000f7e Mon Sep 17 00:00:00 2001 From: CPerezz Date: Mon, 1 Feb 2021 01:11:36 +0100 Subject: [PATCH 17/22] Improve & fix doc dir removal process --- .../compiler/build_context/target_info.rs | 21 +++++++++++++------ tests/testsuite/doc.rs | 1 + 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/cargo/core/compiler/build_context/target_info.rs b/src/cargo/core/compiler/build_context/target_info.rs index f98bf903466..e4002fcad69 100644 --- a/src/cargo/core/compiler/build_context/target_info.rs +++ b/src/cargo/core/compiler/build_context/target_info.rs @@ -773,6 +773,19 @@ impl RustDocFingerprint { ) } + fn remove_doc_dirs(doc_dirs: &Vec) -> CargoResult<()> { + let errs: Vec> = doc_dirs + .iter() + .map(|path| paths::remove_dir_all(&path)) + .filter(|res| res.is_err()) + .collect(); + if !errs.is_empty() { + Err(anyhow::anyhow!("Dir removal error")) + } else { + Ok(()) + } + } + /// 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. @@ -804,9 +817,7 @@ impl RustDocFingerprint { // 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 { - doc_dirs - .iter() - .try_for_each(|path| paths::remove_dir_all(&path))?; + Self::remove_doc_dirs(&doc_dirs)?; actual_rustdoc_target_data.write(cx)? } } @@ -816,9 +827,7 @@ impl RustDocFingerprint { // exists neither, we simply do nothing and continue. Err(_) => { // We don't care if this suceeds as explained above. - let _ = doc_dirs - .iter() - .try_for_each(|path| paths::remove_dir_all(&path)); + let _ = Self::remove_doc_dirs(&doc_dirs); actual_rustdoc_target_data.write(cx)? } } diff --git a/tests/testsuite/doc.rs b/tests/testsuite/doc.rs index 610a57c001d..19a720a8d5b 100644 --- a/tests/testsuite/doc.rs +++ b/tests/testsuite/doc.rs @@ -1723,6 +1723,7 @@ LLVM version: 9.0 ); } +#[cfg(target_os = "linux")] #[cargo_test] fn doc_fingerprint_respects_target_paths() { // Random rustc verbose version From 1af0027ff34a4941fd7a1b4a05ba800ee9194aae Mon Sep 17 00:00:00 2001 From: CPerezz Date: Wed, 3 Feb 2021 19:38:46 +0100 Subject: [PATCH 18/22] Fix upstream changes of #9122 --- src/cargo/core/compiler/context/mod.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/cargo/core/compiler/context/mod.rs b/src/cargo/core/compiler/context/mod.rs index 8eb70beeb62..910c0c77370 100644 --- a/src/cargo/core/compiler/context/mod.rs +++ b/src/cargo/core/compiler/context/mod.rs @@ -21,10 +21,6 @@ use super::unit_graph::UnitDep; use super::{ BuildContext, Compilation, CompileKind, CompileMode, Executor, FileFlavor, RustDocFingerprint, }; -use crate::core::compiler::{self, compilation, Unit}; -use crate::core::PackageId; -use crate::util::errors::{CargoResult, CargoResultExt}; -use crate::util::profile; mod compilation_files; use self::compilation_files::CompilationFiles; From 7bfb3d09828da4bc43a8bb599c6f6615b570b1a2 Mon Sep 17 00:00:00 2001 From: CPerezz Date: Wed, 3 Feb 2021 23:50:59 +0100 Subject: [PATCH 19/22] Fix spelling --- src/cargo/core/compiler/build_context/target_info.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cargo/core/compiler/build_context/target_info.rs b/src/cargo/core/compiler/build_context/target_info.rs index e4002fcad69..07d8cfc0418 100644 --- a/src/cargo/core/compiler/build_context/target_info.rs +++ b/src/cargo/core/compiler/build_context/target_info.rs @@ -790,9 +790,9 @@ impl RustDocFingerprint { /// `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 removig the `doc/` folder as well as overwriting + /// 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 `Rustc` autogenerates which do not have + /// 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 { @@ -822,11 +822,11 @@ impl RustDocFingerprint { } } // If the file does not exist, then we cannot assume that the docs were compiled - // with the actual Rustc instace version. Therefore, we try to remove the + // 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 suceeds as explained above. + // We don't care if this succeeds as explained above. let _ = Self::remove_doc_dirs(&doc_dirs); actual_rustdoc_target_data.write(cx)? } From e78f91c04e1c9dbf1a33f3334c19339e9abccccb Mon Sep 17 00:00:00 2001 From: CPerezz Date: Thu, 4 Feb 2021 00:09:36 +0100 Subject: [PATCH 20/22] Refactor doc_dirs obtention We should try to avoid dealing with paths ourseleves. Now the access/parsing and construction is handled via `Layout`. --- .../core/compiler/build_context/target_info.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/cargo/core/compiler/build_context/target_info.rs b/src/cargo/core/compiler/build_context/target_info.rs index 07d8cfc0418..dd052e72204 100644 --- a/src/cargo/core/compiler/build_context/target_info.rs +++ b/src/cargo/core/compiler/build_context/target_info.rs @@ -773,7 +773,7 @@ impl RustDocFingerprint { ) } - fn remove_doc_dirs(doc_dirs: &Vec) -> CargoResult<()> { + fn remove_doc_dirs(doc_dirs: &Vec<&Path>) -> CargoResult<()> { let errs: Vec> = doc_dirs .iter() .map(|path| paths::remove_dir_all(&path)) @@ -800,15 +800,11 @@ impl RustDocFingerprint { }; // Collect all of the target doc paths for which the docs need to be compiled for. - let doc_dirs: Vec = cx - .compilation - .root_output + let doc_dirs: Vec<&Path> = cx + .bcx + .all_kinds .iter() - .map(|(ck, _)| match ck { - CompileKind::Host => cx.files().host_root().to_path_buf(), - CompileKind::Target(t) => cx.files().host_root().join(Path::new(t.rustc_target())), - }) - .map(|path| path.join("doc")) + .map(|kind| cx.files().layout(*kind).doc()) .collect(); // Check wether `.rustdoc_fingerprint.json` exists From 4afa58511fd7ec3a838f8fe9c49f67b7adfe5edd Mon Sep 17 00:00:00 2001 From: CPerezz Date: Thu, 4 Feb 2021 01:03:04 +0100 Subject: [PATCH 21/22] Improve RustDocFingerprint::remove_doc_dirs We were not filtering the cases where the doc folder paths did not exist. Also, we were overrwriting the error when it doesn't make sense. We now return the first one by folding the results of the doc_dirs removal resolutions. --- src/cargo/core/compiler/build_context/target_info.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/cargo/core/compiler/build_context/target_info.rs b/src/cargo/core/compiler/build_context/target_info.rs index dd052e72204..8dc3b516f35 100644 --- a/src/cargo/core/compiler/build_context/target_info.rs +++ b/src/cargo/core/compiler/build_context/target_info.rs @@ -774,16 +774,13 @@ impl RustDocFingerprint { } fn remove_doc_dirs(doc_dirs: &Vec<&Path>) -> CargoResult<()> { - let errs: Vec> = doc_dirs + doc_dirs .iter() + .filter(|path| path.exists()) .map(|path| paths::remove_dir_all(&path)) .filter(|res| res.is_err()) - .collect(); - if !errs.is_empty() { - Err(anyhow::anyhow!("Dir removal error")) - } else { - Ok(()) - } + .collect::>>() + .map(|_| ()) } /// This function checks whether the latest version of `Rustc` used to compile this From 61c233209a690c532c57aa213c3736ba8f83165c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez?= <37264926+CPerezz@users.noreply.github.com> Date: Sat, 13 Feb 2021 21:40:09 +0100 Subject: [PATCH 22/22] Update src/cargo/core/compiler/build_context/target_info.rs Co-authored-by: Eric Huss --- src/cargo/core/compiler/build_context/target_info.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/cargo/core/compiler/build_context/target_info.rs b/src/cargo/core/compiler/build_context/target_info.rs index 8dc3b516f35..816c6561207 100644 --- a/src/cargo/core/compiler/build_context/target_info.rs +++ b/src/cargo/core/compiler/build_context/target_info.rs @@ -778,9 +778,7 @@ impl RustDocFingerprint { .iter() .filter(|path| path.exists()) .map(|path| paths::remove_dir_all(&path)) - .filter(|res| res.is_err()) - .collect::>>() - .map(|_| ()) + .collect::>() } /// This function checks whether the latest version of `Rustc` used to compile this