diff mbox series

[bug#66099,gnome-team,v6,3/5] gnu: udev-service-type: accept hwdb file extensions.

Message ID c5b55ee09b6323ee9b1d4036e36008421eb03971.1696455423.git.vivien@planete-kraus.eu
State New
Headers show
Series Update eudev, udev-service-type, upower | expand

Commit Message

Vivien Kraus Oct. 2, 2023, 7:08 p.m. UTC
The "rules" field in the udev-configuration record can now hold both rules and
hwdb files.

The udev-rules-union has been made generic, and renamed to
udev-configuration-union, so that it works with either rules or hwdb files.

Some udev-related auxiliary functions in (gnu services base) had non-texinfo
variable references in their docstrings ("FILE-NAME" instead of
"@var{file-name}").

The contents of the /etc directory now includes hwdb.d and hwdb.bin, which is
computed immediately.

The documentation has been reworked so as to explain why creating udev rules
or hwdb needs helper functions for configuration or extension.

The hwdb.bin file is conditionally computed by a native version of eudev, that
may be configured in the udev-service-type configuration. The condition is
that both target and native eudev have the same version. If so, we can
reasonably expect that the hwdb.bin file created by native eudev can later be
read by target eudev.

* gnu/services/base.scm (udev-configuration-union): Make udev-rules-union generic.
(udev-hwdb): New function.
(file->udev-rule): Fix docstring.
(file->udev-hwdb): New function.
(udev-rules-service): Fix docstring.
(udev-hwdb-service): New function.
(udev-etc): Add hwdb.d and hwdb.bin.
(module): Export udev-hwdb, file->udev-hwdb, and udev-hwdb-service.
(<udev-configuration>): Add the native-udev field.
* doc/guix.texi (Base Services)[udev-service-type]: Explain configuration and
extension values.
* doc/guix.texi (Base Services)[udev-hwdb]: Document it.
[udev-hwdb-service]: Same.
* doc/guix.texi (Base Services)[udev-configuration]: Document the native-udev
field.
---
 doc/guix.texi         | 52 +++++++++++++++++++-----
 gnu/services/base.scm | 94 ++++++++++++++++++++++++++++++++++++-------
 2 files changed, 122 insertions(+), 24 deletions(-)

Comments

Liliana Marie Prikler Oct. 5, 2023, 5:42 a.m. UTC | #1
Am Montag, dem 02.10.2023 um 21:08 +0200 schrieb Vivien Kraus:
> The "rules" field in the udev-configuration record can now hold both
> rules and hwdb files.
Yeah, how about we instead have separate fields – one named "rules" and
one named "hardware".  We're already breaking ABI stability anyway, so
we might as well.

Speaking of which, we might want to rename "hwdb.d" to "hardware" in
udev itself to make that name readable, but keep /etc/udev/hwdb.bin as-
is.

Cheers
Vivien Kraus Oct. 5, 2023, 5:55 a.m. UTC | #2
Le jeudi 05 octobre 2023 à 07:42 +0200, Liliana Marie Prikler a écrit :
> Am Montag, dem 02.10.2023 um 21:08 +0200 schrieb Vivien Kraus:
> > The "rules" field in the udev-configuration record can now hold
> > both
> > rules and hwdb files.
> Yeah, how about we instead have separate fields – one named "rules"
> and
> one named "hardware".

Since the extensions are untyped anyway, I thought we would not care in
the configuration record itself. Should we also introduce types for the
extension values?

Vivien
Liliana Marie Prikler Oct. 5, 2023, 6:53 a.m. UTC | #3
Am Donnerstag, dem 05.10.2023 um 07:55 +0200 schrieb Vivien Kraus:
> Le jeudi 05 octobre 2023 à 07:42 +0200, Liliana Marie Prikler a
> écrit :
> > Am Montag, dem 02.10.2023 um 21:08 +0200 schrieb Vivien Kraus:
> > > The "rules" field in the udev-configuration record can now hold
> > > both rules and hwdb files.
> > Yeah, how about we instead have separate fields – one named "rules"
> > and one named "hardware".
> 
> Since the extensions are untyped anyway, I thought we would not care
> in the configuration record itself. Should we also introduce types
> for the extension values?
This is about field semantics rather than type theory, but if you
prefer, I think we can use list-of file-like? or similar for both.

Cheers
Liliana Marie Prikler Oct. 6, 2023, 6:23 p.m. UTC | #4
Am Freitag, dem 06.10.2023 um 18:45 +0200 schrieb Vivien Kraus:
> Le jeudi 05 octobre 2023 à 07:42 +0200, Liliana Marie Prikler a écrit
> :
> > Speaking of which, we might want to rename "hwdb.d" to "hardware"
> > in udev itself to make that name readable, but keep
> > /etc/udev/hwdb.bin as-is.
> 
> I renamed to "hardware" in Guix documentation, but individual
> packages (if they behave like upower) expect to install their
> hardware files in "hwdb.d", so I think we should keep "hwdb.d" as the
> directory name.
We could patch those or supply configuration arguments, but fair enough
if you don't want to.

> Le jeudi 05 octobre 2023 à 08:53 +0200, Liliana Marie Prikler a
> écrit :
> > This sounds more like a limitation/bug in our udev-service-type
> > than in eudev?  If eudev wants its files installed to /etc, they
> > should be left there I think.  Going against this is more
> > maintenance down the road, and surprise from users.
> 
> As discussed in IRC, we set sysconfdir to /etc so that it can have
> hardware files from many packages. So the hardware files are searched
> from either "/etc/udev/hwdb.d" or
> "$individual_package_prefix/lib/udev/hwdb.d".
> So, the only standard directory to install hwdb files during a build
> of a package is $package_prefix/lib/udev/hwdb.d.
Which makes me think we could also make this $prefix/lib/udev/hardware.
But again, if you like ltrsld, I'm fine with that.

> Vivien Kraus (6):
>   gnu: eudev: Update to 3.2.14.
>   services: udev: Rewrite udev-rule to use file->udev-rule.
>   services: udev: Make udev-rule helper functions generic.
>   gnu: udev-service-type: accept hardware description file
> extensions.
>   gnu: libgudev: Update to 238.
>   gnu: upower: Update to 1.90.2.
> 
>  doc/guix.texi                                 |  57 ++++++--
>  gnu/packages/gnome.scm                        |  43 +++---
>  gnu/packages/linux.scm                        |  57 +++++---
>  .../patches/eudev-rules-directory.patch       |   9 +-
>  gnu/services/base.scm                         | 124 +++++++++++++---
> --
>  5 files changed, 205 insertions(+), 85 deletions(-)
Will look at those shortly.

Cheers
diff mbox series

Patch

diff --git a/doc/guix.texi b/doc/guix.texi
index 46591b2f64..3310271ec8 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -19322,9 +19322,23 @@  Base Services
 @file{/dev} directory dynamically, whose value is a
 @code{<udev-configuration>} object.
 
-This service type can be @emph{extended} using procedures
-@code{udev-rules-service} along with @code{file->udev-rule} or
-@code{udev-rule} which simplify the process of writing udev rules.
+Since the file names for udev rules and hwdb files matters, the
+configuration items for rules and hwdb cannot simply be plain file-like
+objects with the rules content, because the name would be
+ignored. Instead, they are directory file-like objects that contain
+optional rules in @code{lib/udev/rules.d} and optional hwdb files in
+@code{lib/udev/hwdb.d}. This way, the service can be configured with
+whole packages from which to take rules and hwdb files.
+
+The @code{udev-service-type} can be @emph{extended} with file-like
+directories that respect this hierarchy. However, to generate a
+configuration or an extension, it is advised to use @code{udev-rule} and
+@code{file->udev-rule} for rules, and @code{udev-hwdb} and
+@code{file->udev-hwdb} for hwdb files.
+
+In an operating-system declaration, this service type can be
+@emph{extended} using procedures @code{udev-rules-service} and
+@code{udev-hwdb-service}.
 @end defvar
 
 @deftp {Data Type} udev-configuration
@@ -19334,8 +19348,18 @@  Base Services
 @item @code{udev} (default: @code{eudev}) (type: file-like)
 Package object of the udev service.
 
+@item @code{native-udev} (default: @code{eudev}) (type: file-like)
+Native udev package to compile @code{hwdb.bin}. The trie format used for
+@code{hwdb.bin} must be compatible with the @code{udev} and
+@code{native-udev} fields of the udev configuration. To avoid generating
+@code{hwdb.bin}, pass @code{#f} as the @code{native-udev} field.
+
+In any case, if the package version string differs between @code{udev}
+and @code{native-udev}, @code{hwdb.bin} is @strong{not} created.
+
 @item @code{rules} (default: @var{'()}) (type: list-of-file-like)
-List of file-like objects denoting udev-rule files.
+List of file-like objects denoting udev-rule or udev-hwdb files under a
+sub-directory.
 
 @end table
 @end deftp
@@ -19358,6 +19382,11 @@  Base Services
 @end lisp
 @end deffn
 
+@deffn {Procedure} udev-hwdb @var{file-name} @var{contents}
+Return a udev-hwdb file named @var{file-name} containing the hardware
+information @var{contents}.
+@end deffn
+
 @deffn {Procedure} udev-rules-service @var{name} @var{rules} [#:groups '()]
 Return a service that extends @code{udev-service-type} with @var{rules}
 and @code{account-service-type} with @var{groups} as system groups.
@@ -19377,6 +19406,11 @@  Base Services
 @end lisp
 @end deffn
 
+@deffn {Procedure} udev-hwdb-service @var{name} @var{hwdb}
+Return a service that extends @code{udev-service-type} with
+@var{hwdb}. The service name is @code{@var{name}-udev-hwdb}.
+@end deffn
+
 @deffn {Procedure} file->udev-rule @var{file-name} @var{file}
 Return a udev-rule file named @var{file-name} containing the rules
 defined within @var{file}, a file-like object.
@@ -19401,12 +19435,10 @@  Base Services
 @end lisp
 @end deffn
 
-Additionally, Guix package definitions can be included in @var{rules} in
-order to extend the udev rules with the definitions found under their
-@file{lib/udev/rules.d} sub-directory.  In lieu of the previous
-@var{file->udev-rule} example, we could have used the
-@var{android-udev-rules} package which exists in Guix in the @code{(gnu
-packages android)} module.
+@deffn {Procedure} file->udev-hwdb @var{file-name} @var{file}
+Return a udev-hwdb file named @var{file-name} containing the rules
+defined within @var{file}, a file-like object.
+@end deffn
 
 The following example shows how to use the @var{android-udev-rules}
 package so that the Android tool @command{adb} can detect devices
diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 190803b780..00916a35e4 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -81,6 +81,7 @@  (define-module (gnu services base)
                 #:select (mount-flags->bit-mask
                           swap-space->flags-bit-mask))
   #:use-module (guix gexp)
+  #:use-module ((guix packages) #:select (package-version))
   #:use-module (guix records)
   #:use-module (guix modules)
   #:use-module (guix pki)
@@ -153,8 +154,11 @@  (define-module (gnu services base)
             udev-service-type
             udev-service  ; deprecated
             udev-rule
+            udev-hwdb
             file->udev-rule
+            file->udev-hwdb
             udev-rules-service
+            udev-hwdb-service
 
             login-configuration
             login-configuration?
@@ -2181,12 +2185,15 @@  (define-record-type* <udev-configuration>
   udev-configuration?
   (udev   udev-configuration-udev                 ;file-like
           (default eudev))
-  (rules  udev-configuration-rules                ;list of file-like
+  (native-udev udev-configuration-native-udev
+               (default eudev))
+  (rules  udev-configuration-rules                ;list of rule- and
+                                                  ;hwdb-providing packages
           (default '())))
 
-(define (udev-rules-union packages)
-  "Return the union of the @code{lib/udev/rules.d} directories found in each
-item of @var{packages}."
+(define (udev-configuration-union subdirectory packages)
+  "Return the union of the @code{lib/udev/@var{subdirectory}.d} directories found
+in each item of @var{packages}."
   (define build
     (with-imported-modules '((guix build union)
                              (guix build utils))
@@ -2197,7 +2204,8 @@  (define (udev-rules-union packages)
                        (srfi srfi-26))
 
           (define %standard-locations
-            '("/lib/udev/rules.d" "/libexec/udev/rules.d"))
+            '(#$(string-append "/lib/udev/" subdirectory ".d")
+                #$(string-append "/libexec/udev/" subdirectory ".d")))
 
           (define (rules-sub-directory directory)
             ;; Return the sub-directory of DIRECTORY containing udev rules, or
@@ -2208,15 +2216,21 @@  (define (udev-rules-union packages)
           (union-build #$output
                        (filter-map rules-sub-directory '#$packages)))))
 
-  (computed-file "udev-rules" build))
+  (computed-file (string-append "udev-" subdirectory) build))
 
 (define (udev-rule file-name contents)
   "Return a directory with a udev rule file @var{file-name} containing
 @var{contents}."
   (file->udev-rule file-name (plain-file file-name contents)))
 
+(define (udev-hwdb file-name contents)
+  "Return a directory with a udev hwdb file @var{file-name} containing
+@var{contents}."
+  (file->udev-hwdb file-name (plain-file file-name contents)))
+
 (define (file->udev-rule file-name file)
-  "Return a directory with a udev rule file FILE-NAME which is a copy of FILE."
+  "Return a directory with a udev rule file @var{file-name} which is a copy of
+@var{file}."
   (computed-file file-name
                  (with-imported-modules '((guix build utils))
                    #~(begin
@@ -2231,6 +2245,23 @@  (define (file->udev-rule file-name file)
                        (mkdir-p rules.d)
                        (copy-file #$file file-copy-dest)))))
 
+(define (file->udev-hwdb file-name file)
+  "Return a directory with a udev hwdb file @var{file-name} which is a copy of
+@var{file}."
+  (computed-file file-name
+                 (with-imported-modules '((guix build utils))
+                   #~(begin
+                       (use-modules (guix build utils))
+
+                       (define hwdb.d
+                         (string-append #$output "/lib/udev/hwdb.d"))
+
+                       (define file-copy-dest
+                         (string-append hwdb.d "/" #$file-name))
+
+                       (mkdir-p hwdb.d)
+                       (copy-file #$file file-copy-dest)))))
+
 (define kvm-udev-rule
   ;; Return a directory with a udev rule that changes the group of /dev/kvm to
   ;; "kvm" and makes it #o660.  Apparently QEMU-KVM used to ship this rule,
@@ -2338,13 +2369,35 @@  (define udev.conf
 
 (define (udev-etc config)
   (match-record config <udev-configuration>
-    (udev rules)
+    (udev native-udev rules)
+    (let* ((hwdb.d
+            (udev-configuration-union "hwdb" (cons* udev rules)))
+           (hwdb.bin
+            (and native-udev
+                 (equal? (package-version udev)
+                         (package-version native-udev))
+                 (computed-file
+                  "hwdb.bin"
+                  (with-imported-modules '((guix build utils))
+                    #~(begin
+                        (use-modules (guix build utils))
+                        (setenv "UDEV_HWDB_PATH" #$hwdb.d)
+                        (invoke #+(file-append native-udev "/bin/udevadm")
+                                "hwdb"
+                                "--update"
+                                "-o" #$output)))))))
     `(("udev"
        ,(file-union "udev"
                     `(("udev.conf" ,udev.conf)
                       ("rules.d"
-                       ,(udev-rules-union (cons* udev kvm-udev-rule
-                                                 rules)))))))))
+                       ,(udev-configuration-union
+                         "rules"
+                         (cons* udev kvm-udev-rule
+                                rules)))
+                      ("hwdb.d" ,hwdb.d)
+                      ,@(if hwdb.bin
+                            `(("hwdb.bin" ,hwdb.bin))
+                            '()))))))))
 
 (define udev-service-type
   (service-type (name 'udev)
@@ -2373,10 +2426,10 @@  (define-deprecated (udev-service #:key (udev eudev) (rules '()))
            (udev-configuration (udev udev) (rules rules))))
 
 (define* (udev-rules-service name rules #:key (groups '()))
-  "Return a service that extends udev-service-type with RULES and
-account-service-type with GROUPS as system groups.  This works by creating a
-singleton service type NAME-udev-rules, of which the returned service is an
-instance."
+  "Return a service that extends udev-service-type with @var{rules} and
+@code{account-service-type} with @var{groups} as system groups.  This works by
+creating a singleton service type @code{@var{name}-udev-rules}, of which the
+returned service is an instance."
   (let* ((name (symbol-append name '-udev-rules))
          (account-extension
           (const (map (lambda (group)
@@ -2393,6 +2446,19 @@  (define* (udev-rules-service name rules #:key (groups '()))
                 (description "This service adds udev rules."))))
     (service type #f)))
 
+(define (udev-hwdb-service name hwdb-files)
+  "Return a service that extends udev-service-type with @var{hwdb-files}, named
+@code{@var{name}-udev-hwdb}."
+  (let* ((name (symbol-append name '-udev-hwdb))
+         (udev-extension (const (list hwdb-files)))
+         (type (service-type
+                (name name)
+                (extensions (list
+                             (service-extension
+                              udev-service-type udev-extension)))
+                (description "This service adds udev hwdb files."))))
+    (service type #f)))
+
 (define (swap-space->shepherd-service-name space)
   (let ((target (swap-space-target space)))
     (symbol-append 'swap-