Package: guix-patches;
Reported by: Julien Lepiller <julien <at> lepiller.eu>
Date: Thu, 21 Sep 2017 20:49:02 UTC
Severity: normal
Done: Julien Lepiller <julien <at> lepiller.eu>
Bug is archived. No further changes may be made.
View this message in rfc822 format
From: Eric Bavier <ericbavier <at> centurylink.net> To: Julien Lepiller <julien <at> lepiller.eu> Cc: 28546 <at> debbugs.gnu.org Subject: [bug#28546] Add some ocaml packages Date: Thu, 21 Sep 2017 23:09:28 -0500
Hi Julien,
Thanks for the patches. Just a few comments below:
On Thu, 21 Sep 2017 22:46:51 +0200
Julien Lepiller <julien <at> lepiller.eu> wrote:
> From d177cda39184fd1a1a879feb93f133eecd3bbcc4 Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <julien <at> lepiller.eu>
> Date: Thu, 21 Sep 2017 20:32:27 +0200
> Subject: [PATCH 01/10] gnu: Add ocaml-ezjsonm.
>
> * gnu/packages/ocaml.scm (ocaml-ezjsonm): New variable.
> ---
> gnu/packages/ocaml.scm | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/gnu/packages/ocaml.scm b/gnu/packages/ocaml.scm
> index 700c5698e..d765a085a 100644
> --- a/gnu/packages/ocaml.scm
> +++ b/gnu/packages/ocaml.scm
> @@ -3197,6 +3197,36 @@ writing to these structures, and they are accessed via the Bigarray module.")
> (description "Hex is a minimal library providing hexadecimal converters.")
> (license license:isc)))
>
> +(define-public ocaml-ezjsonm
> + (package
> + (name "ocaml-ezjsonm")
> + (version "0.4.3")
> + (source (origin
> + (method url-fetch)
> + (uri (string-append "https://github.com/mirage/ezjsonm/archive/"
> + version ".tar.gz"))
In a few of the later patches, you declared 'home-page' before 'source'
so that it could be used in the uri. That seams reasonable to me. Did
you want to do that in all these patches?
> + (home-page "https://github.com/mirage/ezjsonm/")
> + (synopsis "Easy interface on top of the Jsonm library")
How about some more widely meaningful, like: "Read and write JSON
data"? IDK, see section 6.7.5 in the manual.
> From 395199079f38b96299dbc02b14a945236ef7d6fe Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <julien <at> lepiller.eu>
> Date: Thu, 21 Sep 2017 20:33:33 +0200
> Subject: [PATCH 02/10] gnu: Add ocaml-uri.
>
> * gnu/packages/ocaml.scm (ocaml-uri): New variable.
> ---
> + (synopsis "RFC3986 URI/URL parsing library")
Similarly, I think we can leave out reference to the RFC in the
synopsis, since its in the description already. "URI/URL parsing
library" seems enough.
> From 8a8f1bedad93fe403df7d374515340f7f2462b2a Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <julien <at> lepiller.eu>
> Date: Thu, 21 Sep 2017 20:35:17 +0200
> Subject: [PATCH 04/10] gnu: Add ocaml-optcomp.
>
> * gnu/packages/ocaml.scm (ocaml-optcomp): New variable.
> ---
> +(define-public ocaml-optcomp
> + (package
> + (name "ocaml-optcomp")
Since this is a tool/application in its own right, rather than a
library, I think this package could be named "optcomp". This would be
in line with other packages like "starman", "scons", "behave",
"snakemake", etc.
> + (version "1.6")
> + (source (origin
> + (method url-fetch)
> + (uri (string-append "https://github.com/diml/optcomp/archive/"
> + version ".tar.gz"))
> + (sha256
> + (base32
> + "0hhhb2gisah1h22zlg5iszbgqxdd7x85cwd57bd4mfkx9l7dh8jh"))
> + (file-name (string-append name "-" version ".tar.gz"))))
> + (build-system ocaml-build-system)
> + (arguments
> + `(#:use-make? #t
> + #:make-flags
^
Remove the tab and align with spaces to '#:' on previous line.
> + (native-inputs `(("camlp4" ,camlp4)))
> + (propagated-inputs `(("camlp4" ,camlp4)))
> + (home-page "https://github.com/diml/optcomp")
> + (synopsis "Optional compilation with cpp-like directives")
Maybe: "Optional compilation for Ocaml"?
> From 3887a8aa6b8610f4ccfece291d7c8bc12a3966bf Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <julien <at> lepiller.eu>
> Date: Thu, 21 Sep 2017 20:44:57 +0200
> Subject: [PATCH 05/10] gnu: Add ocaml-piqilib.
>
> * gnu/packages/ocaml.scm (ocaml-piqilib): New variable.
> ---
> gnu/packages/ocaml.scm | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 53 insertions(+)
>
> diff --git a/gnu/packages/ocaml.scm b/gnu/packages/ocaml.scm
> index 910c892d6..86fe1a9e4 100644
> --- a/gnu/packages/ocaml.scm
> +++ b/gnu/packages/ocaml.scm
> @@ -3303,6 +3303,59 @@ Format module of the OCaml standard library.")
> cpp-like directives.")
> (license license:bsd-3)))
>
> +(define-public ocaml-piqilib
> + (package
> + (name "ocaml-piqilib")
> + (version "0.6.13")
> + (source (origin
> + (method url-fetch)
> + (uri (string-append "https://github.com/alavrik/piqi/archive/v"
> + version ".tar.gz"))
> + (sha256
> + (base32
> + "1whqr2bb3gds2zmrzqnv8vqka9928w4lx6mi6g244kmbwb2h8d8l"))
> + (file-name (string-append name "-" version ".tar.gz"))
> + (patches (search-patches "ocaml-piqilib-fix-makefile.patch"))))
^
This patch is missing.
> + (build-system ocaml-build-system)
> + (arguments
> + `(#:phases
> + (modify-phases %standard-phases
> + (replace 'configure
> + (lambda* (#:key outputs #:allow-other-keys)
> + (let ((out (assoc-ref outputs "out")))
> + (substitute* "make/OCamlMakefile"
> + (("/bin/sh") (which "bash")))
Does setting the "SHELL" environment variable work instead?
> + (zero? (system* "./configure" "--prefix" out "--ocaml-libdir"
> + (string-append out "/lib/ocaml/site-lib"))))))
Is passing '#:configure-flags' not enough?
> + (add-after 'build 'build-ocaml
> + (lambda* (#:key outputs #:allow-other-keys)
> + (zero? (system* "make" "ocaml"))))
> + (add-after 'install 'install-ocaml
> + (lambda* (#:key outputs #:allow-other-keys)
> + (zero? (system* "make" "ocaml-install"))))
> + (add-after 'install-ocaml 'link-stubs
> + (lambda* (#:key outputs #:allow-other-keys)
> + (let* ((out (assoc-ref outputs "out"))
> + (stubs (string-append out "/lib/ocaml/site-lib/stubslibs"))
> + (lib (string-append out "/lib/ocaml/site-lib/piqilib")))
> + (mkdir-p stubs)
> + (symlink (string-append lib "/dllpiqilib_stubs.so")
> + (string-append stubs "/dllpiqilib_stubs.so"))))))))
Is there some sort of configuration flag that can be used to install
these library into the right place?
Is this package to avoid having to build the entire piqi tool?
> From 4afbe64878f1b047ba936015e005f7d48a847afa Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <julien <at> lepiller.eu>
> Date: Thu, 21 Sep 2017 20:45:47 +0200
> Subject: [PATCH 06/10] gnu: Add ocaml-uuidm.
>
> * gnu/packages/ocaml.scm (ocaml-uuidm): New variable.
> ---
> +(define-public ocaml-uuidm
> + (package
...
> + (synopsis "Universally unique identifiers (UUIDs) for OCaml")
I think we'd rather leave the acronym out of the synopsis, and place it
in the description instead.
> + (description "Uuidm is an OCaml module implementing 128 bits universally
> +unique identifiers version 3, 5 (named based with MD5, SHA-1 hashing) and 4
> +(random based) according to RFC 4122.")
> From 3756da5e60dae4a109c18e552c6d07233ddfb5b9 Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <julien <at> lepiller.eu>
> Date: Thu, 21 Sep 2017 20:46:37 +0200
> Subject: [PATCH 07/10] gnu: Add ocamlgraph.
>
> * gnu/packages/ocaml.scm (ocamlgraph): New variable.
> ---
> +(define-public ocamlgraph
> + (package
> + (name "ocamlgraph")
I think we can name this packages "ocaml-graph". Similar to python
modules whose names begins with "python".
> + (version "1.8.7")
> + (source (origin
> + (method url-fetch)
> + (uri (string-append "http://ocamlgraph.lri.fr/download/"
> + "ocamlgraph-" version ".tar.gz"))
> + (sha256
> + (base32
> + "1845r537swjil2fcj7lgbibc2zybfwqqasrd2s7bncajs83cl1nz"))
> + (patches (search-patches "ocamlgraph-honor-source-date-epoch.patch"))))
^
This patch is missing.
> + (build-system ocaml-build-system)
> + (arguments
> + `(#:install-target "install-findlib"
> + #:phases
> + (modify-phases %standard-phases
> + (add-before 'configure 'fix-/bin/sh
> + (lambda* (#:key inputs #:allow-other-keys)
> + (substitute* "configure"
> + (("-/bin/sh") (string-append "-" (assoc-ref inputs "bash")
> + "/bin/sh"))))))))
> + (inputs `(("lablgtk" ,lablgtk)))
> + (home-page "http://ocamlgraph.lri.fr/")
> + (synopsis "A generic graph library for OCaml")
Synopses should not start with 'A'. 'guix lint' will warn about this.
> + (description "A generic graph library for OCaml.")
Rather: "OCamlgraph is a generic graph library for OCaml."
> From aa994f7c4ca830f6b3834fd100d7395fcb83dbf5 Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <julien <at> lepiller.eu>
> Date: Thu, 21 Sep 2017 20:47:40 +0200
> Subject: [PATCH 08/10] gnu: Add ocaml-piqi.
>
> * gnu/packages/ocaml.scm (ocaml-piqi): New variable.
> ---
> +(define-public ocaml-piqi
> + (package
> + (name "ocaml-piqi")
> + (version "0.7.5")
> + (source (origin
> + (method url-fetch)
> + (uri (string-append "https://github.com/alavrik/piqi-ocaml/"
> + "archive/v" version ".tar.gz"))
> + (sha256
> + (base32
> + "0ngz6y8i98i5v2ma8nk6mc83pdsmf2z0ks7m3xi6clfg3zqbddrv"))))
> + (build-system ocaml-build-system)
> + (arguments
> + `(#:make-flags
> + (list (string-append "DESTDIR=" (assoc-ref %outputs "out")))
Too bad this is so different from GNU's DESTDIR semantics...
> + #:phases
> + (modify-phases %standard-phases
> + (delete 'configure)
> + (add-before 'build 'patch-/bin/sh
> + (lambda _
> + (substitute* "make/OCamlMakefile"
> + (("/bin/sh") (which "sh")))
Again, I wonder if setting "SHELL" in the environment would suffice.
It would be less heavy-handed.
> From 2e4a7549a148e56d5d4fe7a869b56e1524dafcb6 Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <julien <at> lepiller.eu>
> Date: Thu, 21 Sep 2017 20:49:29 +0200
> Subject: [PATCH 09/10] gnu: Add ocaml-bap.
>
> * gnu/packages/ocaml.scm (ocaml-bap): New variable.
> ---
> gnu/packages/ocaml.scm | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 78 insertions(+)
>
> diff --git a/gnu/packages/ocaml.scm b/gnu/packages/ocaml.scm
> index 7f65ce859..2e7001c4c 100644
> --- a/gnu/packages/ocaml.scm
> +++ b/gnu/packages/ocaml.scm
> @@ -3449,6 +3449,84 @@ It provides a uniform interface for serializing OCaml data structures to JSON,
> XML and Protocol Buffers formats.")
> (license license:asl2.0)))
>
> +(define-public ocaml-bap
> + (package
> + (name "ocaml-bap")
This package should be simply named "bap".
> + (version "1.1.0")
> + (home-page "https://github.com/BinaryAnalysisPlatform/bap")
> + (source (origin
> + (method url-fetch)
> + (uri (string-append home-page "/archive/v" version ".tar.gz"))
> + (sha256
> + (base32
> + "1ms95m4j1qrmy7zqmsn2izh7gq68lnmssl7chyhk977kd3sxj66m"))
> + (file-name (string-append name "-" version ".tar.gz"))))
> + (build-system ocaml-build-system)
> + (native-inputs
> + `(("oasis" ,ocaml-oasis)
> + ("clang" ,clang)
> + ("ounit" ,ocaml-ounit)))
> + (propagated-inputs
> + `(("core-kernel" ,ocaml-core-kernel)
> + ("ppx-driver" ,ocaml-ppx-driver)
> + ("uri" ,ocaml-uri)
> + ("llvm" ,llvm)
> + ("gmp" ,gmp)
> + ("clang-runtime" ,clang-runtime)
> + ("fileutils" ,ocaml-fileutils)
> + ("cmdliner" ,ocaml-cmdliner)
> + ("zarith" ,ocaml-zarith)
> + ("uuidm" ,ocaml-uuidm)
> + ("camlzip" ,camlzip)
> + ("frontc" ,ocaml-frontc)
> + ("ezjsonm" ,ocaml-ezjsonm)
> + ("ocurl" ,ocaml-ocurl)
> + ("piqi" ,ocaml-piqi)
> + ("ocamlgraph" ,ocamlgraph)
> + ("bitstring" ,ocaml-bitstring)
> + ("ppx-jane" ,ocaml-ppx-jane)
> + ("re" ,ocaml-re)))
> + (inputs `(("llvm" ,llvm)))
> + (arguments
> + `(#:use-make? #t
> + #:phases
> + (modify-phases %standard-phases
> + (replace 'configure
> + (lambda* (#:key outputs #:allow-other-keys)
> + (zero? (system* "./configure" "--prefix"
> + (assoc-ref outputs "out")
> + "--libdir"
> + (string-append
> + (assoc-ref outputs "out")
> + "/lib/ocaml/site-lib")
> + "--with-llvm-version=3.8"
> + "--with-llvm-config=llvm-config"
> + "--enable-everything"))))
Could you put these flags in '#:configure-flags' instead?
> + (add-before 'install 'fix-ocamlpath
A short comment on why this phase is needed might be useful.
> + (lambda* (#:key outputs #:allow-other-keys)
> + (setenv "OCAMLPATH"
> + (string-append
> + (getenv "OCAMLPATH") ":"
> + (assoc-ref outputs "out")
> + "/lib/ocaml/site-lib"))
> + (setenv "PATH"
> + (string-append (getenv "PATH") ":"
> + (assoc-ref outputs "out") "/bin"))))
> From 553a30b6fd292a7b69a271e61e7a64a1ad9bcbf5 Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <julien <at> lepiller.eu>
> Date: Thu, 21 Sep 2017 20:51:14 +0200
> Subject: [PATCH 10/10] gnu: Add ocaml-camomile.
>
> * gnu/packages/ocaml.scm (ocaml-camomile): New variable.
> ---
> gnu/packages/ocaml.scm | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/gnu/packages/ocaml.scm b/gnu/packages/ocaml.scm
> index 2e7001c4c..31dde1952 100644
> --- a/gnu/packages/ocaml.scm
> +++ b/gnu/packages/ocaml.scm
> @@ -3527,6 +3527,36 @@ of libraries, plugins, and frontends. The libraries provide code reusability,
> the plugins facilitate extensibility, and the frontends serve as entry points.")
> (license license:expat)))
>
> +(define-public ocaml-camomile
> + (package
> + (name "ocaml-camomile")
> + (version "0.8.5")
> + (home-page "https://github.com/yoriyuki/Camomile")
> + (source (origin
> + (method url-fetch)
> + (uri (string-append home-page "/releases/download/rel-" version
> + "/camomile-" version ".tar.bz2"))
> + (sha256
> + (base32
> + "003ikpvpaliy5hblhckfmln34zqz0mk3y2m1fqvbjngh3h2np045"))))
> + (build-system ocaml-build-system)
> + (native-inputs `(("camlp4" ,camlp4)))
> + (arguments
> + `(#:phases
> + (modify-phases %standard-phases
> + (add-before 'configure 'fix-bin/sh
> + (lambda* (#:key #:allow-other-keys)
> + (substitute* "configure"
> + (("CONFIG_SHELL-/bin/sh")
> + (string-append "CONFIG_SHELL-" (which "bash")))))))))
Maybe our ocaml-build-system should be defining SHELL and CONFIG_SHELL
in the flags passed to configure, like gnu-build-system does. Or does
that not work for some ocaml projects?
Otherwise looks good to me.
Sorry for long reply and all the questions,
`~Eric
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.