search

mirleft / ocaml-nocrypto

OCaml cryptographic library
ISC License
111 stars 53 forks source link

Auto-detect ppx_sexp_conv runtime dependencies #146

Closed ghost closed 5 years ago

ghost commented 6 years ago

This PR allows to automatically detect the runtime dependencies of ppx_sexp_conv in order to store them in the META file. This allows ocaml-nocrypto to be compatible with both ppx_sexp_conv < v0.11 and >= v0.11.

The method used should work with any ppx rewriter, not just ppx_sexp_conv.

hannesm commented 6 years ago

thanks @diml, I looked into this locally, and discovered that this expands on my system to base.caml base.shadow_stdlib sexplib0 base ppx_sexp_conv.runtime-lib result ppx_deriving.runtime -- this means ppx_sexp_conv depends at runtime on base (similar to sexplib=v0.11.0), is this the desired behaviour (again, similar to sexplib, this is not a good option for MirageOS due to binary size)!?

hannesm commented 6 years ago

oops, sorry, please ignore my comment, I was stuck with a sexplib v0.11.0, upgrading to v0.11.1 solved this issue.

samoht commented 6 years ago

Maybe we don't need to add this complexity to the build system and depend unconditionally on the latest version of ppx_sexp_conv?

copy commented 6 years ago

@samoht That's what https://github.com/mirleft/ocaml-nocrypto/pull/144 does.

hcarty commented 6 years ago

Where does this patch/fix stand currently? From what I can see, the current state in opam prevents the use of ppxlib and nocrypto (and therefore tls) in the same project.

copy commented 6 years ago

Where does this patch/fix stand currently? From what I can see, the current state in opam prevents the use of ppxlib and nocrypto (and therefore tls) in the same project.

The patches are in opam now, so nocrypto and ppxlib can be used together. The tls issue is being tracked here: https://github.com/mirleft/ocaml-tls/issues/379, and we need to summon @hannesm for that.

hcarty commented 6 years ago

Thank you @copy!

cfcs commented 5 years ago

master got rid of the sexp stuff now, so I think this PR can be closed? https://github.com/mirleft/ocaml-nocrypto/commit/a58c6534c5cc5da1598f8c979ea397ea2c6b92e1

XVilka commented 5 years ago

Ping? Sounds like indeed should be closed.

ghost commented 5 years ago

Sure, let's close this