-
Notifications
You must be signed in to change notification settings - Fork 409
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
pkg: avoid unstable compiler packages #10596
Conversation
dd77b0f
to
1e0f839
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to split the compiler specific hack with the support for avoid-version
.
Also, this implementation with a bunch of post processing steps is rather confusing. Is it possible to just put all of this logic into to the comparison function we pass to the sort? This a much simpler way of thinking about what version we're going to get.
src/dune_pkg/opam_solver.ml
Outdated
released ocaml-base-compiler, in which case the ocaml | ||
package will depend development releases of the underlying | ||
compiler, such as +trunk releases of | ||
ocaml-variants. Development releases of packages are |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this PR implementing support for the avoid-version flag? That is, it's taking this flag into account when returning the list of available versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No not completely. After this change the solver will respect avoid-version
between different versions a given package (e.g. avoiding beta releases of the compiler) however it will not respect avoid-version
between different packages in general. If foo
depends on the disjunction bar | baz
and bar
is avoid-version
while baz
is not, bar
may still be chosen to satisfy the constraint.
I'll move the part of this change that reorders package versions by |
Here is a PR that reorders package versions by |
This introduces two workarounds to problems caused by the fact that opam-0install-solver - the library underlying dune's dependency solver - does not understand the avoid-version flag which is set on opam packages to indicate that the solver should choose an alternative package if it's able to do so. The first problem happens when ocaml-base-complier has an alpha, beta, or rc package available with a higher version number than the latest stable release of ocaml-base-compiler. In this case, dune would choose the higher versioned (unstable) package rather than the latest stable version. The workaround is to reorder the list of candidate versions for each package such that packages that have the avoid-version flag set appear later than those that do not have this flag set. (Note that this is done per-package name, not between different package names.) This solution will fix the problem for all packages - not just ocaml-base-compiler. The second problem happens when a version of the meta package named "ocaml" is available with a higher version than the latest version of ocaml-base-compiler, and there is a development version of ocaml-variants available which corresponds to the version of the latest "ocaml" package. In this case, dune would try to choose the latest version of "ocaml" and end up depending on the development version of ocaml-variants rather than ocaml-base-compiler. All versions of ocaml-variants have the avoid-version flag set. The workaround here is to determine which version of ocaml-base-compiler is the latest stable release (the version with the highest version number that does not have avoid-version set). Then, when considering the version candidates for the "ocaml" meta package, reorder the list of candidates such that any versions that are higher than the latest release of ocaml-base-compiler are moved to the end of the list. It's worth noting that in both cases, the solution does not restrict which package solutions are possible, it just changes the way that the solver prioritizes packages relative to one another; if a user specifies a dependency on a development version of a package, or the ocaml-variants package, it will still be included in the solution. Signed-off-by: Stephen Sherratt <stephen@sherra.tt>
1e0f839
to
82c31cf
Compare
Signed-off-by: Marek Kubica <marek@tarides.com>
Also includes a note that in the future this should work in a more expected way. Signed-off-by: Marek Kubica <marek@tarides.com>
the package. This is a hack to work around the fact that | ||
opam-0install-solver does not implement the avoid-versions | ||
flag. *) | ||
let move_avoid_versions_packages_to_end packages = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is needed? This is already handled by the new comparison function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is still needed, as we need to make sure the most recent version of the ocaml
package that depends on a non-avoid-version
compiler package is located in front of versions of ocaml
that have only packages that are marked avoid-version
.
E.g. the current state of the ocaml.5.3.0
depends on ocaml-base-compiler.5.3.0
(doesn't exist) or ocaml-variants {>= "5.3.0~" & < "5.3.1~"}
which can be fulfilled by ocaml-variants.5.3.0+trunk
.
Thus without this function the solver would attempt to pick ocaml.5.3.0
; see that there are no candidates for ocaml-base-compiler
and skip; see that there is one candidate for ocaml-variants
(we have sorted it to the back due to avoid-version
but the candidates list only contains one entry anyway) and pick that one.
This is a consistent pattern in opam-repository (that there is a new ocaml
package before the final release of ocaml-base-compiler
exists) that in many cases Dune would always pick ocaml-variants.<latest-development-release>+trunk
that this hack might be worth it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to break the logic for restricting the ocaml version to the latest stable ocaml-base-compiler version into its own PR for clarity, then close this PR.
I've made a PR that splits out the workaround for the issue where the solver always chooses unstable versions of the compiler, moving the logic into the version comparison function as @rgrinberg suggested. #10668 |
This introduces two workarounds to problems caused by the fact that opam-0install-solver - the library underlying dune's dependency solver - does not understand the avoid-version flag which is set on opam packages to indicate that the solver should choose an alternative package if it's able to do so.
The first problem happens when ocaml-base-complier has an alpha, beta, or rc package available with a higher version number than the latest stable release of ocaml-base-compiler. In this case, dune would choose the higher versioned (unstable) package rather than the latest stable version.
The workaround is to reorder the list of candidate versions for each package such that packages that have the avoid-version flag set appear later than those that do not have this flag set. (Note that this is done per-package name, not between different package names.) This solution will fix the problem for all packages - not just ocaml-base-compiler.
The second problem happens when a version of the meta package named "ocaml" is available with a higher version than the latest version of ocaml-base-compiler, and there is a development version of ocaml-variants available which corresponds to the version of the latest "ocaml" package. In this case, dune would try to choose the latest version of "ocaml" and end up depending on the development version of ocaml-variants rather than ocaml-base-compiler. All versions of ocaml-variants have the avoid-version flag set.
The workaround here is to determine which version of ocaml-base-compiler is the latest stable release (the version with the highest version number that does not have avoid-version set). Then, when considering the version candidates for the "ocaml" meta package, reorder the list of candidates such that any versions that are higher than the latest release of ocaml-base-compiler are moved to the end of the list.
It's worth noting that in both cases, the solution does not restrict which package solutions are possible, it just changes the way that the solver prioritizes packages relative to one another; if a user specifies a dependency on a development version of a package, or the ocaml-variants package, it will still be included in the solution.