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

Upgrade to latest OCaml version #10940

Closed
mrmr1993 opened this issue May 12, 2022 · 11 comments · Fixed by #11288
Closed

Upgrade to latest OCaml version #10940

mrmr1993 opened this issue May 12, 2022 · 11 comments · Fixed by #11288
Assignees
Labels
Epic Issues which contain other issues.

Comments

@mrmr1993
Copy link
Member

mrmr1993 commented May 12, 2022

EDIT by @Firobe: Upgrade the compiler and needed packages for OCaml 4.14.0 if possible.

@Firobe
Copy link
Contributor

Firobe commented May 12, 2022

Initial conflicts during upgrade to 4.14, as shown by opam:

  ⊘ remove    rpc_parallel            v0.13.0*           [conflicts with ocaml]
  ⊘ remove    graphql_ppx             0.0.4*             [conflicts with ocaml]
  ⊘ remove    ppx_deriving_yojson     3.5.3*             [conflicts with ocaml]
  ⊘ remove    caqti-async             1.3.0              [conflicts with ocaml]
  ⊘ remove    graphql-async           0.13.0             [conflicts with ocaml]
  ⊘ remove    graphql_ppx_base        0.0.4*             [conflicts with ocaml]
  ⊘ remove    cohttp-async            2.5.2-1            [conflicts with ocaml]
  ⊘ remove    ppxfind                 1.4                [conflicts with ocaml]
  ⊘ remove    ppx_tools_versioned     5.4.0              [conflicts with ocaml]
  ⊘ remove    conduit-async           4.0.0              [conflicts with ocaml]
  ⊘ remove    async_ssl               v0.13.0            [conflicts with ocaml]
  ⊘ remove    async                   v0.13.0            [conflicts with ocaml]
  ⊘ remove    async_unix              v0.13.1            [conflicts with ocaml]
  ⊘ remove    async_rpc_kernel        v0.13.0            [conflicts with ocaml]
  ⊘ remove    async_kernel            v0.13.0*           [conflicts with ocaml]

73 packages are otherwise upgraded in the process

There's no guarantees the code base will correctly build after the conflicts are fixed, but it's the first obstacle. Most of the conflicts are seemingly simply because these packages are pinned.

@Firobe
Copy link
Contributor

Firobe commented May 13, 2022

The reason ppx_deriving_yojson is pinned is because in the past a fix from a newer version was manually backported. It now can safely be upgraded to that newer version and we can remove the pin and submodule altogether.

This makes ppxfind removal OK, since it's only use was in the legacy version of ppx_deriving_yojson.

Actions:

  • remove pin and on ppx_deriving_yojson and its associated submodule
  • remove ppxfind

New list of conflitcts:

  ⊘ remove    rpc_parallel            v0.13.0*           [conflicts with ocaml]
  ⊘ remove    graphql_ppx             0.0.4*             [conflicts with ocaml]
  ⊘ remove    caqti-async             1.3.0              [conflicts with ocaml]
  ⊘ remove    graphql-async           0.13.0             [conflicts with ocaml]
  ⊘ remove    graphql_ppx_base        0.0.4*             [conflicts with ocaml]
  ⊘ remove    cohttp-async            2.5.2-1            [conflicts with ocaml]
  ⊘ remove    ppx_tools_versioned     5.4.0              [conflicts with ocaml]
  ⊘ remove    conduit-async           4.0.0              [conflicts with ocaml]
  ⊘ remove    async_ssl               v0.13.0            [conflicts with ocaml]
  ⊘ remove    async                   v0.13.0            [conflicts with ocaml]
  ⊘ remove    async_unix              v0.13.1            [conflicts with ocaml]
  ⊘ remove    async_rpc_kernel        v0.13.0            [conflicts with ocaml]
  ⊘ remove    async_kernel            v0.13.0*           [conflicts with ocaml]

@Firobe
Copy link
Contributor

Firobe commented May 13, 2022

Most async conflicts are due to the fact that async_kernel is tied to the v0.13 version of the Jane Street ecosystem (same for rpc-parallel. We need to either:

  • rebase Mina changes on async_kernel and rpc_parallel on a newer version of both of these packages
  • relax the "core_kernel" {< "v0.14"} on both of the packages, and fix any problem that might happen

The first solution would obviously be more preferable, but I don't know yet how much work that would represent.

@Firobe
Copy link
Contributor

Firobe commented May 13, 2022

The graphql_ppx(_base) conflict is because it needs ppx_versioned_tools, which needs on old version of ocaml-migrate-parsetree, incompatible with newer OCaml versions.

Ideally, the usage of ppx_versioned_tools should be replaced by ppxlib. In the long term, hopefully graphql_ppx can be replaced altogether.

In the meantime, we can try to relax constraints where possible.

@Firobe
Copy link
Contributor

Firobe commented May 13, 2022

Most async conflicts are due to the fact that async_kernel is tied to the v0.13 version of the Jane Street ecosystem (same for rpc-parallel. We need to either:

* rebase Mina changes on `async_kernel` and `rpc_parallel` on a newer version of both of these packages

* relax the `"core_kernel" {< "v0.14"}` on both of the packages, and fix any problem that might happen

The first solution would obviously be more preferable, but I don't know yet how much work that would represent.

Experimenting with the second solution, the following actions resolve the conflicts:

  • relax constraints on all async and Jane Street packages from < 0.14 to < 0.15 in rpc_parallel and async_kernel
  • artificially change the version of async_kernel from 0.13 to 0.14

Every package correctly upgrades and rebuilds correctly, except:

  • extlib because of the old pin. It seems the pin is not needed anymore, because its only change is now incorporated in the last upstream version. Removing the pin and installing the upstream version works.
  • rpc_parallel, because of an interface change. I'm investigating.

@Firobe
Copy link
Contributor

Firobe commented May 13, 2022

* `rpc_parallel`, because of an interface change. I'm investigating.

This can be easily resolved by minor changes in rpc_parallel, mostly due to the interface change of failwiths in the new core version. It would be even better (and probably feasible, for rpc_parallel), to rebase on the latest version of rpc_parallel, which contains the same changes.

With this, the only conflicts left are graphql_ppx* ones (and then the code base has to build).

@Firobe
Copy link
Contributor

Firobe commented May 16, 2022

This can be easily resolved by minor changes in rpc_parallel, mostly due to the interface change of failwiths in the new core version. It would be even better (and probably feasible, for rpc_parallel), to rebase on the latest version of rpc_parallel, which contains the same changes.

This PR merges upstream changes in the custom rpc_parallel version, which fixes related conflicts: MinaProtocol/rpc_parallel#3

@Firobe
Copy link
Contributor

Firobe commented Jun 7, 2022

The road is now free to upgrade OCaml itself!

@robinbb
Copy link
Collaborator

robinbb commented Jun 7, 2022

@Firobe Is there an issue for that task? Is it in this epic? What remains in order to close this epic?

@Firobe
Copy link
Contributor

Firobe commented Jun 7, 2022

@robinbb There is one last PR to be made (this week) to actually bump OCaml (and fix related breakages). It'll be associated directly to this epic, instead of a sub-issue.

@Firobe Firobe mentioned this issue Jun 9, 2022
7 tasks
@Firobe Firobe linked a pull request Jun 15, 2022 that will close this issue
4 tasks
@robinbb
Copy link
Collaborator

robinbb commented Jun 20, 2022

Congratulations on the closing of this issue, @Firobe ! Well done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Epic Issues which contain other issues.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants