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

RFC: eio-only version of cohttp #857

Merged
merged 12 commits into from
Jun 9, 2022
4 changes: 3 additions & 1 deletion .github/workflows/nix.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,6 @@ jobs:
- uses: cachix/install-nix-action@v13
with:
nix_path: nixpkgs=channel:nixos-unstable
- run: nix-shell --command "dune runtest"
bikallem marked this conversation as resolved.
Show resolved Hide resolved
- run: |
nix-shell --command "dune runtest --profile=dev cohttp cohttp-async cohttp-curl cohttp-curl-async cohttp-curl-lwt cohttp-lwt cohttp-lwt-unix cohttp-mirage cohttp-server-lwt-unix cohttp-top http"
nix-shell --command "dune build --profile=dev cohttp-lwt-jsoo"
65 changes: 43 additions & 22 deletions .github/workflows/workflow.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,13 @@ on:
- cron: 0 1 * * MON

jobs:
build-test-ubuntu:
build-test-default:
strategy:
fail-fast: false
matrix:
os:
- ubuntu-latest
- macos-latest
ocaml-compiler:
- ocaml-variants.4.08.1+afl
- ocaml-variants.4.09.1+afl
Expand All @@ -23,62 +24,82 @@ jobs:
- ocaml-variants.4.12.1+options,ocaml-option-afl
- ocaml-variants.4.13.1+options,ocaml-option-afl
- ocaml-variants.4.14.0+options,ocaml-option-afl
local-packages:
- |
*.opam
!cohttp-eio.opam

runs-on: ${{ matrix.os }}

steps:
- name: Checkout code
uses: actions/checkout@v3


- name: Use OCaml ${{ matrix.ocaml-compiler }}
uses: ocaml/setup-ocaml@v2
with:
ocaml-compiler: ${{ matrix.ocaml-compiler }}
dune-cache: true
dune-cache: ${{ matrix.os == 'ubuntu-latest' }}
opam-depext: true
opam-depext-flags: --with-test
opam-local-packages: ${{ matrix.local-packages }}

- run: yarn --frozen-lockfile
working-directory: cohttp-lwt-jsoo/test

- run: opam install . --deps-only --with-test
- run: |
sudo apt update
sudo apt upgrade
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
sudo apt upgrade

I just made one more test, it works also without this step (and it is slightly faster)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am testing if we can also drop the next one

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mseri could we do these small tweaks on the main branch itself after this PR is merged?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. I can probably also push in this branch myself. I am currently waiting a bit longer to see if I can get extra feedback

Copy link
Contributor Author

@bikallem bikallem May 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have started working on the eio client API so would appreciate it if this was merged so that I can build on a bit of a solid base. :)

opam depext conf-libcurl
if: ${{ matrix.os == 'ubuntu-latest' }}

- run: opam exec -- make build
- run: echo "PKG_CONFIG_PATH=$(brew --prefix openssl)/lib/pkgconfig" >>"$GITHUB_ENV"
if: ${{ matrix.os == 'macos-latest' }}

- run: opam exec -- make test
- run: opam install --with-test --deps-only http cohttp cohttp-lwt cohttp-lwt-unix cohttp-server-lwt-unix cohttp-mirage cohttp-async cohttp-curl-async cohttp-curl-lwt cohttp-curl cohttp-top

- run: opam exec -- make js-test
- run: |
opam exec -- dune build --profile=dev --only-packages http,cohttp,cohttp-lwt,cohttp-lwt-unix,cohttp-server-lwt-unix,cohttp-mirage,cohttp-async,cohttp-curl-async,cohttp-curl-lwt,cohttp-curl,cohttp-top @install
opam exec -- dune build bench/ examples/ cohttp-curl-async/bin/ cohttp-curl-lwt/bin/

- run: opam exec -- dune runtest --profile=dev cohttp cohttp-async cohttp-curl cohttp-curl-async cohttp-curl-lwt cohttp-lwt cohttp-lwt-unix cohttp-mirage cohttp-server-lwt-unix cohttp-top http

