The tests fail when using OCaml 5.1.1

[kit-ty-kate]

#=== ERROR while compiling ppxlib.0.30.0 ======================================#
# context              2.2.0~alpha3 | linux/x86_64 | ocaml-variants.5.1.1+trunk | file:///home/opam/opam-repository
# path                 ~/.opam/5.1.1/.opam-switch/build/ppxlib.0.30.0
# command              ~/.opam/5.1.1/bin/dune build -p ppxlib -j 1 @install @runtest
# exit-code            1
# env-file             ~/.opam/log/ppxlib-557-0157af.env
# output-file          ~/.opam/log/ppxlib-557-0157af.out
### output ###
# File "test/501_migrations/normal_migrations.t", line 1, characters 0-0:
# /usr/bin/git --no-pager diff --no-index --color=always -u _build/.sandbox/9c1746358f77f3bd8d0c52c5cf65af49/default/test/501_migrations/normal_migrations.t _build/.sandbox/9c1746358f77f3bd8d0c52c5cf65af49/default/test/501_migrations/normal_migrations.t.corrected
# diff --git a/_build/.sandbox/9c1746358f77f3bd8d0c52c5cf65af49/default/test/501_migrations/normal_migrations.t b/_build/.sandbox/9c1746358f77f3bd8d0c52c5cf65af49/default/test/501_migrations/normal_migrations.t.corrected
# index c9d2c8d..5f94a8c 100644
# --- a/_build/.sandbox/9c1746358f77f3bd8d0c52c5cf65af49/default/test/501_migrations/normal_migrations.t
# +++ b/_build/.sandbox/9c1746358f77f3bd8d0c52c5cf65af49/default/test/501_migrations/normal_migrations.t.corrected
# @@ -49,46 +49,47 @@ Here might be a problem in the upward migration: the 5.1.0 parser parses the con
#  However, the upward migration makes a value binding constraint out of it.
#    $ echo "let ((x,y) : (int*int)) = (assert false: int * int)" > file.ml
#    $ ./compare_on.exe file.ml ./identity_driver.exe
# -  6,25c6,23
# -  <         pattern (file.ml[1,0+4]..[1,0+23])
# -  <           Ppat_constraint
# -  <           pattern (file.ml[1,0+5]..[1,0+10])
# -  <             Ppat_tuple
# -  <             [
# -  <               pattern (file.ml[1,0+6]..[1,0+7])
# -  <                 Ppat_var "x" (file.ml[1,0+6]..[1,0+7])
# -  <               pattern (file.ml[1,0+8]..[1,0+9])
# -  <                 Ppat_var "y" (file.ml[1,0+8]..[1,0+9])
# -  <             ]
# -  <           core_type (file.ml[1,0+14]..[1,0+21])
# -  <             Ptyp_tuple
# -  <             [
# -  <               core_type (file.ml[1,0+14]..[1,0+17])
# -  <                 Ptyp_constr "int" (file.ml[1,0+14]..[1,0+17])
# -  <                 []
# -  <               core_type (file.ml[1,0+18]..[1,0+21])
# -  <                 Ptyp_constr "int" (file.ml[1,0+18]..[1,0+21])
# -  <                 []
# -  <             ]
# -  ---
# -  >         pattern (file.ml[1,0+5]..[1,0+10])
# -  >           Ppat_tuple
# -  >           [
# -  >             pattern (file.ml[1,0+6]..[1,0+7])
# -  >               Ppat_var "x" (file.ml[1,0+6]..[1,0+7])
# -  >             pattern (file.ml[1,0+8]..[1,0+9])
# -  >               Ppat_var "y" (file.ml[1,0+8]..[1,0+9])
# -  >           ]
# -  >         core_type (file.ml[1,0+14]..[1,0+21])
# -  >           Ptyp_tuple
# -  >           [
# -  >             core_type (file.ml[1,0+14]..[1,0+17])
# -  >               Ptyp_constr "int" (file.ml[1,0+14]..[1,0+17])
# -  >               []
# -  >             core_type (file.ml[1,0+18]..[1,0+21])
# -  >               Ptyp_constr "int" (file.ml[1,0+18]..[1,0+21])
# -  >               []
# -  >           ]
# +  --- without_migrations 2023-11-29 10:59:13.141022958 +0000
# +  +++ with_migrations    2023-11-29 10:59:13.394352161 +0000
# +  @@ -6,20 +6,18 @@
# +  -        pattern (file.ml[1,0+4]..[1,0+23])
# +  -          Ppat_constraint
# +  -          pattern (file.ml[1,0+5]..[1,0+10])
# +  -            Ppat_tuple
# +  -            [
# +  -              pattern (file.ml[1,0+6]..[1,0+7])
# +  -                Ppat_var "x" (file.ml[1,0+6]..[1,0+7])
# +  -              pattern (file.ml[1,0+8]..[1,0+9])
# +  -                Ppat_var "y" (file.ml[1,0+8]..[1,0+9])
# +  -            ]
# +  -          core_type (file.ml[1,0+14]..[1,0+21])
# +  -            Ptyp_tuple
# +  -            [
# +  -              core_type (file.ml[1,0+14]..[1,0+17])
# +  -                Ptyp_constr "int" (file.ml[1,0+14]..[1,0+17])
# +  -                []
# +  -              core_type (file.ml[1,0+18]..[1,0+21])
# +  -                Ptyp_constr "int" (file.ml[1,0+18]..[1,0+21])
# +  -                []
# +  -            ]
# +  +        pattern (file.ml[1,0+5]..[1,0+10])
# +  +          Ppat_tuple
# +  +          [
# +  +            pattern (file.ml[1,0+6]..[1,0+7])
# +  +              Ppat_var "x" (file.ml[1,0+6]..[1,0+7])
# +  +            pattern (file.ml[1,0+8]..[1,0+9])
# +  +              Ppat_var "y" (file.ml[1,0+8]..[1,0+9])
# +  +          ]
# +  +        core_type (file.ml[1,0+14]..[1,0+21])
# +  +          Ptyp_tuple
# +  +          [
# +  +            core_type (file.ml[1,0+14]..[1,0+17])
# +  +              Ptyp_constr "int" (file.ml[1,0+14]..[1,0+17])
# +  +              []
# +  +            core_type (file.ml[1,0+18]..[1,0+21])
# +  +              Ptyp_constr "int" (file.ml[1,0+18]..[1,0+21])
# +  +              []
# +  +          ]
#  
#    $ echo "let f: type a. a option -> _ = assert false" > file.ml
#    $ ./compare_on.exe file.ml ./identity_driver.exe

