-
Notifications
You must be signed in to change notification settings - Fork 353
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
Bring back read_dir and remove_dir_all support #1966
Comments
rustup: disable read_dir test for now I don't currently have time to fix our read_dir support, so I disabled the tests for now. #1966 tracks bringing back that functionality.
Partial fix for rust-lang#1966.
Partial fix for rust-lang#1966.
Partial fix for rust-lang#1966.
Partial fix for rust-lang#1966.
Partial fix for rust-lang#1966.
Partial fix for rust-lang#1966.
Partial fix for rust-lang#1966.
Implement a readdir64() shim for Linux Partial fix for #1966.
read_dir works again. :) Thanks to @tavianator! However, remove_dir_all still fails. Test case diff: diff --git a/tests/run-pass/fs.rs b/tests/run-pass/fs.rs
index 7f5553e2..67817e3e 100644
--- a/tests/run-pass/fs.rs
+++ b/tests/run-pass/fs.rs
@@ -395,6 +395,12 @@ fn test_directory() {
remove_dir(&dir_path).unwrap();
// Reading the metadata of a non-existent directory should fail with a "not found" error.
assert_eq!(ErrorKind::NotFound, check_metadata(&[], &dir_path).unwrap_err().kind());
+
+ // To test remove_dir_all, re-create the directory with a file and a directory in it.
+ create_dir(&dir_path).unwrap();
+ drop(File::create(&path_1).unwrap());
+ create_dir(&path_2).unwrap();
+ remove_dir_all(&dir_path).unwrap();
}
fn test_dup_stdout_stderr() { Miri error:
|
Is it even possible to implement this in a cross-platform way? std does not expose a way to call So we could only implement this on Linux (and possibly macOS) hosts. Or we could patch libstd to use the fallback implementation -- that's what already happens on macOS targets (since dynamically resolving |
std should expose an Adding the fallback might be okay, but I worry that it might negate the race protection that was the point of all those changes. (I'm not sure about this, I'd have to study the implementation in depth.) Then again the race probably doesn't matter in miri, and |
The fallback would be gated under |
You could use cap-std I think for |
Oh, nice! I am inclined to go with |
cap-std doesn't support things like |
You can do |
Right, it's the |
…iper remove_dir_all: use fallback implementation on Miri Fixes rust-lang/miri#1966 The new implementation requires `openat`, `unlinkat`, and `fdopendir`. These cannot easily be shimmed in Miri since libstd does not expose APIs corresponding to them. So for now it is probably easiest to just use the fallback code in Miri. Nobody should run Miri as root anyway...
test remove_dir_all Blocked on rust-lang/rust#94749 Cc #1966
The tests for read_dir were temporarily disabled since the implementation was switched to
readdir64
, which Miri does not implement. Implementing it will be a bit of work due to having to manage the memory where the dirent is stored.Also, we currently do not have a test that runs
remove_dir_all
on an existing directory; we should add such a test -- this used to work but now also requiresopenat
.The text was updated successfully, but these errors were encountered: