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

Test packages on the full range of supported OCaml versions #297

Closed
craigfe opened this issue Jan 8, 2021 · 6 comments · Fixed by #447
Closed

Test packages on the full range of supported OCaml versions #297

craigfe opened this issue Jan 8, 2021 · 6 comments · Fixed by #447

Comments

@craigfe
Copy link
Member

craigfe commented Jan 8, 2021

To my understanding, the analysis phase for Opam projects works roughly as follows:

  • determine the set of OCaml versions at which all packages are co-installable (by attempting to solve the constraints);
  • for each such version, emit the set of opam packages to install (as returned by the constraint solver).

This has the consequence that only OCaml versions at which all packages are co-installable are tested in CI, even if certain packages have support for a larger set. This is problematic for repositories containing many packages, since it can severely limit the test matrix. For instance, Irmin is currently only tested on OCaml 4.11 despite almost all of the packages supporting back to at least 4.08.

It would be nice to have a mechanism that tests each package on the full range of OCaml versions that it (allegedly) supports. An obvious solution is to branch the pipeline for each package and install dependencies separately – as was previously done with ocaml-ci-scripts – but this is inefficient and doesn't provide any information about co-installability. Here's an initial proposal for how we might do better:

  • for each OCaml version in the test matrix, determine the largest co-installable subset of opam packages p_i;
  • for each non-empty p_i, emit the set of opam dependencies to install and the set of Dune packages d_i corresponding to p_i. The test phase can then use dune runtest --only-packages [... d_i] to ensure that only these packages are tested at that version.

The magic here is in calculating the p_is and d_is. I suspect p_i can be approximated by initially assuming total co-installability and then using counter-examples to iterate downwards, but perhaps there is better built-in solver magic for this. A complication is that bizarre sets of constraints can result in multiple p_is, in which case it seems sufficient to just pick one deterministically. Once that's done, d_i might be computable from p_i using opam-dune-lint?

@talex5
Copy link
Contributor

talex5 commented Jan 8, 2021

This might be easier:

If a package's opam file contains an explicit constraint on the version of OCaml then:

  • Exclude it from non-matching versions
  • Force it to be included for matching versions (complain if not)

@craigfe
Copy link
Member Author

craigfe commented Jan 8, 2021

If a package's opam file contains an explicit constraint on the version of OCaml then:

* Exclude it from non-matching versions

* Force it to be included for matching versions (complain if not)

I'm not sure I like the idea of adding artificial constraints in .opam files purely to signal intention to the CI – it's not easy to check when / if it's possible to relax those constraints – but it's definitely easier to implement and clearly better than the prior approach of replicating this info in config files. I'd be happy with this as a solution 🙂

@dra27
Copy link
Contributor

dra27 commented Jan 9, 2021

Given the potential for stopping testing a version without realising it, might it be an idea to be stronger and actually require a version constraint on ocaml?

@dra27
Copy link
Contributor

dra27 commented Jan 9, 2021

So if you have no constraint on OCaml, then your package should be testing on all supported versions, and either the lint or analyse phases will fail if packages aren’t installable in a given version

@talex5
Copy link
Contributor

talex5 commented Jan 10, 2021

Given the potential for stopping testing a version without realising it, might it be an idea to be stronger and actually require a version constraint on ocaml?

Yes, I've been wondering about that too.

My only concern is that sometimes a package is temporarily uninstallable on a new compiler release due to missing dependencies (e.g. Lwt not available on OCaml 4.12). We don't want to encourage people to go around adding ocaml < 4.12 constraints everywhere while that's being fixed.

But for lower bounds at least it seems fine. So if I say ocaml >= 4.02 then it must work with 4.02.

@avsm
Copy link
Member

avsm commented Jan 22, 2021

This is also an issue for Lwt now, since lwt_ppx is 4.04 only, but the rest of the opam packages should build on 4.02 but aren't being tested.

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 a pull request may close this issue.

4 participants