[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#57537] [PATCH v2] gnu: Add ec
From: |
Tobias Geerinckx-Rice |
Subject: |
[bug#57537] [PATCH v2] gnu: Add ec |
Date: |
Fri, 02 Sep 2022 06:47:39 +0200 |
Hi Denis,
This tool has come in handy a few times. (But did I package it? Nope…)
Thank you for doing so!
Don't forget to add a copyright line for yourself at the top of the
file.
Denis 'GNUtoo' Carikli 写道:
This installs the 'ec' binary in DESTDIR/usr/sbin which is wrong.
Indeed, using ‘DESTDIR’ is almost always a mistake.
There are a handful of occurrences in Guix, as a hack to get a buggy
Makefile to work, but those are (thankfully) rare.
Your v2 still has the ‘DESTDIR’ in it, though. Missed when committing?
gnu: Add ec
We ♥ full stops, even here.
+(define-public ec
+ (package
+ (name "ec")
+ (version (package-version linux-libre))
+ (source (package-source linux-libre))
+ (build-system gnu-build-system)
+ (native-inputs (list coreutils))
I doubt this has any effect?
+ (arguments
+ '(#:make-flags (list (string-append "DESTDIR="
+ (assoc-ref %outputs "out"))
The magical %output{s,} is fragile and almost never necessary in new
code. Make the arguments a list of keywords/gexps and use #$output
instead.
+ "sbindir=/sbin")
+ #:phases (modify-phases %standard-phases
+ (delete 'configure) ;no configure script
It makes no difference to Guix, but for the sake of humans: manipulate
phases in their original order when there's no reason not to.
Here, 'configure runs after 'unpack.
+ (add-after 'unpack 'patch-Makefile
+ (lambda _
+ (substitute* "tools/power/acpi/Makefile.config"
+ (("/bin/true")
+ (which "true")))
+ (substitute* "tools/power/acpi/Makefile.config"
+ (("/usr/bin/install")
+ (which "install"))) #t))
Aside: no need to call substitute* twice on the exact same (list of)
file(s).
Nor is it necessary to look up binaries in PATH ‘by hand’. Most tools
do that already.
(substitute* "Makefile.config"
(("/bin/true") "true")
(("/usr/bin/install") "install"))
However, substitution is overkill as that Makefile doesn't hard-code
either name.
Instead, let's just use the make-flags upstream provides:
λ grep '= /.*bin' Makefile*
Makefile.config:INSTALL = /usr/bin/install -c
Makefile.config: STRIPCMD = /bin/true -Since_we_are_debugging
+ (add-after 'patch-Makefile 'enter-subdirectory
+ (lambda _
+ (chdir "tools/power/acpi/tools/ec") #t)))
I'm happy to be the one to inform you that trailing #ts are
long-obsolete.
Feel free to remove them whenever you encounter them. It is very
satisfying.
+ #:tests? #f)) ;no tests
+ (home-page (package-home-page linux-libre))
+ (synopsis
+ "Low level utility for reading or writing Embedded Controller
registers")
+ (description
+ "This utility can read or write specific registers or all the
+available registers of the Embedded Controllers supported by the Linux
The ‘ec’ description should probably mention the word ‘EC’ *somewhere*…
;-)
→ ‘@acronym{EC, Embedded Controller}’
+kernel. To work it needs to run as root, to have the ec_sys driver
+loaded, and to have the debugfs filesystem mounted at
+/sys/kernel/debug/.
‘@code{ec_sys} module loaded’, ‘@code{debugfs} file system’,
‘@file{/sys/kernel/debug}’.
To make write support work, the ec_sys module
+needs to be loaded with the write_support=1 parameter. Write support
+can also be enabled after loading the module with
+the 'echo 1 > /sys/module/ec_sys/parameters/write_support' command.")
Using @code{}/@file{}/@samp{}… also applies here, but do you know if
there's *any* documentation we could install? This kind of
highly-specific how-to isn't a great package description.
I wanted to send back a proper patch but this (below) is all I have time
for. I can't push yet anyway, so no rush.
Thanks again,
T G-R
(define-public ec
(package
(name "ec")
(version (package-version linux-libre))
(source (package-source linux-libre))
(build-system gnu-build-system)
(arguments
(list
#:tests? #f ; no tests
#:make-flags
#~(list (string-append "sbindir=" #$output "/sbin")
"INSTALL=install" "STRIPCMD=true")
#:phases
#~(modify-phases %standard-phases
(add-after 'unpack 'enter-subdirectory
(lambda _
(chdir "tools/power/acpi/tools/ec")))
(delete 'configure)))) ; no configure script
(home-page (package-home-page linux-libre))
(synopsis "Low level utility to read or write Embedded Controller
registers")
(description
"…")
(license license:gpl2)))