[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] gnu: Add lua-lpeg
From: |
Marius Bakke |
Subject: |
Re: [PATCH] gnu: Add lua-lpeg |
Date: |
Tue, 06 Dec 2016 15:13:51 +0100 |
User-agent: |
Notmuch/0.23.3 (https://notmuchmail.org) Emacs/25.1.1 (x86_64-unknown-linux-gnu) |
José Miguel Sánchez García <address@hidden> writes:
> Add lua-lpeg (I hope it's finally correct!)
Cool! Nice to see some Lua love :)
> --
> José Miguel Sánchez García
> From 4ebd6648b2dcf1e02d39532173781b91850e4f41 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Jos=C3=A9=20Miguel=20S=C3=A1nchez=20Garc=C3=ADa?=
> <address@hidden>
> Date: Mon, 5 Dec 2016 15:15:33 +0100
> Subject: [PATCH] gnu: Add lua-lpeg
>
> ---
> gnu/packages/lua-lpeg.scm | 52
> +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 52 insertions(+)
> create mode 100644 gnu/packages/lua-lpeg.scm
I think this can go in 'lua.scm', similar to what we do for perl, ruby,
python etc libraries. It's not a lot there currently, so this will help
tremendously :-)
> diff --git a/gnu/packages/lua-lpeg.scm b/gnu/packages/lua-lpeg.scm
> new file mode 100644
> index 0000000..50042bd
> --- /dev/null
> +++ b/gnu/packages/lua-lpeg.scm
> @@ -0,0 +1,51 @@
> +;;; GNU Guix --- Functional package management for GNU
> +;;; Copyright © 2016 José Miguel Sánchez García <address@hidden>
> +;;;
> +;;; This file is part of GNU Guix.
> +;;;
> +;;; GNU Guix is free software; you can redistribute it and/or modify it
> +;;; under the terms of the GNU General Public License as published by
> +;;; the Free Software Foundation; either version 3 of the License, or (at
> +;;; your option) any later version.
> +;;;
> +;;; GNU Guix is distributed in the hope that it will be useful, but
> +;;; WITHOUT ANY WARRANTY; without even the implied warranty of
> +;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +;;; GNU General Public License for more details.
> +;;;
> +;;; You should have received a copy of the GNU General Public License
> +;;; along with GNU Guix. If not, see <http://www.gnu.org/licenses/>.
> +
> +(define-module (gnu packages lua-lpeg)
> + #:use-module (guix packages)
> + #:use-module (guix download)
> + #:use-module (guix build-system gnu)
> + #:use-module (guix licenses)
> + #:use-module (gnu packages lua))
> +
> +(define-public lua-lpeg
> + (package
> + (name "lua-lpeg")
> + (version "1.0.0")
> + (source (origin
> + (method url-fetch)
> + (uri (string-append
> "http://www.inf.puc-rio.br/~roberto/lpeg/lpeg-"
> + version ".tar.gz"))
> + (sha256
> + (base32
> "13mz18s359wlkwm9d9iqlyyrrwjc6iqfpa99ai0icam2b3khl68h"))))
> + (build-system gnu-build-system)
> + (arguments '(
The cosmetic improvement from the previous email applies here too.
> + #:make-flags '("LUADIR=/usr/include")
What is LUADIR in this context? Note that Guix does not use a '/usr'
partition, so we typically refer directly to the 'include' dir of each
package directly. This usually works "out of the box" however. Since
there is no '/usr/include' in the build chroot, it can probably be
removed.
> + #:phases (modify-phases %standard-phases
> + (delete 'configure)
> + (replace 'install (lambda _
> + (let* ((out (assoc-ref %outputs "out")))
Minor nit-pick: Using 'let' here (without the star) is sufficient. The
star is only needed if any of the declared variables uses other
variables declared by the same 'let' statement.
> + (install-file "lpeg.so" (string-append out "/usr/lib/lua/5.3"))
> + (install-file "re.lua" (string-append out
> "/usr/share/lua/5.3"))))))
This looks correct, but '/usr' should be removed. Then they will end up
in the correct profile directories so things Just Work. Could you add a
comment stating why using the normal 'install' phase does not work?
> + #:tests? #f))
Please also add a comment stating why tests are disabled (if there are
none, or if they need some packages we don't yet have in Guix, etc).
> + (inputs `(("lua", lua)))
> + (synopsis "Pattern-matching library for Lua")
> + (description
> + "LPeg is a pattern-matching library for Lua, based on Parsing
> Expression
> +Grammars (PEGs).")
> + (home-page "http://www.inf.puc-rio.br/~roberto/lpeg")
> + (license expat)))
Sorry for the nit-picking! With the mentioned improvements I think this
is good to go. Can you send an updated patch? :-)
signature.asc
Description: PGP signature