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.
This commit is contained in:
CPerezz 2021-01-27 01:14:31 +01:00
parent 8ddc56f14d
commit d2572a2800
No known key found for this signature in database
GPG Key ID: 6EE573EDC452F806
4 changed files with 58 additions and 124 deletions

View File

@ -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<Self> {
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(())
}
}

View File

@ -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

View File

@ -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;

View File

@ -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");
dummy_project.change_file(
"target/.rustdoc_fingerprint.json",
&old_rustc_verbose_version,
);
fs::write(
dummy_project
.root()
.join("target")
.join("doc")
.join(".rustdoc_fingerprint.json"),
stable_rustc_verbose_version,
)
.expect("Error writing old rustc 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());
assert!(!dummy_project.build_dir().join("doc/bogus_file").exists());
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())
);
}