Stabilize io_error_more

[albertlarsan68]

FCP Completed in #86442 (comment)

Closes #86442

[rustbot]

r? @joshtriplett

(rustbot has picked a reviewer for you, use r? to override)

[rustbot]

The Miri subtree was changed

cc @rust-lang/miri

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

[albertlarsan68]

@rustbot label +T-libs-api -T-libs

[ijackson]

@albertlarsan68 thanks :-).

Reading #86442 (comment) there is a possibility that we want to change the categorisation of the Windows error ERROR_CANT_RESOLVE_FILENAME from Uncategorized to FilesystemLoop.

I don't think that needs to be done in this MR, and I don't think it needs to block the stabilisation. I think it is probably best to leave this to Windows experts.

(FTAOD I don't agree with the suggestion there to rename the error to LoopError, for both substantive and procedural reasons.)

[kalcutter]

I think FilesystemQuotaExceeded should be renamed QuotaExceeded (see #86442 (comment)).

Would you also consider NotSameDevice instead of CrossesDevices?

[albertlarsan68]

I would leave this decision to a T-libs-api member, there did not seem to be a consensus.

[joshtriplett]

With my libs-api hat on: 👍 for renaming FilesystemQuotaExceeded to QuotaExceeded, as OSes may use it for other kinds of quotas.

I have a mild preference for renaming FilesystemLoop to something that doesn't include Filesystem, for the same reason: OSes do use it for other errors. For instance, Linux also uses it for keyrings, BPF, network routing/filtering, vhost, and network bridges.

I personally don't think we should rename CrossesDevices.

[albertlarsan68]

Is LoopDetected good enough for FilesystemLoop?

[ijackson]

Process

This seems highly irregular from a governance perspective.

We have completed the FCP on the names as they have been in unstable for a year and a half now. Proposals to rename them were made in the FCP but not accepted and instead the FCP was approved with a disposition to "merge".

Changing the names and therefore implied semantics after 18 months in unstable, and after the FCP, seems quite wrong to me.

If there is an open question about some of these, we could stabilise a subset. Any proposal to change the names and meanings ought then to be dealt with in a separate MR.

Given the amount of extensive discussion and consideration these received in the tracking issue thread, any such change ought to be made with deliberation.

Substance

I disagree with renaming FilesystemLoop.

It is true that Unix has a tendency to reuse errno values, so that any particular errno value can often mean a variety of things. Particularly, less-common (even, obscure) APIs and facilities (ab)use errno values. Attempting to represent all these obscure possibilities leads to descriptions and categorisations that are vague and overlapping. We generally haven't done that and I don't think we should start now. (All of this was discussed at length in the earlier conversations in the tracking issue.)

The APIs available in std will produce this error for filesystem operations, not obscure other purposes. I think calling oit FilesystemLoop is sensible.

I am not quite so sure about FilesystemQuotaExceeded but again I think APIs in std won't produce this error other than for filesystem operations.

[albertlarsan68]

I am thus dropping the rename of FilesystemLoop, but since the change of FilesystemQuotaExceeded was approved by a T-libs-api member, I propose to keep it, unless I should stabilize only a subset, which would be quite sad.

PS: the commit that does the loop rename is 913ac3ef23d8e704ffb02c0c68bb31909cf1968f

[kalcutter]

There is also the winsock error WSAEDQUOT. Shouldn't that be mapped here https://github.com/rust-lang/rust/blob/master/library/std/src/sys/windows/mod.rs#L112 to QuotaExceeded?

[ijackson]

There is also the winsock error WSAEDQUOT. Shouldn't that be mapped here https://github.com/rust-lang/rust/blob/master/library/std/src/sys/windows/mod.rs#L112 to QuotaExceeded?

Yes, I think so. I'm not sure why we overlooked that. Generally I would expect that a WSAE error should be mapped the same way as the correspondingly named E error on Unix. I think I tired to do that. Are there other omissions?

[albertlarsan68]

I think moving errors from Uncategorized to a specific category is outside the scope for this PR and FCP, but feel free to do so in another PR. This will not be a breaking change, since the current Uncategorized errors are not matchable on stable.

[albertlarsan68]

Ping to @joshtriplett, what is the state ?

[vi]

Maybe uncontroversial variants can be stabilised first and other variants (such as QuotaExceeded or FilesystemLoop) later?

[ChrisDenton]

There have been questions raised about InvalidFilename too.. See the tracking issue.

[jdahlstrom]

Seeing that this doesn't seem to be moving and there are other unresolved questions despite the FCP, I'll mention kornel's comment from Dec 30, 2023 that doesn't seem to have been acknowledged:

std uses file_name in functions, rather than filename. It'd be odd to break correspondence between snake_case and CamelCase, so I think this should be FileNameTooLong to match. Same goes for InvalidFileName.

[tertsdiepraam]

I would like to voice support stabilization of the uncontroversial variants. This would get those variants to stable and focus the discussion around the more controversial ones. I don't see any particular reason that all of these must be stabilized at the same time. Is there prior art to stabilizing a part of a feature?

In particular, in uutils, we would like to use IsADirectory, NotADirectory & DirectoryNotEmpty, which come up often with file system operations.

[joshtriplett]

Please do not consider me a blocker for anything here.

[bors]

☔ The latest upstream changes (presumably #117285) made this pull request unmergeable. Please resolve the merge conflicts.

[joshtriplett]

r? libs-api

[ARitz-Cracker]

Are there any remaining blockers on this? Because some of these ErrorKind extensions, including the network and directory related ones, are long overdue.

If this is being held up due to a single comment suggesting splitting some kind of "File name too long" variant of ErrorKind from InvalidFilename, then I'd like to voice a disagreement with that.

In the vast majority of cases, any reason why InvalidFilename would be returned in its current state is entirely system specific, and I'd argue that if your archive or bulk-downloading program encounters that error, it should examine the entire path and correct whatever's necessary. For example, blindly truncating won't help you in a situation where you're dealing with Window's 260 max path char length when you're not using full canonical paths. (iirc it was the cause of some source engine RCE vulnerability) This illustrates there are even system specific quirks as to how long a file name and path can be depending on how you format the path.

Since Rust has no builtin mechanism to answer the question "What are the limitations of the filesystem and OS I'm currently dealing with?" in a cross platform manner, then I don't think it's productive to have error codes which can encourage naive and potentially harmful error handling.

The introduction of InvalidFilename already tells us there's something wrong with our path, and that already gives us enough information for what corrective action we should take. We're already on the cold path anyway, so is it really that hard to check whether or not our path is canonical and all the segments are less than 255 chars long while also checking whether or not the path contains characters or reserved keywords that the OS doesn't like?

Edit: Apologies for writing 2 variants, I had multiple tabs open and I thought I lost the previous message instead of posting it.

[GrigorenkoPV]

Now that #128316 is merged, I guess this PR can be closed. Not to mention it has had merge conflicts for nearly 8 months.

The 4 variants that remain unstable first need some discussion, and when (and if ever) we get to stabilizing them, a new PR can be opened.