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.
This commit is contained in:
commit
559421e8e3
@ -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.
|
/// 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.
|
/// This `struct` is used to represent the exit status or other termination of a child process.
|
||||||
|
@ -13,7 +13,7 @@ use crate::sys::fd::FileDesc;
|
|||||||
use crate::sys::fs::File;
|
use crate::sys::fs::File;
|
||||||
use crate::sys::pipe::{self, AnonPipe};
|
use crate::sys::pipe::{self, AnonPipe};
|
||||||
use crate::sys_common::process::{CommandEnv, CommandEnvs};
|
use crate::sys_common::process::{CommandEnv, CommandEnvs};
|
||||||
use crate::sys_common::IntoInner;
|
use crate::sys_common::{FromInner, IntoInner};
|
||||||
|
|
||||||
#[cfg(not(target_os = "fuchsia"))]
|
#[cfg(not(target_os = "fuchsia"))]
|
||||||
use crate::sys::fs::OpenOptions;
|
use crate::sys::fs::OpenOptions;
|
||||||
@ -150,6 +150,7 @@ pub enum Stdio {
|
|||||||
Null,
|
Null,
|
||||||
MakePipe,
|
MakePipe,
|
||||||
Fd(FileDesc),
|
Fd(FileDesc),
|
||||||
|
StaticFd(BorrowedFd<'static>),
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
|
#[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 => {
|
Stdio::MakePipe => {
|
||||||
let (reader, writer) = pipe::anon_pipe()?;
|
let (reader, writer) = pipe::anon_pipe()?;
|
||||||
let (ours, theirs) = if readable { (writer, reader) } else { (reader, writer) };
|
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 {
|
impl ChildStdio {
|
||||||
pub fn fd(&self) -> Option<c_int> {
|
pub fn fd(&self) -> Option<c_int> {
|
||||||
match *self {
|
match *self {
|
||||||
|
@ -27,6 +27,8 @@ pub struct StdioPipes {
|
|||||||
pub stderr: Option<AnonPipe>,
|
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 {
|
pub enum Stdio {
|
||||||
Inherit,
|
Inherit,
|
||||||
Null,
|
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 {
|
impl From<File> for Stdio {
|
||||||
fn from(_file: File) -> Stdio {
|
fn from(_file: File) -> Stdio {
|
||||||
|
// FIXME: This is wrong.
|
||||||
|
// Instead, the Stdio we have here should be a unit struct.
|
||||||
panic!("unsupported")
|
panic!("unsupported")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -172,6 +172,7 @@ pub struct Command {
|
|||||||
|
|
||||||
pub enum Stdio {
|
pub enum Stdio {
|
||||||
Inherit,
|
Inherit,
|
||||||
|
InheritSpecific { from_stdio_id: c::DWORD },
|
||||||
Null,
|
Null,
|
||||||
MakePipe,
|
MakePipe,
|
||||||
Pipe(AnonPipe),
|
Pipe(AnonPipe),
|
||||||
@ -555,8 +556,7 @@ fn program_exists(path: &Path) -> Option<Vec<u16>> {
|
|||||||
|
|
||||||
impl Stdio {
|
impl Stdio {
|
||||||
fn to_handle(&self, stdio_id: c::DWORD, pipe: &mut Option<AnonPipe>) -> io::Result<Handle> {
|
fn to_handle(&self, stdio_id: c::DWORD, pipe: &mut Option<AnonPipe>) -> io::Result<Handle> {
|
||||||
match *self {
|
let use_stdio_id = |stdio_id| match stdio::get_handle(stdio_id) {
|
||||||
Stdio::Inherit => match stdio::get_handle(stdio_id) {
|
|
||||||
Ok(io) => unsafe {
|
Ok(io) => unsafe {
|
||||||
let io = Handle::from_raw_handle(io);
|
let io = Handle::from_raw_handle(io);
|
||||||
let ret = io.duplicate(0, true, c::DUPLICATE_SAME_ACCESS);
|
let ret = io.duplicate(0, true, c::DUPLICATE_SAME_ACCESS);
|
||||||
@ -565,7 +565,10 @@ impl Stdio {
|
|||||||
},
|
},
|
||||||
// If no stdio handle is available, then propagate the null value.
|
// If no stdio handle is available, then propagate the null value.
|
||||||
Err(..) => unsafe { Ok(Handle::from_raw_handle(ptr::null_mut())) },
|
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 => {
|
Stdio::MakePipe => {
|
||||||
let ours_readable = stdio_id != c::STD_INPUT_HANDLE;
|
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
|
// Processes
|
||||||
////////////////////////////////////////////////////////////////////////////////
|
////////////////////////////////////////////////////////////////////////////////
|
||||||
|
Loading…
x
Reference in New Issue
Block a user