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.

Sign up for GitHub

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

Jump to bottom

libinput: Fix commands #92715

Merged
merged 5 commits into from Jul 16, 2020
Merged

libinput: Fix commands #92715

merged 5 commits into from Jul 16, 2020

Conversation

jtojnar
Copy link
Contributor

@jtojnar jtojnar commented Jul 8, 2020 •
edited

Motivation for this change

Running subcommands like libinput measure requires python and some python modules.

I previously added the dependencies in #51304 but de14f0c accidentally moved them to checkInputs so they are not available at runtime by patchShebangs (especially since tests are disabled).

Additionally, the tools were ported from evdev python module to python-libevdev in libinput 1.14, which was missed during upgrade.

Finally, other python modules are needed so let's add them as well.

Closure size
before:
/nix/store/q1fzqnxjfiklxxw0w43amavbg9171vs0-libinput-1.15.5    	  52.4M
/nix/store/43nkzxizpfaq9ljhjhfwk146p8jf9pa5-libinput-1.15.5-bin	  53.0M
/nix/store/04r1xdb26swsf7rykdmpf5llsh918nsy-libinput-1.15.5-dev	 152.4M

after:
/nix/store/7yxwhlrr7n0i53jd352mg4slrfihqd31-libinput-1.15.5    	  52.4M
/nix/store/lh3q2fvd4jf0j0q1x39by770l3mkaymp-libinput-1.15.5-bin	 220.6M
/nix/store/lk4qiabwznc8hpava8wiwf2xsgbgbdzx-libinput-1.15.5-dev	 220.8M

original before regressions:
/nix/store/55xasqvnqw88cficppnkmvs6y3dqq213-libinput-1.15.5    	  52.4M
/nix/store/qbhb51ad9anvp36pdgkrc4h13r26x203-libinput-1.15.5-bin	 126.5M
/nix/store/cd25ap98596ws5wrjcib3hsv8dk65grm-libinput-1.15.5-dev	 217.2M

after, simplified with pyudev not propagating systemd:
/nix/store/4xzs08bm15p55x2b6wlw99nn9xr2z2ka-libinput-1.15.5    	  52.4M
/nix/store/qhpd9c5q9lymz9pd87bqgfakk2n39234-libinput-1.15.5-bin	 130.1M
/nix/store/fllaxc6hi1rppfvw9nss14rbh949nqw3-libinput-1.15.5-dev	 220.8M

Packages typically depend on the out output, so their runtime closure should not be affected by the increase in bin.

Fixes: #48252

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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.

@jtojnar jtojnar changed the base branch from master to staging July 8, 2020 13:35
@jtojnar jtojnar mentioned this pull request Jul 8, 2020
Closed
@ofborg ofborg bot requested review from codyopel and NickHu July 8, 2020 13:42
@jtojnar jtojnar force-pushed the fix-libinput-commands branch 2 times, most recently from 44c8228 to f1c9459 Compare July 8, 2020 14:27
@jtojnar jtojnar requested a review from matthewbauer July 8, 2020 14:30
@jtojnar
Copy link
Contributor Author

jtojnar commented Jul 8, 2020

