Package: guix-patches;
Reported by: aecepoglu <aecepoglu <at> fastmail.fm>
Date: Sun, 10 Jan 2021 12:38:03 UTC
Severity: normal
Tags: patch
Done: Julien Lepiller <julien <at> lepiller.eu>
Bug is archived. No further changes may be made.
View this message in rfc822 format
From: Julien Lepiller <julien <at> lepiller.eu> To: aecepoglu <aecepoglu <at> fastmail.fm> Cc: 45762 <at> debbugs.gnu.org Subject: [bug#45762] [PATCH] ocaml: Add ocaml-down and ocaml-ocp-index Date: Mon, 11 Jan 2021 16:15:14 +0100
Hi,
thanks for the patch!
Since it adds three packages, could you make this a series of three
patches, where each adds just one package? First, ocp-indent, then
ocp-index and finally down. Below are remarks that you should address in
these patches too :)
For each of them, the title of the patch should be "gnu: Add
<package-name>.", followed by a blank line and "*
gnu/packages/ocaml.scm (<package-name>): New variable.".
Le Sun, 10 Jan 2021 13:49:15 +0300,
aecepoglu <aecepoglu <at> fastmail.fm> a écrit :
> ---
> gnu/packages/ocaml.scm | 132
> +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 132
> insertions(+)
>
> diff --git a/gnu/packages/ocaml.scm b/gnu/packages/ocaml.scm
> index 2df16e7cd2..6699eb1050 100644
> --- a/gnu/packages/ocaml.scm
> +++ b/gnu/packages/ocaml.scm
> @@ -390,6 +390,71 @@ repository-wide uninstallability checks.")
> ;; with static-linking exception
> (license license:lgpl2.1+)))
In the first patch of the series, could you add a copyright line for
yourself? It's fine to use a pseudonym if you don't want to use your
legal name.
>
> +(define-public ocaml-down
> + (package
> + (name "ocaml-down")
> + (version "0.0.3")
> + (source
> + (origin
> + (method url-fetch)
> + (uri
> "https://erratique.ch/software/down/releases/down-0.0.3.tbz")
> + (sha256
> + (base32
> +
> "1nz2f5j17frgr2vrslcz9klmi6w9sm2vqwwwpi33ngcm3rgmsrlg"))))
> + (build-system ocaml-build-system)
> + (arguments
> + `(#:tests? #f
Please add a comment explaining why you skip tests (could be "no tests"
if there is no tests) :)
> + #:phases (modify-phases %standard-phases
> + (delete 'configure))
> + #:build-flags (list "build"
> + "--lib-dir" (string-append (assoc-ref
> %outputs "out")
> +
> "/lib/ocaml/site-lib"))))
I think the indentation is off here, but I'll fix it, don't worry.
> + (native-inputs
> + `(("ocaml-findlib" ,ocaml-findlib)
> + ("ocamlbuild" ,ocamlbuild)
> + ("ocaml-topkg" ,ocaml-topkg)
> + ("opam" ,opam)))
Same here, there's one space too much below native-inputs.
> + (home-page "https://erratique.ch/software/down")
> + (synopsis "An OCaml toplevel (REPL) upgrade")
Synpsis and description should not start with an article, so make
it "OCaml toplevel (REPL) upgrade" :)
> + (description
> + "
> +Down is an unintrusive user experience upgrade for the `ocaml`
> +toplevel (REPL).
> +
> +Simply load the zero dependency `Down` library in the `ocaml`
> toplevel +and you get line edition, history, session support and
> identifier +completion and documentation (courtesy of
> [`ocp-index`][ocp-index]). +
> +Add this to your `~/.ocamlinit`:
> +
> + #use \"down.top\"
> +
> +You may also need to add this to your `~/.ocamlinit` and declare
> +the environment variable OCAML_TOPLEVEL_PATH
> +
> + let () =
> + try Topdirs.dir_directory (Sys.getenv \"OCAML_TOPLEVEL_PATH\")
> + with Not_found -> ()
> +
> +OR
> +
> + let () = String.split_on_char ':' (Sys.getenv \"OCAMLPATH\")
> + |> List.filter (fun x -> Filename.check_suffix x
> \"/site-lib\")
> + |> List.map (fun x -> x ^ \"/toplevel\")
> + (* remove the line below if you don't want to see the
> text
> + every time you start the toplevel *)
> + |> List.map (fun x -> Printf.printf \"adding directory
> %s\\n\" x; x)
> + |> List.iter Topdirs.dir_directory;;
> +
> +
> +
> +
> +Down is distributed under the ISC license.
> +
> +
> +[ocp-index]: https://github.com/OCamlPro/ocp-index")
> + (license license:isc)))
> +
I supposed you used the description from the opam importer? It doesn't
use the same syntax as guix, so you have to translate them usually.
Don't worry too much though if you can't make it, I'll take care of it
when applying your patch :)
Basically, don't start with a new line, links are not supported, and
generally use texinfo syntax.
> (define-public ocaml-opam-file-format
> (package
> (name "ocaml-opam-file-format")
> @@ -2363,6 +2428,73 @@ the JSON data format. It can process JSON
> text without blocking on IO and without a complete in-memory
> representation of the data.") (license license:isc)))
>
> +(define-public ocaml-ocp-indent
> + (package
> + (name "ocaml-ocp-indent")
> + (version "1.8.1")
> + (source
> + (origin
> + (method url-fetch)
> + (uri
> "https://github.com/OCamlPro/ocp-indent/archive/1.8.1.tar.gz")
> + (sha256
> + (base32
> +
> "0rqcvwzpwwzwqvrzg1l37jjiglrk2ijgjg1c7vy3xvc8lk9zyysw"))))
> + (build-system dune-build-system)
> + (arguments
> + `(#:tests? #f
Same question, why disable tests?
> + #:build-flags (list "--profile=release")))
Unless there's a specific reason, I think #:package the usual way to do
it.
> + (propagated-inputs
> + `(("ocaml-cmdliner" ,ocaml-cmdliner)))
> + (home-page
> + "http://www.typerex.org/ocp-indent.html")
> + (synopsis
> + "A simple tool to indent OCaml programs")
Remove the article, so "Simple tool to indent OCaml programs"
> + (description
> + "Ocp-indent is based on an approximate, tolerant OCaml parser
> and a simple stack +machine ; this is much faster and more reliable
> than using regexps. Presets and +configuration options available,
> with the possibility to set them project-wide. +Supports most common
> syntax extensions, and extensible for others. +
> +Includes:
> +- An indentor program, callable from the command-line or from within
> editors +- Bindings for popular editors
> +- A library that can be directly used by editor writers, or just
> for\n fault-tolerant/approximate parsing. +")
Sorry, my client is making a mess out of this. I'll make sure it looks
alright, don't worry about it.
> + (license #f)))
Can you check the license and fill in the field please? #f is not
acceptable in Guix :)
> +
> +(define-public ocaml-ocp-index
> + (package
> + (name "ocaml-ocp-index")
> + (version "1.2.1")
> + (source
> + (origin
> + (method url-fetch)
> + (uri
> "https://github.com/OCamlPro/ocp-index/archive/1.2.1.tar.gz")
> + (sha256
> + (base32
> +
> "169msb3k1ip5lp79lr62nsk715rwrn53x9jcrnxz4js7c0fv0cyr"))))
> + (build-system dune-build-system)
> + (arguments
> + `(#:package "ocp-index"))
> + (propagated-inputs
> + `(("ocaml-ocp-indent" ,ocaml-ocp-indent)
> + ("ocaml-re" ,ocaml-re)
> + ("ocaml-cmdliner" ,ocaml-cmdliner)))
> + (native-inputs `(("ocaml-cppo" ,ocaml-cppo)))
> + (home-page
> + "http://www.typerex.org/ocp-index.html")
> + (synopsis
> + "Lightweight completion and documentation browsing for OCaml
> libraries")
> + (description
> + "This package includes
> +* The `ocp-index` library and command-line tool
> +* `ocp-grep`, a tool that finds uses of a given (qualified)
> identifier in a source tree +* bindings for emacs and vim (sublime
> text also
> [available](https://github.com/whitequark/sublime-ocp-index/)) + +To
> automatically configure your editors, install this with package
> `user-setup`. +")
My client is again making a mess out of this, but I'll fix this anyway,
don't worry.
> + (license #f)))
Same, please specify the license.
> +
> (define-public ocaml-ocurl
> (package
> (name "ocaml-ocurl")
Overall great contribution! Thank you again!
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.