Skip to content

Add a one-call run_command() helper that captures output and supports a timeout #100

@zackees

Description

@zackees

Context

Downstream consumer FastLED/fbuild is migrating every `std::process::Command` / `tokio::process::Command` call site to `running-process-core` so containment, env handling, and pipe draining are handled in one place (FastLED/fbuild#141).

The migration is straightforward but verbose: each call site has to build a `ProcessConfig`, instantiate `NativeProcess`, call `start()`, call `wait(timeout)`, then call `captured_stdout()` and `captured_stderr()`. For the ~30 simple "run a tool, get its output, return" sites in fbuild, that's 5–6 lines of boilerplate per site.

Request

Add a synchronous convenience helper, e.g.:

```rust
pub struct RunOutput {
pub stdout: Vec,
pub stderr: Vec,
pub exit_code: i32,
}

/// Spawn, fully drain stdout/stderr concurrently, and wait — with optional timeout.
/// Returns `ProcessError::Timeout` when the deadline elapses; the child is killed.
pub fn run_command(
config: ProcessConfig,
timeout: Option,
) -> Result<RunOutput, ProcessError>;
```

Semantics that fbuild needs (and that bit us recently when hand-rolled — see FastLED/fbuild#141):

  • Concurrent draining. Stdout and stderr must be read in parallel from the moment the child starts, so a child that fills one pipe (e.g. a verbose compiler hammering stderr) cannot deadlock against the parent reading the other.
  • Containment honored. If `config.containment` is `Some(Contained)`, the child must die when the parent dies.
  • Captured output returned. No need to call `captured_stdout()` / `captured_stderr()` separately — they're already in the returned struct.
  • Timeout = kill + error. On timeout, kill the child (and the contained group, on Windows) and return `ProcessError::Timeout`. Drain whatever output is buffered.

A `tokio` flavor (e.g. `run_command_async`) returning the same `RunOutput` would let fbuild's emulator handlers (QEMU, node) drop their custom drain loops too.

Why this isn't just a downstream wrapper

Two reasons it belongs upstream:

  1. The deadlock is non-obvious. Anyone composing `NativeProcess` from outside has to know "start, then wait, then capture" only works if the background readers are running — and the temptation to do "spawn, read stdout, read stderr, wait" is exactly what wrecked fbuild CI for two days.
  2. The std-equivalent is one call. `std::process::Command::output()` and `Child::wait_with_output()` already collapse this pattern. Migrators reasonably expect a `running-process` analogue.

Reference implementation

Today's hand-rolled fbuild equivalent (now using std `wait_with_output` after the recent fix): https://github.com/FastLED/fbuild/blob/main/crates/fbuild-core/src/subprocess.rs#L167-L246 — a `run_command()` upstream would let those ~80 lines collapse to one delegating call.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions