Re: [PATCH] gnu: Add mcelog.

[Marius Bakke]

Tobias Geerinckx-Rice <address@hidden> writes:

> * gnu/packages/linux.scm (mcelog): New variable.

Thanks!

> ---
>  gnu/packages/linux.scm | 41 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>
> diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
> index 3ec6514..fc4faa4 100644
> --- a/gnu/packages/linux.scm
> +++ b/gnu/packages/linux.scm
> @@ -74,6 +74,7 @@
>    #:use-module (guix build-system python)
>    #:use-module (guix build-system trivial)
>    #:use-module (guix download)
> +  #:use-module (guix git-download)
>    #:use-module ((guix licenses) #:prefix license:)
>    #:use-module (guix packages)
>    #:use-module (guix utils)
> @@ -2939,3 +2940,43 @@ the default @code{nsswitch} and the experimental 
> @code{umich_ldap}.")
>       "Tools for loading and managing Linux kernel modules, such as 
> `modprobe',
>  `insmod', `lsmod', and more.")
>      (license license:gpl2+)))
> +
> +(define-public mcelog
> +  (package
> +    (name "mcelog")
> +    (version "141")
> +    (source
> +     (let ((commit (string-append "v" version)))
> +       (origin
> +         (method git-fetch)
> +         (uri (git-reference
> +               (url 
> "https://git.kernel.org/pub/scm/utils/cpu/mce/mcelog.git";)
> +               (commit commit)))

It's not visible in the cgit interface, but it actually seems to support
normal snapshot downloads:
https://git.kernel.org/cgit/utils/cpu/mce/mcelog.git/snapshot/v141.tar.gz

> +         (sha256
> +          (base32
> +           "0iqvqiwf3aawmgjcyg2rj427m8nvfbfnmmfv0606nhr59l14h5jr"))
> +         (file-name (string-append name "-" version))
> +         (modules '((guix build utils)))
> +         (snippet
> +          ;; Hard-code version to avoid a git (and .git/) build dependency.
> +          `(substitute* "Makefile"
> +             (("\"unknown\"") (string-append "\"" ,commit "\"")))))))
> +    (build-system gnu-build-system)
> +    (arguments
> +     `(#:phases (modify-phases %standard-phases
> +                  (delete 'configure))  ; no configure script
> +                #:make-flags (list "CC=gcc"
> +                                   (string-append "DESTDIR="
> +                                                  (assoc-ref %outputs "out"))
> +                                   "prefix="
> +                                   "DOCDIR=/share/doc/mcelog"
> +                                   "etcprefix=$(DOCDIR)/examples")
> +                #:tests? #f))           ; tests must be run as root

Does all tests have to run as root? Also, could you reverse the order of
the arguments to match other package definitions?

> +    (home-page "http://mcelog.org/";)

Nit-pick: the trailing slash is unnecessary :)

> +    (synopsis "Machine check monitor for x86 Linux systems")

If this is x86-only, perhaps we should set (supported-systems)?

> +    (description
> +     "The mcelog daemon is required by the Linux kernel to log memory, I/O,
> +      CPU, and other hardware errors on x86 systems.  It can also perform
> +      user-defined tasks, such as bringing bad pages off-line, when
> +      configurable error thresholds are exceeded.")
> +    (license license:gpl2)))

The rest LGTM.