search

janestreet / ppx_sexp_conv

Generation of S-expression conversion functions from type definitions
MIT License
88 stars 17 forks source link

Doesn't work with explicit transitive deps #33

Closed mimoo closed 3 years ago

mimoo commented 3 years ago

Hey!

When setting implicit_transitive_deps to false in a dune-project it complains if I use ppx_sexp_conv:

Error: The module Ppx_sexp_conv_lib.Sexp is an alias for module Sexplib0.Sexp, which is missing
aalekseyev commented 3 years ago

When using implicit_transitive_deps false, you should be prepared to have to add random things transitive dependencies to your jbuild files. In this case I think you have to add sexplib0.

Closing since this is expected. Feel free to reopen if you think we can/should do something on our end about it.

mimoo commented 3 years ago

That's not really what the documentation of implicit_transitive_deps implies. I'm relatively new to OCaml so I'm not sure if this is the solution, but I've seen other ppx libraries mentioning "runtime" libraries that you have to include in addition to the ppx.

aalekseyev commented 3 years ago

Oh, I see. Since this is a ppx rewriter, we do have a mechanism to make implicit_transitive_deps false work with no annotation. Since it's a simple change, I'll make it, but I do think it's weird that implicit_transitive_deps false creates this compatibility burden for the individual sexp rewriters written and tested against implicit_transitive_deps true.

mimoo commented 3 years ago

Yeah :/ ideally implicit_transitive_deps would be set to true by default but this is still not the case.

aalekseyev commented 3 years ago

This change landed in 5ae3e96d6703802a02ea7d190a33caec0376e95d.