-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Check if rerun-if-changed points to a directory. #8973
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems a bit odd to me, if walkdir follows symlinks then how come we need to manually check the mtime of the symlink target? If the symlink points to a directory doesn't that mean we're doing the same thing as before where we're looking at the mtime of a directory which isn't too useful? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you have:
If you run a build, it will use the max mtime of those 3 files. If you then modify If the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah I see. I didn't actually realized until just before this that the mtime of directories was important. I was mostly wondering here why it dealt with both the target and the source since for file-based symlinks I think the symlink mtime itself should be appropriate, but even then I can see how it would matter if the file is located outside of the directory. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This seems like an error-prone way to find out a file was deleted. I think instead Cargo should cache the list of files in the directory (recursively) and consider the directory changed if any file that is in that cached list doesn't exist (or a new file exists that isn't on the list), in addition to the file timestamping. |
||
} | ||
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> { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm actually a little surprised we already had this dependency...