[PATCH] gnu: eudev: Look for rules in /etc/udev/rules.d

  • Open
  • quality assurance status badge
Details
4 participants
  • Felix Lechner
  • Liliana Marie Prikler
  • Maxim Cournoyer
  • Bruno Victal
Owner
unassigned
Submitted by
Felix Lechner
Severity
normal

Debbugs page

Felix Lechner wrote 2 years ago
[PATCH 0/3] Use MAC-based names for network interfaces
(address . guix-patches@gnu.org)(name . Felix Lechner)(address . felix.lechner@lease-up.com)
cover.1684100044.git.felix.lechner@lease-up.com
Hi,

This patch set proposes the use of ID_NET_NAME_MAC for standard network
interfaces. In the author's opinion, predictable names for interfaces work
better with the declarative configuration style used in Guix.

Kind regards
Felix

Felix Lechner (3):
gnu: eudev: Convert native-inputs to new style.
gnu: eudev: Convert build arguments to gexps.
gnu: eudev: Always use MAC-based names for network interfaces.

gnu/packages/linux.scm | 100 +++++++++++++++++++++--------------------
1 file changed, 51 insertions(+), 49 deletions(-)


base-commit: 263f235cd0a2955e865fe38036f84c2bf34375ff
--
2.40.1
Felix Lechner wrote 2 years ago
[PATCH 1/3] gnu: eudev: Convert native-inputs to new style.
(address . 63508@debbugs.gnu.org)(name . Felix Lechner)(address . felix.lechner@lease-up.com)
efe9ecb3d2c22fe20c82dd92dceaf992ef954b04.1684100044.git.felix.lechner@lease-up.com
* gnu/packages/linux.scm (eudev): Convert native-inputs to new style.
---
gnu/packages/linux.scm | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)

