Auto merge of #9647 - ehuss:fingerprint-linker, r=alexcrichton

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.

Author: bors

crates/cargo-test-support/src/lib.rs

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

src/cargo/core/compiler/fingerprint.rs

@@ -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};
@@ -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;
@@ -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
@@ -594,7 +594,7 @@ impl Serialize for DepFingerprint {
             &self.pkg_id,
             &self.name,
             &self.public,
-            &self.fingerprint.hash(),
+            &self.fingerprint.hash_u64(),
         )
             .serialize(ser)
     }
@@ -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;
         }
@@ -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);
@@ -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());
         }
     }
 }
@@ -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 {
@@ -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,
@@ -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(())
@@ -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(());
@@ -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());

tests/testsuite/freshness.rs

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

tests/testsuite/tool_paths.rs

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