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

mouse-actions: init at 0.4.4 #265056

Merged
merged 2 commits into from
Jan 27, 2024
Merged

mouse-actions: init at 0.4.4 #265056

merged 2 commits into from
Jan 27, 2024

Conversation

rgri
Copy link
Contributor

@rgri rgri commented Nov 2, 2023

Description of changes

https://github.com/jersou/mouse-actions

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/)
  • 23.11 Release Notes (or backporting 23.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.

@NixOSInfra NixOSInfra added the 12. first-time contribution This PR is the author's first one; please be gentle! label Nov 2, 2023
@rgri rgri changed the title Add mouse actions Add mouse-actions Nov 2, 2023
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 0 This PR does not cause any packages to rebuild labels Nov 2, 2023
Copy link
Member

@donovanglover donovanglover left a comment

Choose a reason for hiding this comment

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

Thanks for contributing to nixpkgs!

pkgs/tools/inputmethods/mouse-actions/default.nix Outdated Show resolved Hide resolved
pkgs/tools/inputmethods/mouse-actions/default.nix Outdated Show resolved Hide resolved
pkgs/tools/inputmethods/mouse-actions/default.nix Outdated Show resolved Hide resolved
pkgs/tools/inputmethods/mouse-actions/default.nix Outdated Show resolved Hide resolved
pkgs/tools/inputmethods/mouse-actions/default.nix Outdated Show resolved Hide resolved
pkgs/tools/inputmethods/mouse-actions/default.nix Outdated Show resolved Hide resolved
@rgri
Copy link
Contributor Author

rgri commented Nov 2, 2023

Alright thanks for the criticism! ^-^ I think I've resolved everything so far. However not too sure how I should organize my commits, is it ok for there to be so many? Specifically the last one which moves and renames default.nix.

@donovanglover
Copy link
Member

1 for the maintainer and 1 for the package would be ideal 👍

@rgri
Copy link
Contributor Author

rgri commented Nov 2, 2023

Alright, I think I've done it.

@donovanglover
Copy link
Member

Result of nixpkgs-review pr 265056 run on x86_64-linux 1

1 package failed to build:
  • mouse-actions

I got it to run after changing cargoHash but it crashes for me. Don't know how we usually handle groups in nixpkgs.

[2023-11-02T19:16:00Z ERROR mouse_actions::process_args] Grab Error: IoError(
        Os {
            code: 13,
            kind: PermissionDenied,
            message: "Permission denied",
        },
    )
[2023-11-02T19:16:00Z ERROR mouse_actions::process_args] The user must be in the file group of /dev/input files, usually 'input' or 'plugdev' :
[2023-11-02T19:16:00Z ERROR mouse_actions::process_args]   $ sudo usermod -a -G input $USER
[2023-11-02T19:16:00Z ERROR mouse_actions::process_args]   $ sudo usermod -a -G plugdev $USER
[2023-11-02T19:16:00Z ERROR mouse_actions::process_args]   $ sudo tee /etc/udev/rules.d/80-mouse-actions.rules <<<'KERNEL=="uinput", SUBSYSTEM=="misc", TAG+="uaccess", OPTIONS+="static_node=uinput"'
[2023-11-02T19:16:00Z ERROR mouse_actions::process_args] Then restart to apply this user modifications.
[2023-11-02T19:16:00Z ERROR mouse_actions::process_args] See https://github.com/jersou/mouse-actions#requirement-

@rgri
Copy link
Contributor Author

rgri commented Nov 2, 2023

I wasn't sure how to do this either. The upstream doesn't give any solution besides just usermod.

@donovanglover
Copy link
Member

Unfortunately I can't approve since I'm unfamiliar with how to proceed from here. Maybe someone else can help out.

@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 10.rebuild-linux: 1 and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 0 This PR does not cause any packages to rebuild labels Nov 2, 2023
@jfvillablanca
Copy link
Member

can you squash your commits from the review, ideally there should only be 2 commits

@FliegendeWurst
Copy link
Member

I got it to run after changing cargoHash but it crashes for me. Don't know how we usually handle groups in nixpkgs.

Usually a programs.<name>.enable or services.<name>.enable nixos option is added that configures the required permissions.

@rgri
Copy link
Contributor Author

rgri commented Nov 3, 2023

Usually a programs..enable or services..enable nixos option is added that configures the required permissions.

Alright, I'll look into this tomorrow morning. Would this mean that it technically doesn't work on distros which aren't NixOS, and I should update meta.platforms?

@FliegendeWurst
Copy link
Member

FliegendeWurst commented Nov 3, 2023

Alright, I'll look into this tomorrow morning. Would this mean that it technically doesn't work on distros which aren't NixOS

The package itself can be run on other distros, the user then just needs to set their groups and udev rules manually.

Besides that I don't think it's possible to exclude other distros using meta.platforms. meta.platforms essentially only states for which [OS, CPU architecture] combinations the package should work.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/1209

@FliegendeWurst FliegendeWurst added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Nov 12, 2023
@rgri
Copy link
Contributor Author

rgri commented Nov 20, 2023

Anything I can do at this point to help get this merged?

@ludat
Copy link
Contributor

ludat commented Jan 22, 2024

I also would like this merged, is there something missing?

@h7x4 h7x4 changed the title Add mouse-actions mouse-actions: init at 0.4.4 Jan 22, 2024
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild and removed 10.rebuild-darwin: 1 10.rebuild-darwin: 1-10 labels Jan 25, 2024
nixos/modules/programs/mouse-actions.nix Outdated Show resolved Hide resolved
nixos/modules/programs/mouse-actions.nix Outdated Show resolved Hide resolved
@rgri rgri force-pushed the init-mouse-actions branch 2 times, most recently from a69ca8a to e94b24e Compare January 25, 2024 18:22
Copy link
Contributor

@eclairevoyant eclairevoyant left a comment

Choose a reason for hiding this comment

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

second commit msg can be cleaned up to just one line, otherwise lgtm

@rgri
Copy link
Contributor Author

rgri commented Jan 25, 2024

I removed the squashed messages, but am leaving the "Co-authored by [...]" lines. Is that OK?

Co-authored-by: Donovan Glover <[email protected]>
Co-authored-by: Arne Keller <[email protected]>
@FliegendeWurst
Copy link
Member

I removed the squashed messages, but am leaving the "Co-authored by [...]" lines. Is that OK?

Yes, that is how it is supposed to be done.

@donovanglover donovanglover added 12.approvals: 3+ This PR was reviewed and approved by three or more reputable people and removed 12.approvals: 1 This PR was reviewed and approved by one reputable person labels Jan 26, 2024
@wegank wegank merged commit 7d021ca into NixOS:master Jan 27, 2024
28 checks passed
@rgri rgri mentioned this pull request Jan 27, 2024
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes 12.approvals: 3+ This PR was reviewed and approved by three or more reputable people 12. first-time contribution This PR is the author's first one; please be gentle!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants