From a0824f1b9a08c0dee9756cdaa50dbfd3077da21d Mon Sep 17 00:00:00 2001 From: Jiri Konecny Date: Fri, 28 Feb 2025 11:58:06 +0100 Subject: [PATCH 1/3] Remove misleading TODO We don't plan to extend Live specific keyboard configuration but rather we want to onboard everyone to localed which will be one single solution. --- pyanaconda/modules/localization/live_keyboard.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pyanaconda/modules/localization/live_keyboard.py b/pyanaconda/modules/localization/live_keyboard.py index 160f6c9a8bc..73d885bd8e7 100644 --- a/pyanaconda/modules/localization/live_keyboard.py +++ b/pyanaconda/modules/localization/live_keyboard.py @@ -34,7 +34,6 @@ def get_live_keyboard_instance(): if conf.system.provides_liveuser: return GnomeShellKeyboard() - # TODO: Add support for other Live systems return None From ef46267579acd15c4ad0d9d1ecdb60eef99b1842 Mon Sep 17 00:00:00 2001 From: Jiri Konecny Date: Fri, 28 Feb 2025 15:45:18 +0100 Subject: [PATCH 2/3] Raise exception on unsupported layouts from Live The Live system (Gnome Shell) supports ibus input methods which are not supported by Anaconda. Anaconda can't support ibus because vconsole nor localed supports this because they need to be usable in TTY environment. Related: rhbz#2348726 --- .../modules/localization/live_keyboard.py | 24 ++++++----- pyanaconda/modules/localization/runtime.py | 2 + .../localization/test_live_keyboard.py | 40 ++++++++++++++----- 3 files changed, 47 insertions(+), 19 deletions(-) diff --git a/pyanaconda/modules/localization/live_keyboard.py b/pyanaconda/modules/localization/live_keyboard.py index 73d885bd8e7..3f1b7c1ade6 100644 --- a/pyanaconda/modules/localization/live_keyboard.py +++ b/pyanaconda/modules/localization/live_keyboard.py @@ -20,7 +20,9 @@ from pyanaconda.anaconda_loggers import get_module_logger from pyanaconda.core.configuration.anaconda import conf +from pyanaconda.core.i18n import _ from pyanaconda.core.util import execWithCaptureAsLiveUser +from pyanaconda.modules.common.errors.configuration import KeyboardConfigurationError log = get_module_logger(__name__) @@ -90,14 +92,18 @@ def _convert_to_xkb_format(self, sources): result = [] for t in sources: - # keep only 'xkb' type and ignore 'ibus' variants which can't be used in localed - if t[0] == "xkb": - layout = t[1] - # change layout variant from 'cz+qwerty' to 'cz (qwerty)' - if '+' in layout: - layout, variant = layout.split('+') - result.append(f"{layout} ({variant})") - else: - result.append(layout) + # keep only 'xkb' type and raise an error on 'ibus' variants which can't + # be used in localed + if t[0] != "xkb": + msg = _("The live system has layout '%s' which can't be used for installation.") + raise KeyboardConfigurationError(msg.format(t[1])) + + layout = t[1] + # change layout variant from 'cz+qwerty' to 'cz (qwerty)' + if '+' in layout: + layout, variant = layout.split('+') + result.append(f"{layout} ({variant})") + else: + result.append(layout) return result diff --git a/pyanaconda/modules/localization/runtime.py b/pyanaconda/modules/localization/runtime.py index d0d1ab283d0..ac7bbcb7998 100644 --- a/pyanaconda/modules/localization/runtime.py +++ b/pyanaconda/modules/localization/runtime.py @@ -90,6 +90,8 @@ def run(self): :returns: tuple of X layouts and VC keyboard settings :rtype: (list(str), str)) + :raises: KeyboardConfigurationError exception when we should use unsupported layouts + from Live """ return get_missing_keyboard_configuration(self._localed_wrapper, self._x_layouts, diff --git a/tests/unit_tests/pyanaconda_tests/modules/localization/test_live_keyboard.py b/tests/unit_tests/pyanaconda_tests/modules/localization/test_live_keyboard.py index e9b00dbff2e..2fc41109924 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/localization/test_live_keyboard.py +++ b/tests/unit_tests/pyanaconda_tests/modules/localization/test_live_keyboard.py @@ -18,6 +18,9 @@ import unittest from unittest.mock import patch +import pytest + +from pyanaconda.modules.common.errors.configuration import KeyboardConfigurationError from pyanaconda.modules.localization.live_keyboard import ( GnomeShellKeyboard, get_live_keyboard_instance, @@ -35,13 +38,24 @@ def test_get_live_keyboard_instance(self, mocked_conf): mocked_conf.system.provides_liveuser = False assert get_live_keyboard_instance() is None - def _check_gnome_shell_layouts_conversion(self, mocked_exec_with_capture, system_input, output): + def _check_gnome_shell_layouts_conversion(self, + mocked_exec_with_capture, + system_input, + output, + should_raise): mocked_exec_with_capture.reset_mock() mocked_exec_with_capture.return_value = system_input gs = GnomeShellKeyboard() - assert gs.read_keyboard_layouts() == output + if should_raise: + with pytest.raises(KeyboardConfigurationError): + gs.read_keyboard_layouts() + return + + result = gs.read_keyboard_layouts() + + assert result == output mocked_exec_with_capture.assert_called_once_with( "gsettings", ["get", "org.gnome.desktop.input-sources", "sources"] @@ -54,40 +68,46 @@ def test_gnome_shell_keyboard(self, mocked_exec_with_capture): self._check_gnome_shell_layouts_conversion( mocked_exec_with_capture=mocked_exec_with_capture, system_input=r"[('xkb', 'cz')]", - output=["cz"] + output=["cz"], + should_raise=False ) # test one complex layout is set self._check_gnome_shell_layouts_conversion( mocked_exec_with_capture=mocked_exec_with_capture, system_input=r"[('xkb', 'cz+qwerty')]", - output=["cz (qwerty)"] + output=["cz (qwerty)"], + should_raise=False ) # test multiple layouts are set self._check_gnome_shell_layouts_conversion( mocked_exec_with_capture=mocked_exec_with_capture, system_input=r"[('xkb', 'cz+qwerty'), ('xkb', 'us'), ('xkb', 'cz+dvorak-ucw')]", - output=["cz (qwerty)", "us", "cz (dvorak-ucw)"] + output=["cz (qwerty)", "us", "cz (dvorak-ucw)"], + should_raise=False ) - # test layouts with ibus (ibus is ignored) + # test layouts with ibus (ibus will raise error) self._check_gnome_shell_layouts_conversion( mocked_exec_with_capture=mocked_exec_with_capture, system_input=r"[('xkb', 'cz'), ('ibus', 'libpinyin')]", - output=["cz"] + output=[], + should_raise=True ) - # test only ibus layout + # test only ibus layout (raise the error) self._check_gnome_shell_layouts_conversion( mocked_exec_with_capture=mocked_exec_with_capture, system_input=r"[('ibus', 'libpinyin')]", - output=[] + output=[], + should_raise=True ) # test wrong input self._check_gnome_shell_layouts_conversion( mocked_exec_with_capture=mocked_exec_with_capture, system_input=r"wrong input", - output=[] + output=[], + should_raise=False ) From 9427fd52976aa4c1fe1330d3cda138887f943d69 Mon Sep 17 00:00:00 2001 From: Jiri Konecny Date: Fri, 28 Feb 2025 14:24:09 +0100 Subject: [PATCH 3/3] Add localization module API to get keyboard config This API is added so that web UI is able to read keyboard layouts without storing them to the localization module. Related: rhbz#2348726 --- .../modules/localization/localization.py | 15 +++++++++ .../localization/localization_interface.py | 28 ++++++++++++++++- pyanaconda/modules/localization/runtime.py | 6 ++++ .../localization/test_module_localization.py | 31 +++++++++++++++++-- 4 files changed, 76 insertions(+), 4 deletions(-) diff --git a/pyanaconda/modules/localization/localization.py b/pyanaconda/modules/localization/localization.py index 77699b3fb92..a45622ff7e5 100644 --- a/pyanaconda/modules/localization/localization.py +++ b/pyanaconda/modules/localization/localization.py @@ -368,6 +368,21 @@ def _update_settings_from_task(self, result): self.set_vc_keymap(vc_keymap) self.set_x_layouts(x_layouts) + def get_keyboard_configuration_with_task(self): + """Get current keyboard configuration without storing it into module. + + This configuration will be used for the installation at the time of task execution. + The task is read only, the results are not stored anywhere. + + :returns: a task reading keyboard configuration + """ + task = GetMissingKeyboardConfigurationTask( + localed_wrapper=self.localed_wrapper, + x_layouts=self.x_layouts, + vc_keymap=self.vc_keymap, + ) + return task + def apply_keyboard_with_task(self): """Apply keyboard configuration to the current system. diff --git a/pyanaconda/modules/localization/localization_interface.py b/pyanaconda/modules/localization/localization_interface.py index 8ac51f05434..6f2447a6e87 100644 --- a/pyanaconda/modules/localization/localization_interface.py +++ b/pyanaconda/modules/localization/localization_interface.py @@ -18,7 +18,7 @@ # Red Hat, Inc. # -from dasbus.server.interface import dbus_interface, dbus_signal +from dasbus.server.interface import dbus_class, dbus_interface, dbus_signal from dasbus.server.property import emits_properties_changed from dasbus.typing import * # pylint: disable=wildcard-import @@ -27,6 +27,21 @@ from pyanaconda.modules.common.containers import TaskContainer from pyanaconda.modules.common.structures.keyboard_layout import KeyboardLayout from pyanaconda.modules.common.structures.language import LanguageData, LocaleData +from pyanaconda.modules.common.task import TaskInterface + + +@dbus_class +class KeyboardConfigurationTaskInterface(TaskInterface): + """Interface to get keyboard configuration data.""" + + @staticmethod + def convert_result(value): + """Convert value to publishable result. + + From: + (("us", "cs (qwerty)"), "cs-qwerty") + """ + return get_variant(Tuple[List[Str], Str], value) @dbus_interface(LOCALIZATION.interface_name) @@ -255,6 +270,17 @@ def PopulateMissingKeyboardConfigurationWithTask(self) -> ObjPath: self.implementation.populate_missing_keyboard_configuration_with_task() ) + def GetKeyboardConfigurationWithTask(self) -> ObjPath: + """Get current keyboard configuration without storing it into module. + + This task will give you a potential configuration to be installed at the time of + task execution. The task is read only, the results are not used anywhere by + the localization module. + """ + return TaskContainer.to_object_path( + self.implementation.get_keyboard_configuration_with_task() + ) + def ApplyKeyboardWithTask(self) -> ObjPath: """Apply keyboard configuration to the current system. diff --git a/pyanaconda/modules/localization/runtime.py b/pyanaconda/modules/localization/runtime.py index ac7bbcb7998..a2d2d651bad 100644 --- a/pyanaconda/modules/localization/runtime.py +++ b/pyanaconda/modules/localization/runtime.py @@ -21,6 +21,9 @@ from pyanaconda.modules.common.errors.configuration import KeyboardConfigurationError from pyanaconda.modules.common.task import Task from pyanaconda.modules.localization.installation import write_vc_configuration +from pyanaconda.modules.localization.localization_interface import ( + KeyboardConfigurationTaskInterface, +) from pyanaconda.modules.localization.utils import get_missing_keyboard_configuration log = get_module_logger(__name__) @@ -81,6 +84,9 @@ def __init__(self, localed_wrapper, x_layouts, vc_keymap): self._x_layouts = x_layouts self._vc_keymap = vc_keymap + def for_publication(self): + return KeyboardConfigurationTaskInterface(self) + @property def name(self): return "Get missing keyboard settings." diff --git a/tests/unit_tests/pyanaconda_tests/modules/localization/test_module_localization.py b/tests/unit_tests/pyanaconda_tests/modules/localization/test_module_localization.py index d900d97e7ba..9f7161a3754 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/localization/test_module_localization.py +++ b/tests/unit_tests/pyanaconda_tests/modules/localization/test_module_localization.py @@ -23,18 +23,21 @@ import langtable from dasbus.signal import Signal -from dasbus.typing import Bool, Str, get_variant +from dasbus.typing import Bool, List, Str, Tuple, get_variant from pyanaconda.modules.common.constants.services import LOCALIZATION from pyanaconda.modules.common.structures.keyboard_layout import KeyboardLayout from pyanaconda.modules.common.structures.language import LanguageData, LocaleData -from pyanaconda.modules.common.task import TaskInterface +from pyanaconda.modules.common.task import TaskInterface, sync_run_task from pyanaconda.modules.localization.installation import ( KeyboardInstallationTask, LanguageInstallationTask, ) from pyanaconda.modules.localization.localization import LocalizationService -from pyanaconda.modules.localization.localization_interface import LocalizationInterface +from pyanaconda.modules.localization.localization_interface import ( + KeyboardConfigurationTaskInterface, + LocalizationInterface, +) from pyanaconda.modules.localization.runtime import ( ApplyKeyboardTask, GetMissingKeyboardConfigurationTask, @@ -350,9 +353,31 @@ def test_populate_missing_keyboard_configuration_with_task(self, publisher): task_path = self.localization_interface.PopulateMissingKeyboardConfigurationWithTask() obj = check_task_creation(task_path, publisher, GetMissingKeyboardConfigurationTask) + assert isinstance(obj, KeyboardConfigurationTaskInterface) + assert obj.implementation._vc_keymap == 'us' + assert obj.implementation._x_layouts == ['cz', 'cz (qwerty)'] + + @patch_dbus_publish_object + def test_get_keyboard_configuration_with_task(self, publisher): + """Test GetKeyboardConfigurationWithTask.""" + self.localization_interface.VirtualConsoleKeymap = 'us' + self.localization_interface.XLayouts = ['cz', 'cz (qwerty)'] + + task_path = self.localization_interface.GetKeyboardConfigurationWithTask() + + obj = check_task_creation(task_path, publisher, GetMissingKeyboardConfigurationTask) + assert isinstance(obj, KeyboardConfigurationTaskInterface) assert obj.implementation._vc_keymap == 'us' assert obj.implementation._x_layouts == ['cz', 'cz (qwerty)'] + with patch.object(obj.implementation, "run") as run_mock: + run_mock.return_value = ("cz (qwerty)", "us"), "cz-qwerty" + + sync_run_task(obj) + # test the conversion from task to DBus variant + assert obj.GetResult() == get_variant(Tuple[List[Str], Str], + (['cz (qwerty)', 'us'], 'cz-qwerty')) + @patch_dbus_publish_object def test_apply_keyboard_with_task(self, publisher): """Test ApplyKeyboardWithTask."""