From d17c85132bf7f69463d3c4a484871a2a73eecf89 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Thu, 15 Apr 2021 17:49:38 -0700 Subject: [PATCH 1/5] Make doc_fingerprint_respects_target_paths work on all targets. --- tests/testsuite/doc.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/tests/testsuite/doc.rs b/tests/testsuite/doc.rs index c06e13a3fe6..a5fadaf832e 100644 --- a/tests/testsuite/doc.rs +++ b/tests/testsuite/doc.rs @@ -1800,7 +1800,6 @@ LLVM version: 9.0 ); } -#[cfg(target_os = "linux")] #[cargo_test] fn doc_fingerprint_respects_target_paths() { // Random rustc verbose version @@ -1831,9 +1830,7 @@ LLVM version: 9.0 .file("src/lib.rs", "//! These are the docs!") .build(); - dummy_project - .cargo("doc --target x86_64-unknown-linux-gnu") - .run(); + dummy_project.cargo("doc --target").arg(rustc_host()).run(); let fingerprint: RustDocFingerprint = serde_json::from_str(&dummy_project.read_file("target/.rustdoc_fingerprint.json")) @@ -1863,7 +1860,8 @@ LLVM version: 9.0 fs::write( dummy_project .build_dir() - .join("x86_64-unknown-linux-gnu/doc/bogus_file"), + .join(rustc_host()) + .join("doc/bogus_file"), String::from("This is a bogus file and should be removed!"), ) .expect("Error writing test bogus file"); @@ -1872,13 +1870,12 @@ LLVM version: 9.0 // of rustc, cargo should remove the entire `/doc` folder (including the fingerprint) // and generating another one with the actual version. // It should also remove the bogus file we created above. - dummy_project - .cargo("doc --target x86_64-unknown-linux-gnu") - .run(); + dummy_project.cargo("doc --target").arg(rustc_host()).run(); assert!(!dummy_project .build_dir() - .join("x86_64-unknown-linux-gnu/doc/bogus_file") + .join(rustc_host()) + .join("doc/bogus_file") .exists()); let fingerprint: RustDocFingerprint = From ca611aefb293908f5e4d8110619a1f2bac338629 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Thu, 15 Apr 2021 17:52:25 -0700 Subject: [PATCH 2/5] Don't delete doc directories if the rustdoc fingerprint is missing. This also rearranges the code a little bit to try to avoid some duplication and to try to make it a little more compact. --- .../compiler/build_context/target_info.rs | 80 ++++++++----------- 1 file changed, 34 insertions(+), 46 deletions(-) diff --git a/src/cargo/core/compiler/build_context/target_info.rs b/src/cargo/core/compiler/build_context/target_info.rs index 610749af9fe..4a34f2c25a2 100644 --- a/src/cargo/core/compiler/build_context/target_info.rs +++ b/src/cargo/core/compiler/build_context/target_info.rs @@ -11,7 +11,7 @@ use serde::{Deserialize, Serialize}; use std::cell::RefCell; use std::collections::hash_map::{Entry, HashMap}; use std::env; -use std::path::{Path, PathBuf}; +use std::path::PathBuf; use std::str::{self, FromStr}; /// Information about the platform target gleaned from querying rustc. @@ -771,27 +771,6 @@ pub struct RustDocFingerprint { } impl RustDocFingerprint { - /// Read the `RustDocFingerprint` info from the fingerprint file. - fn read(cx: &Context<'_, '_>) -> CargoResult { - let rustdoc_data = paths::read(&cx.files().host_root().join(".rustdoc_fingerprint.json"))?; - serde_json::from_str(&rustdoc_data).map_err(|e| anyhow::anyhow!("{:?}", e)) - } - - /// Write the `RustDocFingerprint` info into the fingerprint file. - fn write<'a, 'cfg>(&self, cx: &Context<'a, 'cfg>) -> CargoResult<()> { - paths::write( - &cx.files().host_root().join(".rustdoc_fingerprint.json"), - serde_json::to_string(&self)?.as_bytes(), - ) - } - - fn remove_doc_dirs(doc_dirs: &[&Path]) -> CargoResult<()> { - doc_dirs - .iter() - .filter(|path| path.exists()) - .try_for_each(|path| paths::remove_dir_all(&path)) - } - /// This function checks whether the latest version of `Rustc` used to compile this /// `Workspace`'s docs was the same as the one is currently being used in this `cargo doc` /// call. @@ -805,34 +784,43 @@ impl 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) { + let fingerprint_path = cx.files().host_root().join(".rustdoc_fingerprint.json"); + let write_fingerprint = || -> CargoResult<()> { + paths::write( + &fingerprint_path, + serde_json::to_string(&actual_rustdoc_target_data)?, + ) + }; + let rustdoc_data = match paths::read(&fingerprint_path) { + Ok(rustdoc_data) => rustdoc_data, + // If the fingerprint does not exist, do not clear out the doc + // directories. Otherwise this ran into problems where projects + // like rustbuild were creating the doc directory before running + // `cargo doc` in a way that deleting it would break it. + Err(_) => return write_fingerprint(), + }; + match serde_json::from_str::(&rustdoc_data) { 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 fingerprint.rustc_vv == actual_rustdoc_target_data.rustc_vv { + return Ok(()); } } - // 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)? + Err(e) => { + log::debug!("could not deserialize {:?}: {}", fingerprint_path, e); } - } + }; + // Fingerprint does not match, delete the doc directories and write a new fingerprint. + log::debug!( + "fingerprint {:?} mismatch, clearing doc directories", + fingerprint_path + ); + cx.bcx + .all_kinds + .iter() + .map(|kind| cx.files().layout(*kind).doc()) + .filter(|path| path.exists()) + .try_for_each(|path| paths::remove_dir_all(path))?; + write_fingerprint()?; Ok(()) } } From f130ef98c2359b380049f30faf7906ed4bb9301d Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Sat, 24 Apr 2021 08:07:57 -0700 Subject: [PATCH 3/5] rustdoc fingerprint: don't delete the top directory In some cases, the directory may actually be a symlink created by the user, and we don't want to delete it. Also, skip any hidden files added by the user as well. --- .../compiler/build_context/target_info.rs | 37 +++++++++++++++++-- 1 file changed, 34 insertions(+), 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 4a34f2c25a2..66475dbce92 100644 --- a/src/cargo/core/compiler/build_context/target_info.rs +++ b/src/cargo/core/compiler/build_context/target_info.rs @@ -11,7 +11,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. @@ -803,6 +803,12 @@ impl RustDocFingerprint { Ok(fingerprint) => { if fingerprint.rustc_vv == actual_rustdoc_target_data.rustc_vv { return Ok(()); + } else { + log::debug!( + "doc fingerprint changed:\noriginal:\n{}\nnew:\n{}", + fingerprint.rustc_vv, + actual_rustdoc_target_data.rustc_vv + ); } } Err(e) => { @@ -819,8 +825,33 @@ impl RustDocFingerprint { .iter() .map(|kind| cx.files().layout(*kind).doc()) .filter(|path| path.exists()) - .try_for_each(|path| paths::remove_dir_all(path))?; + .try_for_each(|path| clean_doc(path))?; write_fingerprint()?; - Ok(()) + return Ok(()); + + fn clean_doc(path: &Path) -> CargoResult<()> { + let entries = path + .read_dir() + .with_context(|| format!("failed to read directory `{}`", path.display()))?; + for entry in entries { + let entry = entry?; + // Don't remove hidden files. Rustdoc does not create them, + // but the user might have. + if entry + .file_name() + .to_str() + .map_or(false, |name| name.starts_with('.')) + { + continue; + } + let path = entry.path(); + if entry.file_type()?.is_dir() { + paths::remove_dir_all(path)?; + } else { + paths::remove_file(path)?; + } + } + Ok(()) + } } } From 6f75160a881551ef0f747e0e16f42060cbe01f02 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Sat, 24 Apr 2021 08:22:44 -0700 Subject: [PATCH 4/5] Add -Zskip-rustdoc-fingerprint This is a hidden flag intended to only be used by rustbuild which will skip the rustdoc fingerprint check. rustbuild does some funky things with sharing the doc directory across multiple target directories via symlinks, and that causes problems where after building in one target directory, then switching to the second one, it will clear the contents. --- src/cargo/core/compiler/build_context/target_info.rs | 3 +++ src/cargo/core/features.rs | 8 +++++--- 2 files changed, 8 insertions(+), 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 66475dbce92..c2e1cce2f00 100644 --- a/src/cargo/core/compiler/build_context/target_info.rs +++ b/src/cargo/core/compiler/build_context/target_info.rs @@ -780,6 +780,9 @@ impl RustDocFingerprint { /// versions of the `js/html/css` files that `rustdoc` autogenerates which do not have /// any versioning. pub fn check_rustdoc_fingerprint(cx: &Context<'_, '_>) -> CargoResult<()> { + if cx.bcx.config.cli_unstable().skip_rustdoc_fingerprint { + return Ok(()); + } let actual_rustdoc_target_data = RustDocFingerprint { rustc_vv: cx.bcx.rustc().verbose_version.clone(), }; diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs index 62520b8c953..83850d48a47 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -542,8 +542,8 @@ macro_rules! unstable_cli_options { ( $( $(#[$meta:meta])? - $element: ident: $ty: ty = ($help: expr ) - ),* + $element: ident: $ty: ty = ($help: expr ), + )* ) => { /// A parsed representation of all unstable flags that Cargo accepts. /// @@ -603,7 +603,8 @@ unstable_cli_options!( terminal_width: Option> = ("Provide a terminal width to rustc for error truncation"), timings: Option> = ("Display concurrency information"), unstable_options: bool = ("Allow the usage of unstable options"), - weak_dep_features: bool = ("Allow `dep_name?/feature` feature syntax") + weak_dep_features: bool = ("Allow `dep_name?/feature` feature syntax"), + skip_rustdoc_fingerprint: bool = (HIDDEN), ); const STABILIZED_COMPILE_PROGRESS: &str = "The progress bar is now always \ @@ -813,6 +814,7 @@ impl CliUnstable { "weak-dep-features" => self.weak_dep_features = parse_empty(k, v)?, "extra-link-arg" => self.extra_link_arg = parse_empty(k, v)?, "credential-process" => self.credential_process = parse_empty(k, v)?, + "skip-rustdoc-fingerprint" => self.skip_rustdoc_fingerprint = parse_empty(k, v)?, "compile-progress" => stabilized_warn(k, "1.30", STABILIZED_COMPILE_PROGRESS), "offline" => stabilized_err(k, "1.36", STABILIZED_OFFLINE)?, "cache-messages" => stabilized_warn(k, "1.40", STABILIZED_CACHE_MESSAGES), From c37386730447f1d57950b0200e1cd20fb66afa2f Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Sat, 24 Apr 2021 09:08:21 -0700 Subject: [PATCH 5/5] Add tests for new rustdoc fingerprint behavior. --- tests/testsuite/doc.rs | 60 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 59 insertions(+), 1 deletion(-) diff --git a/tests/testsuite/doc.rs b/tests/testsuite/doc.rs index a5fadaf832e..e03b5a18513 100644 --- a/tests/testsuite/doc.rs +++ b/tests/testsuite/doc.rs @@ -4,7 +4,7 @@ use cargo::core::compiler::RustDocFingerprint; use cargo_test_support::paths::CargoPathExt; use cargo_test_support::registry::Package; use cargo_test_support::{basic_lib_manifest, basic_manifest, git, project}; -use cargo_test_support::{is_nightly, rustc_host}; +use cargo_test_support::{is_nightly, rustc_host, symlink_supported}; use std::fs; use std::str; @@ -1889,3 +1889,61 @@ LLVM version: 9.0 (String::from_utf8_lossy(&output.stdout).as_ref()) ); } + +#[cargo_test] +fn doc_fingerprint_unusual_behavior() { + // Checks for some unusual circumstances with clearing the doc directory. + if !symlink_supported() { + return; + } + let p = project().file("src/lib.rs", "").build(); + p.build_dir().mkdir_p(); + let real_doc = p.root().join("doc"); + real_doc.mkdir_p(); + let build_doc = p.build_dir().join("doc"); + p.symlink(&real_doc, &build_doc); + fs::write(real_doc.join("somefile"), "test").unwrap(); + fs::write(real_doc.join(".hidden"), "test").unwrap(); + p.cargo("doc").run(); + // Make sure for the first run, it does not delete any files and does not + // break the symlink. + assert!(build_doc.join("somefile").exists()); + assert!(real_doc.join("somefile").exists()); + assert!(real_doc.join(".hidden").exists()); + assert!(real_doc.join("foo/index.html").exists()); + // Pretend that the last build was generated by an older version. + p.change_file( + "target/.rustdoc_fingerprint.json", + "{\"rustc_vv\": \"I am old\"}", + ); + // Change file to trigger a new build. + p.change_file("src/lib.rs", "// changed"); + p.cargo("doc") + .with_stderr( + "[DOCUMENTING] foo [..]\n\ + [FINISHED] [..]", + ) + .run(); + // This will delete somefile, but not .hidden. + assert!(!real_doc.join("somefile").exists()); + assert!(real_doc.join(".hidden").exists()); + assert!(real_doc.join("foo/index.html").exists()); + // And also check the -Z flag behavior. + p.change_file( + "target/.rustdoc_fingerprint.json", + "{\"rustc_vv\": \"I am old\"}", + ); + // Change file to trigger a new build. + p.change_file("src/lib.rs", "// changed2"); + fs::write(real_doc.join("somefile"), "test").unwrap(); + p.cargo("doc -Z skip-rustdoc-fingerprint") + .masquerade_as_nightly_cargo() + .with_stderr( + "[DOCUMENTING] foo [..]\n\ + [FINISHED] [..]", + ) + .run(); + // Should not have deleted anything. + assert!(build_doc.join("somefile").exists()); + assert!(real_doc.join("somefile").exists()); +}