Re: [PATCH] gnu: Add moreutils.

[Mark H Weaver]

address@hidden (Taylan Ulrich "Bayırlı/Kammer") writes:

> I still use setenv for XML_CATALOG_FILES; is there a more correct way?

This is handled automatically, so you can simply omit that part.

> From b38dd34fce834a54ccd0ae4628d8806418b41269 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Taylan=20Ulrich=20Bay=C4=B1rl=C4=B1/Kammer?=
>  <address@hidden>
> Date: Sun, 1 Feb 2015 02:21:11 +0100
> Subject: [PATCH] gnu: Add moreutils.
>
> * gnu-system.am (GNU_SYSTEM_MODULES): Add moreutils.scm.
> * gnu/packages/moreutils.scm: New file.

It's not important, but for brevity we usually write it like this:

* gnu/packages/moreutils.scm: New file.
* gnu-system.am (GNU_SYSTEM_MODULES): Add it.

> ---
>  gnu-system.am              |  1 +
>  gnu/packages/moreutils.scm | 71 
> ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 72 insertions(+)
>  create mode 100644 gnu/packages/moreutils.scm
>
>
> diff --git a/gnu-system.am b/gnu-system.am
> index 706ad57..8ae5b79 100644
> --- a/gnu-system.am
> +++ b/gnu-system.am
> @@ -187,6 +187,7 @@ GNU_SYSTEM_MODULES =                              \
>    gnu/packages/miscfiles.scm                 \
>    gnu/packages/mit-krb5.scm                  \
>    gnu/packages/moe.scm                               \
> +  gnu/packages/moreutils.scm                 \
>    gnu/packages/mpd.scm                               \
>    gnu/packages/mp3.scm                               \
>    gnu/packages/mpi.scm                               \
> diff --git a/gnu/packages/moreutils.scm b/gnu/packages/moreutils.scm
> new file mode 100644
> index 0000000..5a09b0f
> --- /dev/null
> +++ b/gnu/packages/moreutils.scm
> @@ -0,0 +1,71 @@
> +;;; GNU Guix --- Functional package management for GNU
> +;;; Copyright © 2015 Taylan Ulrich Bayırlı/Kammer <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 moreutils)
> +  #:use-module ((guix licenses) #:prefix l:)
> +  #:use-module (guix packages)
> +  #:use-module (guix download)
> +  #:use-module (guix build-system gnu)
> +  #:use-module (gnu packages perl)
> +  #:use-module (gnu packages xml)
> +  #:use-module (gnu packages docbook))
> +
> +(define-public moreutils
> +  (package
> +    (name "moreutils")
> +    (version "0.55")
> +    (source (origin
> +              (method url-fetch)
> +              (uri (string-append
> +                    
> "http://ftp.debian.org/debian/pool/main/m/moreutils/moreutils_";

Rather: "mirror://debian/pool/main/m/moreutils/moreutils_"

(there are other Debian URLs in our tree that could use this change)

> +                    version ".orig.tar.gz"))
> +              (sha256
> +               (base32
> +                "1dcah2jx8dbznn8966xl7sf1jrld2qfh6l6xcmx9dsnf8p8mr7fs"))))
> +    (build-system gnu-build-system)
> +    (inputs `(("perl" ,perl)
> +              ("libxml2" ,libxml2)
> +              ("libxslt" ,libxslt)
> +              ("docbook-xml" ,docbook-xml-4.4)
> +              ("docbook-xsl" ,docbook-xsl)))
> +    (arguments
> +     `(#:phases
> +       (alist-replace
> +        'configure
> +        (lambda* (#:key inputs outputs #:allow-other-keys)
> +          (let ((out (assoc-ref outputs "out"))
> +                (docbook-xml (assoc-ref inputs "docbook-xml"))
> +                (docbook-xsl (assoc-ref inputs "docbook-xsl")))
> +            (substitute* "Makefile"
> +              (("/usr/share/xml/.*/docbook.xsl")
> +               (string-append
> +                docbook-xsl
> +                "/xml/xsl/docbook-xsl-1.78.1/manpages/docbook.xsl")))
> +            (setenv
> +             "XML_CATALOG_FILES"
> +             (string-append docbook-xml "/xml/dtd/docbook/catalog.xml"))))

XML_CATALOG_FILES is set automatically by gnu-build-system, so you can
omit it.  This means you can also omit the 'docbook-xml' variable.

The 'out' variable is not used, so you can omit it.  Then the 'outputs'
argument will be unused, so omit it.  'docbook-xsl' is used in only one
place, so the expression to compute it could simply be put where it is
used, though it's a matter of taste and up to you :)

With these simplifications, it would look like this:

        (lambda* (#:key inputs #:allow-other-keys)
          (substitute* "Makefile"
            (("/usr/share/xml/.*/docbook.xsl")
             (string-append
              (assoc-ref inputs "docbook-xsl")
              "/xml/xsl/docbook-xsl-1.78.1/manpages/docbook.xsl"))))

> +        %standard-phases)
> +       #:make-flags
> +       (list (string-append "PREFIX=" (assoc-ref %outputs "out"))
> +             "CC=gcc")))
> +    (home-page "http://joeyh.name/code/moreutils/";)
> +    (synopsis "Miscellaneous general-purpose command-line tools")
> +    (description
> +     "Moreutils is a collection of general-purpose command-line tools to
> +augment the traditional Unix toolbox.")
> +    (license l:gpl2+)))

Otherwise looks good.  Can you send an updated patch?

     Thanks!
       Mark