-
-
Notifications
You must be signed in to change notification settings - Fork 74
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
Ignore invalid GUI related features and options #315
Ignore invalid GUI related features and options #315
Conversation
2531edd
to
d49915f
Compare
I am trying to add |
d49915f
to
12effce
Compare
@marmarek apologies for pinging you here. I need to know how to include |
Unit tests do not use the spec file. For installing system packages add them to .gitlab-ci.yml. |
c3253c7
to
f814b88
Compare
I am marking this PR as ready for review. |
Try adding xvfb-run (see desktop-linux-manager) |
f814b88
to
908fdf8
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #315 +/- ##
==========================================
+ Coverage 75.09% 75.14% +0.04%
==========================================
Files 52 52
Lines 8554 8610 +56
==========================================
+ Hits 6424 6470 +46
- Misses 2130 2140 +10 ☔ View full report in Codecov by Sentry. |
908fdf8
to
823f7d4
Compare
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2024111418-4.3&flavor=pull-requests Test run included the following:
New failures, excluding unstableCompared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2024091704-4.3&flavor=update
Failed tests5 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/112766#dependencies 201 fixed
Unstable tests
|
# pylint: disable=wrong-import-position,wrong-import-order | ||
from Xlib import X, XK | ||
from Xlib.display import Display | ||
from Xlib.error import DisplayConnectionError |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two issues here:
- I believe nowadays it should be "xcffib" - "xcb" is preferred over "xlib" directly, and its python bindings are in python3-xcffib
- You forgot to add dependency to the debian package, so qvm-start-daemon fails on all Debian debian tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I believe nowadays it should be "xcffib" - "xcb" is preferred over "xlib" directly, and its python bindings are in python3-xcffib
I have to study xcffib documentation as it appears to be a drop-in replacement for xcb rather than xlib. It does not provide alloc_named_color
and string_to_keysym
functions (or I can not find it).
2. You forgot to add dependency to the debian package, so qvm-start-daemon fails on all Debian debian tests.
OK. I will do it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there are no easy equivalents, then I guess it may stay with python3-xlib, but check first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://stackoverflow.com/a/50539803
(Except that it only shows keysym values, not keysym names, which you can find in /usr/include/X11/keysymdef.h or using Xlib's XKeysymToString(), and I don't think there exists an XCB port of that function, but you could write your own based on keysymdef.h.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As Geoffrey comments in the Stackoverflow, his suggested code does not show keysym names (only syms & values). And suggests XKeysymToString
(which I am using it's python-xlib equivalent here).
For the keyboard, It might be possible to revert to python3-xkbcommon which is Python bindings for libxkbcommon using cffi. And what is used nowadays with Wayland as well.
For the color, I have to look for alternatives
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@marmarek there are positive and negative things about python3-xkbcommon
. I tested it and it works well. While it is the new modern approach and what is currently used with both X & Wayland, It is only available for Fedora 38 onward. Not Fedora 37. So it is time to decide if this patch is going to come only to Qubes R4.3 or R4.2 as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For color, using xcffib.xproto.xprotoExtension().AllocNamedColor()
works. But not as good as the Xlib's alloc_named_color
. Since Xlib supports #RGB
, #RRGGBB
, rgb:RRRRGGGGBBBB
, etc (as well as the named colors).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is only available for Fedora 38 onward.
It also isn't available in Debian... So, I guess python3-xlib it is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It also isn't available in Debian... So, I guess python3-xlib it is.
During the early stages of writing this PR, I used higher level GTK 4.0 for Keysym name validation (reference). It is still possible. And the ugly approach of try import xcffib except import xlib
is also possible. Still your decision.
823f7d4
to
36b5082
Compare
36b5082
to
cb43fd6
Compare
See QubesOS/qubes-issues#9564, you made it too good ;) |
cb43fd6
to
53f7b09
Compare
This should be fixed now. I just added this: if sequence.lower() in ['none', 'disable']:
return True |
resolves: QubesOS/qubes-issues#7730