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

libratbag: init at v0.9.903 #46274

Merged
merged 2 commits into from
Sep 21, 2018
Merged

libratbag: init at v0.9.903 #46274

merged 2 commits into from
Sep 21, 2018

Conversation

mvnetbiz
Copy link
Contributor

@mvnetbiz mvnetbiz commented Sep 7, 2018

Motivation for this change

Libratbag is a daemon and utility for configuring DPI, buttons, etc. of programmable gaming mice.

Things done

Added package for libratbag and service module ratbagd.
Added piper: gtk frontend app for ratbagctl

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Fits CONTRIBUTING.md.

@GrahamcOfBorg GrahamcOfBorg added 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 labels Sep 7, 2018
@jtojnar
Copy link
Member

jtojnar commented Sep 8, 2018

See also #35516

pkgs/os-specific/linux/libratbag/default.nix Outdated Show resolved Hide resolved
};

nativeBuildInputs = [
(python3.withPackages (ps: with ps; [ evdev pygobject3 ]))
Copy link
Member

Choose a reason for hiding this comment

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

This should be moved to buildInputs.

Copy link
Contributor Author

@mvnetbiz mvnetbiz Sep 10, 2018

Choose a reason for hiding this comment

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

If this is moved to buildInputs, the patch shebangs for "ratbagctl" gives the wrong python, and program fails because it can't import the right modules. This seemed like the wrong place to put it based on nixos manual's description of the different build inputs, but I am not sure how to get around that.

Copy link
Member

Choose a reason for hiding this comment

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

The issue is probably that meson propagates its own python, it is going to be fixed in #46020. For now, could you add a TODO comment saying that Python should go to buildInputs once #46020 is merged?

# Give users access to the "ratbagctl" tool
environment.systemPackages = [ pkgs.libratbag ];

systemd.services.ratbagd = {
Copy link
Member

Choose a reason for hiding this comment

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

You should be able to use systemd.packages = [ pkgs.libratbag ];

pkgs/os-specific/linux/libratbag/default.nix Outdated Show resolved Hide resolved
@mvnetbiz
Copy link
Contributor Author

I've included your suggestions, thanks.

environment.systemPackages = [ pkgs.libratbag ];

systemd.packages = [ pkgs.libratbag ];
systemd.services.ratbagd = {
Copy link
Member

@jtojnar jtojnar Sep 13, 2018

Choose a reason for hiding this comment

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

Why did you keep this? It should already be specified in the service file in the package, which is installed by the systemd.packages option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't figure out of that actually used the packages own unit file. It didn't seem to work for me when I removed my own definition. Is there a way to point it to where the build outputs it?

Copy link
Member

Choose a reason for hiding this comment

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

It does, only the servcie file does not have wantedBy. You can do systemd.services.ratbagd.wantedBy = [ "multi-user.target" ];, which will add the key to the override.

I wonder why the upstream service does not have that, though – maybe the daemon is supposed to be launched using a different method.

@jtojnar
Copy link
Member

jtojnar commented Sep 13, 2018

Do you plan to package piper as well? It would be nice to have some app to test if this is working.

@mvnetbiz
Copy link
Contributor Author

I was going to do that too. Would that be a separate PR? Libratbag package does have a command line that I have tested with my Logitech G403.

@jtojnar
Copy link
Member

jtojnar commented Sep 14, 2018

For closely related packages like this, a single PR is fine.

@mvnetbiz
Copy link
Contributor Author

I was looking at packaging piper and got stuck on something. The piper application is a python script that includes a import like "import piper.application" in itself. Since meson propagates the python from the nativebuildinputs into this piper script, the python env it runs in does not include the piper package.
Do I need to make a python module in nix for piper, and then include a python with piper in the buildinputs and patch the script after build to use that? I am a bit lost here :(

@jtojnar
Copy link
Member

jtojnar commented Sep 18, 2018

I am aware of the issue and it will be fixed in the next meson release (#46020). At the moment we try to fix this by adding python before meson in nativeBuildInputs but I am not sure 100% sure it will work for Python programs. What expression do you have?

@mvnetbiz
Copy link
Contributor Author

Pushed my expression for piper

@mvnetbiz
Copy link
Contributor Author

I patched out 2 lines from meson.build. One is it can't verify python gobject is installed. This must have something to do with pkg-config? I don't know how any of that works. And the other line includes a script that does something with gtk-update-icon-cache which fails. I think we don't need icon cache in the store anyways?

@jtojnar
Copy link
Member

jtojnar commented Sep 18, 2018

Yep, only packages listed in build inputs get added to the PKG_CONFIG_PATH. You can add pygobject there.

Right, gtk-update-icon-cache is not good for us, we have hicolor-icon-theme setup hook removing the generated cache file for us, though, maybe we should still run the file in case the glib-compile-schemas is ever uncommented. See, for example, this for how to make it runnable:

postPatch = ''
chmod +x meson_post_install.py # patchShebangs requires executable file
patchShebangs meson_post_install.py
'';

@mvnetbiz
Copy link
Contributor Author

Ok I've fixed up piper, using the python builder now. It works with my logitech mouse.

@mvnetbiz
Copy link
Contributor Author

Is there anything else I need to do? Should I squash commits or anything?

@jtojnar
Copy link
Member

jtojnar commented Sep 20, 2018

Each package should be in a separate commit; other than that, it looks good.

Add package libratbag and service module ratbagd
Libratbag contains ratbagd daemon and ratbagctl cli to configure
buttons, dpi, leds, etc. of gaming mice.
Add mvnetbiz to maintainers.
Add package piper. Piper is a gtk frontend application for ratbagctl
to configure gaming mice.
@mvnetbiz
Copy link
Contributor Author

Ok thanks. I have split the commits and updated the PR description to include piper.

@jtojnar
Copy link
Member

jtojnar commented Sep 21, 2018

Thank you.

@jtojnar jtojnar merged commit 93408ae into NixOS:master Sep 21, 2018
@mvnetbiz mvnetbiz deleted the libratbag branch November 28, 2020 03:24
@Janik-Haag Janik-Haag added the 12. first-time contribution This PR is the author's first one; please be gentle! label Jun 12, 2023
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 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.

4 participants