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

New packages: interception-tools-0.6.7, caps2esc-0.3.2 #31575

Merged
merged 2 commits into from Jul 20, 2021

Conversation

sbogomolov
Copy link
Contributor

@sbogomolov sbogomolov commented Jun 19, 2021

General

Have the results of the proposed changes been tested?

  • I use the packages affected by the proposed changes on a regular basis and confirm this PR works for me
  • I generally don't use the affected packages but briefly tested this PR

@sbogomolov
Copy link
Contributor Author

I've added README files as docs. It's better than nothing :)

@sbogomolov
Copy link
Contributor Author

Is there something I have to do in order to get that workflow approval?

@sbogomolov
Copy link
Contributor Author

This is my first contribution, so I have no idea how long it should take until the first reaction or until the workflow to run the CI gets approved. Are the re some steps I should take in order for this to happen?

@ericonr
Copy link
Member

ericonr commented Jul 11, 2021

Pinging the PR is ok, or join us on IRC and mention it. We can't configure GH to auto-allow CI :/

New package PRs are usually quite low priority, unfortunately.

srcpkgs/interception-tools/files/udevmon/run Outdated Show resolved Hide resolved
srcpkgs/interception-tools/template Outdated Show resolved Hide resolved
@sbogomolov
Copy link
Contributor Author

Thanks, I'll try IRC channel next time :)

@sbogomolov
Copy link
Contributor Author

@ericonr Thank you for reviewing this change. I have addressed all your comments in the last commit.

@sbogomolov
Copy link
Contributor Author

I have checked the aarch64 cross compilation and it seems that the include directory is wrong.

FAILED: CMakeFiles/uinput.dir/uinput.cpp.o 
/builddir/.xbps-interception-tools/wrappers/aarch64-linux-gnu-c++  -I/usr/include/libevdev-1.0 -fstack-clash-protection -D_FORTIFY_SOURCE=2 -O2 -march=armv8-a   -I/usr/aarch64-linux-gnu/usr/include -Wall -Wextra -pedantic -std=c++11 -MD -MT CMakeFiles/uinput.dir/uinput.cpp.o -MF CMakeFiles/uinput.dir/uinput.cpp.o.d -o CMakeFiles/uinput.dir/uinput.cpp.o -c ../uinput.cpp
../uinput.cpp:15:10: fatal error: libevdev/libevdev-uinput.h: No such file or directory
   15 | #include <libevdev/libevdev-uinput.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~

It uses -I/usr/include/libevdev-1.0 instead of -I/usr/aarch64-linux-gnu/usr/include/libevdev-1.0.

@ericonr
Copy link
Member

ericonr commented Jul 12, 2021

Yeah, they don't know how to use cmake... https://gitlab.com/interception/linux/tools/-/blob/master/CMakeLists.txt#L9

@sbogomolov
Copy link
Contributor Author

Yeah, saw that. I've submitted a MR: https://gitlab.com/interception/linux/tools/-/merge_requests/16.
For now I'll add a patch to work around this.

@sbogomolov
Copy link
Contributor Author

sbogomolov commented Jul 12, 2021

Another merge request to replace __pid_t with pid_t: https://gitlab.com/interception/linux/tools/-/merge_requests/17.
This allows to build it with musl. I'll add a patch for that as well (until it gets merged and new version is released).

These two patches should fix all the builds. At lest "it works on my machine" (as in builds for all architectures) :)

@sbogomolov sbogomolov force-pushed the caps2esc branch 2 times, most recently from 01aff2d to 8f4911e Compare July 12, 2021 21:32
@sbogomolov
Copy link
Contributor Author

@ericonr it worked :) Is there anything else I should change in order for this to be merged?

@sbogomolov
Copy link
Contributor Author

I've updated the license from GPL-3.0-or-later to GPL-3.0-only. Upstream does not mention which one it is, but dual-licensing should require explicit mentioning, so I'll go with the safer option.

@ericonr
Copy link
Member

ericonr commented Jul 14, 2021

Ok, the packages look reasonable and I see why it can be useful (though note that with framebuffer, X and xkbcommon keymaps (for wayland) one should be able to implement what it does).

Since it required some patching, I'd prefer to wait for upstream to answer to your PRs so we can see how "packageable" it will end up being :)

@sbogomolov
Copy link
Contributor Author

sbogomolov commented Jul 14, 2021

Sounds reasonable, let's wait.

Regarding the alternative solutions, I did not find any (for Wayland). What this achieves is that Caps lock key has two functions:

  • ESC on tap
  • Ctrl on press

There are multiple ways to achieve any one of those behaviours, but not both. Or at least I could not find them :)

@sbogomolov sbogomolov changed the title New packages: interception-tools-0.6.6, caps2esc-0.3.2 New packages: interception-tools-0.6.7, caps2esc-0.3.2 Jul 15, 2021
@sbogomolov
Copy link
Contributor Author

@ericonr Both PRs were accepted and new version 0.6.7 was tagged. In the last commit I have removed both patches and bumped the version.

Copy link
Member

@ericonr ericonr left a comment

Choose a reason for hiding this comment

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

Great to hear the patches were upstreamed!

srcpkgs/interception-tools/files/udevmon/run Outdated Show resolved Hide resolved
srcpkgs/interception-tools/template Outdated Show resolved Hide resolved
srcpkgs/caps2esc/template Outdated Show resolved Hide resolved
@sbogomolov
Copy link
Contributor Author

@ericonr All your comments were addressed in the last push.

@sbogomolov
Copy link
Contributor Author

@ericonr do you think this can be merged now?

@ericonr ericonr merged commit 6d28ee7 into void-linux:master Jul 20, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
new-package This PR adds a new package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants