robur-coop / happy-eyeballs

An implementation of happy eyeballs (RFC 8305) in OCaml with lwt
ISC License
22 stars 4 forks source link

Provide a module type signature and a connect_device to be able to create a Mirage device #22

Closed dinosaure closed 2 years ago

dinosaure commented 2 years ago

In the same view than mirage/ocaml-dns#297 and in the goal of mirage/mirage#1291, this PR wants to provide:

You probably are a bit afraid by a constraint: Dns_client_mirage.S with type Transport.stack = S.t - for my point of view, it's completely fine. The functoria DSL does not have the ability to unify types/devices and if the user uses an other implementation of Tcpip.Stack..V4V6 than the one used to make the Dns_client_mirage.S module, OCaml will complain about incompatible type. As far as I can say, we are not able to constraint that via the functoria DSL.

However, I really think that it should help us to define our final Mirage.happy_eyeballs (& our Mirage.dns_client). The last goal is to use them to create an mimic_happy_eyeballs which can be used then by Git devices.

dinosaure commented 2 years ago

I took the opportunity to open Msg of string errors to be more composable with something else.

dinosaure commented 2 years ago

Finally, I fixed constraints to be able to build a mimic device with a possible dns and a happy-eyeballs devices. However, I would like to say that it was very difficult to figure out how to fix these constraints when a module type of Dns_client.Make(Transport) into signatures generates new types without constraints along projects.

It's a more systemic problem but I really think that in the purpose of mirage/functoria, we should not generate a signature from a functor.

dinosaure commented 2 years ago

NOTE: by design, connect_device should not appear into the signature but only into the functor. I put it but if you are more confident to put it outside (as any devices in mirage), I can do it.

dinosaure commented 2 years ago

As far as I can say, I just moved connect_device outside the signature S (as we usually do for MirageOS device) but nothing should be upgraded then from dns.6.2.0. So, you are able to review this PR :+1:. Then, we will able to propose a device into mirage/mirage.

dinosaure commented 2 years ago

I finally did the mimic layer (expected by git and paf) here: https://github.com/dinosaure/mimic/pull/11 which will be compatible with MirageOS.

hannesm commented 2 years ago

I did not look deep into the implementation, but I don't think adding abstract types to the interface is the way to go. It'd be nice if we could find a path without that noise.

hannesm commented 2 years ago

Thanks. I merged this PR manually after minor code style changes.

dinosaure commented 2 years ago

Thanks, it unlock me several projects then, I will continue the stuff on the MirageOS side :+1:.