Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't re-use rustc cache when RUSTC_WRAPPER changes #9348

Merged
merged 1 commit into from
Apr 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/cargo/core/compiler/build_context/target_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ impl TargetInfo {
"RUSTFLAGS",
)?;
let extra_fingerprint = kind.fingerprint_hash();
let mut process = rustc.process();
let mut process = rustc.workspace_process();
process
.arg("-")
.arg("--crate-name")
Expand Down
47 changes: 39 additions & 8 deletions src/cargo/util/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,13 @@ impl Rustc {
) -> CargoResult<Rustc> {
let _p = profile::start("Rustc::new");

let mut cache = Cache::load(&path, rustup_rustc, cache_location);
let mut cache = Cache::load(
wrapper.as_deref(),
workspace_wrapper.as_deref(),
&path,
rustup_rustc,
cache_location,
);

let mut cmd = ProcessBuilder::new(&path);
cmd.arg("-vV");
Expand Down Expand Up @@ -154,8 +160,17 @@ struct Output {
}

impl Cache {
fn load(rustc: &Path, rustup_rustc: &Path, cache_location: Option<PathBuf>) -> Cache {
match (cache_location, rustc_fingerprint(rustc, rustup_rustc)) {
fn load(
wrapper: Option<&Path>,
workspace_wrapper: Option<&Path>,
rustc: &Path,
rustup_rustc: &Path,
cache_location: Option<PathBuf>,
) -> Cache {
match (
cache_location,
rustc_fingerprint(wrapper, workspace_wrapper, rustc, rustup_rustc),
) {
(Some(cache_location), Ok(rustc_fingerprint)) => {
let empty = CacheData {
rustc_fingerprint,
Expand Down Expand Up @@ -272,13 +287,29 @@ impl Drop for Cache {
}
}

fn rustc_fingerprint(path: &Path, rustup_rustc: &Path) -> CargoResult<u64> {
fn rustc_fingerprint(
wrapper: Option<&Path>,
workspace_wrapper: Option<&Path>,
rustc: &Path,
rustup_rustc: &Path,
) -> CargoResult<u64> {
let mut hasher = StableHasher::new();

let path = paths::resolve_executable(path)?;
path.hash(&mut hasher);
let hash_exe = |hasher: &mut _, path| -> CargoResult<()> {
let path = paths::resolve_executable(path)?;
path.hash(hasher);

paths::mtime(&path)?.hash(hasher);
Ok(())
};

paths::mtime(&path)?.hash(&mut hasher);
hash_exe(&mut hasher, rustc)?;
if let Some(wrapper) = wrapper {
hash_exe(&mut hasher, wrapper)?;
}
if let Some(workspace_wrapper) = workspace_wrapper {
hash_exe(&mut hasher, workspace_wrapper)?;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could there be an else case?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It maybe could, but that would be worse, imo. The current approach of always hashing both is robust to future changes in the surrounding code.


// Rustup can change the effective compiler without touching
// the `rustc` binary, so we try to account for this here.
Expand All @@ -291,7 +322,7 @@ fn rustc_fingerprint(path: &Path, rustup_rustc: &Path) -> CargoResult<u64> {
//
// If we don't see rustup env vars, but it looks like the compiler
// is managed by rustup, we conservatively bail out.
let maybe_rustup = rustup_rustc == path;
let maybe_rustup = rustup_rustc == rustc;
match (
maybe_rustup,
env::var("RUSTUP_HOME"),
Expand Down
130 changes: 106 additions & 24 deletions tests/testsuite/rustc_info_cache.rs
Original file line number Diff line number Diff line change
@@ -1,40 +1,40 @@
//! Tests for the cache file for the rustc version info.

use cargo_test_support::paths::CargoPathExt;
use cargo_test_support::{basic_bin_manifest, paths::CargoPathExt};
use cargo_test_support::{basic_manifest, project};
use std::env;

const MISS: &str = "[..] rustc info cache miss[..]";
const HIT: &str = "[..]rustc info cache hit[..]";
const UPDATE: &str = "[..]updated rustc info cache[..]";

#[cargo_test]
fn rustc_info_cache() {
let p = project()
.file("src/main.rs", r#"fn main() { println!("hello"); }"#)
.build();

let miss = "[..] rustc info cache miss[..]";
let hit = "[..]rustc info cache hit[..]";
let update = "[..]updated rustc info cache[..]";

p.cargo("build")
.env("CARGO_LOG", "cargo::util::rustc=debug")
.with_stderr_contains("[..]failed to read rustc info cache[..]")
.with_stderr_contains(miss)
.with_stderr_does_not_contain(hit)
.with_stderr_contains(update)
.with_stderr_contains(MISS)
.with_stderr_does_not_contain(HIT)
.with_stderr_contains(UPDATE)
.run();

p.cargo("build")
.env("CARGO_LOG", "cargo::util::rustc=debug")
.with_stderr_contains("[..]reusing existing rustc info cache[..]")
.with_stderr_contains(hit)
.with_stderr_does_not_contain(miss)
.with_stderr_does_not_contain(update)
.with_stderr_contains(HIT)
.with_stderr_does_not_contain(MISS)
.with_stderr_does_not_contain(UPDATE)
.run();

p.cargo("build")
.env("CARGO_LOG", "cargo::util::rustc=debug")
.env("CARGO_CACHE_RUSTC_INFO", "0")
.with_stderr_contains("[..]rustc info cache disabled[..]")
.with_stderr_does_not_contain(update)
.with_stderr_does_not_contain(UPDATE)
.run();

let other_rustc = {
Expand Down Expand Up @@ -68,18 +68,18 @@ fn rustc_info_cache() {
.env("CARGO_LOG", "cargo::util::rustc=debug")
.env("RUSTC", other_rustc.display().to_string())
.with_stderr_contains("[..]different compiler, creating new rustc info cache[..]")
.with_stderr_contains(miss)
.with_stderr_does_not_contain(hit)
.with_stderr_contains(update)
.with_stderr_contains(MISS)
.with_stderr_does_not_contain(HIT)
.with_stderr_contains(UPDATE)
.run();

p.cargo("build")
.env("CARGO_LOG", "cargo::util::rustc=debug")
.env("RUSTC", other_rustc.display().to_string())
.with_stderr_contains("[..]reusing existing rustc info cache[..]")
.with_stderr_contains(hit)
.with_stderr_does_not_contain(miss)
.with_stderr_does_not_contain(update)
.with_stderr_contains(HIT)
.with_stderr_does_not_contain(MISS)
.with_stderr_does_not_contain(UPDATE)
.run();

other_rustc.move_into_the_future();
Expand All @@ -88,17 +88,99 @@ fn rustc_info_cache() {
.env("CARGO_LOG", "cargo::util::rustc=debug")
.env("RUSTC", other_rustc.display().to_string())
.with_stderr_contains("[..]different compiler, creating new rustc info cache[..]")
.with_stderr_contains(miss)
.with_stderr_does_not_contain(hit)
.with_stderr_contains(update)
.with_stderr_contains(MISS)
.with_stderr_does_not_contain(HIT)
.with_stderr_contains(UPDATE)
.run();

p.cargo("build")
.env("CARGO_LOG", "cargo::util::rustc=debug")
.env("RUSTC", other_rustc.display().to_string())
.with_stderr_contains("[..]reusing existing rustc info cache[..]")
.with_stderr_contains(hit)
.with_stderr_does_not_contain(miss)
.with_stderr_does_not_contain(update)
.with_stderr_contains(HIT)
.with_stderr_does_not_contain(MISS)
.with_stderr_does_not_contain(UPDATE)
.run();
}

#[cargo_test]
fn rustc_info_cache_with_wrappers() {
let wrapper_project = project()
.at("wrapper")
.file("Cargo.toml", &basic_bin_manifest("wrapper"))
.file("src/main.rs", r#"fn main() { }"#)
.build();
let wrapper = wrapper_project.bin("wrapper");

let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "test"
version = "0.0.0"
authors = []
[workspace]
"#,
)
.file("src/main.rs", r#"fn main() { println!("hello"); }"#)
.build();

for &wrapper_env in ["RUSTC_WRAPPER", "RUSTC_WORKSPACE_WRAPPER"].iter() {
p.cargo("clean").with_status(0).run();
wrapper_project.change_file(
"src/main.rs",
r#"
fn main() {
let mut args = std::env::args_os();
let _me = args.next().unwrap();
let rustc = args.next().unwrap();
let status = std::process::Command::new(rustc).args(args).status().unwrap();
std::process::exit(if status.success() { 0 } else { 1 })
}
"#,
);
wrapper_project.cargo("build").with_status(0).run();

p.cargo("build")
.env("CARGO_LOG", "cargo::util::rustc=debug")
.env(wrapper_env, &wrapper)
.with_stderr_contains("[..]failed to read rustc info cache[..]")
.with_stderr_contains(MISS)
.with_stderr_contains(UPDATE)
.with_stderr_does_not_contain(HIT)
.with_status(0)
.run();
p.cargo("build")
.env("CARGO_LOG", "cargo::util::rustc=debug")
.env(wrapper_env, &wrapper)
.with_stderr_contains("[..]reusing existing rustc info cache[..]")
.with_stderr_contains(HIT)
.with_stderr_does_not_contain(UPDATE)
.with_stderr_does_not_contain(MISS)
.with_status(0)
.run();

wrapper_project.change_file("src/main.rs", r#"fn main() { panic!() }"#);
wrapper_project.cargo("build").with_status(0).run();

p.cargo("build")
.env("CARGO_LOG", "cargo::util::rustc=debug")
.env(wrapper_env, &wrapper)
.with_stderr_contains("[..]different compiler, creating new rustc info cache[..]")
.with_stderr_contains(MISS)
.with_stderr_contains(UPDATE)
.with_stderr_does_not_contain(HIT)
.with_status(101)
.run();
p.cargo("build")
.env("CARGO_LOG", "cargo::util::rustc=debug")
.env(wrapper_env, &wrapper)
.with_stderr_contains("[..]reusing existing rustc info cache[..]")
.with_stderr_contains(HIT)
.with_stderr_does_not_contain(UPDATE)
.with_stderr_does_not_contain(MISS)
.with_status(101)
.run();
}
}