Subject: Re: bug#61774: [PATCH] gnu: Add luarocks.

From: Ludovic Courtès <[email-masked]>
Date: Thu, 30 Mar 2023 23:14:49 +0200

Hi,

Timo Wilken <[email-masked]> skribis:

> Luarocks is a package manager for Lua modules.
>
> It is used by the Prosody XMPP server (already packaged in Guix) to
> install extensions.
>
> * gnu/packages/lua.scm (luarocks): Add variable.

Could resend the two patches merged as one?

Some comments:

> +                      (substitute* '("GNUmakefile" "src/luarocks/fs/unix.lua"
> +                                     "src/luarocks/core/sysdetect.lua")
> +                        (("/bin/sh")
> +                         (string-append (assoc-ref inputs "bash-minimal")
> +                                        "/bin/sh")))))

Rather (search-input-file inputs "/bin/sh").

> +    (synopsis "A package manager for Lua modules")

I believe ‘guix lint’ will tel you to remove “A”.  :-)

> +    ;; The home page says:
> +    ;; "LuaRocks is free software and uses the same license as Lua."
> +    (license license:x11)))

Please double-check the license in the source (and remove the comment).

> +         (add-after 'install 'patch-unzip
> +           (lambda* (#:key inputs outputs #:allow-other-keys)
> +             (substitute*
> +                 (string-append
> +                  (assoc-ref outputs "out") "/etc/luarocks/config-"
> +                  ,(substring (package-version lua) 0 3) ".lua") ;e.g. "5.2"
> +               (("variables = \\{")
> +                (string-append
> +                 "variables = {\n"
> +                 "   AR = \"" (assoc-ref inputs "binutils") "/bin/ar\";\n"
> +                 "   BUNZIP2 = \"" (assoc-ref inputs "bzip2") "/bin/bunzip2\";\n"
> +                 "   CC = \"" (assoc-ref inputs "gcc") "/bin/gcc\";\n"
> +                 "   CHMOD = \"" (assoc-ref inputs "coreutils") "/bin/chmod\";\n"
> +                 "   CMAKE = \"" (assoc-ref inputs "coreutils") "/bin/cmake\";\n"

These should all use ‘search-input-file’.  However…

> +                 "   CP = \"" (assoc-ref inputs "coreutils") "/bin/cp\";\n"
> +                 "   CURL = \"" (assoc-ref inputs "curl") "/bin/curl\";\n"
> +                 "   CVS = \"" (assoc-ref inputs "cvs") "/bin/cvs\";\n"
> +                 "   FIND = \"" (assoc-ref inputs "findutils") "/bin/find\";\n"
> +                 "   GIT = \"" (assoc-ref inputs "git") "/bin/git\";\n"
> +                 "   GPG = \"" (assoc-ref inputs "gnupg") "/bin/gpg\";\n"

[...]

> +                 "   RSYNC = \"" (assoc-ref inputs "rsync") "/bin/rsync\";\n"
> +                 "   SCP = \"" (assoc-ref inputs "openssh") "/bin/scp\";\n"
> +                 "   SEVENZ = \"" (assoc-ref inputs "p7zip") "/bin/7z\";\n"
> +                 "   SVN = \"" (assoc-ref inputs "subversion") "/bin/svn\";\n"
> +                 "   TAR = \"" (assoc-ref inputs "tar") "/bin/tar\";\n"

Does it really need all these things?  What does ‘guix size luarocks’
say now?

Maybe it’s OK to assume that some of the rarely-used dependencies (say,
CVS, SVN, 7zip) will be picked from $PATH and that it will fail
otherwise?  That would help keep the closure size under control.

> +    (inputs (list lua bash-minimal
> +                  ;; Executables required by luarocks.
> +                  binutils bzip2 coreutils curl cvs findutils gcc git gnupg
> +                  gzip gnu-make mercurial openssh openssl p7zip rsync
> +                  subversion tar unzip wget zip))

Please one per line since there are many of them (you can run ‘guix
style luarocks’).

Could you send an updated patch?

Thanks!

Ludo’.