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

[Proposal] Add previews of some of the platform packages with OCaml 4.13 compatibility #18825

Merged
merged 5 commits into from
Jul 12, 2021

Conversation

kit-ty-kate
Copy link
Member

@kit-ty-kate kit-ty-kate commented Jun 8, 2021

This PR copies some of the platforms packages from https://github.com/kit-ty-kate/opam-alpha-repository/ to make OCaml 4.13 simpler to test for people.

Proposal

This PR adds a preview of the next version of ocamlformat, odoc and mdx namely:

  • ocamlformat.0.19.0~4.13preview
  • odoc.1.5.3~4.13preview
  • mdx.1.11.0~4.13preview

For the moment they point to specific git branches to make them support OCaml 4.13. They all have their corresponding PRs or are already merged in their main branch (see #18791 for details)

The goal is to update the package and the branch as the alpha/beta cycle progresses (there should only be one version of these preview packages at all time, no ~4.13preview2 and whatnot), and once the proper release for them land, we can simply mark it as available: false and forget about the previews.

This version number scheme should allow you to still release alpha/beta/rc versions and let them be taken first above those preview packages. It also allows us to be more explicit about what this version is (a simple preview of this package's next version with OCaml 4.13 support)

In parallel, those packages will also be available in opam-alpha-repository but marked available with the older stable versions of the compiler for more broad testing purpose.

cc @jonludlam for odoc
cc @NathanReb @emillon for mdx
cc @gpetiot @emillon for ocamlformat

What are your (package maintainers) views on this? Are the proposed version numbers the correct one for the next release?

@rgrinberg
Copy link
Member

Sounds good to me.

@avsm
Copy link
Member

avsm commented Jun 9, 2021

Superb, thanks @kit-ty-kate! In the interests of longer term reproducibility, how about replacing the git branches with explicit sha256 changesets instead? This'll be a little more work for us to maintain, but in return those packages will build into the longer term with just a checkout of opam-repository.

@pitag-ha
Copy link
Member

Thanks @kit-ty-kate! For OMP2 that sounds good to me. For ppxlib in the end we'll follow your idea and add the 4.13 support on top of the last release rather than the main branch for now, so the first ppxlib version supporting 4.13 will be

ppxlib.0.22.2

It will still depend on OMP2 and it isn't implemented yet, but we can re-use one of your commits (thanks for separating them!).

@avsm
Copy link
Member

avsm commented Jun 21, 2021

Thinking about this more:

how about replacing the git branches with explicit sha256 changesets instead

I think merging just the branches as they are in this PR is fine, and then "freeze" them to sha256 changesets when we release 4.13 and mark the package as unavailable. That'll preserve the history but not make day-to-day development a pain.

@kit-ty-kate kit-ty-kate marked this pull request as ready for review June 23, 2021 09:36
@kit-ty-kate
Copy link
Member Author

I've removed OMP from the PR as it was properly released with OCaml 4.13 support yesterday in #18902

@kit-ty-kate kit-ty-kate force-pushed the 413-alpha-pkgs branch 3 times, most recently from c7c7199 to 5b84634 Compare June 23, 2021 19:29
@avsm
Copy link
Member

avsm commented Jun 24, 2021

@Octachron @jonludlam this appears to be failing on 4.13...

#   The return type of this pattern-matching is ambiguous.
#   Please add a type annotation, as the choice of `s' is not principal.
# File "src/parser_automaton_internal.ml", line 485, characters 44-66:
# 485 |     | Sexp -> if is_not_ignoring state then Sexp (Atom str, stack) else stack
#                                                   ^^^^^^^^^^^^^^^^^^^^^^
# Error: The constructor Sexp expects 0 argument(s),
#        but is applied here to 1 argument(s)
#     ocamlopt src/.parsexp.objs/native/parsexp__Parser_automaton_internal.{cmx,o} (exit 2)
# (cd _build/default && /home/opam/.opam/4.13/bin/ocamlopt.opt -w -40 -g -I src/.parsexp.objs/byte -I src/.parsexp.objs/native -I /home/opam/.opam/4.13/lib/base/caml -I /home/opam/.opam/4.13/lib/sexplib0 -intf-suffix .ml -no-alias-deps -open Parsexp__ -o src/.parsexp.objs/native/parsexp__Parser_automaton_internal.cmx -c -impl src/parser_automaton_internal.ml)
# File "src/parser_automaton_internal.ml", lines 440-454, characters 6-66:
# 440 | ......match state.kind with
# 441 |       | Positions ->
# 442 |         (* Note we store end positions as inclusive in [Positions.t], so we use [delta:0],
# 443 |            while in the [Cst] case we save directly the final ranges, so we use
# 444 |            [delta:1]. *)
# ...
# 451 |           add_pos state ~delta:0;
# 452 |           make_list [] stack)
# 453 |         else stack
# 454 |       | Cst -> make_list_cst (current_pos state ~delta:1) [] stack
# Warning 18 [not-principal]: 
#   The return type of this pattern-matching is ambiguous.
#   Please add a type annotation, as the choice of `s' is not principal.
# File "src/parser_automaton_internal.ml", line 485, characters 44-66:
# 485 |     | Sexp -> if is_not_ignoring state then Sexp (Atom str, stack) else stack
#                                                   ^^^^^^^^^^^^^^^^^^^^^^
# Error: The constructor Sexp expects 0 argument(s),
#        but is applied here to 1 argument(s)

@kit-ty-kate
Copy link
Member Author

This failure is in parsexp not odoc. It's a known issue in the first alpha: ocaml/ocaml#10364
I have a temporary fix in opam-alpha-repository here: https://github.com/kit-ty-kate/parsexp/tree/v0.14, I might have to move it to opam-repository then.

@avsm
Copy link
Member

avsm commented Jun 30, 2021

This should work after #18977 is merged

@kit-ty-kate
Copy link
Member Author

Given the rest of the packages were all released and ppxlib.0.23.0 is not necessary anymore I've updated this PR to include ocamlformat, mdx and odoc (1.5 branch, instead of 2.0)

Are the proposed version numbers what they are expected to be when the support for OCaml 4.13 is going to be included?

@gpetiot
Copy link
Contributor

gpetiot commented Jul 6, 2021

The version number for ocamlformat is correct indeed (0.19.0), I just merged the 4.13 support by the way. The release is expected before July 16th.

@kit-ty-kate kit-ty-kate merged commit 087b76e into ocaml:master Jul 12, 2021
@kit-ty-kate kit-ty-kate deleted the 413-alpha-pkgs branch July 23, 2021 14:40
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.

5 participants