From 18e1ed3201dfc35692b778c6e807d38a2d105e41 Mon Sep 17 00:00:00 2001 From: Adam Williamson Date: Tue, 19 Sep 2023 14:52:46 -0700 Subject: [PATCH] Backport PR #29215 to improve keyboard layout matching This lays the ground for simplifying anaconda's keyboard layout handling while improving results from where they are currently. --- ...cy_keymap-fix-empty-variant-matching.patch | 58 +++++++++ ...ap-try-matching-with-layout-order-re.patch | 117 ++++++++++++++++++ systemd.spec | 6 + 3 files changed, 181 insertions(+) create mode 100644 0001-find_legacy_keymap-fix-empty-variant-matching.patch create mode 100644 0002-find_legacy_keymap-try-matching-with-layout-order-re.patch diff --git a/0001-find_legacy_keymap-fix-empty-variant-matching.patch b/0001-find_legacy_keymap-fix-empty-variant-matching.patch new file mode 100644 index 0000000..c15a017 --- /dev/null +++ b/0001-find_legacy_keymap-fix-empty-variant-matching.patch @@ -0,0 +1,58 @@ +From a30ae31351ffa701ca860779495d4f52db4c462c Mon Sep 17 00:00:00 2001 +From: Adam Williamson +Date: Fri, 15 Sep 2023 15:35:36 -0700 +Subject: [PATCH 1/2] find_legacy_keymap: fix empty variant matching + +We should give a match bonus if the X context variant is empty +and the xvariant column in kbd-model-map is "-" (which means +none). Currently, we don't, which means that if you call this +on a context with layouts bg,us and no variant, you get the +console layout bg_pho-utf8 instead of bg_bds-utf8 (because both +score the same, and the bg_pho-utf8 row comes first). You should +get bg_bds-utf8 in this case. + +Signed-off-by: Adam Williamson +--- + src/locale/localed-util.c | 2 +- + src/locale/test-localed-util.c | 12 ++++++++++++ + 2 files changed, 13 insertions(+), 1 deletion(-) + +diff --git a/src/locale/localed-util.c b/src/locale/localed-util.c +index 02fac9786b..6a05b50a31 100644 +--- a/src/locale/localed-util.c ++++ b/src/locale/localed-util.c +@@ -825,7 +825,7 @@ int find_legacy_keymap(const X11Context *xc, char **ret) { + if (isempty(xc->model) || streq_ptr(xc->model, a[2])) { + matching++; + +- if (streq_ptr(xc->variant, a[3])) { ++ if (streq_ptr(xc->variant, a[3]) || (isempty(xc->variant) && streq(a[3], "-"))) { + matching++; + + if (streq_ptr(xc->options, a[4])) +diff --git a/src/locale/test-localed-util.c b/src/locale/test-localed-util.c +index cb66dffd48..a19d80a967 100644 +--- a/src/locale/test-localed-util.c ++++ b/src/locale/test-localed-util.c +@@ -173,6 +173,18 @@ TEST(x11_convert_to_vconsole) { + assert_se(streq(vc.keymap, "es-dvorak")); + vc_context_clear(&vc); + ++ /* es no-variant test is not very good as the desired match ++ comes first in the list so will win if both candidates score ++ the same. in this case the desired match comes second so will ++ not win unless we correctly give the no-variant match a bonus ++ */ ++ log_info("/* test without variant, desired match second (bg,us:) */"); ++ assert_se(free_and_strdup(&xc.layout, "bg,us") >= 0); ++ assert_se(free_and_strdup(&xc.variant, NULL) >= 0); ++ assert_se(x11_convert_to_vconsole(&xc, &vc) >= 0); ++ assert_se(streq(vc.keymap, "bg_bds-utf8")); ++ vc_context_clear(&vc); ++ + log_info("/* test with old mapping (fr:latin9) */"); + assert_se(free_and_strdup(&xc.layout, "fr") >= 0); + assert_se(free_and_strdup(&xc.variant, "latin9") >= 0); +-- +2.41.0 + diff --git a/0002-find_legacy_keymap-try-matching-with-layout-order-re.patch b/0002-find_legacy_keymap-try-matching-with-layout-order-re.patch new file mode 100644 index 0000000..d0eb7d0 --- /dev/null +++ b/0002-find_legacy_keymap-try-matching-with-layout-order-re.patch @@ -0,0 +1,117 @@ +From cf649cc21bf997b90606db664d74726fcaf002de Mon Sep 17 00:00:00 2001 +From: Adam Williamson +Date: Fri, 15 Sep 2023 16:02:29 -0700 +Subject: [PATCH 2/2] find_legacy_keymap: try matching with layout order + reversed + +The lines in kbd-model-map date back to ye olde times (RH's old +system-config-keyboard), and I think predate this bug: + +https://bugzilla.redhat.com/show_bug.cgi?id=1039185 + +where we got strong feedback that, for 'switched' layout setups +like Russian, US English should be the *first* layout and the +native layout the *second* one. This is how anaconda and, as of +recently, gnome-initial-setup configure such cases - but that +means, if we try to use localed to convert these configurations +using kbd-model-map, we get the wrong result (we get "us" as the +console layout). See also: + +https://bugzilla.redhat.com/show_bug.cgi?id=1912609 + +where we first noticed this wasn't working right, but sadly, we +'fixed' it with a not-really-correct bodge in anaconda instead +of doing it properly. + +Signed-off-by: Adam Williamson +--- + src/locale/localed-util.c | 44 ++++++++++++++++++++++------------ + src/locale/test-localed-util.c | 5 +++- + 2 files changed, 33 insertions(+), 16 deletions(-) + +diff --git a/src/locale/localed-util.c b/src/locale/localed-util.c +index 6a05b50a31..eba13a2ac3 100644 +--- a/src/locale/localed-util.c ++++ b/src/locale/localed-util.c +@@ -803,21 +803,35 @@ int find_legacy_keymap(const X11Context *xc, char **ret) { + /* If we got an exact match, this is the best */ + matching = 10; + else { +- /* We have multiple X layouts, look for an +- * entry that matches our key with everything +- * but the first layout stripped off. */ +- if (startswith_comma(xc->layout, a[1])) +- matching = 5; ++ /* see if we get an exact match with the order reversed */ ++ _cleanup_strv_free_ char **b = NULL; ++ _cleanup_free_ char *c = NULL; ++ r = strv_split_full(&b, a[1], ",", 0); ++ if (r < 0) ++ return r; ++ strv_reverse(b); ++ c = strv_join(b, ","); ++ if (!c) ++ return log_oom(); ++ if (streq(xc->layout, c)) ++ matching = 9; + else { +- _cleanup_free_ char *x = NULL; +- +- /* If that didn't work, strip off the +- * other layouts from the entry, too */ +- x = strdupcspn(a[1], ","); +- if (!x) +- return -ENOMEM; +- if (startswith_comma(xc->layout, x)) +- matching = 1; ++ /* We have multiple X layouts, look for an ++ * entry that matches our key with everything ++ * but the first layout stripped off. */ ++ if (startswith_comma(xc->layout, a[1])) ++ matching = 5; ++ else { ++ _cleanup_free_ char *x = NULL; ++ ++ /* If that didn't work, strip off the ++ * other layouts from the entry, too */ ++ x = strdupcspn(a[1], ","); ++ if (!x) ++ return -ENOMEM; ++ if (startswith_comma(xc->layout, x)) ++ matching = 1; ++ } + } + } + +@@ -848,7 +862,7 @@ int find_legacy_keymap(const X11Context *xc, char **ret) { + } + } + +- if (best_matching < 10 && !isempty(xc->layout)) { ++ if (best_matching < 9 && !isempty(xc->layout)) { + _cleanup_free_ char *l = NULL, *v = NULL, *converted = NULL; + + /* The best match is only the first part of the X11 +diff --git a/src/locale/test-localed-util.c b/src/locale/test-localed-util.c +index a19d80a967..f702ff29b0 100644 +--- a/src/locale/test-localed-util.c ++++ b/src/locale/test-localed-util.c +@@ -192,11 +192,14 @@ TEST(x11_convert_to_vconsole) { + assert_se(streq(vc.keymap, "fr-latin9")); + vc_context_clear(&vc); + ++ /* https://bugzilla.redhat.com/show_bug.cgi?id=1039185 */ ++ /* us,ru is the x config users want, but they still want ru ++ as the console layout in this case */ + log_info("/* test with a compound mapping (us,ru:) */"); + assert_se(free_and_strdup(&xc.layout, "us,ru") >= 0); + assert_se(free_and_strdup(&xc.variant, NULL) >= 0); + assert_se(x11_convert_to_vconsole(&xc, &vc) >= 0); +- assert_se(streq(vc.keymap, "us")); ++ assert_se(streq(vc.keymap, "ru")); + vc_context_clear(&vc); + + log_info("/* test with a compound mapping (ru,us:) */"); +-- +2.41.0 + diff --git a/systemd.spec b/systemd.spec index e1420f8..cf86d44 100644 --- a/systemd.spec +++ b/systemd.spec @@ -107,6 +107,12 @@ Patch0001: https://github.com/systemd/systemd/pull/26494.patch # Backport of patches that allow reloading of units Patch0002: https://github.com/systemd/systemd/pull/28521/commits/631d2b05ec5195d1f8f8fbff8a2dfcbf23d0b7aa.patch +# Backport of improvements to console keyboard layout guessing +# https://github.com/systemd/systemd/pull/29215 +# https://bugzilla.redhat.com/show_bug.cgi?id=1912609 +Patch0003: 0001-find_legacy_keymap-fix-empty-variant-matching.patch +Patch0004: 0002-find_legacy_keymap-try-matching-with-layout-order-re.patch + # Those are downstream-only patches, but we don't want them in packit builds: # https://bugzilla.redhat.com/show_bug.cgi?id=1738828 Patch0490: use-bfq-scheduler.patch