Skip to content
This repository was archived by the owner on Dec 29, 2022. It is now read-only.

Commit e2c8c28

Browse files
committed
Revert calculating dirty units from pre-cached inputs
Reverts change made in bdf0d92. There were some edge cases wrt relative/absolute paths and varying inputs cwd, so for now don't use the patch and rely on the old logic, instead.
1 parent bef52e3 commit e2c8c28

File tree

1 file changed

+55
-15
lines changed

1 file changed

+55
-15
lines changed

src/build/cargo_plan.rs

Lines changed: 55 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ impl CargoPlan {
128128
let unit_key = (id.clone(), target.clone(), mode);
129129
trace!("Caching these files: {:#?} for {:?} key", &input_files, &unit_key);
130130

131-
// Create reverse file -> unit mapping (used for dirty unit calculation)
131+
// Create reverse file -> unit mapping (to be used for dirty unit calculation)
132132
for file in &input_files {
133133
self.file_key_mapping
134134
.entry(file.to_path_buf())
@@ -197,7 +197,6 @@ impl CargoPlan {
197197
}
198198
}
199199

200-
/// TODO: Update TODO below
201200
/// TODO: Improve detecting dirty crate targets for a set of dirty file paths.
202201
/// This uses a lousy heuristic of checking path prefix for a given crate
203202
/// target to determine whether a given unit (crate target) is dirty. This
@@ -207,7 +206,7 @@ impl CargoPlan {
207206
/// Because of that, build scripts are checked separately and only other
208207
/// crate targets are checked with path prefixes.
209208
fn fetch_dirty_units<T: AsRef<Path>>(&self, files: &[T]) -> HashSet<UnitKey> {
210-
let files = files.iter().map(|f| f.as_ref());
209+
let mut result = HashSet::new();
211210

212211
let build_scripts: HashMap<&Path, UnitKey> = self
213212
.units
@@ -217,18 +216,59 @@ impl CargoPlan {
217216
})
218217
.map(|(key, unit)| (unit.target.src_path().path(), key.clone()))
219218
.collect();
220-
221-
let mut result = HashSet::new();
222-
223-
let dirty_rustc_units = files
224-
.clone()
225-
.filter_map(|f| self.file_key_mapping.get(f))
226-
.flat_map(|units| units);
227-
let dirty_build_scripts = files.filter_map(|f| build_scripts.get(f));
228-
229-
result.extend(dirty_rustc_units.cloned());
230-
result.extend(dirty_build_scripts.cloned());
231-
219+
let other_targets: HashMap<UnitKey, &Path> = self
220+
.units
221+
.iter()
222+
.filter(|&(&(_, ref target, _), _)| *target.kind() != TargetKind::CustomBuild)
223+
.map(|(key, unit)| {
224+
(
225+
key.clone(),
226+
unit.target
227+
.src_path()
228+
.path()
229+
.parent()
230+
.expect("no parent for src_path"),
231+
)
232+
}).collect();
233+
234+
for modified in files.iter().map(|x| x.as_ref()) {
235+
if let Some(unit) = build_scripts.get(modified) {
236+
result.insert(unit.clone());
237+
} else {
238+
// Not a build script, so we associate a dirty file with a
239+
// package by finding longest (most specified) path prefix.
240+
let matching_prefix_components = |a: &Path, b: &Path| -> usize {
241+
assert!(a.is_absolute() && b.is_absolute());
242+
a.components().zip(b.components())
243+
.skip(1) // Skip RootDir
244+
.take_while(|&(x, y)| x == y)
245+
.count()
246+
};
247+
// Since a package can correspond to many units (e.g. compiled
248+
// as a regular binary or a test harness for unit tests), we
249+
// collect every unit having the longest path prefix.
250+
let max_matching_prefix = other_targets
251+
.values()
252+
.map(|src_dir| matching_prefix_components(modified, src_dir))
253+
.max();
254+
255+
match max_matching_prefix {
256+
Some(0) => error!(
257+
"Modified file {} didn't correspond to any buildable unit!",
258+
modified.display()
259+
),
260+
Some(max) => {
261+
let dirty_units = other_targets
262+
.iter()
263+
.filter(|(_, dir)| max == matching_prefix_components(modified, dir))
264+
.map(|(unit, _)| unit);
265+
266+
result.extend(dirty_units.cloned());
267+
}
268+
None => {} // Possible that only build scripts were modified
269+
}
270+
}
271+
}
232272
result
233273
}
234274

0 commit comments

Comments
 (0)