Support OCaml 5.2 #470

leostera · 2023-11-13T17:07:00Z

Hi folks! I saw Sabine's tweet and I figured I'd give this a shot. There's a few things that I'd like to upstream to the compiler (like Ast_helpers to create function params), but otherwise this seems to be working.

Happy to amend anything here, just figured I'd open the PR early to get feedback / directions.

Ideally would close #466

src/lib/uTop_compat.ml

ManasJayanth · 2023-11-13T18:27:55Z

Got here too late: My branch is here https://github.com/DiningPhilosophersCo/utop/tree/prometheansacrifice%40ocaml-5-2
Atleast I think I can lend a hand with a code review. Will leave a comment if necessary. This was fun!

emillon · 2023-11-14T12:32:15Z

Thanks for this PR.
I think that this is going in the right direction but the main thing that is left to address is compatibility. This version of utop needs to build and run on OCaml 4.11 to 5.2.0 (that seems large but bear in mind that I've already axed much of the compatibility so it's a lot easier than it used to be).
To have a system where it is easy to test compat code, I recommend the following set up:

put this in dune-workspace.multi (do not commit this file to the repository)

(lang dune 1.0)
(context (opam (switch utop-411)))
(context (opam (switch utop-412)))
(context (opam (switch utop-413)))
(context (opam (switch utop-414)))
(context (opam (switch utop-500)))
(context (opam (switch utop-510)))
(context (opam (switch utop-520)))

create N global switches named like the above and based on the corresponding ocaml compiler version, and install the dev-deps in them. You'll never activate these switches directly

$ opam sw create utop-411 ocaml-base-compiler.4.11.1
$ opam install --switch utop-411 utop --deps-only --with-test
...
$ opam sw create utop-510 ocaml-base-compiler.5.1.0
$ opam install --switch utop-510 utop --deps-only --with-test
$ opam sw create utop-520 ocaml-variants.5.2.0+trunk
$ opam install --switch utop-520 utop --deps-only --with-test

(note the --switch ... flags passed to install, and the fact we used ocaml-base-compiler everywhere but ocaml-variants in the 5.2.0 case)

then instead of the usual dune build, use dune build --workspace dune-workspace.multi. This is going to make a dune build that builds using all the different switches. Error messages will carry the name of the opam switch that failed, so you can use that to know what went wrong.

This is the main trick and the most important part to get a green build on all supported versions.

Now, regarding architecture, utop used to have many #ifs sprinkled all around the code base. I moved most of them to uTop_compat.ml. Please aim to put new compat code in there but I appreciate that it's not always possible.

I'll leave the rest of my comments as a review. Thanks again.

emillon

Thanks, many comments regarding organization but this is going to work.

src/lib/uTop.ml

src/lib/uTop.mli

src/lib/uTop_main.ml

src/lib/uTop_compat.ml

leostera · 2023-11-17T23:10:42Z

@emillon thanks for the guidance! 🙏🏼 made most of the changes (except the add_cmi_hook, but left you a question there). Let me know how you want to proceed 🙌🏼

CONTRIBUTING.md

emillon · 2023-11-20T09:53:22Z

src/lib/uTop_compat.ml

+#endif
+end
+
+let is_abstract_type_kind type_kind =


minor remark, but I think we can keep the type_kind = part in the guard and just expose abstract_type_kind

emillon · 2023-11-20T09:53:48Z

src/lib/uTop_complete.ml

@@ -394,7 +394,7 @@ let visible_modules () =
              (Sys.readdir (if dir = "" then Filename.current_dir_name else dir))
          with Sys_error _ ->
            acc)
-        String_set.empty @@ Load_path.get_paths ()
+        String_set.empty @@ (UTop_compat.get_load_path ())


@@ and parens are redundant here

src/lib/uTop_main.ml

emillon · 2023-12-22T16:37:24Z

(I haven't forgotten, I'll get back to you about this during week 1)

Co-authored-by: Rashid Al Muhairi <r.muhairi@pm.me>

emillon · 2024-02-06T10:43:59Z

@leostera I rebased this on top of master, added a changelog crediting all coauthors.

This is good to go for a release supporting 5.2 but there will probably be things to fix regarding hidden paths, and we'll have to decide if we want to provide a precise API for the load path.

emillon · 2024-02-06T10:44:33Z

Thanks everyone. that was the first time I was looking for external contributors and this was very succesful, despite the delay in merging!

kit-ty-kate · 2024-02-08T22:04:29Z

Would it be possible to get a new release?