[kit-ty-kate]

ah nevermind it’s only ppxlib.0.30.0 not the latest ppxlib.0.31.0. Still I’m a bit puzzled at why it happens when it works fine with OCaml 5.1.0

[pitag-ha]

That's quite interesting! The reason why ppxlib.0.31.0 is fine, while ppxlib.0.30.0 isn't is that in 0.31.1, we fixed a location bug in one of the AST 5.0 <-> AST 5.1 migrations.

And about why it works fine with OCaml 5.0 and doesn't with OCaml 5.1: Might be because of https://github.com/ocaml/ocaml/issues/10852 and because in ppxlib.0.30.0, we kind of "overfitted" the locations in the migration to the example that's breaking here rather than getting them right generally, whereas in ppxlib.0.31.0 we (hopefully) get them right generally.

[kit-ty-kate]

should that version be concidered unavailable on opam-repository or just its test? Would you be able to do the PR?

[pitag-ha]

Ok, I've had a closer look and the problem is not in the diff itself, but in the way the diff is formatted. In 0.30.0 we hadn't unified the output via the diff's -U 0 option yet. So to answer your question:

should that version be concidered unavailable on opam-repository or just its test?

Just its tests.

Would you be able to do the PR?

Sure.

Thanks for noticing!

[pitag-ha]

Btw, the reason why the tests for ppxlib.0.30.0 pass for OCaml 5.1.0 is that when writing the test conditions, I assumed that 5.1.0 would be considered greater than 5.1.0~alpha2, but it seems to be the other way around. So on 5.1.0 (considered < 5.1.0~alpha2), the tests are simply not run, whereas on 5.1.1 (considered > 5.1.0~alpha2), they are.

Was 5.1.0~alpha1 the only 5.1.0 pre-release before 5.1.0~alpha2? (I.e. was there no beta release?) In that case, I can make the test condition better by simply saying ocaml >= "5.1.0"` and < "5.2.0" and != "5.1.0~alpha1".

[pitag-ha]

I've just opened the opam-repo PR.

Btw, in case you wonder: The reason why those tests needed to be disabled on any OCaml 5.1.0 pre-release before alpha2 is that 5.1.0.alpha2 introduced a parsetree change. It's very rare that a OCaml pre-release changes the parsetree, but in that particular case it happened.

[pitag-ha]

This is fixed on opam-repo now: https://github.com/ocaml/opam-repository/pull/24919

Thanks for reporting, @kit-ty-kate!