Skip to content

Refine constraints on lwt6#29530

Open
raphael-proust wants to merge 18 commits intoocaml:masterfrom
raphael-proust:refine-constraints-on-lwt6
Open

Refine constraints on lwt6#29530
raphael-proust wants to merge 18 commits intoocaml:masterfrom
raphael-proust:refine-constraints-on-lwt6

Conversation

@raphael-proust
Copy link
Contributor

let the CI run, I'll add constraints if needs arises

6.1 changes engines which lwt_eio uses
because it uses a long-deprecated now-removed Lwt_main.enter_iter_hooks
because it uses long-deprecated now-removed Lwt_main.exit_hooks
notifications is now abstract type rather than int (in lwt<6)
notifications have abstract types ratehr than int (in lwt<6)
notifications have abstract type rather than int (in lwt<6)
@raphael-proust
Copy link
Contributor Author

broad categories of incompatibilities:

  • Lwt_unix.notification type mismatch: type has changed to abstract (was int), simple fix (if there is arithmetics done on the notification then we should burn the package down rather than fix), picos is already fixed but needs release, not sure about others
  • Lwt_main.enter_iter_hooks and other hooks: those long-deprecated values have been removed and the code should just call Lwt_main.Enter_iter_hooks.* functions to manipulate hooks
  • Lwt_engine has abstract method: add constraint to lwt<6.1.0, fix in package by adding an lwt_engine id, see WIP compatibility with lwt.6 ocaml-multicore/lwt_eio#31

because of Lwt_main.enter_iter_hooks
because of Lwt_main.exit_hooks
because of Lwt_unix.notification (is not int anymore)
because Lwt_unix.run (use Lwt_main.run instead)
@raphael-proust
Copy link
Contributor Author

  • Lwt_unix.run: long-depercated, now-removed, use Lwt_main.run instead

because tests use `Lwt_unix.retained` which is internals only and has
been removed
@raphael-proust
Copy link
Contributor Author

idk what to do about eio:
the tests break, but it's tests about the order of mixed lwt+eio scheduling, and the order is "better" now (for some notion of better anyway)

I think it's ok to use eio_lwt with lwt6, but the tests do fail so i'll think about what's the right thing to do

"ocaml" {< "5.0"}
"dune" {>= "1.0"}
"lwt" {>= "4.0.0" & < "6~"}
"lwt" {>= "4.0.0" }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With lwt.6.1.1 there are 440 occurrences of this kind of error:
https://opam.ci.ocaml.org/github/ocaml/opam-repository/commit/3af7d0a25910b3d20c909e559efb484eb6a78128/variant/compilers,4.14,lambda-term.3.3.0

#=== ERROR while compiling lwt_log.1.1.1 ======================================#
# context              2.5.0 | linux/x86_64 | ocaml-base-compiler.4.14.2 | file:///home/opam/opam-repository
# path                 ~/.opam/4.14/.opam-switch/build/lwt_log.1.1.1
# command              ~/.opam/opam-init/hooks/sandbox.sh build dune build -p lwt_log -j 71
# exit-code            1
# env-file             ~/.opam/log/lwt_log-7-afc58f.env
# output-file          ~/.opam/log/lwt_log-7-afc58f.out
### output ###
# (cd _build/default && /home/opam/.opam/4.14/bin/ocamlc.opt -w -40 -w +A -g -bin-annot -I src/unix/.lwt_log.objs/byte -I /home/opam/.opam/4.14/lib/bytes -I /home/opam/.opam/4.14/lib/lwt -I /home/opam/.opam/4.14/lib/lwt/unix -I /home/opam/.opam/4.14/lib/ocaml/threads -I /home/opam/.opam/4.14/lib/ocplib-endian -I /home/opam/.opam/4.14/lib/ocplib-endian/bigstring -I src/core/.lwt_log_core.objs/byte -intf-suffix .ml -no-alias-deps -o src/unix/.lwt_log.objs/byte/lwt_daemon.cmo -c -impl src/unix/lwt_daemon.ml)
# File "src/unix/lwt_daemon.ml", line 64, characters 49-68:
# 64 |     Lwt_sequence.iter_node_l Lwt_sequence.remove Lwt_main.exit_hooks [@ocaml.warning "-3"];
#                                                       ^^^^^^^^^^^^^^^^^^^
# Error: Unbound value Lwt_main.exit_hooks
# (cd _build/default && /home/opam/.opam/4.14/bin/ocamlopt.opt -w -40 -w +A -g -I src/unix/.lwt_log.objs/byte -I src/unix/.lwt_log.objs/native -I /home/opam/.opam/4.14/lib/bytes -I /home/opam/.opam/4.14/lib/lwt -I /home/opam/.opam/4.14/lib/lwt/unix -I /home/opam/.opam/4.14/lib/ocaml/threads -I /home/opam/.opam/4.14/lib/ocplib-endian -I /home/opam/.opam/4.14/lib/ocplib-endian/bigstring -I src/core/.lwt_log_core.objs/byte -I src/core/.lwt_log_core.objs/native -intf-suffix .ml -no-alias-deps -o src/unix/.lwt_log.objs/native/lwt_daemon.cmx -c -impl src/unix/lwt_daemon.ml)
# File "src/unix/lwt_daemon.ml", line 64, characters 49-68:
# 64 |     Lwt_sequence.iter_node_l Lwt_sequence.remove Lwt_main.exit_hooks [@ocaml.warning "-3"];
#                                                       ^^^^^^^^^^^^^^^^^^^
# Error: Unbound value Lwt_main.exit_hooks

Is it a good idea to lift this upper bound - or are extra conflicts needed? 🤔

@kit-ty-kate
Copy link
Member

note that to have a real sense of which packages actually need lwt5 vs not, you would need to temporarily modify the lwt.6.1.1 package to force its revdeps to build. Otherwise some package might still choose lwt5 based on other factors and you'll miss that data.

@kit-ty-kate
Copy link
Member

but overall this is probably more of a job for opam-health-check-ng. I can help you set it up locally if you need.

@raphael-proust
Copy link
Contributor Author

thanks for the help and the suggestions and all

I'll use opam grep to find occurrences of exit_hooks and other such deprecated-now-removed functions, once this is done i'll see about doing a local health-check

@raphael-proust
Copy link
Contributor Author

in the case of the broken lwt_eio tests: eio tests the scheduling order of mixed eio+lwt programs, i'm still unclear why the order has changed but

  • it has changed to a less surprising scheduling
  • lwt doesn't make guarantees about scheduling order
  • i think it'd be very wrong to rely on the scheduling order of a mixed lwt+eio program

i'll just remove the tests altogether methinks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants