Simplify keyboard layout handling, rely on localed more #5195
Conversation
|
I have backported the initial two systemd fixes to Rawhide, so this should already give mostly correct results on Rawhide (as long as you test with systemd-254.2-12.fc40 ). I haven't yet backported the patch to fix the tricky Bulgarian non-phonetic case, waiting for upstream to review that, but the behaviour in that case with this patch is still better than before (at least you get a Bulgarian console layout, it's just the wrong one, before you got errors and a fallback to US). To defend the change to the live path a bit more: I don't think it's ever going to be a good idea to assume the 'current' layout is what the console layout should match. In the non-switched layout case, the question is irrelevant (the 'current' layout and the 'all layouts' list will be the same single item). In the switched case, you can't assume the layout a user is using in the installer is the layout they want on the console. They will likely need to use a Latin-capable layout in the installer as they need to type Latin user names, passphrases, mount names, whatever. And besides, the approach ignores the difference in how switching works between kbd and xkb. The kbd 'ru' layout is equivalent to the xkb configuration "us,ru", not just "ru". And so on. I just don't see how we are ever going to be able to get a better result by basing the conversion on the single currently selected layout than by basing it on the full xkb config. Here's a couple of images for testing this. They contain a build of anaconda with this patch applied, and a build of systemd with all three patches applied. Please do try whatever crazy paths you can think of and see if you can find any where this behaves worse than before! |
|
@jkonecny12 for the live keyboard path, @halfline for info |
There are a couple of specific bugs in the current code: https://bugzilla.redhat.com/show_bug.cgi?id=2238854 https://bugzilla.redhat.com/show_bug.cgi?id=2239213 which could both be solved by just...trying less hard. Let's not try to set the console layout directly on the old set_x_keyboard_defaults path, and let's not have special code to set the console layout when reading configs from the live image, let's just read in the X layouts from the live config then rely on the existing code to ask localed to pick a console layout for us. This also sets the keyboard model to "pc105" when requesting a console layout from localed. This is because localed only applies match "bonuses" for matching variants and options if the model matches. We need the variant "bonus" match to get the right result for Bulgarian, at least. Almost every line in kbd-model-map has the model set to "pc105", so let's just go with that. The only line with a different model *and* a variant is es-dvorak, and there is an xkb-converted layout for that, so it shouldn't make any difference there. With current stable systemd this will give us wrong results in several cases (e.g. Russian installs will get US console layout). With current git HEAD systemd - after systemd/systemd#29215 - I believe this gives the same or better results in all cases on both new and old workflows (I have tested at least Russian and several Bulgarian paths). With systemd/systemd#29236 on top, we also get the correct result for Bulgarian with no variant, "bg_bds-utf8". Without that PR but with this change, we get the incorrect "bg_pho-utf8" in that case. Without this change, we set the console layout to "bg", which does not exist and causes errors on system startup and fallback to the "us" layout. Resolves: rhbz#2238854 Resolves: rhbz#2239213 Signed-off-by: Adam Williamson <awilliam@redhat.com>
AdamWill
force-pushed
the
gis-switched-layouts
branch
from
26678eb
to
e53efc2
Compare
|
I had actually adjusted the tests already but forgot to save the file... |
|
/kickstart-test --testtype smoke |
|
/build-image --live |
|
Images built based on commit e53efc2:
Download the images from the bottom of the job status page. |
|
Images built based on commit e53efc2:
Download the images from the bottom of the job status page. |
|
/kickstart-test --testtype keyboard |
|
I like this change. I'll do some testing but other then that I agree on your suggestion. |
|
I did some simple testing and I somehow wonder. With bulgarian language I was able to get to GIS after Live install - however, I wasn't able to create a user because of some issue with bulgarian characters in Username (or real name?). Anyhow, this doesn't look like incorrectly configured. |
|
ACKed @AdamWill please tell us when everything is set in Rawhide to merge and release this. |
|
It can be merged for Rawhide already, just not for F39. In general it's not a good idea to include non-ASCII characters in usernames. Things like anaconda and g-i-s tend to either warn you about this or just disallow it (I don't remember which g-i-s is doing ATM). That's why it's so important that we make sure 'us' layout is also available in these cases :) So I don't think that's unexpected. |
There are a couple of specific bugs in the current code:
https://bugzilla.redhat.com/show_bug.cgi?id=2238854 https://bugzilla.redhat.com/show_bug.cgi?id=2239213
which could both be solved by just...trying less hard. Let's not try to set the console layout directly on the old set_x_keyboard_defaults path, and let's not have special code to set the console layout when reading configs from the live image, let's just read in the X layouts from the live config then rely on the existing code to ask localed to pick a console layout for us.
This also sets the keyboard model to "pc105" when requesting a console layout from localed. This is because localed only applies match "bonuses" for matching variants and options if the model matches. We need the variant "bonus" match to get the right result for Bulgarian, at least. Almost every line in kbd-model-map has the model set to "pc105", so let's just go with that. The only line with a different model and a variant is es-dvorak, and there is an xkb-converted layout for that, so it shouldn't make any difference there.
With current stable systemd this will give us wrong results in several cases (e.g. Russian installs will get US console layout). With current git HEAD systemd - after
systemd/systemd#29215 - I believe this gives the same or better results in all cases on both new and old workflows (I have tested at least Russian and several Bulgarian paths). With
systemd/systemd#29236 on top, we also get the correct result for Bulgarian with no variant, "bg_bds-utf8". Without that PR but with this change, we get the incorrect "bg_pho-utf8" in that case. Without this change, we set the console layout to "bg", which does not exist and causes errors on system startup and fallback to the "us" layout.
Resolves: rhbz#2238854
Resolves: rhbz#2239213