Message ID | e7aecfbe4642b43ea5837584b7666c87098ef15f.1684370595.git.felix.lechner@lease-up.com |
---|---|
State | New |
Headers | show |
Series | [bug#63508,v2,1/4] gnu: eudev: Convert native-inputs to new style. | expand |
Am Mittwoch, dem 17.05.2023 um 17:52 -0700 schrieb Felix Lechner: > 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. > 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
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: > > 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 [1] https://lists.gnu.org/archive/html/guix-devel/2023-05/msg00217.html
Am Sonntag, dem 28.05.2023 um 16:23 -0700 schrieb Felix Lechner: > Hi Liliana, > > Thank you for your kind review! I will push a new version with > squashed commits you requested shortly. You mean "submit", right? > 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. > 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. > 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
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"