Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conflicts with other ppx_deriving plugins #22

Open
Leonidas-from-XIV opened this issue Oct 31, 2018 · 5 comments
Open

Conflicts with other ppx_deriving plugins #22

Leonidas-from-XIV opened this issue Oct 31, 2018 · 5 comments

Comments

@Leonidas-from-XIV
Copy link

When adding a [@@deriving protobuf] stanza to a type it works, but when any other deriver is added to the build (using dune at least), e.g. ppx_deriving.show or ppx_deriving_yojson compilation breaks with

Error: Cannot locate deriver protobuf

All the other derivers work fine with each other.

@ahem
Copy link

ahem commented Nov 2, 2018

I can reproduce this like this:

dune:

(library
  (name foo)
  (preprocess (pps ppx_deriving_protobuf ppx_deriving.show)))

foo.ml:

type t = int [@@deriving protobuf, show]
 $ dune build foo/foo.a
         ppx foo/foo.pp.ml (exit 1)
(cd _build/default && .ppx/ppx_deriving.show+ppx_deriving_protobuf/ppx.exe --cookie 'library-name="foo"' -o foo/foo.pp.ml --impl foo/foo.ml --dump-ast)
File "foo/foo.ml", line 1, characters 25-33:
Error: Cannot locate deriver protobuf

If I remove ppx_deriving.show and the show annotation the file builds without problems.

This is with ocaml-base-compiler.4.06.1, ppx_deriving_protobuf version 2.6 and ppx_deriving version 4.2.1

@rgrinberg
Copy link
Contributor

@whitequark now that we've transitioned ppx_deriving_protobuf and ppx_deriving_yojson to dune, how about we proceed with moving them into ppx_deriving? We've had this discussion on github before (I cant find the link), and we agreed on doing this will keeping the opam structure the same. So things would still be published as 3 opam packages, but things will be easier to maintain as the 3 projects will be maintained together. We'd be able to add tests for bugs such as this.

@whitequark
Copy link
Collaborator

Sure, go ahead. What do we do with the repos? I would prefer to update them to say that the main repo should now be used instead.

@XVilka
Copy link
Contributor

XVilka commented Aug 1, 2019

It is better to skip this step completely and do this only after ppx_deriving is ported to ppxlib (or future ppx).

@rgrinberg
Copy link
Contributor

It would be worse. It is much easier to port everything to ppxlib all at once.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants