Add opam file for uri-sexp

[Julow]

https://github.com/mirage/ocaml-uri/pull/121 was merged a long time ago but opam file were untouched and sexp dependencies were still here ~and Uri_sexp inaccessible~. ~This means that it was never used, so maybe we could remove Uri_sexp ?~

[avsm]

Sorry, I don't understand the bnug report. What problem are you trying to solve exactly? Uri_sexp is accessible via:

# #require "uri.sexp";;
# Uri_sexp.compare;;
- : Uri_sexp.t -> Uri_sexp.t -> int = <fun>

I deliberately didn't make it two opam packages in the first instance for simplicity, so this PR would mean another big round of compat changes (e.g. in cohttp). I'm not keen to do that work without either some benefit or some usecase being fixed.

[Julow]

Yes, sorry, you are right. The goal of https://github.com/mirage/ocaml-uri/pull/121 was to remove dependencies which is useless if they are still in the opam file.

[avsm]

There are two dependencies: the build time dependencies for the ocamlfind package and the package install time ones. The PR removed the build-time deps so that a unikernel could be linked without sexp when using just uri. That's not useless...

It would be helpful to understand what problem you're trying to solve with this PR. Are you trying to minimise the number of opam packages required to build a unikernel?

[Julow]

In my use case I'm using uri alone and ppx_sexp_conv bring a lot of dependencies. I misjudged the situation before and now I understand that an entire new package for this is overkill and harder to maintain.

[dinosaure]

/cc @avsm

• [x] add a META.template as a compat layer that requires: uri-sexp so that revdeps dont break
• [x] onstrain the tests in uri to only run on uri-sexp with a (package) entry. You can test this with dune-release distrib
• [ ] a CHANGES entry to explain how to migrate to the new (just replace uri.sexp -> uri-sexp)

[dinosaure]

Ok we need to clarify a situation about cyclic dependencies over this PR:

The idea is to split uri under two packages:

• uri
• uri-sexp

However, due to compatibility, we want to expose an uri.sexp sub-package (META) which will be a part of the uri package (OPAM).

At the end, the goal is to remove the ppx_sexp_conv dependency on uri (and let it in uri-sexp) to unlock several packages which depend by transitivity to ppx_sexp_conv where they never use it.

In details, META file describes a new sub-package sexp which depends on uri-sexp of course. So, if we continue on this way, uri should depend on uri-sexp (to be able to provide properly uri.sexp sub-package). Currently, this is the error reported by Travis where cohttp wants to link with uri.sexp but did not find uri-sexp.

To fix Travis, we should had in uri.opam the dependency uri-sexp and then, because uri requires OPAM package uri-sexp (to properly provide META sub-package uri.sexp), cohttp (in my example) will be able to be compiled (without any changes) - REVDEP will success.

However, this is not what we want and currently, this PR provide a strange (IMHO) pattern where it wants to provide a sub-package which requires another OPAM package (uri-sexp) - this one requires the first OPAM package (uri). So it's why I talk about cyclic dependencies due to uri.sexp.

So, for me we can merge this PR where the fix about Travis should not be what we want but we let, as I said, a strange situation where we will need explicitly update packages to put uri-sexp as new dependency. But if we go to this situation, IMHO, it's just preferable to remove uri.sexp and provide uri-sexp directly instead to have a unstable transition step.

I know the point to provide for a limited time uri.sexp for a new release as we discussed about that in #136.

[avsm]

It might be simpler to give plenty of notice to people, post on discuss.ocaml.org, and not have the uri.sexp compatability package in the major release

[hannesm]

my experience with subpackages that require other opam packages is really bad - while it avoids to put constraints right now, it bites you back in the future (see some cstruct revdeps, I lost track of issues and fixes). I personally would avoid them, and make a breaking release - either you use uri < 3.0.0 and have a uri.sexp, or you use uri > 3.0.0 and uri-sexp (which should then conflict with uri < 3.0.0).

since I'm not familiar with the usage of uri at all, are there many uri-sexp users?

[dinosaure]

Ok, so I force-pushed a commit which splits uri and uri-sexp. In this commit, we did not provide a uri.sexp sub-package. Distribution still works (with tests). REVDEP will fails of course and, as I said, we will need to fix by ourselves packages like cohttp to put uri-sexp as a new dependency.

[avsm]

I'll resolve this one after doing a minor release for the Re deprecation in #137, so that we have a release with just this change in it

[hannesm]

thx @dinosaure, I added two suggestions. CI failures are due to revdeps (as expected).

[hannesm]

to me it seems, Uri_sexp is referenced 5 times in the tests, and (in order to avoid a cp ../lib/uri_sexp.ml .) it'd be possible to refactor the tests to compare t directly instead of the s-expression representation. WDYT?

[dinosaure]

This is what I think at the end ...

[dinosaure]

Done!

[dinosaure]

PR cleaned, if @mirage/core is happy with it, merge and release.

[avsm]

Posted a PSA up at https://discuss.ocaml.org/t/psa-uri-and-ipaddr-libraries-will-both-have-optional-sexp-opam-packages/4067