-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Re-enable a few packages for OCaml 5.3 #26831
Conversation
Add `OCAMLPARAM=_,keywords=5.2` to `build-env` which allows these packages to build in OCaml 5.3 by disabling the `effect` keyword during the build only.
Not sure that it's necessarily worth fixing, but this is a shame - the warning shouldn't trigger when the |
Caisar uses Re__core.replace_string directly, which got caught by a refactoring in 1.12.0
These errors are there because we are building against the alpha1 still, but should we add a conflict to prevent this to happen?
In the meantime I mark it as do not merge until we can test it on the beta1 |
It fails when building with ocaml-protoc-plugin.4.2.0 with 2.9.3: File "lib/onnx/dune", line 11, characters 11-30: 11 | (package ocaml-protoc-plugin)) ^^^^^^^^^^^^^^^^^^^ Error: Package ocaml-protoc-plugin does not exist Note that all subsequent versions of ocaml-proto-plugin require at least Dune 3.2, so the simple lowerbound is sufficient - there's no version of dependencies where this package can succesfully build with Dune 2.9.x
Indeed - the beta1 base image has just pushed! Up to you about the conflict - I wasn't sure if that was overly pedantic or not (for a pre-release)? |
Uses Option.value_or_thunk which was added in base v0.15.0
For the conflicts I did not have a strong opinion, but I think it is nice to prevent unnecessary failures when we can. Thanks a lot, it builds fine! |
OCaml 5.3.0~beta1 (merged in #26822) includes ocaml/ocaml#13471, which permits a few packages to be re-enabled for OCaml 5.3 (effectively reverting #26546 for coq-core, #26548 for yocaml, #26713 for prbnmcn-dagger, #26715 for mock, #26717 for MlFront_Cli, and #26719 for mopsa).
For MlFront_Cli and coq-core there are later versions which are already 5.3-compatible but note that for the others this actually enables the package for 5.3 (in yocaml's case, the later versions are unavailable on 5.3 for other reasons).
coq-core accidentally runs its configure script using the wrong profile, which is why the
build-env
for those also includeswarn-error=-a
- cf. coq/coq#19805 to ensure that wouldn't be necessary if this kind of change were made in future (or for anything else in Coq's configure program which may trigger warnings in a future version of OCaml).Note that omake and why3 were marked as conflicting base-effects at release.
irmin and octez-internal-libs (#26709 and #26711) cannot be fixed this way as the
effect
keyword appears within a ppx (something which might want to be considered separately if more invasive keywords get added to future OCaml releases).Overall, I think this is probably worth doing, as it increases the number of available packages. Note that these changes don't necessarily want to go upstream - the better fix upstream is to switch away from the
effect
keyword and cut a new release.