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

build libinput to look for local quirks in /etc #70520

Merged
merged 1 commit into from Nov 24, 2019

Conversation

telent
Copy link
Contributor

@telent telent commented Oct 6, 2019

Motivation for this change

If the user wants to configure libinput locally (e.g. to tune
touchpad sensitivity) the advertised[*] mechanism to do this
upstream is by adding a file /etc/libinput/local-overrides.quirks

This doesn't work in NixOS, which is instead looking at (e.g) /nix/store/9lwm03xqd8pkbxc3hgq9iiginddiyha3-libinput-1.12.6/etc/libinput/local-overrides.quirks

[*] can't quite claim it as "supported", but it's certainly the
mechanism that's publically described. See
https://wayland.freedesktop.org/libinput/doc/latest/device-quirks.html

Things done

Add --sysconfdir option to the libinput configurePhase so that we
build libinput to look for the file there instead of in libinput's
store path

  • Tested on my laptop using an overlay to override the libinput derivation [https://ww.telent.net/2019/10/2/light_touch_regulation]

  • building the virtualbox image nix-build nixos/release-combined.nix -A nixos.ova as I write, will update this when finished

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)

  • Built on platform(s)

    • NixOS
    • [n/a] 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 nix-review --run "nix-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)

  • Ensured that relevant documentation is up to date

  • Fits CONTRIBUTING.md.

Notify maintainers

cc @

If the user wants to configure libinput locally (e.g. to tune
touchpad sensitivity) the advertised[*] mechanism to do this
upstream is by adding a file /etc/libinput/local-overrides.quirks

Add --sysconfdir option to the libinput configurePhase so that we
build libinput to look for the file there instead of in libinput's
store path

[*] can't quite claim it as "supported", but it's certainly the
mechanism that's publically described.  See
https://wayland.freedesktop.org/libinput/doc/latest/device-quirks.html
@telent
Copy link
Contributor Author

telent commented Oct 6, 2019

It's arguable whether this is The Right Thing and should we instead be providing some declarative/structured way to add local device quirks. That said, upstream seem quite firm on the idea that local quirks are temporary and should be pushed to them for distribution in later libinput versions, so one could make a case for intentionally keeping our quirks in the same text-based format (ini files) as they will expect.

@matthewbauer
Copy link
Member

matthewbauer commented Oct 6, 2019

I think this is the right thing to do. /etc directory is declarative if you are using NixOS. Putting configuration in libinput's closure leads to needless rebuilds as well. I actually thought we set this globally for Meson! While autoconf defaults sysconfdir to /etc, Meson defaults to $prefix/etc. We should try to make this more consistent.

@worldofpeace worldofpeace removed their assignment Oct 6, 2019
@worldofpeace
Copy link
Contributor

I think this is the right thing to do. /etc directory is declarative if you are using NixOS. Putting configuration in libinput's closure leads to needless rebuilds as well. I actually thought we set this globally for Meson! While autoconf defaults sysconfdir to /etc, Meson defaults to $prefix/etc. We should try to make this more consistent.

I think you fixed something with fuse but didn't pr the change for that to happen.
Would be right to have that change though, but being careful since we have all those sysconfdir_install patches.

@joepie91
Copy link
Contributor

Even if this isn't philosophically the right solution, we need some way to add libinput quirks locally. Right now I can't use my double-click button on my RollerMouse Free3 because upstream does not have it in quirks yet, and the quirks mechanism in NixOS is broken.

Unfortunately that means that it is currently a pain (quite literally, ergonomically) to use my mouse. I'm dependent on the functioning of that button to reduce my RSI issues.

What needs to happen to get some sort of solution applied here ASAP?

@worldofpeace
Copy link
Contributor

@joepie91 I don't think anyone said we didn't need this fix, in the end libinput needs to be configured to find configs from /etc if we wanted to support this in the module.

@joepie91
Copy link
Contributor

@worldofpeace Right, but from the above conversation it feels to me as if this PR is going to get stuck in "what would the philosophically correct solution be" limbo while leaving the immediate issue unsolved. As this is a significant health issue, that would be a serious problem to me.

I'd rather just see an immediate solution applied (and backported to stable), and - if necessary - that solution getting replaced with a more generic/philosophically-correct one later.

@telent
Copy link
Contributor Author

telent commented Oct 30, 2019

I agree with @joepie19. I patched it locally, for me, by adding an overlay in /etc/nixos/configuration.nix - https://ww.telent.net/2019/10/2/light_touch_regulation , but it does need an awful lot of package rebuilding as a result

I should add that I don't have a commit bit myself, if anyone in the conversation is wondering why I haven't done it already

@FRidh
Copy link
Member

FRidh commented Nov 3, 2019

Please add a libinput module option as well for it.

@FRidh FRidh changed the base branch from master to staging November 3, 2019 09:54
@FRidh FRidh added this to WIP in Staging via automation Nov 3, 2019
Staging automation moved this from WIP to Ready Nov 4, 2019
@telent
Copy link
Contributor Author

telent commented Nov 8, 2019

Please add a libinput module option as well for it.

config.services.xserver.libinput.quirks = [
''
[Section touchpad pressure]
MatchUdevType=touchpad
MatchName=*SynPS/2 Synaptics TouchPad
MatchDMIModalias=dmi:*svnLENOVO:*:pvrThinkPadX1Carbon4th*
AttrPressureRange=45:40
''];

something like that, you mean? It's a good idea (I'd be happy to try it as soon as I have the bandwidth), but I think it could be tackled as a separate PR instead of coupling to this one. It's not that hard to set environment.etc."libinput/local-overrides.quirks" in configuration.nix, so although it'd maybe make things prettier/more disoverable/more semantic, it wouldn't actually unlock any new capabilities.

@FRidh FRidh merged commit c36e1b0 into NixOS:staging Nov 24, 2019
Staging automation moved this from Ready to Done Nov 24, 2019
@FRidh
Copy link
Member

FRidh commented Nov 24, 2019

Yes, like that. The reason is discoverability.

@joepie91
Copy link
Contributor

Has this also been backported to stable? I'm not sure what the staging branch is for.

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

Successfully merging this pull request may close these issues.

None yet

5 participants