Repository navigation
find: compare selected timestamps for -newerXY - #886
SichenLiang wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #886 +/- ##
==========================================
+ Coverage 92.52% 92.59% +0.07%
==========================================
Files 36 36
Lines 7694 7686 -8
Branches 391 390 -1
==========================================
- Hits 7119 7117 -2
+ Misses 437 431 -6
Partials 138 138 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Merging this PR will not alter performance
|
|
|
||
| /// 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. |
There was a problem hiding this comment.
NewerMatcher is now the same thing as this with m/m.
could we drop it and route -newer here too?
There was a problem hiding this comment.
Done, -newer now uses the -newerXY matcher with m/m, and NewerMatcher is removed.
Output and status are unchanged. The only visible difference is the per-file error when a directory cannot be read: “modification time” is now “Modified time”.
| x_option, | ||
| y_option, | ||
| given_modification_time: metadata.modified()?, | ||
| reference_time: y_option.get_file_time(&metadata)?, |
There was a problem hiding this comment.
on Windows with Y=c, the user only gets "find: unsupported".
could you please return a clearer error here?
There was a problem hiding this comment.
It now fails during parsing with -newermc: change times are not available on this platform, following the wording already used for -inum and -links.
| eprintln!("skipping {x_option}/{y_option}: birth time unsupported"); | ||
| continue; | ||
| }; | ||
| let expected = if ["a", "m"].contains(&x_option) && ["a", "m"].contains(&y_option) { |
There was a problem hiding this comment.
i don't get this special case, x_time > y_time gives the same result for a/m, no?
also the expected value is just the implementation copied again, so it doesn't test much
There was a problem hiding this comment.
ah ur right, x_time > y_time already handles I removed the special case. The test is now a table of fixed timestamps with the expected result written out for each pair.
| } | ||
|
|
||
| #[test] | ||
| fn newer_option_uses_reference_birth_time_when_available() { |
There was a problem hiding this comment.
this test is long and has two skip paths.
the integration tests already cover this, do we need it?
There was a problem hiding this comment.
Those tests don't cover this on Linux, since B is never used as Y there. I added one a/B row to the table and dropped the longer separate test.
`-newerXY` compared both the candidate's X and Y timestamps with the reference file's mtime. Read the reference's Y timestamp instead, then compare only the candidate's X timestamp with it. This fixes `-newerXY` when Y is not `m`. It also fixes `-anewer` and `-cnewer` when the candidate's mtime is older than the reference's. `-newer` is the m/m case, so it now uses the same matcher and `NewerMatcher` is removed. Reference symlinks follow the active `-P`, `-H` or `-L` setting. `-P` uses the link's own timestamps, while `-H` and `-L` use the target when it exists. A later `-follow` affects only predicates that come after it. Windows has no change time for the reference, so predicates with `Y=c` fail during parsing with “change times are not available on this platform”.
6d34680 to
e6d58a0
Compare
2 participants
-newerXYalways used the reference mtime regardless ofY, and required both the candidate'sXandYtimestamps to be newer than it. Compare the candidate'sXtimestamp directly with the reference'sYtimestamp instead.This fixes
-anewerand-cnewerwhen the candidate's mtime is older but the selected timestamp is newer.Reference symlinks now follow the active dereference mode:
-Puses the link timestamp, while-Hand-Luse the target timestamp, or the link's own timestamp if the target does not exist. Because-followtakes effect where it appears in the expression, it changes reference handling only for later predicates. This matches the GNU find documentation.On Windows,
Y=cwas rejected during parsing with-newermc: change times are not available on this platformand status 1, because the reference's change time is unavailable.Fixes #885