Port to ppx

[rgrinberg]

First commit explains why we must use ppx_driver over ppx_deriving for now.

The only thing that remains to be done here is to somehow tell ocamlbuild that
conduit_config.mlh is a dependency for every file that is being preprocessed.

Any ocamlbuild gurus can help out with this?

gasche samoht Drup dbuenzli agarwal (removed the mentions)

Also, we need to wait for TLS to switch over to ppx as well.

[gasche]

Below is a patch that seems to work on my machine. The #import change seems to suggest that your patch is not quite correct, and that more changes are needed, though.

diff --git a/build.sh b/build.sh
index 01c7bbe..01ed8bb 100755
--- a/build.sh
+++ b/build.sh
@@ -26,8 +26,7 @@ add_pkg () {

-CONDUIT_CONFIG=""
 mlh_exp() {
   if [ "$1" != "" ]; then
-    CONDUIT_CONFIG="$CONDUIT_CONFIG\n#let $2 = true"
+    echo "#let $2 = true" >> lib/conduit_config.mlh
   else
-    CONDUIT_CONFIG="$CONDUIT_CONFIG\n#let $2 = false"
+    echo "#let $2 = false" >> lib/conduit_config.mlh
   fi
@@ -46,4 +45,2 @@ mlh_exp "$HAVE_LAUNCHD_LWT" HAVE_LAUNCHD_LWT

-echo $CONDUIT_CONFIG > lib/conduit_config.mlh
-
 add_pkg "$BASE_PKG"
@@ -60,3 +57,5 @@ mkdir -p _install

-echo 'true: pp(/Users/rgrinberg/reps/ml/ocaml-conduit/ppx)' >> _tags
+echo "true: config" >> _tags
+
+echo "true: pp($(pwd)/ppx)" >> _tags

diff --git a/lib/conduit_lwt_unix.ml b/lib/conduit_lwt_unix.ml
index 3760976..f1c725e 100644
--- a/lib/conduit_lwt_unix.ml
+++ b/lib/conduit_lwt_unix.ml
@@ -18,3 +18,3 @@

-#import "conduit_config.mlh"
+#import "lib/conduit_config.mlh"

diff --git a/myocamlbuild.ml b/myocamlbuild.ml
index f1472a7..ec92d99 100644
--- a/myocamlbuild.ml
+++ b/myocamlbuild.ml
@@ -4,2 +4,3 @@ dispatch (
   | After_rules ->
+    dep ["ocaml"; "config"] ["lib/conduit_config.mlh"];
     dep ["ocaml"; "doc"] ["lib/intro.html"];

[rgrinberg]

Thanks a lot @gasche it all seems to be working well for me. Why did you change the #import path? The original path seems to be correct because otherwise ppx_optcomp will error out saying it's unable to find the file.

[rgrinberg]

By the way, I will switch to ppx_driver's ocamlbuild plugin once it hits opam

[rgrinberg]

@samoht @avsm think we can go ahead with this?

[rgrinberg]

If nobody objects, I will merge this and release a ppx based cohttp/conduit.

[gasche]

I had to change the import path because otherwise the compilation would fail on my machine. I think it would be nice to double-check the intended path lookup semantics to understand which version is the "correct" one -- in particular, if lib/ is the correct one, then other import instructions may need to be fixed. I don't know about any of this stuff but I can test a re-build later in the week if that can help.

[rgrinberg]

Could you give a compilation another try confirm that it's failing for you? It seems to be working for me and in CI.

Does this clarify the path semantics? https://github.com/janestreet/ppx_optcomp/blob/485720bdb4be74201945453fc05489e2d7771f3f/README.md#imports

[avsm]

Hm I'm seeing the failure as well:

File "lib/conduit_async.ml", line 18, characters 1-29:
optcomp: cannot open "conduit_config.mlh": conduit_config.mlh: No such file or directory
File "lib/conduit_async.ml", line 1:
Error: Error while running external preprocessor
Command line: /Users/avsm/src/git/avsm/ocaml-conduit/ppx 'lib/conduit_async.ml' > /var/folders/vr/h2mdzb2d2gv29pmvbywrnfjh0000gn/T/ocamlpp1e2b39

with

$ opam info ppx_optcomp
             package: ppx_optcomp
             version: 113.09.00
          repository: default
        upstream-url: https://github.com/janestreet/ppx_optcomp/archive/113.09.00.tar.gz
       upstream-kind: http
   upstream-checksum: f6b0d80f9d1224914c71f94ff62c2939
            homepage: https://github.com/janestreet/ppx_optcomp
         bug-reports: https://github.com/janestreet/ppx_optcomp/issues
            dev-repo: https://github.com/janestreet/ppx_optcomp.git
              author: Jane Street Group, LLC <opensource@janestreet.com>
             license: Apache-2.0
             depends: ocamlfind >= 1.3.2 & ppx_core (>= 113.09.00 & < 113.10.00) & ppx_tools & ppx_deriving & ocamlbuild
  installed-versions: 113.09.00 [system], 113.24.00 [latest]
  available-versions: 113.09.00, 113.24.00, 113.33.00
         description: Optional compilation for OCaml
Part of the Jane Street's PPX rewriters collection.

wonder if the path semantics have changed in a newer version. now upgrading

[solongordon]

@avsm I encountered the same build error when I tried to build with core suite 113.09.00. It was resolved after I upgraded to core 113.33.00 (including ppx_optcomp).

[gasche]

This probably means that there should be stricter dependency bounds in the ocaml-conduit metadata.

[rgrinberg]

OK will update. Thanks to @gasche @avsm and @solongordon for triaging.