diff --git a/Cargo.lock b/Cargo.lock index e224b8e2..90e2cf19 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -76,6 +76,17 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "argmax" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5b7e3ef5e3a7f2c5e5a49d90ad087c03d38258e75155daac64deb62c50972c66" +dependencies = [ + "lazy_static", + "libc", + "nix 0.24.3", +] + [[package]] name = "assert_cmd" version = "2.0.16" @@ -290,12 +301,13 @@ dependencies = [ name = "findutils" version = "0.7.0" dependencies = [ + "argmax", "assert_cmd", "chrono", "clap", "faccess", "filetime", - "nix", + "nix 0.29.0", "onig", "predicates", "pretty_assertions", @@ -449,6 +461,12 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "lazy_static" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bbd2bcb4c963f2ddae06a2efc7e9f3591312473c50c6685e1f298068316e66fe" + [[package]] name = "libc" version = "0.2.169" @@ -503,6 +521,17 @@ version = "2.7.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6c8640c5d730cb13ebd907d8d04b52f55ac9a2eec55b440c8892f40d56c76c1d" +[[package]] +name = "nix" +version = "0.24.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fa52e972a9a719cecb6864fb88568781eb706bac2cd1d4f04a648542dbf78069" +dependencies = [ + "bitflags 1.3.2", + "cfg-if", + "libc", +] + [[package]] name = "nix" version = "0.29.0" @@ -960,7 +989,7 @@ dependencies = [ "glob", "iana-time-zone", "libc", - "nix", + "nix 0.29.0", "number_prefix", "os_display", "uucore_procs", diff --git a/Cargo.toml b/Cargo.toml index b7078761..62d3cc23 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,6 +18,7 @@ regex = "1.11" onig = { version = "6.4", default-features = false } uucore = { version = "0.0.30", features = ["entries", "fs", "fsext", "mode"] } nix = { version = "0.29", features = ["fs", "user"] } +argmax = "0.3.1" [dev-dependencies] assert_cmd = "2" diff --git a/src/find/matchers/exec.rs b/src/find/matchers/exec.rs index 23055e06..51337953 100644 --- a/src/find/matchers/exec.rs +++ b/src/find/matchers/exec.rs @@ -4,6 +4,7 @@ // license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. +use std::cell::RefCell; use std::error::Error; use std::ffi::OsString; use std::io::{stderr, Write}; @@ -97,6 +98,125 @@ impl Matcher for SingleExecMatcher { } } +pub struct MultiExecMatcher { + executable: String, + args: Vec, + exec_in_parent_dir: bool, + /// Command to build while matching. + command: RefCell>, +} + +impl MultiExecMatcher { + pub fn new( + executable: &str, + args: &[&str], + exec_in_parent_dir: bool, + ) -> Result> { + let transformed_args = args.iter().map(OsString::from).collect(); + + Ok(Self { + executable: executable.to_string(), + args: transformed_args, + exec_in_parent_dir, + command: RefCell::new(None), + }) + } + + fn new_command(&self) -> argmax::Command { + let mut command = argmax::Command::new(&self.executable); + command.try_args(&self.args).unwrap(); + command + } + + fn run_command(&self, command: &mut argmax::Command, matcher_io: &mut MatcherIO) { + match command.status() { + Ok(status) => { + if !status.success() { + matcher_io.set_exit_code(1); + } + } + Err(e) => { + writeln!(&mut stderr(), "Failed to run {}: {}", self.executable, e).unwrap(); + matcher_io.set_exit_code(1); + } + } + } +} + +impl Matcher for MultiExecMatcher { + fn matches(&self, file_info: &WalkEntry, matcher_io: &mut MatcherIO) -> bool { + let path_to_file = if self.exec_in_parent_dir { + if let Some(f) = file_info.path().file_name() { + Path::new(".").join(f) + } else { + Path::new(".").join(file_info.path()) + } + } else { + file_info.path().to_path_buf() + }; + let mut command = self.command.borrow_mut(); + let command = command.get_or_insert_with(|| self.new_command()); + + // Build command, or dispatch it before when it is long enough. + if command.try_arg(&path_to_file).is_err() { + if self.exec_in_parent_dir { + match file_info.path().parent() { + None => { + // Root paths like "/" have no parent. Run them from the root to match GNU find. + command.current_dir(file_info.path()); + } + Some(parent) if parent == Path::new("") => { + // Paths like "foo" have a parent of "". Avoid chdir(""). + } + Some(parent) => { + command.current_dir(parent); + } + } + } + self.run_command(command, matcher_io); + + // Reset command status. + *command = self.new_command(); + if let Err(e) = command.try_arg(&path_to_file) { + writeln!( + &mut stderr(), + "Cannot fit a single argument {}: {}", + &path_to_file.to_string_lossy(), + e + ) + .unwrap(); + matcher_io.set_exit_code(1); + } + } + true + } + + fn finished_dir(&self, dir: &Path, matcher_io: &mut MatcherIO) { + // Dispatch command for -execdir. + if self.exec_in_parent_dir { + let mut command = self.command.borrow_mut(); + if let Some(mut command) = command.take() { + command.current_dir(Path::new(".").join(dir)); + self.run_command(&mut command, matcher_io); + } + } + } + + fn finished(&self, matcher_io: &mut MatcherIO) { + // Dispatch command for -exec. + if !self.exec_in_parent_dir { + let mut command = self.command.borrow_mut(); + if let Some(mut command) = command.take() { + self.run_command(&mut command, matcher_io); + } + } + } + + fn has_side_effects(&self) -> bool { + true + } +} + #[cfg(test)] /// No tests here, because we need to call out to an external executable. See /// `tests/exec_unit_tests.rs` instead. diff --git a/src/find/matchers/logical_matchers.rs b/src/find/matchers/logical_matchers.rs index f2067da9..a892e12c 100644 --- a/src/find/matchers/logical_matchers.rs +++ b/src/find/matchers/logical_matchers.rs @@ -51,15 +51,15 @@ impl Matcher for AndMatcher { .any(super::Matcher::has_side_effects) } - fn finished_dir(&self, dir: &Path) { + fn finished_dir(&self, dir: &Path, matcher_io: &mut MatcherIO) { for m in &self.submatchers { - m.finished_dir(dir); + m.finished_dir(dir, matcher_io); } } - fn finished(&self) { + fn finished(&self, matcher_io: &mut MatcherIO) { for m in &self.submatchers { - m.finished(); + m.finished(matcher_io); } } } @@ -127,15 +127,15 @@ impl Matcher for OrMatcher { .any(super::Matcher::has_side_effects) } - fn finished_dir(&self, dir: &Path) { + fn finished_dir(&self, dir: &Path, matcher_io: &mut MatcherIO) { for m in &self.submatchers { - m.finished_dir(dir); + m.finished_dir(dir, matcher_io); } } - fn finished(&self) { + fn finished(&self, matcher_io: &mut MatcherIO) { for m in &self.submatchers { - m.finished(); + m.finished(matcher_io); } } } @@ -222,15 +222,15 @@ impl Matcher for ListMatcher { .any(super::Matcher::has_side_effects) } - fn finished_dir(&self, dir: &Path) { + fn finished_dir(&self, dir: &Path, matcher_io: &mut MatcherIO) { for m in &self.submatchers { - m.finished_dir(dir); + m.finished_dir(dir, matcher_io); } } - fn finished(&self) { + fn finished(&self, matcher_io: &mut MatcherIO) { for m in &self.submatchers { - m.finished(); + m.finished(matcher_io); } } } @@ -346,12 +346,12 @@ impl Matcher for NotMatcher { self.submatcher.has_side_effects() } - fn finished_dir(&self, dir: &Path) { - self.submatcher.finished_dir(dir); + fn finished_dir(&self, dir: &Path, matcher_io: &mut MatcherIO) { + self.submatcher.finished_dir(dir, matcher_io); } - fn finished(&self) { - self.submatcher.finished(); + fn finished(&self, matcher_io: &mut MatcherIO) { + self.submatcher.finished(matcher_io); } } diff --git a/src/find/matchers/mod.rs b/src/find/matchers/mod.rs index e27b0e8e..7e192e7a 100644 --- a/src/find/matchers/mod.rs +++ b/src/find/matchers/mod.rs @@ -34,7 +34,7 @@ mod user; use self::access::AccessMatcher; use self::delete::DeleteMatcher; use self::empty::EmptyMatcher; -use self::exec::SingleExecMatcher; +use self::exec::{MultiExecMatcher, SingleExecMatcher}; use self::group::{GroupMatcher, NoGroupMatcher}; use self::lname::LinkNameMatcher; use self::logical_matchers::{ @@ -215,13 +215,13 @@ pub trait Matcher: 'static { false } - /// Notification that find has finished processing a given directory. - fn finished_dir(&self, _finished_directory: &Path) {} + /// Notification that find is leaving a given directory. + fn finished_dir(&self, _finished_directory: &Path, _matcher_io: &mut MatcherIO) {} /// Notification that find has finished processing all directories - /// allowing for any cleanup that isn't suitable for destructors (e.g. /// blocking calls, I/O etc.) - fn finished(&self) {} + fn finished(&self, _matcher_io: &mut MatcherIO) {} } impl Matcher for Box { @@ -237,12 +237,12 @@ impl Matcher for Box { (**self).has_side_effects() } - fn finished_dir(&self, finished_directory: &Path) { - (**self).finished_dir(finished_directory); + fn finished_dir(&self, finished_directory: &Path, matcher_io: &mut MatcherIO) { + (**self).finished_dir(finished_directory, matcher_io); } - fn finished(&self) { - (**self).finished(); + fn finished(&self, matcher_io: &mut MatcherIO) { + (**self).finished(matcher_io); } } @@ -623,29 +623,49 @@ fn build_matcher_tree( "-empty" => Some(EmptyMatcher::new().into_box()), "-exec" | "-execdir" => { let mut arg_index = i + 1; - while arg_index < args.len() && args[arg_index] != ";" { - if args[arg_index - 1] == "{}" && args[arg_index] == "+" { - // MultiExecMatcher isn't written yet - return Err(From::from(format!( - "{} [args...] + isn't supported yet. \ - Only {} [args...] ;", - args[i], args[i] - ))); - } + while arg_index < args.len() + && args[arg_index] != ";" + && (args[arg_index - 1] != "{}" || args[arg_index] != "+") + { arg_index += 1; } - if arg_index < i + 2 || arg_index == args.len() { + let required_arg = if arg_index < args.len() && args[arg_index] == "+" { + 3 + } else { + 2 + }; + if arg_index < i + required_arg || arg_index == args.len() { // at the minimum we need the executable and the ';' + // or the executable and the '{} +' return Err(From::from(format!("missing argument to {}", args[i]))); } let expression = args[i]; let executable = args[i + 1]; let exec_args = &args[i + 2..arg_index]; i = arg_index; - Some( - SingleExecMatcher::new(executable, exec_args, expression == "-execdir")? - .into_box(), - ) + match args[arg_index] { + ";" => Some( + SingleExecMatcher::new(executable, exec_args, expression == "-execdir")? + .into_box(), + ), + "+" => { + if exec_args.iter().filter(|x| matches!(**x, "{}")).count() == 1 { + Some( + MultiExecMatcher::new( + executable, + &exec_args[0..exec_args.len() - 1], + expression == "-execdir", + )? + .into_box(), + ) + } else { + return Err(From::from( + "Only one instance of {} is supported with -execdir ... +", + )); + } + } + _ => unreachable!("Encountered unexpected value {}", args[arg_index]), + } } #[cfg(unix)] "-inum" => { @@ -1576,6 +1596,30 @@ mod tests { } else { panic!("parsing argument list with exec and no executable should fail"); } + + if let Err(e) = build_top_level_matcher(&["-exec", "+"], &mut config) { + assert!(e.to_string().contains("missing argument")); + } else { + panic!("parsing argument list with exec and no executable should fail"); + } + + if let Err(e) = build_top_level_matcher(&["-exec", "foo", "+"], &mut config) { + assert!(e.to_string().contains("missing argument")); + } else { + panic!("parsing argument list with exec and no brackets should fail"); + } + + if let Err(e) = build_top_level_matcher(&["-exec", "{}", "+"], &mut config) { + assert!(e.to_string().contains("missing argument")); + } else { + panic!("parsing argument list with exec and no executable should fail"); + } + + if let Err(e) = build_top_level_matcher(&["-exec", "foo", "{}", "foo", "+"], &mut config) { + assert!(e.to_string().contains("missing argument")); + } else { + panic!("parsing argument list with exec and + not following {{}} should fail"); + } } #[test] @@ -1592,6 +1636,18 @@ mod tests { .expect("only {} + should be considered a multi-exec"); } + #[test] + fn build_top_level_multi_exec_too_many_holders() { + let mut config = Config::default(); + if let Err(e) = + build_top_level_matcher(&["-exec", "foo", "{}", "foo", "{}", "+", ";"], &mut config) + { + assert!(e.to_string().contains("Only one instance of {}")); + } else { + panic!("parsing argument list with more than one {{}} for + should fail"); + } + } + #[test] #[cfg(unix)] fn build_top_level_matcher_perm() { diff --git a/src/find/mod.rs b/src/find/mod.rs index 36b128dd..f3519aad 100644 --- a/src/find/mod.rs +++ b/src/find/mod.rs @@ -10,6 +10,7 @@ use matchers::{Follow, WalkEntry}; use std::cell::RefCell; use std::error::Error; use std::io::{stderr, stdout, Write}; +use std::path::PathBuf; use std::rc::Rc; use std::time::SystemTime; use walkdir::WalkDir; @@ -175,6 +176,9 @@ fn process_dir( // Slightly yucky loop handling here :-(. See docs for // WalkDirIterator::skip_current_dir for explanation. let mut it = walkdir.into_iter(); + // As WalkDir seems not providing a function to check its stack, + // using current_dir is a workaround to check leaving directory. + let mut current_dir: Option = None; while let Some(result) = it.next() { match WalkEntry::from_walkdir(result, config.follow) { Err(err) => { @@ -184,6 +188,14 @@ fn process_dir( Ok(entry) => { let mut matcher_io = matchers::MatcherIO::new(deps); + let new_dir = entry.path().parent().map(|x| x.to_path_buf()); + if new_dir != current_dir { + if let Some(dir) = current_dir.take() { + matcher.finished_dir(dir.as_path(), &mut matcher_io); + } + current_dir = new_dir; + } + matcher.matches(&entry, &mut matcher_io); match matcher_io.exit_code() { 0 => {} @@ -200,6 +212,17 @@ fn process_dir( } } + let mut matcher_io = matchers::MatcherIO::new(deps); + if let Some(dir) = current_dir.take() { + matcher.finished_dir(dir.as_path(), &mut matcher_io); + } + matcher.finished(&mut matcher_io); + // This is implemented for exec +. + match matcher_io.exit_code() { + 0 => {} + code => ret = code, + } + ret } diff --git a/tests/exec_unit_tests.rs b/tests/exec_unit_tests.rs index 7f1502c4..e952e405 100644 --- a/tests/exec_unit_tests.rs +++ b/tests/exec_unit_tests.rs @@ -11,13 +11,14 @@ use std::env; use std::fs::File; use std::io::Read; +use std::path::Path; use tempfile::Builder; use common::test_helpers::{ fix_up_slashes, get_dir_entry_for, path_to_testing_commandline, FakeDependencies, }; -use findutils::find::matchers::exec::SingleExecMatcher; -use findutils::find::matchers::Matcher; +use findutils::find::matchers::exec::{MultiExecMatcher, SingleExecMatcher}; +use findutils::find::matchers::{Matcher, MatcherIO}; mod common; @@ -223,3 +224,115 @@ fn matching_fails_if_executable_fails() { )) ); } + +#[test] +fn matching_multi_executes_code() { + let temp_dir = Builder::new() + .prefix("matching_executes_code") + .tempdir() + .unwrap(); + let temp_dir_path = temp_dir.path().to_string_lossy(); + + let abbbc = get_dir_entry_for("test_data/simple", "abbbc"); + let matcher = MultiExecMatcher::new( + &path_to_testing_commandline(), + &[temp_dir_path.as_ref(), "abc"], + false, + ) + .expect("Failed to create matcher"); + let deps = FakeDependencies::new(); + let mut matcher_io = MatcherIO::new(&deps); + assert!(matcher.matches(&abbbc, &mut deps.new_matcher_io())); + matcher.finished(&mut matcher_io); + + let mut f = File::open(temp_dir.path().join("1.txt")).expect("Failed to open output file"); + let mut s = String::new(); + f.read_to_string(&mut s) + .expect("failed to read output file"); + assert_eq!( + s, + fix_up_slashes(&format!( + "cwd={}\nargs=\nabc\ntest_data/simple/abbbc\n", + env::current_dir().unwrap().to_string_lossy() + )) + ); +} + +#[test] +fn execdir_multi_in_current_directory() { + let temp_dir = Builder::new() + .prefix("execdir_in_current_directory") + .tempdir() + .unwrap(); + let temp_dir_path = temp_dir.path().to_string_lossy(); + + let current_dir_entry = get_dir_entry_for(".", ""); + let matcher = MultiExecMatcher::new( + &path_to_testing_commandline(), + &[temp_dir_path.as_ref(), "abc"], + true, + ) + .expect("Failed to create matcher"); + let deps = FakeDependencies::new(); + let mut matcher_io = MatcherIO::new(&deps); + assert!(matcher.matches(¤t_dir_entry, &mut deps.new_matcher_io())); + matcher.finished_dir(Path::new(""), &mut matcher_io); + matcher.finished(&mut matcher_io); + + let mut f = File::open(temp_dir.path().join("1.txt")).expect("Failed to open output file"); + let mut s = String::new(); + f.read_to_string(&mut s) + .expect("failed to read output file"); + assert_eq!( + s, + fix_up_slashes(&format!( + "cwd={}\nargs=\nabc\n./.\n", + env::current_dir().unwrap().to_string_lossy() + )) + ); +} + +#[test] +fn multi_set_exit_code_if_executable_fails() { + let temp_dir = Builder::new() + .prefix("multi_set_exit_code_if_executable_fails") + .tempdir() + .unwrap(); + let temp_dir_path = temp_dir.path().to_string_lossy(); + + let abbbc = get_dir_entry_for("test_data/simple", "abbbc"); + let matcher = MultiExecMatcher::new( + &path_to_testing_commandline(), + &[temp_dir_path.as_ref(), "--exit_with_failure", "abc"], + true, + ) + .expect("Failed to create matcher"); + let deps = FakeDependencies::new(); + assert!(matcher.matches(&abbbc, &mut deps.new_matcher_io())); + let mut matcher_io = deps.new_matcher_io(); + matcher.finished_dir(Path::new("test_data/simple"), &mut matcher_io); + assert!(matcher_io.exit_code() == 1); + + let mut f = File::open(temp_dir.path().join("1.txt")).expect("Failed to open output file"); + let mut s = String::new(); + f.read_to_string(&mut s) + .expect("failed to read output file"); + assert_eq!( + s, + fix_up_slashes(&format!( + "cwd={}/test_data/simple\nargs=\n--exit_with_failure\nabc\n./abbbc\n", + env::current_dir().unwrap().to_string_lossy() + )) + ); +} + +#[test] +fn multi_set_exit_code_if_command_fails() { + let abbbc = get_dir_entry_for("test_data/simple", "abbbc"); + let matcher = MultiExecMatcher::new("1337", &["abc"], true).expect("Failed to create matcher"); + let deps = FakeDependencies::new(); + assert!(matcher.matches(&abbbc, &mut deps.new_matcher_io())); + let mut matcher_io = deps.new_matcher_io(); + matcher.finished_dir(Path::new("test_data/simple"), &mut matcher_io); + assert!(matcher_io.exit_code() == 1); +} diff --git a/tests/find_exec_tests.rs b/tests/find_exec_tests.rs index 8cf81703..758ee0ff 100644 --- a/tests/find_exec_tests.rs +++ b/tests/find_exec_tests.rs @@ -105,3 +105,113 @@ fn find_execdir() { )) ); } + +#[test] +fn find_exec_multi() { + let temp_dir = tempfile::Builder::new() + .prefix("find_exec_multi") + .tempdir() + .unwrap(); + let temp_dir_path = temp_dir.path().to_string_lossy(); + let deps = FakeDependencies::new(); + + let rc = find_main( + &[ + "find", + &fix_up_slashes("./test_data/simple"), + "-type", + "f", + "-exec", + &path_to_testing_commandline(), + temp_dir_path.as_ref(), + "(", + "-o", + "{}", + "+", + ], + &deps, + ); + + assert_eq!(rc, 0); + // exec has side effects, so we won't output anything unless -print is + // explicitly passed in. + assert_eq!(deps.get_output_as_string(), ""); + + // check the executable ran as expected + let mut f = File::open(temp_dir.path().join("1.txt")).expect("Failed to open output file"); + let mut s = String::new(); + f.read_to_string(&mut s) + .expect("failed to read output file"); + assert_eq!( + s, + fix_up_slashes(&format!( + "cwd={}\nargs=\n(\n-o\n./test_data/simple/abbbc\n./test_data/simple/subdir/ABBBC\n", + env::current_dir().unwrap().to_string_lossy() + )) + ); +} + +#[test] +fn find_execdir_multi() { + let temp_dir = Builder::new() + .prefix("find_execdir_multi") + .tempdir() + .unwrap(); + let temp_dir_path = temp_dir.path().to_string_lossy(); + let deps = FakeDependencies::new(); + // only look at files because the "size" of a directory is a system (and filesystem) + // dependent thing and we want these tests to be universal. + let rc = find_main( + &[ + "find", + &fix_up_slashes("./test_data/simple"), + "-execdir", + &path_to_testing_commandline(), + temp_dir_path.as_ref(), + ")", + "{}", + "+", + ], + &deps, + ); + + assert_eq!(rc, 0); + // exec has side effects, so we won't output anything unless -print is + // explicitly passed in. + assert_eq!(deps.get_output_as_string(), ""); + + // check the executable ran as expected + let mut f = File::open(temp_dir.path().join("1.txt")).expect("Failed to open output file"); + let mut s = String::new(); + f.read_to_string(&mut s) + .expect("failed to read output file"); + assert_eq!( + s, + fix_up_slashes(&format!( + "cwd={}/test_data\nargs=\n)\n./simple\n", + env::current_dir().unwrap().to_string_lossy() + )) + ); + let mut f = File::open(temp_dir.path().join("2.txt")).expect("Failed to open output file"); + let mut s = String::new(); + f.read_to_string(&mut s) + .expect("failed to read output file"); + assert_eq!( + s, + fix_up_slashes(&format!( + "cwd={}/test_data/simple\nargs=\n)\n./abbbc\n./subdir\n", + env::current_dir().unwrap().to_string_lossy() + )) + ); + let mut f = File::open(temp_dir.path().join("3.txt")).expect("Failed to open output file"); + let mut s = String::new(); + f.read_to_string(&mut s) + .expect("failed to read output file"); + assert_eq!( + s, + fix_up_slashes(&format!( + "cwd={}/test_data/simple/subdir\nargs=\n)\n./ABBBC\n", + env::current_dir().unwrap().to_string_lossy() + )) + ); +}