* [PATCH] gnu: Add mcelog.
@ 2016-09-14 13:20 Tobias Geerinckx-Rice
2016-09-14 13:45 ` Marius Bakke
2016-09-14 15:02 ` Ludovic Courtès
0 siblings, 2 replies; 12+ messages in thread
From: Tobias Geerinckx-Rice @ 2016-09-14 13:20 UTC (permalink / raw)
To: guix-devel
* gnu/packages/linux.scm (mcelog): New variable.
---
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)))
+ (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
+ (home-page "http://mcelog.org/")
+ (synopsis "Machine check monitor for x86 Linux 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)))
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] gnu: Add mcelog.
2016-09-14 13:20 [PATCH] gnu: Add mcelog Tobias Geerinckx-Rice
@ 2016-09-14 13:45 ` Marius Bakke
2016-09-14 15:03 ` Tobias Geerinckx-Rice
2016-09-14 15:02 ` Ludovic Courtès
1 sibling, 1 reply; 12+ messages in thread
From: Marius Bakke @ 2016-09-14 13:45 UTC (permalink / raw)
To: Tobias Geerinckx-Rice, guix-devel
Tobias Geerinckx-Rice <me@tobias.gr> 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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] gnu: Add mcelog.
2016-09-14 13:20 [PATCH] gnu: Add mcelog Tobias Geerinckx-Rice
2016-09-14 13:45 ` Marius Bakke
@ 2016-09-14 15:02 ` Ludovic Courtès
2016-09-14 15:06 ` Tobias Geerinckx-Rice
1 sibling, 1 reply; 12+ messages in thread
From: Ludovic Courtès @ 2016-09-14 15:02 UTC (permalink / raw)
To: Tobias Geerinckx-Rice; +Cc: guix-devel
Tobias Geerinckx-Rice <me@tobias.gr> skribis:
> * gnu/packages/linux.scm (mcelog): New variable.
To complement what Marius wrote…
> + (arguments
> + `(#:phases (modify-phases %standard-phases
> + (delete 'configure)) ; no configure script
> + #:make-flags (list "CC=gcc"
Please align the #:.
> + (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.")
^^^^^^
Remove leading space here.
Thanks. :-)
Ludo’.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] gnu: Add mcelog.
2016-09-14 13:45 ` Marius Bakke
@ 2016-09-14 15:03 ` Tobias Geerinckx-Rice
2016-09-14 15:19 ` Marius Bakke
2016-09-14 15:23 ` Danny Milosavljevic
0 siblings, 2 replies; 12+ messages in thread
From: Tobias Geerinckx-Rice @ 2016-09-14 15:03 UTC (permalink / raw)
To: mbakke, guix-devel
[-- Attachment #1.1: Type: text/plain, Size: 1265 bytes --]
Marius,
On 14/09/16 15:45, Marius Bakke wrote:
> 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
I did not know that. Thanks for the tip! It still requires a snippet,
unfortunately.
> Tobias Geerinckx-Rice <me@tobias.gr> writes:
>> + #:tests? #f)) ; tests must be run as
>> root
> Does all tests have to run as root?
Yes. Each test wants to load modules & inject synthetic MCE events. The
daemon will even fail to start on an unsupported CPU like my current AMD
laptop.
> Also, could you reverse the order of the arguments to match other
> package definitions?
Hm: *some* other. I rather keep them in approximate order of use.
Unrelated: I see the ‘arguments‘ indentation went funky. Will fix.
>> + (home-page "http://mcelog.org/")
> Nit-pick: the trailing slash is unnecessary :)
Oh, I know, I just have a thing for proper root paths in URIs.
I'm seeing someone about that.
>> + (synopsis "Machine check monitor for x86 Linux systems")
> If this is x86-only, perhaps we should set (supported-systems)?
Indeed. Thanks!
Kind regards,
T G-R
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] gnu: Add mcelog.
2016-09-14 15:02 ` Ludovic Courtès
@ 2016-09-14 15:06 ` Tobias Geerinckx-Rice
0 siblings, 0 replies; 12+ messages in thread
From: Tobias Geerinckx-Rice @ 2016-09-14 15:06 UTC (permalink / raw)
To: ludo; +Cc: guix-devel
[-- Attachment #1.1: Type: text/plain, Size: 166 bytes --]
Ludo',
On 14/09/16 17:02, Ludovic Courtès wrote:
> Please align the #:.
> Remove leading space here.
Yup, I noticed :-) Thanks.
Kind regards,
T G-R
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] gnu: Add mcelog.
2016-09-14 15:03 ` Tobias Geerinckx-Rice
@ 2016-09-14 15:19 ` Marius Bakke
2016-09-14 15:27 ` Danny Milosavljevic
2016-09-14 19:12 ` Tobias Geerinckx-Rice
2016-09-14 15:23 ` Danny Milosavljevic
1 sibling, 2 replies; 12+ messages in thread
From: Marius Bakke @ 2016-09-14 15:19 UTC (permalink / raw)
To: Tobias Geerinckx-Rice, guix-devel
Tobias Geerinckx-Rice <me@tobias.gr> writes:
> On 14/09/16 15:45, Marius Bakke wrote:
>> 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
>
> I did not know that. Thanks for the tip! It still requires a snippet,
> unfortunately.
Another thing, I think the snippet should be moved to a phase, as AFAIK
origin snippets should be reserved for removing unwanted files, or for
reproducibility. But, I may be wrong here.
>>> + (home-page "http://mcelog.org/")
>> Nit-pick: the trailing slash is unnecessary :)
>
> Oh, I know, I just have a thing for proper root paths in URIs.
>
> I'm seeing someone about that.
This made me chuckle. Perhaps I should see someone about saving that
precious byte, too :)
Also, is DESTDIR supposed to be /share, shouldn't it be $out/share?
I think I'd define destdir as a variable, and use that also for the
etcprefix instead of using a make variable.
Thanks!
Marius
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] gnu: Add mcelog.
2016-09-14 15:03 ` Tobias Geerinckx-Rice
2016-09-14 15:19 ` Marius Bakke
@ 2016-09-14 15:23 ` Danny Milosavljevic
1 sibling, 0 replies; 12+ messages in thread
From: Danny Milosavljevic @ 2016-09-14 15:23 UTC (permalink / raw)
To: Tobias Geerinckx-Rice; +Cc: guix-devel
> >> + (home-page "http://mcelog.org/")
> > Nit-pick: the trailing slash is unnecessary :)
If you don't include the trailing slash it will require an additional round-trip to the server in order to add it via a (302) redirect.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] gnu: Add mcelog.
2016-09-14 15:19 ` Marius Bakke
@ 2016-09-14 15:27 ` Danny Milosavljevic
2016-09-14 15:32 ` Marius Bakke
2016-09-14 19:12 ` Tobias Geerinckx-Rice
1 sibling, 1 reply; 12+ messages in thread
From: Danny Milosavljevic @ 2016-09-14 15:27 UTC (permalink / raw)
To: Marius Bakke; +Cc: guix-devel
DESTDIR is a feature which lets you install into a tempdir. The idea is that you install it all there and then atomically replace the ORIGINAL version at the non-DESTDIR location.
It's very bad to set DESTDIR to out because the outdir is used after installation, too (your program will stay at that place). This is not expected by DESTDIR.
So for example let's say you'd have
DESTDIR=/tmp
PREFIX=/usr
.
Then for example a theme is first installed to /tmp/usr/share/themes (by make) - but after it's all done, the distribution is supposed to move it to /usr/share/themes atomically. The program will *always* (also when its still installed to /tmp/usr/share/themes) try to load its files from /usr/share/themes.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] gnu: Add mcelog.
2016-09-14 15:27 ` Danny Milosavljevic
@ 2016-09-14 15:32 ` Marius Bakke
2016-09-14 18:46 ` Tobias Geerinckx-Rice
0 siblings, 1 reply; 12+ messages in thread
From: Marius Bakke @ 2016-09-14 15:32 UTC (permalink / raw)
To: Danny Milosavljevic; +Cc: guix-devel
Danny Milosavljevic <dannym@scratchpost.org> writes:
> DESTDIR is a feature which lets you install into a tempdir. The idea
> is that you install it all there and then atomically replace the
> ORIGINAL version at the non-DESTDIR location.
Oops, I meant DOCDIR! Sorry for the confusion!
>> >> + (home-page "http://mcelog.org/")
>> > Nit-pick: the trailing slash is unnecessary :)
>
> If you don't include the trailing slash it will require an additional
> round-trip to the server in order to add it via a (302) redirect.
That's a very good point, although all HTTP clients I've used add it
automatically. But, explicit is better than implicit and all that.
Now I'll have to start doing it too :)
Thanks!
Marius
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] gnu: Add mcelog.
2016-09-14 15:32 ` Marius Bakke
@ 2016-09-14 18:46 ` Tobias Geerinckx-Rice
0 siblings, 0 replies; 12+ messages in thread
From: Tobias Geerinckx-Rice @ 2016-09-14 18:46 UTC (permalink / raw)
To: mbakke, dannym; +Cc: guix-devel
[-- Attachment #1.1: Type: text/plain, Size: 673 bytes --]
Danny, Marius,
Danny Milosavljevic <dannym@scratchpost.org> writes:
> DESTDIR is a feature which lets you install into a tempdir. The idea
> is that you install it all there and then atomically replace the
> ORIGINAL version at the non-DESTDIR location.
Herp. I know all this, no idea how I managed to confuse them.
On 14/09/16 17:32, Marius Bakke wrote:
> Oops, I meant DOCDIR! Sorry for the confusion!
I think my confusion above had to do with wrestling DOCDIR when I first
packaged this. It should indeed be $out/share... when using
prefix/DESTDIR correctly!
> Now I'll have to start doing it too :)
We have cookies.
Kind regards,
T G-R
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] gnu: Add mcelog.
2016-09-14 15:19 ` Marius Bakke
2016-09-14 15:27 ` Danny Milosavljevic
@ 2016-09-14 19:12 ` Tobias Geerinckx-Rice
2016-09-14 19:32 ` Marius Bakke
1 sibling, 1 reply; 12+ messages in thread
From: Tobias Geerinckx-Rice @ 2016-09-14 19:12 UTC (permalink / raw)
To: mbakke, guix-devel
[-- Attachment #1.1: Type: text/plain, Size: 592 bytes --]
Marius,
On 14/09/16 17:19, Marius Bakke wrote:
> Another thing, I think the snippet should be moved to a phase, as AFAIK
> origin snippets should be reserved for removing unwanted files, or for
> reproducibility.
Not if we use your suggested snapshot tarball, which rightly lack a .git
directory. There is no way for the build system to divine the version
number at build time with ‘git describe’.
Unpatched mcelog, manually built from ‘guix download’ed sources, would
report its version as ‘unknown’. With this snippet, it just works.
Kind regards,
T G-R
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] gnu: Add mcelog.
2016-09-14 19:12 ` Tobias Geerinckx-Rice
@ 2016-09-14 19:32 ` Marius Bakke
0 siblings, 0 replies; 12+ messages in thread
From: Marius Bakke @ 2016-09-14 19:32 UTC (permalink / raw)
To: Tobias Geerinckx-Rice, guix-devel
Tobias Geerinckx-Rice <me@tobias.gr> writes:
> Marius,
>
> On 14/09/16 17:19, Marius Bakke wrote:
>> Another thing, I think the snippet should be moved to a phase, as AFAIK
>> origin snippets should be reserved for removing unwanted files, or for
>> reproducibility.
>
> Not if we use your suggested snapshot tarball, which rightly lack a .git
> directory. There is no way for the build system to divine the version
> number at build time with ‘git describe’.
>
> Unpatched mcelog, manually built from ‘guix download’ed sources, would
> report its version as ‘unknown’. With this snippet, it just works.
Yes, I was mostly echoing Leos sentiment from this post:
https://lists.gnu.org/archive/html/guix-devel/2016-08/msg00937.html
I don't think reporting "unknown" as a version is a critical bug, but
don't have any strong opinions either way. It seems like the manual
could use some clarification on its use, though.
Cheers,
Marius
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-09-14 19:32 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-14 13:20 [PATCH] gnu: Add mcelog Tobias Geerinckx-Rice
2016-09-14 13:45 ` Marius Bakke
2016-09-14 15:03 ` Tobias Geerinckx-Rice
2016-09-14 15:19 ` Marius Bakke
2016-09-14 15:27 ` Danny Milosavljevic
2016-09-14 15:32 ` Marius Bakke
2016-09-14 18:46 ` Tobias Geerinckx-Rice
2016-09-14 19:12 ` Tobias Geerinckx-Rice
2016-09-14 19:32 ` Marius Bakke
2016-09-14 15:23 ` Danny Milosavljevic
2016-09-14 15:02 ` Ludovic Courtès
2016-09-14 15:06 ` Tobias Geerinckx-Rice
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).