pmetzger · 2024-02-08T22:31:21Z

@kit-ty-kate You don't have access to do that on your own? We could give it to you.

emillon · 2024-02-09T09:32:29Z

as I said previously, I'm waiting for 5.2 in ocaml-ci to cut the release to make sure it works there. this should happen after the first alpha, right?

kit-ty-kate · 2024-02-09T13:38:39Z

alpha1 is already available but ocaml-ci hasn't been updated yet. This is tracked at ocurrent/ocaml-ci#917

In the meantime, is there any chance you could spawn an opam switch locally instead? It should take less than 3 minutes

@leostera

CHANGES: * Add support for OCaml 5.2 (ocaml-community/utop#470, fixes ocaml-community/utop#466, @leostera, @ManasJayanth, @huwaireb)

leostera requested a review from rgrinberg as a code owner November 13, 2023 17:07

huwaireb mentioned this pull request Nov 13, 2023

feat(utop): OCaml 5.2 support #469

Closed

leostera commented Nov 13, 2023

View reviewed changes

src/lib/uTop_compat.ml Outdated Show resolved Hide resolved

leostera force-pushed the ocaml-5-support branch from bac8750 to f26000b Compare November 13, 2023 19:11

emillon reviewed Nov 14, 2023

View reviewed changes

ManasJayanth reviewed Nov 15, 2023

View reviewed changes

src/lib/uTop_compat.ml Outdated Show resolved Hide resolved

leostera force-pushed the ocaml-5-support branch from 7acabde to bcc2048 Compare November 17, 2023 23:03

emillon reviewed Nov 20, 2023

View reviewed changes

leostera force-pushed the ocaml-5-support branch from 43ef436 to bc6b9b1 Compare November 25, 2023 01:27

Octachron mentioned this pull request Feb 5, 2024

OCaml 5.2.0 Release Readiness ocaml/opam-repository#25182

Closed

18 tasks

emillon mentioned this pull request Feb 6, 2024

Add a contributing guide #479

Merged

emillon force-pushed the ocaml-5-support branch from bc6b9b1 to d73ff65 Compare February 6, 2024 10:31

feat: make it work on ocaml 5.2

67f84d6

Co-authored-by: Rashid Al Muhairi <r.muhairi@pm.me>

emillon force-pushed the ocaml-5-support branch from 130ea98 to 67f84d6 Compare February 6, 2024 10:42

emillon merged commit 4a97c4c into ocaml-community:master Feb 6, 2024
1 check passed

leostera deleted the ocaml-5-support branch February 6, 2024 11:59

emillon mentioned this pull request Feb 26, 2024

[new release] utop (2.14.0) ocaml/opam-repository#25354

Merged

emillon added a commit to emillon/opam-repository that referenced this pull request Feb 26, 2024

[new release] utop (2.14.0)

3b612f0

CHANGES: * Add support for OCaml 5.2 (ocaml-community/utop#470, fixes ocaml-community/utop#466, @leostera, @ManasJayanth, @huwaireb)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support OCaml 5.2 #470

Support OCaml 5.2 #470

leostera commented Nov 13, 2023 •

edited

Loading

ManasJayanth commented Nov 13, 2023

emillon commented Nov 14, 2023 •

edited

Loading

emillon left a comment

leostera commented Nov 17, 2023

emillon Nov 20, 2023

emillon Nov 20, 2023

emillon commented Dec 22, 2023

emillon commented Feb 6, 2024

emillon commented Feb 6, 2024

kit-ty-kate commented Feb 8, 2024

pmetzger commented Feb 8, 2024

emillon commented Feb 9, 2024

kit-ty-kate commented Feb 9, 2024

Support OCaml 5.2 #470

Support OCaml 5.2 #470

Conversation

leostera commented Nov 13, 2023 • edited Loading

ManasJayanth commented Nov 13, 2023

emillon commented Nov 14, 2023 • edited Loading

emillon left a comment

Choose a reason for hiding this comment

leostera commented Nov 17, 2023

emillon Nov 20, 2023

Choose a reason for hiding this comment

emillon Nov 20, 2023

Choose a reason for hiding this comment

emillon commented Dec 22, 2023

emillon commented Feb 6, 2024

emillon commented Feb 6, 2024

kit-ty-kate commented Feb 8, 2024

pmetzger commented Feb 8, 2024

emillon commented Feb 9, 2024

kit-ty-kate commented Feb 9, 2024

leostera commented Nov 13, 2023 •

edited

Loading

emillon commented Nov 14, 2023 •

edited

Loading