Be resilient to most IO error and filesystem loop while walking dirs#10214
Be resilient to most IO error and filesystem loop while walking dirs#10214bors merged 3 commits intorust-lang:masterfrom
Conversation
|
r? @ehuss (rust-highfive has picked a reviewer for you, use r? to override) |
|
Is there a way we could detect and filter these cases from |
|
Indeed we can recover from broken symlink and some IO errors. For filesystem loop, I guess we can simply emit a warning inside The code snippet need to change as below: for entry in walkdir {
match entry {
Ok(entry) => {
if !entry.file_type().is_dir() {
ret.push(entry.into_path());
}
},
Err(e) => {
if e.loop_ancestor().is_some() {
// emit some warning!!!!!
continue
}
if let Some(path) = e.path() {
ret.push(path.to_path_buf());
}
}
}
} |
|
If a loop is as common as |
67491f3 to
a457dee
Compare
|
From my own experience, a normal Node.js project should not have any nested |
src/cargo/sources/path.rs
Outdated
There was a problem hiding this comment.
I'm curious, what's going on here? This looks like it's ignoring all errors that are not permission denied?
There was a problem hiding this comment.
If cargo doesn't handle permission error intentionally, this EACCES test case might pass silently. The old behavior before adopting walkdir returns errors from fs::read_dir calls directly, as well as it just unwraps DirEntry without handling Result.
cargo/src/cargo/sources/path.rs
Lines 413 to 416 in a359ce1
With walkdir, cargo cannot tell whether the error is permission error or a broken link. Generally, walkdir emits three kinds of errors:
- Symlink loop errors: Cargo already takes care of it and shows a warning in this PR.
- IO errors with a path: I guess cargo can recover from the path, and later let the caller decide to ignore it or if the caller does access the path and hits the error.
- IO errros without a path: Cargo can simply propagate it to callers. This is rare because walkdir only emits this kind for either opening the file to check symlink loop or handling the
Result<DirEntry>generated fromread_dir().
I've updated this PR to reflect aforementioned change. There is an enhancement can do such as emitting warnings for those recoverable errors, but I doubt its usefulness.
|
Ah ok if we don't expect this to be common then a warning seems fine and we can adjust later if necessary. |
Recover from IO errors wile walking directories, only abort when error from `walkdir` is without an path to recover.
Remove `build_script::build_script_scan_eacces` test case because cargo ignores it and returns its path during a `cargo build`. The caller still has a chance to hit the IO error if they does access it.
a457dee to
d92dcea
Compare
|
@bors: r+ Ok sounds good to me! |
|
📌 Commit d92dcea has been approved by |
|
☀️ Test successful - checks-actions |
|
Should this fix be included in next beta? I feel like it should or we need to backport it after 1.59-beta released. |
|
Yeah that seems reasonable to me, would you be up for preparing the PR? |
…alexcrichton Be resilient to most IO error and filesystem loop while walking dirs Let `PathSource::walk` be resilient to most IO errors and filesystem loop. This PR also - Add a test validating the resilience against filesystem loop to prevent regression. - Emit warning when filesystem loop found while walking the filesystem. This is the only way I can think of now to solve rust-lang#9528 Fixes rust-lang#10213.
[Beta] backport "Be resilient to most IO error and filesystem loop while walking dirs" Beta backport of #10214.
Update cargo 6 commits in 358e79fe56fe374649275ca7aebaafd57ade0e8d..06b9d31743210b788b130c8a484c2838afa6fc27 2022-01-04 18:39:45 +0000 to 2022-01-11 23:47:29 +0000 - Port cargo to clap3 (rust-lang/cargo#10265) - feat: support rustflags per profile (rust-lang/cargo#10217) - Make bors ignore the PR template so it doesn't end up in merge messages (rust-lang/cargo#10267) - Be resilient to most IO error and filesystem loop while walking dirs (rust-lang/cargo#10214) - Remove the option to disable pipelining (rust-lang/cargo#10258) - Always ask rustc for messages about artifacts, and always process them (rust-lang/cargo#10255)
Update cargo 6 commits in 358e79fe56fe374649275ca7aebaafd57ade0e8d..06b9d31743210b788b130c8a484c2838afa6fc27 2022-01-04 18:39:45 +0000 to 2022-01-11 23:47:29 +0000 - Port cargo to clap3 (rust-lang/cargo#10265) - feat: support rustflags per profile (rust-lang/cargo#10217) - Make bors ignore the PR template so it doesn't end up in merge messages (rust-lang/cargo#10267) - Be resilient to most IO error and filesystem loop while walking dirs (rust-lang/cargo#10214) - Remove the option to disable pipelining (rust-lang/cargo#10258) - Always ask rustc for messages about artifacts, and always process them (rust-lang/cargo#10255)
Update cargo 6 commits in 358e79fe56fe374649275ca7aebaafd57ade0e8d..06b9d31743210b788b130c8a484c2838afa6fc27 2022-01-04 18:39:45 +0000 to 2022-01-11 23:47:29 +0000 - Port cargo to clap3 (rust-lang/cargo#10265) - feat: support rustflags per profile (rust-lang/cargo#10217) - Make bors ignore the PR template so it doesn't end up in merge messages (rust-lang/cargo#10267) - Be resilient to most IO error and filesystem loop while walking dirs (rust-lang/cargo#10214) - Remove the option to disable pipelining (rust-lang/cargo#10258) - Always ask rustc for messages about artifacts, and always process them (rust-lang/cargo#10255)
Update cargo 16 commits in 358e79fe56fe374649275ca7aebaafd57ade0e8d..95bb3c92bf516017e812e7f1c14c2dea3845b30e 2022-01-04 18:39:45 +0000 to 2022-01-18 17:39:35 +0000 - Error when setting crate type of both dylib and cdylib in library (rust-lang/cargo#10243) - Include `help` in `--list` (rust-lang/cargo#10300) - Add report subcommand to bash completion. (rust-lang/cargo#10295) - Downgrade some log messages. (rust-lang/cargo#10296) - Enable shortcut for triage bot (rust-lang/cargo#10298) - Bump to 0.61.0, update changelog (rust-lang/cargo#10294) - use new cargo fmt option (rust-lang/cargo#10291) - Add `run-fail` to semver-check for docs (rust-lang/cargo#10287) - Use `is_symlink()` method (rust-lang/cargo#10290) - Stabilize namespaced and weak dependency features. (rust-lang/cargo#10269) - Port cargo to clap3 (rust-lang/cargo#10265) - feat: support rustflags per profile (rust-lang/cargo#10217) - Make bors ignore the PR template so it doesn't end up in merge messages (rust-lang/cargo#10267) - Be resilient to most IO error and filesystem loop while walking dirs (rust-lang/cargo#10214) - Remove the option to disable pipelining (rust-lang/cargo#10258) - Always ask rustc for messages about artifacts, and always process them (rust-lang/cargo#10255)
Let
PathSource::walkbe resilient to most IO errors and filesystem loop.This PR also
Fixes #10213.