From 6b40a075cdf43a609fe9483e9c4cf531b27ab368 Mon Sep 17 00:00:00 2001 From: kimsnj Date: Mon, 12 Nov 2018 18:43:40 +0100 Subject: [PATCH] exec-batch: fix a panic with -X "echo {}" and pass stdio to child cmd --- src/exec/mod.rs | 24 +++++++++++++++--------- src/walk.rs | 2 +- tests/testenv/mod.rs | 15 ++++++--------- tests/tests.rs | 14 +++++++++++--- 4 files changed, 33 insertions(+), 22 deletions(-) diff --git a/src/exec/mod.rs b/src/exec/mod.rs index c52afc8..6b58661 100644 --- a/src/exec/mod.rs +++ b/src/exec/mod.rs @@ -14,7 +14,7 @@ mod token; use std::borrow::Cow; use std::path::{Path, PathBuf}; -use std::process::Command; +use std::process::{Command, Stdio}; use std::sync::{Arc, Mutex}; use regex::Regex; @@ -27,7 +27,7 @@ use self::token::Token; /// Execution mode of the command #[derive(Debug, Clone, Copy, PartialEq)] pub enum ExecutionMode { - /// Command is executed for each path found + /// Command is executed for each search result OneByOne, /// Command is run for a batch of results at once Batch, @@ -58,9 +58,12 @@ impl CommandTemplate { S: AsRef, { 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"); } + if cmd.args[0].has_tokens() { + return Err("First argument of exec-batch is expected to be a fixed executable"); + } Ok(cmd) } @@ -124,7 +127,7 @@ impl CommandTemplate { CommandTemplate { args, mode } } - fn tokens_number(&self) -> usize { + fn number_of_tokens(&self) -> usize { self.args.iter().filter(|arg| arg.has_tokens()).count() } @@ -151,7 +154,7 @@ impl CommandTemplate { execute_command(cmd, &out_perm) } - pub fn is_batch(&self) -> bool { + pub fn in_batch_mode(&self) -> bool { self.mode == ExecutionMode::Batch } @@ -160,6 +163,10 @@ impl CommandTemplate { I: Iterator, { 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 has_path = false; @@ -194,10 +201,9 @@ enum ArgumentTemplate { impl ArgumentTemplate { pub fn has_tokens(&self) -> bool { - if let ArgumentTemplate::Tokens(_) = self { - true - } else { - false + match self { + ArgumentTemplate::Tokens(_) => true, + _ => false, } } diff --git a/src/walk.rs b/src/walk.rs index 339b454..7943e29 100644 --- a/src/walk.rs +++ b/src/walk.rs @@ -126,7 +126,7 @@ pub fn scan(path_vec: &[PathBuf], pattern: Arc, config: Arc) { let receiver_thread = thread::spawn(move || { // This will be set to `Some` if the `--exec` argument was supplied. if let Some(ref cmd) = rx_config.command { - if cmd.is_batch() { + if cmd.in_batch_mode() { exec::batch(rx, cmd, show_filesystem_errors); } else { let shared_rx = Arc::new(Mutex::new(rx)); diff --git a/tests/testenv/mod.rs b/tests/testenv/mod.rs index 675b91b..1a6f9d9 100644 --- a/tests/testenv/mod.rs +++ b/tests/testenv/mod.rs @@ -33,7 +33,7 @@ pub struct TestEnv { /// Path to the *fd* executable. 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, } @@ -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 = line.replace('/', &std::path::MAIN_SEPARATOR.to_string()); if normalize_line { - let mut worlds: Vec<_> = line.split_whitespace().collect(); - worlds.sort(); - return worlds.join(" "); + let mut words: Vec<_> = line.split_whitespace().collect(); + words.sort(); + return words.join(" "); } line }) @@ -234,15 +234,12 @@ impl TestEnv { // Check for exit status. if output.status.success() { - panic!( - "fd exited successfully. Expected error {} did not occur.", - expected - ); + panic!("error '{}' did not occur.", expected); } // Compare actual output to expected output. 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)); } } diff --git a/tests/tests.rs b/tests/tests.rs index 9d47dcb..8659043 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -987,13 +987,11 @@ fn assert_exec_output(exec_style: &str) { } } -/// Shell script execution using -exec #[test] fn test_exec_batch() { assert_exec_batch_output("--exec-batch"); } -// Shell script execution using -x #[test] fn test_exec_batch_short_arg() { 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 = te.normalize_line(true); - // TODO Windows tests: D:file.txt \file.txt \\server\share\file.txt ... + // TODO Test for windows if !cfg!(windows) { te.assert_output( &["--absolute-path", "foo", exec_style, "echo"], @@ -1035,6 +1033,16 @@ fn assert_exec_batch_output(exec_style: &str) { &["foo", exec_style, "echo", "{/}", ";", "-x", "echo"], "error: The argument '--exec ' cannot be used with '--exec-batch '", ); + + te.assert_error( + &["foo", exec_style], + "error: The argument '--exec-batch ' 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", + ); } }