| Message ID | 0c74eedb3a07933206ed2127a2b26f433ab77b18.1624528372.git.public@yoctocell.xyz |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Add ocaml-cohttp and dependencies | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
| cbaines/issue | success | View issue |
Thanks for the patches! There is already an `ocaml-uri`. How about updating that to 4.2.0 (currently at 4.1.0) and let `ocaml-uri-sexp` inherit from ocaml-uri with #:package set to "uri-sexp"? Another alternatively: remove the #:package argument from `ocaml-uri` so that all packages are built (`uri`, `uri-sexp` and `uri-re`). Xinglu Chen <public@yoctocell.xyz> writes: > * gnu/packages/ocaml.scm (ocaml-uri-sexp): New variable. > --- > gnu/packages/ocaml.scm | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/gnu/packages/ocaml.scm b/gnu/packages/ocaml.scm > index 1262c0e303..681d77dd81 100644 > --- a/gnu/packages/ocaml.scm > +++ b/gnu/packages/ocaml.scm > @@ -7071,6 +7071,34 @@ Graphics.open_graph is called. This > library used to be distributed with OCaml > up to OCaml 4.08.") > (license license:lgpl2.1+))) > > +(define-public ocaml-uri-sexp > + (package > + (name "ocaml-uri-sexp") > + (version "4.2.0") > + (source > + (origin > + (method git-fetch) > + (uri (git-reference > + (url "https://github.com/mirage/ocaml-uri") > + (commit (string-append "v" version)))) > + (file-name (git-file-name name version)) > + (sha256 > + (base32 > + > "1bgkc66cq00mgnkz3i535srwzwc4cpdsv0mly5dzvvq33451xwf0")))) > + (build-system dune-build-system) > + (arguments > + '(#:package "uri-sexp" > + #:test-target ".")) > + (propagated-inputs > + `(("ocaml-uri" ,ocaml-uri) > + ("ocaml-ppx-sexp-conv" ,ocaml-ppx-sexp-conv) > + ("ocaml-sexplib0" ,ocaml-sexplib0))) > + (native-inputs `(("ocaml-ounit" ,ocaml-ounit))) > + (home-page "https://github.com/mirage/ocaml-uri") > + (synopsis "RFC3986 URI/URL parsing library") > + (description "This package adds S-exp support to > @code{ocaml-uri}.") > + (license license:isc))) > + > (define-public js-of-ocaml > (package > (name "js-of-ocaml")
On Thu, Jun 24 2021, Adu O'Hara wrote: > Thanks for the patches! > > There is already an `ocaml-uri`. How about updating that to 4.2.0 > (currently at 4.1.0) and let `ocaml-uri-sexp` inherit from > ocaml-uri with #:package set to "uri-sexp"? That could probably be done in a separate patch series; this series is about adding ‘ocaml-cohttp’. > Another alternatively: remove the #:package argument from > `ocaml-uri` so that all packages are built (`uri`, `uri-sexp` and > `uri-re`). Those are three separate packages on Opam, so I don’t think it makes sense for us to merge them into one. Not everyone who installs ‘ocaml-uri’ is going to want ‘ocaml-uri-sexp’ and ‘ocaml-uri-re’. WDYT?
Xinglu Chen <public@yoctocell.xyz> writes: > On Thu, Jun 24 2021, Adu O'Hara wrote: > >> Thanks for the patches! >> >> There is already an `ocaml-uri`. How about updating that to >> 4.2.0 >> (currently at 4.1.0) and let `ocaml-uri-sexp` inherit from >> ocaml-uri with #:package set to "uri-sexp"? > > That could probably be done in a separate patch series; this > series is > about adding ‘ocaml-cohttp’. True, what I was suggesting would add another patch to the series and mix up intentions of the series. Still, I'd suggest for ocaml-uri-sexp to inherit from ocaml-uri. Otherwise we have them at two different versions. wdyt? > >> Another alternatively: remove the #:package argument from >> `ocaml-uri` so that all packages are built (`uri`, `uri-sexp` >> and >> `uri-re`). > > Those are three separate packages on Opam, so I don’t think it > makes > sense for us to merge them into one. Not everyone who installs > ‘ocaml-uri’ is going to want ‘ocaml-uri-sexp’ and > ‘ocaml-uri-re’. > > WDYT? Yeah, that make sense. Keeping them separate sounds good.
On Thu, Jun 24 2021, Adu O'Hara wrote: > Xinglu Chen <public@yoctocell.xyz> writes: > >> On Thu, Jun 24 2021, Adu O'Hara wrote: >> >>> Thanks for the patches! >>> >>> There is already an `ocaml-uri`. How about updating that to >>> 4.2.0 >>> (currently at 4.1.0) and let `ocaml-uri-sexp` inherit from >>> ocaml-uri with #:package set to "uri-sexp"? >> >> That could probably be done in a separate patch series; this >> series is >> about adding ‘ocaml-cohttp’. > > True, what I was suggesting would add another patch to the series > and mix up intentions of the series. Sorry, I don’t get what you are trying to say here. > Still, I'd suggest for ocaml-uri-sexp to inherit from > ocaml-uri. Otherwise we have them at two different versions. I think it makes more sense to do that in a separate series since updating ‘ocaml-uri’ isn’t really related to adding ‘ocaml-cohttp’.
Xinglu Chen <public@yoctocell.xyz> writes: >> True, what I was suggesting would add another patch to the >> series >> and mix up intentions of the series. > > Sorry, I don’t get what you are trying to say here. > >> Still, I'd suggest for ocaml-uri-sexp to inherit from >> ocaml-uri. Otherwise we have them at two different versions. > > I think it makes more sense to do that in a separate series > since > updating ‘ocaml-uri’ isn’t really related to adding > ‘ocaml-cohttp’. Sorry for being unclear. Here's what I mean: (define-public ocaml-uri-sexp (package (inherit ocaml-uri) (name "ocaml-uri-sexp") (arguments `(#:package "uri-sexp" #:test-target ".")) (propagated-inputs `(("ocaml-uri" ,ocaml-uri) ("ocaml-ppx-sexp-conv" ,ocaml-ppx-sexp-conv) ("ocaml-sexplib0" ,ocaml-sexplib0))) (native-inputs `(("ocaml-ounit" ,ocaml-ounit))))) Then when `ocaml-uri` eventually gets updated `ocaml-uri` and `ocaml-uri-sexp` will be at same version.
On Fri, Jun 25 2021, Adu O'Hara wrote: > Xinglu Chen <public@yoctocell.xyz> writes: > >>> True, what I was suggesting would add another patch to the >>> series >>> and mix up intentions of the series. >> >> Sorry, I don’t get what you are trying to say here. >> >>> Still, I'd suggest for ocaml-uri-sexp to inherit from >>> ocaml-uri. Otherwise we have them at two different versions. >> >> I think it makes more sense to do that in a separate series >> since >> updating ‘ocaml-uri’ isn’t really related to adding >> ‘ocaml-cohttp’. > > Sorry for being unclear. Here's what I mean: > > (define-public ocaml-uri-sexp > (package > (inherit ocaml-uri) > (name "ocaml-uri-sexp") > (arguments > `(#:package "uri-sexp" > #:test-target ".")) > (propagated-inputs > `(("ocaml-uri" ,ocaml-uri) > ("ocaml-ppx-sexp-conv" ,ocaml-ppx-sexp-conv) > ("ocaml-sexplib0" ,ocaml-sexplib0))) > (native-inputs `(("ocaml-ounit" ,ocaml-ounit))))) > > Then when `ocaml-uri` eventually gets updated `ocaml-uri` and > `ocaml-uri-sexp` will be at same version. But ‘ocaml-uri’ would first have to be updated to the latest version, which I think is out of scope of this series. Once this series gets merged, I think it would be great to update ‘ocaml-uri’, and then make ‘ocaml-uri-sexp’ inherit from it. WDYT?
Hi, what matters is that patches focus on one thing, and one one. I agree with Adu here, we should make sure uri anl uri-sexp are the same version. If you really don't want to do that in this series, send it as another series, and make it blocking for this one. I'll try to merge your work this week-end. Le 25 juin 2021 06:58:06 GMT-04:00, Xinglu Chen <public@yoctocell.xyz> a écrit : >On Fri, Jun 25 2021, Adu O'Hara wrote: > >> Xinglu Chen <public@yoctocell.xyz> writes: >> >>>> True, what I was suggesting would add another patch to the >>>> series >>>> and mix up intentions of the series. >>> >>> Sorry, I don’t get what you are trying to say here. >>> >>>> Still, I'd suggest for ocaml-uri-sexp to inherit from >>>> ocaml-uri. Otherwise we have them at two different versions. >>> >>> I think it makes more sense to do that in a separate series >>> since >>> updating ‘ocaml-uri’ isn’t really related to adding >>> ‘ocaml-cohttp’. >> >> Sorry for being unclear. Here's what I mean: >> >> (define-public ocaml-uri-sexp >> (package >> (inherit ocaml-uri) >> (name "ocaml-uri-sexp") >> (arguments >> `(#:package "uri-sexp" >> #:test-target ".")) >> (propagated-inputs >> `(("ocaml-uri" ,ocaml-uri) >> ("ocaml-ppx-sexp-conv" ,ocaml-ppx-sexp-conv) >> ("ocaml-sexplib0" ,ocaml-sexplib0))) >> (native-inputs `(("ocaml-ounit" ,ocaml-ounit))))) >> >> Then when `ocaml-uri` eventually gets updated `ocaml-uri` and >> `ocaml-uri-sexp` will be at same version. > >But ‘ocaml-uri’ would first have to be updated to the latest version, >which I think is out of scope of this series. > >Once this series gets merged, I think it would be great to update >‘ocaml-uri’, and then make ‘ocaml-uri-sexp’ inherit from it. > >WDYT?
On Fri, Jun 25 2021, Julien Lepiller wrote: > Hi, what matters is that patches focus on one thing, and one one. I > agree with Adu here, we should make sure uri anl uri-sexp are the same > version. If you really don't want to do that in this series, send it > as another series, and make it blocking for this one. I'll try to > merge your work this week-end. Cool, I will send another series that updates ‘ocaml-uri’.
diff --git a/gnu/packages/ocaml.scm b/gnu/packages/ocaml.scm index 1262c0e303..681d77dd81 100644 --- a/gnu/packages/ocaml.scm +++ b/gnu/packages/ocaml.scm @@ -7071,6 +7071,34 @@ Graphics.open_graph is called. This library used to be distributed with OCaml up to OCaml 4.08.") (license license:lgpl2.1+))) +(define-public ocaml-uri-sexp + (package + (name "ocaml-uri-sexp") + (version "4.2.0") + (source + (origin + (method git-fetch) + (uri (git-reference + (url "https://github.com/mirage/ocaml-uri") + (commit (string-append "v" version)))) + (file-name (git-file-name name version)) + (sha256 + (base32 + "1bgkc66cq00mgnkz3i535srwzwc4cpdsv0mly5dzvvq33451xwf0")))) + (build-system dune-build-system) + (arguments + '(#:package "uri-sexp" + #:test-target ".")) + (propagated-inputs + `(("ocaml-uri" ,ocaml-uri) + ("ocaml-ppx-sexp-conv" ,ocaml-ppx-sexp-conv) + ("ocaml-sexplib0" ,ocaml-sexplib0))) + (native-inputs `(("ocaml-ounit" ,ocaml-ounit))) + (home-page "https://github.com/mirage/ocaml-uri") + (synopsis "RFC3986 URI/URL parsing library") + (description "This package adds S-exp support to @code{ocaml-uri}.") + (license license:isc))) + (define-public js-of-ocaml (package (name "js-of-ocaml")