Skip to content

Commit

Permalink
cli: ensure child processes are wait()ed on I/O error
Browse files Browse the repository at this point in the history
  • Loading branch information
yuja committed Mar 5, 2024
1 parent c8023db commit 2817e30
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 24 deletions.
38 changes: 15 additions & 23 deletions cli/src/git_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,30 +94,22 @@ fn pinentry_get_pw(url: &str) -> Option<String> {
.stdout(Stdio::piped())
.spawn()
.ok()?;
#[rustfmt::skip]
pinentry
.stdin
.take()
.unwrap()
.write_all(
format!(
"SETTITLE jj passphrase\n\
SETDESC Enter passphrase for {url}\n\
SETPROMPT Passphrase:\n\
GETPIN\n"
)
.as_bytes(),
)
.ok()?;
let mut out = String::new();
pinentry
.stdout
.take()
.unwrap()
.read_to_string(&mut out)
.ok()?;
let mut interact = || -> std::io::Result<_> {
#[rustfmt::skip]
let req = format!(
"SETTITLE jj passphrase\n\
SETDESC Enter passphrase for {url}\n\
SETPROMPT Passphrase:\n\
GETPIN\n"
);
pinentry.stdin.take().unwrap().write_all(req.as_bytes())?;
let mut out = String::new();
pinentry.stdout.take().unwrap().read_to_string(&mut out)?;
Ok(out)
};
let maybe_out = interact();
_ = pinentry.wait();
for line in out.split('\n') {
for line in maybe_out.ok()?.split('\n') {
if !line.starts_with("D ") {
continue;
}
Expand Down
3 changes: 2 additions & 1 deletion cli/src/merge_tools/external.rs
Original file line number Diff line number Diff line change
Expand Up @@ -552,14 +552,15 @@ pub fn generate_diff(
tool_binary: tool.program.clone(),
source,
})?;
io::copy(&mut child.stdout.take().unwrap(), writer).map_err(ExternalToolError::Io)?;
let copy_result = io::copy(&mut child.stdout.take().unwrap(), writer);
// Non-zero exit code isn't an error. For example, the traditional diff command
// will exit with 1 if inputs are different.
let exit_status = child.wait().map_err(ExternalToolError::Io)?;
tracing::info!(?cmd, ?exit_status, "The external diff generator exited:");
if !exit_status.success() {
writeln!(ui.warning(), "{}", format_tool_aborted(&exit_status)).ok();
}
copy_result.map_err(ExternalToolError::Io)?;
Ok(())
}

Expand Down

0 comments on commit 2817e30

Please sign in to comment.