Async implementation, including examples

[torinnd]

I'd like to offer an Async I/O implementation for Ocaml_tls. This is heavily cribbed from the existing Lwt implementation and is offered as a I/O-agnostic functor similar to cohttp.

I'm not familiar enough with Lwt to port that code over but I believe it should be relatively straightforward for someone more familiar with Lwt.

A test client and echo server are provided in an example subdirectory.

I believe this PR supersedes #355, which doesn't appear to be actively developed.

Credit to @dinosaure for authoring janestreet#1 which was used for inspiration and provided some Async IO code.

[dinosaure]

By principle, I don't really like the functor way, specially for this case. My experience tell me that this is a bad idea to functorize I/O specially in that case. More concretely, the inherent double maintenance job is needed even if we use a functor due to the inherent differences between lwt/async (which are not expressed by types of signatures...).

Finally, and it's my opinion (then, I will let @hannesm to take the best solution), I prefer a completely dissociate tls.async & tls.lwt where the common code will be only tls without any functors/signatures which are, from my point of view, too restrictive about possibilities inherent & different between these backends - and heterogeneous & complex codebases that have decided to go in this direction confirm my suspicions.

PS: I'm not the owner of this repository, so even though I may seem very critical, it's only my opinion 👍 and I hope that such work will finally be merge upstream.

[avsm]

offline discussion with @torinnd: he's going to discuss maintaining this in the janestreet/ org instead of here and update when that's figured out.

[hannesm]

Dear @torinnd, thanks for your contribution. I'm fine with merging a tls-async package into this repository. IMHO it makes sense to maintain the code in this repository, so any changes to the core library will be reflected in the async layer as well.

I'm not convinced of the lib/io.ml and lib/io_intf.ml, could you fold them into the async subdirectory? I am aware that there is already some duplication (of rather intricate code) in lwt/ and mirage/ subdirectories, but I'm not convinced an Io functor is the way to go here.

[hannesm]

Thanks, this looks fine (note: I have not looked into detail of the async package). Would you mind to squash everything into a single commit and add a Co-Authored-By: Calascibetta Romain <romain.calascibetta@gmail.com> in the commit message, so @dinosaure is attributed (AFAIU the code is based on his work)?

[torinnd]

Sure, I'd be happy to fix the commits once I convince travis-ci and ocaml-ci to build this. I think the error I can explain is that it's unhappy because tls.opam doesn't contain ppx_jane (or ppx_let, which I believe is the only preprocessor used in this PR). There are other things it's complaining about from the lwt/examples directory that don't make much sense to me.

Would you be open to adding ppx_let to tls.opam @hannesm? If not I can rewrite it to avoid this dependency.

[hannesm]

@torinnd ppx_let is only used in async AFAICT. so eventually a (package tls-async) is needed for the examples (thus they're not compiled when dune build -p tls is executed).

the travis issues look like a tls-async:. is missing somewhere... but you can safely ignore them and focus on ocaml-ci (travis is mainly here to do some mirageos unikernel builds).

[torinnd]

I think the lwt examples in mirleft:main are the only remaining issue for the ocaml-ci check. Afaict the return type of [Tls_lwt.epoch] changed from a polymorphic [Ok 'a | `Error] to a [('a, unit) result] in 3fabf04. Do you want me to try and fix that in this PR as well?

[hannesm]

@torinnd thanks. no need to fix this in this PR, I'll do it separately.

[hannesm]

sorry, I fixed the lwt example issues in main, and then force-pushed main to your branch (where instead I intended to force-push a rebased version to your main branch). now I'm no longer allowed to push to your main branch, would you mind to rebase your commit to the current main branch and reopen this PR?

thanks.

[torinnd]

It should be rebased. I don't believe I've got permission to re-open though. Maybe you need to do that @hannesm? Otherwise I can just mint another one.

[hannesm]

I did a minor adjustment, Io_intf.S.epoch now returns a result (as done in #429 for Lwt and Mirage). I also added you @torinnd and @dinosaure to the authors list of tls-async.opam. When CI passes, I'll merge and release.

[torinnd]

@hannesm I think the return type for Async's epoch should be an Or_error.t rather than a result to be consistent with the rest of the interface. Aside from that I'm happy with the tweaks.

[hannesm]

@torinnd well spotted, I changed and force-pushed.

[hannesm]

(awaiting review from @dinosaure)

[dinosaure]

From what I see and what I did, the implementation seems fine and it follows the same design than lwt 👍. It seems good to me to merge.