[bug#63508,core-updates,v5,0/3] eudev: Build with udevrulesdir pointing to /etc/udev/rules.d

  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
  

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 ‾\_(ツ)_/‾

Cheers
  

Hello,

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

> 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:

> 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?
  

Am Donnerstag, dem 06.02.2025 um 18:12 +0900 schrieb Maxim Cournoyer:
> 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.¹

> 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.  (:
  

Hi Liliana,

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

[...]

>> 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).

> 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?
  

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.

Cheers
  

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?
  

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

Cheers
  

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?

Kind regards
Felix
  

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.

Cheers
  

Hi Liliana,

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

> 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.
  

Hi Liliana,

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

> 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?
  

Hi,

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> Hi Liliana,
>
> Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
>
>> 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?

I've merged this to the elogind-updates branch that is coming for merge
soon.  We can always revisit anything I've overlooked later, but I'm
convinced this is an improvement over the status quo.