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

cohttp.6.0.0~alpha0 uses the bytes library #23262

Merged
merged 3 commits into from
Feb 9, 2023

Conversation

emillon
Copy link
Contributor

@emillon emillon commented Feb 8, 2023

Adjust constraint accordingly.

Adjust constraint accordingly.
@emillon
Copy link
Contributor Author

emillon commented Feb 8, 2023

I believe that's the right rune but it's the first time I'm doing it.

@mseri
Copy link
Member

mseri commented Feb 8, 2023

Do you have the log of the error? I can fix it upstream before the next release

@emillon
Copy link
Contributor Author

emillon commented Feb 9, 2023

Oops, sorry:

#=== ERROR while compiling cohttp.6.0.0~alpha0 ================================#
# context              2.2.0~alpha~dev | linux/x86_64 | ocaml-base-compiler.5.0.0 | file:///home/opam/opam-repository
# path                 ~/.opam/5.0/.opam-switch/build/cohttp.6.0.0~alpha0
# command              ~/.opam/opam-init/hooks/sandbox.sh build dune build -p cohttp -j 31 --promote-install-files=false @install
# exit-code            1
# env-file             ~/.opam/log/cohttp-7-a468f1.env
# output-file          ~/.opam/log/cohttp-7-a468f1.out
### output ###
# File "cohttp/src/dune", line 19, characters 2-7:
# 19 |   bytes
#        ^^^^^
# Error: Library "bytes" not found.
# -> required by library "cohttp" in _build/default/cohttp/src
# -> required by _build/default/META.cohttp
# -> required by _build/install/default/lib/cohttp/META
# -> required by _build/default/cohttp.install
# -> required by alias install

So the upstream fix is likely to just remove bytes from libraries.

mseri added a commit to mseri/ocaml-cohttp that referenced this pull request Feb 9, 2023
The failure was seen in the opam-repository CI, see ocaml/opam-repository#23262 (comment)

Signed-off-by: Marcello Seri <marcello.seri@gmail.com>
@mseri
Copy link
Member

mseri commented Feb 9, 2023

I am suprised that if fails with dune >= 2.9. We used the following fix in the past in the opam metadata:

("dune" {>= "1.4.0"} | "dune" {< "1.4.0"} & "base-bytes")

So this one should have worked. Does it have to do with further changes in dune?

@emillon
Copy link
Contributor Author

emillon commented Feb 9, 2023

I don't think so, but I'm not completely up to date about what changed in 5.0. Possibly the compiler-provided META files (that dune uses if dune >= 2.9-ish and ocaml >= 5) do not include bytes?

In any case, this seems to be similar to #23012 for example which would fail with a recent dune too.

@mseri
Copy link
Member

mseri commented Feb 9, 2023

Thanks, I updated the wiki to mention the need for both changes.

@mseri
Copy link
Member

mseri commented Feb 9, 2023

Cohttp top fails with

[14:14](https://ocamllabs.slack.com/archives/C0483TAL30F/p1675948443818109)

#=== ERROR while compiling cohttp-top.1.0.0 ===================================#
# context              2.2.0~alpha~dev | linux/x86_64 | ocaml-base-compiler.4.14.1 | file:///home/opam/opam-repository
# path                 ~/.opam/4.14/.opam-switch/build/cohttp-top.1.0.0
# command              ~/.opam/opam-init/hooks/sandbox.sh build jbuilder build -p cohttp-top -j 47
# exit-code            1
# env-file             ~/.opam/log/cohttp-top-7-495c32.env
# output-file          ~/.opam/log/cohttp-top-7-495c32.out
### output ###
# File "/home/opam/.opam/4.14/lib/cohttp/META", line 1, characters 0-0:
# Error: Library "bytes" not found.
# -> required by library "cohttp" in /home/opam/.opam/4.14/lib/cohttp
# Hint: try: jbuilder external-lib-deps --missing -p cohttp-top @install

Fails with
```
File "lwt/websocket_lwt_unix.ml", line 87, characters 35-37:
# 87 |     (fun () -> drain_handshake req ic oc nonce)
#                                         ^^
# Error: This expression has type
#          Conduit_lwt_unix.ic = Lwt_io.input Lwt_io.channel
#        but an expression was expected of type
#          Response.IO.ic = Cohttp_lwt_unix__Input_channel.t
```
@mseri
Copy link
Member

mseri commented Feb 9, 2023

All other failures are addressed in separate PRs

@mseri mseri merged commit 5206ac7 into ocaml:master Feb 9, 2023
@mseri
Copy link
Member

mseri commented Feb 9, 2023

Thanks

bikallem pushed a commit to bikallem/ocaml-cohttp that referenced this pull request Feb 15, 2023
The failure was seen in the opam-repository CI, see ocaml/opam-repository#23262 (comment)

Signed-off-by: Marcello Seri <marcello.seri@gmail.com>
bikallem pushed a commit to bikallem/ocaml-cohttp that referenced this pull request Feb 15, 2023
The failure was seen in the opam-repository CI, see ocaml/opam-repository#23262 (comment)

Signed-off-by: Marcello Seri <marcello.seri@gmail.com>
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

Successfully merging this pull request may close these issues.

2 participants