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

Possible errors when accessing file metadata are platform specific #85760

Merged
merged 1 commit into from
Jun 5, 2021

Conversation

ChrisDenton
Copy link
Member

In particular the is_dir, is_file and exists functions suggests that querying a file requires querying the directory. On Windows this is not normally true.

r? @m-ou-se

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 27, 2021
library/std/src/path.rs Outdated Show resolved Hide resolved
@m-ou-se
Copy link
Member

m-ou-se commented May 27, 2021

Are you saying that on windows you can always query the metadata of any file, regardless of the permissions you have to any of the directories it is in? That sounds wrong.

@ChrisDenton
Copy link
Member Author

Yes that's what I'm saying. On Windows permissions are based on the file only. There is a setting to do traverse-checking but doing so would likely break things. Both the OS and Windows software would not expect this behaviour.

@ChrisDenton
Copy link
Member Author

ChrisDenton commented May 27, 2021

I'm trying to find a good explanatory article but in most of what I can find it's taken as a given. Perhaps this (scroll to "Traverse Folder/Execute File"): https://www.techrepublic.com/article/windows-101-know-the-basics-about-ntfs-permissions/

@m-ou-se
Copy link
Member

m-ou-se commented May 27, 2021

The "access the file" wording is also not accurate, since you can read the metadata of a file even without having any (read/write) access to it.

Maybe instead of adding platform-specific notes here, it could just say:

    /// If you cannot access the metadata of the file, e.g. because of a
    /// permission error or broken symbolic links, this will return `false`.

@ChrisDenton
Copy link
Member Author

I was slightly concerned about removing an explicitly documented situation in case it was documented for a good reason. But looking back at the commit that added it it seems the intent was only to document what happens on error without being too specific about implementation details.

So I agree that the simpler wording is much clearer. I'll push the revised wording.

In particular the `is_dir`, `is_file` and `exists` functions says that querying a file requires querying the directory. On Windows this is not normally true.
@ChrisDenton ChrisDenton force-pushed the path-doc-platform-specific branch from 98f9adb to 536d982 Compare May 27, 2021 21:07
@m-ou-se
Copy link
Member

m-ou-se commented Jun 5, 2021

Thanks!

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jun 5, 2021

📌 Commit 536d982 has been approved by m-ou-se

@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 Jun 5, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 5, 2021
…laumeGomez

Rollup of 7 pull requests

Successful merges:

 - rust-lang#83646 (Add a map method to Bound)
 - rust-lang#85501 (Fix `deny(invalid_doc_attributes)`)
 - rust-lang#85503 (rustdoc: add tooltips to some buttons)
 - rust-lang#85710 (Document `From` impls in path.rs)
 - rust-lang#85760 (Possible errors when accessing file metadata are platform specific)
 - rust-lang#85974 (td align attribute)
 - rust-lang#86014 (msp430 linker does not accept -znoexecstack. Set linker_is_gnu to fal…)

Failed merges:

 - rust-lang#85972 (Rustdoc html fixes)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 114aff5 into rust-lang:master Jun 5, 2021
@rustbot rustbot added this to the 1.54.0 milestone Jun 5, 2021
@ChrisDenton ChrisDenton deleted the path-doc-platform-specific branch June 6, 2021 04:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants