Switch the build system to dune #80
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Sure, amazing. Hopefully it will be merged soon. |
|
Here the difference with and without this PR in terms of what is installed: and in terms of what is different: The binaries and libraries are compiled differently so I assume this is normal. |
|
@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 |
|
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 ()Is there no better solution? |
|
No, you can't avoid these files being installed. |
|
(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!) |
|
revdeps check in opam-repository showed that i forgot to list the ppx runtime dependencies for |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.