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

Stabilize most of io_error_more #128316

Merged
merged 1 commit into from
Sep 10, 2024
Merged

Conversation

GrigorenkoPV
Copy link
Contributor

Sadly, venting my frustration with t-libs-api is not a constructive way to solve problems and get things done, so I will try to stick to stuff that actually matters here.

So, to highlight a common sentiment:

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

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. [...] 2

Maybe some less-controversial subset could be stabilized sooner? What’s blocking this issue from making progress? 3

So this is exactly what this PR does. It stabilizes the non-controversial variants now, leaving just a few of them behind.

Namely, this PR stabilizes:

  • HostUnreachable
  • NetworkUnreachable
  • NetworkDown
  • NotADirectory
  • IsADirectory
  • DirectoryNotEmpty
  • ReadOnlyFilesystem
  • StaleNetworkFileHandle
  • StorageFull
  • NotSeekable
  • FileTooLarge
  • ResourceBusy
  • ExecutableFileBusy
  • Deadlock
  • TooManyLinks
  • ArgumentListTooLong
  • Unsupported

This PR does not stabilize:

  • FilesystemLoop
  • FilesystemQuotaExceeded
  • CrossesDevices
  • InvalidFilename

Hopefully, this will allow us to move forward with this highly and long awaited addition to std, both allowing to still polish the less clear parts of it and not leading to stagnation.

r? joshtriplett
because they seem to be listed as a part of t-libs-api and were one of the most responsive persons previously

Footnotes

  1. https://github.com/rust-lang/rust/issues/106375#issuecomment-1435762236

  2. https://github.com/rust-lang/rust/pull/106375#issuecomment-1742661555

  3. https://github.com/rust-lang/rust/issues/86442#issuecomment-1691187483 (got 30 upvotes btw) (and no response)

@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 Jul 28, 2024
@GrigorenkoPV GrigorenkoPV changed the title Partially stabilize io_error_more Stabilize most of io_error_more Aug 1, 2024
@GrigorenkoPV
Copy link
Contributor Author

r? libs

@rustbot rustbot assigned joboet and unassigned joshtriplett Aug 12, 2024
@joboet
Copy link
Contributor

joboet commented Aug 12, 2024

As much as I'd like to help out here, I am not in the libs-api team and should not be the one reviewing this.

r? libs-api

@rustbot rustbot added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Aug 12, 2024
@rustbot rustbot assigned m-ou-se and unassigned joboet Aug 12, 2024
@GrigorenkoPV
Copy link
Contributor Author

r? libs-api

@rustbot rustbot assigned BurntSushi and unassigned m-ou-se Aug 26, 2024
@GrigorenkoPV
Copy link
Contributor Author

r? libs-api

@rustbot rustbot assigned joshtriplett and unassigned BurntSushi Sep 9, 2024
@slanterns
Copy link
Contributor

Maybe you can nominate it for T-libs-api discussion.

@slanterns

This comment was marked as resolved.

@rustbot rustbot added the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Sep 9, 2024
@GrigorenkoPV
Copy link
Contributor Author

Let's try

@rustbot label +I-libs-api-nominated

Nevermind, it was done while I was writing this. Thank you!


My apologies if I am beeing too pushy with this, it is just that I am worried this PR may suffer the same fate as the previous efforts and fall into indefinite stagnation.

As for re-rolling the reviewers, my reasoning was that if a reviewer is too busy to review this PR, someone else might be less occupied at the moment. Rustbot's default message promises "two weeks for the reviewer to take a look at your PR or pass it to someone else", so that is how long I have waited each time.

Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Thank you!

I confirmed that all of these variants are covered by the completed libs-api team FCP in #106375 (i.e. none of these are a recent addition) and that neither the tracking issue nor the previous stabilization PR #128316 contains any disagreement applicable to this subset of the variants.

@dtolnay
Copy link
Member

dtolnay commented Sep 9, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Sep 9, 2024

