mirage / charrua

A DHCP library in OCaml
http://mirage.github.io/charrua/
ISC License
55 stars 18 forks source link

Add support for Mtime 2.0.0 #121

Closed tmcgilchrist closed 1 year ago

tmcgilchrist commented 1 year ago

This PR adds an upper bound on mtime library and includes a missing logs-syslog dependency for EIO.

hannesm commented 1 year ago

Thanks for this PR, but wouldn't it be nicer to move on and support mtime >= 2.0.0? Shouldn't be too hard to figure out which parts needs to be adapted.

haesbaert commented 1 year ago

It's kinda sad how complicated it is to get "uptime in seconds", I think this should do it without the manual conversion: Int64.div (Mtime_clock.elapsed () |> Mtime.Span.to_uint64_ns) (of_int 1_000_000_000)

haesbaert commented 1 year ago

lgtm

tmcgilchrist commented 1 year ago

Thanks @haesbaert for the help with the conversions.

Commit a89cc6f updates to mtime >= 2.0.0 as requested @hannesm The other commit might be interesting as a meta-data back port for https://ocaml.org/p/charrua-unix/1.5.0 which has the added constraint mtime >= "1.0.0" & < "2.0.0".

hannesm commented 1 year ago

It's kinda sad how complicated it is to get "uptime in seconds", I think this should do it without the manual conversion: Int64.div (Mtime_clock.elapsed () |> Mtime.Span.to_uint64_ns) (of_int 1_000_000_000)

What you can as well do is use duration and Duration.to_sec (Mtime_clock.elapsed () |> Mtime.Span.to_uint64_ns) if you like to avoid hardcoded digits.

haesbaert commented 1 year ago

It's kinda sad how complicated it is to get "uptime in seconds", I think this should do it without the manual conversion: Int64.div (Mtime_clock.elapsed () |> Mtime.Span.to_uint64_ns) (of_int 1_000_000_000)

What you can as well do is use duration and Duration.to_sec (Mtime_clock.elapsed () |> Mtime.Span.to_uint64_ns) if you like to avoid hardcoded digits.

Ah yes please that looks much better, both us missed I guess.

haesbaert commented 1 year ago

If there are no objections I'd like to merge this as is, I can make a subsequent PR for the small Duration need later, as @tmcgilchrist already spent enough time handling my bikeshedding.

haesbaert commented 1 year ago

Duration.to_sec is actually gone, there is Mtime.Span.s, but there is no division arithmetics for it (there is mul though)