Port to dune and have uri.sexp module

[mseri]

If this is merged I am happy to update ocaml-cohttp and synchronize the release

[avsm]

I'm still wondering what the best way to do it is. I don't like the current one, which requires

module Uri = struct
  include Uri
  include Uri_sexp
end

before every use of Uri.t in a [@@deriving sexp] context.

[mseri]

Yes, I have seen the discussion on slack. I agree that it is quite suboptimal :-(

[avsm]

Oh, the answer was pretty simple -- just expose a type alias Uri_sexp.t = Uri.t. I've pushed this to this branch, and the cohttp diff is quite small too https://github.com/mirage/ocaml-cohttp/pull/634

[mseri]

Seems quite a good compromise

[mseri]

Thanks for the ocaml-cohttp PR. I'll wait for your PR, I have also a branch with cohttp using sexplib0

[avsm]

at icfp at the moment, so i'll merge this when i'm back in a few days and can check the fallout better

[struktured]

@avsm Is there no way to decorate the Uri module automatically with Uri.t_of_sexp and Uri.sexp_of_t if a user explicitly depends on uri.sexp? That is what I assumed would happen, but perhaps this is not easily accomplished.

[avsm]

@struktured that seems like a bit too much magic (it would change the Uri module type depending on whether some other dependency is selected). I prefer having an explicit type alias like Uri_sexp.t instead and not rely on linking-level tricks to get this to work -- unless you have a usecase that needs the latter?

[struktured]

@avsm Nothing required, just found it a little bit disruptive. I can live with the type alias. Thanks for the quick response.

[ozanmakes]

@avsm I just got the 2.0.0 update, migrating my app code to Uri_sexp.t was easy but I can't figure out how to patch a library I'm using as Uri_sexp doesn't seem to include all of the Uri module.

The type in question: https://github.com/brendanlong/sentry-ocaml/blob/2cd5b6220f79709ae51c7e251110cdf93ebbfe1e/src/dsn.ml#L3-L8

If I change this to use Uri_sexp, it cannot find the compare function. Including both Uri and Uri_sexp in a module like the snippet above fails with Multiple definition of the type name component.

Is there a way to do this without introducing some boilerplate to the original type declaration?

/cc @brendanlong

[avsm]

@osener the problem is that Uri_sexp doesn't define the compare deriver, so you can workaround it by:

module Uri_sexp = struct
  include Uri_sexp
  let compare = Uri.compare
end

type t' =
  { uri : Uri_sexp.t
  ; public_key : string
  ; private_key : string option
  ; project_id : int }
[@@deriving compare, sexp_of]

We could add a compare function to Uri_sexp to remove the need for this; see https://github.com/mirage/ocaml-uri/pull/127

It does mean that Uri_sexp is increasingly misnamed :-) Perhaps we should call it Uri_derive or similar to indicate that it contains derive-related extensions to the base module.

[ozanmakes]

127 covers everything I need, thanks for the quick response!