-
Notifications
You must be signed in to change notification settings - Fork 66
Port to OCaml 4.12 #408
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
Port to OCaml 4.12 #408
Conversation
2961a83
to
19e7438
Compare
46c6ce9
to
bf8fe96
Compare
How do you run this to test? The docker command seems to not work ❯ docker run --rm \
-v .:/repository:ro \
-v learn-ocaml-sync:/sync \
-p 80:8080 --name learn-ocaml-server \
ocamlsf/learn-ocaml:master
docker: invalid reference format. |
At first sight, it's because of your This would lead to:
BTW I've removed the However, note that by running this command, you'd not get the intended goal of testing this PR here, because Just FTR, the date of the last push to Docker Hub of the master branch is mentioned here: |
@idkjs so to fully address your question:
The basic answer is: by checking-out the PR (see item 1. below) and compiling it (see item 2. below); admittedly we could have arranged some CI/CD config to automatically deploy another image (as "pre-prod") while the PR is not yet ready (like I did for another PR in pfitaxel@18da153), How to test this PR
|
Dockerfile
Outdated
@@ -68,7 +68,7 @@ WORKDIR /home/learn-ocaml | |||
|
|||
COPY --from=compilation /home/opam/install-prefix /usr | |||
|
|||
ENTRYPOINT ["dumb-init","learn-ocaml","--sync-dir=/sync","--repo=/repository"] | |||
ENTRYPOINT ["dumb-init","/usr/bin∕learn-ocaml","--sync-dir=/sync","--repo=/repository"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @AltGr, I believe I understood why the CI failed (it seems a weird but very funny explanation :)
The 3rd slash in this diff is not a true slash:
So the fix should just be:
ENTRYPOINT ["dumb-init","/usr/bin∕learn-ocaml","--sync-dir=/sync","--repo=/repository"] | |
ENTRYPOINT ["dumb-init","/usr/bin/learn-ocaml","--sync-dir=/sync","--repo=/repository"] |
(feel free to force-push)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed! I finally found it before seeing your message 😅 how could that ever have happened ?
As mentionned in the first comment, the Docker containers weren't fixed yet, but now it should be ok.
Last CI failure is because ocaml-sf/learn-ocaml-corpus#32 would need merging first (but in its current state it would break the corpus on released learn-ocaml :/ )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how could that ever have happened ?
I think that's because this character may be associated to a keybinding very close to Shift+/:
with my keyboard config, it is AltGr+Shift+/
Last CI failure is because ocaml-sf/learn-ocaml-corpus#32 would need merging first
OK, thanks for opening this other PR!
(but in its current state it would break the corpus on released learn-ocaml :/ )
Oh, that's unfortunate… would you have ideas to overcome this? I mean:
- Keeping the whole learn-ocaml-corpus run within learn-ocaml's CI;
- Make it pass for both learn-ocaml.0.12 and learn-ocaml@master ?
How did this happen? I thought I was going mad for a moment
dune no longer accepts manually setting `-custom` and doesn't allow us to set complete bytecode binaries to be installed :/ (see ocaml/dune#4837) it seems this combination with manual install is the only layout that allows normal and static linking to work...
Here is an error I am getting running the second command: ~/Github/learn-ocaml pr/408 ✘ 99 3.0.1
❯ opam switch create . --deps-only && opam install opam-installer && eval (opam env)
<><> Installing new switch packages ><><><><><><><><><><><> 🐫
Switch invariant: ["ocaml" {>= "4.05.0"}]
[ERROR] Could not determine which packages to install for this
switch:
* No agreement on the version of ocaml:
- learn-ocaml → asak → ocaml < 4.09.0
- learn-ocaml-client → ocaml = 4.12.0 ❯ git show
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
commit 82cea091b6af6e4d961b86607ae264d4f7e44ce2 (HEAD -> pr/408
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Author: Louis Gesbert <[email protected]>
Date: Thu Jul 29 15:51:08 2021 +0200
Fix linking
dune no longer accepts manually setting `-custom` and doesn
complete bytecode binaries to be installed :/ (see
https://github.com/ocaml/dune/pull/4837)
it seems this combination with manual install is the only l
normal and static linking to work...
FYI. Happy to test. Standing by for instructions. Thanks for all you do! |
You need to run |
Looks like Details...
Hint: try:
opam install asak base64 checkseum cohttp-lwt cohttp-lwt-unix conduit decompress digestif ezjsonm gg js_of_ocaml js_of_ocaml-compiler js_of_ocaml-lwt js_of_ocaml-ppx js_of_ocaml-tyxml lwt lwt_react magic-mime markup markup-lwt ocaml-migrate-parsetree ocp-indent-nlfork ocplib-json-typed ocplib-json-typed-browser ocplib-ocamlres omd ppx_tools vg
~/Github/learn-ocaml pr/408 3.0.1
❯ opam install asak base64 checkseum cohttp-lwt cohttp-lwt-unix conduit decompress digestif ezjsonm gg js_of_ocaml js_of_ocaml-compiler js_of_ocaml-lwt js_of_ocaml-ppx js_of_ocaml-tyxml lwt lwt_react magic-mime markup markup-lwt ocaml-migrate-parsetree ocp-indent-nlfork ocplib-json-typed ocplib-json-typed-browser ocplib-ocamlres omd ppx_tools vg
[ERROR] No package named ocplib-ocamlres found.
~/Github/learn-ocaml pr/408 ✘ 5 3.0.1
❯ opam install asak base64 checkseum cohttp-lwt cohttp-lwt-unix conduit decompress digestif ezjsonm gg js_of_ocaml js_of_ocaml-compiler js_of_ocaml-lwt js_of_ocaml-ppx js_of_ocaml-tyxml lwt lwt_react magic-mime markup markup-lwt ocaml-migrate-parsetree ocp-indent-nlfork ocplib-json-typed ocplib-json-typed-browser omd ppx_tools vg
...
# Run eval (opam env) to update the current shell environment
The former state can be restored with:
/usr/local/bin/opam switch import
"/Users/mando/Github/learn-ocaml/_opam/.opam-switch/backup/state-20210730124425.export"
~/Github/learn-ocaml pr/408 ✘ 31 2m 16s 3.0.1
❯ eval (opam env)
~/Github/learn-ocaml pr/408 3.0.1
❯ make && make opaminstall
make[2]: Nothing to be done for `all'.
File "src/ppx-metaquot/ppx_metaquot_main.ml", line 2, characters 2-35:
2 | Migrate_parsetree.Driver.register ~name:"ppx_metaquot" (module Migrate_parsetree.OCaml_412)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Error: Unbound module Migrate_parsetree.Driver
File "src/repo/learnocaml_exercise.ml", line 321, characters 6-28:
321 | | Omd_representation.Url(href,s,title) ->
^^^^^^^^^^^^^^^^^^^^^^
Error: Unbound module Omd_representation
File "src/utils/ppx_ocplib_i18n.ml", line 14, characters 5-18:
14 | open OCaml_405.Ast
^^^^^^^^^^^^^
Error: Unbound module OCaml_405
File "src/grader/dune", line 6, characters 13-34:
6 | (preprocess (pps ppx_ocplib_i18n))
^^^^^^^^^^^^^^^^^^^^^
Error: No ppx driver were found.
Hint: Try upgrading or reinstalling ocaml-migrate-parsetree.
File "src/grader/dune", line 33, characters 13-42:
33 | (preprocess (pps learnocaml_ppx_metaquot))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Error: No ppx driver were found.
Hint: Try upgrading or reinstalling ocaml-migrate-parsetree.
File "src/grader/dune", line 126, characters 41-53:
126 | (action (with-stdout-to %{targets} (run ocp-ocamlres -format ocamlres %{stdlib_cmis} %{local_cmis} %{lib_cmis})))
^^^^^^^^^^^^
Error: Program ocp-ocamlres not found in the tree or in PATH
(context: default)
File "src/grader/dune", line 134, characters 12-35:
134 | (libraries ocplib-ocamlres.runtime bigarray
^^^^^^^^^^^^^^^^^^^^^^^
Error: Library "ocplib-ocamlres.runtime" not found.
Hint: try:
dune external-lib-deps --missing --profile release @@default
File "src/grader/dune", line 158, characters 16-28:
158 | (run ocp-ocamlres -format ocamlres %{compiler-cmis} %{generated-cmis})))
^^^^^^^^^^^^
Error: Program ocp-ocamlres not found in the tree or in PATH
(context: default)
File "src/grader/dune", line 168, characters 12-35:
168 | ocplib-ocamlres.runtime
^^^^^^^^^^^^^^^^^^^^^^^
Error: Library "ocplib-ocamlres.runtime" not found.
Hint: try:
dune external-lib-deps --missing --profile release @@default
File "src/grader/dune", line 174, characters 26-71:
174 | (preprocess (per_module ((pps ppx_ocplib_i18n learnocaml_ppx_metaquot) Grading)))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Error: No ppx driver were found.
Hint: Try upgrading or reinstalling ocaml-migrate-parsetree.
File "src/grader/dune", line 184, characters 12-27:
184 | ocplib-ocamlres
^^^^^^^^^^^^^^^
Error: Library "ocplib-ocamlres" not found.
Hint: try:
dune external-lib-deps --missing --profile release @@default
File "src/main/dune", line 43, characters 12-27:
43 | cohttp-lwt-unix
^^^^^^^^^^^^^^^
Error: Library "cohttp-lwt-unix" not found.
Hint: try:
dune external-lib-deps --missing --profile release @@default
File "src/server/dune", line 14, characters 12-22:
14 | decompress
^^^^^^^^^^
Error: Library "decompress" not found.
Hint: try:
dune external-lib-deps --missing --profile release @@default
File "src/state/dune", line 25, characters 12-19:
25 | conduit
^^^^^^^
Error: Library "conduit" not found.
Hint: try:
dune external-lib-deps --missing --profile release @@default
File "src/toplevel/dune", line 21, characters 12-35:
21 | ocplib-ocamlres.runtime
^^^^^^^^^^^^^^^^^^^^^^^
Error: Library "ocplib-ocamlres.runtime" not found.
Hint: try:
dune external-lib-deps --missing --profile release @@default
File "src/utils/dune", line 14, characters 13-34:
14 | (preprocess (pps ppx_ocplib_i18n))
^^^^^^^^^^^^^^^^^^^^^
Error: No ppx driver were found.
Hint: Try upgrading or reinstalling ocaml-migrate-parsetree.
make: *** [build] Error 1
~/Github/learn-ocaml pr/408 ✘ 2 5s 3.0.1
❯ dune external-lib-deps --missing --profile release @@default
Error: The following libraries are missing in the default context:
- cohttp-lwt-unix
- conduit
- decompress
- ocplib-ocamlres
- ocplib-ocamlres.runtime
Hint: try:
opam install cohttp-lwt-unix conduit decompress ocplib-ocamlres
~/Github/learn-ocaml pr/408 3.0.1
❯ opam install cohttp-lwt-unix conduit decompress ocplib-ocamlres
[ERROR] No package named ocplib-ocamlres found.
~/Github/learn-ocaml pr/408 ✘ 5 3.0.1
❯ opam install cohttp-lwt-unix conduit decompress
[NOTE] Package decompress is already installed (current
version is 1.4.1).
The following actions will be performed:
∗ install num 1.4 [required by sexplib]
∗ install sexplib v0.14.0 [required by conduit]
∗ install mirage-crypto-pk 0.10.3 [required by x509]
∗ install conduit 4.0.0
∗ install x509 0.14.0 [required by ca-certs]
∗ install conduit-lwt 4.0.0
[required by cohttp-lwt-unix]
∗ install ca-certs 0.2.1
[required by conduit-lwt-unix]
∗ install conduit-lwt-unix 4.0.0
[required by cohttp-lwt-unix]
∗ install cohttp-lwt-unix 4.0.0
===== ∗ 9 =====
Do you want to continue? [Y/n] y
<><> Processing actions ><><><><><><><><><><><><><><><><><> 🐫
⬇ retrieved ca-certs.0.2.1 (cached)
⬇ retrieved conduit.4.0.0 (cached)
⬇ retrieved conduit-lwt.4.0.0 (cached)
⬇ retrieved conduit-lwt-unix.4.0.0 (cached)
⬇ retrieved cohttp-lwt-unix.4.0.0 (cached)
⬇ retrieved num.1.4 (cached)
⬇ retrieved x509.0.14.0 (cached)
⬇ retrieved sexplib.v0.14.0 (cached)
⬇ retrieved mirage-crypto-pk.0.10.3 (cached)
[ERROR] The installation of num failed at "make
findlib-install".
#=== ERROR while installing num.1.4 ==========================#
# context 2.1.0~rc2 | macos/x86_64 | | https://opam.ocaml.org#12525fc3
# path ~/Github/learn-ocaml/_opam/.opam-switch/build/num.1.4
# command ~/.opam/opam-init/hooks/sandbox.sh install make findlib-install
# exit-code 2
# env-file ~/.opam/log/num-33897-322c5d.env
# output-file ~/.opam/log/num-33897-322c5d.out
### output ###
# /Applications/Xcode.app/Contents/Developer/usr/bin/make -C src findlib-install
# sed -e '/\^/d' -e 's/%%VERSION%%/1.4/g' META.in > META
# ocamlfind install num META nums.cma libnums.a big_int.cmi nat.cmi num.cmi ratio.cmi arith_status.cmi big_int.mli nat.mli num.mli ratio.mli arith_status.mli big_int.cmti nat.cmti num.cmti ratio.cmti arith_status.cmti nums.cmxa nums.a int_misc.cmx nat.cmx big_int.cmx arith_flags.cmx ratio.cmx num.cmx arith_status.cmx nums.cmxs dllnums.so
# ocamlfind: Package num is already installed
# - (file /Users/mando/Github/learn-ocaml/_opam/lib/num/META already exists)
# make[1]: *** [findlib-install] Error 2
# make: *** [findlib-install] Error 2 |
the package is |
Is there any progress on this PR? |
Thanks @Ailrun for your comment! Basically, there are only two reasons for not merging this PR right away (but these shouldn't delay the process that much):
More precisely: For 1., the issue is that the CI relies on running learn-ocaml on the whole For 2., I have personally no objections for merging #408 quickly (once 1. is fixed)… except that doing that, each PR we'd like to ship in learn-ocaml 0.13 would need very similar tweaks (merge + compatibility fixes for the .opam files, etc.), while it may be simpler to merge the few other PRs, then rebase #408 😅 (which only contains 7 commits). |
I see, thank you for the detailed explanation! I really look forward to use this new version for courses. |
FTR, we had already decided that #408 will be merged and released before #362 is merged and released. And given many classes may want to rely on learn-ocaml as from the Autumn term, I sincerely hope that a new release could be shipped by September 15. I think I'll open a meta-issue summarizing the release plan we had thought about with Yann lately… and which just needs to be refined a bit. |
BTW @AltGr, do you agree with that suggestion:
or do you believe that "rebasing #408 after merging several PRs" would be more tricky and not worth waiting for? in which case, no worries, we could merge #408 sooner… |
I don't think it will interfere too much with other PRs and should be merged as soon as possible, this has already been stalled for quite long. Note that the number of commits shouldn't matter too much for the order, but the number of affected files (and the "liveness" of these files) is a more important concern; people have different practices but for long-standing branches with lots of commits it's often more reasonable to use merges rather than rebases (I do always prefer rebases for my work branches, but sometimes they just become unnecessarily tedious. I really disapprove reverse merges from master into work branches though!) As for the CI, there is no reason not to merge https://github.com/ocaml-sf/learn-ocaml-corpus/pull/32/files , it's backwards-compatible with 4.05 and should fix this. |
Thank you @AltGr! so @yurug just merged ocaml-sf/learn-ocaml-corpus#32 and I'm closing/reopening this PR to restart the CI. |
The Docker targets and CI scripts are probably going to need some more
work, but compiling the base application and test exercises already
works. Testers welcome!