Skip to content

Commit

Permalink
find: ctime is "changed" time, not "created" time
Browse files Browse the repository at this point in the history 
Fixes #358.
  • Loading branch information
@tavianator
tavianator committed Jul 10, 2024
1 parent 9c11f11 commit 7ad945d
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 54 deletions.
6 changes: 3 additions & 3 deletions src/find/matchers/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ mod samefile;
mod size;
#[cfg(unix)]
mod stat;
mod time;
pub mod time;
mod type_matcher;
mod user;

Expand Down Expand Up @@ -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.
Expand All @@ -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]),
};
Expand Down
124 changes: 81 additions & 43 deletions src/find/matchers/time.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -88,8 +91,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(),
}
}
Expand Down Expand Up @@ -204,18 +206,47 @@ 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<SystemTime>;
}

#[cfg(unix)]
impl ChangeTime for Metadata {
fn changed(&self) -> std::io::Result<SystemTime> {
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)

Check warning on line 223 in src/find/matchers/time.rs

View check run for this annotation

Codecov / codecov/patch

src/find/matchers/time.rs#L223 

Added line #L223 was not covered by tests
};
Ok(ctime)
}
}

#[cfg(not(unix))]
impl ChangeTime for Metadata {
fn changed(&self) -> std::io::Result<SystemTime> {
// 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,
}

impl FileTimeType {
fn get_file_time(self, metadata: Metadata) -> std::io::Result<SystemTime> {
match self {
FileTimeType::Accessed => metadata.accessed(),
FileTimeType::Created => metadata.created(),
FileTimeType::Changed => metadata.changed(),
FileTimeType::Modified => metadata.modified(),
}
}
Expand Down Expand Up @@ -469,9 +500,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();

Expand Down Expand Up @@ -521,16 +552,16 @@ 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() {
test_matcher_for_file_time_type(&file_info, modified_time, FileTimeType::Modified);
}
}

/// 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,
Expand All @@ -556,10 +587,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 {
Expand Down Expand Up @@ -669,32 +704,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,
Expand Down Expand Up @@ -728,7 +765,7 @@ mod tests {
// Means to find files accessed / modified more than 1 minute ago.
[
FileTimeType::Accessed,
FileTimeType::Created,
FileTimeType::Changed,
FileTimeType::Modified,
]
.iter()
Expand All @@ -738,10 +775,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",
}
)
Expand All @@ -756,7 +793,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()
Expand All @@ -766,10 +804,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",
}
)
Expand Down
18 changes: 10 additions & 8 deletions src/find/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -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");
}
Expand All @@ -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");
}
}
Expand All @@ -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");
}
Expand Down Expand Up @@ -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"))]
Expand All @@ -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"))]
Expand Down

0 comments on commit 7ad945d

Please sign in to comment.