Add Dir equivalents of fs::metadata & fs::symlink_metadata - #163024
Conversation
|
Any special-casing of Miri in the standard library requires review. cc @rust-lang/miri |
|
rustbot has assigned @Mark-Simulacrum. Use Why was this reviewer chosen?The reviewer was selected based on:
|
| fn metadata_at_native(&self, path: &[u16], reparse: ReparsePoint) -> io::Result<FileAttr> { | ||
| let mut opts = OpenOptions::new(); | ||
| // No read or write permissions are necessary | ||
| opts.access_mode(0); | ||
| opts.custom_flags(c::FILE_FLAG_BACKUP_SEMANTICS | reparse.as_flag()); | ||
|
|
||
| // Attempt to open the file normally. | ||
| // FIXME: `fs::metadata` has a fallback path when that fails. That has not (yet) been ported | ||
| // to directory handles. | ||
| let name = UnicodeStrRef::from_slice(path); | ||
| let object_attributes = c::OBJECT_ATTRIBUTES { | ||
| RootDirectory: self.handle.as_raw_handle(), | ||
| ObjectName: name.as_ptr().cast_mut(), | ||
| ..c::OBJECT_ATTRIBUTES::with_length() | ||
| }; | ||
| let create_opt = 0; // We don't want to create anything, only open existing things. | ||
| let handle = unsafe { nt_create_file(&opts, &object_attributes, create_opt)? }; | ||
| File { handle }.file_attr() | ||
| } |
There was a problem hiding this comment.
@ChrisDenton I hope this makes sense, I am mostly guessing here.^^ Especially about passing 0 for create_opt; the only other caller passes if dir { c::FILE_DIRECTORY_FILE } else { c::FILE_NON_DIRECTORY_FILE } but we cannot know in advance whether this is a directory or a file...
I also didn't copy the complicated fallback stuff from fs::metadata as I had no idea what that would look like with directory handles. I hope this first step is still useful and we can always add more fallback code later?
There was a problem hiding this comment.
I left some notes but essentially NT APIs are a bit different to the higher level win32 APIs. I'm ok with a partial implementation since I think I'm likely going to be rewriting (or at least refactoring) a lot of this anyway.
This comment was marked as resolved.
This comment was marked as resolved.
This comment has been minimized.
This comment has been minimized.
Add `Dir::metadata_at` & `Dir::symlink_metadata_at` try-job: test*msvc*
63a2987 to
6edd866
Compare
This comment has been minimized.
This comment has been minimized.
23f96b7 to
473f0c0
Compare
This comment has been minimized.
This comment has been minimized.
|
Hm... Does this not support This isn't even in my new method, the failure is from dir.open_file_with("subdir/bar.txt", &OpenOptions::new().create(true).write(true))I should file an issue... EDIT: #163032 |
473f0c0 to
cb648f2
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment has been minimized.
This comment has been minimized.
Add `Dir::metadata_at` & `Dir::symlink_metadata_at` try-job: test-*-msvc-1
| /// } | ||
| /// ``` | ||
| #[unstable(feature = "dirfd", issue = "120426")] | ||
| pub fn metadata_at<P: AsRef<Path>>(&self, path: P) -> io::Result<Metadata> { |
There was a problem hiding this comment.
The name does match the operation in fs that it corresponds to -- but Dir::metadata is already taken. So I went for a new name, inspired by the Linux naming scheme with the *at calls working on directory handles.
Alternatives that have been suggested or that I can think of:
- Use just
metadatafor this, and ask people to writedir.metadata(".")to get the metadata of the directory itself. But that seems silly in terms of the extra syscalls it causes. - Use just
metadatafor this, and useself_metadata/metadata_selfor so for the metadata of the directory handle itself.
There was a problem hiding this comment.
I'll be honest, I don't love the name and it would be especially weird if this would be our only use of the _at suffix (imho).
Another alternative I can think of is just making people open the fd and call fstat but I think that would be more controversial.
There was a problem hiding this comment.
Fair. I got rid of the _at suffix and renamed the old method to self_metadata.
This comment has been minimized.
This comment has been minimized.
This comment was marked as outdated.
This comment was marked as outdated.
cb648f2 to
eca304d
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
This comment has been minimized.
This comment has been minimized.
Add `Dir::metadata_at` & `Dir::symlink_metadata_at` try-job: test-*-msvc-1
f78d296 to
cb556bf
Compare
cb556bf to
5b172d9
Compare
|
@bors try jobs=various,android |
|
❌ Encountered an error while executing command |
|
@bors try jobs=various,android |
|
❌ Encountered an error while executing command |
|
@bors try jobs=various,android |
|
❌ Encountered an error while executing command |
|
@bors try cancel |
|
❌ Encountered an error while executing command |
|
@bors r- |
|
❌ Encountered an error while executing command |
|
@bors try jobs=various,android |
Add `Dir` equivalents of `fs::metadata` & `fs::symlink_metadata` try-job: *various* try-job: *android*
This comment has been minimized.
This comment has been minimized.
|
@bors r=ChrisDenton |
…enton Add `Dir` equivalents of `fs::metadata` & `fs::symlink_metadata` This adds methods to `Dir` that allow querying the metadata of files/directories relative to a `Dir`. Miri would [really like](rust-lang/miri#5327) to be able to do this so I figured I'd give it a shot. :) The first commit refactors the Unix `DirEntry` methods a bit with `cfg_select` to avoid repeating the `cfg` condition. Tracking issue: rust-lang#120426 try-jobs: test-x86_64-msvc-1
…uwer Rollup of 4 pull requests Successful merges: - #163024 (Add `Dir` equivalents of `fs::metadata` & `fs::symlink_metadata`) - #162839 (Bump min Emscripten version to 4.0, drop deprecated -sWASM_BIGINT for wasm32-unknown-emscripten) - #163370 (Fix incorrect typo suggestion for `struct field` shorthands) - #163471 (do not suggest capturing `'_` twice in `use<...>` for E0700)
Rollup merge of #163024 - RalfJung:dir-metadata-at, r=ChrisDenton Add `Dir` equivalents of `fs::metadata` & `fs::symlink_metadata` This adds methods to `Dir` that allow querying the metadata of files/directories relative to a `Dir`. Miri would [really like](rust-lang/miri#5327) to be able to do this so I figured I'd give it a shot. :) The first commit refactors the Unix `DirEntry` methods a bit with `cfg_select` to avoid repeating the `cfg` condition. Tracking issue: #120426 try-jobs: test-x86_64-msvc-1
…uwer Rollup of 4 pull requests Successful merges: - rust-lang/rust#163024 (Add `Dir` equivalents of `fs::metadata` & `fs::symlink_metadata`) - rust-lang/rust#162839 (Bump min Emscripten version to 4.0, drop deprecated -sWASM_BIGINT for wasm32-unknown-emscripten) - rust-lang/rust#163370 (Fix incorrect typo suggestion for `struct field` shorthands) - rust-lang/rust#163471 (do not suggest capturing `'_` twice in `use<...>` for E0700)
View all comments
This adds methods to
Dirthat allow querying the metadata of files/directories relative to aDir. Miri would really like to be able to do this so I figured I'd give it a shot. :)The first commit refactors the Unix
DirEntrymethods a bit withcfg_selectto avoid repeating thecfgcondition.Tracking issue: #120426
try-jobs: test-x86_64-msvc-1