anaconda
anaconda copied to clipboard
libxklavier removal
RHEL 10 only! Please do not merge until the branch for RHEL 10 is created.
libxklavier is deprecated and X11 only. A Wayland alternative is required for RHEL 10.
While there is an ongoing conversation about enabling org.freedesktop.locale1
as a way to set the keyboard layout used by the compositor, not all compositors used by the different Fedora Spins support it.
This PR allows to use GNOME Kiosk's API to replace libxklavier in the environments were it is used.
Hello @jexposit! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:
Comment last updated at 2024-05-28 15:10:18 UTC
/build-image --boot.iso
/build-image --live
Images built based on commit e0f19bceef72399418c2512d49a5861bf4fa9062:
- Live: failure
Download the images from the bottom of the job status page.
Images built based on commit e0f19bceef72399418c2512d49a5861bf4fa9062:
-
boot.iso
: failure
Download the images from the bottom of the job status page.
@jexposit feel free to ignore Live ISO
use case because RHEL doesn't support Live ISO so we don't have to care about that here.
What I wonder is if we can reduce the C code logic to a bare minimum and change the widgets to just a
set/get/signal
code without doing the switching directly. Would it be doable?
That was something I was also thinking about originally, but then I realized it makes more sense for Jose to do it like this dues to his strong C background + its closer to how the widget worked originally.
The main issues could be with maintenance for use (with our GTK/C API knowledge level), but maybe it will be fine for the expected remaining GTK GUI lifetime ?
Edit: Though looking at the C code in detail, it uses the Gnome Kiosk API directly, so while perfectly fine for RHEL 10, this would have to be changed if we want to have keyboard switching working again in Fedora Live images with their different compositors.
Looks like boot boot.iso and Live image generation fails due to some issues with the C widget compilation:
make[7]: Entering directory '/tmp/anaconda/widgets/src'
gdbus-codegen \
--interface-prefix org.gnome. \
--c-namespace Gk \
--generate-c-code gk-kiosk \
--output-directory . \
/usr/local/share/dbus/org.gnome.Kiosk.xml
Traceback (most recent call last):
File "/usr/bin/gdbus-codegen", line 55, in <module>
sys.exit(codegen_main.codegen_main())
^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/share/glib-2.0/codegen/codegen_main.py", line 419, in codegen_main
with open(fname, "rb") as f:
^^^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory: '/usr/local/share/dbus/org.gnome.Kiosk.xml'
I see that file included in the PR, but it seems to be unable to locate it at build time ?
I don't see the new file listed in a makefile & in spec file, so it might not actually be shipped in the tarball & this is why its missing at build time.
/build-image --boot.iso
The unit test task is failing with error:
LayoutIndicator.c:31:10: fatal error: gk-kiosk.h: No such file or directory
31 | #include "gk-kiosk.h"
| ^~~~~~~~~~~~
compilation terminated.
It looks like gdbus-codegen
is not being invoked. However, when I run it locally, it is generating the right files in /tmp/anaconda/widgets/src
.
Am I missing an extra step?
Images built based on commit 72f4d07893a2bf7d8161dd8db39f6add7ad453e7:
-
boot.iso
: success
Download the images from the bottom of the job status page.
It looks like
gdbus-codegen
is not being invoked. However, when I run it locally, it is generating the right files in/tmp/anaconda/widgets/src
.Am I missing an extra step?
I'll try to reproduce the issues locally & see whats going on. :)
Weird, doing make -f ./Makefile.am container-rpms-scratch
does build & I end up with successfully built RPM. So I tried to restart the failed unit tests, but now it seems to hit a merge conflict apparently ?
Run git config user.name github-actions
c949c03b27 Merge pull request #5464 from poncovka/master-session_bus
Auto-merging anaconda.py
CONFLICT (content): Merge conflict in anaconda.py
Auto-merging pyanaconda/display.py
CONFLICT (content): Merge conflict in pyanaconda/display.py
Rebasing (1/6)
error: could not apply 3baee6de49... Set up the default session bus
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply 3baee6de49... Set up the default session bus
Error: Process completed with exit code 1.
@jexposit
Weird, doing
make -f ./Makefile.am container-rpms-scratch
does build & I end up with successfully built RPM. So I tried to restart the failed unit tests, but now it seems to hit a merge conflict apparently ?
Yes, I cherry-picked a commit from Vendula (https://github.com/rhinstaller/anaconda/pull/5464) and now that it is merged there was a conflict. Rebased.
/build-image --boot.iso
In addition to the changes requested, I added a few fixes for bugs I noticed during testing.
In addition to the UI, I tested the inst.keymap=
boot option and I didn't noticed any difference from the master
branch.
I wasn't able to make the keyboard
kickstart option work. Both in the master
branch and in this branch it doesn't affect the UI or the installed system. My code changes shouldn't affect this option, but it'd nice to investigate why it is not working (or why I'm not being able to make it work).
I'm pretty happy with this revision of the code, I think it's ready for another round of review/testintg.
Images built based on commit 46ac1ea93c98bfe97410cdeb74627a48e3b53af4:
-
boot.iso
: success
Download the images from the bottom of the job status page.
I've done some testing to compare behavior of regular Rawhide Anaconda and this PR, using this kickstart:
graphical
halt
lang cs_CZ.UTF-8
keyboard cz
timezone America/New_York
rootpw anaconda
bootloader --location=mbr
zerombr
clearpart --all --initlabel
autopart
%packages
bash
%end
Regular rawhide
boot options
installation running, note keyboard badge in upper right
localectl output in installation env
localectl output in installed system
Rawhide with this PR applied
boot options
installation running, note keyboard badge in upper right
localectl output in installation env
localectl output in installed system
So as far as I can tell keyboard configuration from kickstart works the same with the PR as before, which is the desired state - eq. things working the same, but without the obsolete libXklavier
dependency. :)
Using the inst.keymap=cz
boot option behaves exactly the same as with kickstart. :)
Can you mark this as a draft PR then?
/kickstart-test --skip-testtypes knownfailure,manual,skip-on-fedora,gh576,gh595,gh640,gh641,gh680,gh740,gh769,gh774,gh777,gh910,gh890,gh871,rhbz1853668,gh975,gh1023
In the end is was implemented as a fallback rather than a extra command line option: When there is no session configuration, the system configuration is used instead.
We are a bit late in the Fedora 40 cycle to change things, but hopefully for Fedora 41, all compositors should be ready to support a mechanism to set locale configuration from org.freedesktop.locale1
.
Nice! :)
In the end is was implemented as a fallback rather than a extra command line option: When there is no session configuration, the system configuration is used instead.
We are a bit late in the Fedora 40 cycle to change things, but hopefully for Fedora 41, all compositors should be ready to support a mechanism to set locale configuration from
org.freedesktop.locale1
.
Yep, makes sense to use this API exclusively on Fedora if possible. :)
Currently I'm a bit preoccupied assuring landing of this version of the Wayland migration patch set to RHEL goes smoothly, but in the near future I plan to start a discussion (kinda like @jkonecny12 recently did for the Web UI stuff) about how to best handle the Wayland migration on Fedora. :)
If we want to get it into F41 which seems we want. I would suggest to just create a system wide change for F41 which is going to start the discussion.
At least to me, the important thing is that we have a framework in Anaconda so that I can slot in KWin or Weston instead of GNOME Kiosk.
Also, could we make sure these improvements are part of RHEL 10 Anaconda?
If we want to get it into F41 which seems we want. I would suggest to just create a system wide change for F41 which is going to start the discussion.
Yeah, might as well do that. :)
At least to me, the important thing is that we have a framework in Anaconda so that I can slot in KWin or Weston instead of GNOME Kiosk.
I think the PR ended up in a way that it could be possible to support more compositors. In general, it needs to keep support for what we need on RHEL, which is GNOME Kiosk as Wayland compositor and GNOME Remote Desktop for RDP access.
Also, could we make sure these improvements are part of RHEL 10 Anaconda?
Indeed, it would be ideal from code commonality PoV to have the Wayland support code to be similar between Fedora and RHEL. But we will see if we can eventually pull it off given the various timelines and requirements of both projects. :P
/kickstart-test --skip-testtypes knownfailure,manual,skip-on-fedora,gh576,gh595,gh640,gh641,gh680,gh740,gh769,gh774,gh777,gh910,gh890,gh871,rhbz1853668,gh975,gh1023
Failed to load packit config file:
Cannot parse package config. TypeError("'NoneType' object is not iterable")
For more info, please check out the documentation or contact the Packit team. You can also use our CLI command validate-config
or our pre-commit hooks for validation of the configuration.