Jbuilder fix

[tizoc]

Jbuilder uses ppx_driver to run ppx rewriters, but for the rewriters to work they have to be compiled with -linkall[1].

Oasis is generating an entry for the dynlink file with the use_ppx_deriving_cconv option, this causes double linking errors, to fix that I had to negate that option in the part of _tags that Oasis doesn't touch.

After these changes the ppx extension works fine on my machine both when using jbuilder and the ocamlbuild based Makefile I was using before.

[1] https://github.com/janestreet/ppx_driver#creating-a-new-ppx_driver-based-rewriter

[c-cube]

Actually that might be a good time to transition cconv to jbuilder, what do y ou think?

[tizoc]

Sounds good to me. Initially my intention was just to make the build process use jbuilder to avoid having to deal with oasis, but then I thought that it would be a bigger change and more work to review.

Anyway, I have to read a bit more, I just started using jbuilder yesterday, and while the experience has been good so far there are some things I have to figure out how to do before being able to port cconv's build system (in particular, handling of optional dependencies).

[c-cube]

Don't hesitate to ask questions, I've migrated several projects to jbuilder so far. For optional dependencies the trick is the (optional) field in sub-libraries (look at how it's done in containers).

[tizoc]

Ok, I have it pretty much working with jbuilder, tests and all, but there is something I'm not fully sure about.

With the change in this PR the file would still be processed with the regular ppx_tools.metaquot preprocessor, but with jbuilder I have to make it work with ppx_driver, and it doesn't seem to like regular metaquot, I have to use the versioned version:

(jbuild_version 1)

(library
 ((name ppx_deriving_cconv)
  (public_name cconv.ppx)
  (synopsis "[@@deriving cconv]")
  (kind ppx_deriver)
  (libraries (cconv ppx_deriving.api))
  (optional)
  (preprocess (pps (ppx_tools_versioned.metaquot_406 ppx_driver.runner)))))

(rule
 ((targets (ppx_deriving_cconv.ml))
  (deps    (ppx_deriving_cconv.cppo.ml))
  (action  (run ${bin:cppo} -V OCAML:${ocaml_version} ${<} -o ${@}))))

Do you know if this will cause problems with earlier versions of OCaml? I'm not familiar with ppx_tools_versioned, I didn't know such thing existed until today.

[rgrinberg]

Use (kind ppx_rewriter). ppx_deriver is deprecated.

I don't see why ppx_driver.runner is necessary. Is there an error if you don't use it?

Also, I think you should get rid of cppo as well. If you're using omp, you should just pick the version of the parsetree you want to program against.

[tizoc]

@rgrinberg thank you, changed it to ppx_rewriter, also removed ppx_driver.runner from the list of preprocessors, not needed. The PR implementing the switch to dune is here https://github.com/c-cube/cconv/pull/14

Edit: cppo dependency is gone too.

[c-cube]

This seems obsolete, isn't it? Please test the new master branch and close this if it's fixed.

Thanks again :)

[c-cube]

Note that the cconv.ppx doesn't seem to require cconv.

[tizoc]

@c-cube yes, this one is obsolete, closing.