ocaml / merlin

Context sensitive completion for OCaml in Vim and Emacs
https://ocaml.github.io/merlin/
MIT License
1.59k stars 233 forks source link

Compatibility with recent menhir #1824

Open SnarkBoojum opened 2 months ago

SnarkBoojum commented 2 months ago

Merlin 5.1-502 doesn't compile with menhir 20240715 ; I have an error report against the Debian package here ; here is the error message for easier reference:

File "src/ocaml/preprocess/recover/synthesis.ml", line 125, characters 24-38:
125 |             ) infinity (Lr1.reductions st)
                              ^^^^^^^^^^^^^^
Error (alert deprecated): G.Lr1.reductions
Please use [get_reductions]

File "src/ocaml/preprocess/recover/synthesis.ml", line 206, characters 19-33:
206 |             ) acc (Lr1.reductions st)
                         ^^^^^^^^^^^^^^
Error (alert deprecated): G.Lr1.reductions
Please use [get_reductions]
File "src/ocaml/preprocess/recover/gen_recover.ml", line 47, characters 13-27:
47 |           ) (Lr1.reductions st);
                  ^^^^^^^^^^^^^^
Error (alert deprecated): G.Lr1.reductions
Please use [get_reductions]
voodoos commented 2 months ago

I think the issue is that the Merlin package should be built in release mode, not in dev move: we ship the generated parser so that the release version of Merlin does not actually depend on Menhir.

You can use: dune build --profile=release @all or dune build -p packages,to,build to build in release mode.

SnarkBoojum commented 2 months ago

In Debian we prefer working from the sources, so we don't use pre-generated files : we regenerate them ourselves.

voodoos commented 2 months ago

I understand that, but it is not the way Merlin is meant to be built in production environments. The parser file is promoted to the sources exactly because we don't want the package to depend on Menhir. Additionally, building in release mode is really what should be done anyway, and that for any OCaml package, since it might enable additional optimizations and build only what is necessary. It is how opam does it.

SnarkBoojum commented 2 months ago

As I said, we want to use the sources. Notice that you'll still need to enable a more recent menhir at some point.

nojb commented 2 months ago

As I said, we want to use the sources. Notice that you'll still need to enable a more recent menhir at some point.

Absent a patch for Merlin itself to make it compatible with the latest Menhir (which would make it incompatible with older versions of it), you could disable the warning from outside the build by setting the environment variable OCAMLPARAM=_,w=-3. Or by patching the Dune file.

SnarkBoojum commented 2 months ago

@nojb: Yes, at one point merlin will need a patch, and will release a fixed version. That is what this is report is about. In the meantime I'll follow the hint of disabling the warning, thanks.

@voodoos: In many projects (C-based), autotools are used, and likewise the developpers have a huge set of deps, but they ship tarballs with pre-generated files and a lower number of deps. What do we do in Debian? We build the packages with the huge set of deps, we re-generate everything from genuine sources, and we ship packages with the reduces deps... The distinction source-generated isn't based on developer declaration, but on where the file is in the written-by-hand to computer-run continuum.