Switch the build system to dune

[kit-ty-kate]

This PR is an alternative to #74 (previously #67). cc @diml @XVilka

The main change regarding to the original PRs is that the dune file is almost a 1 to 1 equivalent to the existing Makefile. I removed the micro-driver module that was questioned in #67 by @Drup and #74 by myself.

The micro-driver module can be added separately but I think personally I'd rather restrain the changes to the minimum at once. I can do several releases, one with just the switch to dune, and then if there is a consensus, something more involved in the next release.

[XVilka]

Sure, amazing. Hopefully it will be merged soon.

[kit-ty-kate]

Here the difference with and without this PR in terms of what is installed:

$ diff -u old new
--- old 2020-01-27 11:24:16.910256982 +0000
+++ new 2020-01-27 11:28:57.421294250 +0000
@@ -1,17 +1,24 @@
+/home/kit_ty_kate/.opam/4.10/doc/ppx_tools
+/home/kit_ty_kate/.opam/4.10/doc/ppx_tools/LICENSE
+/home/kit_ty_kate/.opam/4.10/doc/ppx_tools/README.md
 /home/kit_ty_kate/.opam/4.10/lib/ppx_tools
 /home/kit_ty_kate/.opam/4.10/lib/ppx_tools/META
 /home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ast_convenience.cmi
 /home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ast_convenience.cmt
 /home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ast_convenience.cmti
 /home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ast_convenience.cmx
+/home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ast_convenience.ml
 /home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ast_convenience.mli
 /home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ast_mapper_class.cmi
 /home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ast_mapper_class.cmt
 /home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ast_mapper_class.cmti
 /home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ast_mapper_class.cmx
+/home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ast_mapper_class.ml
 /home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ast_mapper_class.mli
 /home/kit_ty_kate/.opam/4.10/lib/ppx_tools/dumpast
+/home/kit_ty_kate/.opam/4.10/lib/ppx_tools/dune-package
 /home/kit_ty_kate/.opam/4.10/lib/ppx_tools/genlifter
+/home/kit_ty_kate/.opam/4.10/lib/ppx_tools/opam
 /home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ppx_metaquot
 /home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ppx_tools.a
 /home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ppx_tools.cma

and in terms of what is different:

$ diff -ru old.dir new.dir
Binary files old.dir/ast_convenience.cmt and new.dir/ast_convenience.cmt differ
Binary files old.dir/ast_convenience.cmti and new.dir/ast_convenience.cmti differ
Binary files old.dir/ast_convenience.cmx and new.dir/ast_convenience.cmx differ
Only in new.dir: ast_convenience.ml
Binary files old.dir/ast_mapper_class.cmt and new.dir/ast_mapper_class.cmt differ
Binary files old.dir/ast_mapper_class.cmti and new.dir/ast_mapper_class.cmti differ
Only in new.dir: ast_mapper_class.ml
Binary files old.dir/dumpast and new.dir/dumpast differ
Only in new.dir: dune-package
Binary files old.dir/genlifter and new.dir/genlifter differ
diff -ru old.dir/META new.dir/META
--- old.dir/META    2020-01-27 11:24:38.862806835 +0000
+++ new.dir/META    2020-01-27 11:28:50.549121636 +0000
@@ -1,8 +1,10 @@
 version = "5.3"
 description = "Tools for authors of ppx rewriters and other syntactic tools"
+requires = "compiler-libs.common"
 archive(byte) = "ppx_tools.cma"
 archive(native) = "ppx_tools.cmxa"
