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

Include the linker in the fingerprint. #9647

Merged
merged 1 commit into from
Jul 2, 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
5 changes: 5 additions & 0 deletions crates/cargo-test-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1035,6 +1035,11 @@ pub fn rustc_host() -> &'static str {
&RUSTC_INFO.host
}

/// The host triple suitable for use in a cargo environment variable (uppercased).
pub fn rustc_host_env() -> String {
rustc_host().to_uppercase().replace('-', "_")
}

pub fn is_nightly() -> bool {
let vv = &RUSTC_INFO.verbose_version;
env::var("CARGO_TEST_DISABLE_NIGHTLY").is_err()
Expand Down
42 changes: 25 additions & 17 deletions src/cargo/core/compiler/fingerprint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@
use std::collections::hash_map::{Entry, HashMap};
use std::convert::TryInto;
use std::env;
use std::hash::{self, Hasher};
use std::hash::{self, Hash, Hasher};
use std::path::{Path, PathBuf};
use std::str;
use std::sync::{Arc, Mutex};
Expand All @@ -334,7 +334,7 @@ use crate::core::Package;
use crate::util;
use crate::util::errors::CargoResult;
use crate::util::interning::InternedString;
use crate::util::{internal, path_args, profile};
use crate::util::{internal, path_args, profile, StableHasher};
use crate::CARGO_ENV;

use super::custom_build::BuildDeps;
Expand Down Expand Up @@ -502,7 +502,7 @@ struct DepFingerprint {
/// as a fingerprint (all source files must be modified *before* this mtime).
/// This dep-info file is not generated, however, until after the crate is
/// compiled. As a result, this structure can be thought of as a fingerprint
/// to-be. The actual value can be calculated via `hash()`, but the operation
/// to-be. The actual value can be calculated via `hash_u64()`, but the operation
/// may fail as some files may not have been generated.
///
/// Note that dependencies are taken into account for fingerprints because rustc
Expand Down Expand Up @@ -594,7 +594,7 @@ impl Serialize for DepFingerprint {
&self.pkg_id,
&self.name,
&self.public,
&self.fingerprint.hash(),
&self.fingerprint.hash_u64(),
)
.serialize(ser)
}
Expand Down Expand Up @@ -812,7 +812,7 @@ impl Fingerprint {
*self.memoized_hash.lock().unwrap() = None;
}

fn hash(&self) -> u64 {
fn hash_u64(&self) -> u64 {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I renamed this because there were two functions with the same name but different signatures (the inherent fn and the trait). This caused a problem when importing the trait, and was also confusing.

if let Some(s) = *self.memoized_hash.lock().unwrap() {
return s;
}
Expand Down Expand Up @@ -956,13 +956,13 @@ impl Fingerprint {
return Err(e);
}

if a.fingerprint.hash() != b.fingerprint.hash() {
if a.fingerprint.hash_u64() != b.fingerprint.hash_u64() {
let e = format_err!(
"new ({}/{:x}) != old ({}/{:x})",
a.name,
a.fingerprint.hash(),
a.fingerprint.hash_u64(),
b.name,
b.fingerprint.hash()
b.fingerprint.hash_u64()
)
.context("unit dependency information changed");
return Err(e);
Expand Down Expand Up @@ -1145,7 +1145,7 @@ impl hash::Hash for Fingerprint {
name.hash(h);
public.hash(h);
// use memoized dep hashes to avoid exponential blowup
h.write_u64(Fingerprint::hash(fingerprint));
h.write_u64(fingerprint.hash_u64());
}
}
}
Expand Down Expand Up @@ -1318,12 +1318,17 @@ fn calculate_normal(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Finger
// Include metadata since it is exposed as environment variables.
let m = unit.pkg.manifest().metadata();
let metadata = util::hash_u64((&m.authors, &m.description, &m.homepage, &m.repository));
let mut config = 0u64;
let mut config = StableHasher::new();
if let Some(linker) = cx.bcx.linker(unit.kind) {
linker.hash(&mut config);
}
if unit.mode.is_doc() && cx.bcx.config.cli_unstable().rustdoc_map {
config = config.wrapping_add(cx.bcx.config.doc_extern_map().map_or(0, util::hash_u64));
if let Ok(map) = cx.bcx.config.doc_extern_map() {
map.hash(&mut config);
}
}
if let Some(allow_features) = &cx.bcx.config.cli_unstable().allow_features {
config = config.wrapping_add(util::hash_u64(allow_features));
allow_features.hash(&mut config);
}
let compile_kind = unit.kind.fingerprint_hash();
Ok(Fingerprint {
Expand All @@ -1338,7 +1343,7 @@ fn calculate_normal(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Finger
local: Mutex::new(local),
memoized_hash: Mutex::new(None),
metadata,
config,
config: config.finish(),
compile_kind,
rustflags: extra_flags,
fs_status: FsStatus::Stale,
Expand Down Expand Up @@ -1570,14 +1575,14 @@ fn write_fingerprint(loc: &Path, fingerprint: &Fingerprint) -> CargoResult<()> {
// fingerprint::new().rustc == 0, make sure it doesn't make it to the file system.
// This is mostly so outside tools can reliably find out what rust version this file is for,
// as we can use the full hash.
let hash = fingerprint.hash();
let hash = fingerprint.hash_u64();
debug!("write fingerprint ({:x}) : {}", hash, loc.display());
paths::write(loc, util::to_hex(hash).as_bytes())?;

let json = serde_json::to_string(fingerprint).unwrap();
if cfg!(debug_assertions) {
let f: Fingerprint = serde_json::from_str(&json).unwrap();
assert_eq!(f.hash(), hash);
assert_eq!(f.hash_u64(), hash);
}
paths::write(&loc.with_extension("json"), json.as_bytes())?;
Ok(())
Expand Down Expand Up @@ -1621,7 +1626,7 @@ fn compare_old_fingerprint(
paths::set_file_time_no_err(loc, t);
}

let new_hash = new_fingerprint.hash();
let new_hash = new_fingerprint.hash_u64();

if util::to_hex(new_hash) == old_fingerprint_short && new_fingerprint.fs_status.up_to_date() {
return Ok(());
Expand All @@ -1632,7 +1637,10 @@ fn compare_old_fingerprint(
.with_context(|| internal("failed to deserialize json"))?;
// Fingerprint can be empty after a failed rebuild (see comment in prepare_target).
if !old_fingerprint_short.is_empty() {
debug_assert_eq!(util::to_hex(old_fingerprint.hash()), old_fingerprint_short);
debug_assert_eq!(
util::to_hex(old_fingerprint.hash_u64()),
old_fingerprint_short
);
}
let result = new_fingerprint.compare(&old_fingerprint);
assert!(result.is_err());
Expand Down
28 changes: 23 additions & 5 deletions tests/testsuite/freshness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ use std::time::SystemTime;
use super::death;
use cargo_test_support::paths::{self, CargoPathExt};
use cargo_test_support::registry::Package;
use cargo_test_support::{basic_manifest, is_coarse_mtime, project, rustc_host, sleep_ms};
use cargo_test_support::{
basic_manifest, is_coarse_mtime, project, rustc_host, rustc_host_env, sleep_ms,
};

#[cargo_test]
fn modifying_and_moving() {
Expand Down Expand Up @@ -2405,10 +2407,7 @@ fn linking_interrupted() {

// Make a change, start a build, then interrupt it.
p.change_file("src/lib.rs", "// modified");
let linker_env = format!(
"CARGO_TARGET_{}_LINKER",
rustc_host().to_uppercase().replace('-', "_")
);
let linker_env = format!("CARGO_TARGET_{}_LINKER", rustc_host_env());
// NOTE: This assumes that the paths to the linker or rustc are not in the
// fingerprint. But maybe they should be?
let mut cmd = p
Expand Down Expand Up @@ -2641,3 +2640,22 @@ fn cargo_env_changes() {
)
.run();
}

#[cargo_test]
fn changing_linker() {
// Changing linker should rebuild.
let p = project().file("src/main.rs", "fn main() {}").build();
p.cargo("build").run();
let linker_env = format!("CARGO_TARGET_{}_LINKER", rustc_host_env());
p.cargo("build --verbose")
.env(&linker_env, "nonexistent-linker")
.with_status(101)
.with_stderr_contains(
"\
[COMPILING] foo v0.0.1 ([..])
[RUNNING] `rustc [..] -C linker=nonexistent-linker [..]`
[ERROR] [..]linker[..]
",
)
.run();
}
27 changes: 7 additions & 20 deletions tests/testsuite/tool_paths.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
//! Tests for configuration values that point to programs.

use cargo_test_support::{basic_lib_manifest, no_such_file_err_msg, project, rustc_host};
use cargo_test_support::{
basic_lib_manifest, no_such_file_err_msg, project, rustc_host, rustc_host_env,
};

#[cargo_test]
fn pathless_tools() {
Expand Down Expand Up @@ -262,13 +264,9 @@ second match `cfg(not(target_os = \"none\"))` located in [..]/foo/.cargo/config

#[cargo_test]
fn custom_runner_env() {
let target = rustc_host();
let p = project().file("src/main.rs", "fn main() {}").build();

let key = format!(
"CARGO_TARGET_{}_RUNNER",
target.to_uppercase().replace('-', "_")
);
let key = format!("CARGO_TARGET_{}_RUNNER", rustc_host_env());

p.cargo("run")
.env(&key, "nonexistent-runner --foo")
Expand Down Expand Up @@ -305,10 +303,7 @@ fn custom_runner_env_overrides_config() {
)
.build();

let key = format!(
"CARGO_TARGET_{}_RUNNER",
target.to_uppercase().replace('-', "_")
);
let key = format!("CARGO_TARGET_{}_RUNNER", rustc_host_env());

p.cargo("run")
.env(&key, "should-run --foo")
Expand All @@ -322,13 +317,9 @@ fn custom_runner_env_overrides_config() {
fn custom_runner_env_true() {
// Check for a bug where "true" was interpreted as a boolean instead of
// the executable.
let target = rustc_host();
let p = project().file("src/main.rs", "fn main() {}").build();

let key = format!(
"CARGO_TARGET_{}_RUNNER",
target.to_uppercase().replace('-', "_")
);
let key = format!("CARGO_TARGET_{}_RUNNER", rustc_host_env());

p.cargo("run")
.env(&key, "true")
Expand All @@ -338,13 +329,9 @@ fn custom_runner_env_true() {

#[cargo_test]
fn custom_linker_env() {
let target = rustc_host();
let p = project().file("src/main.rs", "fn main() {}").build();

let key = format!(
"CARGO_TARGET_{}_LINKER",
target.to_uppercase().replace('-', "_")
);
let key = format!("CARGO_TARGET_{}_LINKER", rustc_host_env());

p.cargo("build -v")
.env(&key, "nonexistent-linker")
Expand Down