-
Notifications
You must be signed in to change notification settings - Fork 13k
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
[discussion] ErrorKind::FilesystemLoop
from io_error_more
#130188
Comments
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Given the previous discussion, I have a mild preference for renaming this to |
…LENAME, r=ChrisDenton Map `ERROR_CANT_RESOLVE_FILENAME` to `ErrorKind::FilesystemLoop` cc rust-lang#86442 As summarized in rust-lang#130188, there seems to be a consensus that this should be done.
…LENAME, r=ChrisDenton Map `ERROR_CANT_RESOLVE_FILENAME` to `ErrorKind::FilesystemLoop` cc rust-lang#86442 As summarized in rust-lang#130188, there seems to be a consensus that this should be done.
Rollup merge of rust-lang#130207 - GrigorenkoPV:ERROR_CANT_RESOLVE_FILENAME, r=ChrisDenton Map `ERROR_CANT_RESOLVE_FILENAME` to `ErrorKind::FilesystemLoop` cc rust-lang#86442 As summarized in rust-lang#130188, there seems to be a consensus that this should be done.
We discussed this in the libs-api meeting. For now we're considering not stabilizing this until we get a concrete use case for why a user would want to match on |
#86442 (comment) allegedly has a use case. I think it's this: #86442 (comment) "mocking APIs, testing error handling and various other things" |
Would something like |
From what I can tell, that guy's complaint was "it is possible to get this error from std, yet I cannot produce it myself", which would apply to any unstable ErrorKind and is not really a concrete use-case. |
I think this is a bad name. It's the "descriptions and categorisations that are vague and overlapping" that I argued against in #106375 (comment). As I wrote:
As I argue there, the fact that Unix (or indeed, some other operating system) sometimes uses an error code for a situation which is not covered by the central meaning, does not mean that the name should not reference the central meaning. Instead, exceptions like this ought to be dealt with in the documentation. I woiuld argue that the central meaning for There is also value in reusing terms from the conventional strerror of the corresponding errno value, since that means that when a Rust program encounters a error used in an unusual way, it will emit an error similar to that of a correspondingly careful C program. The fact that ELOOP is sometimes used for excessive depth, without necessarily verifying an actual loop, or in obscure situations for loops of other kinds, doesn't mean that we shouldn't include "loop", "filesystem" and "(sym)link" in the name. An attempt to capture all the possibilities in the name is misguided: the only description that will definitely remain true is "something went wrong somewhere", and even attempts to capture current usage go quickly into the weeds and produce bland, nearly-useless, names like "unresolvable something". (Note that "indirection" isn't always true; AIUI you can sometimes get PS thanks to @GrigorenkoPV and others for picking this up again after I got blocked and discouraged by the libs team member's process violation in 2022. I'm glad to see that most of the new kinds got merged and that these remaining four are also getting there. |
@rustbot label C-discussion
Main tracking issue: #86442
Background
The
io_error_more
feature introduced 21 new variants intoErrorKind
. They were FCP'd back in December 2022, but there appeared to be quite a lot of disagreement about 4 of the added variants, so the stabilization (#106375) got stalled for over twenty months. Thankfully, the 17 uncontroversial variants got stabilized in #128316, so now we just need to iron out a satisfactory design for the remaining 4 variants, and then they can be stabilized too.In order to not block any of the remaining variants on each other and to not intertwine the discussions, I've created 4 separate issues, which summarize the concerns & suggestions voiced up until this point and can serve as a place for further discussion.
FilesystemLoop
: you are hereFilesystemQuotaExceeded
: [discussion]ErrorKind::FilesystemQuotaExceeded
fromio_error_more
#130190CrossesDevices
: [discussion]ErrorKind::CrossesDevices
fromio_error_more
#130191InvalidFilename
: [discussion]ErrorKind::InvalidFilename
fromio_error_more
#130192FilesystemLoop
Currently corresponds to
ELOOP
on Unix andnothingERROR_CANT_RESOLVE_FILENAME
on Windows. (#86442 (comment), #130207)Current docs description:
Make it correspond toERROR_CANT_RESOLVE_FILENAME
on WindowsDone in #130207
Old description
Originally posted by Robert Collins in #86442 (comment)
In #86442 (comment) Ian Jackson voices a concern that this might not be the only place where
ERROR_CANT_RESOLVE_FILENAME
appears.Chris Denton in #86442 (comment) and Robert Collins in #86442 (comment) confirm that this is the only place where Windows currently gives
ERROR_CANT_RESOLVE_FILENAME
and that there is a good correspondence with Unix'sELOOP
(when it comes to symlikns, see below for the other usages ofELOOP
).Ian Jackson agrees with them in #106375 (comment), but proposes this should be done separately from stabilization.
There seems to be a consensus regarding this point.
Bikshed the name: be about loops in general, drop "filesystem" from the name
Unix's
ELOOP
is not just for symlink loops (or too long symlink chains).Originally posted by Robert Collins in #86442 (comment)
Originally posted by Josh Triplett in #106375 (comment)
Originally posted by Ian Jackson in #106375 (comment)
Bikeshed the name: be about symlink resolution failure in general, stop mentioning loops
Originally posted by Alain Emilia Anna Zscheile in #86442 (comment)
The text was updated successfully, but these errors were encountered: