From 6d34680a856adccc73958e86d57bd5fbd187fd70 Mon Sep 17 00:00:00 2001 From: Sichen Date: Sat, 26 Sep 2026 06:02:17 +0000 Subject: [PATCH] find: compare selected timestamps for -newerXY MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Select the reference file's Y timestamp when building the matcher, then compare only the candidate's X timestamp. This fixes -newerXY when Y is not m and lets -anewer/-cnewer match even when the candidate's mtime is older than the reference mtime. Read reference symlinks according to -P/-H/-L: -P uses the link's time. Under -H/-L, reference paths to -newer, -anewer, and -cnewer "are dereferenced and the timestamp from the pointed-to file is used instead (if possible – otherwise the timestamp from the symbolic link is used)." GNU find manual, "Symbolic Links" section: https://www.gnu.org/software/findutils/manual/find.html A later -follow changes reference handling only for later predicates. On Windows, -newerXc now fails while parsing with "find: unsupported" and status 1 because change time is unavailable as a reference timestamp. --- src/find/matchers/mod.rs | 9 ++- src/find/matchers/time.rs | 158 +++++++++++++++++++++++++++++--------- src/find/mod.rs | 5 +- tests/test_find.rs | 157 +++++++++++++++++++++++++++++++++++++ 4 files changed, 290 insertions(+), 39 deletions(-) diff --git a/src/find/matchers/mod.rs b/src/find/matchers/mod.rs index a33d47c8..89c929f2 100644 --- a/src/find/matchers/mod.rs +++ b/src/find/matchers/mod.rs @@ -997,8 +997,13 @@ fn build_matcher_tree( let file_path = args[i + 1]; i += 1; Some( - NewerOptionMatcher::new(&x_option, &y_option, file_path)? - .into_box(), + NewerOptionMatcher::new( + &x_option, + &y_option, + file_path, + config.follow, + )? + .into_box(), ) } } diff --git a/src/find/matchers/time.rs b/src/find/matchers/time.rs index 384c2ad3..bc19f088 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}; @@ -115,38 +115,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 +149,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 ) @@ -725,30 +717,126 @@ mod tests { "m", ]; + let temp_dir = Builder::new().prefix("newer_option_").tempdir().unwrap(); + let temp_dir_path = temp_dir.path().to_string_lossy(); + let reference = temp_dir.path().join("reference"); + let candidate = temp_dir.path().join("candidate"); + File::create(&reference).unwrap(); + File::create(&candidate).unwrap(); + 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 + 200, 0), + filetime::FileTime::from_unix_time(base + 400, 0), + ) + .unwrap(); + let candidate_entry = get_dir_entry_for(&temp_dir_path, "candidate"); + let reference_metadata = reference.metadata().unwrap(); + let candidate_metadata = candidate_entry.metadata().unwrap(); + 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 x_time = observed_newer_time(candidate_metadata, x_option); + let y_time = observed_newer_time(&reference_metadata, y_option); + if y_time.is_none() { + assert!(NewerOptionMatcher::new( + x_option, + y_option, + &reference.to_string_lossy(), + Follow::Never, + ) + .is_err()); + } + let (Some(x_time), Some(y_time)) = (x_time, y_time) else { + eprintln!("skipping {x_option}/{y_option}: birth time unsupported"); + continue; + }; + let expected = if ["a", "m"].contains(&x_option) && ["a", "m"].contains(&y_option) { + // The explicitly set seconds put only a/m in the false case. + x_option != "a" || y_option != "m" + } else { + x_time > y_time + }; let deps = FakeDependencies::new(); - let matcher = - NewerOptionMatcher::new(x_option, y_option, &old_file.path().to_string_lossy()); - - assert!( + let matcher = NewerOptionMatcher::new( + x_option, + y_option, + &reference.to_string_lossy(), + Follow::Never, + ); + assert_eq!( matcher .unwrap() - .matches(&new_file, &mut deps.new_matcher_io()), - "new_file should be newer than old_dir" + .matches(&candidate_entry, &mut deps.new_matcher_io()), + expected, + "wrong result for {x_option}/{y_option}" ); } } } + fn observed_newer_time(metadata: &Metadata, option: &str) -> Option { + match option { + "a" => Some(metadata.accessed().expect("access time")), + "m" => Some(metadata.modified().expect("modification time")), + "c" => Some(metadata.changed().expect("change time")), + "B" => match metadata.created() { + Ok(time) => Some(time), + Err(error) if error.kind() == std::io::ErrorKind::Unsupported => None, + Err(error) => panic!("reading birth time failed: {error}"), + }, + _ => panic!("unexpected newer option: {option}"), + } + } + + #[test] + fn newer_option_uses_reference_birth_time_when_available() { + let dir = Builder::new().prefix("newer_birth_").tempdir().unwrap(); + let reference = dir.path().join("reference"); + let candidate = dir.path().join("candidate"); + File::create(&reference).unwrap(); + File::create(&candidate).unwrap(); + + let Some(birth) = observed_newer_time(&reference.metadata().unwrap(), "B") else { + assert!( + NewerOptionMatcher::new("m", "B", &reference.to_string_lossy(), Follow::Never) + .is_err() + ); + eprintln!( + "skipping birth-time comparison: this filesystem does not provide creation time" + ); + return; + }; + let between = filetime::FileTime::from_system_time(birth + Duration::from_secs(10)); + let future = filetime::FileTime::from_system_time(birth + Duration::from_secs(20)); + filetime::set_file_times(&reference, future, future).unwrap(); + filetime::set_file_times(&candidate, between, between).unwrap(); + let metadata = reference.metadata().unwrap(); + if observed_newer_time(&metadata, "B") != Some(birth) + || filetime::FileTime::from_last_modification_time(&metadata) != future + || filetime::FileTime::from_last_modification_time(&candidate.metadata().unwrap()) + != between + { + eprintln!("skipping birth-time comparison: this filesystem cannot preserve the required timestamps"); + return; + } + + let entry = get_dir_entry_for(&dir.path().to_string_lossy(), "candidate"); + let deps = FakeDependencies::new(); + let with_birth = + NewerOptionMatcher::new("m", "B", &reference.to_string_lossy(), Follow::Never).unwrap(); + let with_mtime = + NewerOptionMatcher::new("m", "m", &reference.to_string_lossy(), Follow::Never).unwrap(); + assert!(with_birth.matches(&entry, &mut deps.new_matcher_io())); + assert!(!with_mtime.matches(&entry, &mut deps.new_matcher_io())); + } + #[test] fn newer_time_matcher() { let deps = FakeDependencies::new(); diff --git a/src/find/mod.rs b/src/find/mod.rs index cbc49f7c..675d84ab 100644 --- a/src/find/mod.rs +++ b/src/find/mod.rs @@ -1224,7 +1224,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 @@ -1241,7 +1242,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 35bcb563..f29b3191 100644 --- a/tests/test_find.rs +++ b/tests/test_find.rs @@ -1150,6 +1150,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"];