Auto merge of #5181 - alexcrichton:selectively-write-dep-info, r=matklad

Don't rewrite dep-info files if they don't change

Similar to how we treat lock files, read the contents, compare, and if they're
the same don't actually write the file.

Closes #5172

Author: bors

src/cargo/ops/cargo_rustc/fingerprint.rs

@@ -738,29 +738,13 @@ pub fn translate_dep_info(rustc_dep_info: &Path,
                           cargo_dep_info: &Path,
                           pkg_root: &Path,
                           rustc_cwd: &Path) -> CargoResult<()> {
-    let contents = paths::read(rustc_dep_info)?;
-    let line = match contents.lines().next() {
-        Some(line) => line,
-        None => return Ok(()),
-    };
-    let pos = line.find(": ").ok_or_else(|| {
-        internal(format!("dep-info not in an understood format: {}",
-                         rustc_dep_info.display()))
-    })?;
-    let deps = &line[pos + 2..];
+    let target = parse_rustc_dep_info(rustc_dep_info)?;
+    let deps = &target.get(0).ok_or_else(|| {
+        internal("malformed dep-info format, no targets".to_string())
+    })?.1;
 
     let mut new_contents = Vec::new();
-    let mut deps = deps.split(' ').map(|s| s.trim()).filter(|s| !s.is_empty());
-    while let Some(s) = deps.next() {
-        let mut file = s.to_string();
-        while file.ends_with('\\') {
-            file.pop();
-            file.push(' ');
-            file.push_str(deps.next().ok_or_else(|| {
-                internal("malformed dep-info format, trailing \\".to_string())
-            })?);
-        }
-
+    for file in deps {
         let absolute = rustc_cwd.join(file);
         let path = absolute.strip_prefix(pkg_root).unwrap_or(&absolute);
         new_contents.extend(util::path2bytes(path)?);
@@ -770,3 +754,29 @@ pub fn translate_dep_info(rustc_dep_info: &Path,
     Ok(())
 }
 
+pub fn parse_rustc_dep_info(rustc_dep_info: &Path)
+    -> CargoResult<Vec<(String, Vec<String>)>>
+{
+    let contents = paths::read(rustc_dep_info)?;
+    contents.lines()
+        .filter_map(|l| l.find(": ").map(|i| (l, i)))
+        .map(|(line, pos)| {
+            let target = &line[..pos];
+            let mut deps = line[pos + 2..].split_whitespace();
+
+            let mut ret = Vec::new();
+            while let Some(s) = deps.next() {
+                let mut file = s.to_string();
+                while file.ends_with('\\') {
+                    file.pop();
+                    file.push(' ');
+                    file.push_str(deps.next().ok_or_else(|| {
+                        internal("malformed dep-info format, trailing \\".to_string())
+                    })?);
+                }
+                ret.push(file);
+            }
+            Ok((target.to_string(), ret))
+        })
+        .collect()
+}

src/cargo/ops/cargo_rustc/output_depinfo.rs

@@ -1,4 +1,4 @@
-use std::collections::HashSet;
+use std::collections::{HashSet, BTreeSet};
 use std::io::{Write, BufWriter};
 use std::fs::File;
 use std::path::{Path, PathBuf};
@@ -21,7 +21,7 @@ fn render_filename<P: AsRef<Path>>(path: P, basedir: Option<&str>) -> CargoResul
 }
 
 fn add_deps_for_unit<'a, 'b>(
-    deps: &mut HashSet<PathBuf>,
+    deps: &mut BTreeSet<PathBuf>,
     context: &mut Context<'a, 'b>,
     unit: &Unit<'a>,
     visited: &mut HashSet<Unit<'a>>,
@@ -67,7 +67,7 @@ fn add_deps_for_unit<'a, 'b>(
 }
 
 pub fn output_depinfo<'a, 'b>(context: &mut Context<'a, 'b>, unit: &Unit<'a>) -> CargoResult<()> {
-    let mut deps = HashSet::new();
+    let mut deps = BTreeSet::new();
     let mut visited = HashSet::new();
     let success = add_deps_for_unit(&mut deps, context, unit, &mut visited).is_ok();
     let basedir_string;
@@ -79,15 +79,31 @@ pub fn output_depinfo<'a, 'b>(context: &mut Context<'a, 'b>, unit: &Unit<'a>) ->
         }
         None => None,
     };
+    let deps = deps.iter()
+        .map(|f| render_filename(f, basedir))
+        .collect::<CargoResult<Vec<_>>>()?;
+
     for &(_, ref link_dst, _) in context.target_filenames(unit)?.iter() {
         if let Some(ref link_dst) = *link_dst {
             let output_path = link_dst.with_extension("d");
             if success {
-                let mut outfile = BufWriter::new(File::create(output_path)?);
                 let target_fn = render_filename(link_dst, basedir)?;
+
+                // If nothing changed don't recreate the file which could alter
+                // its mtime
+                if let Ok(previous) = fingerprint::parse_rustc_dep_info(&output_path) {
+                    if previous.len() == 1 &&
+                        previous[0].0 == target_fn &&
+                        previous[0].1 == deps {
+                        continue
+                    }
+                }
+
+                // Otherwise write it all out
+                let mut outfile = BufWriter::new(File::create(output_path)?);
                 write!(outfile, "{}:", target_fn)?;
                 for dep in &deps {
-                    write!(outfile, " {}", render_filename(dep, basedir)?)?;
+                    write!(outfile, " {}", dep)?;
                 }
                 writeln!(outfile, "")?;
 

tests/testsuite/dep_info.rs

@@ -1,4 +1,5 @@
 use cargotest::support::{basic_bin_manifest, main_file, execs, project};
+use filetime::FileTime;
 use hamcrest::{assert_that, existing_file};
 
 #[test]
@@ -79,3 +80,27 @@ fn build_dep_info_dylib() {
     assert_that(p.cargo("build").arg("--example=ex"), execs().with_status(0));
     assert_that(&p.example_lib("ex", "dylib").with_extension("d"), existing_file());
 }
+
+#[test]
+fn no_rewrite_if_no_change() {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+        "#)
+        .file("src/lib.rs", "")
+        .build();
+
+    assert_that(p.cargo("build"), execs().with_status(0));
+    let dep_info = p.root().join("target/debug/libfoo.d");
+    let metadata1 = dep_info.metadata().unwrap();
+    assert_that(p.cargo("build"), execs().with_status(0));
+    let metadata2 = dep_info.metadata().unwrap();
+
+    assert_eq!(
+        FileTime::from_last_modification_time(&metadata1),
+        FileTime::from_last_modification_time(&metadata2),
+    );
+}