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

Added another error to be processed in fallback #107985

Merged
merged 1 commit into from
Feb 14, 2023
Merged

Added another error to be processed in fallback #107985

merged 1 commit into from
Feb 14, 2023

Conversation

alesito85
Copy link
Contributor

@alesito85 alesito85 commented Feb 13, 2023 •
edited
Loading

This pull request addresses the problem of Rust not being able to read file/directory metadata because the current user doesn't have permission to read the file and are thus inaccessible.

One particular example is System Volume Information. But any example can be made by having a file/directory, which the current user can't access even though the system does allow to view the metadata, which is handled by the fallback.

The fallback exists to get the metadata but it was limited to one error type. Having added ERROR_ACCESS_DENIED per Chris Denton's suggestion, file/directory properties are now properly read.

Solution suggested by Chris Denton nushell/nushell#6857 (comment)

@rustbot
Copy link
Collaborator

rustbot commented Feb 13, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @thomcc (or someone else) soon.

Please see the contribution instructions for more information.

@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 Feb 13, 2023
@rustbot
Copy link
Collaborator

rustbot commented Feb 13, 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

@alesito85
Copy link
Contributor Author

@ChrisDenton hey I've made this PR per your suggestion, works like a charm!

Let me know if anything else should be done (or done better).

ChrisDenton
ChrisDenton approved these changes Feb 13, 2023
View reviewed changes
Copy link
Member

@ChrisDenton ChrisDenton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Looks good to me. Could you update the comment below to briefly mention ERROR_ACCESS_DENIED is returned by files/directories that exist but are inaccessible (and maybe mention "System Volume Information" as an example)?

@alesito85
Copy link
Contributor Author

I've expanded the comment, thanks for your input.

@alesito85 alesito85 mentioned this pull request Feb 13, 2023
Open
@ChrisDenton ChrisDenton assigned ChrisDenton and unassigned thomcc Feb 13, 2023
@ChrisDenton
Copy link
Member

@alesito85 did you push your changes? I'm not yet seeing them.

@alesito85
Copy link
Contributor Author

@ChrisDenton the changes are in now.

@ChrisDenton
Copy link
Member

Thanks, comment looks good. The only thing I'd ask is to squish the two commits into one, seeing as they're both small.

@alesito85
Add another error to Windows file open fallback
f72eb47 
Added another error to be processed in fallback

Solution suggested by Chris Denton nushell/nushell#6857 (comment)
@alesito85
Copy link
Contributor Author

@ChrisDenton done.

@ChrisDenton
Copy link
Member

Thanks for contributing to Rust! Let's try to get this merged.

@bors r+ rollup

If all the tests go well then this should be in nightly soon and released in Rust 1.69.

@bors
Copy link
Contributor

bors commented Feb 13, 2023

📌 Commit f72eb47 has been approved by ChrisDenton

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 Feb 13, 2023
@matthiaskrgr matthiaskrgr mentioned this pull request Feb 13, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 13, 2023
@bors
Auto merge of rust-lang#108015 - matthiaskrgr:rollup-qerohjn, r=matth…
5348a89 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#107902 (fix: improve the suggestion on future not awaited)
 - rust-lang#107913 (Update broken link in cargo style guide)
 - rust-lang#107942 (Tighter spans for bad inherent `impl` self types)
 - rust-lang#107948 (Allow shortcuts to directories to be used for ./x.py fmt)
 - rust-lang#107971 (Clearly document intentional UB in mir-opt tests)
 - rust-lang#107985 (Added another error to be processed in fallback)
 - rust-lang#108002 (Update books)
 - rust-lang#108013 (rustdoc: use a string with one-character codes for search index types)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 56193b0 into rust-lang:master Feb 14, 2023
@rustbot rustbot added this to the 1.69.0 milestone Feb 14, 2023
@roblabla roblabla mentioned this pull request May 31, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChrisDenton ChrisDenton ChrisDenton approved these changes

Assignees

@ChrisDenton ChrisDenton

Labels
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.69.0
Development

Successfully merging this pull request may close these issues.
5 participants
@alesito85 @rustbot @ChrisDenton @bors @thomcc