Windows: remove readonly files #134679
Windows: remove readonly files #134679
Conversation
|
rustbot has assigned @Mark-Simulacrum. Use |
rustbot
added
O-windows
|
Could you add that to the platform-specific behavior section of |
|
I've added a note about which functions are used. |
| let mut opts = OpenOptions::new(); | ||
| opts.access_mode(c::DELETE); | ||
| opts.custom_flags(c::FILE_FLAG_OPEN_REPARSE_POINT); | ||
| if File::open_native(&p_u16s, &opts).map(|f| f.posix_delete()).is_ok() { |
There was a problem hiding this comment.
Could this cause concurrent deletes of the same file to fail due to having an open handle (iirc windows doesn't let you delete files with open handles?)? I guess before it would also fail with the same error, so not sure that matters even if true...
There was a problem hiding this comment.
DeleteFile does also open the file in a similar way to we're doing so there's actually no change there.
iirc windows doesn't let you delete files with open handles
That's not quite right. When opening files there's a "sharing mode" that lets you allow others to open the same file. One of those modes is FILE_SHARE_DELETE. If someone opens a file without that share mode then it will prevent any deletions no matter what we do.
Deletes can also race with each other because there's a time between marking a file for deletion and closing the handle where the file is in a kind of limbo state. It can't be opened but it still exists on the filesystem.
But either case is the same whether we use DeleteFile or open the file ourselves.
There was a problem hiding this comment.
I see, sounds good. Thanks for clarifying! :)
I'll add a label to see if libs-api want to discuss it. This aligns the behaviour with other platforms so I think it's worthwhile. |
ChrisDenton
added
T-libs-api
|
Oh, I should write a summary of the question being asked of libs-api @rust-lang/libs-api on platforms except Windows, read-only files are deleted when calling This PR changes Does this new (for Windows) behaviour require a libs-api FCP? |
|
@rustbot ping windows |
|
Hey Windows Group! This bug has been identified as a good "Windows candidate". cc @albertlarsan68 @arlosi @ChrisDenton @danielframpton @gdr-at-ms @kennykerr @luqmana @lzybkr @nico-abram @retep998 @sivadeilra @wesleywiser |
|
@rfcbot fcp merge |
|
Team member @dtolnay has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
rfcbot
added
proposed-final-comment-period
When calling
remove_file, we shouldn't fail to delete readonly files. As the test makes clear, this make the Windows behaviour consistent with other platforms. This also makes us internally consistent withremove_dir_all.