Skip to content

compatibility with lwt.6 #55

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.

Sign up for GitHub

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

Open
raphael-proust wants to merge 5 commits into
base: master
Choose a base branch
from
Open

compatibility with lwt.6 #55

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
213d585
compatibility with lwt.6
raphael-proust Feb 18, 2026
0e2a76b
engine selection is based on lwt reporting its engine
raphael-proust Feb 18, 2026
048634d
incompatible with lwt<6
raphael-proust Feb 18, 2026
ce8acf6
suppress method override warning in engine's id
raphael-proust Sep 8, 2026
f1b5744
opam constraint for lwt: 5.10
raphael-proust Sep 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions devkit.opam
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,12 @@ depends: [
"ounit2"
"camlzip"
"libevent" {>= "0.8.0"}
"ocurl" {>= "0.7.2"}
"curl" {>= "0.10.0"}
"curl_lwt"
"pcre2" {>= "8.0.3"}
"trace" {>= "0.12"}
"extunix" {>= "0.1.4"}
"lwt" {>= "5.7.0"}
"lwt" {>= "5.10"}
"lwt_ppx"
"base-bytes"
"base-unix"
Expand Down
5 changes: 5 additions & 0 deletions lwt_engines.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,18 @@ method poll fds timeout =
l
end

type Lwt_engine.engine_id += Engine_id__Devkit_libevent

(** libevent-based engine for lwt *)
class libevent =
let once_block = Ev.[ONCE] in
let once_nonblock = Ev.[ONCE;NONBLOCK] in
object(self)
inherit Lwt_engine.abstract

(* suppressing warning 7: for compatibility with both lwt.5.10 and lwt.6
once we move fully to lwt.6 and drop compat with 5.10, we can remove the attribute *)
method[@warning "-7"] id = Engine_id__Devkit_libevent
val events_ = Ev.init ()
val mutable pid = Unix.getpid ()
method events =
Expand Down
11 changes: 9 additions & 2 deletions prelude.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,13 @@ let call_me_maybe f x =
and poll is guaranteed to be available without the fd limitation.
*)
let () =
if not (Lwt_config._HAVE_LIBEV && Lwt_config.libev_default) then begin

@rr0gi rr0gi Mar 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this method not available in lwt 6?

@raphael-proust raphael-proust Mar 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it is available

is the concern that then we don't have a version of devkit that's compatible with both 5.9 and 6 so it makes the upgrade for users more difficult? if that's the case then i have ocsigen/lwt#1106 in the works to allow specifically for that

@rr0gi rr0gi Mar 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yes, would prefer to be less disruptive

match Lwt_engine.id () with
| Lwt_engine.Engine_id__libev _ -> ()
| Lwt_engine.Engine_id__select ->
(* Otherwise, prefer poll over select, because select can only monitor fds up to 1024,
and poll is guaranteed to be available without the fd limitation. *)
Lwt_engine.set @@ new Lwt_engines.poll
end
| Lwt_engine.Engine_id__poll -> ()
Comment on lines +65 to +70

@rr0gi rr0gi May 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
| Lwt_engine.Engine_id__libev _ -> ()
| Lwt_engine.Engine_id__select ->
(* Otherwise, prefer poll over select, because select can only monitor fds up to 1024,
and poll is guaranteed to be available without the fd limitation. *)
Lwt_engine.set @@ new Lwt_engines.poll
end
| Lwt_engine.Engine_id__poll -> ()
| Engine_id__libev _ -> ()
| Engine_id__select ->
(* Otherwise, prefer poll over select, because select can only monitor fds up to 1024,
and poll is guaranteed to be available without the fd limitation. *)
Lwt_engine.set @@ new Lwt_engines.poll
| Engine_id__poll -> ()

| lwteng ->
eprintfn "Unknown Lwt engine (%s) in use, leaving as is" Obj.Extension_constructor.(name (of_val lwteng));

@rr0gi rr0gi Mar 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i see Obj i suspect
there is no name function to use?
but then anw i would rather emit log for non-default behaviour branch (ie select=>poll) and do not log anything in other branches (ie here)

@raphael-proust raphael-proust Mar 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i can make that change

@rr0gi rr0gi May 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this comment remains, no need to log when we dont change anything, only need to log when switching select=>poll

()
Loading