Skip to content

Commit

Permalink
Propagate failed status code from subprocess with exec-batch
Browse files Browse the repository at this point in the history
If a child process returns a failure status code for --exec-batch, then
have fd also return a failed status code.

Closes: sharkdp#1136
  • Loading branch information
tmccombs committed Oct 15, 2022
1 parent 4257034 commit 9b0744f
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 17 deletions.
38 changes: 28 additions & 10 deletions src/exec/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,10 @@ pub fn execute_commands<I: Iterator<Item = io::Result<Command>>>(
for result in cmds {
let mut cmd = match result {
Ok(cmd) => cmd,
Err(e) => return handle_cmd_error(None, e),
Err(e) => {
print_cmd_error(e);
return ExitCode::GeneralError;
}
};

// Spawn the supplied command.
Expand All @@ -85,26 +88,41 @@ pub fn execute_commands<I: Iterator<Item = io::Result<Command>>>(
}
Err(why) => {
output_buffer.write();
return handle_cmd_error(Some(&cmd), why);
return handle_cmd_error(&cmd, CommandError::Io(why));
}
}
}
output_buffer.write();
ExitCode::Success
}

pub fn handle_cmd_error(cmd: Option<&Command>, err: io::Error) -> ExitCode {
pub enum CommandError {
Io(io::Error),
Failed,
}

impl From<io::Error> for CommandError {
fn from(e: io::Error) -> Self {
CommandError::Io(e)
}
}

pub fn handle_cmd_error(cmd: &Command, err: CommandError) -> ExitCode {
match (cmd, err) {
(Some(cmd), err) if err.kind() == io::ErrorKind::NotFound => {
(_, CommandError::Failed) => {
// The child process probably already wrote an error message if appropriate
}
(cmd, CommandError::Io(err)) if err.kind() == io::ErrorKind::NotFound => {
print_error(format!(
"Command not found: {}",
cmd.get_program().to_string_lossy()
));
ExitCode::GeneralError
}
(_, err) => {
print_error(format!("Problem while executing command: {}", err));
ExitCode::GeneralError
}
}
(_, CommandError::Io(err)) => print_cmd_error(err),
};
ExitCode::GeneralError
}

pub fn print_cmd_error(err: io::Error) {
print_error(format!("Problem while executing command: {}", err));
}
19 changes: 12 additions & 7 deletions src/exec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use regex::Regex;

use crate::exit_codes::ExitCode;

use self::command::{execute_commands, handle_cmd_error};
use self::command::{execute_commands, handle_cmd_error, print_cmd_error, CommandError};
use self::input::{basename, dirname, remove_extension};
pub use self::job::{batch, job};
use self::token::Token;
Expand Down Expand Up @@ -109,20 +109,23 @@ impl CommandSet {
for path in paths {
for builder in &mut builders {
if let Err(e) = builder.push(&path, path_separator) {
return handle_cmd_error(Some(&builder.cmd), e);
return handle_cmd_error(&builder.cmd, e);
}
}
}

for builder in &mut builders {
if let Err(e) = builder.finish() {
return handle_cmd_error(Some(&builder.cmd), e);
return handle_cmd_error(&builder.cmd, e);
}
}

ExitCode::Success
}
Err(e) => handle_cmd_error(None, e),
Err(e) => {
print_cmd_error(e);
ExitCode::GeneralError
}
}
}
}
Expand Down Expand Up @@ -175,7 +178,7 @@ impl CommandBuilder {
Ok(cmd)
}

fn push(&mut self, path: &Path, separator: Option<&str>) -> io::Result<()> {
fn push(&mut self, path: &Path, separator: Option<&str>) -> Result<(), CommandError> {
if self.limit > 0 && self.count >= self.limit {
self.finish()?;
}
Expand All @@ -193,10 +196,12 @@ impl CommandBuilder {
Ok(())
}

fn finish(&mut self) -> io::Result<()> {
fn finish(&mut self) -> Result<(), CommandError> {
if self.count > 0 {
self.cmd.try_args(&self.post_args)?;
self.cmd.status()?;
if !self.cmd.status()?.success() {
return Err(CommandError::Failed);
}

self.cmd = Self::new_command(&self.pre_args)?;
self.count = 0;
Expand Down
4 changes: 4 additions & 0 deletions tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1496,6 +1496,8 @@ fn test_exec_batch() {
&["foo", "--exec-batch", "echo {}"],
"[fd error]: First argument of exec-batch is expected to be a fixed executable",
);

te.assert_failure_with_error(&["a.foo", "--exec-batch", "false"], "");
}
}

Expand Down Expand Up @@ -1551,6 +1553,8 @@ fn test_exec_batch_multi() {
],
]
);

te.assert_failure_with_error(&["a.foo", "--exec-batch", "echo", ";", "false"], "");
}

#[test]
Expand Down

0 comments on commit 9b0744f

Please sign in to comment.