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.

Sign up for GitHub

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

Jump to bottom

remove_dir_all: try deleting the directory even if FILE_LIST_DIRECTORY access is denied #110918

Merged
merged 1 commit into from
Apr 28, 2023
Merged

remove_dir_all: try deleting the directory even if FILE_LIST_DIRECTORY access is denied #110918

merged 1 commit into from
Apr 28, 2023

Conversation

ChrisDenton
Copy link
Member

@ChrisDenton ChrisDenton commented Apr 28, 2023 •
edited
Loading

If opening a directory with FILE_LIST_DIRECTORY access fails then we should try opening without requesting that access. We may still be able to delete it if it's empty or a link.

Fixes rust-lang/cargo#12042

@ChrisDenton ChrisDenton added the O-windows Operating system: Windows label Apr 28, 2023
@rustbot rustbot added 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 Apr 28, 2023
@rustbot
Copy link
Collaborator

rustbot commented Apr 28, 2023

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

@ChrisDenton
Copy link
Member Author

Hm, the diff is quite a bit larger than the actual changes as it affects some indentation. The important bit is that it allows failing to opening the directory with traverse access and falls back to treating like any other file.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@ChrisDenton
remove_dir_all: delete directory with fewer perms
ddff7f0 
If opening a directory with `FILE_LIST_DIRECTORY` access fails then we should try opening without requesting that access. We may still be able to delete it if it's empty or a link.
@ChrisDenton ChrisDenton force-pushed the on-error-resume-next branch from b3766d4 to ddff7f0 Compare April 28, 2023 01:31
@ChrisDenton ChrisDenton changed the title remove_dir_all: delete directory with fewer perms remove_dir_all: try deleting the directory even if FILE_LIST_DIRECTORY access is denied Apr 28, 2023
@ehuss ehuss mentioned this pull request Apr 28, 2023
Closed
@cuviper
Copy link
Member

cuviper commented Apr 28, 2023

Looks simple enough to me!

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 28, 2023

📌 Commit ddff7f0 has been approved by cuviper

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 Apr 28, 2023
@matthiaskrgr matthiaskrgr mentioned this pull request Apr 28, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 28, 2023
@bors
Auto merge of rust-lang#110924 - matthiaskrgr:rollup-jvznpq2, r=matth…
2fce229 
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#110766 (More core::fmt::rt cleanup.)
 - rust-lang#110873 (Migrate trivially translatable `rustc_parse` diagnostics)
 - rust-lang#110904 (rustdoc: rebind bound vars to type-outlives predicates)
 - rust-lang#110913 (Add some missing built-in lints)
 - rust-lang#110918 (`remove_dir_all`: try deleting the directory even if `FILE_LIST_DIRECTORY` access is denied)
 - rust-lang#110920 (Fix unavailable url)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 6476b79 into rust-lang:master Apr 28, 2023
@rustbot rustbot added this to the 1.71.0 milestone Apr 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@cuviper cuviper

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
Milestone
1.71.0
Development

Successfully merging this pull request may close these issues.

cargo test on cargo itself fails spuriously on Windows
5 participants
@ChrisDenton @rustbot @rust-log-analyzer @cuviper @bors