Repository: sharkdp/fd (language: rust)

Issue:
--change-newer-than requires using UTC time, "newer" check is incorrect
# Describe the bug you encountered

It looks like there's differences to `--change-newer-than`' s behavior vs. its documentation, as well as having a potential bug.

## Bug 1: Documentation of UTC time usage

It appears that it needs the time to be stated in UTC, not local time.

```
$ touch a

$ sleep 61

$ touch b

$ ls -l a b
-rw-r--r--  1 zachriggle  staff  12168 Jul 15 19:36 a
-rw-r--r--  1 zachriggle  staff      0 Jul 15 19:37 b

$ date -r b '+%Y-%m-%d %H:%M:%S'
2021-07-15 19:37:01

$ fd --change-newer-than '2021-07-15 19:37:01'
a
b
```

However, if we add the `-u` flag to BSD `date` to get UTC time, we get closer to the expected result.

## Bug 2: Files not actually "newer" are printed

Even if we specify in UTC time, file `b` is still printed.

```
$ date -ur b '+%Y-%m-%d %H:%M:%S'
2021-07-16 00:37:01

$ fd --change-newer-than '2021-07-16 00:37:01'
b
```

This is probably technically correct, since the file was created sometime within that second -- but I consider this to be a bug.

However, if I specify the time in seconds, the comparison should be of the same granularity.

## Bug 3: Time comparison seems to be incorrect

Let's try with fractions of a second.  Luckily, GNU `date` does this (macOS default `date` does not) and is available via homebrew.

```
$ gdate -r b +"%Y-%m%-d %H:%M:%S.%N"
2021-0715 19:37:01.866605000

$ fd --change-newer-than '2021-07-16 00:37:01.866605000'
b
```

So it looks like `fd` supports fractions of a second, but is still flagging file `b` as being "newer".  Let's adjust the time by a tiny fraction of a second.

```
$ fd --change-newer-than '2021-07-16 00:37:01.866605001'
<no output>
```

This is the desired output, but it took a long time to get here.

# Describe what you expected to happen:

No files should show up, since the date was extracted from the newer file's timestamp.

# What version of `fd` are you using?

```
$ fd --version
fd 8.2.1
```

**Which operating system / distribution are you on?**

```
$ uname -srm
Darwin 21.0.0 x86_64
```



Discussion / hints:
Bug 1 is a duplicate of #631.

For bug 2, that may actually be desired behavior in many cases. For example, if I wanted to find all files that were changed since 00:00:00 today, I would probably want to include files changed within the first second.  

For bug 3, it does appear that the comparison uses `<=` instead ofjust `<`. So it is really `newer-than-or-equal`. I'm not sure if that was intended behavior or not. See https://github.com/sharkdp/fd/blob/ac4ded5b2700890c75f9cf474c1838aa4368807a/src/filter/time.rs#L29

Thanks for the info on Bug 1.

Regarding bug 2, consider if you were allowed to specify a date but not a time -- e.g. `--newer-than 2021-01-01`.  I would NOT expect files modified at 00:01 UTC on 2021-01-01 to show up in the results.  Would you?  I feel the same applies for hours / minutes / seconds.

Sounds like bug 3 is actually an implementation error.  I would create a PR to fix this, but I am disallowed from contributing to FOSS projects by my employer.
>  I would NOT expect files modified at 00:01 UTC on 2021-01-01

 I actually would, but I can understand why you wouldn't. Perhaps the behavior needs to be better documented.


> I would create a PR to fix this, but I am disallowed from contributing to FOSS projects by my employer.

Yikes. I hope you're paid extremely well because that sounds awful.

Edit: Google disallows open source contributions? That's extremely depressing.
I (partially) agree with @zachriggle: "newer than" should imply `>`, not `>=`. But on the other hand I agree with @tmccombs that `--newer-than 2021-01-01` *should* include files modified at 00:01 UTC on 2021-01-01. They are *newer than* 00:00 UTC on 2021-01-01.

So what are the options here?

1. Change from `>=` to `>` and improve the documentation (e.g. making it clear that 'newer than' does not include 'equal to')
2. Leave `>=` and improve the documentation (making it clear that - despite the name - "newer than" includes "or equal to").

Argument for option 2 would be:
- no breaking change
- "newer than or equal to" seems more natural to me. If I want to search for files that I modified yesterday (2021-08-07) or today, I don't want to specify *the day before yesterday*: `--change-newer-than 2021-08-06 23:59:59`.
I also lean more towards 2. However, the use case of listing matches that are newer than a certain file is difficult to do with a >= comparison. 

A couple other options are

