Skip to content

Commit

Permalink
refactor(fingerprint): Track the intent for each use of UnitHash
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
epage committed Nov 15, 2024
1 parent 528ef7e commit 6bacd81
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 30 deletions.
28 changes: 14 additions & 14 deletions src/cargo/core/compiler/build_runner/compilation_files.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<UnitHash> {
self.use_extra_filename.then_some(self.meta_hash)
}
}

Expand Down Expand Up @@ -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))
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/compiler/build_runner/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/compiler/job_queue/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
26 changes: 12 additions & 14 deletions src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -844,7 +841,7 @@ fn rustdoc(build_runner: &mut BuildRunner<'_, '_>, unit: &Unit) -> CargoResult<W
.iter()
.map(|unit| {
Ok((
build_runner.files().metadata(unit).meta_hash(),
build_runner.files().metadata(unit).unit_id(),
scrape_output_path(build_runner, unit)?,
))
})
Expand Down Expand Up @@ -1158,10 +1155,11 @@ fn build_base_args(
cmd.args(&check_cfg_args(unit));

let meta = build_runner.files().metadata(unit);
cmd.arg("-C").arg(&format!("metadata={}", meta.meta_hash()));
if meta.use_extra_filename() {
cmd.arg("-C")
.arg(&format!("metadata={}", meta.c_metadata()));
if let Some(c_extra_filename) = meta.c_extra_filename() {
cmd.arg("-C")
.arg(&format!("extra-filename=-{}", meta.meta_hash()));
.arg(&format!("extra-filename=-{c_extra_filename}"));
}

if rpath {
Expand Down

0 comments on commit 6bacd81

Please sign in to comment.