Prevent unecessary selection of beta versions #269
Conversation
There was a problem hiding this comment.
I think this is a nice solution and I especially like how flexible this is to not only work for the compiler but also for any other package (like alpha/beta/release candidate releases etc) that has that flag set. My comments are mostly about making the test better/more reliable.
| @@ -0,0 +1,132 @@ | |||
| We have a simple project that only depends on OCaml | |||
There was a problem hiding this comment.
Since the code doesn't do anything compiler-specific (which is a good thing!) maybe it would make sense to just set up dummy packages? Dummy packages can have more monimal opam files that are easier to grasp and review.
There was a problem hiding this comment.
That would work but I like the fact that it shows it solves a real world problem, with real world metadata.
There was a problem hiding this comment.
I agree in principle, but in practice I find these files very long and distracting to read (if there is something wrong, do you want to go through the whole file and try to understand what each of these stanzas does and how it interacts with the others?), so to review it I would need to read through every stanza, understand it, see if it is important etc. I would prefer the tests to concentrate on one aspect and show it working.
Kinda like we want to use minimal examples for tests, so reading and modifying these is easy instead of dealing with accidental complexity.
There was a problem hiding this comment.
I think here it's important that it also checks that it works with the ocaml and ocaml-base-compiler split. If we change this test we can still break this behaviour while having the dummy tests pass. I'm happy to extract another test just for the avoid-version if you'd like but this test will stay as a regression test.
Signed-off-by: Nathan Rebours <[email protected]>
Signed-off-by: Nathan Rebours <[email protected]>
Signed-off-by: Nathan Rebours <[email protected]>
Signed-off-by: Nathan Rebours <[email protected]>
Signed-off-by: Nathan Rebours <[email protected]>
Signed-off-by: Nathan Rebours <[email protected]>
Signed-off-by: Nathan Rebours <[email protected]>
NathanReb
force-pushed
the
deal-with-avoid-version
branch
from
575cf25
to
96f571c
Compare
|
Rebased! |
Signed-off-by: Nathan Rebours <[email protected]>
CHANGES: ### Added - Add opam extensions `x-opam-monorepo-opam-repositories` and `x-opam-monorepo-global-opam-vars` to make `lock` fully reproducible. (tarides/opam-monorepo#250, tarides/opam-monorepo#253, @NathanReb) - Show an error message when the solver can't find any version that satisfies the requested version constraint in the user's OPAM file (tarides/opam-monorepo#215, tarides/opam-monorepo#248, @Leonidas-from-XIV) - Allow packages to be marked as being provided by Opam and not to be pulled by `opam-monorepo`. To control this a new optional Opam file field, `x-opam-monorepo-opam-provided` is introduced. Its value is a list of package names that are to be excluded from being pulled (tarides/opam-monorepo#234, @Leonidas-from-XIV) - Show an error message when the OCaml version of the lock file does not match the OCaml version of the switch (tarides/opam-monorepo#267, tarides/opam-monorepo#268, @Leonidas-from-XIV) - Generate a `duniverse/README.md` file to explain the basics of `opam-monorepo` in the vendored directory (tarides/opam-monorepo#272, tarides/opam-monorepo#274, @Leonidas-from-XIV) - Add a `--prefer-cross-compile` flag for the solver to select cross-compiling versions of packages when available. This is determined through the presence of the `"cross-compile"` tag in the opam metadata. ### Changed - Bump lockfile version to 0.3 (tarides/opam-monorepo#285, @NathanReb) - Mark packages to be pulled by opam-monorepo with the `vendor` variable so using OPAM with `opam install --deps-only --locked .` will not install packages that will be installed with `opam-monorepo pull` (tarides/opam-monorepo#237, @Leonidas-from-XIV) ### Deprecated ### Fixed - Fix a bug where a package which had a single version that built with dune and got selected by the solver would be reported has having no version building with dune. (tarides/opam-monorepo#245, @Leonidas-from-XIV) - Fix the solver so it does not select beta versions of the compiler unless forced to by version constraints or `--ocaml-version`. (tarides/opam-monorepo#269, @NathanReb) ### Removed - Drop support for lockfile versions 0.2 and lower (tarides/opam-monorepo#285, @NathanReb) ### Security
CHANGES: ### Added - Add opam extensions `x-opam-monorepo-opam-repositories` and `x-opam-monorepo-global-opam-vars` to make `lock` fully reproducible. (tarides/opam-monorepo#250, tarides/opam-monorepo#253, @NathanReb) - Show an error message when the solver can't find any version that satisfies the requested version constraint in the user's OPAM file (tarides/opam-monorepo#215, tarides/opam-monorepo#248, tarides/opam-monorepo#290 @Leonidas-from-XIV) - Allow packages to be marked as being provided by Opam and not to be pulled by `opam-monorepo`. To control this a new optional Opam file field, `x-opam-monorepo-opam-provided` is introduced. Its value is a list of package names that are to be excluded from being pulled (tarides/opam-monorepo#234, @Leonidas-from-XIV) - Show an error message when the OCaml version of the lock file does not match the OCaml version of the switch (tarides/opam-monorepo#267, tarides/opam-monorepo#268, @Leonidas-from-XIV) - Generate a `duniverse/README.md` file to explain the basics of `opam-monorepo` in the vendored directory (tarides/opam-monorepo#272, tarides/opam-monorepo#274, @Leonidas-from-XIV) - Add a `--prefer-cross-compile` flag for the solver to select cross-compiling versions of packages when available. This is determined through the presence of the `"cross-compile"` tag in the opam metadata. ### Changed - Bump lockfile version to 0.3 (tarides/opam-monorepo#285, @NathanReb) - Mark packages to be pulled by opam-monorepo with the `vendor` variable so using OPAM with `opam install --deps-only --locked .` will not install packages that will be installed with `opam-monorepo pull` (tarides/opam-monorepo#237, @Leonidas-from-XIV) ### Fixed - Fix a bug where a package which had a single version that built with dune and got selected by the solver would be reported has having no version building with dune. (tarides/opam-monorepo#245, @Leonidas-from-XIV) - Fix the solver so it does not select beta versions of the compiler unless forced to by version constraints or `--ocaml-version`. (tarides/opam-monorepo#269, @NathanReb) ### Removed - Drop support for lockfile versions 0.2 and lower (tarides/opam-monorepo#285, @NathanReb)
Fixes #265 !
Starting recently, beta versions of the compiler are released as
ocaml-base-compilerand notocaml-variantanymore. This had the side effect of making our solver pick beta versions if the constraints allowed it.This fixes this issue but making the solver prefer regular versions over ones flagged with
avoid-versionin their opam metadata.Such versions can still be selected when no regular version match the requirements.