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 anonymous_pipe #135822

Closed
wants to merge 5 commits into from
Closed

Conversation

tbu-
Copy link
Contributor

@tbu- tbu- commented Jan 21, 2025

Closes #127154.

Based on top of #135635 to avoid conflicts.

@rustbot
Copy link
Collaborator

rustbot commented Jan 21, 2025

r? @joboet

rustbot has assigned @joboet.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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 Jan 21, 2025
@jieyouxu jieyouxu added the F-anonymous_pipe `#![feature(anonymous_pipe)]` label Jan 21, 2025
@tbu-
Copy link
Contributor Author

tbu- commented Jan 21, 2025

@tbu-
Copy link
Contributor Author

tbu- commented Jan 21, 2025

CC @NobodyXu

Copy link
Contributor

@NobodyXu NobodyXu 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'm so happy that this is getting stabilised 🎉

@rust-log-analyzer

This comment has been minimized.

@tbu- tbu- force-pushed the pr_io_pipe_stabilisation branch from 42584e1 to aba3017 Compare January 24, 2025 10:26
@rust-log-analyzer

This comment has been minimized.

Copy link
Member

@joboet joboet left a comment

Choose a reason for hiding this comment

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

The changes here look good to me, though I'd like to see another change before stabilization:

Currently, things like the Stdio: From<PipeWriter> implementations live in sys:: which IMHO is really confusing and dangerous (because people will forget to add these implementations when porting std). Could you move all the public trait implementations to io::pipe (and os::_::pipe for the platform-specific implementations)?

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 24, 2025
@tbu- tbu- force-pushed the pr_io_pipe_stabilisation branch from aba3017 to 887fb7f Compare January 24, 2025 13:02
@rustbot
Copy link
Collaborator

rustbot commented Jan 24, 2025

The Miri subtree was changed

cc @rust-lang/miri

@bors
Copy link
Contributor

bors commented Jan 26, 2025

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

@tbu- tbu- force-pushed the pr_io_pipe_stabilisation branch from 887fb7f to 604faa6 Compare January 26, 2025 11:46
@rust-log-analyzer

This comment has been minimized.

Also get rid of the now mostly-empty `std::sys::anonymous_pipe` module.
It basically just unwrapped the `AnonPipe` abstraction from
`std::sys::pipe`.
@tbu- tbu- force-pushed the pr_io_pipe_stabilisation branch from 5815404 to 0778f24 Compare January 26, 2025 13:42
@rustbot rustbot added O-unix Operating system: Unix-like O-windows Operating system: Windows labels Jan 26, 2025
@rust-log-analyzer
Copy link
Collaborator

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
   --> library/std/src/io/pipe.rs:78:5
    |
76  | #[derive(Debug)]
    |          ----- in this derive macro expansion
77  | pub struct PipeReader {
78  |     inner: AnonPipe,
    |     ^^^^^^^^^^^^^^^ `AnonPipe` cannot be formatted using `{:?}`
   ::: /checkout/library/core/src/fmt/mod.rs:910:5
    |
910 |     pub macro Debug($item:item) {
    |     --------------- in this expansion of `#[derive(Debug)]`
    |     --------------- in this expansion of `#[derive(Debug)]`
    |
    = help: the trait `core::fmt::Debug` is not implemented for `AnonPipe`
    = note: add `#[derive(Debug)]` to `AnonPipe` or manually `impl core::fmt::Debug for AnonPipe`
help: consider annotating `AnonPipe` with `#[derive(Debug)]`
   --> library/std/src/sys/pal/windows/pipe.rs:19:1
19  + #[derive(Debug)]
20  | pub struct AnonPipe {
    |


error[E0277]: `AnonPipe` doesn't implement `core::fmt::Debug`
   --> library/std/src/io/pipe.rs:85:5
    |
83  | #[derive(Debug)]
    |          ----- in this derive macro expansion
84  | pub struct PipeWriter {
85  |     inner: AnonPipe,
    |     ^^^^^^^^^^^^^^^ `AnonPipe` cannot be formatted using `{:?}`
   ::: /checkout/library/core/src/fmt/mod.rs:910:5
    |
910 |     pub macro Debug($item:item) {
    |     --------------- in this expansion of `#[derive(Debug)]`
    |     --------------- in this expansion of `#[derive(Debug)]`
    |
    = help: the trait `core::fmt::Debug` is not implemented for `AnonPipe`
    = note: add `#[derive(Debug)]` to `AnonPipe` or manually `impl core::fmt::Debug for AnonPipe`
help: consider annotating `AnonPipe` with `#[derive(Debug)]`
   --> library/std/src/sys/pal/windows/pipe.rs:19:1
19  + #[derive(Debug)]
20  | pub struct AnonPipe {
    |


error[E0599]: the method `as_handle` exists for reference `&AnonPipe`, but its trait bounds were not satisfied
   --> library/std/src/os/windows/io/handle.rs:552:25
552 |         self.as_inner().as_handle()
    |                         ^^^^^^^^^
    |
   ::: library/std/src/sys/pal/windows/pipe.rs:19:1
   ::: library/std/src/sys/pal/windows/pipe.rs:19:1
    |
19  | pub struct AnonPipe {
    | ------------------- doesn't satisfy `AnonPipe: os::windows::io::handle::AsHandle`
note: trait bound `AnonPipe: os::windows::io::handle::AsHandle` was not satisfied
note: trait bound `AnonPipe: os::windows::io::handle::AsHandle` was not satisfied
   --> library/std/src/os/windows/io/handle.rs:445:9
    |
445 | impl<T: AsHandle + ?Sized> AsHandle for &T {
    |         |
    |         unsatisfied trait bound introduced here
note: the trait `os::windows::io::handle::AsHandle` must be implemented
note: the trait `os::windows::io::handle::AsHandle` must be implemented
   --> library/std/src/os/windows/io/handle.rs:426:1
426 | pub trait AsHandle {
    | ^^^^^^^^^^^^^^^^^^
    = help: items from traits can only be used if the trait is implemented and in scope
    = help: items from traits can only be used if the trait is implemented and in scope
note: `os::windows::io::handle::AsHandle` defines an item `as_handle`, perhaps you need to implement it
   --> library/std/src/os/windows/io/handle.rs:426:1
426 | pub trait AsHandle {
    | ^^^^^^^^^^^^^^^^^^


error[E0599]: the method `as_handle` exists for reference `&AnonPipe`, but its trait bounds were not satisfied
   --> library/std/src/os/windows/io/handle.rs:571:25
571 |         self.as_inner().as_handle()
    |                         ^^^^^^^^^
    |
   ::: library/std/src/sys/pal/windows/pipe.rs:19:1
   ::: library/std/src/sys/pal/windows/pipe.rs:19:1
    |
19  | pub struct AnonPipe {
    | ------------------- doesn't satisfy `AnonPipe: os::windows::io::handle::AsHandle`
note: trait bound `AnonPipe: os::windows::io::handle::AsHandle` was not satisfied
note: trait bound `AnonPipe: os::windows::io::handle::AsHandle` was not satisfied
   --> library/std/src/os/windows/io/handle.rs:445:9
    |
445 | impl<T: AsHandle + ?Sized> AsHandle for &T {
    |         |
    |         unsatisfied trait bound introduced here
note: the trait `os::windows::io::handle::AsHandle` must be implemented
note: the trait `os::windows::io::handle::AsHandle` must be implemented
   --> library/std/src/os/windows/io/handle.rs:426:1
426 | pub trait AsHandle {
    | ^^^^^^^^^^^^^^^^^^
    = help: items from traits can only be used if the trait is implemented and in scope
    = help: items from traits can only be used if the trait is implemented and in scope
note: `os::windows::io::handle::AsHandle` defines an item `as_handle`, perhaps you need to implement it
   --> library/std/src/os/windows/io/handle.rs:426:1
426 | pub trait AsHandle {
    | ^^^^^^^^^^^^^^^^^^


error[E0599]: no method named `as_raw_handle` found for reference `&AnonPipe` in the current scope
   --> library/std/src/os/windows/io/raw.rs:104:25
104 |         self.as_inner().as_raw_handle()
    |                         ^^^^^^^^^^^^^ method not found in `&AnonPipe`
    |
    = help: items from traits can only be used if the trait is implemented and in scope
    = help: items from traits can only be used if the trait is implemented and in scope
note: `os::windows::io::raw::AsRawHandle` defines an item `as_raw_handle`, perhaps you need to implement it
   --> library/std/src/os/windows/io/raw.rs:22:1
22  | pub trait AsRawHandle {
    | ^^^^^^^^^^^^^^^^^^^^^


error[E0599]: no method named `as_raw_handle` found for reference `&AnonPipe` in the current scope
   --> library/std/src/os/windows/io/raw.rs:111:25
111 |         self.as_inner().as_raw_handle()
    |                         ^^^^^^^^^^^^^ method not found in `&AnonPipe`
    |
    = help: items from traits can only be used if the trait is implemented and in scope
    = help: items from traits can only be used if the trait is implemented and in scope
note: `os::windows::io::raw::AsRawHandle` defines an item `as_raw_handle`, perhaps you need to implement it
   --> library/std/src/os/windows/io/raw.rs:22:1
22  | pub trait AsRawHandle {
    | ^^^^^^^^^^^^^^^^^^^^^

error[E0599]: no method named `into_raw_handle` found for struct `AnonPipe` in the current scope
---
19  | pub struct AnonPipe {
    | ------------------- method `into_raw_handle` not found for this struct
    |
    = help: items from traits can only be used if the trait is implemented and in scope
note: `os::windows::io::raw::IntoRawHandle` defines an item `into_raw_handle`, perhaps you need to implement it
   --> library/std/src/os/windows/io/raw.rs:78:1
78  | pub trait IntoRawHandle {
    | ^^^^^^^^^^^^^^^^^^^^^^^
help: there is a method `into_handle` with a similar name
    |
---
    |
210 |       unsafe fn from_raw_handle(raw_handle: RawHandle) -> io::PipeWriter {
    |                                                           -------------- expected `PipeWriter` because of return type
...
213 | /             io::PipeReader::from_inner(FromInner::from_inner(FromInner::from_inner(
214 | |                 OwnedHandle::from_raw_handle(handle),
    | |_______________^ expected `PipeWriter`, found `PipeReader`

error[E0599]: no method named `into_raw_handle` found for struct `AnonPipe` in the current scope
   --> library/std/src/os/windows/io/raw.rs:222:27
---
19  | pub struct AnonPipe {
    | ------------------- method `into_raw_handle` not found for this struct
    |
    = help: items from traits can only be used if the trait is implemented and in scope
note: `os::windows::io::raw::IntoRawHandle` defines an item `into_raw_handle`, perhaps you need to implement it
   --> library/std/src/os/windows/io/raw.rs:78:1
78  | pub trait IntoRawHandle {
    | ^^^^^^^^^^^^^^^^^^^^^^^
help: there is a method `into_handle` with a similar name
    |

@NobodyXu
Copy link
Contributor

Hi @tbu- If you are busy and don't have time to continue this, I can open separate PRs to stablise annoynous_pipe and do the refactoring

workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Mar 7, 2025
Prevent `rmake.rs` from using unstable features, and fix 3 run-make tests that currently do

Addresses (mostly) rust-lang#137532.
Follow-up to rust-lang#137373.

### Summary

- Fix 3 run-make tests that currently use unstable features:
    1. `tests/run-make/issue-107495-archive-permissions/rmake.rs` uses `#![feature(rustc_private)]` for `libc` on `unix`, but `run_make_support` already exports `libc`, so just use that.
    2. `tests/run-make/cross-lang-lto/rmake.rs` uses `#![feature(path_file_prefix)]` for convenience, replaced with similar filename prefix logic.
    3. `tests/run-make/broken-pipe-no-ice/rmake.rs` uses `#![feature(anonymous_pipe)]` for anonymous pipes. This is more complicated[^race-condition], and I decided to temporarily introduce a dependency on [`os_pipe`] before std's `anonymous_pipe` library feature is stabilized[^pipe-stab]. I left a FIXME tracked by rust-lang#137532 to make the switch once `anonymous_pipe` stabilizes and reaches beta.
- Use `RUSTC_BOOTSTRAP=-1` when building `rmake.rs` to have the stage 0 rustc reject any unstable features used in `rmake.rs`.

- The requirement that `rmake.rs` may not use any unstable features is now documented in rustc-dev-guide.
- This PR does not impose `RUSTC_BOOTSTRAP=-1` when building `run-make-support`, but I suppose we could.

r? `@Kobzol`

[`os_pipe`]: https://github.com/oconnor663/os_pipe.rs

[^race-condition]: We can't just try to spawn `rustc` and immediate close the stderr handle because of race condition, as there's no guarantee `rustc` will not try to print to stderr before the handle gets closed.
[^pipe-stab]: In-progress stabilization PR over at rust-lang#135822.
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 7, 2025
Prevent `rmake.rs` from using unstable features, and fix 3 run-make tests that currently do

Addresses (mostly) rust-lang#137532.
Follow-up to rust-lang#137373.

### Summary

- Fix 3 run-make tests that currently use unstable features:
    1. `tests/run-make/issue-107495-archive-permissions/rmake.rs` uses `#![feature(rustc_private)]` for `libc` on `unix`, but `run_make_support` already exports `libc`, so just use that.
    2. `tests/run-make/cross-lang-lto/rmake.rs` uses `#![feature(path_file_prefix)]` for convenience, replaced with similar filename prefix logic.
    3. `tests/run-make/broken-pipe-no-ice/rmake.rs` uses `#![feature(anonymous_pipe)]` for anonymous pipes. This is more complicated[^race-condition], and I decided to temporarily introduce a dependency on [`os_pipe`] before std's `anonymous_pipe` library feature is stabilized[^pipe-stab]. I left a FIXME tracked by rust-lang#137532 to make the switch once `anonymous_pipe` stabilizes and reaches beta.
- Use `RUSTC_BOOTSTRAP=-1` when building `rmake.rs` to have the stage 0 rustc reject any unstable features used in `rmake.rs`.

- The requirement that `rmake.rs` may not use any unstable features is now documented in rustc-dev-guide.
- This PR does not impose `RUSTC_BOOTSTRAP=-1` when building `run-make-support`, but I suppose we could.

r? `@Kobzol`

try-job: x86_64-msvc-1
try-job: x86_64-mingw-1

[`os_pipe`]: https://github.com/oconnor663/os_pipe.rs

[^race-condition]: We can't just try to spawn `rustc` and immediate close the stderr handle because of race condition, as there's no guarantee `rustc` will not try to print to stderr before the handle gets closed.
[^pipe-stab]: In-progress stabilization PR over at rust-lang#135822.
jhpratt added a commit to jhpratt/rust that referenced this pull request Mar 8, 2025
Prevent `rmake.rs` from using unstable features, and fix 3 run-make tests that currently do

Addresses (mostly) rust-lang#137532.
Follow-up to rust-lang#137373.

### Summary

- Fix 3 run-make tests that currently use unstable features:
    1. `tests/run-make/issue-107495-archive-permissions/rmake.rs` uses `#![feature(rustc_private)]` for `libc` on `unix`, but `run_make_support` already exports `libc`, so just use that.
    2. `tests/run-make/cross-lang-lto/rmake.rs` uses `#![feature(path_file_prefix)]` for convenience, replaced with similar filename prefix logic.
    3. `tests/run-make/broken-pipe-no-ice/rmake.rs` uses `#![feature(anonymous_pipe)]` for anonymous pipes. This is more complicated[^race-condition], and I decided to temporarily introduce a dependency on [`os_pipe`] before std's `anonymous_pipe` library feature is stabilized[^pipe-stab]. I left a FIXME tracked by rust-lang#137532 to make the switch once `anonymous_pipe` stabilizes and reaches beta.
- Use `RUSTC_BOOTSTRAP=-1` when building `rmake.rs` to have the stage 0 rustc reject any unstable features used in `rmake.rs`.

- The requirement that `rmake.rs` may not use any unstable features is now documented in rustc-dev-guide.
- This PR does not impose `RUSTC_BOOTSTRAP=-1` when building `run-make-support`, but I suppose we could.

r? `@Kobzol`

try-job: x86_64-msvc-1
try-job: x86_64-mingw-1

[`os_pipe`]: https://github.com/oconnor663/os_pipe.rs

[^race-condition]: We can't just try to spawn `rustc` and immediate close the stderr handle because of race condition, as there's no guarantee `rustc` will not try to print to stderr before the handle gets closed.
[^pipe-stab]: In-progress stabilization PR over at rust-lang#135822.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 8, 2025
Rollup merge of rust-lang#137537 - jieyouxu:daily-rmake, r=Kobzol

Prevent `rmake.rs` from using unstable features, and fix 3 run-make tests that currently do

Addresses (mostly) rust-lang#137532.
Follow-up to rust-lang#137373.

### Summary

- Fix 3 run-make tests that currently use unstable features:
    1. `tests/run-make/issue-107495-archive-permissions/rmake.rs` uses `#![feature(rustc_private)]` for `libc` on `unix`, but `run_make_support` already exports `libc`, so just use that.
    2. `tests/run-make/cross-lang-lto/rmake.rs` uses `#![feature(path_file_prefix)]` for convenience, replaced with similar filename prefix logic.
    3. `tests/run-make/broken-pipe-no-ice/rmake.rs` uses `#![feature(anonymous_pipe)]` for anonymous pipes. This is more complicated[^race-condition], and I decided to temporarily introduce a dependency on [`os_pipe`] before std's `anonymous_pipe` library feature is stabilized[^pipe-stab]. I left a FIXME tracked by rust-lang#137532 to make the switch once `anonymous_pipe` stabilizes and reaches beta.
- Use `RUSTC_BOOTSTRAP=-1` when building `rmake.rs` to have the stage 0 rustc reject any unstable features used in `rmake.rs`.

- The requirement that `rmake.rs` may not use any unstable features is now documented in rustc-dev-guide.
- This PR does not impose `RUSTC_BOOTSTRAP=-1` when building `run-make-support`, but I suppose we could.

r? `@Kobzol`

try-job: x86_64-msvc-1
try-job: x86_64-mingw-1

[`os_pipe`]: https://github.com/oconnor663/os_pipe.rs

[^race-condition]: We can't just try to spawn `rustc` and immediate close the stderr handle because of race condition, as there's no guarantee `rustc` will not try to print to stderr before the handle gets closed.
[^pipe-stab]: In-progress stabilization PR over at rust-lang#135822.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 9, 2025
…r=joshtriplett

Stablize anonymous pipe

Since rust-lang#135822 is staled, I create this PR to stablise anonymous pipe

Closes rust-lang#127154
jieyouxu pushed a commit to jieyouxu/rustc-dev-guide that referenced this pull request Mar 13, 2025
Prevent `rmake.rs` from using unstable features, and fix 3 run-make tests that currently do

Addresses (mostly) #137532.
Follow-up to #137373.

### Summary

- Fix 3 run-make tests that currently use unstable features:
    1. `tests/run-make/issue-107495-archive-permissions/rmake.rs` uses `#![feature(rustc_private)]` for `libc` on `unix`, but `run_make_support` already exports `libc`, so just use that.
    2. `tests/run-make/cross-lang-lto/rmake.rs` uses `#![feature(path_file_prefix)]` for convenience, replaced with similar filename prefix logic.
    3. `tests/run-make/broken-pipe-no-ice/rmake.rs` uses `#![feature(anonymous_pipe)]` for anonymous pipes. This is more complicated[^race-condition], and I decided to temporarily introduce a dependency on [`os_pipe`] before std's `anonymous_pipe` library feature is stabilized[^pipe-stab]. I left a FIXME tracked by #137532 to make the switch once `anonymous_pipe` stabilizes and reaches beta.
- Use `RUSTC_BOOTSTRAP=-1` when building `rmake.rs` to have the stage 0 rustc reject any unstable features used in `rmake.rs`.

- The requirement that `rmake.rs` may not use any unstable features is now documented in rustc-dev-guide.
- This PR does not impose `RUSTC_BOOTSTRAP=-1` when building `run-make-support`, but I suppose we could.

r? `@Kobzol`

try-job: x86_64-msvc-1
try-job: x86_64-mingw-1

[`os_pipe`]: https://github.com/oconnor663/os_pipe.rs

[^race-condition]: We can't just try to spawn `rustc` and immediate close the stderr handle because of race condition, as there's no guarantee `rustc` will not try to print to stderr before the handle gets closed.
[^pipe-stab]: In-progress stabilization PR over at rust-lang/rust#135822.
@bors
Copy link
Contributor

bors commented Mar 13, 2025

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

bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 13, 2025
…<try>

Stablize anonymous pipe

Since rust-lang#135822 is staled, I create this PR to stablise anonymous pipe

Closes rust-lang#127154

try-job: test-various
jhpratt added a commit to jhpratt/rust that referenced this pull request Mar 17, 2025
…r=joshtriplett

Stablize anonymous pipe

Since rust-lang#135822 is staled, I create this PR to stablise anonymous pipe

Closes rust-lang#127154

try-job: test-various
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 17, 2025
Rollup merge of rust-lang#137793 - NobodyXu:stablise-annoymous-pipe, r=joshtriplett

Stablize anonymous pipe

Since rust-lang#135822 is staled, I create this PR to stablise anonymous pipe

Closes rust-lang#127154

try-job: test-various
@tbu-
Copy link
Contributor Author

tbu- commented Mar 17, 2025

Superseded by #137793.

Sorry to the additional delay from my stalled PR, @NobodyXu.

@tbu- tbu- closed this Mar 17, 2025
@NobodyXu
Copy link
Contributor

No worries, we all have our own lives and just doing this over spare time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-anonymous_pipe `#![feature(anonymous_pipe)]` O-unix Operating system: Unix-like O-windows Operating system: Windows S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tracking Issue for anonymous pipe API
7 participants