build-test-macos:
bikallem marked this conversation as resolved.
Show resolved Hide resolved
build-test-cohttp-eio:
strategy:
fail-fast: false
matrix:
os:
- ubuntu-latest
- macos-latest
ocaml-compiler:
- 4.14.x
- ocaml-variants.5.0.0+trunk
local-packages:
- |
http.opam
cohttp-eio.opam

runs-on: ${{ matrix.os }}

steps:
- run: sudo apt-get install ncat
if: ${{ matrix.os == 'ubuntu-latest' }}

- run: brew install nmap
if: ${{ matrix.os == 'macos-latest' }}
bikallem marked this conversation as resolved.
Show resolved Hide resolved

- name: Checkout code
uses: actions/checkout@v3

- name: Use OCaml ${{ matrix.ocaml-compiler }}
uses: ocaml/setup-ocaml@v2
with:
ocaml-compiler: ${{ matrix.ocaml-compiler }}
dune-cache: false
opam-depext: true
opam-depext-flags: --with-test

- run: echo "PKG_CONFIG_PATH=$(brew --prefix openssl)/lib/pkgconfig" >>"$GITHUB_ENV"
dune-cache: ${{ matrix.os == 'ubuntu-latest' }}
opam-local-packages: ${{ matrix.local-packages }}
opam-repositories: |
default: https://github.com/ocaml/opam-repository.git
alpha: https://github.com/kit-ty-kate/opam-alpha-repository.git

- run: opam install --with-test --deps-only http cohttp cohttp-lwt cohttp-lwt-unix cohttp-server-lwt-unix cohttp-mirage cohttp-async cohttp-curl-async cohttp-curl-lwt cohttp-curl cohttp-top
- run: opam install cstruct eio_main bigstringaf fmt mdx
bikallem marked this conversation as resolved.
Show resolved Hide resolved

- run: |
opam exec -- dune build --profile=dev --only-packages http,cohttp,cohttp-lwt,cohttp-lwt-unix,cohttp-server-lwt-unix,cohttp-mirage,cohttp-async,cohttp-curl-async,cohttp-curl-lwt,cohttp-curl,cohttp-top @install
opam exec -- dune build -j1 bench/ examples/ cohttp-curl-async/bin/ cohttp-curl-lwt/bin/

- run: opam exec -- dune runtest --profile=dev cohttp cohttp-async cohttp-curl cohttp-curl-async cohttp-curl-lwt cohttp-lwt cohttp-lwt-unix cohttp-mirage cohttp-server-lwt-unix cohttp-top http
- run: opam exec -- dune build -p http,cohttp-eio --profile release

- run: opam exec -- dune runtest cohttp-eio --profile release
1 change: 0 additions & 1 deletion .ocamlformat
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
version = 0.21.0
profile = conventional
break-infix = fit-or-vertical
parse-docstrings = true
5 changes: 3 additions & 2 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
## Unreleased
- New curl based clients (@rgrinberg #813)
- New eio based client and server on top of the http library (bikallem #857)
- New curl based clients (rgrinberg #813)
+ cohttp-curl-lwt for an Lwt backend
+ cohttp-curl-async for an Async backend
- Completely new Parsing layers for servers (@anuragsoni #819)
- Completely new Parsing layers for servers (anuragsoni #819)
+ Cohttp now uses an optimized parser for requests.
+ The new parser produces much less temporary buffers during read operations
in servers.
Expand Down
14 changes: 13 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
.PHONY: build clean test clean
.PHONY: build clean test clean eio eio-shell eio-test

build:
dune build
Expand All @@ -12,6 +12,18 @@ js-test:
clean:
dune clean

fmt:
dune b @fmt --auto-promote

.PHONY: nix/opam-selection.nix
nix/opam-selection.nix:
nix-shell -A resolve default.nix

eio: #build eio
dune build cohttp-eio

eio-test:
dune runtest cohttp-eio

eio-shell: # nix-shell for eio dev
nix-shell -p gmp libev nmap
50 changes: 50 additions & 0 deletions cohttp-eio.opam
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# This file is generated by dune, edit dune-project instead
opam-version: "2.0"
synopsis: "CoHTTP implementation with eio backend"
description:
"A CoHTTP server and client implementation based on `eio` library. `cohttp-eio`features a multicore capable HTTP 1.1 server. The library promotes and is built with direct style of coding as opposed to a monadic."
maintainer: ["Anil Madhavapeddy <anil@recoil.org>"]
authors: [
"Anil Madhavapeddy"
"Stefano Zacchiroli"
"David Sheets"
"Thomas Gazagnaire"
"David Scott"
"Rudi Grinberg"
"Andy Ray"
"Anurag Soni"
]
license: "ISC"
homepage: "https://github.com/mirage/ocaml-cohttp"
doc: "https://mirage.github.io/ocaml-cohttp/"
bug-reports: "https://github.com/mirage/ocaml-cohttp/issues"
depends: [
"dune" {>= "2.9"}
"base-domains"
"eio"
"cstruct"
"bigstringaf"
"fmt"
"mdx" {with-test}
"eio_main" {with-test}
"conf-nmap" {with-test}
"http" {= version}
"odoc" {with-doc}
]
build: [
["dune" "subst"] {dev}
[
"dune"
"build"
"-p"
name
"-j"
jobs
"--promote-install-files=false"
"@install"
"@runtest" {with-test}
"@doc" {with-doc}
]
["dune" "install" "-p" name "--create-install-files" name]
]
dev-repo: "git+https://github.com/mirage/ocaml-cohttp.git"
3 changes: 3 additions & 0 deletions cohttp-eio/examples/dune
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
(executable
(name server1)
(libraries cohttp-eio uri eio_main))
46 changes: 46 additions & 0 deletions cohttp-eio/examples/server1.ml
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
let text =
"CHAPTER I. Down the Rabbit-Hole Alice was beginning to get very tired of \
sitting by her sister on the bank, and of having nothing to do: once or \
twice she had peeped into the book her sister was reading, but it had no \
pictures or conversations in it, <and what is the use of a book,> thought \
Alice <without pictures or conversations?> So she was considering in her \
own mind (as well as she could, for the hot day made her feel very sleepy \
and stupid), whether the pleasure of making a daisy-chain would be worth \
the trouble of getting up and picking the daisies, when suddenly a White \
Rabbit with pink eyes ran close by her. There was nothing so very \
remarkable in that; nor did Alice think it so very much out of the way to \
hear the Rabbit say to itself, <Oh dear! Oh dear! I shall be late!> (when \
she thought it over afterwards, it occurred to her that she ought to have \
wondered at this, but at the time it all seemed quite natural); but when \
the Rabbit actually took a watch out of its waistcoat-pocket, and looked at \
it, and then hurried on, Alice started to her feet, for it flashed across \
her mind that she had never before seen a rabbit with either a \
waistcoat-pocket, or a watch to take out of it, and burning with curiosity, \
she ran across the field after it, and fortunately was just in time to see \
it pop down a large rabbit-hole under the hedge. In another moment down \
went Alice after it, never once considering how in the world she was to get \
out again. The rabbit-hole went straight on like a tunnel for some way, and \
then dipped suddenly down, so suddenly that Alice had not a moment to think \
about stopping herself before she found herself falling down a very deep \
well. Either the well was very deep, or she fell very slowly, for she had \
plenty of time as she went down to look about her and to wonder what was \
going to happen next. First, she tried to look down and make out what she \
was coming to, but it was too dark to see anything; then she looked at the \
sides of the well, and noticed that they were filled with cupboards......"

open Cohttp_eio

let app (req, _reader) =
match Http.Request.resource req with
| "/" -> Server.text_response text
| "/html" -> Server.html_response text
| _ -> Server.not_found_response

let () =
let port = ref 8080 in
Arg.parse
[ ("-p", Arg.Set_int port, " Listening port number(8080 by default)") ]
ignore "An HTTP/1.1 server";

Eio_main.run @@ fun env ->
Eio.Switch.run @@ fun sw -> Server.run ~port:!port env sw app
Loading