GNU bug report logs - #66099
[PATCH gnome-team 0/3] Update upower

Previous Next

Full log

View this message in rfc822 format

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Vivien Kraus <vivien <at> planete-kraus.eu>
Cc: 66099 <at> debbugs.gnu.org, Liliana Marie Prikler <liliana.prikler <at> gmail.com>, rg <at> raghavgururajan.name
Subject: [bug#66099] [PATCH gnome-team v3 1/3] gnu: eudev: Update libudev version to 251.
Date: Sun, 01 Oct 2023 18:02:51 -0400

Hi,

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

> Support for version 251 is only provided as a merge request for now:
>
> https://github.com/eudev-project/eudev/pull/253
>
> This merge request bumps the eudev version to 3.2.14, but it has not been
> released yet.
>
> Eudev now has a hardware database that installs descriptions of hardware in
> /etc, but they should go to /lib prior to being used. I added a phase to copy
> all the /etc files to /lib.
>
> I submitted 3 patches to make udev hwdb more guix-friendly, fused in
> eudev-hwdb-bin-path.patch. libudev requires an indexed binary file that knows
> about all devices, and this file is generated by udevadm hwdb --update -o
> <prefix>/lib/udev/hwdb.bin. This udevadm hwdb command respects the
> UDEV_HWDB_PATH to collect all entries for the hwdb. Then, libudev can use
> UDEV_HWDB_BIN to select the database at run-time.
>
> Another unreleased patch for PR 255 is also included.
>
> I think everything could work out for Guix if we add a new profile hook that:
> 1. Calls udevadm hwdb --update -o <profile>/lib/udev/hwdb.bin;
> 2. Set UDEV_HWDB_BIN to <profile>/lib/udev/hwdb.bin.
>
> This is why there are actually 2 search paths: one for UDEV_HWDB_PATH, and one
> for UDEV_HWDB_BIN (the latter accepting only 1 value).
>
> * gnu/packages/linux.scm (eudev): Update to v3.2.12, but bump version to
> 3.2.14.beta.
> [#:phases] <allow-eudev-hwdb>: New phase.
> <build-hwdb>: Update accordingly.
> [native-search-paths]: Add UDEV_HWDB_PATH.
> * gnu/packages/patches/eudev-bump-to-251.patch: New file.
> * gnu/packages/patches/eudev-hwdb-bin-path.patch: New file.
> * gnu/packages/patches/eudev-pr-255.patch: New file.
> * gnu/packages/linux.scm (eudev): Use them here.
> * gnu/local.mk (dist_patch_DATA): Register them here.
> * guix/profiles.scm (udev-hwdb-bin): New profile hook to generate hwdb.bin.
> (%default-profile-hooks): Register it here.

Impressive work!  It seems your patches were warmly welcomed (already
merged!) in eudev, which is excellent (and should mean a v4 doesn't need
patches anymore, using a specific commit with a git-version computed
version.

[...]

>      (build-system gnu-build-system)
>      (arguments
>       (list
> @@ -4297,18 +4300,25 @@ (define-public eudev
>                  (substitute* (string-append #$output "/lib/libudev.la")
>                    (("old_library=.*")
>                     "old_library=''\n")))))
> -          (add-after 'install 'build-hwdb
> +          (add-after 'install 'allow-eudev-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"))))))
> +              ;; eudev distributes the hwdb, but each file has to be enabled
> +              ;; by being copied under /lib/udev/hwdb.d. We accept all of
> +              ;; them.
> +              (symlink (string-append #$output "/etc/udev/hwdb.d")
> +                       (string-append #$output "/lib/udev/hwdb.d")))))
>         #:configure-flags #~(list "--enable-manpages")))
> +    (native-search-paths
> +      (list (search-path-specification
> +              (variable "UDEV_HWDB_PATH")
> +              (files '("lib/udev/hwdb.d")))
> +            ;; Never install a hwdb.bin file, always let the udev-hwdb-bin
> +            ;; profile hook generate it.
> +            (search-path-specification
> +              (variable "UDEV_HWDB_BIN")
> +              (files '("lib/udev/hwdb.bin"))
> +              (file-type 'regular)
> +              (separator #f))))

Maybe add a ;singled valued comment to the end of the above line.  Note
that given that file is to be generated in a profile hook, UDEV_HWDB_BIN
would never be set at *build* time, but that should not matter
much (the build environment being containerized and all).)

[...]

> diff --git a/guix/profiles.scm b/guix/profiles.scm
> index c88672c25a..d308e7fb88 100644
> --- a/guix/profiles.scm
> +++ b/guix/profiles.scm
> @@ -1905,6 +1905,34 @@ (define (texlive-font-maps manifest)
>                              (hook . texlive-font-maps)))
>          (return #f))))
>  
> +(define (udev-hwdb-bin manifest)
> +  "Return a derivation that builds lib/udev/hwdb.bin."
                                      ^
                                      @file{lib/udev/hwdb.bin}

> +  (define eudev
> +    (module-ref (resolve-interface '(gnu packages linux)) 'eudev))
> +  (define build
> +    (with-imported-modules
> +        (source-module-closure '((guix build utils)))
> +      #~(begin
> +          (use-modules (guix build utils))
> +          (let* ((inputs '#$(manifest-inputs manifest))
> +                 (hwdb-directories
> +                  (filter
> +                   file-exists?
> +                   (map (lambda (directory)
> +                          (string-append directory "/lib/udev/hwdb.d"))
> +                        inputs))))
> +            (setenv "UDEV_HWDB_PATH"
> +                    (string-join hwdb-directories ":")))

It'd be nicer to user 'evaluate-search-paths' with the UDEV_HWDB_PATH
search path.

> +          (invoke #$(file-append eudev "/bin/udevadm")
> +                  "hwdb" "--update"
> +                  "-o" (string-append #$output "/lib/udev/hwdb.bin")))))
> +  (gexp->derivation "udev-hwdb-bin" build
> +                    #:substitutable? #f
> +                    #:local-build? #t
> +                    #:properties
> +                    `((type . profile-hook)
> +                      (hook . udev-hwdb-bin))))
>
>  (define %default-profile-hooks
>    ;; This is the list of derivation-returning procedures that are called by
>    ;; default when making a non-empty profile.
> @@ -1919,6 +1947,7 @@ (define %default-profile-hooks
>          gtk-icon-themes
>          gtk-im-modules
>          texlive-font-maps
> +        udev-hwdb-bin
>          xdg-desktop-database
>          xdg-mime-database))

My only issue with this hook is that it introduces 'eudev' as a hard
dependency for generating guix profiles.  Would it make sense to follow
the approach used in 'gdk-pixbuf-loaders-cache-file', which is to look
if eudev is already part of the profile inputs (transitively), and skip
the hook if it isn't?

-- 
Thanks,
Maxim

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.