Skip to content
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

Win: Open dir for sync access in remove_dir_all #129934

Merged
merged 1 commit into from
Sep 19, 2024

Conversation

ChrisDenton
Copy link
Member

@ChrisDenton ChrisDenton commented Sep 3, 2024

A small follow up to #129800.

We should explicitly open directories for synchronous access. We ultimately use GetFileInformationByHandleEx to read directories which should paper over any issues caused by using async directory reads (or else return an error) but it's better to do the right thing in the first place. Note though that delete does not read or write any data so it's not necessary there.

@rustbot rustbot added O-windows Operating system: Windows S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 3, 2024
@Noratrieb
Copy link
Member

r? Amanieu since you approved the previous PR

@rustbot rustbot assigned Amanieu and unassigned Noratrieb Sep 15, 2024
@Amanieu
Copy link
Member

Amanieu commented Sep 18, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Sep 18, 2024

📌 Commit 6b0fc97 has been approved by Amanieu

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 18, 2024
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Sep 18, 2024
…nieu

Win: Open dir for sync access in remove_dir_all

A small follow up to rust-lang#129800.

We should explicitly open directories for synchronous access. We ultimately use `GetFileInformationByHandleEx` to read directories which should paper over any issues caused by using async directory reads (or else return an error) but it's better to do the right thing in the first place. Note though that `delete` does not read or write any data so it's not necessary there.
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 18, 2024
…kingjubilee

Rollup of 9 pull requests

Successful merges:

 - rust-lang#97524 (Add `Thread::{into_raw, from_raw}`)
 - rust-lang#127988 (Do not ICE with incorrect empty suggestion)
 - rust-lang#129422 (Gate `repr(Rust)` correctly on non-ADT items)
 - rust-lang#129934 (Win: Open dir for sync access in remove_dir_all)
 - rust-lang#130450 (Reduce confusion about `make_indirect_byval` by renaming it)
 - rust-lang#130476 (Implement ACP 429: add `Lazy{Cell,Lock}::get[_mut]` and `force_mut`)
 - rust-lang#130487 (Update the minimum external LLVM to 18)
 - rust-lang#130513 (Clarify docs for std::fs::File::write)
 - rust-lang#130522 ([Clippy] Swap `manual_retain` to use diagnostic items instead of paths)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 591ec6c into rust-lang:master Sep 19, 2024
6 checks passed
@rustbot rustbot added this to the 1.83.0 milestone Sep 19, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Sep 19, 2024
Rollup merge of rust-lang#129934 - ChrisDenton:remove-dir-all3, r=Amanieu

Win: Open dir for sync access in remove_dir_all

A small follow up to rust-lang#129800.

We should explicitly open directories for synchronous access. We ultimately use `GetFileInformationByHandleEx` to read directories which should paper over any issues caused by using async directory reads (or else return an error) but it's better to do the right thing in the first place. Note though that `delete` does not read or write any data so it's not necessary there.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-windows Operating system: Windows S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants