-
Notifications
You must be signed in to change notification settings - Fork 517
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
Bump time to 0.2.23 #567
Bump time to 0.2.23 #567
Conversation
I've tried to debug this, and solved the original panic, but the test case still fails with some overflow errors. Here are my results so far, maybe it will help somebody. I will investigate it further though and get back here if I've got anything. So the failing test case is The problematic call is a simple subtraction of two
Unfortunately now the code panics at the overflow calculations here |
I think I figured this out. So the last part of
The result is correctly calculated to be 08:29:51.000000001 but the remainder is so huge, it cannot be stored in an i64. We could detect the overflow on the subtraction, but the This could only be solved by changing the return type of the function, which would be a breaking change! Maybe a better alternative would be to consider this as a "known limitation" of |
Fix remaining unit tests. Bump time dependency to mitigate security vulnerability. See #1
Do you suggest to resolve this (here) before merging this PR? |
I already did it, and you already merged it 🙂 I just copied the existing MAX_SECS_BITS check to check it before and after the addition as well, and it did the trick. I'm not sure if it's required after the addition, but it doesn't hurt. |
Superseded by #578, which implements these changes but for the newer |
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 chrono is unaffected by https://rustsec.org/advisories/RUSTSEC-2020-0071.) |
Fixes #553
This updates the
time
dependency to0.2.23
. The crate has been refactored to followtime
API changes.This is mostly done, but still requires some work. I'm seeing a failing test (this case fails) for which I don't clearly see the problem nor a solution.@ quodlibetor Do you have some insight on this failing test?Tasks:
time
to0.2.23
time
API changesnaive::datetime::tests::test_datetime_add
(failing case) (thanks @blackghost1987!)