janestreet / base

Standard library for OCaml
MIT License
848 stars 124 forks source link

[v0.14] Add support for OCaml 5.00 #127

Closed kit-ty-kate closed 2 months ago

kit-ty-kate commented 2 years ago

Fixes #126

bcc32 commented 2 years ago

Thanks for the fix, Kate! I've pulled it in and made a couple changes to make sure it builds internally. I tried the naive thing to make sure it still compiles with 5.00, but I got an error:

$ opam switch create 5.00 ocaml-base-compiler.5.00
[ERROR] No compiler matching 'ocaml-base-compiler.5.00' found, use 'opam switch list-available' to see what is available, or use '--packages' to select packages explicitly.

Am I missing something obvious (I did remember to opam update this time :smile:)? Alternatively, would you prefer for me to just edit this PR and you can test it on your end?

Sudha247 commented 2 years ago

@bcc32 you can install the compiler with:

opam switch create 5.00.0+trunk
Sudha247 commented 2 years ago

@kit-ty-kate any idea why I'm getting this error on trunk (is it missing some patched dependency?)

src/.base.objs/byte/base__Import.cmo [...]
# File "src/import.ml", line 3, characters 8-40:
# 3 | include Sexp.Private.Raw_grammar.Builtin
#             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
# Error: Unbound module Sexp.Private.Raw_grammar
kit-ty-kate commented 2 years ago

@Sudha247 are you sure you’re not using base’s master branch instead? Everything is already in https://github.com/kit-ty-kate/opam-alpha-repository so you shouldn’t have to compile branches manually

bcc32 commented 2 years ago

Thanks both. I needed to add @kit-ty-kate's opam repo and then use the correct compiler name that @Sudha247 gave. I made some changes that build with 5.00 on my end but if you would like to check the compile on your end again, please do.

aalekseyev commented 2 years ago

I hate being the DCO police, but I won't be able to release this code publicly unless the patch is properly signed off. @kit-ty-kate, can you sign off your commits? (I'm assuming you know what that is about)

kit-ty-kate commented 2 years ago

@bcc32 any news on that?