Remove chrono dependency and use transitive time dependency

[Rapptz]

While adding this as a dependency to my project, I noticed my dependency tree kind of blew up due to the inclusion of chrono (it added unrelated things related to wasm and Android, for example). In an effort to reduce my dependency count I decided to port over the limited chrono functionality in this repository to use the pre-existing transitive time dependency from x509-parser instead.

I admit to not testing this code, but a look at the documentation, source code of both of these libraries, and running some cargo check to ensure this still compiles I believe the changes here are correct.

[FlorianUekermann]

Hi thanks for the PR. chrono might indeed be a bit unnecessary here. But before I remove it I would like to understand why you got so many extra indirect dependencies, because I don't see them in cargo tree:

├── chrono v0.4.31
│   ├── iana-time-zone v0.1.58
│   └── num-traits v0.2.17
│       [build-dependencies]
│       └── autocfg v1.1.0

[Rapptz]

For me, all the dependencies listed here were marked as active. This was changed during a refactor in 0.4.32 as seen here.

I'm testing my branch in production without issue for what it's worth.

[Rapptz]

I've fixed the conflicts in this PR. I've also used this in production for 1 cert renewal period (60 days) with no issues. I'm pretty sure the code as given is correct by this point.

[FlorianUekermann]

Closing for now, but thanks for the PR. chrono and time seem to be equally popular atm and the benefit of switching to time isn't obvious to me. If time gains significantly more popularity (which does seem somewhat likely), the maintenance situation changes, or some other more obvious benefit can be demonstrated, I'll reconsider.

[Rapptz]

The benefit is just reducing dependencies, since there is no obvious need to use chrono in the first place. It's fine if you don't want to merge it though -- I've been using my fork just fine for the past 9 months. Thanks for the consideration anyway.

[FlorianUekermann]

The benefit is just reducing dependencies

On current main cargo tree still shows:

├── chrono v0.4.38
│   ├── iana-time-zone v0.1.61
│   └── num-traits v0.2.19
│       [build-dependencies]
│       └── autocfg v1.4.0

This seems no worse than time. The remaining argument for time is that it's already a dependency of x509-parser. We only use that to extract the validity period, so I would like to remove that in favor of using the tools provided by rustls (not entirely sure if possible yet, but this seems promising).

However, on closer inspection it seems that rcgen also uses time and we won't remove that any time soon. I wasn't aware of this, so I'll reopen this.

[Rapptz]

Yeah, the idea was to use the transitive pre-existing time dependency instead of adding a new dependency with chrono for the same functionality. The cargo tree blow-up I mentioned was just the original motivation that led me to want to make the change, albeit it only happened in my dev environment and not in production and by now I don't even know if it happens since I trimmed all my dependencies.

[FlorianUekermann]

Let's make this a bit safer and more obvious while we are at it. Something like this:

let [valid_from, valid_until] = validity;
let total_valid_duration = valid_until.saturating_sub(valid_from).max(Duration::ZERO);
let renew_at = valid_until.saturating_sub(total_valid_duration/3);
let wait_duration = renew_at.saturating_sub(OffsetDateTime::now_utc());
self.wait = Some(Timer::after(wait_duration.abs()));

[Rapptz]

As far as I know, this doesn't actually work because OffsetDateTime::saturating_sub takes a Duration but both arguments are OffsetDateTime. The only way to get dt - dt to produce a Duration without using Sub seems to be either reimplementing the logic within Sub or just working with "instants" (i.e. converting the dt to nanoseconds since the UNIX epoch) and doing the subtraction that way. Note that this approach does not produce a Duration but an i128 instead.

Another consideration that might make it easier to work with is whether nanosecond precision is necessary, since by using the regular second precision unix_timestamp might be easier than dealing with an i128.

[FlorianUekermann]

Working with i64 (seconds) for timestamps and durations seems fine.
It's a bit annoying that time doesn't provide any safe "time between" function... If lhs > rhs and the offsets are the same lhs - rhs may be safe. But I find that difficult to confirm looking at the implementation.

The more I think about it, it seems like it may actually be easiest not to use time here, convert to u64 immediately and use saturating operations on that.