diff mbox series

[bug#66099,gnome-team,v8,3/6] services: udev: Make udev-rule helper functions generic.

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

Commit Message

Vivien Kraus Oct. 5, 2023, 5:24 p.m. UTC 
* gnu/services/base.scm (udev-configurations-union): New function.
(udev-configuration-file): New function, use file->udev-configuration-file.
(file->udev-configuration-file): New function.
(udev-rules-union): Use udev-configurations-union.
(udev-rule): Use udev-configuration-file.
(file->udev-rule): Use file->udev-configuration-file.
---
 gnu/services/base.scm | 50 +++++++++++++++++++++++++++++--------------
 1 file changed, 34 insertions(+), 16 deletions(-)

Comments

Maxim Cournoyer Oct. 7, 2023, 2:45 p.m. UTC | #1 
Hi,

Vivien Kraus <vivien@planete-kraus.eu> writes:

[...]

> -                       (define rules.d
> -                         (string-append #$output "/lib/udev/rules.d"))
> +                       (define conf.d
> +                         (string-append #$output
> +                                        "/lib/udev/"
> +                                        #$subdirectory
> +                                        ".d"))
>  
>                         (define file-copy-dest
> -                         (string-append rules.d "/" #$file-name))
> +                         (string-append conf.d "/" #$file-name))
>  
> -                       (mkdir-p rules.d)
> +                       (mkdir-p conf.d)

Since conf.d doesn't map to a directory of the same name, I'd use
something more abstract like config-directory or similar.
Maxim Cournoyer Oct. 8, 2023, 12:27 p.m. UTC | #2 
Hi Vivien,

Vivien Kraus <vivien@planete-kraus.eu> writes:

> Dear guix,
>
> I find it surprising that we should rename /lib/udev/hwdb.d to
> /lib/udev/hardware, but the impact is minimal (udevadm on Guix for the
> udev-service-type relies only on UDEV_HWDB_PATH, which can accept /hardware
> easily), and you insist, so, let’s do that.

I thought I mentioned it earlier (can't find it in my replies now) but
I'd prefer sticking to eudev's naming of choice (hwdb.d); going against
this will necessarily cause some friction and maintenance in the future,
for no gain (also, it may be more readable but unexpected by eudev's
users).  I'd say it's an extraneous change.

Sorry for the back and forth.
Vivien Kraus Oct. 8, 2023, 12:33 p.m. UTC | #3 
Le dimanche 08 octobre 2023 à 08:27 -0400, Maxim Cournoyer a écrit :
> > I find it surprising that we should rename /lib/udev/hwdb.d to
> > /lib/udev/hardware, but the impact is minimal (udevadm on Guix for
> > the
> > udev-service-type relies only on UDEV_HWDB_PATH, which can accept
> > /hardware
> > easily), and you insist, so, let’s do that.
> 
> I thought I mentioned it earlier (can't find it in my replies now)
> but
> I'd prefer sticking to eudev's naming of choice (hwdb.d); going
> against
> this will necessarily cause some friction and maintenance in the
> future,
> for no gain (also, it may be more readable but unexpected by eudev's
> users).  I'd say it's an extraneous change.
> 
> Sorry for the back and forth.

No problem, I’ll remove it in the next iteration, and if this is the
last, then the commiter can just ignore the last patch.

Best regards,

Vivien
diff mbox series

Patch

diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 30372bf1b5..0bf0568a4e 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -2184,9 +2184,9 @@  (define-record-type* <udev-configuration>
   (rules  udev-configuration-rules                ;list of file-like
           (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-configurations-union subdirectory packages)
+  "Return the union of the lib/udev/SUBDIRECTORY.d directories found in each
+item of PACKAGES."
   (define build
     (with-imported-modules '((guix build union)
                              (guix build utils))
@@ -2197,39 +2197,57 @@  (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
-            ;; #f if none was found.
+          (define (configuration-sub-directory directory)
+            ;; Return the sub-directory of DIRECTORY containing udev
+            ;; configurations, or #f if none was found.
             (find directory-exists?
                   (map (cut string-append directory <>) %standard-locations)))
 
           (union-build #$output
-                       (filter-map rules-sub-directory '#$packages)))))
+                       (filter-map configuration-sub-directory '#$packages)))))
+
+  (computed-file (string-append "udev-" subdirectory) build))
 
-  (computed-file "udev-rules" build))
+(define (udev-rules-union packages)
+  "Return the union of the lib/udev/rules.d directories found in each
+item of PACKAGES."
+  (udev-configurations-union "rules" packages))
+
+(define (udev-configuration-file subdirectory file-name contents)
+  "Return a directory with a udev configuration file FILE-NAME containing CONTENTS."
+  (file->udev-configuration-file subdirectory file-name (plain-file file-name contents)))
 
 (define (udev-rule file-name contents)
   "Return a directory with a udev rule file FILE-NAME containing CONTENTS."
-  (file->udev-rule file-name (plain-file file-name contents)))
+  (udev-configuration-file "rules" 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."
+(define (file->udev-configuration-file subdirectory file-name file)
+  "Return a directory with a udev configuration file FILE-NAME which is a copy
+ of FILE."
   (computed-file file-name
                  (with-imported-modules '((guix build utils))
                    #~(begin
                        (use-modules (guix build utils))
 
-                       (define rules.d
-                         (string-append #$output "/lib/udev/rules.d"))
+                       (define conf.d
+                         (string-append #$output
+                                        "/lib/udev/"
+                                        #$subdirectory
+                                        ".d"))
 
                        (define file-copy-dest
-                         (string-append rules.d "/" #$file-name))
+                         (string-append conf.d "/" #$file-name))
 
-                       (mkdir-p rules.d)
+                       (mkdir-p conf.d)
                        (copy-file #$file file-copy-dest)))))
 
+(define (file->udev-rule file-name file)
+  "Return a directory with a udev rule file FILE-NAME which is a copy of FILE."
+  (file->udev-configuration-file "rules" file-name file))
+
 (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,