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
[RFC]: Convert to direct-style with Eio #2149
base: main
Are you sure you want to change the base?
Conversation
cdd06d1
to
ecfabf4
Compare
448d615
to
0b5c73a
Compare
Thanks a lot @patricoferris, this is fantastic work! We've rebased your branch and completed the missing libraries using |
Awesome! Sorry for the slow reply, please do whatever is easiest (pushing the changes here, new PR etc.) |
ca52020
to
a0e31eb
Compare
Thanks a lot for your patience! I've updated your PR with all the work done by @ElectreAAS and @clecat to make So this PR now has feature parity with
WDYT @samoht ? If we can merge this wip, should we add a disclaimer in the readme that the repo is migrating to Eio and that the public API will still change? |
Could we add temporary workarounds for the failing tests? I'd be very happy to merge that PR but I also would like to keep the CI green to detect future regressions. |
0f1ff3f
to
e70e4dd
Compare
What's ported?
This draft PR ports parts of Irmin to https://github.com/ocaml-multicore/eio. This includes
irmin
,irmin-fs
,irmin-pack
,irmin-containers
andirmin-chunk
along with the tests. Most of the other libraries are not yet ported because of the effort to try and port the dependencies in particulargraphql
,ocaml-git
andwebmachine
(although I thinkirmin-http
might be on its way out).I'm opening this draft PR to let people know of the existence of this work (in case they want to do some hacking ;)) and to also let people comment on it. It might be interesting to see if there are impacts on the benchmarks.
Some issues
One problem that has come up a few times, is Irmin's use of top-level values (even if lazy). In the Eio world this becomes a little tricky because they often need either access to some capability or a
Eio.Switch.t
. There is a port ofirmin-watcher
for example that requires the installation of a handler in order to be able to get a switch to the hooks.irmin-fs
also requires something similar so that users would have to do this (see the fs unix tests):Which I'm definitely not a fan of! As pointed out in ocaml-multicore/eio#364 (comment) fiber-local storage could be used but comes with it's own problems. The
Irmin_fs
handler was more about getting thefs
capability to the repo constructor that can only take a configuration because I wasn't sure how to do that otherwise. The only way to get a capability is if it is passed to you from Eio's standard environment.Porting to Eio already is a massive breaking change, so perhaps some of this could be reworked to avoid all of this :)) Would love to know your thoughts?