Use the Yojson 2.0 `Seq` API by Leonidas-from-XIV · Pull Request #1475 · ocaml/merlin

Leonidas-from-XIV · 2022-06-02T13:11:49Z

Yojson 2.0 is dropping☨ the Stream API for compatibility with OCaml 5, thus it added an Seq based API. This PR does the small changes that are required to use it instead of Stream.

Which also comes with the added benefit of removing usage of Stream from Merlin itself.

☨Yojson 2.0 will drop the API when it is released. As such this PR requires the master version of Yojson but the idea is to prepare most Yojson consumers in advantage, so when Yojson 2.0 ends up on OPAM the migration is as smooth as possible.

src/frontend/ocamlmerlin/old/old_IO.ml

Bug: ocaml/merlin#1475 Signed-off-by: Maciej Barć <xgqt@gentoo.org>

rand00 · 2022-06-13T17:29:18Z

I get this error when trying it out:

> opam pin add https://github.com/Leonidas-from-XIV/merlin.git#yojson-2-compat
This will pin the following packages: dot-merlin-reader, merlin-lib, merlin. Continue? [Y/n] y
[NOTE] Package dot-merlin-reader is already pinned to git+https://github.com/Leonidas-from-XIV/merlin.git#yojson-2-compat (version 4.2).
dot-merlin-reader is now pinned to git+https://github.com/Leonidas-from-XIV/merlin.git#yojson-2-compat (version 4.2)
[NOTE] Package merlin-lib is already pinned to git+https://github.com/Leonidas-from-XIV/merlin.git#yojson-2-compat (version ~dev).
merlin-lib is now pinned to git+https://github.com/Leonidas-from-XIV/merlin.git#yojson-2-compat (version ~dev)
[NOTE] Package merlin is already pinned to git+https://github.com/Leonidas-from-XIV/merlin.git#yojson-2-compat (version 4.5-414).
merlin is now pinned to git+https://github.com/Leonidas-from-XIV/merlin.git#yojson-2-compat (version 4.5-414)

[ERROR] Package conflict!
  * Missing dependency:
    - merlin-lib >= 4.2
    no matching version

[NOTE] Pinning command successful, but your installed packages may be out of sync.

voodoos

Thank you @Leonidas-from-XIV !

Could you add a changelog entry ?

voodoos · 2022-06-15T13:02:03Z

@trefis : yojson's update makes older Merlin and Yojson 2.0.0 incompatible.

I feel like we should release old versions of Merlin (3.5 and 411) with this patch too for users convenience.
What do you think ?

voodoos · 2022-06-15T13:08:41Z

@Leonidas-from-XIV the windows CI failed because Yojson 2.0.0 appears to be unavailable. Is that a known issue ?

Leonidas-from-XIV · 2022-06-15T13:20:25Z

@voodoos If I see correctly the build on CI doesn't use opam-repository but https://github.com/fdopen/opam-repository-mingw#opam2 which does not have Yojson 2.0.0. I don't know anything about the Windows CI so I don't know why it uses its own opam-repository or how to contribute to it. Apparently it is discontinued so I am not sure it makes sense to contribute.

It seems like setup-ocaml choses to add the mingw-repo instead of upstream opam-repository: https://github.com/ocaml/setup-ocaml/blob/99ffdbe56b436bc6c29c803570b158d347765af6/src/setup-ocaml/constants.ts#L69-L72

I can go ahead raise this issue at setup-ocaml.

voodoos · 2022-06-17T12:43:18Z

The fdopen repository was a mirror of opam repository with some patched packages specifically for windows.
Now that it is deprecated, I don't know what is the official way to go. Maybe you know, @nojb or @dra27 ?

Leonidas-from-XIV · 2022-06-17T12:55:44Z

According to the blog post

To ease the transition, I will create a special branch in the near future, which will only contain the compiler packages of the old Windows repository, and can be used in addition to the normal upstream repository (opam repo add ...). However, this branch will not be maintained.

So I think the best would be to use upstream opam-repository + this branch. Unfortunately I was not able to find that branch, it is possible that it never came into existence. In any case, this is more an issue with setup-ocaml, so I opened a ticket here: ocaml/setup-ocaml#547

