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

Remove the compiler flag from ocaml-compiler #21

Closed
wants to merge 1 commit into from

Conversation

dra27
Copy link
Owner

@dra27 dra27 commented Dec 6, 2024

Spotted by @kit-ty-kate. At present, if you run opam init -c 4.14.2 (or opam switch create 4.14.2) you get a switch invariant choosing between either ocaml-system.4.14.2 or ocaml-base-compiler.4.14.2

With the new ocaml-compiler package layout, you'd now also get ocaml-compiler.4.14.2 - but this version allows optional reconfiguration options (flambda, etc.) which is a user surprise we should avoid.

The fix is simple and is to remove the compiler flag from the package. This makes explicitly using ocaml-compiler in opam 2.0 slightly more awkward, but that hasn't been being suggested anyway.

The presence of the flag in both ocaml-base-compiler and ocaml-compiler
causes opam's invariant computation to produce a poor invariant:

e.g. opam switch create 5.3.0~beta2 produced an invariant of
  "ocaml-base-compiler" {= "5.3.0~beta2"} | "ocaml-compiler" {= "5.3.0~beta2"}
instead of the desired
  "ocaml-base-compiler" {= "5.3.0~beta2"}
@dra27
Copy link
Owner Author

dra27 commented Dec 6, 2024

I'd recommend we pull this commit in just before the release candidate (which would want the same thing), just to stop churn?

cc @Octachron, @mseri, @raphael-proust

@mseri
Copy link

mseri commented Dec 6, 2024

I think it is a good idea

@raphael-proust
Copy link

seems ok 👍

@kit-ty-kate
Copy link

This PR can now be closed as ocaml#27145 was merged

@dra27
Copy link
Owner Author

dra27 commented Jan 7, 2025

Ideed!

@dra27 dra27 closed this Jan 7, 2025
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.

4 participants