Toggle diff (39 lines)
diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index ea64e9d241..7ae34d1d4a 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -4190,19 +4190,19 @@ (define-public eudev
"hwdb" "--update")))))))
#:configure-flags (list "--enable-manpages")))
(native-inputs
- `(("autoconf" ,autoconf)
- ("automake" ,automake)
- ("gperf" ,gperf)
- ("libtool" ,libtool)
- ("pkg-config" ,pkg-config)
- ;; For tests.
- ("perl" ,perl)
- ("python" ,python-wrapper)
- ;; For documentation.
- ("docbook-xml" ,docbook-xml-4.2)
- ("docbook-xsl" ,docbook-xsl)
- ("libxml2" ,libxml2) ;for $XML_CATALOG_FILES
- ("xsltproc" ,libxslt)))
+ (list autoconf
+ automake
+ gperf
+ libtool
+ pkg-config
+ ;; For tests.
+ perl
+ python-wrapper
+ ;; For documentation.
+ docbook-xml-4.2
+ docbook-xsl
+ libxml2 ;for $XML_CATALOG_FILES
+ libxslt))
(inputs
;; When linked against libblkid, eudev can populate /dev/disk/by-label
;; and similar; it also installs the '60-persistent-storage.rules' file,
--
2.40.1
Felix Lechner wrote 2 years ago
[PATCH 2/3] gnu: eudev: Convert build arguments to gexps.
(address . 63508@debbugs.gnu.org)(name . Felix Lechner)(address . felix.lechner@lease-up.com)
85af3647b4729638ce33464c835095edc6e208bd.1684100044.git.felix.lechner@lease-up.com
* gnu/packages/linux.scm (eudev): Convert build arguments to gexps.
---
gnu/packages/linux.scm | 68 ++++++++++++++++++++----------------------
1 file changed, 32 insertions(+), 36 deletions(-)

Toggle diff (81 lines)
diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index 7ae34d1d4a..98e683bdb0 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -4153,42 +4153,38 @@ (define-public eudev
(patches (search-patches "eudev-rules-directory.patch"))))
(build-system gnu-build-system)
(arguments
- `(#:phases
- (modify-phases %standard-phases
- (add-before 'bootstrap 'patch-file-names
- (lambda* (#:key inputs native-inputs #:allow-other-keys)
- (substitute* "man/make.sh"
- (("/usr/bin/xsltproc")
- (string-append (assoc-ref
- (or native-inputs inputs) "xsltproc")
- "/bin/xsltproc")))))
- (add-after 'install 'move-static-library
- (lambda* (#:key outputs #:allow-other-keys)
- (let* ((out (assoc-ref outputs "out"))
- (static (assoc-ref outputs "static"))
- (source (string-append out "/lib/libudev.a"))
- (target (string-append static "/lib/libudev.a")))
- (mkdir-p (dirname target))
- (link source target)
- (delete-file source)
- ;; Remove reference to the static library from the .la file
- ;; such that Libtool looks for it in the usual places.
- (substitute* (string-append out "/lib/libudev.la")
- (("old_library=.*")
- "old_library=''\n")))))
- (add-after 'install 'build-hwdb
- (lambda* (#:key outputs #:allow-other-keys)
- ;; 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.
- (let ((out (assoc-ref outputs "out")))
- ,@(if (%current-target-system)
- '(#t)
- '((invoke (string-append out "/bin/udevadm")
- "hwdb" "--update")))))))
- #:configure-flags (list "--enable-manpages")))
+ (list
+ #:phases
+ #~(modify-phases %standard-phases
+ (add-before 'bootstrap 'patch-file-names
+ (lambda* (#:key inputs native-inputs #:allow-other-keys)
+ (substitute* "man/make.sh"
+ (("/usr/bin/xsltproc")
+ (search-input-file (or native-inputs inputs) "/bin/xsltproc")))))
+ (add-after 'install 'move-static-library
+ (lambda _
+ (let ((source (string-append #$output "/lib/libudev.a"))
+ (target (string-append #$output:static "/lib/libudev.a")))
+ (mkdir-p (dirname target))
+ (link source target)
+ (delete-file source)
+ ;; Remove reference to the static library from the .la file
+ ;; 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")))
(native-inputs
(list autoconf
automake
--
2.40.1
Felix Lechner wrote 2 years ago
[PATCH 3/3] gnu: eudev: Always use MAC-based names for network interfaces.
(address . 63508@debbugs.gnu.org)(name . Felix Lechner)(address . felix.lechner@lease-up.com)
89b635e974a7d570cbd19b847fc3eb8ac903103b.1684100044.git.felix.lechner@lease-up.com
Upon personal reflection, a declarative operating system like Guix probably
ought to use only predictable interface names.

While shorter names like 'eno1' offer an indisputable convenience and beauty
when typing on the command line, administrators in Guix are unlikely to do so
due to the declarative configuration system.

Some system services may explicitly refer to interface names in their
configuration. They would also benefit from the predictable and constant
nature of MAC-based names.

The latter is particularly relevant on multi-homed machines, i.e. those with
more than one network connection.

A MAC-based interface name as issued by 'eudev' looks like this:

enx0123456789af (fictitious)

This commit was deployed on two production machines. The migration to
MAC-based interface names took place without issues. A second reconfiguration
was the used to add the new interface name in services tha needed it. The
second step can be skipped, since the name is known with certainty in advance.

The current naming scheme is less desirable because some services may silently
refuse to start after equipment was added or removed. A removal may take
place, for example, when something broke or when equipment was sold.

The device enumeration may also change when a CMOS battery fails and system
options are lost. In the author's option, Guix should not depend on BIOS
enumeration for device names.

In the author's case, the name of the sole network interface changed from
enp3s0 to enp4s0 when a PCIe disk controller (a SAS host-based adapter) was
installed. As a result, OpenSMTPd silently failed to start.

This commit switches 'eudev' from the standard naming order

ID_NET_NAME_ONBOARD
ID_NET_NAME_SLOT
ID_NET_NAME_PATH

to ID_NET_NAME_MAC, which is always available. [1]

The author initially attempted to achieve the same result via

(udev-rules-service 'net-name-mac
(udev-rule
"01-net-name-mac.rules"
"SUBSYSTEM==\"net\", ACTION==\"add\", NAME=\"$env{ID_NET_NAME_MAC}\"
")))

but that did not work. While the situation was not examined exhaustively, it
was not clear that udevadm can currently work because the standard command to
test udev setups: [2]

$ udevadm --debug test /sys/class/net/*

did not find the script installed via the 'udev-service-type'.

A review of the 'eudev' sources indicated that the path to find rules [3] is
hard-coded to the store location during installation. An attempt to set the
path to /etc/udev/rules.d yielded a build error because that target folder
outside the store was understandably not writable.

The manual page for udevadm did not offer a way to select the runtime location
of the udev/rules.d folder via environment variables or a command-line option.

Anyone for whom such a setup is working properly should please contact the
author. Thank you!

This commit may result in some loss of privacy, although it is presently not
clear how meaningful that is. With this commit, anyone using privacy-enhanced
IPv6 addresses risks having their MAC exposed when they publish their
configuration files in Git or post a well-meant sample in a chat rooms,
because that configuration may mention the MAC address.

Moreover, the compatibilty with schemes to generate fake one-time MAC
addresses upon boot should be evaluated. One concern is that the explicit
reference to a network interface in a configuration file would likely force
the use of a single and constant MAC address for that interface.

This commit was tested in production and is currently being used.

The change here resulted in the recompilation of several seemingly unrelated
packages such as Emacs and GTK. Perhaps those dependency relationships should
be examined.


* gnu/packages/linux.scm (eudev): Always use MAC-based names for network
interfaces.
---
gnu/packages/linux.scm | 6 ++++++
1 file changed, 6 insertions(+)

Toggle diff (19 lines)
diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index 98e683bdb0..724c621fed 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -4173,6 +4173,12 @@ (define-public eudev
(substitute* (string-append #$output "/lib/libudev.la")
(("old_library=.*")
"old_library=''\n")))))
+ (add-before 'install 'net-name-mac
+ (lambda _
+ (use-modules (ice-9 regex))
+ (substitute* "rules/80-net-name-slot.rules"
+ (((regexp-quote "ID_NET_NAME_ONBOARD"))
+ "ID_NET_NAME_MAC"))))
(add-after 'install 'build-hwdb
(lambda _
;; Build OUT/etc/udev/hwdb.bin. This allows 'lsusb' and
--
2.40.1
Liliana Marie Prikler wrote 2 years ago
9f3182a690a147e440cef8dc414365730383172d.camel@gmail.com
Am Sonntag, dem 14.05.2023 um 14:42 -0700 schrieb Felix Lechner:
Toggle quote (3 lines)
> The change here resulted in the recompilation of several seemingly
> unrelated packages such as Emacs and GTK. Perhaps those dependency
> relationships should be examined.
What? GUI libraries and packages that depend on them need to be
rebuilt when we change the way we handle our (input, media) devices?!
Can't have that!

Toggle quote (17 lines)
>  gnu/packages/linux.scm | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
> index 98e683bdb0..724c621fed 100644
> --- a/gnu/packages/linux.scm
> +++ b/gnu/packages/linux.scm
> @@ -4173,6 +4173,12 @@ (define-public eudev
>                  (substitute* (string-append #$output
> "/lib/libudev.la")
>                    (("old_library=.*")
>                     "old_library=''\n")))))
> +          (add-before 'install 'net-name-mac
> +            (lambda _
> +              (use-modules (ice-9 regex))
> +              (substitute* "rules/80-net-name-slot.rules"
> +                (((regexp-quote "ID_NET_NAME_ONBOARD"))
Wherefore the regexp-quote? There is no regexp to be found here, is
there?
Toggle quote (1 lines)
> +                 "ID_NET_NAME_MAC"))))
I don't see how this change allows users *or upstream package
maintainers* to continue using onboard names as they have done for ages
and as they would want to continue to do. I think you should
a) File a patch upstream to add ID_NET_NAME_MAC into net-name-
slot.rules
b) Add that patch to our eudev package with a reference to the upstream
bug report.

Cheers
Felix Lechner wrote 2 years ago
(name . Liliana Marie Prikler)(address . liliana.prikler@gmail.com)(address . 63508@debbugs.gnu.org)
CAFHYt55dZUXX0MkXr32216foTxJvJD-49_aJ0nYdj5Ki2PVb6w@mail.gmail.com
Hi Liliana,

Thank you for your review!

On Sun, May 14, 2023 at 9:31 PM Liliana Marie Prikler
<liliana.prikler@gmail.com> wrote:
Toggle quote (3 lines)
>
> Wherefore the regexp-quote?

Whoops! That was left over from the attempt to get udevadm to use
/etc/udev/rules.d as it should. Thanks for the pointer!

Toggle quote (4 lines)
> I don't see how this change allows users *or upstream package
> maintainers* to continue using onboard names as they have done for ages
> and as they would want to continue to do.

I have one of those cards. While the interface name did not change
when I fiddled with the PCI configuration, I am not sure that
prioritizing ID_NET_NAME_ONBOARD over ID_NET_NAME_MAC is a reasonable
default for Guix.

To rank ID_NET_NAME_ONBOARD below ID_NET_NAME_MAC would not address
the shortcoming you perceived because the latter always exists.

Instead, I think people wishing to use ID_NET_NAME_ONBOARD should
install a custom udev script (and those should be recognized by the
udevadm we ship).

Toggle quote (3 lines)
> a) File a patch upstream to add ID_NET_NAME_MAC into net-name-
> slot.rules

If upstream accepts such a patch, I believe they would give
ID_NET_NAME_MAC the lowest possible priority, but that does nothing
for Guix. Common relative priorities are outlined here. [1]

For Guix, I think we would like to see ID_NET_NAME_MAC at the top.

Kind regards
Felix

Liliana Marie Prikler wrote 2 years ago
[PATCH] gnu: udev: Allow EUDEV_RULES_DIRECTORY to shadow built-in rules.
(address . 63508@debbugs.gnu.org)(address . felix.lechner@lease-up.com)
06ea6673ca13ed6bc7fb00336dafc7a3457412ee.1684178049.git.liliana.prikler@gmail.com
* gnu/packages/patches/eudev-rules-directory.patch (rules_dirs):
Move placeholder to the start of the array.
(rules_dirs_real): New procedure.
(udev_rules_dirs_new, udev_rules_check_timestamp): Adjust accordingly.
---
Hi Felix,

Am Sonntag, dem 14.05.2023 um 21:56 -0700 schrieb Felix Lechner:
Toggle quote (15 lines)
> > I don't see how this change allows users *or upstream package
> > maintainers* to continue using onboard names as they have done for
> > ages and as they would want to continue to do.
>
> I have one of those cards. While the interface name did not change
> when I fiddled with the PCI configuration, I am not sure that
> prioritizing ID_NET_NAME_ONBOARD over ID_NET_NAME_MAC is a reasonable
> default for Guix.
>
> To rank ID_NET_NAME_ONBOARD below ID_NET_NAME_MAC would not address
> the shortcoming you perceived because the latter always exists.
>
> Instead, I think people wishing to use ID_NET_NAME_ONBOARD should
> install a custom udev script (and those should be recognized by the
> udevadm we ship).
I think the current default is probably fine for more users than the
proposed change (ain't no one got the time to type their MAC addresses).

I do however see your point in that udev should let you choose to prefer
ID_NET_NAME_MAC over the other rules. Now, the shortcoming here
actually lies with our incomplete support for EUDEV_RULES_DIRECTORY,
see the patch :)

Toggle quote (1 lines)
> For Guix, I think we would like to see ID_NET_NAME_MAC at the top.
Now, I respectully disagree on that proposition, but am here to fix the
original bug of udev not honouring your preference. With the following
patch your udev-rule-service should be able to override the default
behaviour. If not, try matching the file name. There shouldn't be any
weird predicates on the name, but if there are, that's how we'll find
out.

Cheers

.../patches/eudev-rules-directory.patch | 44 ++++++++++++++++---
1 file changed, 37 insertions(+), 7 deletions(-)

Toggle diff (78 lines)
diff --git a/gnu/packages/patches/eudev-rules-directory.patch b/gnu/packages/patches/eudev-rules-directory.patch
index 54fc01c6d5..7cc3f97451 100644
--- a/gnu/packages/patches/eudev-rules-directory.patch
+++ b/gnu/packages/patches/eudev-rules-directory.patch
@@ -4,14 +4,17 @@ 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 @@
+Index: eudev/src/udev/udev-rules.c
+===================================================================
+--- eudev.orig/src/udev/udev-rules.c
++++ eudev/src/udev/udev-rules.c
+@@ -48,15 +48,11 @@ struct uid_gid {
};
};
-static const char* const rules_dirs[] = {
+static const char* rules_dirs[] = {
++ NULL, /* placeholder for $EUDEV_RULES_DIRECTORY */
UDEV_CONF_DIR "/rules.d",
UDEV_RULES_DIR,
- UDEV_ROOT_RUN "/udev/rules.d",
@@ -20,17 +23,44 @@ this hack.
- "/lib/udev/rules.d",
- "/usr/lib/udev/rules.d",
-#endif
-+ NULL, /* placeholder for $EUDEV_RULES_DIRECTORY */
NULL};
struct udev_rules {
-@@ -1704,6 +1700,9 @@
+@@ -1691,6 +1687,14 @@ static int parse_file(struct udev_rules
+ return 0;
+ }
+
++static const char** rules_dirs_real()
++{
++ if (rules_dirs[0])
++ return rules_dirs;
++ else
++ return rules_dirs + 1;
++}
++
+ struct udev_rules *udev_rules_new(struct udev *udev, int resolve_names) {
+ struct udev_rules *rules;
+ struct udev_list file_list;
+@@ -1717,7 +1721,10 @@ struct udev_rules *udev_rules_new(struct
udev_rules_check_timestamp(rules);
+- r = conf_files_list_strv(&files, ".rules", NULL, rules_dirs);
+ /* Allow the user to specify an additional rules directory. */
-+ rules_dirs[3] = getenv("EUDEV_RULES_DIRECTORY");
++ rules_dirs[0] = getenv("EUDEV_RULES_DIRECTORY");
+
- r = conf_files_list_strv(&files, ".rules", NULL, rules_dirs);
++ r = conf_files_list_strv(&files, ".rules", NULL, rules_dirs_real ());
if (r < 0) {
log_error_errno(r, "failed to enumerate rules files: %m");
+ return udev_rules_unref(rules);
+@@ -1776,7 +1783,9 @@ bool udev_rules_check_timestamp(struct u
+ if (!rules)
+ return false;
+
+- return paths_check_timestamp(rules_dirs, &rules->dirs_ts_usec, true);
++ return paths_check_timestamp(rules_dirs_real (),
++ &rules->dirs_ts_usec,
++ true);
+ }
+
+ static int match_key(struct udev_rules *rules, struct token *token, const char *val) {

base-commit: 28bfc5cd081458313fa8601133386209b23deb12
--
2.40.1
Felix Lechner wrote 2 years ago
(address . control@debbugs.gnu.org)
CAFHYt57FVZuo3ePxXAGqpcFCs23V3BFHnxuGTV77W+QhH+N6fA@mail.gmail.com
retitle 63508 [PATCH 0/3] Have udevadm look in /etc/udev/rules.d
thanks
Felix Lechner wrote 2 years ago
(address . control@debbugs.gnu.org)
CAFHYt54zEgJ_-4w8=MmB7p=bq0dinoHVrOgiUVqw4Pcj7eysqA@mail.gmail.com
retitle 63508 [PATCH v2 0/4] Have udevadm look in /etc/udev/rules.d
thanks
Felix Lechner wrote 2 years ago
(address . 63508@debbugs.gnu.org)
29b8c15130a15487142ecf2089cba2a76ee8cb6d.1684370595.git.felix.lechner@lease-up.com
* gnu/packages/linux.scm (eudev): Convert native-inputs to new style.
---
gnu/packages/linux.scm | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)

Toggle diff (41 lines)
diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index 90c1adde53..1f1b319dbf 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -4190,19 +4190,19 @@ (define-public eudev
"hwdb" "--update")))))))
#:configure-flags (list "--enable-manpages")))
(native-inputs
- `(("autoconf" ,autoconf)
- ("automake" ,automake)
- ("gperf" ,gperf)
- ("libtool" ,libtool)
- ("pkg-config" ,pkg-config)
- ;; For tests.
- ("perl" ,perl)
- ("python" ,python-wrapper)
- ;; For documentation.
- ("docbook-xml" ,docbook-xml-4.2)
- ("docbook-xsl" ,docbook-xsl)
- ("libxml2" ,libxml2) ;for $XML_CATALOG_FILES
- ("xsltproc" ,libxslt)))
+ (list autoconf
+ automake
+ gperf
+ libtool
+ pkg-config
+ ;; For tests.
+ perl
+ python-wrapper
+ ;; For documentation.
+ docbook-xml-4.2
+ docbook-xsl
+ libxml2 ;for $XML_CATALOG_FILES
+ libxslt))
(inputs
;; When linked against libblkid, eudev can populate /dev/disk/by-label
;; and similar; it also installs the '60-persistent-storage.rules' file,

base-commit: 6e38ec447f98383e0722ac300734f8d7c8c5c7b0
--
2.40.1
Felix Lechner wrote 2 years ago
[PATCH v2 2/4] gnu: eudev: Convert build arguments to gexps.
(address . 63508@debbugs.gnu.org)
b0a81891c6d68a0fb0cd0c6fdebb3f522f8a2379.1684370595.git.felix.lechner@lease-up.com
* gnu/packages/linux.scm (eudev): Convert build arguments to gexps.
---
gnu/packages/linux.scm | 68 ++++++++++++++++++++----------------------
1 file changed, 32 insertions(+), 36 deletions(-)

Toggle diff (81 lines)
diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index 1f1b319dbf..9058648700 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -4153,42 +4153,38 @@ (define-public eudev
(patches (search-patches "eudev-rules-directory.patch"))))
(build-system gnu-build-system)
(arguments
- `(#:phases
- (modify-phases %standard-phases
- (add-before 'bootstrap 'patch-file-names
- (lambda* (#:key inputs native-inputs #:allow-other-keys)
- (substitute* "man/make.sh"
- (("/usr/bin/xsltproc")
- (string-append (assoc-ref
- (or native-inputs inputs) "xsltproc")
- "/bin/xsltproc")))))
- (add-after 'install 'move-static-library
- (lambda* (#:key outputs #:allow-other-keys)
- (let* ((out (assoc-ref outputs "out"))
- (static (assoc-ref outputs "static"))
- (source (string-append out "/lib/libudev.a"))
- (target (string-append static "/lib/libudev.a")))
- (mkdir-p (dirname target))
- (link source target)
- (delete-file source)
- ;; Remove reference to the static library from the .la file
- ;; such that Libtool looks for it in the usual places.
- (substitute* (string-append out "/lib/libudev.la")
- (("old_library=.*")
- "old_library=''\n")))))
- (add-after 'install 'build-hwdb
- (lambda* (#:key outputs #:allow-other-keys)
- ;; 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.
- (let ((out (assoc-ref outputs "out")))
- ,@(if (%current-target-system)
- '(#t)
- '((invoke (string-append out "/bin/udevadm")
- "hwdb" "--update")))))))
- #:configure-flags (list "--enable-manpages")))
+ (list
+ #:phases
+ #~(modify-phases %standard-phases
+ (add-before 'bootstrap 'patch-file-names
+ (lambda* (#:key inputs native-inputs #:allow-other-keys)
+ (substitute* "man/make.sh"
+ (("/usr/bin/xsltproc")
+ (search-input-file (or native-inputs inputs) "/bin/xsltproc")))))
+ (add-after 'install 'move-static-library
+ (lambda _
+ (let ((source (string-append #$output "/lib/libudev.a"))
+ (target (string-append #$output:static "/lib/libudev.a")))
+ (mkdir-p (dirname target))
+ (link source target)
+ (delete-file source)
+ ;; Remove reference to the static library from the .la file
+ ;; 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")))
(native-inputs
(list autoconf
automake
--
2.40.1
Felix Lechner wrote 2 years ago
[PATCH v2 3/4] gnu: eudev: Use new project URL.
(address . 63508@debbugs.gnu.org)
771fb5f5b89e13b43d3efe9c455410f7e712a600.1684370595.git.felix.lechner@lease-up.com
* gnu/packages/linux.scm (eudev): Use new project URL.
---
gnu/packages/linux.scm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Toggle diff (15 lines)
diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index 9058648700..7b989a466c 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -4144,7 +4144,7 @@ (define-public eudev
(version "3.2.11")
(source (origin
(method git-fetch)
- (uri (git-reference (url "https://github.com/gentoo/eudev")
+ (uri (git-reference (url "https://github.com/eudev-project/eudev")
(commit (string-append "v" version))))
(file-name (git-file-name name version))
(sha256
--
2.40.1
Felix Lechner wrote 2 years ago
[PATCH v2 4/4] gnu: eudev: Have udevadm look in /etc/udev/rules.d. (Closes: #63508)
(address . 63508@debbugs.gnu.org)
e7aecfbe4642b43ea5837584b7666c87098ef15f.1684370595.git.felix.lechner@lease-up.com
Note for Liliana (and not part of the commit message): Hi, I hope you
are not offended by this patch. The one-line substitution here makes
the custom rule work, as well. The enviroment variable you proposed is
probably superior but the patch is relatively complex and the
resulting flexibility may not be needed. Also, I retitled the bug to
sidestep the controversy around the default for now. I was surprised
by your opposition and think that should be a separate
discussion. Thanks!

This substitution ensures that udevadm sees the rules that are actually in
effect for the declared operating system. It allows administrators to use the
udev-rules-service for network interfaces.

Some of Guix's customizations for udev rules appear to work as it is [1] but
that is not true for network interfaces (which invoke udevadm for naming
purposes). [2]

The author uses this snippet to select MAC-based names for all network
interfaces:

(udev-rules-service 'net-name-mac
(udev-rule
"79-net-name-mac.rules"
"
SUBSYSTEM==\"net\", ACTION==\"add\", NAME=\"$env{ID_NET_NAME_MAC}\"
")))

Without this commit, udevadm will consult the rules that were present at build
time and were installed in the store).


* gnu/packages/linux.scm (eudev): Have udevadm look in
/etc/udev/rules.d. (Closes: #63508)
---
gnu/packages/linux.scm | 5 +++++
1 file changed, 5 insertions(+)

Toggle diff (18 lines)
diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index 7b989a466c..750016d572 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -4156,6 +4156,11 @@ (define-public eudev
(list
#:phases
#~(modify-phases %standard-phases
+ (add-before 'bootstrap 'hardcode-runtime-rules-dir
+ (lambda _
+ (use-modules (ice-9 regex))
+ (substitute* "src/udev/Makefile.am"
+ (((regexp-quote "$(udevrulesdir)")) "/etc/udev/rules.d"))))
(add-before 'bootstrap 'patch-file-names
(lambda* (#:key inputs native-inputs #:allow-other-keys)
(substitute* "man/make.sh"
--
2.40.1
Liliana Marie Prikler wrote 2 years ago
Re: [PATCH v2 2/4] gnu: eudev: Convert build arguments to gexps.
832ba582fd3da735ca64160c64d8bea843287076.camel@gmail.com
Am Mittwoch, dem 17.05.2023 um 17:52 -0700 schrieb Felix Lechner:
Toggle quote (2 lines)
> * gnu/packages/linux.scm (eudev): Convert build arguments to gexps.
> ---
Since both this and 1/4 are style changes, they should be squashed into
a single commit named "gnu: eudev: Use new package style".

Cheers
Liliana Marie Prikler wrote 2 years ago
Re: [PATCH v2 4/4] gnu: eudev: Have udevadm look in /etc/udev/rules.d. (Closes: #63508)
eeecf052fb86351d916c91f415dfe343069cdad2.camel@gmail.com
Am Mittwoch, dem 17.05.2023 um 17:52 -0700 schrieb Felix Lechner:
Toggle quote (1 lines)
> Note for Liliana (and not part of the commit message):
Stuff that isn't part of the commit message ought to go below the
dashed (---) line so it's automatically ignored by git.

Toggle quote (7 lines)
> Hi, I hope you are not offended by this patch. The one-line
> substitution here makes the custom rule work, as well. The enviroment
> variable you proposed is probably superior but the patch is
> relatively complex and the resulting flexibility may not be needed.
> Also, I retitled the bug to sidestep the controversy around the
> default for now. I was surprised by your opposition and think that
> should be a separate discussion. Thanks!
Note that our udev already uses this environment variable, I am only
changing how it is interpreted, i.e. allowing it to override built-in
rules just as is needed for your use case. Now, you may object that
this doesn't mention /etc/udev/rules.d and thus could be problematic on
foreign distributions, but I argue that you probably shouldn't mess
with foreign udev anyway, and if you do that setting
EUDEV_RULES_DIRECTORY is appropriate.


Cheers
Felix Lechner wrote 2 years ago
(name . Liliana Marie Prikler)(address . liliana.prikler@gmail.com)(address . 63508@debbugs.gnu.org)
CAFHYt5554wnWZrL9H809xVj172Sd0_gB+QTpYvmWVKz1aoFA9w@mail.gmail.com
Hi Liliana,

Thank you for your kind review! I will push a new version with
squashed commits you requested shortly.

While I am new to Guix, I am not sure that Gexp conversions fall under
"style changes" in my book. I believe they are considerably more
complex, and fraught with greater error.

On Wed, May 17, 2023 at 9:19 PM Liliana Marie Prikler
<liliana.prikler@gmail.com> wrote:
Toggle quote (7 lines)
>
> you may object that
> this doesn't mention /etc/udev/rules.d and thus could be problematic on
> foreign distributions, but I argue that you probably shouldn't mess
> with foreign udev anyway, and if you do that setting
> EUDEV_RULES_DIRECTORY is appropriate.

The intent of my patch was not to mention /etc/udev/rules.d
explicitly, but rather to replace the store folder that holds the
upstream rules, which we are currently using, with the one Guix
constructs in order to use rules from other places. That just happens
to be /etc/udev/rules.d as well.

On that note, my patch is not suitable for upstream because it
hardcodes the location to the runtime path in Guix. Other
distributions may keep them in a different place. The current Autoconf
setup probably works well for them.

Either way, udevadm in Guix is currently broken. This patch fixes it
and should please be accepted. Thanks!

As noted elsewhere [1] I am separately working on an update to eudev
3.2.12 but that will require more testing locally before I can send it
in.

Kind regards
Felix

Felix Lechner wrote 2 years ago
[PATCH v3 1/3] gnu: eudev: Convert native-inputs to new style, and build arguments to Gexps.
(address . 63508@debbugs.gnu.org)
475e3d003e5a1edf2d57e61466aa646db0ad7ebc.1685316502.git.felix.lechner@lease-up.com
* gnu/packages/linux.scm (eudev): Convert native-inputs to new style, and
build arguments to Gexps.
---
gnu/packages/linux.scm | 94 ++++++++++++++++++++----------------------
1 file changed, 45 insertions(+), 49 deletions(-)

Toggle diff (110 lines)
diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index 1be505d949..ae2792825b 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -4189,56 +4189,52 @@ (define-public eudev
(patches (search-patches "eudev-rules-directory.patch"))))
(build-system gnu-build-system)
(arguments
- `(#:phases
- (modify-phases %standard-phases
- (add-before 'bootstrap 'patch-file-names
- (lambda* (#:key inputs native-inputs #:allow-other-keys)
- (substitute* "man/make.sh"
- (("/usr/bin/xsltproc")
- (string-append (assoc-ref
- (or native-inputs inputs) "xsltproc")
- "/bin/xsltproc")))))
- (add-after 'install 'move-static-library
- (lambda* (#:key outputs #:allow-other-keys)
- (let* ((out (assoc-ref outputs "out"))
- (static (assoc-ref outputs "static"))
- (source (string-append out "/lib/libudev.a"))
- (target (string-append static "/lib/libudev.a")))
- (mkdir-p (dirname target))
- (link source target)
- (delete-file source)
- ;; Remove reference to the static library from the .la file
- ;; such that Libtool looks for it in the usual places.
- (substitute* (string-append out "/lib/libudev.la")
- (("old_library=.*")
- "old_library=''\n")))))
- (add-after 'install 'build-hwdb
- (lambda* (#:key outputs #:allow-other-keys)
- ;; 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.
- (let ((out (assoc-ref outputs "out")))
- ,@(if (%current-target-system)
- '(#t)
- '((invoke (string-append out "/bin/udevadm")
- "hwdb" "--update")))))))
- #:configure-flags (list "--enable-manpages")))
+ (list
+ #:phases
+ #~(modify-phases %standard-phases
+ (add-before 'bootstrap 'patch-file-names
+ (lambda* (#:key inputs native-inputs #:allow-other-keys)
+ (substitute* "man/make.sh"
+ (("/usr/bin/xsltproc")
+ (search-input-file (or native-inputs inputs) "/bin/xsltproc")))))
+ (add-after 'install 'move-static-library
+ (lambda _
+ (let ((source (string-append #$output "/lib/libudev.a"))
+ (target (string-append #$output:static "/lib/libudev.a")))
+ (mkdir-p (dirname target))
+ (link source target)
+ (delete-file source)
+ ;; Remove reference to the static library from the .la file
+ ;; 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")))
(native-inputs
- `(("autoconf" ,autoconf)
- ("automake" ,automake)
- ("gperf" ,gperf)
- ("libtool" ,libtool)
- ("pkg-config" ,pkg-config)
- ;; For tests.
- ("perl" ,perl)
- ("python" ,python-wrapper)
- ;; For documentation.
- ("docbook-xml" ,docbook-xml-4.2)
- ("docbook-xsl" ,docbook-xsl)
- ("libxml2" ,libxml2) ;for $XML_CATALOG_FILES
- ("xsltproc" ,libxslt)))
+ (list autoconf
+ automake
+ gperf
+ libtool
+ pkg-config
+ ;; For tests.
+ perl
+ python-wrapper
+ ;; For documentation.
+ docbook-xml-4.2
+ docbook-xsl
+ libxml2 ;for $XML_CATALOG_FILES
+ libxslt))
(inputs
;; When linked against libblkid, eudev can populate /dev/disk/by-label
;; and similar; it also installs the '60-persistent-storage.rules' file,

base-commit: d64d6ea2cf5a1be801be355031fb2cfa5901a92a
--
2.40.1
Felix Lechner wrote 2 years ago
[PATCH v3 2/3] gnu: eudev: Use new project URL.
(address . 63508@debbugs.gnu.org)
bdcbcd3cafa77ffc0e156fa840ecbe5b61be763d.1685316502.git.felix.lechner@lease-up.com
* gnu/packages/linux.scm (eudev): Use new project URL.
---
gnu/packages/linux.scm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Toggle diff (15 lines)
diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index ae2792825b..90a44a518d 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -4180,7 +4180,7 @@ (define-public eudev
(version "3.2.11")
(source (origin
(method git-fetch)
- (uri (git-reference (url "https://github.com/gentoo/eudev")
+ (uri (git-reference (url "https://github.com/eudev-project/eudev")
(commit (string-append "v" version))))
(file-name (git-file-name name version))
(sha256
--
2.40.1
Felix Lechner wrote 2 years ago
[PATCH v3 3/3] gnu: eudev: Have udevadm look in /etc/udev/rules.d. (Closes: #63508)
(address . 63508@debbugs.gnu.org)
b68c9fa5ca46052aaf1d888a6eaf704614f96dec.1685316502.git.felix.lechner@lease-up.com
This substitution ensures that udevadm sees the rules that are actually in
effect for the declared operating system. It allows administrators to use the
udev-rules-service for network interfaces.

Some of Guix's customizations for udev rules appear to work as it is [1] but
that is not true for network interfaces (which invoke udevadm for naming
purposes). [2]

Without this commit, udevadm will consult the rules that were present at build
time and were installed in the store).


* gnu/packages/linux.scm (eudev): Have udevadm look in
/etc/udev/rules.d. (Closes: #63508)
---
gnu/packages/linux.scm | 5 +++++
1 file changed, 5 insertions(+)

Toggle diff (18 lines)
diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index 90a44a518d..57b722d97d 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -4192,6 +4192,11 @@ (define-public eudev
(list
#:phases
#~(modify-phases %standard-phases
+ (add-before 'bootstrap 'hardcode-runtime-rules-dir
+ (lambda _
+ (use-modules (ice-9 regex))
+ (substitute* "src/udev/Makefile.am"
+ (((regexp-quote "$(udevrulesdir)")) "/etc/udev/rules.d"))))
(add-before 'bootstrap 'patch-file-names
(lambda* (#:key inputs native-inputs #:allow-other-keys)
(substitute* "man/make.sh"
--
2.40.1
Liliana Marie Prikler wrote 2 years ago
a7e91916fa699b179173124e90bf810f6264c8ac.camel@gmail.com
Am Sonntag, dem 28.05.2023 um 16:28 -0700 schrieb Felix Lechner:
Toggle quote (39 lines)
> This substitution ensures that udevadm sees the rules that are
> actually in effect for the declared operating system. It allows
> administrators to use the udev-rules-service for network interfaces.
>
> Some of Guix's customizations for udev rules appear to work as it is
> [1] but that is not true for network interfaces (which invoke udevadm
> for naming purposes). [2]
>
> Without this commit, udevadm will consult the rules that were present
> at build time and were installed in the store).
>
> [1]
> https://lists.gnu.org/archive/html/guix-devel/2023-05/msg00195.html
> [2]
> https://lists.gnu.org/archive/html/guix-devel/2023-05/msg00192.html
>
> * gnu/packages/linux.scm (eudev): Have udevadm look in
> /etc/udev/rules.d. (Closes: #63508)
> ---
>  gnu/packages/linux.scm | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
> index 90a44a518d..57b722d97d 100644
> --- a/gnu/packages/linux.scm
> +++ b/gnu/packages/linux.scm
> @@ -4192,6 +4192,11 @@ (define-public eudev
>       (list
>        #:phases
>        #~(modify-phases %standard-phases
> +          (add-before 'bootstrap 'hardcode-runtime-rules-dir
> +            (lambda _
> +              (use-modules (ice-9 regex))
> +              (substitute* "src/udev/Makefile.am"
> +                (((regexp-quote "$(udevrulesdir)"))
> "/etc/udev/rules.d"))))
>            (add-before 'bootstrap 'patch-file-names
>              (lambda* (#:key inputs native-inputs #:allow-other-keys)
>                (substitute* "man/make.sh"
I still think the proper fix is to consult $EUDEV_RULES_DIRECTORY first

Cheers
Liliana Marie Prikler wrote 2 years ago
Re: [PATCH v3 1/3] gnu: eudev: Convert native-inputs to new style, and build arguments to Gexps.
42d5da6862685fef3cec7a8a79b32e0a89c0675d.camel@gmail.com
Am Sonntag, dem 28.05.2023 um 16:28 -0700 schrieb Felix Lechner:
Toggle quote (3 lines)
> * gnu/packages/linux.scm (eudev): Convert native-inputs to new style,
> and build arguments to Gexps.
> ---
Proper subject is "gnu: eudev: Use new package style".

Proper ChangeLog is
* gnu/packages/linux.scm (eudev)[arguments]: Convert to list of
G-Expressions.
[native-inputs]: Drop labels.


The commit itself LGTM.

Cheers
Liliana Marie Prikler wrote 2 years ago
Re: [PATCH v3 2/3] gnu: eudev: Use new project URL.
08c1b343addc553cf84c96fa062209e1682f43eb.camel@gmail.com
Am Sonntag, dem 28.05.2023 um 16:28 -0700 schrieb Felix Lechner:
Toggle quote (1 lines)
> * gnu/packages/linux.scm (eudev): Use new project URL.
Be specific when changes affect parts of a package. Also, in the
ChangeLog you can mention the full URL.

Toggle quote (20 lines)
> ---
>  gnu/packages/linux.scm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
> index ae2792825b..90a44a518d 100644
> --- a/gnu/packages/linux.scm
> +++ b/gnu/packages/linux.scm
> @@ -4180,7 +4180,7 @@ (define-public eudev
>      (version "3.2.11")
>      (source (origin
>                (method git-fetch)
> -              (uri (git-reference (url
> "https://github.com/gentoo/eudev")
> +              (uri (git-reference (url
> "https://github.com/eudev-project/eudev")
>                                    (commit (string-append "v"
> version))))
>                (file-name (git-file-name name version))
>                (sha256
According to this page, the home-page is also
https://github.com/gentoo/eudev and should thus be updated as well.

Cheers
Liliana Marie Prikler wrote 2 years ago
Re: [PATCH v2 4/4] gnu: eudev: Have udevadm look in /etc/udev/rules.d. (Closes: #63508)
(name . Felix Lechner)(address . felix.lechner@lease-up.com)(address . 63508@debbugs.gnu.org)
b73727cc3da96e74a8205930829a762389867603.camel@gmail.com
Am Sonntag, dem 28.05.2023 um 16:23 -0700 schrieb Felix Lechner:
Toggle quote (4 lines)
> Hi Liliana,
>
> Thank you for your kind review! I will push a new version with
> squashed commits you requested shortly.
You mean "submit", right?

Toggle quote (3 lines)
> While I am new to Guix, I am not sure that Gexp conversions fall
> under "style changes" in my book. I believe they are considerably
> more complex, and fraught with greater error.
True, but there is room for error in dropping input labels as well. In
fact, eudev's labels do cause a rebuild, but I decided to push v3 1/3
anyway to get CI ready.

Toggle quote (18 lines)
> On Wed, May 17, 2023 at 9:19 PM Liliana Marie Prikler
> <liliana.prikler@gmail.com> wrote:
> >
> > you may object that this doesn't mention /etc/udev/rules.d and thus
> > could be problematic on foreign distributions, but I argue that you
> > probably shouldn't mess with foreign udev anyway, and if you do
> > that setting EUDEV_RULES_DIRECTORY is appropriate.
>
> The intent of my patch was not to mention /etc/udev/rules.d
> explicitly, but rather to replace the store folder that holds the
> upstream rules, which we are currently using, with the one Guix
> constructs in order to use rules from other places. That just happens
> to be /etc/udev/rules.d as well.
>
> On that note, my patch is not suitable for upstream because it
> hardcodes the location to the runtime path in Guix. Other
> distributions may keep them in a different place. The current
> Autoconf setup probably works well for them.
The same reason why your patch wouldn't fly upstream is why it won't
fly in Guix. We do have to consider foreign distributions as well.

Toggle quote (2 lines)
> Either way, udevadm in Guix is currently broken. This patch fixes it
> and should please be accepted. Thanks!
There is more than one way to fix a bug and I argue that the one you
have chosen is not the right one. Granted, same could be said for my
patch, but you have yet to file a formal complaint. The closest I can
recall is "the resulting flexibility may not be needed", but here we
are discussing foreign distros storing udev rules in some other
location.

Cheers
Felix Lechner wrote 2 years ago
[PATCH v4 1/2] gnu: eudev: Use new project URL for Git repo and home page.
(address . 63508@debbugs.gnu.org)
d6fc82a44190d2c5b16d8f8f434102b68681ccfe.1685379443.git.felix.lechner@lease-up.com
* gnu/packages/linux.scm (eudev): Use new project URL
https://github.com/eudev-project/eudevfor Git repo and home page.
---
gnu/packages/linux.scm | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

Toggle diff (24 lines)
diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index ae2792825b..7a365e2e22 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -4180,7 +4180,7 @@ (define-public eudev
(version "3.2.11")
(source (origin
(method git-fetch)
- (uri (git-reference (url "https://github.com/gentoo/eudev")
+ (uri (git-reference (url "https://github.com/eudev-project/eudev")
(commit (string-append "v" version))))
(file-name (git-file-name name version))
(sha256
@@ -4242,7 +4242,7 @@ (define-public eudev
(list `(,util-linux "lib") ;for blkid
kmod))
(outputs '("out" "static"))
- (home-page "https://wiki.gentoo.org/wiki/Project:Eudev")
+ (home-page "https://github.com/eudev-project/eudev")
(synopsis "Userspace device management")
(description "Udev is a daemon which dynamically creates and removes
device nodes from /dev/, handles hotplug events and loads drivers at boot
--
2.40.1
Felix Lechner wrote 2 years ago
[PATCH v4 2/2] gnu: eudev: Have udevadm look in /etc/udev/rules.d. (Closes: #63508)
(address . 63508@debbugs.gnu.org)
2d51ddd4e6865a40a15b8f7948d2479952571a4e.1685379443.git.felix.lechner@lease-up.com
This substitution ensures that udevadm sees the rules that are actually in
effect for the declared operating system. It allows administrators to use the
udev-rules-service for network interfaces.

Some of Guix's customizations for udev rules appear to work as it is [1] but
that is not true for network interfaces (which invoke udevadm for naming
purposes). [2]

Without this commit, udevadm will consult the rules that were present at build
time and were installed in the store).


* gnu/packages/linux.scm (eudev): Have udevadm look in
/etc/udev/rules.d. (Closes: #63508)
---
gnu/packages/linux.scm | 5 +++++
1 file changed, 5 insertions(+)

Toggle diff (18 lines)
diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index 7a365e2e22..55255e576e 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -4192,6 +4192,11 @@ (define-public eudev
(list
#:phases
#~(modify-phases %standard-phases
+ (add-before 'bootstrap 'hardcode-runtime-rules-dir
+ (lambda _
+ (use-modules (ice-9 regex))
+ (substitute* "src/udev/Makefile.am"
+ (((regexp-quote "$(udevrulesdir)")) "/etc/udev/rules.d"))))
(add-before 'bootstrap 'patch-file-names
(lambda* (#:key inputs native-inputs #:allow-other-keys)
(substitute* "man/make.sh"
--
2.40.1
Bruno Victal wrote 2 years ago
(address . 63508@debbugs.gnu.org)
5543a014-6ace-f212-0dc2-046ef9e99086@makinata.eu
Hi Lechner,

On 2023-05-29 17:57, Felix Lechner via Guix-patches via wrote:
Toggle quote (8 lines)
> #:phases
> #~(modify-phases %standard-phases
> + (add-before 'bootstrap 'hardcode-runtime-rules-dir
> + (lambda _
> + (use-modules (ice-9 regex))
> + (substitute* "src/udev/Makefile.am"
> + (((regexp-quote "$(udevrulesdir)")) "/etc/udev/rules.d"))))

Perhaps instead of substituting, can you check if
#:configure-flags or #:make-flags could be used instead?


--
Furthermore, I consider that nonfree software must be eradicated.

Cheers,
Bruno.
Felix Lechner wrote 2 years ago
(name . Bruno Victal)(address . mirai@makinata.eu)
CAFHYt54LpOV1oOJFRphDP8hduVCae68X0ytLw2iiSz3uobq8WA@mail.gmail.com
Hi Bruno,

On Mon, May 29, 2023 at 1:28 PM Bruno Victal <mirai@makinata.eu> wrote:
Toggle quote (4 lines)
>
> Perhaps instead of substituting, can you check if
> #:configure-flags or #:make-flags could be used instead?

Thank you for that suggestion! I did, and I do not believe it is
possible to specify a separate runtime path for udevadm via
./configure at this time.

It would be possible to add a second variable in configure.ac [1] and
use that in the relevant Makefile.am. [2] The change would get picked
up for the runtime search path throughout [3] and for the inotify
watch in udevd. [4]

In that scenario, it would be crucial not to modify the installation
directory that was specified via the variable prefix "udevrules_" in
several Makefiles.am, [5][6][7] which use Automake's "uniform" naming
scheme that leaves off 'dir'. [8]

Unfortunately, for most distros the distinctions are meaningless, and
perhaps even incomprehensible. I estimated the chance of upstream
acceptance as low. After some reflection, it was easier to patch the
runtime path directly in the lone place that matters to Guix. [2,
again]

I also do not see how the second variable needed can be introduced by
invoking 'make' in a different way. Please let me know if you find a
way. Thanks!

Kind regards
Felix

Maxim Cournoyer wrote 4 weeks ago
Re: [bug#63508] [PATCH 3/3] gnu: eudev: Always use MAC-based names for network interfaces.
(name . Felix Lechner)(address . felix.lechner@lease-up.com)(address . 63508@debbugs.gnu.org)
874j188pid.fsf@gmail.com
Hi Felix,

Felix Lechner <felix.lechner@lease-up.com> writes:

Toggle quote (7 lines)
> Upon personal reflection, a declarative operating system like Guix probably
> ought to use only predictable interface names.
>
> While shorter names like 'eno1' offer an indisputable convenience and beauty
> when typing on the command line, administrators in Guix are unlikely to do so
> due to the declarative configuration system.

While it's true that predictable interface names are nice, I thought
they already were. But more to the point, I don't think we should
change something that is the default and has been used for a long time
without major complaints.

I personally don't think we should merge this 3/3 patch. It's something
to be left to users to choose for themselves based on their own needs.

--
Thanks,
Maxim
Felix Lechner wrote 4 weeks ago
retitle 63508
(address . control@debbugs.gnu.org)
87tt98beiy.fsf@lease-up.com
retitle 63508 [PATCH] gnu: eudev: Look for rules in /etc/udev/rules.d
thanks
Felix Lechner wrote 4 weeks ago
Re: [bug#63508] [PATCH 3/3] gnu: eudev: Always use MAC-based names for network interfaces.
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)(address . 63508@debbugs.gnu.org)
87o6zgbdqp.fsf@lease-up.com
Hi Maxim,

On Wed, Feb 05 2025, Maxim Cournoyer wrote:

Toggle quote (3 lines)
> It's something to be left to users to choose for themselves based on
> their own needs.

You misunderstood. The patch merely allows users to choose
ID_NET_NAME_MAC.

It does not change the default naming.

Eudev on Guix is broken. It does not look in /etc/udev/rules.d even
though that's where udev-rules-service installs its rules.

The confusion arose because v1 changed the default, but I realized
quickly that it went too far. Please have a look at v4 from May 2023.

I have been using that patch in production in the two years since.

Kind regards
Felix
Maxim Cournoyer wrote 4 weeks ago
Re: [bug#63508] [PATCH v4 2/2] gnu: eudev: Have udevadm look in /etc/udev/rules.d. (Closes: #63508)
(name . Felix Lechner)(address . felix.lechner@lease-up.com)
87zfiz7qk4.fsf@gmail.com
Hello,

Felix Lechner <felix.lechner@lease-up.com> writes:

Toggle quote (11 lines)
> Hi Bruno,
>
> On Mon, May 29, 2023 at 1:28 PM Bruno Victal <mirai@makinata.eu> wrote:
>>
>> Perhaps instead of substituting, can you check if
>> #:configure-flags or #:make-flags could be used instead?
>
> Thank you for that suggestion! I did, and I do not believe it is
> possible to specify a separate runtime path for udevadm via
> ./configure at this time.

Turns out it's possible, and that's what NixPkgs does. I'll send a v5
soon implementing that.

--
Thanks,
Maxim
Maxim Cournoyer wrote 4 weeks ago
[PATCH core-updates v5 0/3] eudev: Build with udevrulesdir pointing to /etc/udev/rules.d
(address . 63508@debbugs.gnu.org)(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
cover.1738809478.git.maxim.cournoyer@gmail.com
Prior to this change, only the udev rules installed to eudev's prefix were
consulted by tools such as udevadm, leading to problems such as when
configuring network interfaces, or attempting to override its default rules.

While our custom eudev patch adding support for the EUDEV_RULES_DIRECTORY
environment variable could have been refined to take precedence over the
package's configured udevrulesdir, this was not pursued for the following
reasons:

1. Due to eudev's using inotify to detect new rules, the EUDEV_RULES_DIRECTORY
is fixed in Guix System, per commit e9fa17eb98 ("services: udev: Use a fixed
location for the rules directory and config.")

2. Users would have had to set EUDEV_RULES_DIRECTORY to the fixed directory
themselves to have udevadm work as expected, which is inconvenient.

3. This simple solution is already implemented and tested in NixPkgs.

Changes in v5:
- Use #:make-flags to configure udev-rules.d prefix
- Remove now unused eudev patch

Felix Lechner (1):
gnu: eudev: Use new project URL for Git repo and home page.

Maxim Cournoyer (2):
services/base: Remove extraneous UDEV_CONFIG_FILE environment
variable.
gnu: eudev: Build with udevrulesdir pointing to /etc/udev/rules.d.

gnu/local.mk | 1 -
gnu/packages/linux.scm | 35 +++++++++++++-----
.../patches/eudev-rules-directory.patch | 37 -------------------
gnu/services/base.scm | 4 --
4 files changed, 25 insertions(+), 52 deletions(-)
delete mode 100644 gnu/packages/patches/eudev-rules-directory.patch


base-commit: 52c05f3b120e641c8bd2d68cfcf0d6af947de27b
--
2.48.1
Maxim Cournoyer wrote 4 weeks ago
[PATCH core-updates v5 1/3] gnu: eudev: Use new project URL for Git repo and home page.
(address . 63508@debbugs.gnu.org)(name . Felix Lechner)(address . felix.lechner@lease-up.com)
a4c0ebaa0f8055453187509a940b5161fcaf6341.1738809478.git.maxim.cournoyer@gmail.com
From: Felix Lechner <felix.lechner@lease-up.com>

* gnu/packages/linux.scm (eudev): Use new project URL
https://github.com/eudev-project/eudevfor Git repo and home page.
---

(no changes since v1)

gnu/packages/linux.scm | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

Toggle diff (24 lines)
diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index 5c48aa7320..e95d8587b4 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -4538,7 +4538,7 @@ (define-public eudev
(version "3.2.14")
(source (origin
(method git-fetch)
- (uri (git-reference (url "https://github.com/gentoo/eudev")
+ (uri (git-reference (url "https://github.com/eudev-project/eudev")
(commit (string-append "v" version))))
(file-name (git-file-name name version))
(sha256
@@ -4623,7 +4623,7 @@ (define-public eudev
(list `(,util-linux "lib") ;for blkid
kmod))
(outputs '("out" "static"))
- (home-page "https://wiki.gentoo.org/wiki/Project:Eudev")
+ (home-page "https://github.com/eudev-project/eudev")
(synopsis "Userspace device management")
(description "Udev is a daemon which dynamically creates and removes
device nodes from /dev/, handles hotplug events and loads drivers at boot
--
2.48.1
Maxim Cournoyer wrote 4 weeks ago
[PATCH core-updates v5 2/3] services/base: Remove extraneous UDEV_CONFIG_FILE environment variable.
(address . 63508@debbugs.gnu.org)(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
48cd1aee0a5b0e140f003e04ac31bd374f0f07bb.1738809478.git.maxim.cournoyer@gmail.com
This environment variable used to be honored by udevd, but that is no longer
the case (as shown by grepping its source).

* gnu/services/base.scm (udev-shepherd-service) <#:environment-variables>:
Remove UDEV_CONFIG_FILE.

Change-Id: I0828de76e8da429432bc0679903aa501c99625af
---

(no changes since v1)

gnu/services/base.scm | 3 ---
1 file changed, 3 deletions(-)

Toggle diff (16 lines)
diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 7331c030d7..654e9479f2 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -2545,9 +2545,6 @@ (define (udev-shepherd-service config)
(list udevd)
#:environment-variables
(cons*
- ;; The first one is for udev, the second one for
- ;; eudev.
- "UDEV_CONFIG_FILE=/etc/udev/udev.conf"
"EUDEV_RULES_DIRECTORY=/etc/udev/rules.d"
(string-append "LINUX_MODULE_DIRECTORY="
(getenv "LINUX_MODULE_DIRECTORY"))
--
2.48.1
Maxim Cournoyer wrote 4 weeks ago
[PATCH core-updates v5 3/3] gnu: eudev: Build with udevrulesdir pointing to /etc/udev/rules.d.
(address . 63508@debbugs.gnu.org)
dfebb023dfbe4c1e8489a662f09a1e98fa0070fb.1738809478.git.maxim.cournoyer@gmail.com
Prior to this change, only the udev rules installed to eudev's prefix were
consulted by tools such as udevadm, leading to problems such as when
configuring network interfaces, or attempting to override its default rules.

While our custom eudev patch adding support for the EUDEV_RULES_DIRECTORY
environment variable could have been refined to take precedence over the
package's configured udevrulesdir, this was not pursued for the following
reasons:

1. Due to eudev's using inotify to detect new rules, the EUDEV_RULES_DIRECTORY
is fixed in Guix System, per commit e9fa17eb98 ("services: udev: Use a fixed
location for the rules directory and config.")

2. Users would have had to set EUDEV_RULES_DIRECTORY to the fixed directory
themselves to have udevadm work as expected, which is inconvenient.

3. This simple solution is already implemented and tested in NixPkgs.

* gnu/packages/linux.scm (eudev) [source]: Remove custom patch.
[arguments] <#:make-flags>: New argument.
<#:phases>: Override install phase to alter installation make flags.
* gnu/services/base.scm (udev-shepherd-service): Do not set
EUDEV_RULES_DIRECTORY environment variable.
* gnu/packages/patches/eudev-rules-directory.patch: Delete file.
* gnu/local.mk (dist_patch_DATA): De-register it.

Reported-by: Felix Lechner <felix.lechner@lease-up.com>
Change-Id: Ib8698f4b452f6fd0951bcd71831705b1be85e6e0
---

Changes in v5:
- Use #:make-flags to configure udev-rules.d prefix
- Remove now unused eudev patch

gnu/local.mk | 1 -
gnu/packages/linux.scm | 31 ++++++++++++----
.../patches/eudev-rules-directory.patch | 37 -------------------
gnu/services/base.scm | 1 -
4 files changed, 23 insertions(+), 47 deletions(-)
delete mode 100644 gnu/packages/patches/eudev-rules-directory.patch

Toggle diff (135 lines)
diff --git a/gnu/local.mk b/gnu/local.mk
index 83abc86fe2..16978da169 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1240,7 +1240,6 @@ dist_patch_DATA = \
%D%/packages/patches/erlang-man-path.patch \
%D%/packages/patches/esmini-use-pkgconfig.patch \
%D%/packages/patches/esmtp-add-lesmtp.patch \
- %D%/packages/patches/eudev-rules-directory.patch \
%D%/packages/patches/exercism-disable-self-update.patch \
%D%/packages/patches/expat-CVE-2024-45490.patch \
%D%/packages/patches/expat-CVE-2024-45491.patch \
diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index e95d8587b4..f4e9e30a75 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -4543,12 +4543,14 @@ (define-public eudev
(file-name (git-file-name name version))
(sha256
(base32
- "1f6lz57igi7iw2ls3fpzgw42bfznam4nf9368h7x8yf1mb737yxz"))
- (patches (search-patches "eudev-rules-directory.patch"))
- (modules '((guix build utils)))))
+ "1f6lz57igi7iw2ls3fpzgw42bfznam4nf9368h7x8yf1mb737yxz"))))
(build-system gnu-build-system)
(arguments
(list
+ ;; The binary should be built to look for its rules under
+ ;; /etc/udev/rules.d, which is where the udev-shepherd-service keeps
+ ;; them.
+ #:make-flags #~(list "udevrulesdir=/etc/udev/rules.d")
#:phases
#~(modify-phases %standard-phases
(add-before 'bootstrap 'patch-file-names
@@ -4592,7 +4594,20 @@ (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"))))))
+ "old_library=''\n")))))
+ (replace 'install
+ (lambda* (#:key make-flags #:allow-other-keys #:rest args)
+ ;; Although the runtime udevrulesdir is set to
+ ;; /etc/udev/rules.d, the package should provide its default
+ ;; rules under $libdir/udev/rules.d.
+ (let* ((default-udev-rules.d (string-append #$output
+ "/lib/udev/rules.d"))
+ (make-flags (cons (string-append "udevrulesdir="
+ default-udev-rules.d)
+ (delete "udevrulesdir=/etc/udev/rules.d"
+ make-flags))))
+ (apply (assoc-ref %standard-phases 'install)
+ `(,@args #:make-flags ,make-flags))))))
#:configure-flags
#~(list "--enable-manpages"
;; By default, autoconf uses $prefix/etc. The udev-service-type
@@ -4600,9 +4615,9 @@ (define-public eudev
;; descriptions.
"--sysconfdir=/etc")))
(native-search-paths
- (list (search-path-specification
- (variable "UDEV_HWDB_PATH")
- (files '("lib/udev/hwdb.d")))))
+ (list (search-path-specification
+ (variable "UDEV_HWDB_PATH")
+ (files '("lib/udev/hwdb.d")))))
(native-inputs
(list autoconf
automake
@@ -4620,7 +4635,7 @@ (define-public eudev
;; When linked against libblkid, eudev can populate /dev/disk/by-label
;; and similar; it also installs the '60-persistent-storage.rules' file,
;; which contains the rules to do that.
- (list `(,util-linux "lib") ;for blkid
+ (list `(,util-linux "lib") ;for blkid
kmod))
(outputs '("out" "static"))
(home-page "https://github.com/eudev-project/eudev")
diff --git a/gnu/packages/patches/eudev-rules-directory.patch b/gnu/packages/patches/eudev-rules-directory.patch
deleted file mode 100644
index c4b1cfae39..0000000000
--- a/gnu/packages/patches/eudev-rules-directory.patch
+++ /dev/null
@@ -1,37 +0,0 @@
-Add $EUDEV_RULES_DIRECTORY to the list of rules directories.
-
-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.
-
---- a/src/udev/udev-rules.c
-+++ b/src/udev/udev-rules.c
-@@ -48,16 +48,11 @@ struct uid_gid {
- };
- };
-
--static const char* const rules_dirs[] = {
-+static const char* rules_dirs[] = {
- UDEV_CONF_DIR "/rules.d",
- UDEV_RULES_DIR,
-- UDEV_ROOT_RUN "/udev/rules.d",
- UDEV_LIBEXEC_DIR "/rules.d",
--#ifdef HAVE_SPLIT_USR
-- "/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 {
-@@ -1718,6 +1713,9 @@ struct udev_rules *udev_rules_new(struct udev *udev, int resolve_names) {
-
- udev_rules_check_timestamp(rules);
-
-+ /* Allow the user to specify an additional rules directory. */
-+ rules_dirs[3] = getenv("EUDEV_RULES_DIRECTORY");
-+
- r = conf_files_list_strv(&files, ".rules", NULL, rules_dirs);
- if (r < 0) {
- log_error_errno(r, "failed to enumerate rules files: %m");
diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 654e9479f2..36fef00457 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -2545,7 +2545,6 @@ (define (udev-shepherd-service config)
(list udevd)
#:environment-variables
(cons*
- "EUDEV_RULES_DIRECTORY=/etc/udev/rules.d"
(string-append "LINUX_MODULE_DIRECTORY="
(getenv "LINUX_MODULE_DIRECTORY"))
(default-environment-variables)))))
--
2.48.1
Maxim Cournoyer wrote 4 weeks ago
Re: [bug#63508] [PATCH 3/3] gnu: eudev: Always use MAC-based names for network interfaces.
(name . Felix Lechner)(address . felix.lechner@lease-up.com)
87seoraieg.fsf@gmail.com
Hi Felix,

Felix Lechner <felix.lechner@lease-up.com> writes:

Toggle quote (18 lines)
> Hi Maxim,
>
> On Wed, Feb 05 2025, Maxim Cournoyer wrote:
>
>> It's something to be left to users to choose for themselves based on
>> their own needs.
>
> You misunderstood. The patch merely allows users to choose
> ID_NET_NAME_MAC.
>
> It does not change the default naming.
>
> Eudev on Guix is broken. It does not look in /etc/udev/rules.d even
> though that's where udev-rules-service installs its rules.
>
> The confusion arose because v1 changed the default, but I realized
> quickly that it went too far. Please have a look at v4 from May 2023.

Yes I was answering to v1. Regarding v4, I've taken a look and came up
with v5.

--
Thanks,
Maxim
Felix Lechner wrote 4 weeks ago
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87cyfvbqmj.fsf@lease-up.com
Hi Maxim,

On Thu, Feb 06 2025, Maxim Cournoyer wrote:

Toggle quote (2 lines)
> I've taken a look and came up with v5.

Thank you so much for working on this! I can't believe it. Someone
please pinch me!

Kind regards
Felix
Liliana Marie Prikler wrote 4 weeks ago
Re: [bug#63508] [PATCH core-updates v5 0/3] eudev: Build with udevrulesdir pointing to /etc/udev/rules.d
4993fa8d776a6c488f8ea0529d8a145db8c96042.camel@gmail.com
Should this still point to "core-updates"?

Am Donnerstag, dem 06.02.2025 um 11:38 +0900 schrieb Maxim Cournoyer:
Toggle quote (11 lines)
> Prior to this change, only the udev rules installed to eudev's prefix
> were consulted by tools such as udevadm, leading to problems such as
> when configuring network interfaces, or attempting to override its
> default rules.
>
> While our custom eudev patch adding support for the
> EUDEV_RULES_DIRECTORY environment variable could have been refined to
> take precedence over the package's configured udevrulesdir, this was
> not pursued for the following reasons:
>
> […]
Now, we are losing some flexibility by removing the environment
variable, but if that's what Nix does, it's what nix does ‾\_(ツ)_/‾

Cheers
Maxim Cournoyer wrote 4 weeks ago
Re: bug#63508: [PATCH] gnu: eudev: Look for rules in /etc/udev/rules.d
(name . Liliana Marie Prikler)(address . liliana.prikler@gmail.com)
87jza38mip.fsf_-_@gmail.com
Hello,

Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

Toggle quote (17 lines)
> Should this still point to "core-updates"?
>
> Am Donnerstag, dem 06.02.2025 um 11:38 +0900 schrieb Maxim Cournoyer:
>> Prior to this change, only the udev rules installed to eudev's prefix
>> were consulted by tools such as udevadm, leading to problems such as
>> when configuring network interfaces, or attempting to override its
>> default rules.
>>
>> While our custom eudev patch adding support for the
>> EUDEV_RULES_DIRECTORY environment variable could have been refined to
>> take precedence over the package's configured udevrulesdir, this was
>> not pursued for the following reasons:
>>
>> […]
> Now, we are losing some flexibility by removing the environment
> variable, but if that's what Nix does, it's what nix does ‾\_(ツ)_/‾

I sense some potential sarcasm here; if so, did you consider also my
second point, which was:

Toggle quote (4 lines)
> 2. Users would have had to set EUDEV_RULES_DIRECTORY to the fixed
> directory themselves to have udevadm work as expected, which is
> inconvenient.

We could also leave the patch in, to let potential users continue making
use of it, but being undocumented in a custom patch that we haven't
upstreamed, used only in the udev service and not in any search path, it
seems likely very few people knew about it (and we'd have to rebase it
from time to time, for dubious value).

I'd suggest if this feature really is important, we could rework the
patch into a EUDEV_RULES_PATH, making it a true path accepting multiple
entries, and try to have this upstreamed. Then we could have a nice
search path specification for it attached to eudev.

How does that sound?

--
Thanks,
Maxim
Maxim Cournoyer wrote 4 weeks ago
Re: [bug#63508] [PATCH 3/3] gnu: eudev: Always use MAC-based names for network interfaces.
(name . Felix Lechner)(address . felix.lechner@lease-up.com)
87frkr8fpx.fsf@gmail.com
Hey Felix,

Felix Lechner <felix.lechner@lease-up.com> writes:

Toggle quote (9 lines)
> Hi Maxim,
>
> On Thu, Feb 06 2025, Maxim Cournoyer wrote:
>
>> I've taken a look and came up with v5.
>
> Thank you so much for working on this! I can't believe it. Someone
> please pinch me!

Sorry for the reviews lagging behind. We have more producers than
reviewers, obviously. More eyes and hands welcome! I think many
reviewers like to keep an eye on https://qa.guix.gnu.org/patches(I do),
so anyone reviewing and tagging with the 'reviewed-looks-good' user tag
can make these stand out as potentially easier/faster targets.

--
Thanks,
Maxim
Felix Lechner wrote 4 weeks ago
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87pljv9jbu.fsf@lease-up.com
Hi Maxim,

On Thu, Feb 06 2025, Maxim Cournoyer wrote:

Toggle quote (2 lines)
> Sorry for the reviews lagging behind.

The initial report was reviewed within eighteen hours of filing.

Toggle quote (3 lines)
> We have more producers than reviewers, obviously. More eyes and hands
> welcome!

I'll help a little. How many hours did you spend here, please?

Toggle quote (3 lines)
> tagging with the 'reviewed-looks-good' user tag can make these stand
> out as potentially easier/faster targets.

That workflow prioritizes easier/faster bugs, which is wrong. May
please I use Debbugs severities [1] to elevate serious bugs?

Also, while I have your attention: This Mumi patch [2] fixes the unicode
errors in Lily's message [3] and many others.

Kind regards
Felix

Liliana Marie Prikler wrote 4 weeks ago
Re: bug#63508: [PATCH] gnu: eudev: Look for rules in /etc/udev/rules.d
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
8fc89640aaa3849793fb087bd6ea29357b6f4c33.camel@gmail.com
Am Donnerstag, dem 06.02.2025 um 18:12 +0900 schrieb Maxim Cournoyer:
Toggle quote (12 lines)
> Hello,
>
> Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
> > Now, we are losing some flexibility by removing the environment
> > variable, but if that's what Nix does, it's what nix does ‾\_(ツ)_/‾
>
> I sense some potential sarcasm here; if so, did you consider also my
> second point, which was:
>
> > 2. Users would have had to set EUDEV_RULES_DIRECTORY to the fixed
> > directory themselves to have udevadm work as expected, which is
> > inconvenient.
I did, this wasn't supposed to be a value judgement. Doing things as
Nix does does have the benefit of being tested already, even if it
doesn't follow our usual style.¹

Toggle quote (12 lines)
> We could also leave the patch in, to let potential users continue
> making use of it, but being undocumented in a custom patch that we
> haven't upstreamed, used only in the udev service and not in any
> search path, it seems likely very few people knew about it (and we'd
> have to rebase it from time to time, for dubious value).
>
> I'd suggest if this feature really is important, we could rework the
> patch into a EUDEV_RULES_PATH, making it a true path accepting
> multiple entries, and try to have this upstreamed.  Then we could
> have a nice search path specification for it attached to eudev.
>
> How does that sound?
Well, if we do widen the directory to a path, we still have the problem
that users would have to set it. Most likely, we will add a search-
path definition to udev and perhaps even catch the common use case in
doing so, but misalignment may still occur.

I think we should talk to upstream about the benefits/drawbacks of an
environment variable. In my personal opinion, this could very well use
a PATH, but I'm aware that I may be biased.

Cheers

¹ From a personal sense of aesthetics, building with one set of make-
flags and installing with another feels weird. So does having
UDEV_HWDB_PATH, but no UDEV_RULES_PATH. (:
Maxim Cournoyer wrote 3 weeks ago
(name . Liliana Marie Prikler)(address . liliana.prikler@gmail.com)
874j166xyu.fsf@gmail.com
Hi Liliana,

Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

[...]

Toggle quote (12 lines)
>> I'd suggest if this feature really is important, we could rework the
>> patch into a EUDEV_RULES_PATH, making it a true path accepting
>> multiple entries, and try to have this upstreamed.  Then we could
>> have a nice search path specification for it attached to eudev.
>>
>> How does that sound?

> Well, if we do widen the directory to a path, we still have the problem
> that users would have to set it. Most likely, we will add a search-
> path definition to udev and perhaps even catch the common use case in
> doing so, but misalignment may still occur.

Right. Especially for something like udevd designed to be used in a
service, where profiles are not involved by default and search paths
not being useful in this case (one would have to go through extra hoops
to have it computed and set).

Toggle quote (4 lines)
> I think we should talk to upstream about the benefits/drawbacks of an
> environment variable. In my personal opinion, this could very well use
> a PATH, but I'm aware that I may be biased.

I'd like to go with this v5 as-is, unless you have a problem with it,
and we can see in time if there's a need for coming up with a
UDEV_RULES_PATH environment variable, for the sake of not expending
efforts where there is little reward.

Do you agree?

--
Thanks,
Maxim
Maxim Cournoyer wrote 3 weeks ago
Re: [bug#63508] [PATCH 3/3] gnu: eudev: Always use MAC-based names for network interfaces.
(name . Felix Lechner)(address . felix.lechner@lease-up.com)
87v7tm5hse.fsf@gmail.com
Hi Felix,

Felix Lechner <felix.lechner@lease-up.com> writes:

Toggle quote (8 lines)
> Hi Maxim,
>
> On Thu, Feb 06 2025, Maxim Cournoyer wrote:
>
>> Sorry for the reviews lagging behind.
>
> The initial report was reviewed within eighteen hours of filing.

Good.

Toggle quote (5 lines)
>> We have more producers than reviewers, obviously. More eyes and hands
>> welcome!
>
> I'll help a little. How many hours did you spend here, please?

I didn't count. Perhaps two or three, to get familiar with the whole
threads of the issue at hand, doing some research, producing the new
revision, and finally communicating it.

Toggle quote (6 lines)
>> tagging with the 'reviewed-looks-good' user tag can make these stand
>> out as potentially easier/faster targets.
>
> That workflow prioritizes easier/faster bugs, which is wrong. May
> please I use Debbugs severities [1] to elevate serious bugs?

You can, and it's useful if used wisely, but that will do little to
lessen the load on the reviewers, as regular contributions still need to
be processed, and they often require less time investment than serious
issues, making them more suitable targets for the typical late evening
contribution.

Toggle quote (3 lines)
> Also, while I have your attention: This Mumi patch [2] fixes the unicode
> errors in Lily's message [3] and many others.

[...]

Toggle quote (3 lines)
> [3] https://issues.guix.gnu.org/63508#4

OK, will try taking a look.

--
Thanks,
Maxim
Liliana Marie Prikler wrote 3 weeks ago
Re: bug#63508: [PATCH] gnu: eudev: Look for rules in /etc/udev/rules.d
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
7417feb35e0153c4ada21ceb46dbfdb6b44cf807.camel@gmail.com
Am Freitag, dem 07.02.2025 um 16:00 +0900 schrieb Maxim Cournoyer:
Toggle quote (6 lines)
> I'd like to go with this v5 as-is, unless you have a problem with it,
> and we can see in time if there's a need for coming up with a
> UDEV_RULES_PATH environment variable, for the sake of not expending
> efforts where there is little reward.
>
> Do you agree?
Sorry for the late reply. Assuming you haven't pushed the change yet,
feel free to do so.

Cheers
Maxim Cournoyer wrote 3 weeks ago
(name . Liliana Marie Prikler)(address . liliana.prikler@gmail.com)
87tt90sg1r.fsf@gmail.com
Hi Liliana,

Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

Toggle quote (10 lines)
> Am Freitag, dem 07.02.2025 um 16:00 +0900 schrieb Maxim Cournoyer:
>> I'd like to go with this v5 as-is, unless you have a problem with it,
>> and we can see in time if there's a need for coming up with a
>> UDEV_RULES_PATH environment variable, for the sake of not expending
>> efforts where there is little reward.
>>
>> Do you agree?
> Sorry for the late reply. Assuming you haven't pushed the change yet,
> feel free to do so.

Thanks for the heads-up! I was waiting on your reply. Bumping this
package will involve a feature branch (large rebuild), unless you'd like
to include it on the gnome-team branch?

--
Thanks,
Maxim
Liliana Marie Prikler wrote 3 weeks ago
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
c04e370b74296b6237ea44143531d1a3019ccf49.camel@gmail.com
Am Dienstag, dem 11.02.2025 um 23:37 +0900 schrieb Maxim Cournoyer:
Toggle quote (17 lines)
> Hi Liliana,
>
> Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
>
> > Am Freitag, dem 07.02.2025 um 16:00 +0900 schrieb Maxim Cournoyer:
> > > I'd like to go with this v5 as-is, unless you have a problem with
> > > it, and we can see in time if there's a need for coming up with a
> > > UDEV_RULES_PATH environment variable, for the sake of not
> > > expending efforts where there is little reward.
> > >
> > > Do you agree?
> > Sorry for the late reply.  Assuming you haven't pushed the change
> > yet, feel free to do so.
>
> Thanks for the heads-up!  I was waiting on your reply.  Bumping this
> package will involve a feature branch (large rebuild), unless you'd
> like to include it on the gnome-team branch?
We currently have large rebuilds headed for gnome-team courtesy of `git
rebase' resulting in subtle code changes, but I recall now why I prefer
EUDEV_RULES_DIRECTORY (or EUDEV_RULES_PATH) to exist:

Am Donnerstag, dem 18.05.2023 um 06:19 +0200 schrieb Liliana Marie
Prikler:
Toggle quote (4 lines)
> Now, you may object that this doesn't mention /etc/udev/rules.d and
> thus could be problematic on foreign distributions, but I argue that
> you probably shouldn't mess with foreign udev anyway, and if you do
> that setting EUDEV_RULES_DIRECTORY is appropriate.
In other words, I do think that a search path is a necessary
prerequisite to prevent cases where Guix and a foreign distro want
their udev rules in the same place. (It can also be an explicit way of
opting into Guix' udev using rules from the foreign distro).

I'm not sure we should relinquish our environment variable just like
that. Especially on foreign distros there might well be a situation
where we would like to load more udev rules from places other than
/etc/udev/rules.d

Cheers
Felix Lechner wrote 3 weeks ago
(name . Liliana Marie Prikler)(address . liliana.prikler@gmail.com)
871pw440l6.fsf@lease-up.com
Hi,

On Tue, Feb 11 2025, Liliana Marie Prikler wrote:

Toggle quote (4 lines)
> I do think that a search path is a necessary prerequisite to prevent
> cases where Guix and a foreign distro want their udev rules in the
> same place.

The argument may have merit, but should a new feature hold up a bug fix?

Kind regards
Felix
Liliana Marie Prikler wrote 3 weeks ago
(name . Felix Lechner)(address . felix.lechner@lease-up.com)
ef3911fc0e3090daae96cc1fa00fc1826e828e41.camel@gmail.com
Am Dienstag, dem 11.02.2025 um 13:45 -0800 schrieb Felix Lechner:
Toggle quote (10 lines)
> Hi,
>
> On Tue, Feb 11 2025, Liliana Marie Prikler wrote:
>
> > I do think that a search path is a necessary prerequisite to
> > prevent cases where Guix and a foreign distro want their udev rules
> > in the same place.
>
> The argument may have merit, but should a new feature hold up a bug
> fix?
The "new feature" is what we have already; our bugfix would introduce a
regression by hardcoding a path that wasn't before. The proposed "fix"
appears to assume that /etc/udev/rules.d is always controlled by Guix
or should for some other reason be the only directory to consider by
udev. I question that assumption.

Cheers
Maxim Cournoyer wrote 3 weeks ago
(name . Liliana Marie Prikler)(address . liliana.prikler@gmail.com)
87tt8zrdzj.fsf@gmail.com
Hi Liliana,

Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

Toggle quote (38 lines)
> Am Dienstag, dem 11.02.2025 um 23:37 +0900 schrieb Maxim Cournoyer:
>> Hi Liliana,
>>
>> Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
>>
>> > Am Freitag, dem 07.02.2025 um 16:00 +0900 schrieb Maxim Cournoyer:
>> > > I'd like to go with this v5 as-is, unless you have a problem with
>> > > it, and we can see in time if there's a need for coming up with a
>> > > UDEV_RULES_PATH environment variable, for the sake of not
>> > > expending efforts where there is little reward.
>> > >
>> > > Do you agree?
>> > Sorry for the late reply.  Assuming you haven't pushed the change
>> > yet, feel free to do so.
>>
>> Thanks for the heads-up!  I was waiting on your reply.  Bumping this
>> package will involve a feature branch (large rebuild), unless you'd
>> like to include it on the gnome-team branch?
> We currently have large rebuilds headed for gnome-team courtesy of `git
> rebase' resulting in subtle code changes, but I recall now why I prefer
> EUDEV_RULES_DIRECTORY (or EUDEV_RULES_PATH) to exist:
>
> Am Donnerstag, dem 18.05.2023 um 06:19 +0200 schrieb Liliana Marie
> Prikler:
>> Now, you may object that this doesn't mention /etc/udev/rules.d and
>> thus could be problematic on foreign distributions, but I argue that
>> you probably shouldn't mess with foreign udev anyway, and if you do
>> that setting EUDEV_RULES_DIRECTORY is appropriate.
> In other words, I do think that a search path is a necessary
> prerequisite to prevent cases where Guix and a foreign distro want
> their udev rules in the same place. (It can also be an explicit way of
> opting into Guix' udev using rules from the foreign distro).
>
> I'm not sure we should relinquish our environment variable just like
> that. Especially on foreign distros there might well be a situation
> where we would like to load more udev rules from places other than
> /etc/udev/rules.d

On a foreign distribution, you'd normally use the foreign distribution
services, so I don't see this as a strong argument in favor of keeping
EUDEV_RULES_DIRECTORY. I do see it making things more flexible and
configurable, but at a usability and maintenance cost that doesn't
appear worth it to me, given that it'd only be useful for use cases I'd
expect to be very rare, such as running the Guix-provided udevd on top
of a foreign distribution.

--
Thanks,
Maxim
Maxim Cournoyer wrote 3 weeks ago
(name . Liliana Marie Prikler)(address . liliana.prikler@gmail.com)
87pljnrdqb.fsf@gmail.com
Hi Liliana,

Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

Toggle quote (17 lines)
> Am Dienstag, dem 11.02.2025 um 13:45 -0800 schrieb Felix Lechner:
>> Hi,
>>
>> On Tue, Feb 11 2025, Liliana Marie Prikler wrote:
>>
>> > I do think that a search path is a necessary prerequisite to
>> > prevent cases where Guix and a foreign distro want their udev rules
>> > in the same place.
>>
>> The argument may have merit, but should a new feature hold up a bug
>> fix?
> The "new feature" is what we have already; our bugfix would introduce a
> regression by hardcoding a path that wasn't before. The proposed "fix"
> appears to assume that /etc/udev/rules.d is always controlled by Guix
> or should for some other reason be the only directory to consider by
> udev. I question that assumption.

In the context of Guix System, which I'd argue is the only context that
matters here, the /etc/udev/rules.d directory *is* controlled by Guix
and is the only directory that should matter, since the
udev-service-type populates it with the rules to be used by the system,
as configured by an operating system definition.

Am I missing something?

--
Thanks,
Maxim
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 63508@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 63508
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch
You may also tag this issue. See list of standard tags. For example, to set the confirmed and easy tags
mumi command -t +confirmed -t +easy
Or, remove the moreinfo tag and set the help tag
mumi command -t -moreinfo -t +help