Make `dep_targets` consistent throughout compilation
authorAlex Crichton <alex@alexcrichton.com>
Sat, 9 Sep 2017 20:52:03 +0000 (13:52 -0700)
committerAlex Crichton <alex@alexcrichton.com>
Sat, 9 Sep 2017 20:56:16 +0000 (13:56 -0700)
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`.

src/cargo/ops/cargo_rustc/context.rs
src/cargo/ops/cargo_rustc/custom_build.rs

index ce3bba0a84115f4559074cd4fc7fce06a6908cbb..56bbe94701d423c1cf1d8f1d32010cba22126a4e 100755 (executable)
@@ -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>>,
@@ -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(),
@@ -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<_>>();
@@ -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| {
@@ -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
@@ -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())
         }
 
index 727f0d324bff32c6b6b9893b9d3610acb32e173e..ec78ce4e44108123154760f623b288b031e962ac 100644 (file)
@@ -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 {
@@ -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 {
@@ -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
@@ -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() {