📌 Commit f693910 has been approved by dtolnay

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 Sep 9, 2024
@dtolnay dtolnay removed the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Sep 9, 2024
@dtolnay dtolnay assigned dtolnay and unassigned joshtriplett Sep 9, 2024
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Sep 10, 2024
…r=dtolnay

Stabilize most of `io_error_more`

Sadly, venting my frustration with t-libs-api is not a constructive way to solve problems and get things done, so I will try to stick to stuff that actually matters here.

- Tracking issue for this feature was opened 3 years ago: rust-lang#86442
- FCP to stabilize it was completed 19(!!) months ago: rust-lang#86442 (comment)
- A PR with stabilization was similarly open for 19 months: rust-lang#106375, but nothing ever came out of it. Presumably (it is hard to judge given the lack of communication) because a few of the variants still had some concerns voiced about them, even after the FCP.

So, to highlight a common sentiment:

> Maybe uncontroversial variants can be stabilised first and other variants (such as `QuotaExceeded` or `FilesystemLoop`) later? [^1]

[^1]: rust-lang#106375 (comment)

> 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. [...] [^2]

[^2]: rust-lang#106375 (comment)

> Maybe some less-controversial subset could be stabilized sooner? What’s blocking this issue from making progress? [^3]

[^3]: rust-lang#86442 (comment) (got 30 upvotes btw) (and no response)

So this is exactly what this PR does. It stabilizes the non-controversial variants now, leaving just a few of them behind.

Namely, this PR stabilizes:

- `HostUnreachable`
- `NetworkUnreachable`
- `NetworkDown`
- `NotADirectory`
- `IsADirectory`
- `DirectoryNotEmpty`
- `ReadOnlyFilesystem`
- `StaleNetworkFileHandle`
- `StorageFull`
- `NotSeekable`
- `FileTooLarge`
- `ResourceBusy`
- `ExecutableFileBusy`
- `Deadlock`
- `TooManyLinks`
- `ArgumentListTooLong`
- `Unsupported`

This PR does not stabilize:
- `FilesystemLoop`
- `FilesystemQuotaExceeded`
- `CrossesDevices`
- `InvalidFilename`

Hopefully, this will allow us to move forward with this highly and long awaited addition to std, both allowing to still polish the less clear parts of it and not leading to stagnation.

r? joshtriplett
because they seem to be listed as a part of t-libs-api and were one of the most responsive persons previously
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 10, 2024
…kingjubilee

Rollup of 14 pull requests

Successful merges:

 - rust-lang#128316 (Stabilize most of `io_error_more`)
 - rust-lang#129473 (use  `download-ci-llvm=true` in the default compiler config)
 - rust-lang#129529 (Add test to build crates used by r-a on stable)
 - rust-lang#129778 (interpret: make typed copies lossy wrt provenance and padding)
 - rust-lang#129981 (Remove `serialized_bitcode` from `LtoModuleCodegen`.)
 - rust-lang#130025 (Also emit `missing_docs` lint with `--test` to fulfil expectations)
 - rust-lang#130040 (unify `llvm-bitcode-linker`, `wasm-component-ld` and llvm-tools logics)
 - rust-lang#130094 (Inform the solver if evaluation is concurrent)
 - rust-lang#130132 ([illumos] enable SIGSEGV handler to detect stack overflows)
 - rust-lang#130146 (bootstrap `naked_asm!` for `compiler-builtins`)
 - rust-lang#130149 (Helper function for formatting with `LifetimeSuggestionPosition`)
 - rust-lang#130152 (adapt a test for llvm 20)
 - rust-lang#130162 (bump download-ci-llvm-stamp)
 - rust-lang#130164 (move some const fn out of the const_ptr_as_ref feature)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 10, 2024
…kingjubilee

Rollup of 11 pull requests

