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

Config dir, log file and lock file command line options #70

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

drdo
Copy link

@drdo drdo commented Dec 26, 2021

Adds command line options for config dir, log file and lock file.

Keeps full backwards compatibility,
the default is to use the hardcoded CONFIG_DIR, LOG_FILE and LOCK_FILE.

Additionally took the opportunity to rewrite the command line option parsing using getopt.

@rvaiya
Copy link
Owner

rvaiya commented Dec 26, 2021

Thanks for the patch. Unfortunately I'm not sure making these configurable at runtime is a good idea. Is there a particular problem you are trying to solve? See also #7, and #41.

@drdo
Copy link
Author

drdo commented Dec 26, 2021

I don't want to keep keyd configuration in /etc/keyd.

I also don't run it as root. It's not really mandatory to run this as root, on most distros if the user is in the "input" group you can do everything just fine.

@rvaiya
Copy link
Owner

rvaiya commented Dec 26, 2021

The best way to achieve this is to symlink your config directory to /etc. This makes it clear that the config applies to the entire system. Even if you have access to /dev/input and /dev/input/event/* you can't run concurrent instances of the program anyway.

@drdo
Copy link
Author

drdo commented Dec 26, 2021

I truly do not understand the downside to being able to override the default configuration location.

@rvaiya
Copy link
Owner

rvaiya commented Dec 26, 2021

Being able to modify the config directory at runtime gives the user the false impression that it is a user level process. Keeping it hard coded to /etc/keyd makes it clear that keyd is a system wide daemon like sshd or systemd.

If you really want to change the default config directory for some unorthodox system you can do so at compile time.

Additionally your patch allows changing the path of the lock file which would allow multiple instances to be run at once.

Finally, running as root is also mostly a non issue unless you have a weird threat model. If a malicious process has access to your input devices your system is effectively compromised anyway.

@drdo
Copy link
Author

drdo commented Dec 26, 2021

My particular use case is that I run keyd via a systemd service for my user, not root.

This is convenient for a few reasons. For example:

  • I have several distros and sometimes need to try something else and it always works.
  • When my girlfriend is using the computer and logs into her account she doesn't get my keyboard config.

@rvaiya
Copy link
Owner

rvaiya commented Dec 26, 2021

I have several distros and sometimes need to try something else and it always works.

Can you clarify what you mean by this? If you are suggesting that some distros dont have /etc (I'm not aware of any) you can just modify the config path at compile time when you install the daemon.

When my girlfriend is using the computer and logs into her account she doesn't get my keyboard config.

This is a legitimate issue. However running as a user process spawned by systemd doesn't change the fact that multiple users can simultaneously be logged into the same machine (and consequently multiple keyd processes may be active).

Unfortunately there isn't a good way to solve this. Properly accommodating multi-user setups would require keyd to get into the business of seat/session management. At the moment it is really intended for single user setups. Having said that, there is nothing stopping you from having some session aware process kill the daemon when appropriate on your machine.

Alternatively you could use the approach taken by most other operating systems and create a dedicated layout that can be toggled with a specific key combination.

@drdo
Copy link
Author

drdo commented Dec 26, 2021

This is rather unfortunate. I thought this would be an uncontroversial option.

Are you not willing have configurable file paths at all?

@rvaiya
Copy link
Owner

rvaiya commented Dec 26, 2021

I am open to considering configurable paths if a good case can be made, however at the moment it is unclear what problem they solve. The lack of multi user support is a real issue that has been discussed elsewhere, but isn't one that is easily remedied by changing the config directory.

My concern is that these options will potentially cause more confusion and imply that the program can be used in ways that it cannot.

@drdo
Copy link
Author

drdo commented Dec 26, 2021

I understand your concerns, specially about the lock file.

Would you be willing to consider then something a bit more constrained?

When running as root everything would be the same with the default paths.

When running as a non-root user:

  • Looking for theconfig dir in $XDG_CONFIG_HOME/keyd (or $HOME/.config/keyd if XDG_CONFIG_HOME is undefined), /etc/keyd. In this order.
  • Creating the lock file in $XDG_STATE_HOME/keyd.lock (or $HOME/.local/state/keyd.lock if XDG_STATE_HOME is undefined)

Regarding the log file. I suggest simplifying here and simply dropping the -d daemonize option and always logging to stdout/stderr. This is almost always what you would want when running keyd via systemd.
If choosing the keep this option, I suggest something similar to the above.

@rvaiya
Copy link
Owner

rvaiya commented Dec 26, 2021

Looking for theconfig dir in $XDG_CONFIG_HOME/keyd (or $HOME/.config/keyd if XDG_CONFIG_HOME is undefined), /etc/keyd. In this order.
Creating the lock file in $XDG_STATE_HOME/keyd.lock (or $HOME/.local/state/keyd.lock if XDG_STATE_HOME is undefined)

The problem with this approach is that it still allows multiple instances of keyd to be launched simultaneously.

Consider what happens when two users try and launch an instance at the same time. Only one instance can manage the input device at a given time and there is no way for the two processes to negotiate which one should take precedence.

This only works if one user is running the process at a given time and is courteous enough to clean up for the next user which logs in (which would require detecting session changes in the event that the original user does not log out).

At this point it is better to drop the pretense of multi-user support altogether and just have the interested user manage the system process.

Regarding the log file. I suggest simplifying here and simply dropping the -d daemonize option and always logging to stdout/stderr. This is almost always what you would want when running keyd via systemd.

Agreed, but the flag is still useful for systems which haven't jumped on the systemd bandwagon (there are still a few).

@drdo
Copy link
Author

drdo commented Dec 26, 2021

Still regarding the lock file.

Do we really need this? Can't you detect that some other instance (or even some other program) is already grabbing the device when you attempt to grab it in keyd?

@rvaiya
Copy link
Owner

rvaiya commented Dec 26, 2021

Can't you detect that some other instance (or even some other program) is already grabbing the device when you attempt to grab it in keyd?

The grabs will silently fail rather than causing keyd to terminate. Having a dedicated lockfile is a better failure mode since it allows keyd to keep running in the event that some input devices are not grabbable (which can happen for a variety of reasons).

In any event, even if we failed eagerly multi user setups would still present the same problems.

@rvaiya rvaiya force-pushed the master branch 2 times, most recently from 5feea0c to ad6d734 Compare December 27, 2021 10:21
@rvaiya rvaiya force-pushed the master branch 2 times, most recently from 2de8d4e to 05d994b Compare January 10, 2022 03:44
@rvaiya rvaiya force-pushed the master branch 3 times, most recently from 88edf1b to 013f1e5 Compare March 26, 2022 08:33
@rvaiya rvaiya force-pushed the master branch 4 times, most recently from ea25d82 to b04ce33 Compare April 1, 2022 23:57
@rvaiya rvaiya force-pushed the master branch 2 times, most recently from a5447c5 to 8f0727c Compare April 11, 2022 06:23
@edrex
Copy link

edrex commented Dec 20, 2022

I have a use case. I want to manage my user environment, including keyd and its config, via my dotfiles repo. NixOS doesn't know about my user config, and I don't want it to, so making a symlink from etc is not desirable. I would very much like to use keyd, but have been blocked for just over a year now by the lack of a config location cmdline option. @rvaiya could you possibly reconsider? It feels like quite a hard line. Thanks.

@rvaiya
Copy link
Owner

rvaiya commented Dec 23, 2022

@edrex Can you elaborate on your use case? I've been meaning to revisit the issue by adding proper multi user support, which is why I've left this open.

I'm still opposed to making config directory configurable at runtime for the reasons outlined above, but am open to persuasion. The daemon will require root (or at least access to /dev/input) no matter what you do, so it is unclear what problem this solves (and why you can't just create a symlink)

@edrex
Copy link

edrex commented Dec 25, 2022

I realize after some thought there are two main classes of use case for an input remapper:

  1. as a "soft firmware" for globally modifying the event stream for input devices
  2. in order to runtime monkeypatch "contextual" (ie shell and application specific) bindings

Both use cases are a kind of hack, but the first is more elegant and an appropriate use of root access, since the kernel is above the input hardware.

As for the second class.. using root access for them is inherently janky: why not change the bindings directly in the shell/application (even if you have to patch/recompile it)?

Even if contextual input rewriting is needed, this can better be done by the shell running on the tty, without needing root. Most wayland compositors provide a binding system (hyprland even supports global passthrough binds). There are also wlroots' virtual-pointer and virtual-keyboard for synthesizing events (update: and libei which I think is what gnome/kde are using). Lastly, maybe libinput could be used to create a bolt-on in-process input rewriting library with a user-facing config interface that shells can use?

Anyway, I think my answer is: maybe I should replace my custom keyboard hwdb rules with keyd, but I should look to higher level solutions for my contextual mappings.

@rvaiya
Copy link
Owner

rvaiya commented Dec 26, 2022

  1. in order to runtime monkeypatch "contextual" (ie shell and application specific) bindings

[...]

using root access for them is inherently janky: why not change the bindings directly in the shell/application (even if you have to patch/recompile it)?

I generally agree with this, but the two are not mutually exclusive. Many applications don't provide a mechanism to rebind [all] keys which is why keyd-application-mapper is useful. It's also worth noting that users don't actually need root access to achieve this, they just need access to the keyd socket (though this should probably be treated as defacto root access given the opportunity for abuse). In any event, it's not really relevant to this issue.

Most wayland compositors provide a binding system (hyprland even supports global passthrough binds).
There are also wlroots' virtual-pointer and virtual-keyboard for synthesizing events

Yes, but there is no way (as far as I know) to add a global key hook. I doubt such a thing would ever be implemented since it appears to be contrary to one of wayland's design goals (which is one of the reasons keyd uses evdev).

Lastly, maybe libinput could be used to create a bolt-on in-process input rewriting library with a user-facing config interface that shells can use?

My understanding is that this is out of scope for libinput. It would also limit keyd's utility (libinput is not used everywhere). keyd was designed to handle multiple input sources and sinks, so it should be fairly easy to inegrate with any input system which provides the right hooks (e.g libinput). I already have experimental macos and windows ports sitting in a private repo.

Anyway, I think my answer is: maybe I should replace my custom keyboard hwdb
rules with keyd, but I should look to higher level solutions for my contextual
mappings.

+1. If in-application remapping facilities allow you to do what you want, and you don't mind spreading your bindings out across multiple configs, it might be worth avoiding keyd-application-mapper altogether. I do this for terminal applications, since the alternative is to rely on ugly title setting hacks (and modal editors like vim need in-app context anyway).

@edrex
Copy link

edrex commented Dec 27, 2022

Yes, but there is no way (as far as I know) to add a global key hook

It's true that there's no portable hook wayland clients can use to register a binding. I think such a protocol for this would be golden. I like the way @reificator put it in two HN responses to the question "Why would you ever let a non-focused application subscribe to any key combinations?":

It's a massive security issue that any installed software can listen to any input activity or view/affect other windows, and that does need to be reigned in. But to claim that there's no legitimate utility to global hotkeys is absurd. We need robust permissions, not a completely crippled experience.

Just let my application tell the window manager:

  • What key events I care about. (I.E. what the keybind should be named in the settings UI)
  • What script or message to pass back to me when that event is activated.
  • What the default keycombo should be, if any.

As mentioned above, Hyprland does support global binding via its pass dispatcher which globally passes events for a binding to a specified client. Bindings can be changed over IPC, satisfying the need for a hook. This is what I am planning to use for dispatching global bindings, instead of something running as root like keyd.

I doubt such a thing would ever be implemented since it appears to be contrary to one of wayland's design goals (which is one of the reasons keyd uses evdev).

What is the design goal you mean? I don't get a lot of the polarized positions people take around wayland, being a pragmatist at heart.

My understanding is that this is out of scope for libinput.

Makes sense. https://wayland.freedesktop.org/libinput/doc/latest/faqs.html#can-i-write-a-program-to-make-libinput-do-foo
.I was more thinking an indirection layer using libinput that makes it easy for processes already using libinput to implement an in-process event dispatch registry, possibly with a standard IPC interface (socket probably, or dbus, or wayland protocol, as a user I don't care). Does this make sense?

It would also limit keyd's utility (libinput is not used everywhere).

Absolutely. I guess I don't care as much about extreme portability, as long as it works across wayland compositors.

keyd was designed to handle multiple input sources and sinks, so it should be fairly easy to inegrate with any input system which provides the right hooks (e.g libinput). I already have experimental macos and windows ports sitting in a private repo.

What would a keyd libinput backend look like? Would it be an in-process library or what?

@edrex
Copy link

edrex commented Dec 27, 2022

This is pretty off-topic for the keyd issue tracker, esp this PR. One more link and I'm done: [RFC] Add action binder protocol (!56) · Merge requests · wayland / wayland-protocols · GitLab

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants