oconnor663 / duct.rs

a Rust library for running child processes
MIT License
829 stars 34 forks source link

Configuration order of stdio joining and redirection matters #116

Open 13k opened 8 months ago

13k commented 8 months ago

Issue

Looks like the order in which joining (Expression::stdout_to_stderr, Expression::stderr_to_stdout) and redirection (Expression::std{out,err}_path) configuration methods are called results in different behaviors (could also be considered incorrect behavior).

If redirection is used before joining, the joining configuration is ignored:

//# duct = "0.13.7"

use duct::cmd;

fn main() {
    let cmd = cmd!("sh", "-c", "echo out && echo err 1>&2")
        .stdout_path("/tmp/out")
        .stderr_to_stdout();

    println!("{:?}", cmd);

    cmd.run().expect("command error");
}
❱ cargo play -q duct.rs 
Io(StderrToStdout, Io(StdoutPath("/tmp/out"), Cmd(["sh", "-c", "echo out && echo err 1>&2"])))
err

❱ cat /tmp/out
out

If redirection is set up after joining, it works as expected:

let cmd = cmd!("sh", "-c", "echo out && echo err 1>&2")
    .stderr_to_stdout()
    .stdout_path("/tmp/out");
}
❱ cargo play -q duct.rs 
Io(StdoutPath("/tmp/out"), Io(StderrToStdout, Cmd(["sh", "-c", "echo out && echo err 1>&2"])))

❱ cat /tmp/out
out
err

Discussion

Since Expression is structured as a binary tree, I'm not sure how to solve this. Rearranging tree nodes in a custom precedence order seems overkill.

Generating an intermediary flat structure from the tree, then sorting it by precedence, seems more viable. In this case, IoExpressionInner could implement Ord with a custom precedence ordering.

oconnor663 commented 3 months ago

Agreed that this can be quite confusing. Bash actually has similar behavior:

$ (echo out && echo err 1>&2) >/tmp/out 2>&1
$ (echo out && echo err 1>&2) 2>&1 >/tmp/out
err

The order of operations is kind of surprising, and putting in more parentheses actually swaps it around:

$ ((echo out && echo err 1>&2) >/tmp/out) 2>&1
err
$ ((echo out && echo err 1>&2) 2>&1) >/tmp/out

The key to understanding all of this is to think about the stdout/stderr that a given expression is inheriting from above. An operator like 2>&1/stderr_to_stdout affects the stderr that an expression will inherit (it becomes a dup of the stdout it will inherit). But if that expression does redirections internally that cause it to ignore its inherited stderr, those take precedence.

But all of this is still confusing :p Do you think there's a better way we could document what's going on?

13k commented 2 months ago

I think this is an issue with clarity of intent. Like you said, in bash redirection is an operator while in duct it's a method that implicitly behaves as operator because of internal implementation.

Like I said, I understand that reworking the implementation would be too complicated just for the sake of clarity for this specific case. Maybe it should be documented with a warning or something that catches the attention?