Skip to content

Commit

Permalink
Make dep_targets consistent throughout compilation
Browse files Browse the repository at this point in the history
Previously it depended on dynamic state that was calculated throughout a
compilation which ended up causing different fingerprints showing up in a few
locations, so this makes the invocation deterministic throughout `cargo_rustc`.
  • Loading branch information
alexcrichton committed Sep 9, 2017
1 parent 67b3b44 commit f90d21f
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 32 deletions.
33 changes: 15 additions & 18 deletions src/cargo/ops/cargo_rustc/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ pub struct Context<'a, 'cfg: 'a> {
pub compilation: Compilation<'cfg>,
pub packages: &'a PackageSet<'cfg>,
pub build_state: Arc<BuildState>,
pub build_script_overridden: HashSet<(PackageId, Kind)>,
pub build_explicit_deps: HashMap<Unit<'a>, BuildDeps>,
pub fingerprints: HashMap<Unit<'a>, Arc<Fingerprint>>,
pub compiled: HashSet<Unit<'a>>,
Expand Down Expand Up @@ -156,6 +157,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
used_in_plugin: HashSet::new(),
incremental_enabled: incremental_enabled,
jobserver: jobserver,
build_script_overridden: HashSet::new(),

// TODO: Pre-Calculate these with a topo-sort, rather than lazy-calculating
target_filenames: HashMap::new(),
Expand Down Expand Up @@ -499,7 +501,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
self.resolve.features_sorted(unit.pkg.package_id()).hash(&mut hasher);

// Mix in the target-metadata of all the dependencies of this target
if let Ok(deps) = self.used_deps(unit) {
if let Ok(deps) = self.dep_targets(unit) {
let mut deps_metadata = deps.into_iter().map(|dep_unit| {
self.target_metadata(&dep_unit)
}).collect::<Vec<_>>();
Expand Down Expand Up @@ -695,10 +697,18 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
Ok(Arc::new(ret))
}

fn used_deps(&self, unit: &Unit<'a>) -> CargoResult<Vec<Unit<'a>>> {
/// For a package, return all targets which are registered as dependencies
/// for that package.
pub fn dep_targets(&self, unit: &Unit<'a>) -> CargoResult<Vec<Unit<'a>>> {
if unit.profile.run_custom_build {
return self.dep_run_custom_build(unit)
} else if unit.profile.doc && !unit.profile.test {
return self.doc_deps(unit);
}

let id = unit.pkg.package_id();
let deps = self.resolve.deps(id);
deps.filter(|dep| {
let mut ret = deps.filter(|dep| {
unit.pkg.dependencies().iter().filter(|d| {
d.name() == dep.name() && d.version_req().matches(dep.version())
}).any(|d| {
Expand Down Expand Up @@ -747,20 +757,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
}
Err(e) => Some(Err(e))
}
}).collect::<CargoResult<Vec<_>>>()
}

/// For a package, return all targets which are registered as dependencies
/// for that package.
pub fn dep_targets(&self, unit: &Unit<'a>) -> CargoResult<Vec<Unit<'a>>> {
if unit.profile.run_custom_build {
return self.dep_run_custom_build(unit)
} else if unit.profile.doc && !unit.profile.test {
return self.doc_deps(unit);
}

let id = unit.pkg.package_id();
let mut ret = self.used_deps(unit)?;
}).collect::<CargoResult<Vec<_>>>()?;

// If this target is a build script, then what we've collected so far is
// all we need. If this isn't a build script, then it depends on the
Expand Down Expand Up @@ -812,7 +809,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
// actually depend on anything, we've reached the end of the dependency
// chain as we've got all the info we're gonna get.
let key = (unit.pkg.package_id().clone(), unit.kind);
if self.build_state.outputs.lock().unwrap().contains_key(&key) {
if self.build_script_overridden.contains(&key) {
return Ok(Vec::new())
}

Expand Down
32 changes: 18 additions & 14 deletions src/cargo/ops/cargo_rustc/custom_build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,9 @@ pub fn prepare<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>)
-> CargoResult<(Work, Work, Freshness)> {
let _p = profile::start(format!("build script prepare: {}/{}",
unit.pkg, unit.target.name()));
let overridden = cx.build_state.has_override(unit);

let key = (unit.pkg.package_id().clone(), unit.kind);
let overridden = cx.build_script_overridden.contains(&key);
let (work_dirty, work_fresh) = if overridden {
(Work::noop(), Work::noop())
} else {
Expand Down Expand Up @@ -314,18 +316,6 @@ impl BuildState {
fn insert(&self, id: PackageId, kind: Kind, output: BuildOutput) {
self.outputs.lock().unwrap().insert((id, kind), output);
}

fn has_override(&self, unit: &Unit) -> bool {
let key = unit.pkg.manifest().links().map(|l| (l.to_string(), unit.kind));
match key.and_then(|k| self.overrides.get(&k)) {
Some(output) => {
self.insert(unit.pkg.package_id().clone(), unit.kind,
output.clone());
true
}
None => false,
}
}
}

impl BuildOutput {
Expand Down Expand Up @@ -483,7 +473,7 @@ pub fn build_map<'b, 'cfg>(cx: &mut Context<'b, 'cfg>,
// Recursive function to build up the map we're constructing. This function
// memoizes all of its return values as it goes along.
fn build<'a, 'b, 'cfg>(out: &'a mut HashMap<Unit<'b>, BuildScripts>,
cx: &Context<'b, 'cfg>,
cx: &mut Context<'b, 'cfg>,
unit: &Unit<'b>)
-> CargoResult<&'a BuildScripts> {
// Do a quick pre-flight check to see if we've already calculated the
Expand All @@ -492,6 +482,20 @@ pub fn build_map<'b, 'cfg>(cx: &mut Context<'b, 'cfg>,
return Ok(&out[unit])
}

{
let key = unit.pkg.manifest().links().map(|l| (l.to_string(), unit.kind));
let build_state = &cx.build_state;
if let Some(output) = key.and_then(|k| build_state.overrides.get(&k)) {
let key = (unit.pkg.package_id().clone(), unit.kind);
cx.build_script_overridden.insert(key.clone());
build_state
.outputs
.lock()
.unwrap()
.insert(key, output.clone());
}
}

let mut ret = BuildScripts::default();

if !unit.target.is_custom_build() && unit.pkg.has_custom_build() {
Expand Down

0 comments on commit f90d21f

Please sign in to comment.