Skip to content

date: Implement setting the date on Unix & Windows #1798

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.

Sign up for GitHub

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

Jump to bottom
Merged
sylvestre merged 19 commits into from
Mar 19, 2021
Merged

date: Implement setting the date on Unix & Windows #1798

sylvestre merged 19 commits into from
Mar 19, 2021

Conversation

@marcosatti
Copy link
Contributor

@marcosatti marcosatti commented Mar 11, 2021 •
edited
Loading

Fixes #1754

Implements basic functionality for setting the date under both Unix & Windows systems.

Unresolved issues:

  • parse_date only accepts limited formats for the dates input - separate issue needed?
  • How are leap seconds handled on Windows? Not immediately clear, seems like they kind of ignore it which is what I've put in.

@marcosatti
date: implement set date for unix and windows
9d8662a 
Parsing the date string is not fully implemented yet, as in it relies
on the internals of chrono - things like "Mon, 14 Aug 2006 02:34:56 -0600"
do not work, nor does "2006-08-14 02:34:56" (no TZ / local time). This
is no different to using the "--date" option however, and will get fixed
when `parse_date` is a bit smarter.

Only supports unix and Windows platforms for now.
@marcosatti
date: fixup windows support
6e033a2
@marcosatti
date: fix Windows support
5519f9d
@marcosatti
date: cleanup formatting
2af762c
@sylvestre
Copy link
Contributor

sylvestre commented Mar 11, 2021

It would be great to add tests too.
I guess it requires root but we could:

  • test that the parsing of the input works
  • test that the parsing of the input fails with the right error
  • setting the date as not root fails the expected way :)

thanks

sylvestre
sylvestre reviewed Mar 11, 2021
View reviewed changes
@sylvestre
Copy link
Contributor

sylvestre commented Mar 11, 2021

Has some builds failures!

@marcosatti
Copy link
Contributor Author

marcosatti commented Mar 12, 2021

Has some builds failures!

Yeah not quite completed yet...

The 32-bit platforms are still using long as time_t's, so I'll jut add in a try_into() call - they should be updated by 2038 I would hope. The nanoseconds can fit into a i32 no matter what, so a static cast should be fine there.

@marcosatti
Copy link
Contributor Author

marcosatti commented Mar 12, 2021

The gnu date always outputs the current date (regardless if there was a permissions error) when setting the date - do you want to create a new issue for this or should it be done in this one?

@marcosatti
Copy link
Contributor Author

marcosatti commented Mar 12, 2021

Damn, try_into() was added in a later Rust version (v1.34?)... I'll just use a normal cast.

@marcosatti
date: use a cast instead of try_into
616b16a 
The TryInto trait was only available after a later version than 1.33 of Rust.
@marcosatti
date: fix warning on test
1e9b772
@sylvestre
Copy link
Contributor

sylvestre commented Mar 12, 2021

please run

cargo +1.33.0 update

sylvestre
sylvestre requested changes Mar 13, 2021
View reviewed changes
Copy link
Contributor

@sylvestre sylvestre left a comment

Choose a reason for hiding this comment

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

only minor requests!
thanks

@sylvestre
Copy link
Contributor

sylvestre commented Mar 17, 2021

@marcosatti ping?

@marcosatti
Copy link
Contributor Author

marcosatti commented Mar 18, 2021

@sylvestre sorry been busy lately - will look at it later on in the week.

sylvestre
sylvestre reviewed Mar 19, 2021
View reviewed changes
@sylvestre sylvestre merged commit 5ec87dc into uutils:master Mar 19, 2021
@sylvestre
Copy link
Contributor

sylvestre commented Mar 19, 2021

Looks great :)
I will merge yet now, please open a PR for the last request!

@marcosatti marcosatti mentioned this pull request Mar 20, 2021
Merged
@marcosatti
Copy link
Contributor Author

marcosatti commented Mar 20, 2021

Created PR #1846 for more tests.

@sylvestre sylvestre mentioned this pull request Mar 22, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sylvestre sylvestre sylvestre requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

date: implement the set date

2 participants

@marcosatti @sylvestre