Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add mode and group to udev rules #23521

Closed
wants to merge 1 commit into from
Closed

Conversation

ismay
Copy link

@ismay ismay commented Apr 14, 2024

This updates the udev rules to work on non-systemd distros as well. See #23517.

Description

See the linked issue.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

closes #23517

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@tzarc
Copy link
Member

tzarc commented Apr 14, 2024

In its current form we cannot accept this PR. This invalidates other distro's that don't set up a plugdev group -- anything systemd-based should be running with TAG+="uaccess" instead, as per the systemd developers (https://bugzilla.redhat.com/show_bug.cgi?id=815093#c2).

For example, with my main Fedora machine I don't have a plugdev group -- if we merge this PR should I subsequently raise a bugfix PR reverting to TAG+="uaccess" as that fixes my system?

The vast majority of distros use systemd nowadays, and whilst you may not find that selection palatable, you're the outlier.

From your perspective I'd suggest manually modifying the rules and installing that as per your system requirements, or figuring out an automated way to select one of two different rules files -- with preference being the TAG+="uaccess" route wherever available.

@tzarc
Copy link
Member

tzarc commented Apr 14, 2024

Remind me to read the actual changed lines next time -- I see the TAG+=uaccess wasn't removed. Let me confirm that this still functions correctly.

@classabbyamp
Copy link

having both MODE/GROUP and TAG makes it work with and without systemd/elogind. it's a common pattern across packages on distros without mandatory systemd/elogind

@ismay
Copy link
Author

ismay commented Apr 17, 2024

@tzarc By the way, in terms of confirming, on one of these lines the MODE and GROUP has been present for a long time already. Without issue as far as I can tell. Good to manually verify of course, but just so you know. Plus, as @classabbyamp mentions, it's a common approach.

@ismay ismay closed this by deleting the head repository Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Most rules in 50-qmk.rules are missing MODE and GROUP for non-systemd distros
3 participants