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

Merged
merged 6 commits into from
Oct 29, 2024
Merged

Update to dune 2 #1035

merged 6 commits into from
Oct 29, 2024

Conversation

shonfeder
Copy link
Contributor

Closes #1034

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

@shonfeder
Copy link
Contributor 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).

@shonfeder shonfeder mentioned this pull request Oct 22, 2024
@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.

lwt.opam.template Outdated Show resolved Hide resolved
@shonfeder
Copy link
Contributor Author

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.

See #1034 where I also tried to analyze the impact, which I expect to be quite small.

(introduced by error when releasing 5.8.0)
@raphael-proust
Copy link
Collaborator

AFAIK, this is ready to merge, I'll do it tomorrow unless someone complains

@shonfeder
Copy link
Contributor Author

Thank you for the review and the additional fixes!

@raphael-proust raphael-proust merged commit 8f45a63 into ocsigen:master Oct 29, 2024
27 of 28 checks passed
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