In theory I assume the Merlin Windows CI run could manually specify to use opam-repository also on Windows, but I don't know if that'll lead to issues with missing Windows packages. It would certainly be nicer to solve it upstream so it works for everyone without local adjustments.

nojb · 2022-06-17T13:31:18Z

The fdopen repository was a mirror of opam repository with some patched packages specifically for windows. Now that it is deprecated, I don't know what is the official way to go. Maybe you know, @nojb or @dra27 ?

I don't use OPAM much, but https://github.com/fdopen/opam-repository-mingw seems to be maintained (last commit was two days ago).

voodoos · 2022-06-17T13:37:48Z

And it looks like Yojson 2.0 was part of the batch: https://github.com/fdopen/opam-repository-mingw/tree/opam2/packages/yojson/yojson.2.0.0 :-)

Yojson 2.0 is dropping the `Stream` API for compatibility with OCaml 5, thus it added an `Seq` based API. This PR does the small changes that are required to use it instead of `Stream`. Which also comes with the added benefit of removing usage of `Stream` from Merlin itself.

Leonidas-from-XIV · 2022-06-17T14:04:18Z

I've squashed the commits to clean up and trigger CI again, so we can see it (hopefully) succeed on the Windows CI.

from Leonidas-from-XIV/yojson-2-compat

@rgrinberg

CHANGES: Thu Jun 30 14:51:42 CEST 2022 + merlin binary - make most library public and split merlin in two packages: the `merlin-lib` package that exposes merlin's internals and the `merlin` package with the frontend. (ocaml/merlin#1448, ocaml/merlin#1455, ocaml/merlin#1457, ocaml/merlin#1497, @rgrinberg, @tmattio, @kit-ty-kate) - Type printing: use best_module_path for paths from Mty_alias (ocaml/merlin#1470) - Attempt at finding the 'real' capitalization of files on windows (ocaml/merlin#1462 by @mlasson) - Use newer `Seq`-based API of Yojson 2.0, avoiding the need for the deprecated `Stream` module (ocaml/merlin#1475 by @Leonidas-from-XIV) - unify parsing of `MERLIN_LOG` (ocaml/merlin#1480 by @ulugbekna) - Fix type deduplication in `type-enclosing` results (ocaml/merlin#1483, fixes ocaml/merlin#1477) - Only weakly reduce the shapes to speed up the new Merlin locate implementation. (ocaml/merlin#1488) - Ignore unknown configuration tags from dune configuration provider but not from dot-merlin-reader (ocaml/merlin#1486) - typing recovery: recover at the granularity of `core_type` (ocaml/merlin#1484) + editor modes - add method imenu items for emacs (ocaml/merlin#1481, @mndrix) - emacs: Make the prefix argument to `merlin-locate` optional, both for consistency with Emacs convention and for backwards compatibility. (ocaml/merlin#1476, @antalsz) - emacs: fix duplicated prefix path in imenu entries (ocaml/merlin#1495, @bcc32)

@Leonidas-from-XIV

