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

guarantee that remove_dir will only return ErrorKind::NotFound if the file does not exist #433

Closed
lolbinarycat opened this issue Aug 24, 2024 · 1 comment
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@lolbinarycat
Copy link

Proposal

Problem statement

with the new error handling logic introduced in rust-lang/rust#127623, the implementation in library/std/src/sys_common/fs.rs relies on this guarantee for correctness, but it is not actually provided. this could potentially allow an implementation for a new platform to return NotFound for other classes of errors and slip past review. guaranteeing this would also mean that any instances of this guarantee being broken would be a definite bug, and not just a platform-specific quirk.

Solution sketch

under the Errors section, add a line like "this function will only return an error of the kind NotFound if the specified file does not exist."

it should also be noted that the inverse is not true. if the file does not exist, remove_file may return other errors, such if the containing directory is not readable.

Alternatives

add this as an internal guarantee, but not one that is part of the user-facing documentation. this allows it to be changed in the future, if that somehow becomes desirable (perhaps if a platform makes file non-existance impossible to efficiently differentiate from some other type of error, although that seems unlikely)

What happens now?

This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

  • We think this problem seems worth solving, and the standard library might be the right place to solve it.
  • We think that this probably doesn't belong in the standard library.

Second, if there's a concrete solution:

  • We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
  • We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.
@lolbinarycat lolbinarycat added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Aug 24, 2024
@Amanieu Amanieu added the ACP-accepted API Change Proposal is accepted (seconded with no objections) label Aug 27, 2024
@Amanieu
Copy link
Member

Amanieu commented Aug 27, 2024

In the libs-api meeting we couldn't think of any reason why we would return NotFound for any reason other than the directory not existing, so this is fine to add. The text is not clear whether this is for remove_file or remove_dir, but it seems fine to guarantee this for both. If a containing directory doesn't exist then it necessarily implies that the target file/directory doesn't exist.

@Amanieu Amanieu closed this as completed Aug 27, 2024
lolbinarycat added a commit to lolbinarycat/rust that referenced this issue Aug 29, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Sep 21, 2024
…ratrieb

add guarantee about remove_dir and remove_file error kinds

approved in ACP rust-lang/libs-team#433
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Sep 21, 2024
Rollup merge of rust-lang#129718 - lolbinarycat:remove_dir-docs, r=Noratrieb

add guarantee about remove_dir and remove_file error kinds

approved in ACP rust-lang/libs-team#433
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api
Projects
None yet
Development

No branches or pull requests

2 participants