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

[TEST] Jexposit xorg server removal tests #5503

Closed
wants to merge 9 commits into from
Closed

[TEST] Jexposit xorg server removal tests #5503

wants to merge 9 commits into from

Conversation

rvykydal
Copy link
Contributor

@rvykydal rvykydal commented Mar 4, 2024

Testing tests for #5485

jexposit and others added 8 commits March 4, 2024 11:16
@jexposit @rvykydal
Add GNOME Kiosk keyboard manager class
00946ba 
Add a class wrapping GNOME Kiosk's input sources API that future commits
will use via the localization service to replace the X11 keyboard
management logic.
@jexposit @rvykydal
Use GNOME Kiosk's API in XklWrapper
25cac7a 
libxklavier is deprecated and X11-only. On RHEL, the GNOME Kiosk API can
be used instead to handle the keyboard configuration.

In order to make the code migration as simple as possible, keep the
XklWrapper class and update its implementation to use GNOME Kiosk's API
via the localization service.
@jexposit @rvykydal
Setup gdbus-codegen
20825ef 
Invoke gdbus-codegen as part of the make build and makeupdates script to
generate C code from org.fedoraproject.Anaconda.Modules.Localization.xml.
@jexposit @rvykydal
Use GNOME Kiosk's API in LayoutIndicator
f31205f 
This C widget is the last place were libxklavier is used.

Use GNOME Kiosk's API via the localization service instead.
@jexposit @rvykydal
Drop libxklavier
e23d499
@jexposit @rvykydal
Drop xrandr
35ac896 
Anaconda uses xrandr to set the screen resolution when the boot option
"inst.resolution" [1] is used.

In order to be able to drop the X.Org server, use Mutter's API instead
of xrandr.

The kickstart equivalent option has been removed, so we don't need to
care about it [2].

[1] https://anaconda-installer.readthedocs.io/en/latest/boot-options.html#inst-resolution
[2] https://pykickstart.readthedocs.io/en/latest/kickstart-docs.html#xconfig
@jexposit @rvykydal
Drop xrdb
2cbdbf1 
It wasn't used and it won't work on Wayland.
@jexposit @Conan-Kudo
@M4rtinK @halfline @rvykydal
Drop the X.Org server dependency
f9e0257 
Start GNOME Kiosk as a Wayland compositor and run Anaconda as a native
Wayland client.

This commit is a follow up on the work done by Neal Gompa [1] and
Martin Kolman [2]. Credit goes to them for the code I copied and pasted.

[1] rhinstaller#5401
[2] rhinstaller#5309

Co-authored-by: Neal Gompa <neal@gompa.dev>
Co-authored-by: Martin Kolman <mkolman@redhat.com>
Co-authored-by: Ray Strode <rstrode@redhat.com>
@pep8speaks
Copy link

pep8speaks commented Mar 4, 2024 •
edited
Loading

Hello @rvykydal! 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! 🍻

Comment last updated at 2024-03-04 12:40:02 UTC

@github-actions github-actions bot added the f41 label Mar 4, 2024
@rvykydal rvykydal force-pushed the jexposit-xorg-server-removal-tests branch from a163e7b to aa5788a Compare March 4, 2024 10:58
@rvykydal rvykydal force-pushed the jexposit-xorg-server-removal-tests branch from aa5788a to 613ccb1 Compare March 4, 2024 12:36
@rvykydal rvykydal force-pushed the jexposit-xorg-server-removal-tests branch from 613ccb1 to 0395bf9 Compare March 4, 2024 12:39
@rvykydal rvykydal mentioned this pull request Mar 4, 2024
Merged
@jexposit
Copy link
Contributor

jexposit commented Mar 4, 2024 •
edited
Loading

Hi @rvykydal,

Thanks a lot for adding these tests. Once all the required PRs are merged (#5470, #5463 and #5485) we should be able to merge this one.

The code looks good to me... But this is out of my area of knowledge, so I hope someone else can add an extra review.

@rvykydal
Copy link
Contributor Author

rvykydal commented Mar 5, 2024

Hi @rvykydal,

Thanks a lot for adding these tests. Once all the required PRs are merged (#5470, #5463 and #5485) we should be able to merge this one.

The code looks good to me... But this is out of my area of knowledge, so I hope someone else can add an extra review.

Hello,
maybe I'd suggest to add the commit with tests from this PR to the #5485 in case the original PR will require some changes. And I am considering the potential changes because we recently re-enabled pylint tests on PRs which fail on some parts of the PR (see the "Run validation tests / unit-tests (pull_request)" check of this pr ... https://github.com/rhinstaller/anaconda/actions/runs/8140317540/job/22245294689?pr=5503). I think we are able to address the pylint issues and we will be happy to do it (possibly in another commit added to the PR). If you prefer to address the pylint issues (or some of them) yourself, you'd need to rebase the original PR on current master (so the pylint test is run on the PR).

@jexposit
Copy link
Contributor

jexposit commented Mar 5, 2024

Oh yes, I already fixed the pylint issues and added your changes to #5485, see my comment #5485 (comment)

I'll rebase on master to get pylint running again.

@rvykydal
Copy link
Contributor Author

rvykydal commented Mar 6, 2024

Closing as the tests are now included in #5485

@rvykydal rvykydal closed this Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
f41
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rvykydal @pep8speaks @jexposit