CHANGES: Thu Jun 30 16:51:42 CEST 2022 + merlin binary - Use newer `Seq`-based API of Yojson 2.0, avoiding the need for the deprecated `Stream` module (ocaml/merlin#1475 by @Leonidas-from-XIV)

@mlasson

CHANGES: Fri Jul 1 12:51:42 CEST 2022 + merlin binary - Type printing: use `best_module_path` for paths from `Mty_alias` (ocaml/merlin#1470) - Attempt at finding the 'real' capitalization of files on windows (ocaml/merlin#1462 by @mlasson) - Use newer `Seq`-based API of Yojson 2.0, avoiding the need for the deprecated `Stream` module (ocaml/merlin#1475 by @Leonidas-from-XIV) - unify parsing of `MERLIN_LOG` (ocaml/merlin#1480 by @ulugbekna) - Fix type deduplication in `type-enclosing` results (ocaml/merlin#1483, fixes ocaml/merlin#1477) - Ignore unknown configuration tags from dune configuration provider but not from dot-merlin-reader (ocaml/merlin#1486) - typing recovery: recover at the granularity of `core_type` (ocaml/merlin#1484) + editor modes - Fix `merlin-locate-in-new-window` is ignored (ocaml/merlin#1461 by @emturner, fixes ocaml/merlin#1460) - add method imenu items for emacs (ocaml/merlin#1481, @mndrix) - emacs: Make the prefix argument to `merlin-locate` optional, both for consistency with Emacs convention and for backwards compatibility. (ocaml/merlin#1476, @antalsz)

@mlasson

CHANGES: Fri Jul 1 12:51:42 CEST 2022 + merlin binary - Type printing: use `best_module_path` for paths from `Mty_alias` (ocaml/merlin#1470) - Attempt at finding the 'real' capitalization of files on windows (ocaml/merlin#1462 by @mlasson) - Use newer `Seq`-based API of Yojson 2.0, avoiding the need for the deprecated `Stream` module (ocaml/merlin#1475 by @Leonidas-from-XIV) - unify parsing of `MERLIN_LOG` (ocaml/merlin#1480 by @ulugbekna) - Fix type deduplication in `type-enclosing` results (ocaml/merlin#1483, fixes ocaml/merlin#1477) - Ignore unknown configuration tags from dune configuration provider but not from dot-merlin-reader (ocaml/merlin#1486) - typing recovery: recover at the granularity of `core_type` (ocaml/merlin#1484) + editor modes - Fix `merlin-locate-in-new-window` is ignored (ocaml/merlin#1461 by @emturner, fixes ocaml/merlin#1460) - add method imenu items for emacs (ocaml/merlin#1481, @mndrix) - emacs: Make the prefix argument to `merlin-locate` optional, both for consistency with Emacs convention and for backwards compatibility. (ocaml/merlin#1476, @antalsz) - emacs: fix duplicated prefix path in imenu entries (ocaml/merlin#1495, @bcc32)

@mlasson

CHANGES: Fri Jul 1 12:51:42 CEST 2022 + merlin binary - Type printing: use `best_module_path` for paths from `Mty_alias` (ocaml/merlin#1470) - Attempt at finding the 'real' capitalization of files on windows (ocaml/merlin#1462 by @mlasson) - Use newer `Seq`-based API of Yojson 2.0, avoiding the need for the deprecated `Stream` module (ocaml/merlin#1475 by @Leonidas-from-XIV) - unify parsing of `MERLIN_LOG` (ocaml/merlin#1480 by @ulugbekna) - Fix type deduplication in `type-enclosing` results (ocaml/merlin#1483, fixes ocaml/merlin#1477) - Ignore unknown configuration tags from dune configuration provider but not from dot-merlin-reader (ocaml/merlin#1486) - typing recovery: recover at the granularity of `core_type` (ocaml/merlin#1484) + editor modes - Fix `merlin-locate-in-new-window` is ignored (ocaml/merlin#1461 by @emturner, fixes ocaml/merlin#1460) - add method imenu items for emacs (ocaml/merlin#1481, @mndrix) - emacs: Make the prefix argument to `merlin-locate` optional, both for consistency with Emacs convention and for backwards compatibility. (ocaml/merlin#1476, @antalsz) - emacs: fix duplicated prefix path in imenu entries (ocaml/merlin#1495, @bcc32)

@mlasson

CHANGES: Fri Jul 1 12:51:42 CEST 2022 + merlin binary - Type printing: use `best_module_path` for paths from `Mty_alias` (ocaml/merlin#1470) - Attempt at finding the 'real' capitalization of files on windows (ocaml/merlin#1462 by @mlasson) - Use newer `Seq`-based API of Yojson 2.0, avoiding the need for the deprecated `Stream` module (ocaml/merlin#1475 by @Leonidas-from-XIV) - unify parsing of `MERLIN_LOG` (ocaml/merlin#1480 by @ulugbekna) - Fix type deduplication in `type-enclosing` results (ocaml/merlin#1483, fixes ocaml/merlin#1477) - Ignore unknown configuration tags from dune configuration provider but not from dot-merlin-reader (ocaml/merlin#1486) - typing recovery: recover at the granularity of `core_type` (ocaml/merlin#1484) + editor modes - Fix `merlin-locate-in-new-window` is ignored (ocaml/merlin#1461 by @emturner, fixes ocaml/merlin#1460) - add method imenu items for emacs (ocaml/merlin#1481, @mndrix) - emacs: Make the prefix argument to `merlin-locate` optional, both for consistency with Emacs convention and for backwards compatibility. (ocaml/merlin#1476, @antalsz)

@Leonidas-from-XIV

CHANGES: Thu Jun 30 16:51:42 CEST 2022 + merlin binary - Use newer `Seq`-based API of Yojson 2.0, avoiding the need for the deprecated `Stream` module (ocaml/merlin#1475 by @Leonidas-from-XIV)

nojb reviewed Jun 2, 2022

View reviewed changes

src/frontend/ocamlmerlin/old/old_IO.ml Outdated Show resolved Hide resolved

nojb reviewed Jun 2, 2022

View reviewed changes

src/frontend/ocamlmerlin/old/old_IO.ml Show resolved Hide resolved

gentoo-bot pushed a commit to gentoo/gentoo that referenced this pull request Jun 8, 2022

dev-ml/merlin: use <yojson-2

f839dbd

Bug: ocaml/merlin#1475 Signed-off-by: Maciej Barć <xgqt@gentoo.org>

voodoos reviewed Jun 15, 2022

View reviewed changes

Leonidas-from-XIV mentioned this pull request Jun 15, 2022

Uses discontinued repository on Windows ocaml/setup-ocaml#547

Closed

Leonidas-from-XIV force-pushed the yojson-2-compat branch from c84fc70 to f31c06d Compare June 15, 2022 13:31

Leonidas-from-XIV force-pushed the yojson-2-compat branch from f31c06d to e6f538e Compare June 17, 2022 14:03

voodoos mentioned this pull request Jun 17, 2022

[new release] biniou (1.2.2) ocaml/opam-repository#21554

Merged

voodoos merged commit dd7663f into ocaml:master Jun 17, 2022

Leonidas-from-XIV deleted the yojson-2-compat branch June 20, 2022 22:29

voodoos added a commit to voodoos/merlin that referenced this pull request Jun 29, 2022

Use the Yojson 2.0 Seq API (ocaml#1475)

5d05cb6

from Leonidas-from-XIV/yojson-2-compat

voodoos added a commit to voodoos/merlin that referenced this pull request Jun 30, 2022

Use the Yojson 2.0 Seq API (ocaml#1475)

ea17779

from Leonidas-from-XIV/yojson-2-compat

voodoos added a commit to voodoos/merlin that referenced this pull request Jun 30, 2022

Use the Yojson 2.0 Seq API (ocaml#1475)

fa5277d

from Leonidas-from-XIV/yojson-2-compat

voodoos mentioned this pull request Jun 30, 2022

[new release] merlin, merlin-lib and dot-merlin-reader (4.6-414) ocaml/opam-repository#21693

Merged

voodoos mentioned this pull request Jul 1, 2022

[new release] merlin (3.8.0) ocaml/opam-repository#21700

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use the Yojson 2.0 `Seq` API#1475

Use the Yojson 2.0 `Seq` API#1475
voodoos merged 1 commit intoocaml:masterfrom
Leonidas-from-XIV:yojson-2-compat

Leonidas-from-XIV commented Jun 2, 2022

Uh oh!

Uh oh!

Uh oh!

rand00 commented Jun 13, 2022

Uh oh!

voodoos left a comment

Uh oh!

voodoos commented Jun 15, 2022

Uh oh!

voodoos commented Jun 15, 2022

Uh oh!

Leonidas-from-XIV commented Jun 15, 2022

Uh oh!

voodoos commented Jun 17, 2022 •

edited

Loading

Uh oh!

Leonidas-from-XIV commented Jun 17, 2022

Uh oh!

nojb commented Jun 17, 2022

Uh oh!

voodoos commented Jun 17, 2022

Uh oh!

Leonidas-from-XIV commented Jun 17, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

Leonidas-from-XIV commented Jun 2, 2022

Uh oh!

Uh oh!

Uh oh!

rand00 commented Jun 13, 2022

Uh oh!

voodoos left a comment

Choose a reason for hiding this comment

Uh oh!

voodoos commented Jun 15, 2022

Uh oh!

voodoos commented Jun 15, 2022

Uh oh!

Leonidas-from-XIV commented Jun 15, 2022

Uh oh!

voodoos commented Jun 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Leonidas-from-XIV commented Jun 17, 2022

Uh oh!

nojb commented Jun 17, 2022

Uh oh!

voodoos commented Jun 17, 2022

Uh oh!

Leonidas-from-XIV commented Jun 17, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

voodoos commented Jun 17, 2022 •

edited

Loading