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

Clarify OCaml version constraint #274

Merged
merged 1 commit into from
Aug 9, 2022

Conversation

talex5
Copy link
Collaborator

@talex5 talex5 commented Aug 4, 2022

ocaml-base-compiler is version 5.0.0~alpha1 but ocaml itself is just 5.0.0.

(spotted by @kit-ty-kate in ocaml/opam-repository#21935)

`ocaml-base-compiler` is version `5.0.0~alpha1` but `ocaml` itself is
just `5.0.0`.
@kit-ty-kate
Copy link
Contributor

could you also add the "@runtest" {with-test & os != "macos"} for eio-main too?

@talex5
Copy link
Collaborator Author

talex5 commented Aug 4, 2022

could you also add the "@runtest" {with-test & os != "macos"} for eio-main too?

I don't think so. That bit is generated by dune.

@kit-ty-kate
Copy link
Contributor

you can always stop generating the opam file by dune. It’s fairly useless and more bothersome than anything once first generated IMO

@talex5
Copy link
Collaborator Author

talex5 commented Aug 5, 2022

It would be better to disable just the network tests. I can disable them on macos (I think) by changing the dune file to:

(mdx
  (package eio_main)
  (packages eio_main)
  (files (:standard \ "network.md")))

(mdx
  (package eio_main)
  (packages eio_main)
  (enabled_if (<> %{system} "macos"))
  (files "network.md"))

However, that disables the tests even outside of the opam sandbox, which isn't what we want.

A better option might be to add a way to skip blocks in MDX if some condition is met. Then we could e.g. skip the IPv6 tests if binding to ::1 raises an exception.

@talex5 talex5 merged commit 047c72b into ocaml-multicore:main Aug 9, 2022
@talex5 talex5 deleted the ocaml-version branch August 9, 2022 08:25
@talex5
Copy link
Collaborator Author

talex5 commented Aug 9, 2022

Merging this part for now. I've opened #275 to track the test problem.

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