Skip to content

Commit

Permalink
Auto merge of #9647 - ehuss:fingerprint-linker, r=alexcrichton
Browse files Browse the repository at this point in the history 
Include the linker in the fingerprint.

This adds the linker from the `[target]` config table to the fingerprint. Previously, changing the value would not trigger a rebuild.
  • Loading branch information
@bors
bors committed Jul 2, 2021
2 parents 6e31c13 + 28c3bef commit 0a38a21
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 42 deletions.
5 changes: 5 additions & 0 deletions crates/cargo-test-support/src/lib.rs
View file
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
View file
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 {
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
View file
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
View file
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

0 comments on commit 0a38a21

Please sign in to comment.