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 both stdout + stderror for Base.run results in an empty output when running multiple commands in sequence #46768

Closed
cdsthreeline opened this issue Sep 14, 2022 · 1 comment · Fixed by #52461

Comments

@cdsthreeline
Copy link

cdsthreeline commented Sep 14, 2022

Trying to run two commands in sequence, with redirected stdout + stderr using Base.pipeline, results in the second command's output being empty on MacOS 12.5.1 on a 2021 M1 Macbook Pro. This is an issue arises when using the M1-native version of Julia 1.8.1

MWE using 1.8.1, aarch64:

julia> module A 
       function f(command)
       stdout = IOBuffer()
       stderr = IOBuffer()
       
       run(pipeline(command; stdout, stderr))
       
       return String(take!(stdout))
       end
       
       function g()
       result1 = f(`echo 123`)
       result2 = f(`echo 456`)
       return result1, result2
       end
       end
Main.A

julia> A.g()
("123\n", "")

Curiously, only redirecting stdout seems to be fine

julia> module A 
       function f(command)
       stdout = IOBuffer()
       stderr = IOBuffer()
       
       run(pipeline(command; stdout, stderr))
       
       return String(take!(stdout))
       end
       
       function g()
       result1 = f(`echo 123`)
       result2 = f(`echo 456`)
       return result1, result2
       end
       end
Main.A

julia> A.g()
("123\n", "456\n")

Running on 1.8.1, x86-64 results in the correct output

julia> module A 
       function f(command)
       stdout = IOBuffer()
       stderr = IOBuffer()
       
       run(pipeline(command; stdout, stderr))
       
       return String(take!(stdout))
       end
       
       function g()
       result1 = f(`echo 123`)
       result2 = f(`echo 456`)
       return result1, result2
       end
       end
Main.A

julia> A.g()
("123\n", "456\n")
@vtjnash
Copy link
Member

vtjnash commented Sep 14, 2022

Redirecting to IOBuffer is supposed to destroy the contents at the end in all cases, not just with multiple streams redirecting to it. Use a Base.PipeEndpoint() to cause them to be the same IO kernel object.

vtjnash added a commit that referenced this issue Dec 8, 2023
People expect to use this (the docs even almost even suggested it at
some point), so it is better to make it work as expected (and better
than they can emulate) than to criticize their choices.

Also fix a few regressions and handling mistakes in setup_stdios:
 - #44500 tried to store a Redirectable into a SpawnIO, dropping FileRedirect
 - CmdRedirect did not allocate a ProcessChain, so it wouldd call
   setup_stdio then call setup_stdios on the result of that, which is
   strongly discouraged as setup_stdio(s) should only be called once
 - BufferStream was missing `check_open` calls before writing, and
   ignored `Base.reseteof` as a possible means of resuming writing after
   `closewrite` sends a shutdown message.

Fixes #39311
Fixes #49234
Closes #49233
Closes #46768
vtjnash added a commit that referenced this issue Dec 8, 2023
People expect to use this (the docs even almost even suggested it at
some point), so it is better to make it work as expected (and better
than they can emulate) than to criticize their choices.

Also fix a few regressions and handling mistakes in setup_stdios:
 - #44500 tried to store a Redirectable into a SpawnIO, dropping FileRedirect
 - CmdRedirect did not allocate a ProcessChain, so it wouldd call
   setup_stdio then call setup_stdios on the result of that, which is
   strongly discouraged as setup_stdio(s) should only be called once
 - BufferStream was missing `check_open` calls before writing, and
   ignored `Base.reseteof` as a possible means of resuming writing after
   `closewrite` sends a shutdown message. Currently this fix is disabled
   because Pkg seems like a bit of a disaster with IO mismanagement.
 - Add `closewrite` to more methods, and document it.

Fixes #39311
Fixes #49234
Closes #49233
Closes #46768
vtjnash added a commit that referenced this issue Dec 8, 2023
People expect to use this (the docs even almost even suggested it at
some point), so it is better to make it work as expected (and better
than they can emulate) than to criticize their choices.

Also fix a few regressions and handling mistakes in setup_stdios:
 - #44500 tried to store a Redirectable into a SpawnIO, dropping FileRedirect
 - CmdRedirect did not allocate a ProcessChain, so it wouldd call
   setup_stdio then call setup_stdios on the result of that, which is
   strongly discouraged as setup_stdio(s) should only be called once
 - BufferStream was missing `check_open` calls before writing, and
   ignored `Base.reseteof` as a possible means of resuming writing after
   `closewrite` sends a shutdown message. Currently this fix is disabled
   because Pkg seems like a bit of a disaster with IO mismanagement.
 - Add `closewrite` to more methods, and document it.

Fixes #39311
Fixes #49234
Closes #49233
Closes #46768
vtjnash added a commit that referenced this issue Dec 9, 2023
People expect to use this (the docs even almost even suggested it at
some point), so it is better to make it work as expected (and better
than they can emulate) than to criticize their choices.

Also fix a few regressions and handling mistakes in setup_stdios:
 - #44500 tried to store a Redirectable into a SpawnIO, dropping FileRedirect
 - CmdRedirect did not allocate a ProcessChain, so it wouldd call
   setup_stdio then call setup_stdios on the result of that, which is
   strongly discouraged as setup_stdio(s) should only be called once
 - BufferStream was missing `check_open` calls before writing, and
   ignored `Base.reseteof` as a possible means of resuming writing after
   `closewrite` sends a shutdown message. Currently this fix is disabled
   because Pkg seems like a bit of a disaster with IO mismanagement.
 - Add `closewrite` to more methods, and document it.

Fixes #39311
Fixes #49234
Closes #49233
Closes #46768
vtjnash added a commit that referenced this issue Dec 11, 2023
People expect to use this (the docs even almost even suggested it at
some point), so it is better to make it work as expected (and better
than they can emulate) than to criticize their choices.

Also fix a few regressions and handling mistakes in setup_stdios:
 - #44500 tried to store a Redirectable into a SpawnIO, dropping FileRedirect
 - CmdRedirect did not allocate a ProcessChain, so it wouldd call
   setup_stdio then call setup_stdios on the result of that, which is
   strongly discouraged as setup_stdio(s) should only be called once
 - BufferStream was missing `check_open` calls before writing, and
   ignored `Base.reseteof` as a possible means of resuming writing after
   `closewrite` sends a shutdown message. Currently this fix is disabled
   because Pkg seems like a bit of a disaster with IO mismanagement.
 - Add `closewrite` to more methods, and document it.

Fixes #39311
Fixes #49234
Closes #49233
Closes #46768
vtjnash added a commit that referenced this issue Dec 13, 2023
People expect to use this (the docs even almost even suggested it at
some point), so it is better to make it work as expected (and better
than they can emulate) than to criticize their choices.

Also fix a few regressions and handling mistakes in setup_stdios:
- #44500 tried to store a Redirectable into a SpawnIO, dropping
FileRedirect
- CmdRedirect did not allocate a ProcessChain, so it would call
setup_stdio then call setup_stdios on the result of that, which is
strongly discouraged as setup_stdio(s) should only be called once
- BufferStream was missing `check_open` calls before writing, and
ignored `Base.reseteof` as a possible means of resuming writing after
`closewrite` sends a shutdown message.
 - Add `closewrite` to more methods, and document it.

Fixes #39311
Fixes #49234
Fixes #49233
Fixes #46768
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

Successfully merging a pull request may close this issue.

2 participants