bug#63787: udev-rules-service inoperable for some rules

bug#63787: udev-rules-service inoperable for some rules

[Felix Lechner via Bug reports for GNU Guix]

Hi,

A brief thread from guix-devel about trying to use MAC-based names for
network interfaces [1] shall be incorporated herein by reference.

  [1] https://lists.gnu.org/archive/html/guix-devel/2023-05/msg00192.html

On Wed, May 17, 2023 at 6:14 PM Felix Lechner
<felix.lechner@lease-up.com> wrote:
>
> In a concession to Liliana's opposition, I retitled the bug to
> sidestep the question of the MAC-based names as a default setting.

It was not enough of a concession.

In Guix, many custom rules like the following—namely those that use
values determined by udevadm—are inoperable.

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

The issue is that udevadm looks in the installation folder for udev
rules. In standard distros, that works fine because the installation
folder and the runtime folder are the same. Unfortunately, that is not
so for Guix. The installation folder is in the store.

The way I understand our strategy in Guix, we construct another,
aggregate folder with links to rules in /etc/udev/rules.d. (That
location also happens to be the installation directory for many
traditional distros.) I proposed a local patch that causes udevadm to
look in that folder instead. [2] It replaces one line in the source
code.

  [2] https://issues.guix.gnu.org/63508#12

Liliana, who kindly reviewed my patch, disagreed with that solution.
For reasons I do not fully understand, she prefers a more generalized
solution via an environmental variable called EUDEV_RULES_DIRECTORY.
[3] As far as I can tell, that variable is not provided by upstream.
It was invented by a custom patch to Guix [4] and currently does not
seem to work all the way.

  [3] https://issues.guix.gnu.org/63508#6
  [4] https://git.savannah.gnu.org/cgit/guix.git/tree/gnu/packages/patches/eudev-rules-directory.patch

Liliana insists on improving the environment variable
EUDEV_RULES_DIRECTORY, while I have several issues with that. For
starters, my substitution is simpler. It is also a common packaging
strategy in Guix. (I furthermore cannot envision setting the variable
to any value other than /etc/udev/rules.d, although maybe the
flexibility comes in handy one day.) Mainly, however, I believe that
her patch goes beyond the typical packaging activity.

By implementing a new feature, the patch for EUDEV_RULES_DIRECTORY
should really be sent upstream. I do not know whether that's already
happened, but I am not convinced upstream would accept it.

In the latest 3.2.12 release, which does not work without further
modifications in Guix, the upstream developers added a command-line
option called '--root' to udevadm [5] that offers another solution to
setting the runtime path. Unfortunately, that option does not change
the default, which is the issue in this bug.

  [5] https://github.com/eudev-project/eudev/blob/2703baf55615b7554fb67c4f1c241f057f8c0a79/man/udevadm.8#L378C2-L380

Finally, programming styles also differ. I have philosophical
objections to the use of pointer arithmetic, although the upstream
code may have some of that, too.

With the discussion at an impasse, I am not sure how to proceed.
Eudev, which is an essential part of any modern Linux operating
system, is not working properly in Guix.

Liliana challenged me to "file a formal complaint." [6] I do not know
what that means and now filed this bug in hope of finding an
acceptable way forward. Maybe it is easier to discuss the reasoning
for one fix or another when the arguments for and against are not
separated by multiple versions of competing inline patches.

  [6] https://issues.guix.gnu.org/63508#22

For the success of any group, it is essential that problems are being
solved. Perhaps someone with an independent perspective would be so
kind to comment and help bring this bug one step closer to being
closed. Thanks!

Kind regards,
Felix

cc: Liliana

bug#63787: udev-rules-service inoperable for some rules

[Liliana Marie Prikler]

Hi Felix,

Am Montag, dem 29.05.2023 um 08:25 -0700 schrieb Felix Lechner:
> Hi,
> 
> A brief thread from guix-devel about trying to use MAC-based names
> for network interfaces [1] shall be incorporated herein by reference.
> 
>   [1]
> https://lists.gnu.org/archive/html/guix-devel/2023-05/msg00192.html
> 
> On Wed, May 17, 2023 at 6:14 PM Felix Lechner
> <felix.lechner@lease-up.com> wrote:
> > 
> > In a concession to Liliana's opposition, I retitled the bug to
> > sidestep the question of the MAC-based names as a default setting.
> 
> It was not enough of a concession.
Heads up, that was not a nice way of phrasing things – at least as I, a
non-native English speaker take it.  To me, "concession" implies some
backdrop of a fight between opposing forces, whereas I do see the
problem you're facing but want to find a better solution.  In other
words, we share a goal.

> In Guix, many custom rules like the following—namely those that use
> values determined by udevadm—are inoperable.
> 
> >             (udev-rules-service 'net-name-mac
> >                                 (udev-rule
> >                                  "79-net-name-mac.rules"
> >                                  "
> > SUBSYSTEM==\"net\", ACTION==\"add\", NAME=\"$env{ID_NET_NAME_MAC}\"
> > ")))
> 
> The issue is that udevadm looks in the installation folder for udev
> rules.  In standard distros, that works fine because the installation
> folder and the runtime folder are the same.  Unfortunately, that is
> not so for Guix.  The installation folder is in the store.
For the record, I'm not sure whether udevadm is the sole component at
play here.  Sure, it's a tool you may invoke at the command line, but
the big daemon thingy, that's udevd.

> The way I understand our strategy in Guix, we construct another,
> aggregate folder with links to rules in /etc/udev/rules.d. (That
> location also happens to be the installation directory for many
> traditional distros.) I proposed a local patch that causes udevadm to
> look in that folder instead. [2] It replaces one line in the source
> code.
> 
>   [2] https://issues.guix.gnu.org/63508#12
> 
> Liliana, who kindly reviewed my patch, disagreed with that solution.
> For reasons I do not fully understand, she prefers a more generalized
> solution via an environmental variable called EUDEV_RULES_DIRECTORY.
> [3] As far as I can tell, that variable is not provided by upstream.
> It was invented by a custom patch to Guix [4] and currently does not
> seem to work all the way.
> 
>   [3] https://issues.guix.gnu.org/63508#6
>   [4]
> https://git.savannah.gnu.org/cgit/guix.git/tree/gnu/packages/patches/eudev-rules-directory.patch
Note: EUDEV_RULES_DIRECTORY was implemented by Ludo in 2014 [6, 7].  In
one year and a little bit it will celebrate ten years of being with us.
I think it is allowed to grow up.

> Liliana insists on improving the environment variable
> EUDEV_RULES_DIRECTORY, while I have several issues with that.
Just as I take issue with not using an environment variable for the
purpose it serves :)

> For starters, my substitution is simpler. 
Simpler is not always better.  One might say that overwriting files as
you install packages is simpler than worrying about setting the right
symlinks, but Guix, being a functional distribution, does the latter.

> It is also a common packaging strategy in Guix. (I furthermore cannot
> envision setting the variable to any value other than
> /etc/udev/rules.d, although maybe the flexibility comes in handy one
> day.) 
Supplying an environment where there previously was none is also a
common packaging strategy for Guix.  The fact that said environment
variable was already introduced prior to my patch should tell you as
much.

> Mainly, however, I believe that her patch goes beyond the typical
> packaging activity.
Note how said environment variable already exists prior to this patch.
It is well within typical packaging activity.

> By implementing a new feature, the patch for EUDEV_RULES_DIRECTORY
> should really be sent upstream. I do not know whether that's already
> happened, but I am not convinced upstream would accept it.
I doubt that I'd need upstream permission to modify local patches that
haven't been accepted upstream yet.  Granted, our patch could be sent
upstream, but at this point I feel like we've already diverged a little
bit from the usual scenario.

> In the latest 3.2.12 release, which does not work without further
> modifications in Guix, the upstream developers added a command-line
> option called '--root' to udevadm [5] that offers another solution to
> setting the runtime path. Unfortunately, that option does not change
> the default, which is the issue in this bug.
> 
>   [5]
> https://github.com/eudev-project/eudev/blob/2703baf55615b7554fb67c4f1c241f057f8c0a79/man/udevadm.8#L378C2-L380
You're again assuming that udevadm is the only component at play here.
I have yet to hear your reason for believing so.

> Finally, programming styles also differ. I have philosophical
> objections to the use of pointer arithmetic, although the upstream
> code may have some of that, too.
You may object to the use of pointer arithmetic, but it's not a
requirement for my solution; it's just how I chose to implement it. 
You can get a semantically equivalent patch without pointer arithmetic
and a virtually equivalent patch without either pointer arithmetic or
equivalent C code by assuming a default value for
EUDEV_RULES_DIRECTORY.  I simply didn't want to do any of those at the
time of writing.

> With the discussion at an impasse, I am not sure how to proceed.
> Eudev, which is an essential part of any modern Linux operating
> system, is not working properly in Guix.
> 
> Liliana challenged me to "file a formal complaint." [6] I do not know
> what that means and now filed this bug in hope of finding an
> acceptable way forward. Maybe it is easier to discuss the reasoning
> for one fix or another when the arguments for and against are not
> separated by multiple versions of competing inline patches.
By "fil[ing] a formal complaint", I meant to discuss my patch in more
than passing.  I would have liked for that to take place in the other
thread or the one you've started over at guix-devel, but I can also
take criticism here.  As for why I "challenged" you in the first place:
I take my time to read and reply to your patches, so by the principle
of reciprocity I assume it to be fair for you to do the same.  I feel
as though you don't give my words the consideration they deserve: I am
reading the same patch for the third time by now and you are still
using (regexp-quote ...) instead of manually quoting the nasty bits as
is done throughout the codebase.

Cheers

[6]
https://git.savannah.gnu.org/cgit/guix.git/patch/?id=c19ce3a711fea24c173d615a4a7b162dbc86ce68
[7]
https://git.savannah.gnu.org/cgit/guix.git/patch/?id=66a99a06761b2cf0aa3fa6d70e97e767ab237fcb