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

[new release] http, cohttp, cohttp-top, cohttp-server-lwt-unix, cohttp-mirage, cohttp-lwt, cohttp-lwt-unix, cohttp-lwt-jsoo, cohttp-eio, cohttp-curl, cohttp-curl-lwt, cohttp-curl-async and cohttp-async (6.0.0~alpha0) #22353

Merged
merged 3 commits into from
Nov 15, 2022
Merged

[new release] http, cohttp, cohttp-top, cohttp-server-lwt-unix, cohttp-mirage, cohttp-lwt, cohttp-lwt-unix, cohttp-lwt-jsoo, cohttp-eio, cohttp-curl, cohttp-curl-lwt, cohttp-curl-async and cohttp-async (6.0.0~alpha0) #22353

merged 3 commits into from
Nov 15, 2022

Conversation

mseri
Copy link
Member

@mseri mseri commented Oct 24, 2022

Cohttp is an OCaml library for creating HTTP daemons. It has a portable HTTP parser, and implementations using various asynchronous programming libraries:

  • Http provides essential type definitions used in Cohttp and an extremely fast http parser. It is designed to have no dependencies and make it easy for other packages to easily interoperate with Cohttp.
  • Cohttp_lwt_unix uses the Lwt library, and specifically the UNIX bindings. It uses ocaml-tls as the TLS implementation to handle HTTPS connections.
  • Cohttp_async uses the Async library and async_ssl to handle HTTPS connections.
  • Cohttp_lwt exposes an OS-independent Lwt interface, which is used by the Mirage interface to generate standalone microkernels (use the cohttp-mirage subpackage).
  • Cohttp_lwt_jsoo compiles to a JavaScript module that maps the Cohttp calls to XMLHTTPRequests. This is used to compile OCaml libraries like the GitHub bindings to JavaScript and still run efficiently.
  • Cohttp_curl uses libcurl (via ocurl), and also provides an lwt and an async backend.
  • Cohttp_eio uses eio to leverage new features from multicore ocaml.

Links:

CHANGES:

@mseri mseri changed the title [new release] http, cohttp, cohttp-top, cohttp-server-lwt-unix, cohtt… [new release] http, cohttp, cohttp-top, cohttp-server-lwt-unix, cohttp-mirage, cohttp-lwt, cohttp-lwt-unix, cohttp-lwt-jsoo, cohttp-eio, cohttp-curl, cohttp-curl-lwt, cohttp-curl-async and cohttp-async (6.0.0~alpha0) Oct 24, 2022
@kit-ty-kate
Copy link
Member

Could you add the avoid-version flag since this is an alpha release?

@kit-ty-kate
Copy link
Member

and maybe make it unavailable with opam-version < "2.1.0"

@mseri
Copy link
Member Author

mseri commented Oct 24, 2022 •
edited
Loading

Should avoid-version be added to all or just to the root package (http)? I did not add it (yet) because I wanted to see what fails.

The opam bound is to make sure avoid-version works?

@kit-ty-kate
Copy link
Member

The opam bound is to make sure avoid-version works?

yes otherwise for people with opam 2.0 it will still get installed regardless given the flag didn't exist then and non-existing flags are ignore by default.

Should avoid-version be added to all or just to the root package (http)? I did not add it (yet) because I wanted to see what fails.

all would be better

@mseri mseri force-pushed the release-http-v6.0.0_alpha0 branch 2 times, most recently from 789868d to 8d3073c Compare October 27, 2022 07:31
@mseri
[new release] http, cohttp, cohttp-top, cohttp-server-lwt-unix, cohtt…
66345be 
…p-mirage, cohttp-lwt, cohttp-lwt-unix, cohttp-lwt-jsoo, cohttp-eio, cohttp-curl, cohttp-curl-lwt, cohttp-curl-async, cohttp-bench and cohttp-async (6.0.0~alpha0)

CHANGES:

