Auto merge of #8973 - ehuss:rerun-if-directory, r=alexcrichton

Check if rerun-if-changed points to a directory.

This changes it so that if a build script emits `cargo:rerun-if-changed` pointing to a directory, then Cargo will scan the entire directory for changes (instead of just looking at the mtime of the directory itself).  I think this is more useful, as otherwise build scripts have to recreate this logic.

I've tried to make it semi-intelligent in the face of symbolic links.  It checks the mtime of the link and its target, and follows the link if it points to a directory.

There are a few other edge cases. For example, if it doesn't have permission for a directory, it will skip it.  I think this is relatively reasonable, though it's hard to say for sure.

Author: bors

src/cargo/core/compiler/fingerprint.rs

@@ -1700,7 +1700,7 @@ where
         let path_mtime = match mtime_cache.entry(path.to_path_buf()) {
             Entry::Occupied(o) => *o.get(),
             Entry::Vacant(v) => {
-                let mtime = match paths::mtime(path) {
+                let mtime = match paths::mtime_recursive(path) {
                     Ok(mtime) => mtime,
                     Err(..) => return Some(StaleItem::MissingFile(path.to_path_buf())),
                 };

src/cargo/util/paths.rs

@@ -180,6 +180,87 @@ pub fn mtime(path: &Path) -> CargoResult<FileTime> {
     Ok(FileTime::from_last_modification_time(&meta))
 }
 
+/// Returns the maximum mtime of the given path, recursing into
+/// subdirectories, and following symlinks.
+pub fn mtime_recursive(path: &Path) -> CargoResult<FileTime> {
+    let meta = fs::metadata(path).chain_err(|| format!("failed to stat `{}`", path.display()))?;
+    if !meta.is_dir() {
+        return Ok(FileTime::from_last_modification_time(&meta));
+    }
+    let max_meta = walkdir::WalkDir::new(path)
+        .follow_links(true)
+        .into_iter()
+        .filter_map(|e| match e {
+            Ok(e) => Some(e),
+            Err(e) => {
+                // Ignore errors while walking. If Cargo can't access it, the
+                // build script probably can't access it, either.
+                log::debug!("failed to determine mtime while walking directory: {}", e);
+                None
+            }
+        })
+        .filter_map(|e| {
+            if e.path_is_symlink() {
+                // Use the mtime of both the symlink and its target, to
+                // handle the case where the symlink is modified to a
+                // different target.
+                let sym_meta = match std::fs::symlink_metadata(e.path()) {
+                    Ok(m) => m,
+                    Err(err) => {
+                        // I'm not sure when this is really possible (maybe a
+                        // race with unlinking?). Regardless, if Cargo can't
+                        // read it, the build script probably can't either.
+                        log::debug!(
+                            "failed to determine mtime while fetching symlink metdata of {}: {}",
+                            e.path().display(),
+                            err
+                        );
+                        return None;
+                    }
+                };
+                let sym_mtime = FileTime::from_last_modification_time(&sym_meta);
+                // Walkdir follows symlinks.
+                match e.metadata() {
+                    Ok(target_meta) => {
+                        let target_mtime = FileTime::from_last_modification_time(&target_meta);
+                        Some(sym_mtime.max(target_mtime))
+                    }
+                    Err(err) => {
+                        // Can't access the symlink target. If Cargo can't
+                        // access it, the build script probably can't access
+                        // it either.
+                        log::debug!(
+                            "failed to determine mtime of symlink target for {}: {}",
+                            e.path().display(),
+                            err
+                        );
+                        Some(sym_mtime)
+                    }
+                }
+            } else {
+                let meta = match e.metadata() {
+                    Ok(m) => m,
+                    Err(err) => {
+                        // I'm not sure when this is really possible (maybe a
+                        // race with unlinking?). Regardless, if Cargo can't
+                        // read it, the build script probably can't either.
+                        log::debug!(
+                            "failed to determine mtime while fetching metadata of {}: {}",
+                            e.path().display(),
+                            err
+                        );
+                        return None;
+                    }
+                };
+                Some(FileTime::from_last_modification_time(&meta))
+            }
+        })
+        .max()
+        // or_else handles the case where there are no files in the directory.
+        .unwrap_or_else(|| FileTime::from_last_modification_time(&meta));
+    Ok(max_meta)
+}
+
 /// Record the current time on the filesystem (using the filesystem's clock)
 /// using a file at the given directory. Returns the current time.
 pub fn set_invocation_time(path: &Path) -> CargoResult<FileTime> {

src/doc/src/reference/build-scripts.md

@@ -258,19 +258,16 @@ filesystem last-modified "mtime" timestamp to determine if the file has
 changed. It compares against an internal cached timestamp of when the build
 script last ran.
 
-If the path points to a directory, it does *not* automatically traverse the
-directory for changes. Only the mtime change of the directory itself is
-considered (which corresponds to some types of changes within the directory,
-depending on platform). To request a re-run on any changes within an entire
-directory, print a line for the directory and separate lines for everything
-inside it, recursively.
+If the path points to a directory, it will scan the entire directory for
+any modifications.
 
 If the build script inherently does not need to re-run under any circumstance,
 then emitting `cargo:rerun-if-changed=build.rs` is a simple way to prevent it
-from being re-run. Cargo automatically handles whether or not the script
-itself needs to be recompiled, and of course the script will be re-run after
-it has been recompiled. Otherwise, specifying `build.rs` is redundant and
-unnecessary.
+from being re-run (otherwise, the default if no `rerun-if` instructions are
+emitted is to scan the entire package directory for changes). Cargo
+automatically handles whether or not the script itself needs to be recompiled,
+and of course the script will be re-run after it has been recompiled.
+Otherwise, specifying `build.rs` is redundant and unnecessary.
 
 <a id="rerun-if-env-changed"></a>
 #### `cargo:rerun-if-env-changed=NAME`

tests/testsuite/build_script.rs

@@ -8,8 +8,8 @@ use std::thread;
 use cargo::util::paths::remove_dir_all;
 use cargo_test_support::paths::CargoPathExt;
 use cargo_test_support::registry::Package;
-use cargo_test_support::{basic_manifest, cross_compile, project};
-use cargo_test_support::{rustc_host, sleep_ms, slow_cpu_multiplier};
+use cargo_test_support::{basic_manifest, cross_compile, is_coarse_mtime, project};
+use cargo_test_support::{rustc_host, sleep_ms, slow_cpu_multiplier, symlink_supported};
 
 #[cargo_test]
 fn custom_build_script_failed() {
@@ -4071,3 +4071,90 @@ fn dev_dep_with_links() {
         .build();
     p.cargo("check --tests").run()
 }
+
+#[cargo_test]
+fn rerun_if_directory() {
+    if !symlink_supported() {
+        return;
+    }
+
+    // rerun-if-changed of a directory should rerun if any file in the directory changes.
+    let p = project()
+        .file("Cargo.toml", &basic_manifest("foo", "0.1.0"))
+        .file("src/lib.rs", "")
+        .file(
+            "build.rs",
+            r#"
+                fn main() {
+                    println!("cargo:rerun-if-changed=somedir");
+                }
+            "#,
+        )
+        .build();
+
+    let dirty = || {
+        p.cargo("check")
+            .with_stderr(
+                "[COMPILING] foo [..]\n\
+                 [FINISHED] [..]",
+            )
+            .run();
+    };
+
+    let fresh = || {
+        p.cargo("check").with_stderr("[FINISHED] [..]").run();
+    };
+
+    // Start with a missing directory.
+    dirty();
+    // Because the directory doesn't exist, it will trigger a rebuild every time.
+    // https://github.com/rust-lang/cargo/issues/6003
+    dirty();
+
+    if is_coarse_mtime() {
+        sleep_ms(1000);
+    }
+
+    // Empty directory.
+    fs::create_dir(p.root().join("somedir")).unwrap();
+    dirty();
+    fresh();
+
+    if is_coarse_mtime() {
+        sleep_ms(1000);
+    }
+
+    // Add a file.
+    p.change_file("somedir/foo", "");
+    p.change_file("somedir/bar", "");
+    dirty();
+    fresh();
+
+    if is_coarse_mtime() {
+        sleep_ms(1000);
+    }
+
+    // Add a symlink.
+    p.symlink("foo", "somedir/link");
+    dirty();
+    fresh();
+
+    if is_coarse_mtime() {
+        sleep_ms(1000);
+    }
+
+    // Move the symlink.
+    fs::remove_file(p.root().join("somedir/link")).unwrap();
+    p.symlink("bar", "somedir/link");
+    dirty();
+    fresh();
+
+    if is_coarse_mtime() {
+        sleep_ms(1000);
+    }
+
+    // Remove a file.
+    fs::remove_file(p.root().join("somedir/foo")).unwrap();
+    dirty();
+    fresh();
+}