Stricter package change detection.

Author: ehuss

Cargo.toml

@@ -59,6 +59,7 @@ termcolor = "1.0"
 toml = "0.4.2"
 url = "1.1"
 url_serde = "0.2.0"
+walkdir = "2.2"
 clap = "2.31.2"
 unicode-width = "0.1.5"
 openssl = { version = '0.10.11', optional = true }

src/cargo/ops/cargo_package.rs

@@ -1,3 +1,4 @@
+use std::collections::HashMap;
 use std::fs::{self, File};
 use std::io::prelude::*;
 use std::io::SeekFrom;
@@ -439,7 +440,7 @@ fn run_verify(ws: &Workspace<'_>, tar: &FileLock, opts: &PackageOpts<'_>) -> Car
     let id = SourceId::for_path(&dst)?;
     let mut src = PathSource::new(&dst, id, ws.config());
     let new_pkg = src.root_package()?;
-    let pkg_fingerprint = src.last_modified_file(&new_pkg)?;
+    let pkg_fingerprint = hash_all(&dst)?;
     let ws = Workspace::ephemeral(new_pkg, config, None, true)?;
 
     let exec: Arc<dyn Executor> = Arc::new(DefaultExecutor);
@@ -465,21 +466,83 @@ fn run_verify(ws: &Workspace<'_>, tar: &FileLock, opts: &PackageOpts<'_>) -> Car
     )?;
 
     // Check that `build.rs` didn't modify any files in the `src` directory.
-    let ws_fingerprint = src.last_modified_file(ws.current()?)?;
+    let ws_fingerprint = hash_all(&dst)?;
     if pkg_fingerprint != ws_fingerprint {
-        let (_, path) = ws_fingerprint;
+        let changes = report_hash_difference(&pkg_fingerprint, &ws_fingerprint);
         failure::bail!(
             "Source directory was modified by build.rs during cargo publish. \
-             Build scripts should not modify anything outside of OUT_DIR. \
-             Modified file: {}\n\n\
+             Build scripts should not modify anything outside of OUT_DIR.\n\
+             {}\n\n\
              To proceed despite this, pass the `--no-verify` flag.",
-            path.display()
+            changes
         )
     }
 
     Ok(())
 }
 
+fn hash_all(path: &Path) -> CargoResult<HashMap<PathBuf, u64>> {
+    fn wrap(path: &Path) -> CargoResult<HashMap<PathBuf, u64>> {
+        let mut result = HashMap::new();
+        let walker = walkdir::WalkDir::new(path).into_iter();
+        for entry in walker.filter_entry(|e| {
+            !(e.depth() == 1 && (e.file_name() == "target" || e.file_name() == "Cargo.lock"))
+        }) {
+            let entry = entry?;
+            let file_type = entry.file_type();
+            if file_type.is_file() {
+                let contents = fs::read(entry.path())?;
+                let hash = util::hex::hash_u64(&contents);
+                result.insert(entry.path().to_path_buf(), hash);
+            } else if file_type.is_symlink() {
+                let hash = util::hex::hash_u64(&fs::read_link(entry.path())?);
+                result.insert(entry.path().to_path_buf(), hash);
+            }
+        }
+        Ok(result)
+    }
+    let result = wrap(path).chain_err(|| format!("failed to verify output at {:?}", path))?;
+    Ok(result)
+}
+
+fn report_hash_difference(
+    orig: &HashMap<PathBuf, u64>,
+    after: &HashMap<PathBuf, u64>,
+) -> String {
+    let mut changed = Vec::new();
+    let mut removed = Vec::new();
+    for (key, value) in orig {
+        match after.get(key) {
+            Some(after_value) => {
+                if value != after_value {
+                    changed.push(key.to_string_lossy());
+                }
+            }
+            None => removed.push(key.to_string_lossy()),
+        }
+    }
+    let mut added: Vec<_> = after
+        .keys()
+        .filter(|key| !orig.contains_key(*key))
+        .map(|key| key.to_string_lossy())
+        .collect();
+    let mut result = Vec::new();
+    if !changed.is_empty() {
+        changed.sort_unstable();
+        result.push(format!("Changed: {}", changed.join("\n\t")));
+    }
+    if !added.is_empty() {
+        added.sort_unstable();
+        result.push(format!("Added: {}", added.join("\n\t")));
+    }
+    if !removed.is_empty() {
+        removed.sort_unstable();
+        result.push(format!("Removed: {}", removed.join("\n\t")));
+    }
+    assert!(!result.is_empty(), "unexpected empty change detection");
+    result.join("\n")
+}
+
 // It can often be the case that files of a particular name on one platform
 // can't actually be created on another platform. For example files with colons
 // in the name are allowed on Unix but not on Windows.

tests/testsuite/package.rs

@@ -3,12 +3,12 @@ use std::fs::File;
 use std::io::prelude::*;
 use std::path::Path;
 
+use crate::support::cargo_process;
 use crate::support::registry::Package;
 use crate::support::{
     basic_manifest, git, is_nightly, path2url, paths, project, publish::validate_crate_contents,
     registry,
 };
-use crate::support::{cargo_process, sleep_ms};
 use git2;
 
 #[test]
@@ -1201,27 +1201,24 @@ fn lock_file_and_workspace() {
 fn do_not_package_if_src_was_modified() {
     let p = project()
         .file("src/main.rs", r#"fn main() { println!("hello"); }"#)
+        .file("foo.txt", "")
+        .file("bar.txt", "")
         .file(
             "build.rs",
             r#"
-            use std::fs::File;
-            use std::io::Write;
+            use std::fs;
 
             fn main() {
-                let mut file = File::create("src/generated.txt").expect("failed to create file");
-                file.write_all(b"Hello, world of generated files.").expect("failed to write");
+                fs::write("src/generated.txt",
+                    "Hello, world of generated files."
+                ).expect("failed to create file");
+                fs::remove_file("foo.txt").expect("failed to remove");
+                fs::write("bar.txt", "updated content").expect("failed to update");
             }
         "#,
         )
         .build();
 
-    if cfg!(target_os = "macos") {
-        // MacOS has 1s resolution filesystem.
-        // If src/main.rs is created within 1s of src/generated.txt, then it
-        // won't trigger the modification check.
-        sleep_ms(1000);
-    }
-
     p.cargo("package")
         .with_status(101)
         .with_stderr_contains(
@@ -1230,7 +1227,10 @@ error: failed to verify package tarball
 
 Caused by:
   Source directory was modified by build.rs during cargo publish. \
-Build scripts should not modify anything outside of OUT_DIR. Modified file: [..]src/generated.txt
+Build scripts should not modify anything outside of OUT_DIR.
+Changed: [CWD]/target/package/foo-0.0.1/bar.txt
+Added: [CWD]/target/package/foo-0.0.1/src/generated.txt
+Removed: [CWD]/target/package/foo-0.0.1/foo.txt
 
 To proceed despite this, pass the `--no-verify` flag.",
         )