Lwt: Avoid dependencies on compatibility modules when possible

Created on 9 Jul 2020  Â·  8Comments  Â·  Source: ocsigen/lwt

Currently, lwt depends on result and seq unconditionally. This is unfortunate for libraries that only use Lwt in combination with modern OCaml versions – including most Mirage libraries. This came up when attempting to disable implicit transitive dependencies in Irmin (see discussion).

One possible solution to this is to conditionally depend on these compatibility modules, in the same way as is already being done with ocaml-syntax-shims (see lwt.opam and conditional dune file). I have a branch that does this for Lwt. (The slight snag being that lwt.mli defines its own result type and then refers to Result.result – some sort of trickery is needed to get around that.)

I'm interested to know your opinions on this. If you think that conditional dependencies are the right way to go, I'm happy to make a PR with my branch.

CC. @avsm

Most helpful comment

Ok, convinced :) I will merge a PR doing this.

All 8 comments

result and seq are pretty much empty packages on recent OCaml versions. What's the issue exactly ?

The packages are still required in order to resolve their type aliases, so the dependencies propagates to all users (when e.g. I suspect the vast majority of users don't support 4.02).

  • When not building with implicit transitive dependencies, this requires depending on result at a build-system level, which in turn requires explicitly opam-depending on result unless one is willing to upper-bound the Lwt dependency to the latest version. In my experience, unconditionally depending on compatibility packages is the biggest annoyance with Dune's (implicit_transitive_deps false) – which is otherwise a very usable feature.

  • It's possible to re-export the type Result.result in the interface of a library, pushing this redundant dependency further downstream; we don't have good tooling to ensure this is avoided. The issue is worse for Seq, since it's necessary to remember to do Stdlib.Seq.t rather than Seq.t in all exported interfaces.

What's the motivation for using (implicit_transitive_deps false) in Mirage?

I can't speak for all Mirage developers, but we've been moving towards (implicit_transitive_deps false) in OCurrent, Irmin and Alcotest for two reasons:

  • it catches plenty of missing explicit Opam dependencies, which are otherwise practically undetectable. (These are particularly frustrating in a Mirage-like workflow with lots of smaller packages since dropping dependencies in upstream packages cascades into build failures downstream, even in historic versions.)

  • code becomes easier to read / debug, since the set of possible in-scope types and modules is always explicitly stated. (Rather than being just "the set of all ocamlfind packages pulled in by whatever dependency versions were picked during installation".)

Thanks.

Thinking about the various effects of this, it occurs to me that we might break some users who expect Result or Seq to be transitively pulled in, if they start compiling on high-enough versions of OCaml.

The conditional Dune file is fine, since it will already be conditional in Lwt until the minimum supported OCaml version is 4.08, and the branch only adds conditions on 4.03 and 4.07.

Indeed, this would be a breaking change for users who rely on Lwt pulling in Result even when it is not necessary (e.g. by having a non-conditional Dune file that states result as a library dependency explicitly), but this is true of Lwt dropping any dependency that might be implicitly relied upon by users.

Ok, convinced :) I will merge a PR doing this.

When not building with implicit transitive dependencies, this requires depending on result at a build-system level, which in turn requires explicitly opam-depending on result

another option is for lwt to re_export result library

Was this page helpful?
0 / 5 - 0 ratings

Related issues

aantron picture aantron  Â·  8Comments

idkjs picture idkjs  Â·  5Comments

aantron picture aantron  Â·  7Comments

aantron picture aantron  Â·  8Comments

m4b picture m4b  Â·  4Comments