Successful merges:

 - rust-lang#128316 (Stabilize most of `io_error_more`)
 - rust-lang#129473 (use  `download-ci-llvm=true` in the default compiler config)
 - rust-lang#129529 (Add test to build crates used by r-a on stable)
 - rust-lang#129981 (Remove `serialized_bitcode` from `LtoModuleCodegen`.)
 - rust-lang#130094 (Inform the solver if evaluation is concurrent)
 - rust-lang#130132 ([illumos] enable SIGSEGV handler to detect stack overflows)
 - rust-lang#130146 (bootstrap `naked_asm!` for `compiler-builtins`)
 - rust-lang#130149 (Helper function for formatting with `LifetimeSuggestionPosition`)
 - rust-lang#130152 (adapt a test for llvm 20)
 - rust-lang#130162 (bump download-ci-llvm-stamp)
 - rust-lang#130164 (move some const fn out of the const_ptr_as_ref feature)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 10, 2024
…kingjubilee

Rollup of 11 pull requests

Successful merges:

 - rust-lang#128316 (Stabilize most of `io_error_more`)
 - rust-lang#129473 (use  `download-ci-llvm=true` in the default compiler config)
 - rust-lang#129529 (Add test to build crates used by r-a on stable)
 - rust-lang#129981 (Remove `serialized_bitcode` from `LtoModuleCodegen`.)
 - rust-lang#130094 (Inform the solver if evaluation is concurrent)
 - rust-lang#130132 ([illumos] enable SIGSEGV handler to detect stack overflows)
 - rust-lang#130146 (bootstrap `naked_asm!` for `compiler-builtins`)
 - rust-lang#130149 (Helper function for formatting with `LifetimeSuggestionPosition`)
 - rust-lang#130152 (adapt a test for llvm 20)
 - rust-lang#130162 (bump download-ci-llvm-stamp)
 - rust-lang#130164 (move some const fn out of the const_ptr_as_ref feature)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 1392965 into rust-lang:master Sep 10, 2024
6 checks passed
@rustbot rustbot added this to the 1.83.0 milestone Sep 10, 2024
@GrigorenkoPV GrigorenkoPV deleted the io_error_a_bit_more branch September 10, 2024 10:01
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Sep 10, 2024
Rollup merge of rust-lang#128316 - GrigorenkoPV:io_error_a_bit_more, r=dtolnay

Stabilize most of `io_error_more`

Sadly, venting my frustration with t-libs-api is not a constructive way to solve problems and get things done, so I will try to stick to stuff that actually matters here.

- Tracking issue for this feature was opened 3 years ago: rust-lang#86442
- FCP to stabilize it was completed 19(!!) months ago: rust-lang#86442 (comment)
- A PR with stabilization was similarly open for 19 months: rust-lang#106375, but nothing ever came out of it. Presumably (it is hard to judge given the lack of communication) because a few of the variants still had some concerns voiced about them, even after the FCP.

So, to highlight a common sentiment:

> Maybe uncontroversial variants can be stabilised first and other variants (such as `QuotaExceeded` or `FilesystemLoop`) later? [^1]

[^1]: rust-lang#106375 (comment)

> 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. [...] [^2]

[^2]: rust-lang#106375 (comment)

> Maybe some less-controversial subset could be stabilized sooner? What’s blocking this issue from making progress? [^3]

[^3]: rust-lang#86442 (comment) (got 30 upvotes btw) (and no response)

So this is exactly what this PR does. It stabilizes the non-controversial variants now, leaving just a few of them behind.

Namely, this PR stabilizes:

- `HostUnreachable`
- `NetworkUnreachable`
- `NetworkDown`
- `NotADirectory`
- `IsADirectory`
- `DirectoryNotEmpty`
- `ReadOnlyFilesystem`
- `StaleNetworkFileHandle`
- `StorageFull`
- `NotSeekable`
- `FileTooLarge`
- `ResourceBusy`
- `ExecutableFileBusy`
- `Deadlock`
- `TooManyLinks`
- `ArgumentListTooLong`
- `Unsupported`

This PR does not stabilize:
- `FilesystemLoop`
- `FilesystemQuotaExceeded`
- `CrossesDevices`
- `InvalidFilename`

Hopefully, this will allow us to move forward with this highly and long awaited addition to std, both allowing to still polish the less clear parts of it and not leading to stagnation.

r? joshtriplett
because they seem to be listed as a part of t-libs-api and were one of the most responsive persons previously
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. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants