Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configuration order of stdio joining and redirection matters #116

Open
13k opened this issue Feb 20, 2024 · 2 comments
Open

Configuration order of stdio joining and redirection matters #116

13k opened this issue Feb 20, 2024 · 2 comments

Comments

@13k
Copy link

13k commented Feb 20, 2024

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
Copy link
Owner

oconnor663 commented Aug 15, 2024

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
Copy link
Author

13k commented Aug 21, 2024

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants