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

Update to dune 2 #1035

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Update to dune 2 #1035

wants to merge 5 commits into from

Conversation

shonfeder
Copy link

Closes #1034

Everything here should be a purely mechanical change required to use dune 2.0 syntax.

@shonfeder
Copy link
Author

It looks like the ocaml-ci failures here are the same 5 that are failing on master https://github.com/ocsigen/lwt/runs/31355838204 (FreeBSD and MacOs).

@raphael-proust
Copy link
Collaborator

We might be preventing some projects to update to more recent Lwt with this PR. I don't think it's a problem because it's project that have a dune<2.0 constraint. They already can't update much of their deps, probably innactive projects.

Still, for good measure I've grepped through the opam-repository and I found multiple packages which have dependency to dune with a version constraint < "2.0".

  • 0install.2.14 and 0install.2.14.1: it's fine because they have a constraints for lwt to be < "5.0.0"
  • base_quickcheck.v0.12.0: maybe fine because it depends on base < "V0.13" which is somewhat dated?
  • blake2.0.1 and blake2.0.2: probably fine because it's not widely used (it's a tezos thing) and is somewhat dated (version 0.3 is out)
  • bun.0.3.3: probably fine because there is a more recent releases, and because it's a binary intended to be used in the CI, so it doesn't mess-up users' dependency cones
  • bytearray.1.0,0: probably fine because it has ocaml<4.10, several newer point releases, no "used by" in the opam-repo
  • core_kernel.v12.*: same as base_quickcheck
  • csv.2.1: that one is used a lot and has a csv-lwt companion package, although there are newer releases and this version is from 2018
  • dokeysto.3.0.0 and dokeysto_*.3.0.0: probably fine because there are point-releases with looser constraints as well as newer releases
  • dolmen.0.4: probably ok because there are point releases following
  • encore.0.2: probably breaks users of angstrom-lwt-unix.0.14.0+dune<2.0 but maybe ok because there are newer releases for all of those.
  • ezsqlite.0.41 and ezsqlite.0.4.1but notezsqlite.0.4.2` so probably ok, especially because this doesn't seem to be an Lwt-aware library.
  • fluent_logger probably ok because is not Lwt-aware
  • gemini.0.1 and gemini.0.2.0: ok because uses async
  • get_line<=6.0 probably ok because it's a binary
  • hacl<=0.2 probably ok because not widely used (tezos only?)
  • herdtools7.7.54 probably ok because there are newer releases and it seems to be for the purpose of running some simulations
  • mccs.1.1+[5-9] probably ok because newer "plus" releases
  • minicli.5.0.0 and minicli.5.0.1: probably ok because there is minicli.5.0.2
  • minisat.0.2 probably ok because there are newer point releases
  • npy.0.0.8 maybe ok because there is a 0.0.9
  • ocaml-migrate-parsetree.1.{1.0,0.11} (use jbuilder) but not the other versions so most likely ok
  • odoc.1.3.0 probably fine because other versions
  • open maybe ok because it's a binary rather than a library?
  • orsvm_e1071.3.0.2, potentially breaking because the next version is 4.0.0 so a major release, on the other hand the deps seem to suggest use of parmap so unlikely to be used in lwt?
  • some parany versions but I don't think it's recommended to mix with lwt
  • patdiff.v0.12.0 most likely ok because newer versions and binary
  • ppx_deriving_cmdliner.0.4.1 probs ok because newer releases
  • ppxlib.0.2.1 and ppxlib.0.3.0 most likely ok because there are so many newer releases
  • qtest.2.10 probs ok because there's a point release following and some more releases after that
  • re2.v0.12.0 point release following plus more releases after
  • some reason version but nothing too recent
  • relit-reason all versions
  • rosetta.0.1.0 maybe ok because further versions
  • some rtop versions but nothing too recent
  • setcore.1.0.1 maybe ok because there's one point release afterwards
  • tensorflow.0.0.11 possibly breaking because it's the only version that even supports dune,. but quite dated
  • uecc.0.1 but has further point release and seem to be tezos only
  • uri.2.0.0 and uri.2.1.0 but there are much newer versions

At this point I timed out, but I think it's fine. There are a couple of things that could potentially be maybe interesting to look into… but nothing screams for attention.

@raphael-proust
Copy link
Collaborator

Maybe there's a better way to handle the conditional preprocessing than the current

let preprocess =
   match Sys.getenv "BISECT_ENABLE" with
   | "yes" -> "(preprocess (pps bisect_ppx))"
   | _ -> ""
   | exception _ -> ""
let () = Jbuild_plugin.V1.send @@

dune envs maybe?

Anyway, that's orthogonal, I'll look into it later.

@raphael-proust
Copy link
Collaborator

I'm trying to fix issues with the CI

@raphael-proust
Copy link
Collaborator

I removed some tests that were not used

  • packaging tests: not ran from anywhere unless implicitely by dune @runtest and then kind of incorrectly anyway, uneeded because opam-repository CI runs revdeps checks which ensures we don't break packaging for users
  • ppx_expect tests: not ran from anywhere, had some type errors when I tried to activate them

The CI is much greener now. I think we can merge this. @smorimoto let me know if you disagree.

@@ -22,3 +22,4 @@ build: [
"@doc" {with-doc}
]
]
available: os-family != "windows" | "ocaml-version" < "5.2"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why restrict Windows support? Does this mean Lwt on Windows is only supported on 5.2?

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.

Update to dune 2.0
3 participants