Update to OCaml 5.1.1, jsoo 5.8.2 #602
Conversation
AltGr
force-pushed
the
ocamlup
branch
3 times, most recently
from
6296e2b to
9da4406
Compare
|
Build on OSX fails, but actually at the OCaml compilation stage ; I have no idea about the state of the OCaml 5.1 support, maybe it's been discontinued for x86_64 ? These issues are mostly unrelated to the current PR though. |
|
All other checks pass ; some more manual testing is still warranted (I checked the printing, running and grading only on a few ones) |
|
Hi @AltGr, thanks a lot for this meticulous work!
Yes I just saw this failure. Given the
FWIW I am a macOS user, not a macOS expert but I could test the macOS artifacts anyway!
Ah OK, great!
Yes. Except the macOS static binaries I'd say, as it will block continuous deployment and release. What do you think of just putting |
on second thought: given master already failed this way because of the arm image, I'll push a separate PR with this change. Then you'll be able to rebase on this small PR (and maybe add |
| (compilation_mode whole_program) | ||
| (flags --no-source-map --opt=2 --enable=use-js-string --target-env=browser))) | ||
|
|
||
| (dev (flags (:standard -safe-string -w -32 -warn-error -a+8)) |
There was a problem hiding this comment.
Two questions:
-
adding this
devprofile, should I change my usual learn-ocaml programming inner loop?Namely, coding then building learn-ocaml with a bash alias that runs:
cd $learn_ocaml && eval $(opam env) && opam install . --deps-only --locked && make && make opaminstall && learn-ocaml build --repo=demo-repository serve -
I had the impression that the dev profile had less warnings enabled than in the release profile ? but probably I'm mistaken
There was a problem hiding this comment.
Historically it was impossible to build in dev mode because (i) dune didn't allow to customise the jsoo flags manually, it was tied to the built-in profiles, and (ii) source-map was blowing on us and the compilation wouldn't terminate. Thankfully both of these are now fixed :)
In the new setting, the jsoo --opt=2 flag makes release compilation quite a bit slower so I'd advise to use the dev mode when, well, developping 🤯 . The Makefile still defaults to release though, I'll add a PROFILE ?= release variable.
Then you could just run make testrun PROFILE=dev
- hm it's quite possible, I haven't taken much attention to the specific warning flags ; I've copied them from another of my projects mainly to disable warn-error in dev mode which I find very hard to work with ("yes I know that this argument is unused, I just commented out the code, will you go on anyways ?" ;) )
This will be required for 4.13 when jsoo hanles it
(and rely on ppxlib, which thankfully offers a compatibility register function)
|
Last question: in the end, would you like that we squash-merge your PR ? or keep all the commits in the master's history. In the latter case, I'd suggest you To make this easier, below are some suggestions for the 31 commits of the PR.
|
|
I'd prefer it not to be squashed as this loses a lot of information. In particular, if one wanted a build working with 4.14, it can easily be extracted now by selecting the correct intermediate commit, and that would be completely lost if squashed. I don't believe, however, that most of these commit belong to the changelog: saying that we updated jsoo and OCaml versions is the gist of it (plus a few aside improvements, like the |
it's a bit annoying to have the client depend on asak just because of a datatype that's not used, but fixing that would require some reorganisation.
(so that it's already in the Docker images)
|
Thanks a lot for reviewing, this last patch should address your concerns :) |
| @@ -68,7 +68,7 @@ case $(uname -s) in | |||
| esac | |||
| ;; | |||
| Darwin) | |||
| COMMON_LIBS="camlstr ssl_stubs /usr/local/opt/openssl/lib/libssl.a /usr/local/opt/openssl/lib/libcrypto.a cstruct_stubs bigstringaf_stubs lwt_unix_stubs unix" | |||
There was a problem hiding this comment.
Darwin compilation failed again later on... probably something to adjust here ? I'd be glad @erikmd if you could have a look 🙏🏿
There was a problem hiding this comment.
Hi @AltGr, sure! I'll be able to take a look (notably, relying on the make detect-libs target I had (co)written)
but I'm going into vacation so I won't be able to do it before the week of 12 August… stay tuned, and see you!
(reopen of #601, closed by mistake)
This mainly required time debugging and finding which parts of the code where silently failing with the newer version. Once I found the way to make our version of ppx_metaquot work with the newer ppxlib (very small code change in the end, but it took quite a bit of digging the ppxlib api), most of the effort was actually in updating jsoo rather than OCaml.
We stick at 5.1 for now because ppx_tools (of which we vendor a patched bit) could easily work with it, but isn't ported to 5.2 yet. There has been lots of improvements in jsoo in the 5.x branch, and I expect things to be faster although I haven't run benches.
I expect this to be part of a 1.1 release and bumped the version number in prevision