Skip to content

Commit

Permalink
exec-batch: fix a panic with -X "echo {}" and pass stdio to child cmd
Browse files Browse the repository at this point in the history
  • Loading branch information
kimsnj authored and Karim SENHAJI committed Nov 12, 2018
1 parent a20045a commit c443b49
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 19 deletions.
24 changes: 15 additions & 9 deletions src/exec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand Down Expand Up @@ -58,9 +58,12 @@ impl CommandTemplate {
S: AsRef<str>,
{
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)
}

Expand Down Expand Up @@ -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()
}

Expand All @@ -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
}

Expand All @@ -160,6 +163,10 @@ impl CommandTemplate {
I: Iterator<Item = PathBuf>,
{
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;

Expand Down Expand Up @@ -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,
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/walk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ pub fn scan(path_vec: &[PathBuf], pattern: Arc<Regex>, config: Arc<FdOptions>) {
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));
Expand Down
12 changes: 6 additions & 6 deletions tests/testenv/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand Down Expand Up @@ -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
})
Expand Down Expand Up @@ -235,14 +235,14 @@ impl TestEnv {
// Check for exit status.
if output.status.success() {
panic!(
"fd exited successfully. Expected error {} did not occur.",
"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));
}
}
Expand Down
14 changes: 11 additions & 3 deletions tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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"],
Expand Down Expand Up @@ -1035,6 +1033,16 @@ fn assert_exec_batch_output(exec_style: &str) {
&["foo", exec_style, "echo", "{/}", ";", "-x", "echo"],
"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"
);
}
}

Expand Down

0 comments on commit c443b49

Please sign in to comment.