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

Redirecting stderr to stdout #546

Open
patricoferris opened this issue Jun 5, 2023 · 3 comments
Open

Redirecting stderr to stdout #546

patricoferris opened this issue Jun 5, 2023 · 3 comments
Labels
enhancement New feature or request

Comments

@patricoferris
Copy link
Collaborator

I don't think we have a convenient way to do this without having to essentially recreate most of what is internal to Eio_unix.Process ? Is it reasonable to expect the following program to work?

open Eio

let () =
  Eio_main.run @@ fun env ->
  let proc = Eio.Stdenv.process_mgr env in
  let out = Eio.Process.parse_out proc ~stderr:env#stdout Buf_read.take_all [ "sh"; "-c"; "echo out1; echo >&2 out2; echo out3" ] in
  Eio.traceln "%s" out

I'm guessing it doesn't because internally stdout is remapped to a pipe and so we really want to say remap stderr to whatever stdout is now mapped to i.e. here:

let stdin_fd = read_of_fd ~sw stdin ~default:Fd.stdin ~to_close in
let stdout_fd = write_of_fd ~sw stdout ~default:Fd.stdout ~to_close in
let stderr_fd = write_of_fd ~sw stderr ~default:Fd.stderr ~to_close in
let fds = [
0, stdin_fd, `Blocking;
1, stdout_fd, `Blocking;
2, stderr_fd, `Blocking;

But maybe I'm missing something.

@talex5
Copy link
Collaborator

talex5 commented Jun 5, 2023

Is it reasonable to expect the following program to work?

It sends the child's stderr to env#stdout, which is the Eio's process's standard output, so I'd consider that to be working correctly.

It would be good to have a way of using the same flow for both stdout and stderr, indeed. It's parse_out that you have to duplicate and modify, I think:

let parse_out_err t parse ?cwd ?stdin ?env ?executable args =
  Switch.run @@ fun sw ->
  let r, w = Eio.Process.pipe t ~sw in
  try
    let child = Eio.Process.spawn ~sw t ?cwd ?stdin ~stdout:w ~stderr:w ?env ?executable args in
    Flow.close w;
    let output = Buf_read.parse_exn parse r ~max_size:max_int in
    Flow.close r;
    Eio.Process.await_exn child;
    output
  with Exn.Io _ as ex ->
    let bt = Printexc.get_raw_backtrace () in
    Eio.Exn.reraise_with_context ex bt "running command: %a" Eio.Process.pp_args args

Not sure what to call the function, or whether it should be merged with parse_out somehow.

@patricoferris
Copy link
Collaborator Author

It sends the child's stderr to env#stdout, which is the Eio's process's standard output, so I'd consider that to be working correctly.

Thank you! Yep 🤦

Ah that's a good point actually, I was doing this incorrectly by passing in the same buffer sink (to spawn) for stdout and stderr which would create two pipes internally writing into the same buffer but which would be more racy in terms of the output, I really needed to create a pipe which would be backed by an FD which would avoid this. Perhaps we don't need a new function, but just some documentation ?

@talex5
Copy link
Collaborator

talex5 commented Jun 6, 2023

I think we should have support for this - it's a common thing to want to do. I'm just not decided on what the best API is. Some options:

  1. Create a new function (like parse_out_err) above. But it has to be kept in sync as we add new features.
  2. Have ?stderr be either Flow of sink | Stdout. Though that is slightly inconsistent with the other functions, and a bit more work when you want to pass a flow.
  3. Have an extra ?include_stderr bool. But then you need to raise invalid_arg if that and stderr are used together.

@talex5 talex5 added the enhancement New feature or request label Jul 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants