Auto merge of rust-lang#114590 - ijackson:stdio-stdio-2, r=dtolnay

Allow redirecting subprocess stdout to our stderr etc. (redux)

This is the code from rust-lang#88561, tidied up, including review suggestions, and with the for-testing-only CI commit removed.  FCP for the API completed in rust-lang#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: rust-lang#88561 (comment)

Portability tests: you can see that this branch works on Windows too by looking at the CI results in rust-lang#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 rust-lang#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 1768861..0842b69`).  You'll see that the only difference is in gitlab CI files.  You can also see that *this* MR doesn't touch those files.

Author: bors

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.

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 {

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")
     }
 }

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
 ////////////////////////////////////////////////////////////////////////////////