Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't use Durations for calculating relative times #85

Merged
merged 6 commits into from
Sep 17, 2024

Conversation

ysthakur
Copy link
Contributor

This PR closes uutils/coreutils#6629.

There were two problems with parse_relative_time, which have been fixed by this PR.

One problem was that parse_relative_time() got the current date using Utc::now().date_naive() while parse_relative_time_at_date() got the current date using Local::now().date_naive(). This means that the date in the end result will be 1 day off if the date in your timezone is different from the date in UTC.

Another problem was that parse_relative_time_at_date() created a chrono::Duration. Unfortunately, Duration doesn't understand months and years, only seconds and nanoseconds. Therefore, the old code would assume every month was 30 days and every year was 365 days. This meant stuff like touch -d "month" wouldn't work in February and touch -d "year" wouldn't work in 2024.

I've now made it so that parse_relative_time_at_date returns a DateTime rather than a Duration. Within that function, checked_(add/sub)_(days/months/signed) is used to add days, months, years, whatever while being aware of the fact that months and years can be different sizes. parse_datetime_at_date() in lib.rs now directly calls this function, so there's no need to calculate a Duration at all.

The old parse_relative_time function has now been relegated to being a helper for the relative time parsing tests.

Testing

I haven't run all the uutils/coreutils tests yet, but I did run the following locally (I ran it between UTC midnight and my midnight). The yesterday bug mentioned in uutils/coreutils#6629 were not triggered.

> cargo run -p uu_touch -- -d "yesterday" foobarbaz.txt
...(cargo output omitted, there was a deprecation warning)
> stat foobarbaz.txt
  File: foobarbaz.txt
  Size: 0               Blocks: 0          IO Block: 4096   regular empty file
Device: 8,32    Inode: 317960      Links: 1
Access: (0644/-rw-r--r--)  Uid: ( 1000/ysthakur)   Gid: ( 1000/ysthakur)
Access: 2024-08-26 22:20:47.096649101 -0400
Modify: 2024-08-26 22:20:47.096649101 -0400
Change: 2024-08-27 22:20:47.092271897 -0400
 Birth: 2024-08-27 21:51:57.632730345 -0400
> touch -d yesterday foobarbaz.txt
> stat foobarbaz.txt
  File: foobarbaz.txt
  Size: 0               Blocks: 0          IO Block: 4096   regular empty file
Device: 8,32    Inode: 317960      Links: 1
Access: (0644/-rw-r--r--)  Uid: ( 1000/ysthakur)   Gid: ( 1000/ysthakur)
Access: 2024-08-26 22:20:57.214983299 -0400
Modify: 2024-08-26 22:20:57.214983299 -0400
Change: 2024-08-27 22:20:57.202269430 -0400
 Birth: 2024-08-27 21:51:57.632730345 -0400

And here it is for -d "month":

> cargo run -p uu_touch -- -d "month" foobarbaz.txt
...(cargo output omitted again)
> stat foobarbaz.txt
  File: foobarbaz.txt
  Size: 0               Blocks: 0          IO Block: 4096   regular empty file
Device: 8,32    Inode: 317960      Links: 1
Access: (0644/-rw-r--r--)  Uid: ( 1000/ysthakur)   Gid: ( 1000/ysthakur)
Access: 2024-09-27 22:22:21.913646930 -0400
Modify: 2024-09-27 22:22:21.913646930 -0400
Change: 2024-08-27 22:22:21.912248547 -0400
 Birth: 2024-08-27 21:51:57.632730345 -0400
> touch -d month foobarbaz.txt
> stat foobarbaz.txt
  File: foobarbaz.txt
  Size: 0               Blocks: 0          IO Block: 4096   regular empty file
Device: 8,32    Inode: 317960      Links: 1
Access: (0644/-rw-r--r--)  Uid: ( 1000/ysthakur)   Gid: ( 1000/ysthakur)
Access: 2024-09-27 22:22:33.164133827 -0400
Modify: 2024-09-27 22:22:33.164133827 -0400
Change: 2024-08-27 22:22:33.152245574 -0400
 Birth: 2024-08-27 21:51:57.632730345 -0400

Copy link

codecov bot commented Aug 28, 2024

Codecov Report

Attention: Patch coverage is 98.26840% with 4 lines in your changes missing coverage. Please review.

Project coverage is 80.26%. Comparing base (ba19b94) to head (e41e26f).
Report is 12 commits behind head on main.

Files with missing lines Patch % Lines
src/parse_relative_time.rs 98.68% 2 Missing and 1 partial ⚠️
src/lib.rs 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main      #85       +/-   ##
===========================================
+ Coverage   70.00%   80.26%   +10.26%     
===========================================
  Files           6        6               
  Lines         820      826        +6     
  Branches      190      182        -8     
===========================================
+ Hits          574      663       +89     
- Misses        127      128        +1     
+ Partials      119       35       -84     
Flag Coverage Δ
macos_latest 80.21% <98.26%> (+10.21%) ⬆️
ubuntu_latest 68.93% <72.72%> (-0.10%) ⬇️
windows_latest 4.71% <0.00%> (-2.92%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

@Krysztal112233 Krysztal112233 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good.

Could you please review this pull request? @cakebaker

@ysthakur
Copy link
Contributor Author

@cakebaker Could you take a look at this if you have time?

@cakebaker cakebaker merged commit 212c6f7 into uutils:main Sep 17, 2024
16 checks passed
@cakebaker
Copy link
Collaborator

Good work, thanks for your PR!

@ysthakur ysthakur deleted the fix-relative-time branch September 17, 2024 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

touch -d with relative day is ahead of its time
3 participants