Time functions assume UTC and don't allow for RFC 3339 offsets #631
Comments
|
Thank you for reporting this. That should definitely be fixed. Is there an easy way to reproduce this bug? |
|
I am also experiencing this problem. I have been looking at the source code and I have found that [ humantime::parse_rfc3339_weak] only supports UTC and timezone conversions are not supported(https://docs.rs/humantime/2.0.1/humantime/fn.parse_rfc3339_weak.html) which is used to parse the date in Line 14 in 74c3431 Was surprizing! I have done a small search and the chrono library seems to be better suited for our usecase. @sharkdp Would you be interested in a PR with an attempt to solve this? |
|
Thank you for the feedback. Yes, I would be interested in a fix for this! But let's please make sure that:
Also, it would be great if someone could post a simple way to reproduce this error. I haven't looked into it in detail, so far. |
|
The It's pretty straightforward to reproduce, here's an example using this repo that I just cloned: |
|
Regarding the reproduction For me there are actually two problems that need to addressed:
The second problem is a actually ease to reproduce as @jacobmischka has demonstrated above. Thanks @jacobmischka for sharing! The first problem is more difficult to reproduce in an automated regression test. They would depend on a the locally setup timezone which is a global system property. The system that is running the tests should be setup in a non UTC timezone & the tests should be able to know or deduce what the setup timezone is. Regarding the implementation I did an effort to find a library that would properly parse local times to solve the first problem. My best result was to use chrono 0.4. I did my tests when we were in EEST but it would assume EET and so still interpreted the local times wrongly. (Just today we switched to EET. If I retested now it would work OK :) ) If anyone knows how to successfully parse time strings appropriately for local times in Rust it would be really helpful if he shared his experience. |
|
I've started this here: https://github.com/jacobmischka/fd/tree/use-local-time-filters This addresses the first problem, but not the second. I'm not sure if it's worth migrating away from |
|
@jacobmischka Have you tried your solution while your system was in Summer time? Maybe chrono is trying to be progressive and has already adopted EUs decision to abolish Summer Time :) |
|
I haven't done extensive testing yet, no, but I don't believe it should matter since chrono is comparing everything to the current timezone offset at runtime. I hope to think of a way to implement some decent tests today at some point. |
|
I got burnt by this issue today. How about adding a hint to the man page? You can easily convert your timestamp to UTC before passing it to So, I think just mentioning UTC on the man page would be a sufficient fix. |
|
Definitely agreed that this should be explicitly in the |
|
@jacobmischka Would you be interested in continuing your work on this? It looks promising! If not, would you consider opening a PR nevertheless, so somebody else can work on it? (only if you agree to share your work, of course) |
|
Oh sure, it kind of fell off my radar. I'll try to pick it back up soon! |
This patch uses Chrono for explicit date or datetime parsing, only using humantime for its relative time parsing. The following formats are accepted: 1. Full RFC3339 parsing, requiring an explicit timezone 2. `YY-MM-DD`, defaulting to time `00:00:00` for the given date in the local time zone 3. `YY-MM-DD HH:MM:SS` in the local time zone Fixes sharkdp#631
|
I did end up doing what @gaganis suggested and used chrono to parse the explicit dates. I don't know why past me didn't think that was a good idea, to be honest. This does change the default time function behavior to use local times by default, so it's a breaking change. Comments and suggestions welcome! |
Personally, I think it is worth it. The current behavior is pretty surprising, and using UTC is straightforward if that is desired. |
This patch uses Chrono for explicit date or datetime parsing, only using humantime for its relative time parsing. The following formats are accepted: 1. Full RFC3339 parsing, requiring an explicit timezone 2. `YY-MM-DD`, defaulting to time `00:00:00` for the given date in the local time zone 3. `YY-MM-DD HH:MM:SS` in the local time zone Fixes sharkdp#631
This patch uses Chrono for explicit date or datetime parsing, only using humantime for its relative time parsing. The following formats are accepted: 1. Full RFC3339 parsing, requiring an explicit timezone 2. `YY-MM-DD`, defaulting to time `00:00:00` for the given date in the local time zone 3. `YY-MM-DD HH:MM:SS` in the local time zone Fixes sharkdp#631, sharkdp#794
This patch uses Chrono for explicit date or datetime parsing, only using humantime for its relative time parsing. The following formats are accepted: 1. Full RFC3339 parsing, requiring an explicit timezone 2. `YY-MM-DD`, defaulting to time `00:00:00` for the given date in the local time zone 3. `YY-MM-DD HH:MM:SS` in the local time zone Fixes sharkdp#631, sharkdp#794
This patch uses Chrono for explicit date or datetime parsing, only using humantime for its relative time parsing. The following formats are accepted: 1. Full RFC3339 parsing, requiring an explicit timezone 2. `YY-MM-DD`, defaulting to time `00:00:00` for the given date in the local time zone 3. `YY-MM-DD HH:MM:SS` in the local time zone Fixes #631, #794
This patch uses Chrono for explicit date or datetime parsing, only using humantime for its relative time parsing. The following formats are accepted: 1. Full RFC3339 parsing, requiring an explicit timezone 2. `YY-MM-DD`, defaulting to time `00:00:00` for the given date in the local time zone 3. `YY-MM-DD HH:MM:SS` in the local time zone Fixes sharkdp#631, sharkdp#794
--change-newer-thanassumes given time is in UTC, not local timezone. Furthermore, offsets like '2020-07-19T00:38:00-07:00' throw errors.fd version 8.1.1 on Arch Linux
The text was updated successfully, but these errors were encountered: