exec-batch: fix a panic with -X "echo {}" and pass stdio to child cmd

This commit is contained in:
kimsnj 2018-11-12 18:43:40 +01:00 committed by David Peter
parent 45d1b15cff
commit 6b40a075cd
4 changed files with 33 additions and 22 deletions

View File

@ -14,7 +14,7 @@ mod token;
use std::borrow::Cow; use std::borrow::Cow;
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
use std::process::Command; use std::process::{Command, Stdio};
use std::sync::{Arc, Mutex}; use std::sync::{Arc, Mutex};
use regex::Regex; use regex::Regex;
@ -27,7 +27,7 @@ use self::token::Token;
/// Execution mode of the command /// Execution mode of the command
#[derive(Debug, Clone, Copy, PartialEq)] #[derive(Debug, Clone, Copy, PartialEq)]
pub enum ExecutionMode { pub enum ExecutionMode {
/// Command is executed for each path found /// Command is executed for each search result
OneByOne, OneByOne,
/// Command is run for a batch of results at once /// Command is run for a batch of results at once
Batch, Batch,
@ -58,9 +58,12 @@ impl CommandTemplate {
S: AsRef<str>, S: AsRef<str>,
{ {
let cmd = Self::build(input, ExecutionMode::Batch); let cmd = Self::build(input, ExecutionMode::Batch);
if cmd.tokens_number() > 1 { if cmd.number_of_tokens() > 1 {
return Err("Only one placeholder allowed for batch commands"); return Err("Only one placeholder allowed for batch commands");
} }
if cmd.args[0].has_tokens() {
return Err("First argument of exec-batch is expected to be a fixed executable");
}
Ok(cmd) Ok(cmd)
} }
@ -124,7 +127,7 @@ impl CommandTemplate {
CommandTemplate { args, mode } CommandTemplate { args, mode }
} }
fn tokens_number(&self) -> usize { fn number_of_tokens(&self) -> usize {
self.args.iter().filter(|arg| arg.has_tokens()).count() self.args.iter().filter(|arg| arg.has_tokens()).count()
} }
@ -151,7 +154,7 @@ impl CommandTemplate {
execute_command(cmd, &out_perm) execute_command(cmd, &out_perm)
} }
pub fn is_batch(&self) -> bool { pub fn in_batch_mode(&self) -> bool {
self.mode == ExecutionMode::Batch self.mode == ExecutionMode::Batch
} }
@ -160,6 +163,10 @@ impl CommandTemplate {
I: Iterator<Item = PathBuf>, I: Iterator<Item = PathBuf>,
{ {
let mut cmd = Command::new(self.args[0].generate("").as_ref()); let mut cmd = Command::new(self.args[0].generate("").as_ref());
cmd.stdin(Stdio::inherit());
cmd.stdout(Stdio::inherit());
cmd.stderr(Stdio::inherit());
let mut paths = paths.map(|p| Self::prepare_path(&p)); let mut paths = paths.map(|p| Self::prepare_path(&p));
let mut has_path = false; let mut has_path = false;
@ -194,10 +201,9 @@ enum ArgumentTemplate {
impl ArgumentTemplate { impl ArgumentTemplate {
pub fn has_tokens(&self) -> bool { pub fn has_tokens(&self) -> bool {
if let ArgumentTemplate::Tokens(_) = self { match self {
true ArgumentTemplate::Tokens(_) => true,
} else { _ => false,
false
} }
} }

View File

@ -126,7 +126,7 @@ pub fn scan(path_vec: &[PathBuf], pattern: Arc<Regex>, config: Arc<FdOptions>) {
let receiver_thread = thread::spawn(move || { let receiver_thread = thread::spawn(move || {
// This will be set to `Some` if the `--exec` argument was supplied. // This will be set to `Some` if the `--exec` argument was supplied.
if let Some(ref cmd) = rx_config.command { if let Some(ref cmd) = rx_config.command {
if cmd.is_batch() { if cmd.in_batch_mode() {
exec::batch(rx, cmd, show_filesystem_errors); exec::batch(rx, cmd, show_filesystem_errors);
} else { } else {
let shared_rx = Arc::new(Mutex::new(rx)); let shared_rx = Arc::new(Mutex::new(rx));

View File

@ -33,7 +33,7 @@ pub struct TestEnv {
/// Path to the *fd* executable. /// Path to the *fd* executable.
fd_exe: PathBuf, fd_exe: PathBuf,
/// Normalize each line by splitting and sorting by whitespace as well /// Normalize each line by sorting the whitespace-separated words
normalize_line: bool, normalize_line: bool,
} }
@ -133,9 +133,9 @@ fn normalize_output(s: &str, trim_left: bool, normalize_line: bool) -> String {
let line = if trim_left { line.trim_left() } else { line }; let line = if trim_left { line.trim_left() } else { line };
let line = line.replace('/', &std::path::MAIN_SEPARATOR.to_string()); let line = line.replace('/', &std::path::MAIN_SEPARATOR.to_string());
if normalize_line { if normalize_line {
let mut worlds: Vec<_> = line.split_whitespace().collect(); let mut words: Vec<_> = line.split_whitespace().collect();
worlds.sort(); words.sort();
return worlds.join(" "); return words.join(" ");
} }
line line
}) })
@ -234,15 +234,12 @@ impl TestEnv {
// Check for exit status. // Check for exit status.
if output.status.success() { if output.status.success() {
panic!( panic!("error '{}' did not occur.", expected);
"fd exited successfully. Expected error {} did not occur.",
expected
);
} }
// Compare actual output to expected output. // Compare actual output to expected output.
let actual = String::from_utf8_lossy(&output.stderr); let actual = String::from_utf8_lossy(&output.stderr);
if expected.len() <= actual.len() && expected != &actual[..expected.len()] { if !actual.starts_with(expected) {
panic!(format_output_error(args, &expected, &actual)); panic!(format_output_error(args, &expected, &actual));
} }
} }

View File

@ -987,13 +987,11 @@ fn assert_exec_output(exec_style: &str) {
} }
} }
/// Shell script execution using -exec
#[test] #[test]
fn test_exec_batch() { fn test_exec_batch() {
assert_exec_batch_output("--exec-batch"); assert_exec_batch_output("--exec-batch");
} }
// Shell script execution using -x
#[test] #[test]
fn test_exec_batch_short_arg() { fn test_exec_batch_short_arg() {
assert_exec_batch_output("-X"); assert_exec_batch_output("-X");
@ -1004,7 +1002,7 @@ fn assert_exec_batch_output(exec_style: &str) {
let (te, abs_path) = get_test_env_with_abs_path(DEFAULT_DIRS, DEFAULT_FILES); let (te, abs_path) = get_test_env_with_abs_path(DEFAULT_DIRS, DEFAULT_FILES);
let te = te.normalize_line(true); let te = te.normalize_line(true);
// TODO Windows tests: D:file.txt \file.txt \\server\share\file.txt ... // TODO Test for windows
if !cfg!(windows) { if !cfg!(windows) {
te.assert_output( te.assert_output(
&["--absolute-path", "foo", exec_style, "echo"], &["--absolute-path", "foo", exec_style, "echo"],
@ -1035,6 +1033,16 @@ fn assert_exec_batch_output(exec_style: &str) {
&["foo", exec_style, "echo", "{/}", ";", "-x", "echo"], &["foo", exec_style, "echo", "{/}", ";", "-x", "echo"],
"error: The argument '--exec <cmd>' cannot be used with '--exec-batch <cmd>'", "error: The argument '--exec <cmd>' cannot be used with '--exec-batch <cmd>'",
); );
te.assert_error(
&["foo", exec_style],
"error: The argument '--exec-batch <cmd>' requires a value but none was supplied",
);
te.assert_error(
&["foo", exec_style, "echo {}"],
"[fd error]: First argument of exec-batch is expected to be a fixed executable",
);
} }
} }