ocsigen / lwt

OCaml promises and concurrent I/O
https://ocsigen.org/lwt
MIT License
708 stars 173 forks source link

Suggestion: make Lwt_unix and Lwt_js implement a virtual library with their shared signature #915

Open roddyyaga opened 2 years ago

roddyyaga commented 2 years ago

Context: I'm trying to get lwt-pipe working with js_of_ocaml. Currently it uses Lwt_unix.sleep, so I need to make it use Lwt_js.sleep instead when js_of_ocaml is used. Splitting into two libraries and functorizing would be one way to do this, but I think a neater thing would be using dune's virtual libraries.

If I declared a dependency in lwt-pipe on some virtual library with the signature

val sleep : float -> unit Lwt.t

val yield : unit -> unit Lwt.t

and Lwt_unix and Lwt_js were both declared as implementing that library, then (if I understand virtual libraries correctly), there could just be one lwt-pipe library and it would magically work as long as either lwt.unix or js_of_ocaml-lwt was a dependency wherever lwt-pipe was.

Would you be happy to accept a PR adding this?

raphael-proust commented 2 years ago

I think you correctly understand virtual libraries and their usage. You might need to tweak your build files a bit to help with the magic part maybe…

FWIW, in the tezos project, we pass around an object with some methods for reading on-disk state, showing UI messages, and sleeping/yielding. It works well there but we also use virtual libraries for js/unix in some other part of the code because it makes more sense there.

On a more pragmatic side, I think we would need to be careful how we introduce such a virtual library. We don't want to break too many builds. I think it's important that we keep the existing working builds working. Do you have a more specific proposal for that? Would we be introducing a virtual library in addition to the existing ones?

A final additional note: the Lwt_unix.yield function is now deprecated in favor of Lwt.pause. Consequently, it would probably not need to be included in the virtual library. run : 'a t -> 'a on the other hand might be good to include.