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
Open

compatibility with lwt.6 #55

Show file tree
Hide file tree
Changes from 6 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
7 changes: 4 additions & 3 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.4"}
"trace" {>= "0.10" & < "0.11"}
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.

why the upper bound?

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.

there's a different incompatibility that's only unearthed when you unlock ocaml5 or lwt6

basically using lwt6+ocaml5 changes the dependency cone so that it allows trace>=0.11 which was not allowed before and that breaks some things, the upper bound was implicit by transitivity but has to be made explicit now

"extunix" {>= "0.1.4"}
"lwt" {>= "5.7.0"}
"lwt" {>= "6.0.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
12 changes: 7 additions & 5 deletions possibly_otel.real.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ module Traceparent = struct
let get_ambient ?explicit_span () =
let* Scope.{ trace_id; span_id; _ } = Scope.get_ambient_scope () in
let span_id = match explicit_span with
| Some {Trace_core.span; _} -> Opentelemetry_trace.Internal.otel_of_otrace span
| Some {Trace_core.span; _} -> Opentelemetry_trace.Conv.span_id_to_otel span
| None -> span_id
in
Trace_context.Traceparent.to_value ~trace_id ~parent_id:span_id ()
Expand All @@ -17,10 +17,12 @@ end
let enter_manual_span ~__FUNCTION__ ~__FILE__ ~__LINE__ ?data name =
match Scope.get_ambient_scope () with
| None ->
Trace_core.enter_manual_toplevel_span ~__FUNCTION__ ~__FILE__ ~__LINE__ ?data name
| Some Scope.{ span_id; _ } ->
Trace_core.enter_manual_span ~parent:None ~__FUNCTION__ ~__FILE__ ~__LINE__ ?data name
| Some Scope.{ span_id; trace_id; _ } ->
let otrace_espan = Trace_core.{
span = Opentelemetry_trace.Internal.otrace_of_otel span_id;
span = Opentelemetry_trace.Conv.span_id_of_otel span_id;
trace_id = Opentelemetry_trace.Conv.trace_id_of_otel trace_id;
meta = Trace_core.Meta_map.empty
} in
Trace_core.enter_manual_sub_span ~parent:otrace_espan ~__FUNCTION__ ~__FILE__ ~__LINE__ ?data name
let parent = Some (Trace_core.ctx_of_span otrace_espan) in
Trace_core.enter_manual_span ~parent ~__FUNCTION__ ~__FILE__ ~__LINE__ ?data name
2 changes: 1 addition & 1 deletion possibly_otel.stub.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@ end


let enter_manual_span ~__FUNCTION__ ~__FILE__ ~__LINE__ ?data name =
Trace_core.enter_manual_toplevel_span ~__FUNCTION__ ~__FILE__ ~__LINE__ ?data name
Trace_core.enter_manual_span ~parent:None ~__FUNCTION__ ~__FILE__ ~__LINE__ ?data name
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

()