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

Update the time dependency #553

Closed
stanislav-tkach opened this issue Apr 14, 2021 · 11 comments
Closed

Update the time dependency #553

stanislav-tkach opened this issue Apr 14, 2021 · 11 comments

Comments

@stanislav-tkach
Copy link

It would be nice to update the time dependency to the 0.2 version before a new release (#552).

As far as I can see, the update isn't that straightforward because some functions are deprecated and a replacement for them returns i128 instead of i64 which requires some code tweaks.

@dallaires
Copy link

I also think it would be a good idea to update the dependency. I'm doing a project using Rocket and Diesel and I have to separate both projects to manage cookies because Rocket depends on time 0.2 and Diesel depends on this project which indirectly depends on time 0.1 because of chrono. I can lend a hand if this issue can be resolved by a new rustacean. Let me know if you want me to do a pull request on this.

@timvisee
Copy link

timvisee commented Jun 4, 2021

I'm looking into updating time to 0.2 now.

I think changing the chrono API to match the time type changes is the correct approach, so it'll definitely be a breaking change.

@timvisee
Copy link

timvisee commented Jun 4, 2021

I did some initial work on this, but got stuck at a failing test. I'd be very happy if someone else could take a look at this as well (see details in the linked pull request below).

See #567

@bootrecords
Copy link

I'd like to point out that the time crate is looking to be updated to version 0.3 with major changes soon. It might make sense to rather look towards bumping the time dependency to 0.3 directly, as 0.2 also has an unmaintained dependency (stdweb), which will be dropped with 0.3.

@DarthHater
Copy link

An FYI, time before 2.2.23 has a vulnerability, so updating this would be helpful. Info on the vulnerability here: https://github.com/RustSec/advisory-db/blob/main/crates/time/RUSTSEC-2020-0071.md

@timvisee
Copy link

timvisee commented Jul 29, 2021

@DarthHater thanks for pointing this out!

The PR (#567) has been updated to use ^0.2.23, thanks to @blackghost1987.

All tasks are now resolved, so I marked it as ready for review. It's now up to the chrono maintainers to merge this.

cc @quodlibetor

@bootrecords
Copy link

Not to nag, but as previously pointed out (though without reference), time v0.3 appears to be around the corner - may be worth syncing up with those developments?

@timvisee
Copy link

timvisee commented Jul 29, 2021

time v0.3 seems to be scheduled for release tomorrow, with various breaking changes. Thanks for pointing that out, totally missed that.

Wouldn't using this be a nice middle ground until v0.3 is implemented, especially for the projects that currently still rely on v0.2?

By the way, I'd be happy to try and update to time v0.3 as well.

@blackghost1987
Copy link

Not to nag, but as previously pointed out (though without reference), time v0.3 appears to be around the corner - may be worth syncing up with those developments?

Maybe it's just me but I have no faith in releases being "just around the corner", delays can happen. I think this PR is a useful addition to Chrono, it's a step in the right direction, also a security fix! I hope it will be easier anyway to upgrade to v0.3 if v0.2 is already handled. I can understand if you think that releasing it is not worth it because there will be another version change soon, but that should not affect merging the PR.

@timvisee timvisee mentioned this issue Jul 29, 2021
5 tasks
@timvisee
Copy link

I've opened #578 as draft for work on time v0.3.

@djc
Copy link
Member

djc commented Mar 23, 2022

I don't think we'll be updating the version of time used in chrono. Since #478, chrono has a minimal dependency on time, and in the next semver-incompatible version we'll remove the dependency entirely. (Note that https://rustsec.org/advisories/RUSTSEC-2020-0071 does not affect chrono, because chrono does not call into the vulnerable time 0.1 API surface.)

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 a pull request may close this issue.

7 participants