From 4807264faadfdef773efff1a2457c5d174b31663 Mon Sep 17 00:00:00 2001 From: Tavian Barnes Date: Wed, 17 Jul 2024 09:14:00 -0400 Subject: [PATCH] find: ctime is "changed" time, not "created" time (#416) Fixes #358 --- src/find/matchers/mod.rs | 6 +- src/find/matchers/time.rs | 125 +++++++++++++++++++++++++------------- src/find/mod.rs | 18 +++--- tests/find_cmd_tests.rs | 33 +++++++--- 4 files changed, 118 insertions(+), 64 deletions(-) diff --git a/src/find/matchers/mod.rs b/src/find/matchers/mod.rs index b6a35453..92dcc01e 100644 --- a/src/find/matchers/mod.rs +++ b/src/find/matchers/mod.rs @@ -25,7 +25,7 @@ mod samefile; mod size; #[cfg(unix)] mod stat; -mod time; +pub mod time; mod type_matcher; mod user; @@ -448,7 +448,7 @@ fn build_matcher_tree( } let file_time_type = match args[i] { "-atime" => FileTimeType::Accessed, - "-ctime" => FileTimeType::Created, + "-ctime" => FileTimeType::Changed, "-mtime" => FileTimeType::Modified, // This shouldn't be possible. We've already checked the value // is one of those three values. @@ -464,7 +464,7 @@ fn build_matcher_tree( } let file_time_type = match args[i] { "-amin" => FileTimeType::Accessed, - "-cmin" => FileTimeType::Created, + "-cmin" => FileTimeType::Changed, "-mmin" => FileTimeType::Modified, _ => unreachable!("Encountered unexpected value {}", args[i]), }; diff --git a/src/find/matchers/time.rs b/src/find/matchers/time.rs index d876058d..18e1f6af 100644 --- a/src/find/matchers/time.rs +++ b/src/find/matchers/time.rs @@ -10,6 +10,9 @@ use std::io::{stderr, Write}; use std::time::{SystemTime, UNIX_EPOCH}; use walkdir::DirEntry; +#[cfg(unix)] +use std::os::unix::fs::MetadataExt; + use super::{ComparableValue, Matcher, MatcherIO}; const SECONDS_PER_DAY: i64 = 60 * 60 * 24; @@ -75,6 +78,7 @@ pub enum NewerOptionType { } impl NewerOptionType { + #[allow(clippy::should_implement_trait)] pub fn from_str(option: &str) -> Self { match option { "a" => NewerOptionType::Accessed, @@ -88,8 +92,7 @@ impl NewerOptionType { match self { NewerOptionType::Accessed => metadata.accessed(), NewerOptionType::Birthed => metadata.created(), - // metadata.ctime() only impl in MetadataExt - NewerOptionType::Changed => metadata.accessed(), + NewerOptionType::Changed => metadata.changed(), NewerOptionType::Modified => metadata.modified(), } } @@ -204,10 +207,39 @@ impl Matcher for NewerTimeMatcher { } } +/// Provide access to the *change* timestamp, since std::fs::Metadata doesn't expose it. +pub trait ChangeTime { + /// Returns the time of the last change to the metadata. + fn changed(&self) -> std::io::Result; +} + +#[cfg(unix)] +impl ChangeTime for Metadata { + fn changed(&self) -> std::io::Result { + let ctime_sec = self.ctime(); + let ctime_nsec = self.ctime_nsec() as u32; + let ctime = if ctime_sec >= 0 { + UNIX_EPOCH + std::time::Duration::new(ctime_sec as u64, ctime_nsec) + } else { + UNIX_EPOCH - std::time::Duration::new(-ctime_sec as u64, ctime_nsec) + }; + Ok(ctime) + } +} + +#[cfg(not(unix))] +impl ChangeTime for Metadata { + fn changed(&self) -> std::io::Result { + // Rust's stdlib doesn't (yet) expose ChangeTime on Windows + // https://github.com/rust-lang/rust/issues/121478 + Err(std::io::Error::from(std::io::ErrorKind::Unsupported)) + } +} + #[derive(Clone, Copy, Debug)] pub enum FileTimeType { Accessed, - Created, + Changed, Modified, } @@ -215,7 +247,7 @@ impl FileTimeType { fn get_file_time(self, metadata: Metadata) -> std::io::Result { match self { FileTimeType::Accessed => metadata.accessed(), - FileTimeType::Created => metadata.created(), + FileTimeType::Changed => metadata.changed(), FileTimeType::Modified => metadata.modified(), } } @@ -469,9 +501,9 @@ mod tests { } #[test] - fn file_time_matcher_modified_created_accessed() { + fn file_time_matcher_modified_changed_accessed() { let temp_dir = Builder::new() - .prefix("file_time_matcher_modified_created_accessed") + .prefix("file_time_matcher_modified_changed_accessed") .tempdir() .unwrap(); @@ -521,8 +553,8 @@ mod tests { test_matcher_for_file_time_type(&file_info, accessed_time, FileTimeType::Accessed); } - if let Ok(creation_time) = metadata.created() { - test_matcher_for_file_time_type(&file_info, creation_time, FileTimeType::Created); + if let Ok(creation_time) = metadata.changed() { + test_matcher_for_file_time_type(&file_info, creation_time, FileTimeType::Changed); } if let Ok(modified_time) = metadata.modified() { @@ -530,7 +562,7 @@ mod tests { } } - /// helper function for `file_time_matcher_modified_created_accessed` + /// helper function for `file_time_matcher_modified_changed_accessed` fn test_matcher_for_file_time_type( file_info: &DirEntry, file_time: SystemTime, @@ -556,10 +588,14 @@ mod tests { #[test] fn newer_option_matcher() { - #[cfg(target_os = "linux")] - let options = ["a", "c", "m"]; - #[cfg(not(target_os = "linux"))] - let options = ["a", "B", "c", "m"]; + let options = [ + "a", + #[cfg(not(target_os = "linux"))] + "B", + #[cfg(unix)] + "c", + "m", + ]; for x_option in options { for y_option in options { @@ -669,32 +705,34 @@ mod tests { "file modified time should after 'time'" ); - let inode_changed_matcher = NewerTimeMatcher::new(NewerOptionType::Changed, time); - // Steps to change inode: - // 1. Copy and rename the file - // 2. Delete the old file - // 3. Change the new file name to the old file name - let _ = File::create(temp_dir.path().join("inode_test_file")).expect("create temp file"); - let _ = fs::copy("inode_test_file", "new_inode_test_file"); - let _ = fs::remove_file("inode_test_file"); - let _ = fs::rename("new_inode_test_file", "inode_test_file"); - let file_info = get_dir_entry_for(&temp_dir.path().to_string_lossy(), "inode_test_file"); - assert!( - inode_changed_matcher.matches(&file_info, &mut deps.new_matcher_io()), - "file inode changed time should after 'std_time'" - ); - - // After the file is deleted, DirEntry will point to an empty file location, - // thus causing the Matcher to generate an IO error after matching. - // - // Note: This test is nondeterministic on Windows, - // because fs::remove_file may not actually remove the file from - // the file system even if it returns Ok. - // Therefore, this test will only be performed on Linux/Unix. - let _ = fs::remove_file(&*file_info.path().to_string_lossy()); - #[cfg(unix)] { + let inode_changed_matcher = NewerTimeMatcher::new(NewerOptionType::Changed, time); + // Steps to change inode: + // 1. Copy and rename the file + // 2. Delete the old file + // 3. Change the new file name to the old file name + let _ = + File::create(temp_dir.path().join("inode_test_file")).expect("create temp file"); + let _ = fs::copy("inode_test_file", "new_inode_test_file"); + let _ = fs::remove_file("inode_test_file"); + let _ = fs::rename("new_inode_test_file", "inode_test_file"); + let file_info = + get_dir_entry_for(&temp_dir.path().to_string_lossy(), "inode_test_file"); + assert!( + inode_changed_matcher.matches(&file_info, &mut deps.new_matcher_io()), + "file inode changed time should after 'std_time'" + ); + + // After the file is deleted, DirEntry will point to an empty file location, + // thus causing the Matcher to generate an IO error after matching. + // + // Note: This test is nondeterministic on Windows, + // because fs::remove_file may not actually remove the file from + // the file system even if it returns Ok. + // Therefore, this test will only be performed on Linux/Unix. + let _ = fs::remove_file(&*file_info.path().to_string_lossy()); + let matchers = [ &created_matcher, &accessed_matcher, @@ -728,7 +766,7 @@ mod tests { // Means to find files accessed / modified more than 1 minute ago. [ FileTimeType::Accessed, - FileTimeType::Created, + FileTimeType::Changed, FileTimeType::Modified, ] .iter() @@ -738,10 +776,10 @@ mod tests { !more_matcher.matches(&new_file, &mut FakeDependencies::new().new_matcher_io()), "{}", format!( - "more minutes old file should match more than 1 minute old in {} test.", + "more minutes old file should not match more than 1 minute old in {} test.", match *time_type { FileTimeType::Accessed => "accessed", - FileTimeType::Created => "created", + FileTimeType::Changed => "changed", FileTimeType::Modified => "modified", } ) @@ -756,7 +794,8 @@ mod tests { // Means to find files accessed / modified less than 1 minute ago. [ FileTimeType::Accessed, - FileTimeType::Created, + #[cfg(unix)] + FileTimeType::Changed, FileTimeType::Modified, ] .iter() @@ -766,10 +805,10 @@ mod tests { less_matcher.matches(&new_file, &mut FakeDependencies::new().new_matcher_io()), "{}", format!( - "less minutes old file should not match less than 1 minute old in {} test.", + "less minutes old file should match less than 1 minute old in {} test.", match *time_type { FileTimeType::Accessed => "accessed", - FileTimeType::Created => "created", + FileTimeType::Changed => "changed", FileTimeType::Modified => "modified", } ) diff --git a/src/find/mod.rs b/src/find/mod.rs index 49832016..3d012199 100644 --- a/src/find/mod.rs +++ b/src/find/mod.rs @@ -285,6 +285,7 @@ mod tests { #[cfg(windows)] use std::os::windows::fs::symlink_file; + use crate::find::matchers::time::ChangeTime; use crate::find::matchers::MatcherIO; use super::*; @@ -673,7 +674,7 @@ mod tests { let meta = fs::metadata("./test_data/simple/subdir/ABBBC").unwrap(); // metadata can return errors like StringError("creation time is not available on this platform currently") - // so skip tests that won't pass due to shortcomings in std:;fs. + // so skip tests that won't pass due to shortcomings in std::fs. if let Ok(file_time) = meta.modified() { file_time_helper(file_time, "-mtime"); } @@ -684,8 +685,8 @@ mod tests { let meta = fs::metadata("./test_data/simple/subdir/ABBBC").unwrap(); // metadata can return errors like StringError("creation time is not available on this platform currently") - // so skip tests that won't pass due to shortcomings in std:;fs. - if let Ok(file_time) = meta.created() { + // so skip tests that won't pass due to shortcomings in std::fs. + if let Ok(file_time) = meta.changed() { file_time_helper(file_time, "-ctime"); } } @@ -695,7 +696,7 @@ mod tests { let meta = fs::metadata("./test_data/simple/subdir/ABBBC").unwrap(); // metadata can return errors like StringError("creation time is not available on this platform currently") - // so skip tests that won't pass due to shortcomings in std:;fs. + // so skip tests that won't pass due to shortcomings in std::fs. if let Ok(file_time) = meta.accessed() { file_time_helper(file_time, "-atime"); } @@ -943,9 +944,10 @@ mod tests { } } + #[cfg(unix)] #[test] - fn test_find_newer_xy_before_created_time() { - // normal - before the created time + fn test_find_newer_xy_before_changed_time() { + // normal - before the changed time #[cfg(target_os = "linux")] let args = ["-newerat", "-newerct", "-newermt"]; #[cfg(not(target_os = "linux"))] @@ -967,8 +969,8 @@ mod tests { } #[test] - fn test_find_newer_xy_after_created_time() { - // normal - after the created time + fn test_find_newer_xy_after_changed_time() { + // normal - after the changed time #[cfg(target_os = "linux")] let args = ["-newerat", "-newerct", "-newermt"]; #[cfg(not(target_os = "linux"))] diff --git a/tests/find_cmd_tests.rs b/tests/find_cmd_tests.rs index b0b2eda0..8fc04218 100644 --- a/tests/find_cmd_tests.rs +++ b/tests/find_cmd_tests.rs @@ -535,7 +535,13 @@ fn find_time() { let args = ["1", "+1", "-1"]; let exception_args = ["1%2", "1%2%3", "1a2", "1%2a", "abc", "-", "+", "%"]; - ["-ctime", "-atime", "-mtime"].iter().for_each(|flag| { + let tests = [ + "-atime", + #[cfg(unix)] + "-ctime", + "-mtime", + ]; + tests.iter().for_each(|flag| { args.iter().for_each(|arg| { Command::cargo_bin("find") .expect("found binary") @@ -704,10 +710,14 @@ fn find_with_gid_predicate() { #[test] #[serial(working_dir)] fn find_newer_xy() { - #[cfg(target_os = "linux")] - let options = ["a", "c", "m"]; - #[cfg(not(target_os = "linux"))] - let options = ["a", "B", "c", "m"]; + let options = [ + "a", + #[cfg(not(target_os = "linux"))] + "B", + #[cfg(unix)] + "c", + "m", + ]; for x in options { for y in options { @@ -725,15 +735,18 @@ fn find_newer_xy() { } } - #[cfg(target_os = "linux")] - let args = ["-newerat", "-newerct", "-newermt"]; - #[cfg(not(target_os = "linux"))] - let args = ["-newerat", "-newerBt", "-newerct", "-newermt"]; + let args = [ + "-newerat", + #[cfg(not(target_os = "linux"))] + "-newerBt", + #[cfg(unix)] + "-newerct", + "-newermt", + ]; let times = ["jan 01, 2000", "jan 01, 2000 00:00:00"]; for arg in args { for time in times { - let arg = &format!("{arg}{time}"); Command::cargo_bin("find") .expect("found binary") .args(["./test_data/simple/subdir", arg, time])