-requires = "compiler-libs.common"
+plugin(byte) = "ppx_tools.cma"
+plugin(native) = "ppx_tools.cmxs"

 package "metaquot" (
   version = "5.3"
Only in new.dir: opam
Binary files old.dir/ppx_metaquot and new.dir/ppx_metaquot differ
Binary files old.dir/ppx_tools.a and new.dir/ppx_tools.a differ
Binary files old.dir/ppx_tools.cma and new.dir/ppx_tools.cma differ
Binary files old.dir/ppx_tools.cmxa and new.dir/ppx_tools.cmxa differ
Binary files old.dir/ppx_tools.cmxs and new.dir/ppx_tools.cmxs differ
Binary files old.dir/rewriter and new.dir/rewriter differ

The binaries and libraries are compiled differently so I assume this is normal.

[ghost]

@kit-ty-kate that all looks good to me. I'm not clicking the "approve" button as I'm not listed as an explicit maintainer of the project, but I read the diff and it looks correct to me.

I would just declare a ppx_metaquot library with (kind ppx_rewriter) to avoid the custom META file. That would mean installing a ppx_metaquot binary twice (assuming you want to keep a binary named <libdir>/ppx_tools/ppx_metaquot), but on the other hand it would mean using simpler features.

[kit-ty-kate]

The issue with this solution is that I feel like it installs way too much mandatory files than necessary:

diff --git a/META.ppx_tools.template b/META.ppx_tools.template
deleted file mode 100644
index 99bacab..0000000
--- a/META.ppx_tools.template
+++ /dev/null
@@ -1,8 +0,0 @@
-# DUNE_GEN
-
-package "metaquot" (
-  version = "5.3"
-  description = "Meta-quotation: Parsetree manipulation using concrete syntax"
-  requires = "ppx_tools"
-  ppx = "./ppx_metaquot"
-)
diff --git a/dune b/dune
index 356b3b9..af9a203 100644
--- a/dune
+++ b/dune
@@ -5,6 +5,14 @@
  (modules ast_convenience ast_mapper_class)
  (libraries compiler-libs.common))

+(library
+ (name ppx_metaquot)
+ (public_name ppx_tools.metaquot)
+ (kind ppx_rewriter)
+ (modules ppx_metaquot)
+ (ppx.driver (main Ppx_metaquot.Main.main))
+ (libraries compiler-libs.common ppx_tools ast_lifter))
+
 (executable
  (name genlifter)
  (modules genlifter)
@@ -16,9 +24,9 @@
  (libraries compiler-libs.common compiler-libs.bytecomp ast_lifter))

 (executable
- (name ppx_metaquot)
- (modules ppx_metaquot)
- (libraries compiler-libs.common ppx_tools ast_lifter))
+ (name ppx_metaquot_main)
+ (modules ppx_metaquot_main)
+ (libraries ppx_metaquot))

 (executable
  (name rewriter)
@@ -31,6 +39,7 @@

 (library
  (name ast_lifter)
+ (public_name ppx_tools.ast_lifter)
  (wrapped false)
  (modules ast_lifter)
  (flags :standard -w -17)
@@ -41,5 +50,5 @@
  (files
   (genlifter.exe as genlifter)
   (dumpast.exe as dumpast)
-  (ppx_metaquot.exe as ppx_metaquot)
+  (ppx_metaquot_main.exe as ppx_metaquot)
   (rewriter.exe as rewriter)))
diff --git a/ppx_metaquot.ml b/ppx_metaquot.ml
index 990992a..943c06a 100644
--- a/ppx_metaquot.ml
+++ b/ppx_metaquot.ml
@@ -59,7 +59,9 @@

 *)

-module Main : sig end = struct
+module Main : sig
+  val main : unit -> unit
+end = struct
   open Asttypes
   open Parsetree
   open Ast_helper
@@ -282,5 +284,5 @@ module Main : sig end = struct
     in
     {super with expr; pat; structure; structure_item; signature; signature_item}

-  let () = Ast_mapper.run_main expander
+  let main () = Ast_mapper.run_main expander
 end
diff --git a/ppx_metaquot_main.ml b/ppx_metaquot_main.ml
new file mode 100644
index 0000000..4bab3f6
--- /dev/null
+++ b/ppx_metaquot_main.ml
@@ -0,0 +1 @@
+let () = Ppx_metaquot.Main.main ()

$ diff -u new new+dune
--- new 2020-01-27 11:28:57.421294250 +0000
+++ new+dune    2020-01-27 17:29:31.539474045 +0000
@@ -9,6 +9,15 @@
 /home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ast_convenience.cmx
 /home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ast_convenience.ml
 /home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ast_convenience.mli
+/home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ast_lifter
+/home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ast_lifter/ast_lifter.a
+/home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ast_lifter/ast_lifter.cma
+/home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ast_lifter/ast_lifter.cmi
+/home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ast_lifter/ast_lifter.cmt
+/home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ast_lifter/ast_lifter.cmx
+/home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ast_lifter/ast_lifter.cmxa
+/home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ast_lifter/ast_lifter.cmxs
+/home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ast_lifter/ast_lifter.ml
 /home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ast_mapper_class.cmi
 /home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ast_mapper_class.cmt
 /home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ast_mapper_class.cmti
