Skip to content

Commit 6766d87

Browse files
committed
unstable-api: actually, don't close stdout
I've been advised that that typically leads to tears. Let's be a bit more elegant and duplicitous and restore the original stdout at the end.
1 parent fe1ca71 commit 6766d87

File tree

1 file changed

+34
-26
lines changed

1 file changed

+34
-26
lines changed

tools/unstable-api/src/main.rs

Lines changed: 34 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -70,32 +70,15 @@ fn with_output_formatting_maybe<F>(f: F) -> Result<(), Error>
7070
where
7171
F: FnOnce() -> Result<(), Error>,
7272
{
73-
use nix::unistd::close;
74-
use std::io::{stdout, Write};
75-
use std::os::unix::io::AsRawFd;
76-
let child = spawn_output_formatter();
77-
78-
let result = f();
79-
let mut stdout = stdout();
80-
stdout.flush().context("couldn't flush stdout")?;
81-
close(stdout.as_raw_fd()).context("couldn't close stdout")?;
82-
83-
if let Some(mut child) = child {
84-
if !child.wait()?.success() {
85-
bail!("output formatting failed");
86-
}
87-
}
88-
89-
result
90-
}
91-
92-
#[cfg(unix)]
93-
fn spawn_output_formatter() -> Option<std::process::Child> {
94-
use nix::unistd::{isatty, dup2};
73+
use nix::unistd::{isatty, close, dup, dup2};
9574
use std::os::unix::io::AsRawFd;
9675
use std::process::{Command, Stdio};
76+
use std::io::{stdout, Write};
77+
78+
let mut original_stdout = None;
79+
let mut inner_child = None;
9780

98-
let mut final_child = None;
81+
stdout().flush().unwrap(); // just in case
9982

10083
if isatty(1) == Ok(true) {
10184
// Pipe the output through `bat` for nice formatting and paging, if available.
@@ -106,9 +89,12 @@ fn spawn_output_formatter() -> Option<std::process::Child> {
10689
.stdout(Stdio::inherit())
10790
.spawn()
10891
{
92+
// Hold on to our stdout for later.
93+
original_stdout = Some(dup(1).unwrap());
10994
// Replace our stdout by the pipe into `bat`.
11095
dup2(bat.stdin.take().unwrap().as_raw_fd(), 1).unwrap();
111-
final_child = Some(bat);
96+
97+
inner_child = Some(bat);
11298
}
11399
}
114100

@@ -118,10 +104,32 @@ fn spawn_output_formatter() -> Option<std::process::Child> {
118104
.stdout(Stdio::inherit()) // This pipes into `bat` if it was executed above.
119105
.spawn()
120106
{
107+
// Hold on to our stdout for later, if we didn't already.
108+
original_stdout.get_or_insert_with(|| dup(1).unwrap());
121109
// Replace our stdout by the pipe into `rustfmt`.
122110
dup2(rustfmt.stdin.take().unwrap().as_raw_fd(), 1).unwrap();
123-
final_child.get_or_insert(rustfmt);
111+
112+
inner_child.get_or_insert(rustfmt);
113+
}
114+
115+
let result = f();
116+
117+
if let Some(fd) = original_stdout {
118+
// Overwriting the current stdout with the original stdout
119+
// closes the pipe to the child's stdin, allowing the child to
120+
// exit.
121+
stdout().flush().unwrap(); // just in case
122+
dup2(fd, 1).unwrap();
123+
close(fd).unwrap();
124124
}
125125

126-
final_child
126+
if let Some(mut child) = inner_child {
127+
// Wait for inner child to exit to ensure it won't write to
128+
// original stdout after we return.
129+
if !child.wait()?.success() {
130+
bail!("output formatting failed");
131+
}
132+
}
133+
134+
result
127135
}

0 commit comments

Comments
 (0)