Skip to content

Commit dd02aa3

Browse files
tylerwhallLexmark-tyhall
authored andcommitted
Stabilize SourceId hashes relative to the workspace
Currently a crate from a path source will have its metadata hash incorporate its absolute path on the system where it is built. This always impacts top-level crates, which means that compiling the same source with the same dependencies and compiler version will generate libraries with symbol names that vary depending on where the workspace resides on the machine. For paths inside the Cargo workspace, hash their SourceId relative to the root of the workspace. Paths outside of a workspace are still hashed as absolute. This stability is important for reproducible builds as part of a larger build system that employs caching of artifacts, such as OpenEmbedded. OpenEmbedded tightly controls all inputs to a build and its caching assumes that an equivalent artifact will always result from the same set of inputs. The workspace path is not considered to be an influential input, however. For example, if Cargo is used to compile libstd shared objects which downstream crates link to dynamically, it must be possible to rebuild libstd given the same inputs and produce a library that is at least link-compatible with the original. If the build system happens to cache the downstream crates but needs to rebuild libstd and the user happens to be building in a different workspace path, currently Cargo will generate a library incompatible with the original and the downstream executables will fail at runtime on the target.
1 parent 58da6b0 commit dd02aa3

File tree

4 files changed

+39
-6
lines changed

4 files changed

+39
-6
lines changed

src/cargo/core/package_id.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use std::cmp::Ordering;
22
use std::fmt::{self, Formatter};
33
use std::hash::Hash;
44
use std::hash;
5+
use std::path::Path;
56
use std::sync::Arc;
67

78
use semver;
@@ -131,6 +132,20 @@ impl PackageId {
131132
}),
132133
}
133134
}
135+
136+
pub fn stable_hash<'a>(&'a self, workspace: &'a Path) -> PackageIdStableHash<'a> {
137+
PackageIdStableHash(&self, workspace)
138+
}
139+
}
140+
141+
pub struct PackageIdStableHash<'a>(&'a PackageId, &'a Path);
142+
143+
impl<'a> Hash for PackageIdStableHash<'a> {
144+
fn hash<S: hash::Hasher>(&self, state: &mut S) {
145+
self.0.inner.name.hash(state);
146+
self.0.inner.version.hash(state);
147+
self.0.inner.source_id.stable_hash(self.1, state);
148+
}
134149
}
135150

136151
impl fmt::Display for PackageId {

src/cargo/core/source.rs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use std::cmp::{self, Ordering};
22
use std::collections::hash_map::{HashMap, Values, IterMut};
33
use std::fmt::{self, Formatter};
4-
use std::hash;
4+
use std::hash::{self, Hash};
55
use std::path::Path;
66
use std::sync::Arc;
77
use std::sync::atomic::{AtomicBool, ATOMIC_BOOL_INIT};
@@ -297,6 +297,17 @@ impl SourceId {
297297
}
298298
self.inner.url.to_string() == CRATES_IO
299299
}
300+
301+
pub fn stable_hash<S: hash::Hasher>(&self, workspace: &Path, into: &mut S) {
302+
if self.is_path() {
303+
if let Ok(p) = self.inner.url.to_file_path().unwrap().strip_prefix(workspace) {
304+
self.inner.kind.hash(into);
305+
p.to_str().unwrap().hash(into);
306+
return
307+
}
308+
}
309+
self.hash(into)
310+
}
300311
}
301312

302313
impl PartialEq for SourceId {
@@ -417,7 +428,7 @@ impl Ord for SourceIdInner {
417428
// The hash of SourceId is used in the name of some Cargo folders, so shouldn't
418429
// vary. `as_str` gives the serialisation of a url (which has a spec) and so
419430
// insulates against possible changes in how the url crate does hashing.
420-
impl hash::Hash for SourceId {
431+
impl Hash for SourceId {
421432
fn hash<S: hash::Hasher>(&self, into: &mut S) {
422433
self.inner.kind.hash(into);
423434
match *self.inner {

src/cargo/ops/cargo_rustc/context.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
407407
let name = unit.pkg.package_id().name();
408408
match self.target_metadata(unit) {
409409
Some(meta) => format!("{}-{}", name, meta),
410-
None => format!("{}-{}", name, util::short_hash(unit.pkg)),
410+
None => format!("{}-{}", name, self.target_short_hash(unit)),
411411
}
412412
}
413413

@@ -426,6 +426,13 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
426426
self.build_config.requested_target.as_ref().map(|s| &s[..])
427427
}
428428

429+
/// Get the short hash based only on the PackageId
430+
/// Used for the metadata when target_metadata returns None
431+
pub fn target_short_hash(&self, unit: &Unit) -> String {
432+
let hashable = unit.pkg.package_id().stable_hash(self.ws.root());
433+
util::short_hash(&hashable)
434+
}
435+
429436
/// Get the metadata for a target in a specific profile
430437
/// We build to the path: "{filename}-{target_metadata}"
431438
/// We use a linking step to link/copy to a predictable filename
@@ -459,7 +466,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
459466

460467
// Unique metadata per (name, source, version) triple. This'll allow us
461468
// to pull crates from anywhere w/o worrying about conflicts
462-
unit.pkg.package_id().hash(&mut hasher);
469+
unit.pkg.package_id().stable_hash(self.ws.root()).hash(&mut hasher);
463470

464471
// Add package properties which map to environment variables
465472
// exposed by Cargo

src/cargo/ops/cargo_rustc/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use core::{Package, PackageId, PackageSet, Target, Resolve};
1212
use core::{Profile, Profiles, Workspace};
1313
use core::shell::ColorChoice;
1414
use util::{self, ProcessBuilder, machine_message};
15-
use util::{Config, internal, profile, join_paths, short_hash};
15+
use util::{Config, internal, profile, join_paths};
1616
use util::errors::{CargoResult, CargoResultExt};
1717
use util::Freshness;
1818

@@ -791,7 +791,7 @@ fn build_base_args(cx: &mut Context,
791791
cmd.arg("-C").arg(&format!("extra-filename=-{}", m));
792792
}
793793
None => {
794-
cmd.arg("-C").arg(&format!("metadata={}", short_hash(unit.pkg)));
794+
cmd.arg("-C").arg(&format!("metadata={}", cx.target_short_hash(unit)));
795795
}
796796
}
797797

0 commit comments

Comments
 (0)