@@ -18,6 +27,16 @@
 /home/kit_ty_kate/.opam/4.10/lib/ppx_tools/dumpast
 /home/kit_ty_kate/.opam/4.10/lib/ppx_tools/dune-package
 /home/kit_ty_kate/.opam/4.10/lib/ppx_tools/genlifter
+/home/kit_ty_kate/.opam/4.10/lib/ppx_tools/metaquot
+/home/kit_ty_kate/.opam/4.10/lib/ppx_tools/metaquot/ppx.exe
+/home/kit_ty_kate/.opam/4.10/lib/ppx_tools/metaquot/ppx_metaquot.a
+/home/kit_ty_kate/.opam/4.10/lib/ppx_tools/metaquot/ppx_metaquot.cma
+/home/kit_ty_kate/.opam/4.10/lib/ppx_tools/metaquot/ppx_metaquot.cmi
+/home/kit_ty_kate/.opam/4.10/lib/ppx_tools/metaquot/ppx_metaquot.cmt
+/home/kit_ty_kate/.opam/4.10/lib/ppx_tools/metaquot/ppx_metaquot.cmx
+/home/kit_ty_kate/.opam/4.10/lib/ppx_tools/metaquot/ppx_metaquot.cmxa
+/home/kit_ty_kate/.opam/4.10/lib/ppx_tools/metaquot/ppx_metaquot.cmxs
+/home/kit_ty_kate/.opam/4.10/lib/ppx_tools/metaquot/ppx_metaquot.ml
 /home/kit_ty_kate/.opam/4.10/lib/ppx_tools/opam
 /home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ppx_metaquot
 /home/kit_ty_kate/.opam/4.10/lib/ppx_tools/ppx_tools.a
$ diff -ru --exclude dune-package new.dir new+dune.dir
Only in new+dune.dir: ast_lifter
diff -ru new.dir/META new+dune.dir/META
--- new.dir/META    2020-01-27 11:28:50.549121636 +0000
+++ new+dune.dir/META   2020-01-27 17:29:19.899394734 +0000
@@ -5,10 +5,27 @@
 archive(native) = "ppx_tools.cmxa"
 plugin(byte) = "ppx_tools.cma"
 plugin(native) = "ppx_tools.cmxs"
-
+package "ast_lifter" (
+  directory = "ast_lifter"
+  version = "5.3"
+  description = ""
+  requires = "compiler-libs.common"
+  archive(byte) = "ast_lifter.cma"
+  archive(native) = "ast_lifter.cmxa"
+  plugin(byte) = "ast_lifter.cma"
+  plugin(native) = "ast_lifter.cmxs"
+)
 package "metaquot" (
+  directory = "metaquot"
   version = "5.3"
-  description = "Meta-quotation: Parsetree manipulation using concrete syntax"
-  requires = "ppx_tools"
-  ppx = "./ppx_metaquot"
+  description = ""
+  requires(ppx_driver) = "compiler-libs.common ppx_tools ppx_tools.ast_lifter"
+  archive(ppx_driver,byte) = "ppx_metaquot.cma"
+  archive(ppx_driver,native) = "ppx_metaquot.cmxa"
+  plugin(ppx_driver,byte) = "ppx_metaquot.cma"
+  plugin(ppx_driver,native) = "ppx_metaquot.cmxs"
+  # This line makes things transparent for people mixing preprocessors
+  # and normal dependencies
+  requires(-ppx_driver) = ""
+  ppx(-ppx_driver,-custom_ppx) = "./ppx.exe --as-ppx"
 )
Only in new+dune.dir: metaquot
Binary files new.dir/ppx_metaquot and new+dune.dir/ppx_metaquot differ

Is there no better solution?

[ghost]

No, you can't avoid these files being installed.

[kit-ty-kate]

Ok let's do this then. Is everyone ok with merging this now? @Drup @gasche?

I'll merge this by Tuesday afternoon if yes and release ppx_tools.6.0+4.08.0 with this PR and ppx_tools.6.1+4.10.0 after merging https://github.com/ocaml-ppx/ppx_tools/pull/79. Does that sound ok for you?

[gasche]

(I don't have time to look into this and give good feedback, so please do as you think is best. Thanks for your work!)

[kit-ty-kate]

revdeps check in opam-repository showed that i forgot to list the ppx runtime dependencies for ppx_tools.metaquot: https://github.com/ocaml-ppx/ppx_tools/commit/69f8c6768872e6204bf4de4d4c0e7db057aa810b