Skip to content

Commit d2e8587

Browse files
committed
Auto merge of #3611 - tylerwhall:stable-metadata, r=alexcrichton
Stable metadata hashes across workspaces 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. This is hopefully a general solution to the hack we've used in meta-rust to make dynamic linking reliable. meta-rust/meta-rust@0e6cf94 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.
2 parents fe56d43 + eb0e4c0 commit d2e8587

File tree

6 files changed

+74
-7
lines changed

6 files changed

+74
-7
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
@@ -460,7 +467,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
460467

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

465472
// Add package properties which map to environment variables
466473
// 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

@@ -802,7 +802,7 @@ fn build_base_args(cx: &mut Context,
802802
cmd.arg("-C").arg(&format!("extra-filename=-{}", m));
803803
}
804804
None => {
805-
cmd.arg("-C").arg(&format!("metadata={}", short_hash(unit.pkg)));
805+
cmd.arg("-C").arg(&format!("metadata={}", cx.target_short_hash(unit)));
806806
}
807807
}
808808

tests/build.rs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
extern crate cargo;
2+
#[macro_use]
23
extern crate cargotest;
34
extern crate hamcrest;
45
extern crate tempdir;
@@ -3519,3 +3520,32 @@ fn inferred_path_in_src_bin_foo() {
35193520
assert_that(p.cargo_process("build"), execs().with_status(0));
35203521
assert_that(&p.bin("bar"), existing_file());
35213522
}
3523+
3524+
#[test]
3525+
fn same_metadata_different_directory() {
3526+
// A top-level crate built in two different workspaces should have the
3527+
// same metadata hash.
3528+
let p = project("foo1")
3529+
.file("Cargo.toml", &basic_bin_manifest("foo"))
3530+
.file("src/foo.rs", &main_file(r#""i am foo""#, &[]));
3531+
let output = t!(String::from_utf8(
3532+
t!(p.cargo_process("build").arg("-v").exec_with_output())
3533+
.stderr,
3534+
));
3535+
let metadata = output
3536+
.split_whitespace()
3537+
.filter(|arg| arg.starts_with("metadata="))
3538+
.next()
3539+
.unwrap();
3540+
3541+
let p = project("foo2")
3542+
.file("Cargo.toml", &basic_bin_manifest("foo"))
3543+
.file("src/foo.rs", &main_file(r#""i am foo""#, &[]));
3544+
3545+
assert_that(
3546+
p.cargo_process("build").arg("-v"),
3547+
execs().with_status(0).with_stderr_contains(
3548+
format!("[..]{}[..]", metadata),
3549+
),
3550+
);
3551+
}

tests/path.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
extern crate cargo;
2+
#[macro_use]
23
extern crate cargotest;
34
extern crate hamcrest;
45

@@ -783,6 +784,8 @@ fn custom_target_no_rebuild() {
783784
authors = []
784785
[dependencies]
785786
a = { path = "a" }
787+
[workspace]
788+
members = ["a", "b"]
786789
"#)
787790
.file("src/lib.rs", "")
788791
.file("a/Cargo.toml", r#"
@@ -810,9 +813,10 @@ fn custom_target_no_rebuild() {
810813
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
811814
"));
812815

816+
t!(fs::rename(p.root().join("target"), p.root().join("target_moved")));
813817
assert_that(p.cargo("build")
814818
.arg("--manifest-path=b/Cargo.toml")
815-
.env("CARGO_TARGET_DIR", "target"),
819+
.env("CARGO_TARGET_DIR", "target_moved"),
816820
execs().with_status(0)
817821
.with_stderr("\
818822
[COMPILING] b v0.5.0 ([..])

0 commit comments

Comments
 (0)