Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
roddyyaga opened this issue Dec 22, 2021 · 1 comment

Comments

@roddyyaga
Copy link

roddyyaga commented Dec 22, 2021

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
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants