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

nixos/keymapper: init module #328921

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

spitulax
Copy link
Contributor

@spitulax spitulax commented Jul 21, 2024

Description of changes

Added NixOS module services.keymapper to enable keymapperd service used to provide communication between the user-level program keymapper which needs permission to grab keyboard and inject keys.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation This PR adds or changes documentation 8.has: changelog 8.has: module (update) This PR changes an existing module in `nixos/` labels Jul 21, 2024
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Jul 21, 2024
Copy link
Member

@felbinger felbinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have quite a complex logic to create the configuration.

What do you think of a module test to ensure that the configuration is created correctly?

, lib
, ...
}:
with lib;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use inherit (lib) mkOption types ...; instead, see #208242

in
{
options.services.keymapper = {
enable = lib.mkEnableOption null // {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replace null with "keymapper", to generate documentation correctly

@spitulax spitulax requested a review from felbinger July 22, 2024 12:31
Copy link
Member

@felbinger felbinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not checked the logic for creating the configuration. Maybe that can be improved.

@felbinger
Copy link
Member

@ofborg test keymapper

@h7x4 h7x4 added 8.has: module (new) This PR adds a module in `nixos/` 8.has: tests This PR has tests labels Jul 23, 2024
@felbinger
Copy link
Member

felbinger commented Jul 25, 2024

A github workflow has been adjusted (#326407), could you trigger the ci again by pushing a new commit?
You should be required to reformat the files:

nix-shell
nixfmt 'nixos/tests/keymapper.nix' 'nixos/modules/services/hardware/keymapper.nix'

@spitulax
Copy link
Contributor Author

The new Github workflow should have been run though. I checked the Github action on commit 99b8d5e and it seems to use the new workflow. Do I still have to retrigger it?

@felbinger
Copy link
Member

@spitulax Just so you know, you need to cleanup the commit history (by git rebase), before this can be reviewed and merged.

Added option to enable `keymapperd` service used to provide
communication between the user-level program `keymapper` which needs
permission to grab keyboard and inject keys.
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog 8.has: documentation This PR adds or changes documentation 8.has: module (new) This PR adds a module in `nixos/` 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: tests This PR has tests 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants