diff mbox series

[bug#66099,gnome-team,v7,1/5] gnu: eudev: Update libudev version to 251.

Message ID 5336473ade7d12a8c96f7212dad22c9fb3d18524.1696482243.git.vivien@planete-kraus.eu
State New
Headers show
Series Update eudev with a snippet, udev-service-type, upower | expand

Commit Message

Vivien Kraus Sept. 19, 2023, 11:23 a.m. UTC
Eudev has significant improvements from 3.2.12, but they are not released
yet. The package version number has already been bumped to 3.2.14, but since
it is not released yet, we still advertise it as 3.2.12.

Everything that eudev searches in "sysconf" (/etc/udev/*) is actually searched
under its installation prefix. The udev-service-type however prepares every
file in /etc/udev, without a prefix.

Eudev has a hardware database that installs descriptions of hardware in /etc,
but they should go to <prefix>/lib prior to being used. The build system can’t
install to /etc, and should not, because this directory is owned by the
udev-service-type. So they are installed directly in <prefix>/lib. Another
file, an empty /etc/udev.conf, is simply ignored.

The hwdb.bin file used to be generated in <prefix>/etc/udev/hwdb.bin, but
since the sysconf dir is now directly /etc, the hwdb.bin index will not be
found under <prefix>/etc/udev/hwdb.bin.

* gnu/packages/linux.scm (eudev): Update to a post-v3.2.12 commit.
[snippet]: New snippet to override the version number.
[modules]: Import (guix build utils).
[#:phases] <allow-eudev-hwdb>: New phase.
<install-in-lib>: New phase.
<build-hwdb>: Remove phase.
[#:configure-flags]: Set sysconfdir to avoid a prefix.
[native-search-paths]: Add UDEV_HWDB_PATH.
* gnu/packages/patches/eudev-rules-directory.patch: Rebase it.
---
 gnu/packages/linux.scm                        | 64 ++++++++++++++-----
 .../patches/eudev-rules-directory.patch       |  9 +--
 2 files changed, 52 insertions(+), 21 deletions(-)

Comments

Maxim Cournoyer Oct. 5, 2023, 1:28 p.m. UTC | #1
Hello,

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

> Eudev has significant improvements from 3.2.12, but they are not released
> yet. The package version number has already been bumped to 3.2.14, but since
> it is not released yet, we still advertise it as 3.2.12.
>
> Everything that eudev searches in "sysconf" (/etc/udev/*) is actually searched
> under its installation prefix. The udev-service-type however prepares every
> file in /etc/udev, without a prefix.
>
> Eudev has a hardware database that installs descriptions of hardware in /etc,
> but they should go to <prefix>/lib prior to being used. The build system can’t
> install to /etc, and should not, because this directory is owned by the
> udev-service-type. So they are installed directly in <prefix>/lib. Another
> file, an empty /etc/udev.conf, is simply ignored.

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.

> The hwdb.bin file used to be generated in <prefix>/etc/udev/hwdb.bin, but
> since the sysconf dir is now directly /etc, the hwdb.bin index will not be
> found under <prefix>/etc/udev/hwdb.bin.
>
> * gnu/packages/linux.scm (eudev): Update to a post-v3.2.12 commit.
> [snippet]: New snippet to override the version number.
> [modules]: Import (guix build utils).
> [#:phases] <allow-eudev-hwdb>: New phase.
> <install-in-lib>: New phase.
> <build-hwdb>: Remove phase.
> [#:configure-flags]: Set sysconfdir to avoid a prefix.

The above should be commented in the code.  What prefix?  sysconfdir
typically defaults to /etc, if I recall correctly.

> [native-search-paths]: Add UDEV_HWDB_PATH.
> * gnu/packages/patches/eudev-rules-directory.patch: Rebase it.
> ---
>  gnu/packages/linux.scm                        | 64 ++++++++++++++-----
>  .../patches/eudev-rules-directory.patch       |  9 +--
>  2 files changed, 52 insertions(+), 21 deletions(-)
>
> diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
> index 85e3d9845d..e75ed0b233 100644
> --- a/gnu/packages/linux.scm
> +++ b/gnu/packages/linux.scm
> @@ -4263,18 +4263,31 @@ (define-public earlyoom
>  
>  (define-public eudev
>    ;; The post-systemd fork, maintained by Gentoo.
> +  (define commit
> +    "c5bae0b656513463f92808f324f8fcbe34a0b401")
> +  (define revision "1")
>    (package
>      (name "eudev")
> -    (version "3.2.11")
> +    ;; Remove the snippet when upgrading to a released version
> +    (version (git-version "3.2.12" revision commit))
>      (source (origin
>                (method git-fetch)
>                (uri (git-reference (url "https://github.com/gentoo/eudev")
> -                                  (commit (string-append "v" version))))
> +                                  (commit commit)))
>                (file-name (git-file-name name version))
>                (sha256
>                 (base32
> -                "0dzaqwjnl55f69ird57wb6skahc6l7zs1slsrzqqfhww33icp6av"))
> -              (patches (search-patches "eudev-rules-directory.patch"))))
> +                "0rqyzmp8kcnxiy1hq13pr2syp4krnf6q97xwlr0bwcd6x4grbak4"))
> +              (patches (search-patches "eudev-rules-directory.patch"))
> +              (modules '((guix build utils)))
> +              (snippet
> +               #~(begin
> +                   ;; configure.ac uses 3.2.14 as the version number, but it
> +                   ;; is not released yet. Remove the snippet once a later
> +                   ;; version is released.
> +                   (substitute* "configure.ac"
> +                     (("AC_INIT\\(\\[eudev\\],\\[3.2.14\\]")
> +                      "AC_INIT([eudev],[3.2.12]"))))))
>      (build-system gnu-build-system)
>      (arguments
>       (list
> @@ -4285,6 +4298,28 @@ (define-public eudev
>                (substitute* "man/make.sh"
>                  (("/usr/bin/xsltproc")
>                   (search-input-file (or native-inputs inputs) "/bin/xsltproc")))))
> +          (add-before 'bootstrap 'install-in-lib
> +            (lambda _
> +              ;; eudev wants to install its provided hwdb files in /etc, but
> +              ;; we want them in udevlibexecdir.
> +              (copy-file "hwdb/Makefile.am" "hwdb/files.am")
> +              (call-with-output-file "hwdb/Makefile.am"
> +                (lambda (port)
> +                  (format port "udevhwdblibdir = $(udevlibexecdir)/hwdb.d\n")
> +                  (format port "include ./files.am")))
> +              (substitute* "hwdb/files.am"
> +                (("dist_udevhwdb_DATA =")
> +                 "dist_udevhwdblib_DATA ="))
> +              ;; eudev wants to install a template udev.conf into /etc, but we
> +              ;; do not care.
> +              (substitute* "src/udev/Makefile.am"
> +                (("dist_udevconf_DATA =")
> +                 "dist_noinst_DATA ="))
> +              ;; eudev thinks we want to make sure /etc/udev/rules.d exists
> +              ;; when installing - we do not.
> +              (substitute* "rules/Makefile.am"
> +                (("\\$\\(MKDIR_P\\) \\$\\(DESTDIR\\)\\$\\(udevconfdir\\)/rules\\.d")
> +                 "true"))))

That's rather complex.  Could we instead modify udev-service-type to
only union what it needs under /etc/udev and not the undesired files from
this package?

>            (add-after 'install 'move-static-library
>              (lambda _
>                (let ((source (string-append #$output "/lib/libudev.a"))
> @@ -4296,19 +4331,14 @@ (define-public eudev
>                  ;; such that Libtool looks for it in the usual places.
>                  (substitute* (string-append #$output "/lib/libudev.la")
>                    (("old_library=.*")
> -                   "old_library=''\n")))))
> -          (add-after 'install 'build-hwdb
> -            (lambda _
> -              ;; Build OUT/etc/udev/hwdb.bin.  This allows 'lsusb' and
> -              ;; similar tools to display product names.
> -              ;;
> -              ;; XXX: This can't be done when cross-compiling. Find another way
> -              ;; to generate hwdb.bin for cross-built systems.
> -              #$@(if (%current-target-system)
> -                     #~(#t)
> -                     #~((invoke (string-append #$output "/bin/udevadm")
> -                                "hwdb" "--update"))))))
> -       #:configure-flags #~(list "--enable-manpages")))
> +                   "old_library=''\n"))))))
> +      #:configure-flags
> +      #~(list "--enable-manpages"
> +              "--sysconfdir=/etc")))

See comment above about justifying this flag.

> +    (native-search-paths
> +      (list (search-path-specification
> +              (variable "UDEV_HWDB_PATH")
> +              (files '("lib/udev/hwdb.d")))))
>      (native-inputs
>       (list autoconf
>             automake
> diff --git a/gnu/packages/patches/eudev-rules-directory.patch b/gnu/packages/patches/eudev-rules-directory.patch
> index 54fc01c6d5..c4b1cfae39 100644
> --- a/gnu/packages/patches/eudev-rules-directory.patch
> +++ b/gnu/packages/patches/eudev-rules-directory.patch
> @@ -4,9 +4,9 @@ The old udev 182 supported $UDEV_CONFIG_FILE, which in turn allowed
>  the search path to be customized, but eudev no longer has this, hence
>  this hack.
>  
> ---- eudev-3.1.5/src/udev/udev-rules.c	2015-10-13 06:22:14.000000000 +0800
> -+++ eudev-3.1.5/src/udev/udev-rules.c	2015-10-16 20:45:38.491934336 +0800
> -@@ -47,15 +47,11 @@
> +--- a/src/udev/udev-rules.c
> ++++ b/src/udev/udev-rules.c
> +@@ -48,16 +48,11 @@ struct uid_gid {
>           };
>   };
>   
> @@ -20,11 +20,12 @@ this hack.
>  -        "/lib/udev/rules.d",
>  -        "/usr/lib/udev/rules.d",
>  -#endif
> +-        "/usr/local/lib/udev/rules.d",
>  +        NULL,			/* placeholder for $EUDEV_RULES_DIRECTORY */
>           NULL};
>   
>   struct udev_rules {
> -@@ -1704,6 +1700,9 @@
> +@@ -1718,6 +1713,9 @@ struct udev_rules *udev_rules_new(struct udev *udev, int resolve_names) {
>   
>           udev_rules_check_timestamp(rules);

It'd be nicer if eudev's build system allowed to configure the above
without patches hard coding things.
diff mbox series

Patch

diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index 85e3d9845d..e75ed0b233 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -4263,18 +4263,31 @@  (define-public earlyoom
 
 (define-public eudev
   ;; The post-systemd fork, maintained by Gentoo.
+  (define commit
+    "c5bae0b656513463f92808f324f8fcbe34a0b401")
+  (define revision "1")
   (package
     (name "eudev")
-    (version "3.2.11")
+    ;; Remove the snippet when upgrading to a released version
+    (version (git-version "3.2.12" revision commit))
     (source (origin
               (method git-fetch)
               (uri (git-reference (url "https://github.com/gentoo/eudev")
-                                  (commit (string-append "v" version))))
+                                  (commit commit)))
               (file-name (git-file-name name version))
               (sha256
                (base32
-                "0dzaqwjnl55f69ird57wb6skahc6l7zs1slsrzqqfhww33icp6av"))
-              (patches (search-patches "eudev-rules-directory.patch"))))
+                "0rqyzmp8kcnxiy1hq13pr2syp4krnf6q97xwlr0bwcd6x4grbak4"))
+              (patches (search-patches "eudev-rules-directory.patch"))
+              (modules '((guix build utils)))
+              (snippet
+               #~(begin
+                   ;; configure.ac uses 3.2.14 as the version number, but it
+                   ;; is not released yet. Remove the snippet once a later
+                   ;; version is released.
+                   (substitute* "configure.ac"
+                     (("AC_INIT\\(\\[eudev\\],\\[3.2.14\\]")
+                      "AC_INIT([eudev],[3.2.12]"))))))
     (build-system gnu-build-system)
     (arguments
      (list
@@ -4285,6 +4298,28 @@  (define-public eudev
               (substitute* "man/make.sh"
                 (("/usr/bin/xsltproc")
                  (search-input-file (or native-inputs inputs) "/bin/xsltproc")))))
+          (add-before 'bootstrap 'install-in-lib
+            (lambda _
+              ;; eudev wants to install its provided hwdb files in /etc, but
+              ;; we want them in udevlibexecdir.
+              (copy-file "hwdb/Makefile.am" "hwdb/files.am")
+              (call-with-output-file "hwdb/Makefile.am"
+                (lambda (port)
+                  (format port "udevhwdblibdir = $(udevlibexecdir)/hwdb.d\n")
+                  (format port "include ./files.am")))
+              (substitute* "hwdb/files.am"
+                (("dist_udevhwdb_DATA =")
+                 "dist_udevhwdblib_DATA ="))
+              ;; eudev wants to install a template udev.conf into /etc, but we
+              ;; do not care.
+              (substitute* "src/udev/Makefile.am"
+                (("dist_udevconf_DATA =")
+                 "dist_noinst_DATA ="))
+              ;; eudev thinks we want to make sure /etc/udev/rules.d exists
+              ;; when installing - we do not.
+              (substitute* "rules/Makefile.am"
+                (("\\$\\(MKDIR_P\\) \\$\\(DESTDIR\\)\\$\\(udevconfdir\\)/rules\\.d")
+                 "true"))))
           (add-after 'install 'move-static-library
             (lambda _
               (let ((source (string-append #$output "/lib/libudev.a"))
@@ -4296,19 +4331,14 @@  (define-public eudev
                 ;; such that Libtool looks for it in the usual places.
                 (substitute* (string-append #$output "/lib/libudev.la")
                   (("old_library=.*")
-                   "old_library=''\n")))))
-          (add-after 'install 'build-hwdb
-            (lambda _
-              ;; Build OUT/etc/udev/hwdb.bin.  This allows 'lsusb' and
-              ;; similar tools to display product names.
-              ;;
-              ;; XXX: This can't be done when cross-compiling. Find another way
-              ;; to generate hwdb.bin for cross-built systems.
-              #$@(if (%current-target-system)
-                     #~(#t)
-                     #~((invoke (string-append #$output "/bin/udevadm")
-                                "hwdb" "--update"))))))
-       #:configure-flags #~(list "--enable-manpages")))
+                   "old_library=''\n"))))))
+      #:configure-flags
+      #~(list "--enable-manpages"
+              "--sysconfdir=/etc")))
+    (native-search-paths
+      (list (search-path-specification
+              (variable "UDEV_HWDB_PATH")
+              (files '("lib/udev/hwdb.d")))))
     (native-inputs
      (list autoconf
            automake
diff --git a/gnu/packages/patches/eudev-rules-directory.patch b/gnu/packages/patches/eudev-rules-directory.patch
index 54fc01c6d5..c4b1cfae39 100644
--- a/gnu/packages/patches/eudev-rules-directory.patch
+++ b/gnu/packages/patches/eudev-rules-directory.patch
@@ -4,9 +4,9 @@  The old udev 182 supported $UDEV_CONFIG_FILE, which in turn allowed
 the search path to be customized, but eudev no longer has this, hence
 this hack.
 
---- eudev-3.1.5/src/udev/udev-rules.c	2015-10-13 06:22:14.000000000 +0800
-+++ eudev-3.1.5/src/udev/udev-rules.c	2015-10-16 20:45:38.491934336 +0800
-@@ -47,15 +47,11 @@
+--- a/src/udev/udev-rules.c
++++ b/src/udev/udev-rules.c
+@@ -48,16 +48,11 @@ struct uid_gid {
          };
  };
  
@@ -20,11 +20,12 @@  this hack.
 -        "/lib/udev/rules.d",
 -        "/usr/lib/udev/rules.d",
 -#endif
+-        "/usr/local/lib/udev/rules.d",
 +        NULL,			/* placeholder for $EUDEV_RULES_DIRECTORY */
          NULL};
  
  struct udev_rules {
-@@ -1704,6 +1700,9 @@
+@@ -1718,6 +1713,9 @@ struct udev_rules *udev_rules_new(struct udev *udev, int resolve_names) {
  
          udev_rules_check_timestamp(rules);