diff --git a/src/find/matchers/mod.rs b/src/find/matchers/mod.rs index 5a86af66..e99b5f39 100644 --- a/src/find/matchers/mod.rs +++ b/src/find/matchers/mod.rs @@ -53,8 +53,8 @@ use self::size::SizeMatcher; #[cfg(unix)] use self::stat::{InodeMatcher, LinksMatcher}; use self::time::{ - FileAgeRangeMatcher, FileTimeMatcher, FileTimeType, NewerMatcher, NewerOptionMatcher, - NewerOptionType, NewerTimeMatcher, + FileAgeRangeMatcher, FileTimeMatcher, FileTimeType, NewerOptionMatcher, NewerOptionType, + NewerTimeMatcher, }; use self::type_matcher::{TypeMatcher, XtypeMatcher}; use self::user::{NoUserMatcher, UserMatcher}; @@ -800,7 +800,7 @@ fn build_matcher_tree( return Err(missing_argument_error(args, i).into()); } i += 1; - Some(NewerMatcher::new(args[i], config.follow)?.into_box()) + Some(NewerOptionMatcher::new("m", "m", args[i], config.follow)?.into_box()) } Predicate::Time(file_time_type) => { if i >= args.len() - 1 { @@ -1177,6 +1177,13 @@ fn build_matcher_tree( "This system does not provide a way to find the birth time of a file.", )); } + #[cfg(not(unix))] + if y_option == "c" { + return Err(From::from(format!( + "{}: change times are not available on this platform", + args[i] + ))); + } if y_option == "t" { let time = args[i + 1]; let newer_time_type = NewerOptionType::from_str(x_option.as_str()); @@ -1192,7 +1199,10 @@ fn build_matcher_tree( } else { let file_path = args[i + 1]; i += 1; - Some(NewerOptionMatcher::new(&x_option, &y_option, file_path)?.into_box()) + Some( + NewerOptionMatcher::new(&x_option, &y_option, file_path, config.follow)? + .into_box(), + ) } } }; @@ -2111,6 +2121,19 @@ mod tests { } } + #[test] + #[cfg(not(unix))] + fn build_top_level_matcher_rejects_newer_change_time_not_unix() { + let mut config = Config::default(); + match build_top_level_matcher(&["-newermc", "test_data/simple"], &mut config) { + Ok(_) => panic!("-newermc needs a change time, which non-unix systems don't have"), + Err(err) => assert_eq!( + err.to_string(), + "-newermc: change times are not available on this platform" + ), + } + } + #[test] fn build_top_level_matcher_rejects_unknown_predicate() { let mut config = Config::default(); diff --git a/src/find/matchers/time.rs b/src/find/matchers/time.rs index 384c2ad3..b3343b6d 100644 --- a/src/find/matchers/time.rs +++ b/src/find/matchers/time.rs @@ -5,7 +5,7 @@ // https://opensource.org/licenses/MIT. use std::error::Error; -use std::fs::{self, Metadata}; +use std::fs::Metadata; use std::io::{stderr, Write}; use std::time::{Duration, SystemTime, UNIX_EPOCH}; @@ -34,52 +34,6 @@ fn get_time(matcher_io: &mut MatcherIO, today_start: bool) -> SystemTime { } } -/// This matcher checks whether a file is newer than the file the matcher is initialized with. -pub struct NewerMatcher { - given_modification_time: SystemTime, -} - -impl NewerMatcher { - pub fn new(path_to_file: &str, follow: Follow) -> Result> { - let metadata = follow.root_metadata(path_to_file)?; - Ok(Self { - given_modification_time: metadata.modified()?, - }) - } - - /// Implementation of matches that returns a result, allowing use to use try! - /// to deal with the errors. - fn matches_impl(&self, file_info: &WalkEntry) -> Result> { - let this_time = file_info.metadata()?.modified()?; - // duration_since returns an Ok duration if this_time <= given_modification_time - // and returns an Err (with a duration) otherwise. So if this_time > - // given_modification_time (in which case we want to return true) then - // duration_since will return an error. - Ok(self - .given_modification_time - .duration_since(this_time) - .is_err()) - } -} - -impl Matcher for NewerMatcher { - fn matches(&self, file_info: &WalkEntry, _: &mut MatcherIO) -> bool { - match self.matches_impl(file_info) { - Err(e) => { - writeln!( - &mut stderr(), - "Error getting modification time for {}: {}", - file_info.path().to_string_lossy(), - e - ) - .unwrap(); - false - } - Ok(t) => t, - } - } -} - /// `-newerXY` option. /// a is meaning Accessed time /// B is meaning Birthed time @@ -115,38 +69,31 @@ impl NewerOptionType { } } -/// This matcher checks whether the file is newer than the file time of any combination of -/// two comparison types from the target file's `NewerOptionType`. +/// Compare a candidate's X time with a reference file's Y time. pub struct NewerOptionMatcher { x_option: NewerOptionType, - y_option: NewerOptionType, - given_modification_time: SystemTime, + reference_time: SystemTime, } impl NewerOptionMatcher { - pub fn new(x_option: &str, y_option: &str, path_to_file: &str) -> Result> { - let metadata = fs::metadata(path_to_file)?; + pub fn new( + x_option: &str, + y_option: &str, + path_to_file: &str, + follow: Follow, + ) -> Result> { + let metadata = follow.root_metadata(path_to_file)?; let x_option = NewerOptionType::from_str(x_option); let y_option = NewerOptionType::from_str(y_option); Ok(Self { x_option, - y_option, - given_modification_time: metadata.modified()?, + reference_time: y_option.get_file_time(&metadata)?, }) } fn matches_impl(&self, file_info: &WalkEntry) -> Result> { let x_option_time = self.x_option.get_file_time(file_info.metadata()?)?; - let y_option_time = self.y_option.get_file_time(file_info.metadata()?)?; - - Ok(self - .given_modification_time - .duration_since(x_option_time) - .is_err() - && self - .given_modification_time - .duration_since(y_option_time) - .is_err()) + Ok(x_option_time > self.reference_time) } } @@ -156,9 +103,8 @@ impl Matcher for NewerOptionMatcher { Err(e) => { writeln!( &mut stderr(), - "Error getting {:?} and {:?} time for {}: {}", + "Error getting {:?} time for {}: {}", self.x_option, - self.y_option, file_info.path().to_string_lossy(), e ) @@ -423,13 +369,16 @@ mod tests { let new_file = get_dir_entry_for(&temp_dir_path, new_file_name); - let matcher_for_new = NewerMatcher::new( + let matcher_for_new = NewerOptionMatcher::new( + "m", + "m", &temp_dir.path().join(new_file_name).to_string_lossy(), Follow::Never, ) .unwrap(); let matcher_for_old = - NewerMatcher::new(&old_file.path().to_string_lossy(), Follow::Never).unwrap(); + NewerOptionMatcher::new("m", "m", &old_file.path().to_string_lossy(), Follow::Never) + .unwrap(); let deps = FakeDependencies::new(); assert!( @@ -716,36 +665,49 @@ mod tests { #[test] fn newer_option_matcher() { - let options = [ - "a", - #[cfg(not(target_os = "linux"))] - "B", + let temp_dir = Builder::new().prefix("newer_option_").tempdir().unwrap(); + let reference = temp_dir.path().join("reference"); + let candidate = temp_dir.path().join("candidate"); + File::create(&reference).unwrap(); + File::create(&candidate).unwrap(); + // Change and birth times cannot be set, so they stay at the current + // time, which is later than all of these. + let base = 946_684_800; + filetime::set_file_times( + &reference, + filetime::FileTime::from_unix_time(base + 100, 0), + filetime::FileTime::from_unix_time(base + 300, 0), + ) + .unwrap(); + filetime::set_file_times( + &candidate, + filetime::FileTime::from_unix_time(base + 400, 0), + filetime::FileTime::from_unix_time(base + 200, 0), + ) + .unwrap(); + let candidate_entry = get_dir_entry_for(&temp_dir.path().to_string_lossy(), "candidate"); + let reference = reference.to_string_lossy(); + + for (x_option, y_option, expected) in [ + ("a", "a", true), + ("a", "m", true), + ("m", "a", true), + ("m", "m", false), #[cfg(unix)] - "c", - "m", - ]; - - for x_option in options { - for y_option in options { - let temp_dir = Builder::new().prefix("example").tempdir().unwrap(); - let temp_dir_path = temp_dir.path().to_string_lossy(); - let new_file_name = "newFile"; - // this has just been created, so should be newer - File::create(temp_dir.path().join(new_file_name)).expect("create temp file"); - let new_file = get_dir_entry_for(&temp_dir_path, new_file_name); - // this file should already exist - let old_file = get_dir_entry_for("test_data", "simple"); - let deps = FakeDependencies::new(); - let matcher = - NewerOptionMatcher::new(x_option, y_option, &old_file.path().to_string_lossy()); - - assert!( - matcher - .unwrap() - .matches(&new_file, &mut deps.new_matcher_io()), - "new_file should be newer than old_dir" - ); - } + ("a", "c", false), + #[cfg(unix)] + ("c", "m", true), + #[cfg(not(target_os = "linux"))] + ("a", "B", false), + ] { + let deps = FakeDependencies::new(); + let matcher = + NewerOptionMatcher::new(x_option, y_option, &reference, Follow::Never).unwrap(); + assert_eq!( + matcher.matches(&candidate_entry, &mut deps.new_matcher_io()), + expected, + "-newer{x_option}{y_option}" + ); } } diff --git a/src/find/mod.rs b/src/find/mod.rs index 3b6ddd5f..5e59c40b 100644 --- a/src/find/mod.rs +++ b/src/find/mod.rs @@ -1241,7 +1241,8 @@ mod tests { &deps, ); - assert_eq!(rc, 0); + let expected_rc = i32::from(cfg!(not(unix)) && y == "c"); + assert_eq!(rc, expected_rc); // -follow and -newerXY are separate argv tokens; they must not // be glued into one string or the whole token is an unknown @@ -1258,7 +1259,7 @@ mod tests { &deps, ); - assert_eq!(rc, 0); + assert_eq!(rc, expected_rc); } } } diff --git a/tests/test_find.rs b/tests/test_find.rs index 9fc3db6d..753638e5 100644 --- a/tests/test_find.rs +++ b/tests/test_find.rs @@ -1221,6 +1221,163 @@ fn find_newer_xy() { )); } +#[test] +#[cfg(unix)] +fn find_newer_uses_reference_time_selected_by_y() { + use std::os::unix::fs::MetadataExt; + + let dir = Builder::new() + .prefix("find_newer_reference_") + .tempdir() + .unwrap(); + let candidate = dir.path().join("candidate"); + let reference = dir.path().join("reference"); + File::create(&candidate).unwrap(); + File::create(&reference).unwrap(); + + let candidate_time = filetime::FileTime::from_unix_time(946_684_802, 0); + let reference_time = filetime::FileTime::from_unix_time(946_684_801, 0); + filetime::set_file_times(&candidate, candidate_time, candidate_time).unwrap(); + filetime::set_file_times(&reference, reference_time, reference_time).unwrap(); + + for (option, should_print) in [ + ("-newer", true), + ("-anewer", true), + ("-cnewer", true), + ("-newerac", false), + ("-newermc", false), + ] { + assert_newer_result(&candidate, option, &reference, should_print); + } + + let reference_metadata = reference.metadata().unwrap(); + let after_reference_change = filetime::FileTime::from_unix_time( + reference_metadata.ctime() + 10, + reference_metadata.ctime_nsec() as u32, + ); + filetime::set_file_times(&candidate, after_reference_change, after_reference_change).unwrap(); + assert_newer_result(&candidate, "-newermc", &reference, true); + + // Changing mtime does not make the candidate's ctime old. + let old_mtime = filetime::FileTime::from_unix_time(631_152_000, 0); + filetime::set_file_times(&candidate, old_mtime, old_mtime).unwrap(); + for option in ["-cnewer", "-newercm"] { + assert_newer_result(&candidate, option, &reference, true); + } +} + +fn assert_newer_result(candidate: &Path, option: &str, reference: &Path, should_print: bool) { + let printed = format!("{}\n", candidate.display()); + ucmd() + .arg(candidate) + .arg(option) + .arg(reference) + .arg("-print") + .succeeds() + .no_stderr() + .stdout_only(if should_print { &printed } else { "" }); +} + +#[test] +fn find_newer_distinguishes_access_and_modification_times() { + let dir = Builder::new() + .prefix("find_newer_distinct_") + .tempdir() + .unwrap(); + let candidate = dir.path().join("candidate"); + let reference = dir.path().join("reference"); + File::create(&candidate).unwrap(); + File::create(&reference).unwrap(); + + let base = 946_684_800; + filetime::set_file_times( + &candidate, + filetime::FileTime::from_unix_time(base + 200, 0), + filetime::FileTime::from_unix_time(base + 300, 0), + ) + .unwrap(); + filetime::set_file_times( + &reference, + filetime::FileTime::from_unix_time(base + 350, 0), + filetime::FileTime::from_unix_time(base + 100, 0), + ) + .unwrap(); + + for (option, should_print) in [ + ("-neweraa", false), + ("-neweram", true), + ("-newerma", false), + ("-newermm", true), + ] { + assert_newer_result(&candidate, option, &reference, should_print); + } + + filetime::set_file_times( + &reference, + filetime::FileTime::from_unix_time(base + 250, 0), + filetime::FileTime::from_unix_time(base + 200, 0), + ) + .unwrap(); + for (option, should_print) in [ + ("-neweram", false), // Equal timestamps do not match. + ("-newermm", true), + ] { + assert_newer_result(&candidate, option, &reference, should_print); + } +} + +#[test] +#[cfg(unix)] +fn find_newer_reference_symlink_obeys_follow_mode() { + let dir = Builder::new().prefix("find_newer_link_").tempdir().unwrap(); + let candidate = dir.path().join("candidate"); + let reference = dir.path().join("reference"); + let link = dir.path().join("link"); + File::create(&candidate).unwrap(); + File::create(&reference).unwrap(); + let candidate_time = filetime::FileTime::from_unix_time(946_684_802, 0); + let reference_time = filetime::FileTime::from_unix_time(946_684_801, 0); + let link_time = filetime::FileTime::from_unix_time(946_684_805, 0); + filetime::set_file_times(&candidate, candidate_time, candidate_time).unwrap(); + filetime::set_file_times(&reference, reference_time, reference_time).unwrap(); + symlink(&reference, &link).unwrap(); + filetime::set_symlink_file_times(&link, link_time, link_time).unwrap(); + + let printed = format!("{}\n", candidate.display()); + for (mode, should_print) in [("-P", false), ("-H", true), ("-L", true)] { + ucmd() + .arg(mode) + .arg(&candidate) + .arg("-newermm") + .arg(&link) + .arg("-print") + .succeeds() + .no_stderr() + .stdout_only(if should_print { &printed } else { "" }); + } + + // -follow changes how references in later predicates are read, but it + // cannot retroactively change a reference in an earlier predicate. + ucmd() + .arg(&candidate) + .arg("-follow") + .arg("-newermm") + .arg(&link) + .arg("-print") + .succeeds() + .no_stderr() + .stdout_only(&printed); + ucmd() + .arg(&candidate) + .arg("-newermm") + .arg(&link) + .arg("-follow") + .arg("-print") + .succeeds() + .no_stderr() + .no_stdout(); +} + #[test] fn find_age_range() { let args = ["-amin", "-cmin", "-mmin"];