Rollup merge of #105458 - Ayush1325:blocking_spawn, r=Mark-Simulacrum

Allow blocking `Command::output`

### Problem
Currently, `Command::output` is internally implemented using `Command::spawn`. This is problematic because some targets (like UEFI) do not actually support multitasking and thus block while the program is executing. This coupling does not make much sense as `Command::output` is supposed to block until the execution is complete anyway and thus does not need to rely on a non-blocking `Child` or any other intermediate.

### Solution
This PR moves the implementation of `Command::output` to `std::sys`. This means targets can choose to implement only `Command::output` without having to implement `Command::spawn`.

### Additional Information

This was originally conceived when working on https://github.com/rust-lang/rust/pull/100316. Currently, the only target I know about that will benefit from this change is UEFI.

This PR can also be used to implement more efficient `Command::output` since the intermediate `Process` is not actually needed anymore, but that is outside the scope of this PR.

Since this is not a public API change, I'm not sure if an RFC is needed or not.
This commit is contained in:
Matthias Krüger 2022-12-17 23:44:26 +01:00 committed by GitHub
commit 6d1cdcaee5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 77 additions and 6 deletions

View File

@ -362,6 +362,10 @@ impl Read for ChildStdout {
fn is_read_vectored(&self) -> bool {
self.inner.is_read_vectored()
}
fn read_to_end(&mut self, buf: &mut Vec<u8>) -> io::Result<usize> {
self.inner.read_to_end(buf)
}
}
impl AsInner<AnonPipe> for ChildStdout {
@ -907,10 +911,8 @@ impl Command {
/// ```
#[stable(feature = "process", since = "1.0.0")]
pub fn output(&mut self) -> io::Result<Output> {
self.inner
.spawn(imp::Stdio::MakePipe, false)
.map(Child::from_inner)
.and_then(|p| p.wait_with_output())
let (status, stdout, stderr) = self.inner.output()?;
Ok(Output { status: ExitStatus(status), stdout, stderr })
}
/// Executes a command as a child process, waiting for it to finish and

View File

@ -58,6 +58,10 @@ impl AnonPipe {
self.0.is_read_vectored()
}
pub fn read_to_end(&self, buf: &mut Vec<u8>) -> io::Result<usize> {
self.0.read_to_end(buf)
}
pub fn write(&self, buf: &[u8]) -> io::Result<usize> {
self.0.write(buf)
}

View File

@ -35,6 +35,11 @@ impl Command {
Ok((Process { handle: Handle::new(process_handle) }, ours))
}
pub fn output(&mut self) -> io::Result<(ExitStatus, Vec<u8>, Vec<u8>)> {
let (proc, pipes) = self.spawn(Stdio::MakePipe, false)?;
crate::sys_common::process::wait_with_output(proc, pipes)
}
pub fn exec(&mut self, default: Stdio) -> io::Error {
if self.saw_nul() {
return io::const_io_error!(

View File

@ -133,6 +133,11 @@ impl Command {
}
}
pub fn output(&mut self) -> io::Result<(ExitStatus, Vec<u8>, Vec<u8>)> {
let (proc, pipes) = self.spawn(Stdio::MakePipe, false)?;
crate::sys_common::process::wait_with_output(proc, pipes)
}
// Attempts to fork the process. If successful, returns Ok((0, -1))
// in the child, and Ok((child_pid, -1)) in the parent.
#[cfg(not(target_os = "linux"))]

View File

@ -20,6 +20,10 @@ impl Command {
unsupported()
}
pub fn output(&mut self) -> io::Result<(ExitStatus, Vec<u8>, Vec<u8>)> {
unsupported()
}
pub fn exec(&mut self, _default: Stdio) -> io::Error {
unsupported_err()
}

View File

@ -108,6 +108,11 @@ impl Command {
}
}
pub fn output(&mut self) -> io::Result<(ExitStatus, Vec<u8>, Vec<u8>)> {
let (proc, pipes) = self.spawn(Stdio::MakePipe, false)?;
crate::sys_common::process::wait_with_output(proc, pipes)
}
pub fn exec(&mut self, default: Stdio) -> io::Error {
let ret = Command::spawn(self, default, false);
match ret {

View File

@ -15,6 +15,10 @@ impl AnonPipe {
self.0
}
pub fn read_to_end(&self, _buf: &mut Vec<u8>) -> io::Result<usize> {
self.0
}
pub fn write(&self, _buf: &[u8]) -> io::Result<usize> {
self.0
}

View File

@ -75,6 +75,10 @@ impl Command {
) -> io::Result<(Process, StdioPipes)> {
unsupported()
}
pub fn output(&mut self) -> io::Result<(ExitStatus, Vec<u8>, Vec<u8>)> {
unsupported()
}
}
impl From<AnonPipe> for Stdio {

View File

@ -1,7 +1,7 @@
use crate::os::windows::prelude::*;
use crate::ffi::OsStr;
use crate::io::{self, IoSlice, IoSliceMut};
use crate::io::{self, IoSlice, IoSliceMut, Read};
use crate::mem;
use crate::path::Path;
use crate::ptr;
@ -261,6 +261,10 @@ impl AnonPipe {
self.inner.is_read_vectored()
}
pub fn read_to_end(&self, buf: &mut Vec<u8>) -> io::Result<usize> {
self.handle().read_to_end(buf)
}
pub fn write(&self, buf: &[u8]) -> io::Result<usize> {
unsafe {
let len = crate::cmp::min(buf.len(), c::DWORD::MAX as usize) as c::DWORD;

View File

@ -351,6 +351,11 @@ impl Command {
))
}
}
pub fn output(&mut self) -> io::Result<(ExitStatus, Vec<u8>, Vec<u8>)> {
let (proc, pipes) = self.spawn(Stdio::MakePipe, false)?;
crate::sys_common::process::wait_with_output(proc, pipes)
}
}
impl fmt::Debug for Command {

View File

@ -4,7 +4,9 @@
use crate::collections::BTreeMap;
use crate::env;
use crate::ffi::{OsStr, OsString};
use crate::sys::process::EnvKey;
use crate::io;
use crate::sys::pipe::read2;
use crate::sys::process::{EnvKey, ExitStatus, Process, StdioPipes};
// Stores a set of changes to an environment
#[derive(Clone, Debug)]
@ -117,3 +119,30 @@ impl<'a> ExactSizeIterator for CommandEnvs<'a> {
self.iter.is_empty()
}
}
pub fn wait_with_output(
mut process: Process,
mut pipes: StdioPipes,
) -> io::Result<(ExitStatus, Vec<u8>, Vec<u8>)> {
drop(pipes.stdin.take());
let (mut stdout, mut stderr) = (Vec::new(), Vec::new());
match (pipes.stdout.take(), pipes.stderr.take()) {
(None, None) => {}
(Some(out), None) => {
let res = out.read_to_end(&mut stdout);
res.unwrap();
}
(None, Some(err)) => {
let res = err.read_to_end(&mut stderr);
res.unwrap();
}
(Some(out), Some(err)) => {
let res = read2(out, &mut stdout, err, &mut stderr);
res.unwrap();
}
}
let status = process.wait()?;
Ok((status, stdout, stderr))
}