3. Add a new option that references a file  and uses > (See #793)
4. Keep the current behavior, but add a new option that uses > instead of >=. I think this would probably be confusing though.

## Review of Comments

@sharkdp @tmccombs: 

Thank you both for looking into this.  That said, I'm convinced that "backwards compatibility" trumps all in some cases, so I understand the aversion to changing behavior.  FWIW, it would only change the behavior for files modified in the same sub-microsecond, which I believe is the desired / intended behavior.  The overlap in behavior exists at the sub-microsecond level, so I don't think breaking backwards compatibility actually has any effect on any entity that uses `fd`.

* Option (1) is ideal for my use-case, but I understand I'm not everybody. 
  * A secondary --newer-than-or-equal could be added, if you so desire
  * Realistically, my use-case would be better served by Option (1) **AND** #631 being implemented as `-newer`.   See below.
* Option (2) is a departure from both the behavior of the documentation AND the behavior of GNU `find . -newer someFile`.  I believe we should `>` instead of `>=`, or rename the flag (or have BOTH flags, whichever is easier for you).  
* Option (3) gives me what I want, but is misleading.  I'd almost prefer `--changed-newer-than-or-equal` to admit that `--newer-than` should match on EXACT time -- the whole purpose of the flag is "files newer than" 😛 
* Option (4) Is least desirable, for the same reasons as above.

@jacobmischka: Wrong employer 😉

## Desired Implementation

My recommendation is to defer to how GNU `find` works. `fd` is intended to be a almost-maybe-kinda-but-not-quite drop-in replacement for GNU `find`.  Or at least, that's how I'd like to use it.

### `--newer-than` flag

GNU find has several `-Xnewer` flags, but the most important is the "modified time" `-mnewer` flag, aka `-newer`.

`--newer-than` has roughly the same description as this flag, and should behave the same way.  I think a LOT of the ambiguity of the operation here would be resolved via #793.  This whole issue is effectively because I tried to work around #793 not being implemented.

```
$ cd $(mktemp -d); touch a; touch b; stat a b | grep mtime; find . -newer a -type f;
mtime   1628503342
mtime   1628503342
./b
```

So it does appear that sub-second time intervals are taken into account with GNU `find`.  I expect that setting the `mtime` to be identical (i.e., the `<=` vs `<` issue) would demonstrate that GNU `find` does an exclusive comparison.

As stated above in a few cases, the only cases that this breaks ANY backward compatibility is if the files were modified in the same sub-microsecond.  While this is a realistic scenario, `--newer-than` is clear in its name what a user is to expect -- only *NEWER* files should be returned.

I think exactly 0 people on the planet would be harmed / hindered by changing `<=` to `<` when considering sub-microsecond timestamps.

### `-newer`

In accordance with the above explanation, `-newer` might be a flag that `fd` should borrow from GNU `find` where the behavior is well-defined.

It takes a file path, and only shows files that are actually *newer* than the provided file -- down to the sub-microsecond.

GNU `find` does not, as best I can tell, have any options that take a raw textual timestamp / UTC date / TZ local timestamp -- so there's no ambiguity here.
> I (partially) agree with @zachriggle: "newer than" should imply >, not >=. But on the other hand I agree with @tmccombs that --newer-than 2021-01-01 should include files modified at 00:01 UTC on 2021-01-01. They are newer than 00:00 UTC on 2021-01-01.

I agree with this as well, and that comparison behavior should work. Instead of entirely new flags (`--change-newer-or-equal`), I think it seems cleaner to add a flag to change the behavior of the existing ones (something like `--time-inclusive` or simply `--inclusive`). Unfortunately there are no clap options to enforce that this flag is only used if one of the time functions are specified, but I think that's probably fine since it will just be ignored anyway.

Please see #821 for an implementation of this. Comments and suggestions welcome.
Looking at `find`, `-mmin` and `-mtime` can be use used for "less than", "greater than", or "exactly equal" tests but not "less than or equal" and `-newer` is strictly newer, not newer or the same age.  Which makes me think that changing it to strictly newer/older might be sufficient.  Although with find you _could_ do something like `find  -mmin -4 -or -mmin 4`. 
Or `-not -mmin +4`
> GNU `find` does not, as best I can tell, have any options that take a raw textual timestamp / UTC date / TZ local timestamp -- so there's no ambiguity here.

GNU find has `-newermt` and friends that take a string for the reference time, e.g. `find -newermt yesterday`.

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index f8a1b6a6a..f3b4e3cfd 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -26,6 +26,7 @@
 - Support `--list-details` on more platforms (like BusyBox), see #783
 - The filters `--owner`, `--size`, and `--changed-{within,before}` now apply to symbolic links
   themselves, rather than the link target, except when `--follow` is specified; see #863
+- Change time comparisons to be exclusive, see #794 (@jacobmischka)
 
 ## Changes
 
diff --git a/doc/fd.1 b/doc/fd.1
index f60a7ddd1..51deaff64 100644
--- a/doc/fd.1
+++ b/doc/fd.1
@@ -269,7 +269,7 @@ tebibytes
 .TP
 .BI "\-\-changed-within " date|duration
 Filter results based on the file modification time.
-Files with modification times greater than or equal to the argument will be returned.
+Files with modification times greater than the argument will be returned.
 The argument can be provided as a duration (\fI10h, 1d, 35min\fR) or as a specific point
 in time in either full RFC3339 format with time zone, or as a date or datetime in the
 local time zone (\fIYYYY-MM-DD\fR or \fIYYYY-MM-DD HH:MM:SS\fR).
@@ -282,7 +282,7 @@ Examples:
 .TP
 .BI "\-\-changed-before " date|duration
 Filter results based on the file modification time.
-Files with modification times less than or equal to the argument will be returned.
+Files with modification times less than the argument will be returned.
 The argument can be provided as a duration (\fI10h, 1d, 35min\fR) or as a specific point
 in time in either full RFC3339 format with time zone, or as a date or datetime in the
 local time zone (\fIYYYY-MM-DD\fR or \fIYYYY-MM-DD HH:MM:SS\fR).
diff --git a/src/filter/time.rs b/src/filter/time.rs
index 39979afaf..eb43c4bb5 100644
--- a/src/filter/time.rs
+++ b/src/filter/time.rs
@@ -39,8 +39,8 @@ impl TimeFilter {
 
     pub fn applies_to(&self, t: &SystemTime) -> bool {
         match self {
-            TimeFilter::Before(limit) => t <= limit,
-            TimeFilter::After(limit) => t >= limit,
+            TimeFilter::Before(limit) => t < limit,
+            TimeFilter::After(limit) => t > limit,
         }
     }
 }


Test patch: