diff options
| author | bors <bors@rust-lang.org> | 2023-09-09 19:56:51 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2023-09-09 19:56:51 +0000 |
| commit | 559421e8e3ab373a296186104b7b0c18251e8182 (patch) | |
| tree | ea335733f19e43675079e1b81091c97839cffcd7 | |
| parent | 37eec5c80a621cea14dee75bf6f2953a3a5a9864 (diff) | |
| parent | 436fe0189564faf0547fc483d8999a62c94704aa (diff) | |
| download | rust-559421e8e3ab373a296186104b7b0c18251e8182.tar.gz rust-559421e8e3ab373a296186104b7b0c18251e8182.zip | |
Auto merge of #114590 - ijackson:stdio-stdio-2, r=dtolnay
Allow redirecting subprocess stdout to our stderr etc. (redux) This is the code from #88561, tidied up, including review suggestions, and with the for-testing-only CI commit removed. FCP for the API completed in #88561. I have made a new MR to facilitate review. The discussion there is very cluttered and the branch is full of changes (in many cases as a result of changes to other Rust stdlib APIs since then). Assuming this MR is approvedl we should close that one. ### Reviewer doing a de novo review Just code review these four commits.. FCP discussion starts here: https://github.com/rust-lang/rust/pull/88561#issuecomment-1640527595 Portability tests: you can see that this branch works on Windows too by looking at the CI results in #88561, which has the same code changes as this branch but with an additional "DO NOT MERGE" commit to make the Windows tests run. ### Reviewer doing an incremental review from some version of #88561 Review the new commits since your last review. I haven't force pushed the branch there. git diff the two branches (eg `git diff 176886197d6..0842b69c219`). You'll see that the only difference is in gitlab CI files. You can also see that *this* MR doesn't touch those files.
| -rw-r--r-- | library/std/src/process.rs | 60 | ||||
| -rw-r--r-- | library/std/src/sys/unix/process/process_common.rs | 30 | ||||
| -rw-r--r-- | library/std/src/sys/unsupported/process.rs | 20 | ||||
| -rw-r--r-- | library/std/src/sys/windows/process.rs | 35 |
4 files changed, 134 insertions, 11 deletions
diff --git a/library/std/src/process.rs b/library/std/src/process.rs index 7380b45b00f..762a1e9b408 100644 --- a/library/std/src/process.rs +++ b/library/std/src/process.rs @@ -1501,6 +1501,66 @@ impl From<fs::File> for Stdio { } } +#[stable(feature = "stdio_from_stdio", since = "CURRENT_RUSTC_VERSION")] +impl From<io::Stdout> for Stdio { + /// Redirect command stdout/stderr to our stdout + /// + /// # Examples + /// + /// ```rust + /// #![feature(exit_status_error)] + /// use std::io; + /// use std::process::Command; + /// + /// # fn test() -> Result<(), Box<dyn std::error::Error>> { + /// let output = Command::new("whoami") + // "whoami" is a command which exists on both Unix and Windows, + // and which succeeds, producing some stdout output but no stderr. + /// .stdout(io::stdout()) + /// .output()?; + /// output.status.exit_ok()?; + /// assert!(output.stdout.is_empty()); + /// # Ok(()) + /// # } + /// # + /// # if cfg!(unix) { + /// # test().unwrap(); + /// # } + /// ``` + fn from(inherit: io::Stdout) -> Stdio { + Stdio::from_inner(inherit.into()) + } +} + +#[stable(feature = "stdio_from_stdio", since = "CURRENT_RUSTC_VERSION")] +impl From<io::Stderr> for Stdio { + /// Redirect command stdout/stderr to our stderr + /// + /// # Examples + /// + /// ```rust + /// #![feature(exit_status_error)] + /// use std::io; + /// use std::process::Command; + /// + /// # fn test() -> Result<(), Box<dyn std::error::Error>> { + /// let output = Command::new("whoami") + /// .stdout(io::stderr()) + /// .output()?; + /// output.status.exit_ok()?; + /// assert!(output.stdout.is_empty()); + /// # Ok(()) + /// # } + /// # + /// # if cfg!(unix) { + /// # test().unwrap(); + /// # } + /// ``` + fn from(inherit: io::Stderr) -> Stdio { + Stdio::from_inner(inherit.into()) + } +} + /// Describes the result of a process after it has terminated. /// /// This `struct` is used to represent the exit status or other termination of a child process. diff --git a/library/std/src/sys/unix/process/process_common.rs b/library/std/src/sys/unix/process/process_common.rs index e7d1533f122..f729da44774 100644 --- a/library/std/src/sys/unix/process/process_common.rs +++ b/library/std/src/sys/unix/process/process_common.rs @@ -13,7 +13,7 @@ use crate::sys::fd::FileDesc; use crate::sys::fs::File; use crate::sys::pipe::{self, AnonPipe}; use crate::sys_common::process::{CommandEnv, CommandEnvs}; -use crate::sys_common::IntoInner; +use crate::sys_common::{FromInner, IntoInner}; #[cfg(not(target_os = "fuchsia"))] use crate::sys::fs::OpenOptions; @@ -150,6 +150,7 @@ pub enum Stdio { Null, MakePipe, Fd(FileDesc), + StaticFd(BorrowedFd<'static>), } #[derive(Copy, Clone, Debug, Eq, PartialEq)] @@ -463,6 +464,11 @@ impl Stdio { } } + Stdio::StaticFd(fd) => { + let fd = FileDesc::from_inner(fd.try_clone_to_owned()?); + Ok((ChildStdio::Owned(fd), None)) + } + Stdio::MakePipe => { let (reader, writer) = pipe::anon_pipe()?; let (ours, theirs) = if readable { (writer, reader) } else { (reader, writer) }; @@ -497,6 +503,28 @@ impl From<File> for Stdio { } } +impl From<io::Stdout> for Stdio { + fn from(_: io::Stdout) -> Stdio { + // This ought really to be is Stdio::StaticFd(input_argument.as_fd()). + // But AsFd::as_fd takes its argument by reference, and yields + // a bounded lifetime, so it's no use here. There is no AsStaticFd. + // + // Additionally AsFd is only implemented for the *locked* versions. + // We don't want to lock them here. (The implications of not locking + // are the same as those for process::Stdio::inherit().) + // + // Arguably the hypothetical AsStaticFd and AsFd<'static> + // should be implemented for io::Stdout, not just for StdoutLocked. + Stdio::StaticFd(unsafe { BorrowedFd::borrow_raw(libc::STDOUT_FILENO) }) + } +} + +impl From<io::Stderr> for Stdio { + fn from(_: io::Stderr) -> Stdio { + Stdio::StaticFd(unsafe { BorrowedFd::borrow_raw(libc::STDERR_FILENO) }) + } +} + impl ChildStdio { pub fn fd(&self) -> Option<c_int> { match *self { diff --git a/library/std/src/sys/unsupported/process.rs b/library/std/src/sys/unsupported/process.rs index 77b675aaa4e..a639afcc674 100644 --- a/library/std/src/sys/unsupported/process.rs +++ b/library/std/src/sys/unsupported/process.rs @@ -27,6 +27,8 @@ pub struct StdioPipes { pub stderr: Option<AnonPipe>, } +// FIXME: This should be a unit struct, so we can always construct it +// The value here should be never used, since we cannot spawn processes. pub enum Stdio { Inherit, Null, @@ -87,8 +89,26 @@ impl From<AnonPipe> for Stdio { } } +impl From<io::Stdout> for Stdio { + fn from(_: io::Stdout) -> Stdio { + // FIXME: This is wrong. + // Instead, the Stdio we have here should be a unit struct. + panic!("unsupported") + } +} + +impl From<io::Stderr> for Stdio { + fn from(_: io::Stderr) -> Stdio { + // FIXME: This is wrong. + // Instead, the Stdio we have here should be a unit struct. + panic!("unsupported") + } +} + impl From<File> for Stdio { fn from(_file: File) -> Stdio { + // FIXME: This is wrong. + // Instead, the Stdio we have here should be a unit struct. panic!("unsupported") } } diff --git a/library/std/src/sys/windows/process.rs b/library/std/src/sys/windows/process.rs index 84a75d21305..cd5bf7f1538 100644 --- a/library/std/src/sys/windows/process.rs +++ b/library/std/src/sys/windows/process.rs @@ -172,6 +172,7 @@ pub struct Command { pub enum Stdio { Inherit, + InheritSpecific { from_stdio_id: c::DWORD }, Null, MakePipe, Pipe(AnonPipe), @@ -555,17 +556,19 @@ fn program_exists(path: &Path) -> Option<Vec<u16>> { impl Stdio { fn to_handle(&self, stdio_id: c::DWORD, pipe: &mut Option<AnonPipe>) -> io::Result<Handle> { - match *self { - Stdio::Inherit => match stdio::get_handle(stdio_id) { - Ok(io) => unsafe { - let io = Handle::from_raw_handle(io); - let ret = io.duplicate(0, true, c::DUPLICATE_SAME_ACCESS); - io.into_raw_handle(); - ret - }, - // If no stdio handle is available, then propagate the null value. - Err(..) => unsafe { Ok(Handle::from_raw_handle(ptr::null_mut())) }, + let use_stdio_id = |stdio_id| match stdio::get_handle(stdio_id) { + Ok(io) => unsafe { + let io = Handle::from_raw_handle(io); + let ret = io.duplicate(0, true, c::DUPLICATE_SAME_ACCESS); + io.into_raw_handle(); + ret }, + // If no stdio handle is available, then propagate the null value. + Err(..) => unsafe { Ok(Handle::from_raw_handle(ptr::null_mut())) }, + }; + match *self { + Stdio::Inherit => use_stdio_id(stdio_id), + Stdio::InheritSpecific { from_stdio_id } => use_stdio_id(from_stdio_id), Stdio::MakePipe => { let ours_readable = stdio_id != c::STD_INPUT_HANDLE; @@ -613,6 +616,18 @@ impl From<File> for Stdio { } } +impl From<io::Stdout> for Stdio { + fn from(_: io::Stdout) -> Stdio { + Stdio::InheritSpecific { from_stdio_id: c::STD_OUTPUT_HANDLE } + } +} + +impl From<io::Stderr> for Stdio { + fn from(_: io::Stderr) -> Stdio { + Stdio::InheritSpecific { from_stdio_id: c::STD_ERROR_HANDLE } + } +} + //////////////////////////////////////////////////////////////////////////////// // Processes //////////////////////////////////////////////////////////////////////////////// |
