mirage / mirage-crypto

Cryptographic primitives for OCaml, in OCaml (also used in MirageOS)
ISC License
77 stars 43 forks source link

Split sub libraries in `mirage-crypto-rng` into individual packages #158

Closed bikallem closed 1 year ago

bikallem commented 2 years ago

At the moment mirage-crypto-rng package contains sub-libraries in the same package.

  1. mirage-crypto-rng.lwt
  2. mirage-crypto-rng.unix

This results in pulling in extra dependencies where they are not needed, for example mirage-crypto-rng-async and the new package mirage-crypto-rng-eio packages pulling in at least unix and lwt where they are not needed. Should these sub libraries be split into their own separate pacakges? i.e. mirage-crypto-rng-lwt and mirage-crypto-rng-unix ?

hannesm commented 2 years ago

There's a tradeoff between number of opam packages (putting load on the opam solver, maintenance burden of opam files) and dependencies. These are all build-time dependencies (and only part of binaries if you use the mirage-crypto-rng.unix / mirage-crypto-rng.lwt packages).

Since this has been the case for quite some time, other opam packages that depend on mirage-crypto-rng would need to be updated to refer to mirage-crypto-rng-unix / mirage-crypto-rng-lwt (your proposal in #159). Another option would be optional dependencies (depopt in opam lingua).

So, what is the motivation for your proposed change? Do you work on platforms where unix and lwt are not available? Do you find the compilation times of lwt and unix inacceptable?

hannesm commented 1 year ago

So I went ahead and opened #168 which splits off the mirage-crypto-rng-lwt library. That'll allow for a leaner build chain.

I'm wondering what you think about "unix" (which is a library distributed with the OCaml code base) -- AFAICT the mirage-crypto-rng library does not have a dependency on it (mirage-crypto-rng.unix has) -- and mirage-crypto-rng-eio shouldn't link that mirage-crypto-rng.unix in -- or am I misguided? (i.e. I don't quite understand why there should be a separate mirage-crypto-rng-unix -- will that shave some further dependencies (if yes, which, and in what scenario?))?

hannesm commented 1 year ago

fixed in #168, please report separately when you feel there should be a mirage-crypto-rng-unix package.