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 8 commits into
base: master
Choose a base branch
from
+16 −4
Open

compatibility with lwt.6 #55

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2a2f3ea
compatibility with lwt.6
raphael-proust Feb 18, 2026
54f1db0
engine selection is based on lwt reporting its engine
raphael-proust Feb 18, 2026
2a212e1
incompatible with lwt<6
raphael-proust Feb 18, 2026
fb1cd3d
compat with recent otel/trace
raphael-proust Feb 19, 2026
11819a7
trace.0.11 has breaking deprecations and incompat with otel
raphael-proust Feb 26, 2026
ab08bb1
fix stub for when otel not installed
raphael-proust Feb 27, 2026
de70637
trace.0.12 compat, but dirty by removing some actual tracing capabilitie
raphael-proust Mar 13, 2026
e0b576f
Merge branch 'master' into lwt6-compat
raphael-proust May 12, 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.0"}
"lwt_ppx"
"base-bytes"
"base-unix"
Expand Down
4 changes: 4 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,17 @@ 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

method! 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
Copy link
Copy Markdown
Contributor

@rr0gi rr0gi Mar 9, 2026

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?

Copy link
Copy Markdown
Contributor Author

@raphael-proust raphael-proust Mar 14, 2026

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

Copy link
Copy Markdown
Contributor

@rr0gi rr0gi Mar 19, 2026

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
Copy link
Copy Markdown
Contributor

@rr0gi rr0gi May 12, 2026

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));
Copy link
Copy Markdown
Contributor

@rr0gi rr0gi Mar 9, 2026

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)

Copy link
Copy Markdown
Contributor Author

@raphael-proust raphael-proust Mar 14, 2026

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

Copy link
Copy Markdown
Contributor

@rr0gi rr0gi May 12, 2026

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