c-cube / cconv

[dead] combinators for type conversion (serialization/deserialization) to/from several formats. See this blog post (outdated): http://cedeela.fr/universal-serialization-and-deserialization.html
https://c-cube.github.io/cconv/
BSD 2-Clause "Simplified" License
27 stars 3 forks source link

Switch build system to dune/jbuilder #14

Closed tizoc closed 6 years ago

tizoc commented 6 years ago

I probably have to tweak things a bit more, but this is what I have so far.

c-cube commented 6 years ago

I've got

File "_none_", line 1:
Error: The field Ast_406.Parsetree.ptyp_desc belongs to the record type
         Ast_406.Parsetree.core_type
       but a field was expected belonging to the record type
         Parsetree.core_type

when trying to compile (I'm on 4.05). I suppose you use a fixed version of the parsetree, but I don't know how it's supposed to be used exactly?

rgrinberg commented 6 years ago

Actually, if you're using ppx_deriving you're supposed to be using the version the parsetree that your ppx_deriving decided upon (using omp).

tizoc commented 6 years ago

I'm confused, because I checked ppx_deriving_yojson and it still uses cppo: https://github.com/ocaml-ppx/ppx_deriving_yojson/blob/master/src/ppx_deriving_yojson.cppo.ml

My guess is that you still have to handle different parsetree versions so that it compiles fine in different versions of ppx_deriving, but I'm not even sure of that, the more I read into Github issues related to ppx in other projects the more confused I get.

@c-cube what version of ppx_deriving are you using?

tizoc commented 6 years ago

Ok, I just made something ugly that may work:

(jbuild_version 1)

(library
 ((name ppx_deriving_cconv)
  (public_name cconv.ppx)
  (synopsis "[@@deriving cconv]")
  (kind ppx_rewriter)
  (libraries (ppx_deriving.api))
  (optional)
  (preprocess (action (run sh -c "${bin:ocamlfind} ppx_tools/rewriter -ppx `ocamlfind query ppx_tools.metaquot`/ppx_metaquot ${<}")))))

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

This uses the non-versioned ppx_tools.metaquot preprocessor (in my system it is an executable, I don't have a driver-based version, not sure if this is normal).

tizoc commented 6 years ago
faust ~/p/o/cconv git:jbuilder✓ > ocaml --version
The OCaml toplevel, version 4.05.0
faust ~/p/o/cconv git:jbuilder✓ > make test
jbuilder runtest --no-buffer
        cppo ppx/ppx_deriving_cconv.ml
          sh ppx/ppx_deriving_cconv.pp.ml
    ocamldep ppx/ppx_deriving_cconv.depends.ocamldep-output
      ocamlc ppx/ppx_deriving_cconv.{cmi,cmo,cmt}
    ocamlopt ppx/ppx_deriving_cconv.{cmx,o}
    ocamlopt ppx/ppx_deriving_cconv.{a,cmxa}
    ocamlopt .ppx/ppx_deriving.std+ppx_deriving_cconv/ppx.exe
         ppx tests/run_tests_ppx.pp.ml
    ocamldep tests/run_tests_ppx.depends.ocamldep-output
    ocamldep bencode/cConvBencode.depends.ocamldep-output
    ocamldep src/cConv.dependsi.ocamldep-output
    ocamldep bencode/cConvBencode.dependsi.ocamldep-output
    ocamldep src/cConv.depends.ocamldep-output
    ocamldep yojson/cConvYojson.depends.ocamldep-output
    ocamldep yojson/cConvYojson.dependsi.ocamldep-output
      ocamlc src/cConv.{cmi,cmti}
      ocamlc bencode/cConvBencode.{cmi,cmti}
      ocamlc yojson/cConvYojson.{cmi,cmti}
      ocamlc tests/run_tests_ppx.{cmi,cmo,cmt}
    ocamlopt src/cConv.{cmx,o}
    ocamlopt src/cConv.{a,cmxa}
    ocamlopt bencode/cConvBencode.{cmx,o}
    ocamlopt yojson/cConvYojson.{cmx,o}
    ocamlopt bencode/cConvBencode.{a,cmxa}
    ocamlopt yojson/cConvYojson.{a,cmxa}
    ocamlopt tests/run_tests_ppx.{cmx,o}
    ocamlopt tests/run_tests_ppx.exe
.............................
Ran: 29 tests in: 0.00 seconds.
OK
run_tests_ppx alias tests/runtest
tizoc commented 6 years ago

Let me know if there is something you think should be different or if you want me to squash the commits or organise them differently btw.

rgrinberg commented 6 years ago

Btw, I was a bit wrong about the ppx_deriver vs. ppx_rewriter thing. You should indeed continue to use ppx_deriver. It was clarified to me that ppx_deriver is still required to generate the correct meta to support being dynlinked by ppx_deriving.

c-cube commented 6 years ago

Nice! Thank you very much 😃