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

Fix dependencies for eio and async #47

Merged
merged 5 commits into from
Nov 6, 2023
Merged

Fix dependencies for eio and async #47

merged 5 commits into from
Nov 6, 2023

Conversation

quernd
Copy link
Collaborator

@quernd quernd commented Nov 1, 2023

We don't require the h2-{async,eio,lwt} packages anymore in the .opam files, so we can't have them in the dune files either.

This was inadvertently broken when making the dependencies more consistent. It's hard to find in OCaml-CI because it installs the dependencies of all packages first (and the examples pull in everything):

RUN opam update --depexts && opam install --cli=2.1 --depext-only -y grpc.dev grpc-lwt.dev grpc-examples.dev grpc-eio.dev grpc-bench.dev grpc-async.dev $DEPS
RUN opam install $DEPS
COPY --chown=1000:1000 . /src
RUN opam exec -- dune build @install @check @runtest && rm -rf _build

(But it caused the Windows workflow to fail.)

OPAM in contrast checks package by package. Can we do that too?

We don't require the `h2-{async,eio,lwt}` packages anymore in the
`.opam` files, so we can't have them in the `dune` files either
@quernd quernd marked this pull request as ready for review November 3, 2023 14:57
@tmcgilchrist
Copy link
Collaborator

@quernd I added additional fixes for the example code and lower bounds on eio. Once it passes CI I will merge it.
Thanks.

@tmcgilchrist tmcgilchrist merged commit 783dc07 into main Nov 6, 2023
3 checks passed
@tmcgilchrist tmcgilchrist deleted the fix-dependencies branch November 6, 2023 00:43
@quernd
Copy link
Collaborator Author

quernd commented Nov 27, 2023

Thanks @tmcgilchrist! I don't know how I missed that, I could swear it was all green 🤔

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