diff --git a/CHANGELOG.md b/CHANGELOG.md index 179631e33..5719ec6fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - Handle invalid working directories gracefully when using `--full-path`, see #1900 (@Xavrir). - Fire the "search pattern contains a path separator" diagnostic for any pattern containing `/`, not just patterns that happen to name an existing directory. Preserves the legacy Windows behaviour that also flags native `\` separators when the pattern resolves to a real directory. See #1873. - Fix bug where passing "-" as a directory argument didn't actually search that directory, see #849 (@Sean-Kenneth-Doherty). +- Preserve `--exec-batch` command order when batching multiple commands, see #2033 (@cyphercodes). # 10.4.2 diff --git a/src/exec/mod.rs b/src/exec/mod.rs index c22e0bd2a..ecee1c182 100644 --- a/src/exec/mod.rs +++ b/src/exec/mod.rs @@ -102,25 +102,46 @@ impl CommandSet { match builders { Ok(mut builders) => { - 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); - } - } + if builders.len() == 1 { + let builder = &mut builders[0]; + return match Self::execute_batch_builder(builder, paths, path_separator) { + Ok(exit_code) => exit_code, + Err(e) => handle_cmd_error(Some(&builder.cmd), e), + }; } + let paths: Vec<_> = paths.collect(); + let mut exit_codes = Vec::with_capacity(builders.len()); for builder in &mut builders { - if let Err(e) = builder.finish() { - return handle_cmd_error(Some(&builder.cmd), e); + match Self::execute_batch_builder(builder, paths.iter(), path_separator) { + Ok(exit_code) => exit_codes.push(exit_code), + Err(e) => return handle_cmd_error(Some(&builder.cmd), e), } } - merge_exitcodes(builders.iter().map(|b| b.exit_code())) + merge_exitcodes(exit_codes) } Err(e) => handle_cmd_error(None, e), } } + + fn execute_batch_builder( + builder: &mut CommandBuilder, + paths: I, + path_separator: Option<&str>, + ) -> io::Result + where + I: IntoIterator, + P: AsRef, + { + for path in paths { + builder.push(path.as_ref(), path_separator)?; + } + + builder.finish()?; + + Ok(builder.exit_code()) + } } /// Represents a multi-exec command as it is built. diff --git a/tests/tests.rs b/tests/tests.rs index 2a87f75a1..0d0392cb1 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -2073,6 +2073,46 @@ fn test_exec_batch_with_limit() { ); } +#[test] +fn test_exec_batch_multi_with_limit_preserves_command_order() { + // TODO Test for windows + if cfg!(windows) { + return; + } + + let te = TestEnv::new(DEFAULT_DIRS, DEFAULT_FILES); + + let output = te.assert_success_and_get_output( + ".", + &[ + "foo", + "--batch-size=2", + "--exec-batch", + "echo", + "first", + "{}", + ";", + "--exec-batch", + "echo", + "second", + "{}", + ], + ); + let stdout = String::from_utf8_lossy(&output.stdout); + let prefixes: Vec<_> = stdout + .lines() + .map(|line| line.split_whitespace().next().unwrap()) + .collect(); + + assert_eq!( + prefixes, + &["first", "first", "first", "second", "second", "second"] + ); + for line in stdout.lines() { + assert_eq!(3, line.split_whitespace().count()); + } +} + /// Shell script execution (--exec) with a custom --path-separator #[test] fn test_exec_with_separator() {