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.

Sign up for GitHub

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

Jump to bottom

Fetch API, v2 #996

Open
bryphe opened this issue Aug 31, 2020 · 7 comments
Open

Fetch API, v2 #996

bryphe opened this issue Aug 31, 2020 · 7 comments

Comments

@bryphe
Copy link
Member

bryphe commented Aug 31, 2020

@lessp has done some awesome work in pioneering a fetch API for us through https://github.com/lessp/fetch

Unfortunately, we can't use it in Onivim 2 yet - I would love to be able to, because we could remove this node-script-shim that we have today: https://github.com/onivim/oni2/blob/63e3159b0f45bed017251e4f35465bcb383a1b50/src/Service/Net/Service_Net.re#L69

There are a couple of blockers:

  1. We need to ensure the fetch strategy works on Windows (and has CI tests for it)
  2. Ideally, the fetch strategy should use libuv as the back-end - I envision Revery / Onivim to use libuv as the event loop. Might be easier once there is a libuv-lwt engine.
@bryphe bryphe mentioned this issue Aug 31, 2020
Merged
bryphe added a commit that referenced this issue Sep 1, 2020
@bryphe
fix(ci): Fix CI for Onivim 2 - revert fetch dependencies (#997)
11d4fb4 
Unfortunately, I'm still seeing issues with the set of dependencies for the `fetch-lwt` library, when building Onivim 2 - we get this error:

I've been trying to avoid reverting this, and maintaining a separate 'oni2' branch w/o it - but it's a lot of extra overhead for me (we can't actually use `fetch` in Onivim 2, either - lack of Windows support & it isn't integrated with libuv).

When building from Onivim 2, I get these errors:
```
Bryans-Mac-mini:oni2 bryphe$ esy run -f
info building @reason-native-web/piaf@1.3.0@d41d8cd9
error: build failed with exit code: 1
  build log:
    # esy-build-package: building: @reason-native-web/piaf@1.3.0
    # esy-build-package: pwd: /Users/bryphe/.esy/source/i/reason_native_web__s__piaf__1.3.0__aac2317a
    # esy-build-package: running: 'dune' 'build' '--only-packages=piaf' '--profile=release' '-j' '4' '--root=./piaf'
    Entering directory '/Users/bryphe/.esy/source/i/reason_native_web__s__piaf__1.3.0__aac2317a/piaf'
    File "lib/http_intf.ml", line 1:
    Error: The files lib/.piaf.objs/byte/piaf__Scheme.cmi
           and /Users/bryphe/.esy/3__________________________________________________________________/i/reason_native_web__s__h2_lwt_unix-0.6.1000-eb9002c6/lib/h2-lwt-unix/h2_lwt_unix.cmi
           make inconsistent assumptions over interface Gluten_lwt_unix__Ssl_io
    error: command failed: 'dune' 'build' '--only-packages=piaf' '--profile=release' '-j' '4' '--root=./piaf' (exited with 1)
    esy-build-package: exiting with errors above...
    
  building @reason-native-web/piaf@1.3.0
esy: exiting due to errors above
```

I'd definitely like to have the `fetch` API - some details here: #996 , and really appreciate all the pioneering work you've done @lessp ! If you have any other ideas, @lessp - let me know 👍
@WhoAteDaCake
Copy link
Contributor

What about using the existing libuv ocaml bindings ? https://github.com/aantron/luv

@lessp
Copy link
Member

lessp commented Nov 8, 2020

What about using the existing libuv ocaml bindings ? https://github.com/aantron/luv

That's absolutely an alternative, I believe Luv is already used in Oni2. For a viable HTTP(S)-client we'd need to hook it up with something like ocaml-tls. 🙂

@bryphe
Copy link
Member Author

bryphe commented Nov 9, 2020

Definitely - luv + a http/https client built on-top would be perfect

@zbaylin
Copy link
Member

zbaylin commented Jan 9, 2021

@bryphe and I discussed using ocurl the other day as a solution to this problem.
Pros

  • libcurl is very robust
  • ocurl seems to be a thin wrapper on top of libcurl
  • ocurl contains functions that integrate with Lwt
  • Builds on Windows

Cons

  • As of now, there is no luv integration
  • Might require an esy-libcurl

I'm going to investigate how hard it would be to write an luv wrapper on top of the existing ocurl code. I think it would be fairly similar to the Lwt code 🤞

@zbaylin
Copy link
Member

zbaylin commented Jan 13, 2021

I made this as a proof of concept: https://github.com/zbaylin/ocurl-luv-test -- the only issues I'm running into as of now is converting a Unix.file_descr to an Luv.Os_fd.Socket.t. Unfortunately, Obj.magic doesn't seem to work and the Unix tools were removed from Luv in order to not depend on Unix. Once I find a fix I think ocurl will definitely be a viable option for this!

@bryphe
Copy link
Member Author

bryphe commented Jan 13, 2021

Awesome progress @zbaylin !

@bryphe
Copy link
Member Author

bryphe commented Jan 13, 2021

Unfortunately, Obj.magic doesn't seem to work and the Unix tools were removed from Luv in order to not depend on Unix

I wonder if we can bring these back in a fork, or just in Revery?

secretdev718 added a commit to secretdev718/outrunlabs that referenced this issue May 9, 2024
@secretdev718
fix(ci): Fix CI for Onivim 2 - revert fetch dependencies (#997)
6a5a813 
Unfortunately, I'm still seeing issues with the set of dependencies for the `fetch-lwt` library, when building Onivim 2 - we get this error:

I've been trying to avoid reverting this, and maintaining a separate 'oni2' branch w/o it - but it's a lot of extra overhead for me (we can't actually use `fetch` in Onivim 2, either - lack of Windows support & it isn't integrated with libuv).

When building from Onivim 2, I get these errors:
```
Bryans-Mac-mini:oni2 bryphe$ esy run -f
info building @reason-native-web/piaf@1.3.0@d41d8cd9
error: build failed with exit code: 1
  build log:
    # esy-build-package: building: @reason-native-web/piaf@1.3.0
    # esy-build-package: pwd: /Users/bryphe/.esy/source/i/reason_native_web__s__piaf__1.3.0__aac2317a
    # esy-build-package: running: 'dune' 'build' '--only-packages=piaf' '--profile=release' '-j' '4' '--root=./piaf'
    Entering directory '/Users/bryphe/.esy/source/i/reason_native_web__s__piaf__1.3.0__aac2317a/piaf'
    File "lib/http_intf.ml", line 1:
    Error: The files lib/.piaf.objs/byte/piaf__Scheme.cmi
           and /Users/bryphe/.esy/3__________________________________________________________________/i/reason_native_web__s__h2_lwt_unix-0.6.1000-eb9002c6/lib/h2-lwt-unix/h2_lwt_unix.cmi
           make inconsistent assumptions over interface Gluten_lwt_unix__Ssl_io
    error: command failed: 'dune' 'build' '--only-packages=piaf' '--profile=release' '-j' '4' '--root=./piaf' (exited with 1)
    esy-build-package: exiting with errors above...
    
  building @reason-native-web/piaf@1.3.0
esy: exiting due to errors above
```

I'd definitely like to have the `fetch` API - some details here: revery-ui/revery#996 , and really appreciate all the pioneering work you've done @lessp ! If you have any other ideas, @lessp - let me know 👍
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@zbaylin @bryphe @WhoAteDaCake @lessp