- cohttp-eio: ensure "Host" header is the first header in http client requests (bikallem mirage/ocaml-cohttp#939)
- cohttp-eio: add TE header in client. Check TE header is server (bikallem mirage/ocaml-cohttp#941)
- cohttp-eio: add User-Agent header to request from Client (bikallem mirage/ocaml-cohttp#940)
- cohttp-eio: add Content-Length header to request/response (bikallem mirage/ocaml-cohttp#929)
- cohttp-eio: add cohttp-eio client api - Cohttp_eio.Client (bikallem mirage/ocaml-cohttp#879)
- http: add requires_content_length function for requests and responses (bikallem mirage/ocaml-cohttp#879)
- cohttp-eio: use Eio.Buf_write and improve server API (talex5 mirage/ocaml-cohttp#887)
- cohttp-eio: update to Eio 0.3 (talex5 mirage/ocaml-cohttp#886)
- cohttp-eio: convert to Eio.Buf_read (talex5 mirage/ocaml-cohttp#882)
- cohttp lwt client: Connection cache and explicit pipelining (madroach mirage/ocaml-cohttp#853)
- http: add Http.Request.make and simplify Http.Response.make (bikallem mseri mirage/ocaml-cohttp#878)
- http: add pretty printer functions (bikallem mirage/ocaml-cohttp#880)
- New eio based client and server on top of the http library (bikallem mirage/ocaml-cohttp#857)
- New curl based clients (rgrinberg mirage/ocaml-cohttp#813)
  + cohttp-curl-lwt for an Lwt backend
  + cohttp-curl-async for an Async backend
- Completely new Parsing layers for servers (anuragsoni mirage/ocaml-cohttp#819)
  + Cohttp now uses an optimized parser for requests.
  + The new parser produces much less temporary buffers during read operations
    in servers.
- Faster header comparison (gasche mirage/ocaml-cohttp#818)
- Introduce http package containing common signatures and structures useful for
  compatibility with cohttp - and no dependencies (rgrinberg mirage/ocaml-cohttp#812)
- async(server): allow reading number of active connections (anuragsoni mirage/ocaml-cohttp#809)
- Various internal refactors (rgrinberg, mseri, mirage/ocaml-cohttp#802, mirage/ocaml-cohttp#812, mirage/ocaml-cohttp#820, mirage/ocaml-cohttp#800, mirage/ocaml-cohttp#799,
  mirage/ocaml-cohttp#797)
- http (all cohttp server backends): Consider the connection header in response
  in addition to the request when deciding on whether to keep a connection
  alive (anuragsoni, mirage/ocaml-cohttp#843)
  + The user provided Response can contain a connection header. That header
    will also be considered in addition to the connection header in requests
    when deciding whether to use keep-alive. This allows a handler to decide to
    close a connection even if the client requested a keep-alive in the
    request.
- async(server): allow creating a server without using conduit (anuragsoni mirage/ocaml-cohttp#839)
  + Add `Cohttp_async.Server.Expert.create` and
    `Cohttp_async.Server.Expert.create_with_response_action`that can be used to
    create a server without going through Conduit. This allows creating an
    async TCP server using the Tcp module from `Async_unix` and lets the user
    have more control over how the `Reader.t` and `Writer.t` are created.
- http(header): faster `to_lines` and `to_frames` implementation (mseri mirage/ocaml-cohttp#847)
- cohttp(cookies): use case-insensitive comparison to check for `set-cookies` (mseri mirage/ocaml-cohttp#858)
- New lwt based server implementation: cohttp-server-lwt-unix
  + This new implementation does not depend on conduit and has a simpler and
    more flexible API
- async: Adapt cohttp-curl-async to work with core_unix.
- *Breaking changes*
  + refactor: move opam metadata to dune-project (rgrinberg mirage/ocaml-cohttp#811)
  + refactor: deprecate Cohttp_async.Io (rgrinberg mirage/ocaml-cohttp#807)
  + fix: move more internals to Private (rgrinberg mirage/ocaml-cohttp#806)
  + fix: deprecate transfer encoding field (rgrinberg mirage/ocaml-cohttp#805)
  + refactor: deprecate Cohttp_async.Body_raw (rgrinberg mirage/ocaml-cohttp#804)
  + fix: deprecate more aliases (rgrinberg mirage/ocaml-cohttp#803)
  + refactor: deprecate connection value(rgrinberg mirage/ocaml-cohttp#798)
  + refactor: deprecate using attributes (rgrinberg mirage/ocaml-cohttp#796)
  + cleanup: remove cohttp-{curl,server}-async (rgrinberg mirage/ocaml-cohttp#904)
  + cleanup: remove cohttp-{curl,server,proxy}-lwt (rgrinberg mirage/ocaml-cohttp#904)
  + fix: all parsers now follow the spec and require `\r\n` endings.
    Previously, the `\r` was optional. (rgrinberg, mirage/ocaml-cohttp#921)
- `cohttp-lwt-jsoo`: do not instantiate `XMLHttpRequest` object on boot (mefyl mirage/ocaml-cohttp#922)
@kit-ty-kate
Copy link
Member

The PR itself looks fine but there are quite a few revdeps to fix first

@mseri mseri mentioned this pull request Nov 2, 2022
Merged
@mseri
Copy link
Member Author

mseri commented Nov 4, 2022 •
edited
Loading

@icristescu @metanivek there is a series of failures of irmin-unix and irmin-http that may totally or partially related to the new cohttp. Can you help me to understand what is happening?

They don't always happen. When they happen they look like the following:

irmin-http.3.4.0

#=== ERROR while compiling irmin-http.3.4.0 ===================================#
# context              2.2.0~alpha~dev | linux/x86_64 | ocaml-base-compiler.4.14.0 | file:///home/opam/opam-repository
# path                 ~/.opam/4.14/.opam-switch/build/irmin-http.3.4.0
# command              ~/.opam/opam-init/hooks/sandbox.sh build dune runtest -p irmin-http -j 47
# exit-code            1
# env-file             ~/.opam/log/irmin-http-7-cf0152.env
# output-file          ~/.opam/log/irmin-http-7-cf0152.out
### output ###
# File "test/irmin-http/dune", line 30, characters 0-146:
# 30 | (rule
# 31 |  (alias runtest)
# 32 |  (package irmin-http)
# 33 |  (locks ../http)
# 34 |  (action
# 35 |   (chdir
# 36 |    %{workspace_root}
# 37 |    (run %{exe:test.exe} -q --color=always))))
# (cd _build/default && test/irmin-http/test.exe -q --color=always)
# +3616us bos                                 [EXEC:8464] ['uname' '-s']
# +19096us mirage-crypto-rng.lwt               [DEBUG] Mirage_crypto_rng_lwt.initialize has already been called, ignoring this call.
# +27481us src/irmin-git/node.ml:156           [DEBUG] Tree.encode_bin
# +29388us src/irmin-git/node.ml:156           [DEBUG] Tree.encode_bin
# +30714us src/irmin-git/node.ml:156           [DEBUG] Tree.encode_bin
# +32284us src/irmin-git/node.ml:156           [DEBUG] Tree.encode_bin
# +40074us src/irmin-git/node.ml:156           [DEBUG] Tree.encode_bin
# +44400us src/irmin-git/node.ml:156           [DEBUG] Tree.encode_bin
# +46725us src/irmin-git/node.ml:156           [DEBUG] Tree.encode_bin
# Testing `irmin-http'.
# This run has ID `QY0YJB5Z'.
# 
#   [OK]          HTTP.MEM               0   High-level operations on trees.
#   [OK]          HTTP.MEM               1   Basic operations on contents.
[...]
#   [OK]          HTTP.FS.UNIX          15   Unrelated merges.
#   [OK]          HTTP.FS.UNIX          16   Low-level concurrency.

irmin-unix.3.2.1


#=== ERROR while compiling irmin-unix.3.2.1 ===================================#
# context              2.2.0~alpha~dev | linux/x86_64 | ocaml-base-compiler.4.14.0 | file:///home/opam/opam-repository
# path                 ~/.opam/4.14/.opam-switch/build/irmin-unix.3.2.1
# command              ~/.opam/opam-init/hooks/sandbox.sh build dune runtest -p irmin-unix -j 127
# exit-code            1
# env-file             ~/.opam/log/irmin-unix-7-db16fb.env
# output-file          ~/.opam/log/irmin-unix-7-db16fb.out
### output ###
# File "test/irmin-unix/dune", line 25, characters 0-163:
# 25 | (rule
# 26 |  (alias runtest)
# 27 |  (package irmin-unix)
# 28 |  (locks ../http)
# 29 |  (deps test.yml)
# 30 |  (action
# 31 |   (chdir
# 32 |    %{workspace_root}
# 33 |    (run %{exe:test.exe} -q --color=always))))
# (cd _build/default && test/irmin-unix/test.exe -q --color=always)
# +4591us mirage-crypto-rng.lwt               [DEBUG] Mirage_crypto_rng_lwt.initialize has already been called, ignoring this call.
# +13870us src/irmin-git/node.ml:156           [DEBUG] Tree.encode_bin
# +15631us src/irmin-git/node.ml:156           [DEBUG] Tree.encode_bin
# +17334us src/irmin-git/node.ml:156           [DEBUG] Tree.encode_bin
# +18626us src/irmin-git/node.ml:156           [DEBUG] Tree.encode_bin
# +23544us bos                                 [EXEC:6723] ['uname' '-s']
# +57268us src/irmin-git/node.ml:156           [DEBUG] Tree.encode_bin
# +58847us src/irmin-git/node.ml:156           [DEBUG] Tree.encode_bin
# +60383us src/irmin-git/node.ml:156           [DEBUG] Tree.encode_bin
# +63050us src/irmin-git/node.ml:156           [DEBUG] Tree.encode_bin
# Testing `irmin-unix'.
# This run has ID `EJHM6QRF'.
# 
#   [OK]          bare              0   non-bare.
#   [OK]          misc              0   Testing sort order.
[...]
#   [OK]          HTTP.FS          15   Unrelated merges.
#   [OK]          HTTP.FS          16   Low-level concurrency.
# > [FAIL]        HTTP.FS          17   Concurrent updates.
#   [FAIL]        HTTP.FS          18   Concurrent head updates.
#   [FAIL]        HTTP.FS          19   Concurrent merges.
#   [FAIL]        HTTP.FS          20   Shallow objects.
#   [FAIL]        HTTP.FS          21   Closure with disconnected commits.
#   [FAIL]        HTTP.FS          22   Prehash collisions.
#   [FAIL]        HTTP.FS          23   Clear.
#   [FAIL]        HTTP.FS          24   Basic operations on slices.
#   [FAIL]        HTTP.FS          25   High-level store synchronisation.
#   [FAIL]        HTTP.FS          26   Graph.Iter.
#   [FAIL]        HTTP.FS          27   Watch.Callbacks and exceptions.
#   [FAIL]        HTTP.GIT          0   High-level operations on trees.
#   [FAIL]        HTTP.GIT          1   Basic operations on contents.
#   [FAIL]        HTTP.GIT          2   Basic operations on nodes.
#   [FAIL]        HTTP.GIT          3   Basic operations on commits.
#   [FAIL]        HTTP.GIT          4   Basic operations on branches.
#   [FAIL]        HTTP.GIT          5   Hash operations on trees.
#   [FAIL]        HTTP.GIT          6   Basic merge operations.
#   [FAIL]        HTTP.GIT          7   Test merges on tree updates.
#   [FAIL]        HTTP.GIT          8   Tree caches and hashconsing.
#   [FAIL]        HTTP.GIT          9   Tree proofs.
#   [FAIL]        HTTP.GIT         10   Complex histories.
#   [FAIL]        HTTP.GIT         11   Empty stores.
#   [FAIL]        HTTP.GIT         12   Backend node manipulation.
#   [FAIL]        HTTP.GIT         13   High-level store operations.
#   [FAIL]        HTTP.GIT         14   High-level store merges.
#   [FAIL]        HTTP.GIT         15   Unrelated merges.
#   [FAIL]        HTTP.GIT         16   Low-level concurrency.
#   [FAIL]        HTTP.GIT         17   Concurrent updates.
#   [FAIL]        HTTP.GIT         18   Concurrent head updates.
#   [FAIL]        HTTP.GIT         19   Concurrent merges.
#   [FAIL]        HTTP.GIT         20   Shallow objects.
#   [FAIL]        HTTP.GIT         21   Closure with disconnected commits.
#   [FAIL]        HTTP.GIT         22   Prehash collisions.

I did re-run them a few times. Sometimes they are fine and sometimes they are not.

@metanivek
Copy link
Contributor

@mseri our http tests are flakey (as you've seen) and in fact we've disabled them in our latest release (002e138). Maybe there is something in this cohttp release that makes them more so, but it's not immediately obvious.

Our usage of cohttp is very small (https://github.com/mirage/irmin/blob/98e0ed00d69847997e125d8a9d0f51fc801b8476/test/irmin-http/test_http.ml#L168-L170) but perhaps it triggers an idea from you. We're investigating on our side as well.

@mseri
Copy link
Member Author

mseri commented Nov 4, 2022

Thanks a lot! That code path has not really changed in this release, so I will play a bit with it but I don't think this should block the release or require upper bounds. @kit-ty-kate what do you think?

@kit-ty-kate
Copy link
Member

kit-ty-kate commented Nov 4, 2022 •
edited
Loading

Ah yes, irmin-unix and so on are known to have flaky tests. We can ignore that

@metanivek
Copy link
Contributor

I've been playing with this on irmin-http.3.4.0 and I cannot get the tests (dune runtest -p irmin-http) to run to completion even once. With cohttp.5.0.0 they run reliably (on my machine 😅), but with cohttp.6.0.0~alpha0 it appears that the process just dies and exits with 1. I'm not sure how to debug further but definitely appears something has changed. I misspoke slightly when I pointed only to cohttp in the tests -- there is also some more cohttp code in irmin-http itself that may be causing some issues.

Also, we have plans to replace irmin-http with irmin-server (see draft mirage/irmin#2031) so that may address some of these issues going forward.

@mseri lmk if you need any further help debugging but I'm happy to ignore these flakey tests to let this move forward (:pray: @kit-ty-kate).

@mseri
Copy link
Member Author

mseri commented Nov 7, 2022

Thanks a lot. I will look into it on the side and try to get to the bottom of it. If I need more help I'll ping you in a new cohttp issue.

@kit-ty-kate
Copy link
Member

Thanks

@kit-ty-kate kit-ty-kate merged commit 79399de into ocaml:master Nov 15, 2022
@mseri mseri deleted the release-http-v6.0.0_alpha0 branch November 15, 2022 13:22
@mseri
Copy link
Member Author

mseri commented Nov 15, 2022

Thanks a lot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
consider for merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mseri @kit-ty-kate @metanivek