Skip to content

Commit 29a267a

Browse files
committed
fix(fingerprint): Hackily keep --remap-path-prefix binaries reproducible
1 parent 306d515 commit 29a267a

File tree

3 files changed

+32
-10
lines changed

3 files changed

+32
-10
lines changed

src/cargo/core/compiler/build_runner/compilation_files.rs

+25-6
Original file line numberDiff line numberDiff line change
@@ -703,14 +703,19 @@ fn compute_metadata(
703703
// Avoid trashing the caches on RUSTFLAGS changing via `c_extra_filename`
704704
//
705705
// Limited to `c_extra_filename` to help with reproducible build / PGO issues.
706-
build_runner
707-
.bcx
708-
.extra_args_for(unit)
709-
.hash(&mut c_extra_filename_hasher);
706+
let default = Vec::new();
707+
let extra_args = build_runner.bcx.extra_args_for(unit).unwrap_or(&default);
708+
if !has_remap_path_prefix(&extra_args) {
709+
extra_args.hash(&mut c_extra_filename_hasher);
710+
}
710711
if unit.mode.is_doc() || unit.mode.is_doc_scrape() {
711-
unit.rustdocflags.hash(&mut c_extra_filename_hasher);
712+
if !has_remap_path_prefix(&unit.rustdocflags) {
713+
unit.rustdocflags.hash(&mut c_extra_filename_hasher);
714+
}
712715
} else {
713-
unit.rustflags.hash(&mut c_extra_filename_hasher);
716+
if !has_remap_path_prefix(&unit.rustflags) {
717+
unit.rustflags.hash(&mut c_extra_filename_hasher);
718+
}
714719
}
715720

716721
let c_metadata = UnitHash(c_metadata_hasher.finish());
@@ -726,6 +731,20 @@ fn compute_metadata(
726731
}
727732
}
728733

734+
/// HACK: Detect the *potential* presence of `--remap-path-prefix`
735+
///
736+
/// As CLI parsing is contextual and dependent on the CLI definition to understand the context, we
737+
/// can't say for sure whether `--remap-path-prefix` is present, so we guess if anything looks like
738+
/// it.
739+
/// If we could, we'd strip it out for hashing.
740+
/// Instead, we use this to avoid hashing rustflags if it might be present to avoid the risk of taking
741+
/// a flag that is trying to make things reproducible and making things less reproducible by the
742+
/// `-Cextra-filename` showing up in the rlib, even with `split-debuginfo`.
743+
fn has_remap_path_prefix(args: &[String]) -> bool {
744+
args.iter()
745+
.any(|s| s.starts_with("--remap-path-prefix=") || s == "--remap-path-prefix")
746+
}
747+
729748
/// Hash the version of rustc being used during the build process.
730749
fn hash_rustc_version(bcx: &BuildContext<'_, '_>, hasher: &mut StableHasher, unit: &Unit) {
731750
let vers = &bcx.rustc().version;

src/cargo/core/compiler/fingerprint/mod.rs

+5-2
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@
6868
//! -------------------------------------------|-------------|---------------------|------------------------|----------
6969
//! rustc | ✓ | ✓ | ✓ | ✓
7070
//! [`Profile`] | ✓ | ✓ | ✓ | ✓
71-
//! `cargo rustc` extra args | ✓ | ✓ | | ✓
71+
//! `cargo rustc` extra args | ✓ | ✓[^7] | | ✓[^7]
7272
//! [`CompileMode`] | ✓ | ✓ | ✓ | ✓
7373
//! Target Name | ✓ | ✓ | ✓ | ✓
7474
//! `TargetKind` (bin/lib/etc.) | ✓ | ✓ | ✓ | ✓
@@ -83,7 +83,7 @@
8383
//! Target flags (test/bench/for_host/edition) | ✓ | | |
8484
//! -C incremental=… flag | ✓ | | |
8585
//! mtime of sources | ✓[^3] | | |
86-
//! RUSTFLAGS/RUSTDOCFLAGS | ✓ | ✓ | | ✓
86+
//! RUSTFLAGS/RUSTDOCFLAGS | ✓ | ✓[^7] | | ✓[^7]
8787
//! [`Lto`] flags | ✓ | ✓ | ✓ | ✓
8888
//! config settings[^5] | ✓ | | |
8989
//! `is_std` | | ✓ | ✓ | ✓
@@ -102,6 +102,9 @@
102102
//!
103103
//! [^6]: Via [`Manifest::lint_rustflags`][crate::core::Manifest::lint_rustflags]
104104
//!
105+
//! [^7]: extra-flags and RUSTFLAGS are conditionally excluded when `--remap-path-prefix` is
106+
//! present to avoid breaking build reproducibility while we wait for trim-paths
107+
//!
105108
//! When deciding what should go in the Metadata vs the Fingerprint, consider
106109
//! that some files (like dylibs) do not have a hash in their filename. Thus,
107110
//! if a value changes, only the fingerprint will detect the change (consider,

tests/testsuite/rustflags.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1592,7 +1592,7 @@ fn rustflags_remap_path_prefix_ignored_for_c_extra_filename() {
15921592
.run();
15931593
let second_c_extra_filename = dbg!(get_c_extra_filename(build_output));
15941594

1595-
assert_ne!(first_c_extra_filename, second_c_extra_filename);
1595+
assert_data_eq!(first_c_extra_filename, second_c_extra_filename);
15961596
}
15971597

15981598
// `--remap-path-prefix` is meant to take two different binaries and make them the same but the
@@ -1613,7 +1613,7 @@ fn rustc_remap_path_prefix_ignored_for_c_extra_filename() {
16131613
.run();
16141614
let second_c_extra_filename = dbg!(get_c_extra_filename(build_output));
16151615

1616-
assert_ne!(first_c_extra_filename, second_c_extra_filename);
1616+
assert_data_eq!(first_c_extra_filename, second_c_extra_filename);
16171617
}
16181618

16191619
fn get_c_metadata(output: RawOutput) -> String {

0 commit comments

Comments
 (0)