Remove hard dependency on camlp4

[whitequark]

The dependency stems from with sexp.

[rgrinberg]

I recall yminsky stating that type_conv would be ported over to ppx. How about we just wait for that. Or you could help them out :D

[whitequark]

@rgrinberg Release 1.0 of ppx_deriving (which is alllmost there) is going to replace type_conv, yes. (There's no ppx_deriving_sexp, though, but it's not that much of a problem).

The question is more of compatibility.

[dbuenzli]

Frankly I don't understand why a library that provides a simple service like uri parsing has to impose everybody a dependency on sexp.

[whitequark]

@dbuenzli To allow serializing Uri.t with sexplib. ppx_deriving solves this problem elegantly: you can do [@@deriving sexp { optional = true }], and if ppx_deriving_sexp is not installed, the definition (or signature) will be merely ignored. Then you just add a depopts: [ "ppx_deriving_sexp" ].

[dbuenzli]

Le jeudi, 23 octobre 2014 à 10:44, Peter Zotov a écrit :

@dbuenzli (https://github.com/dbuenzli) To allow serializing Uri.t with sexplib. ppx_deriving solves this problem elegantly:

I wouldn't call anything that depends on ppx elegant but allowing to move that to a deptopt is surely an improvement.

Daniel

[whitequark]

@dbuenzli Why not?

[dbuenzli]

Because like pre-processors ast rewriters are ugly non-compositional tools that are oblivious of the language's scoping rules and that complicate build systems. See this comment for more details.

[whitequark]

@dbuenzli Ah, I see. I agree with your complaints 2 and 3, but complaint 1 is now immaterial; you just do -package whatever.ppx.

[dbuenzli]

Le jeudi, 23 octobre 2014 à 11:31, Peter Zotov a écrit :

@dbuenzli (https://github.com/dbuenzli) Ah, I see. I agree with your complaints 2 and 3, but complaint 1 is now immaterial; you just do -package whatever.ppx.

Not really, this is through the obscure layer of indirection that ocamlfind is; at the compiler tool level you had to introduce a new concept, namely -ppx flags.

Daniel

[whitequark]

I don't think this anyhow differs from e.g. passing library.cma to the compiler when you want to link an external library; in both cases you either do it manually or use "the obscure layer of indirection". That being said, I do think ocamlfind ought to be a part of the compiler.

[dbuenzli]

Le jeudi, 23 octobre 2014 à 11:55, Peter Zotov a écrit :

I don't think this anyhow differs from e.g. passing library.cma to the compiler when you want to link an external library;

Sure but you still have two different concepts, libraries and ast rewriters. You could have only one. Conceptual economy is important.

Daniel

[whitequark]

I don't think so. They differ by the phase in which they execute, and this is important. Cross-compilation would be even harder than it is today if we unified libraries and AST rewriters conceptually, among other things. Languages which try to achieve conceptual similarity between these (e.g.: Scheme, Rust) still have the phase distinction anyway, and for a good reason.

[dbuenzli]

Le jeudi, 23 octobre 2014 à 12:07, Peter Zotov a écrit :

I don't think so. They differ by the phase in which they execute, and this is important. Cross-compilation would be even harder than it is today if we unified libraries and AST rewriters conceptually, among other things.

That's unclear to me. I suspect it depends if you shift the cross-compilation capabilities in the compiler itself rather than use path hacks. Currently AST rewriters have to be compiled for the host platform which is problematic.

Daniel

[whitequark]

This will be exactly same as long as we allow arbitrary code to appear in AST rewriters. Consider this: ocamlc.opt and ocamlopt.opt can only be extended by adding more native code built for the host platform. So AST rewriters will always require invoking a compiler for the host platform, and in case of cross-compilers, a second compiler, because ocamlopt is not retargetable.

[dbuenzli]

Le jeudi, 23 octobre 2014 à 12:34, Peter Zotov a écrit :

This will be exactly same as long as we allow arbitrary code to appear in AST rewriters.

Not from the end user's point of view.

Daniel

[rgrinberg]

Anyway, the reason why uri uses sexp is mostly for downstream users that use sexp (cohttp for example). I would not mind reaching a compromise by writing the sexp converters by hand to get rid of camlp4.

This also makes me wish that the sexp type was a polymorphic variant so that we didn't have to depend on sexp at all.

[whitequark]

In my opinion, adding a dependency on cppo and surrounding with sexp with #ifs is the better variant, as it removes hard dependency on sexp and still doesn't require you to write them by hand.

[dsheets]

I'd rather sexp used ppx and this library shipped a no-op sexpppx macro and used the real sexp ppx when it is available.

[avsm]

I'm really not keen on turning the library into ifdef hell with optcomp. Either a camlp4 rewriter that outputs ppx annotations as a one-shot conversion (that could be annotated), or just making future revisions of the library 4.02-only. For now, I'm prioritising 4.01 support over removing the camlp4 build dependency until a better solution is available.

[whitequark]

@avsm Your first solution will not make it camlp4-free though...

@dsheets That's possible with ppx_deriving. However, it still means a hard dep on ocaml >=4.02.

[avsm]

@whitequark: the camlp4 rewriter can be run as a preprocessing phase to generate a 4.02/ppx distribution version. Eventually we'd cut over to the ppx version when 4.01 compatibility is no longer needed.

[whitequark]

I see. That is possible once https://github.com/ocaml/camlp4/issues/55 is fixed and sexplib gains ppx_deriving support.

[whitequark]

I think one possible replacement for with sexp would be ppx_deriving_cconv, to derive calls to CConv combinators. The main advantage is that CConv doesn't constrain you to a single format like sexp, but allows to use whatever you like more.

[hcarty]

The dependency on a pre-processor (currently ppx rather than camlp4) makes cross-compilation of Uri with @whitequark's opam-cross-windows much more complicated than it would otherwise be due to all the extra dependencies. How feasible would it be to make the sexp pieces completely optional, now that the move has been made from camlp4 to ppx?

[hcarty]

Making the library compile looks relatively straightforward - this seems to a be a minimal diff to "just" remove sexp functions, ignoring tests.

diff --git a/_oasis b/_oasis
index 9a9d30b..c21278e 100644
--- a/_oasis
+++ b/_oasis
@@ -20,8 +20,8 @@ Flag allservices
 Library uri
   Path:       lib
   Modules:    Uri,Uri_re
-  BuildDepends: re.posix,stringext,sexplib,ppx_sexp_conv
-  XMETARequires: re.posix,stringext,sexplib
+  BuildDepends: re.posix,stringext
+  XMETARequires: re.posix,stringext

 Library services
   Path:       lib
@@ -68,7 +68,7 @@ Executable test_runner
   Custom:             true
   CompiledObject:     best
   Install:            false
-  BuildDepends:       uri,oUnit (>= 1.0.2), sexplib
+  BuildDepends:       uri,oUnit (>= 1.0.2)

 Test test_runner
   Run$:               flag(tests)
diff --git a/lib/uri.ml b/lib/uri.ml
index 46fbb32..53f4dbf 100644
--- a/lib/uri.ml
+++ b/lib/uri.ml
@@ -16,9 +16,6 @@
  *
  *)

-open Sexplib.Std
-open Sexplib.Conv (* Workaround for bug in Sexplib when used without Core *)
-
 type component = [
   | `Scheme
   | `Authority
@@ -429,9 +426,6 @@ module Query = struct
     | KV of kv
     | Raw of string option * kv Lazy.t

-  let t_of_sexp sexp = KV (kv_of_sexp sexp)
-  let sexp_of_t = function Raw (_,lazy kv) | KV kv -> sexp_of_kv kv
-
   let compare x y = match x, y with
     | KV kvl, KV kvl'
     | Raw (_, lazy kvl), KV kvl'
@@ -510,13 +504,13 @@ let encoded_of_query ?scheme = Query.encoded_of_query ?scheme

 (* Type of the URI, with most bits being optional *)
 type t = {
-  scheme: Pct.decoded sexp_option;
-  userinfo: Userinfo.t sexp_option;
-  host: Pct.decoded sexp_option;
-  port: int sexp_option;
+  scheme: Pct.decoded option;
+  userinfo: Userinfo.t option;
+  host: Pct.decoded option;
+  port: int option;
   path: Path.t;
   query: Query.t;
-  fragment: Pct.decoded sexp_option;
+  fragment: Pct.decoded option;
 } [@@deriving sexp]

 let empty = {

[whitequark]

@hcarty In these cases I prefer the "kill it with fire" approach, i.e. sed -e s/with sexp// && diff -burN ... >patches/..., since anything depending on both uri and sexp won't be usable with such option turned off anyway, and so there's no harm in simply ripping it out instead.

[hcarty]

@whitequark Good point. Once/if I do this for opam-cross-windows I'll submit a PR there with the relevant pieces patched out.

[rgrinberg]

The patch seems a bit excessive. Isn't it enough to just skip the ppx preprocessing step? I.e. turn off ppx_deriving and ppx_sexp_conv?

[hcarty]

@rgrinberg Removing all references to the sexp modules avoids the need to port sexplib and js-build-tools. Neither should be that difficult but I don't personally have any use for them.

If someone else does the porting work then I'd be happy to simplify the patch accordingly.

[hannesm]

closing, uri now uses ppx. if i missed something in the thread which still needs to be done, please reopen or open a new issue.