@matthewbauer checkInputs are subset of buildInputs so I do not see a point in moving them there regarding cross-compilation (#81858).

@matthewbauer
Copy link
Member

My bad! Yeah the issue was it didn't cross compile and it looked like it was just used for tests (which are disabled in cross). But fixing native should take priority over that.

@jtojnar jtojnar requested a review from worldofpeace July 8, 2020 15:42
worldofpeace
worldofpeace approved these changes Jul 8, 2020
View reviewed changes
Copy link
Contributor

@worldofpeace worldofpeace left a comment

Choose a reason for hiding this comment

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

I tested the following

❯ sudo ./result-bin/bin/libinput measure touchpad-tap                                                                                        
Touchpad: /dev/input/event15

Ready for recording data.
Tap the touchpad multiple times with a single finger only.
For useful data we recommend at least 20 taps.
Ctrl+C to exit
Touch sequences detected: 20
^C
Time: 
  Max delta: 96ms
  Min delta: 55ms
  Average delta: 71ms
  Median delta: 69ms
  90th percentile: 91ms
  95th percentile: 96ms

No issues.

@jtojnar
Copy link
Contributor Author

jtojnar commented Jul 8, 2020 •
edited

Looks like it fails on aarch64:


=================================== FAILURES ===================================
________________ TestNameConversion.test_value_to_name_invalid _________________
self = <test_clib.TestNameConversion testMethod=test_value_to_name_invalid>
    def test_value_to_name_invalid(self):
        name = Libevdev.event_to_name(3, 0x37, 1000)
        self.assertIsNone(name)
        name = Libevdev.event_to_name(3, 0x37, -1)
>       self.assertIsNone(name)
E       AssertionError: 'MSC_MAX' is not None
test/test_clib.py:106: AssertionError

But no idea what MSC_MAX is supposed to be, do not see anything like that in the code: Edit: did not notice the quotes.

https://gitlab.freedesktop.org/libevdev/python-libevdev/-/blob/0.9/test/test_clib.py#L102-108

Edit: opened https://gitlab.freedesktop.org/libevdev/python-libevdev/-/issues/9

@jtojnar jtojnar mentioned this pull request Jul 10, 2020
Open
@jtojnar
python3.pkgs.libevdev: hardcode path
10e5b93
@jtojnar
python3.pkgs.libevdev: enable tests
7c75c1e
@jtojnar
libinput: fix subcommands
6cf8110 
Running subcommands like `libinput measure` requires python and some python modules.

I previously added the dependencies in [1] but [2] accidentally moved them to checkInputs so they are not available at runtime by patchShebangs (especially since tests are disabled).

Additionally, the tools were ported from evdev python module to python-libevdev in libinput 1.14, which was missed [3] during upgrade.

Finally, other python modules are needed so let's add them as well.

[1]: NixOS#51304
[2]: NixOS@de14f0c
[3]: NixOS@b291f2a
@jtojnar
python3.pkgs.pyudev: do not propagate systemd
d9f4163 
It is not linked against so there is no need to include it in build inputs, much less propagate it.

This removes systemd.dev from the runtime closure of packages using pyudev.
@jtojnar
libevdev: fix aarch64 support
f83c9a7
@ofborg ofborg bot requested review from makefu, womfoo and bendlas July 15, 2020 04:23
@jtojnar jtojnar changed the base branch from staging to staging-next July 15, 2020 07:46
@jtojnar jtojnar changed the base branch from staging-next to staging July 15, 2020 07:47
@jtojnar
Copy link
Contributor Author

jtojnar commented Jul 15, 2020

@GrahamcOfBorg build python3.pkgs.libevdev

@makefu makefu requested a review from worldofpeace July 15, 2020 12:01
@makefu
Copy link
Contributor

makefu commented Jul 15, 2020

@worldofpeace wupps, accidentally clicked a button (no way to undo), sorry for the inconvenience and ignore the request :)

makefu
makefu approved these changes Jul 15, 2020
View reviewed changes
Copy link
Contributor

@makefu makefu left a comment

Choose a reason for hiding this comment

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

lgtm

@jtojnar jtojnar merged commit d49b38b into NixOS:staging Jul 16, 2020
@jtojnar jtojnar deleted the fix-libinput-commands branch July 16, 2020 02:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@makefu makefu makefu approved these changes

@FRidh FRidh Awaiting requested review from FRidh

@jonringer jonringer Awaiting requested review from jonringer

@codyopel codyopel Awaiting requested review from codyopel

@NickHu NickHu Awaiting requested review from NickHu

@matthewbauer matthewbauer Awaiting requested review from matthewbauer

@womfoo womfoo Awaiting requested review from womfoo

@bendlas bendlas Awaiting requested review from bendlas

@worldofpeace worldofpeace Awaiting requested review from worldofpeace

Assignees
No one assigned
Labels
6.topic: python 10.rebuild-darwin: 11-100 10.rebuild-linux: 501+ 10.rebuild-linux: 1001-2500
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@jtojnar @matthewbauer @makefu @worldofpeace