From 1f4eeb4fcabeb16a421a1faaabaedb04b7dfdf41 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 15 Nov 2024 12:24:24 -0600 Subject: [PATCH] refactor(fingerprint): Track the intent for each use of UnitHash We have - `-C metadata` - `-C extra-filename` - Uniquifying build scripts - Uniquifying scraped documentation The last two I'm tracking as a `unit_id`. As we evolve the first two hashes, we can decide which would be a better fit for backing this. For scraping, we could treat this as `-C extra-filename` but then we'd have a lot of `.expect()`s. --- .../build_runner/compilation_files.rs | 28 +++++++++---------- src/cargo/core/compiler/build_runner/mod.rs | 2 +- src/cargo/core/compiler/job_queue/mod.rs | 2 +- src/cargo/core/compiler/mod.rs | 26 ++++++++--------- 4 files changed, 28 insertions(+), 30 deletions(-) diff --git a/src/cargo/core/compiler/build_runner/compilation_files.rs b/src/cargo/core/compiler/build_runner/compilation_files.rs index d0a404fb189..d8199c72614 100644 --- a/src/cargo/core/compiler/build_runner/compilation_files.rs +++ b/src/cargo/core/compiler/build_runner/compilation_files.rs @@ -91,17 +91,19 @@ pub struct Metadata { } impl Metadata { - /// The symbol hash to use. - pub fn meta_hash(&self) -> UnitHash { + /// A hash to identify a given [`Unit`] in the build graph + pub fn unit_id(&self) -> UnitHash { self.meta_hash } - /// Whether or not the `-C extra-filename` flag is used to generate unique - /// output filenames for this `Unit`. - /// - /// If this is `true`, the `meta_hash` is used for the filename. - pub fn use_extra_filename(&self) -> bool { - self.use_extra_filename + /// A hash to add to symbol naming through `-C metadata` + pub fn c_metadata(&self) -> UnitHash { + self.meta_hash + } + + /// A hash to add to file names through `-C extra-filename` + pub fn c_extra_filename(&self) -> Option { + self.use_extra_filename.then_some(self.meta_hash) } } @@ -230,8 +232,8 @@ impl<'a, 'gctx: 'a> CompilationFiles<'a, 'gctx> { fn pkg_dir(&self, unit: &Unit) -> String { let name = unit.pkg.package_id().name(); let meta = self.metas[unit]; - if meta.use_extra_filename() { - format!("{}-{}", name, meta.meta_hash()) + if let Some(c_extra_filename) = meta.c_extra_filename() { + format!("{}-{}", name, c_extra_filename) } else { format!("{}-{}", name, self.target_short_hash(unit)) } @@ -475,7 +477,7 @@ impl<'a, 'gctx: 'a> CompilationFiles<'a, 'gctx> { let file_name = format!( "{}-{}.examples", unit.pkg.name(), - self.metadata(unit).meta_hash() + self.metadata(unit).unit_id() ); let path = self.deps_dir(unit).join(file_name); vec![OutputFile { @@ -533,9 +535,7 @@ impl<'a, 'gctx: 'a> CompilationFiles<'a, 'gctx> { let mut outputs = Vec::new(); for file_type in file_types { let meta = self.metas[unit]; - let meta_opt = meta - .use_extra_filename() - .then(|| meta.meta_hash().to_string()); + let meta_opt = meta.c_extra_filename().map(|h| h.to_string()); let path = out_dir.join(file_type.output_filename(&unit.target, meta_opt.as_deref())); // If, the `different_binary_name` feature is enabled, the name of the hardlink will diff --git a/src/cargo/core/compiler/build_runner/mod.rs b/src/cargo/core/compiler/build_runner/mod.rs index e7a79a55a7d..1a55d211068 100644 --- a/src/cargo/core/compiler/build_runner/mod.rs +++ b/src/cargo/core/compiler/build_runner/mod.rs @@ -443,7 +443,7 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> { /// Returns the metadata hash for a `RunCustomBuild` unit. pub fn get_run_build_script_metadata(&self, unit: &Unit) -> UnitHash { assert!(unit.mode.is_run_custom_build()); - self.files().metadata(unit).meta_hash() + self.files().metadata(unit).unit_id() } pub fn is_primary_package(&self, unit: &Unit) -> bool { diff --git a/src/cargo/core/compiler/job_queue/mod.rs b/src/cargo/core/compiler/job_queue/mod.rs index 6f4c66fcd98..7ab12a3a265 100644 --- a/src/cargo/core/compiler/job_queue/mod.rs +++ b/src/cargo/core/compiler/job_queue/mod.rs @@ -685,7 +685,7 @@ impl<'gctx> DrainState<'gctx> { .failed_scrape_units .lock() .unwrap() - .insert(build_runner.files().metadata(&unit).meta_hash()); + .insert(build_runner.files().metadata(&unit).unit_id()); self.queue.finish(&unit, &artifact); } Err(error) => { diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 599056dad21..406280c6dbd 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -279,15 +279,12 @@ fn rustc( // don't pass the `-l` flags. let pass_l_flag = unit.target.is_lib() || !unit.pkg.targets().iter().any(|t| t.is_lib()); - let dep_info_name = if build_runner.files().metadata(unit).use_extra_filename() { - format!( - "{}-{}.d", - unit.target.crate_name(), - build_runner.files().metadata(unit).meta_hash() - ) - } else { - format!("{}.d", unit.target.crate_name()) - }; + let dep_info_name = + if let Some(c_extra_filename) = build_runner.files().metadata(unit).c_extra_filename() { + format!("{}-{}.d", unit.target.crate_name(), c_extra_filename) + } else { + format!("{}.d", unit.target.crate_name()) + }; let rustc_dep_info_loc = root.join(dep_info_name); let dep_info_loc = fingerprint::dep_info_loc(build_runner, unit); @@ -766,7 +763,7 @@ fn prepare_rustdoc(build_runner: &BuildRunner<'_, '_>, unit: &Unit) -> CargoResu let metadata = build_runner.metadata_for_doc_units[unit]; rustdoc .arg("-C") - .arg(format!("metadata={}", metadata.meta_hash())); + .arg(format!("metadata={}", metadata.c_metadata())); if unit.mode.is_doc_scrape() { debug_assert!(build_runner.bcx.scrape_units.contains(unit)); @@ -844,7 +841,7 @@ fn rustdoc(build_runner: &mut BuildRunner<'_, '_>, unit: &Unit) -> CargoResult