PATCH 1/2 replaces the anaconda.keyboard object, which was a Keyboard object from system-config-keyboard. The only method that was used was the activate() method, that basically called loadkeys and setxkbmap. We now use libxklavier to handle X layouts and we can call loadkeys ourselves. Once systemd-localed has the capability to load X layouts in the virtual console, we could easily replace calling loadkeys with calling systemd-localed methods via DBus.
The only problem is, that system-config-keyboard uses a table mapping console keymaps to X layouts. But this table is not at all complete (only few keymaps compared to the available keymaps in /lib/xkb/keymaps/i386 directory contents) and once systemd-localed gets patched to load X layouts for console, this mapping will not be necessary. For now, I've added the code, that tries to load the keymap as given by user (e.g. 'cz_cp1250' or it could be an X layout) and if it fails, it tries to load the basic variant (e.g. 'cz' for 'cz (qwerty)'). This way, given a valid keymap, we load it, given an invalid keymap (X layout), we load the basic variant and setup proper layout for the X session.
Could we drop console keymaps at all once systemd-localed gets patched to load X layouts supporting only X layouts and not the legacy keymaps?
Ideas? Suggestions?
I believe PATCH 2/2 is okay, we don't use our loadKeymap code anywhere and I see no reason to use it instead of the loadkeys command.
Vratislav Podzimek (2): Replace system-config-keyboard with our methods using ksdata.keyboard Remove our loadKeymap code from isys
anaconda | 6 +- anaconda.spec.in | 2 - pyanaconda/__init__.py | 2 - pyanaconda/isys/__init__.py | 6 -- pyanaconda/isys/isys.c | 1 - pyanaconda/isys/lang.c | 108 -------------------------------- pyanaconda/isys/lang.h | 18 ------ pyanaconda/keyboard.py | 67 +++++++++++++++++--- pyanaconda/language.py | 12 ---- tests/pyanaconda_test/language_test.py | 20 ------ 10 files changed, 63 insertions(+), 179 deletions(-)
The only thing we were using from the system-config-keyboard was the activate method that basically just calls loadkeys command. This can be easily replaced with our own method using data from ksdata.keyboard allowing us to remove anaconda.keyboard object. --- anaconda | 6 ++- anaconda.spec.in | 2 - pyanaconda/__init__.py | 2 - pyanaconda/keyboard.py | 67 ++++++++++++++++++++++++++++---- pyanaconda/language.py | 12 ------ tests/pyanaconda_test/language_test.py | 20 ---------- 6 files changed, 63 insertions(+), 46 deletions(-)
diff --git a/anaconda b/anaconda index 256e7cd..2c17b58 100755 --- a/anaconda +++ b/anaconda @@ -725,6 +725,7 @@ if __name__ == "__main__": from pyanaconda import vnc from pyanaconda import kickstart from pyanaconda import ntp + from pyanaconda import keyboard
# to set UTF8 mode on the terminal, we need LANG to be set usefully if os.environ.get("LANG", "C") == "C": @@ -901,8 +902,9 @@ if __name__ == "__main__": if opts.keymap: if not ksdata.keyboard.keyboard: ksdata.keyboard.keyboard = opts.keymap - anaconda.keyboard.set(opts.keymap) - anaconda.keyboard.activate() + + if ksdata.keyboard.keyboard: + keyboard.activate_console_keymap(ksdata.keyboard.keyboard)
# now start the interface setupDisplay(anaconda, opts) diff --git a/anaconda.spec.in b/anaconda.spec.in index 2c5b246..3eb99a6 100644 --- a/anaconda.spec.in +++ b/anaconda.spec.in @@ -85,7 +85,6 @@ BuildRequires: zlib-devel BuildRequires: NetworkManager-devel >= %{nmver} BuildRequires: NetworkManager-glib-devel >= %{nmver} BuildRequires: dbus-devel >= %{dbusver}, dbus-python -BuildRequires: system-config-keyboard >= %{sckeyboardver} %ifarch %livearches BuildRequires: desktop-file-utils %endif @@ -126,7 +125,6 @@ Requires: python-cryptsetup >= %{pythoncryptsetupver} Requires: mdadm Requires: lvm2 Requires: util-linux >= 2.15.1 -Requires: system-config-keyboard >= %{sckeyboardver} Requires: dbus-python Requires: python-pwquality Requires: python-bugzilla diff --git a/pyanaconda/__init__.py b/pyanaconda/__init__.py index eb44ca9..4eb3179 100644 --- a/pyanaconda/__init__.py +++ b/pyanaconda/__init__.py @@ -44,7 +44,6 @@ _ = lambda x: gettext.ldgettext("anaconda", x) class Anaconda(object): def __init__(self): import desktop, firewall, security - import system_config_keyboard.keyboard as keyboard from flags import flags
self._backend = None @@ -60,7 +59,6 @@ class Anaconda(object): self._instLanguage = None self._intf = None self.isHeadless = False - self.keyboard = keyboard.Keyboard() self.ksdata = None self.mediaDevice = None self.methodstr = None diff --git a/pyanaconda/keyboard.py b/pyanaconda/keyboard.py index d0497d6..9a6c21f 100755 --- a/pyanaconda/keyboard.py +++ b/pyanaconda/keyboard.py @@ -31,6 +31,11 @@ for listing and various modifications of keyboard layouts settings. """
import os +import re +from pyanaconda import iutil + +import logging +log = logging.getLogger("anaconda")
class KeyboardConfigError(Exception): """Exception class for keyboard configuration related problems""" @@ -100,30 +105,76 @@ def get_layouts_xorg_conf(keyboard):
def write_layouts_config(keyboard, root): """ - Function that writes a file with layouts configuration to - $root/etc/X11/xorg.conf.d/01-anaconda-layouts.conf + Function that writes files with layouts configuration to + $root/etc/X11/xorg.conf.d/01-anaconda-layouts.conf and + $root/etc/sysconfig/keyboard.
@param keyboard: ksdata.keyboard object @param root: path to the root of the installed system
"""
- conf_dir = os.path.normpath(root + "/etc/X11/xorg.conf.d") - conf_file = "01-anaconda-keyboard.conf" + xconf_dir = os.path.normpath(root + "/etc/X11/xorg.conf.d") + xconf_file = "01-anaconda-keyboard.conf" + + sysconf_file = "/etc/sysconfig/keyboard"
try: - if not os.path.isdir(conf_dir): - os.makedirs(conf_dir) + if not os.path.isdir(xconf_dir): + os.makedirs(xconf_dir)
except OSError as oserr: raise KeyboardConfigError("Cannot create directory xorg.conf.d")
try: - with open(os.path.join(conf_dir, conf_file), "w") as f: + with open(os.path.join(xconf_dir, xconf_file), "w") as f: f.write(get_layouts_xorg_conf(keyboard))
+ with open(sysconf_file, "w") as f: + f.write('KEYTABLE="%s"\n' % keyboard.keyboard) + except IOError as ioerr: - raise KeyboardConfigError("Cannot write keyboard configuration file") + raise KeyboardConfigError("Cannot write keyboard configuration files") + +def activate_console_keymap(keymap): + """ + Try to setup a given keymap as a console keymap. If there is no such + keymap, try to setup a basic variant (e.g. 'cz' instead of 'cz (qwerty)'). + + @param keymap: a keymap + @type keymap: string + @raise KeyboardConfigError: if loadkeys command is not available + @return: False if failed to activate both the given keymap and its basic + variant, True otherwise + + """ + + try: + #TODO: replace with calling systemd-localed methods once it can load + # X layouts + ret = iutil.execWithRedirect("loadkeys", [keymap], stdout="/dev/tty5", + stderr="/dev/tty5") + except OSError as oserr: + msg = "'loadkeys' command not available (%s)" % oserr.strerror + raise KeyboardConfigError(msg) + + if ret != 0: + log.error("Failed to activate keymap %s" % keymap) + + #failed to activate the given keymap, extract and try + #the basic keymap -- e.g. 'cz-cp1250' -> 'cz' + parts = re.split(r'[- _(]', keymap, 1) + keymap = parts[0] + + ret = iutil.execWithRedirect("loadkeys", [keymap], stdout="/dev/tty5", + stderr="/dev/tty5") + + if ret != 0: + log.error("Failed to activate basic variant %s" % keymap) + else: + log.error("Activated basic variant %s, instead" % keymap) + + return ret == 0
def item_str(s): """Convert a zero-terminated byte array to a proper str""" diff --git a/pyanaconda/language.py b/pyanaconda/language.py index f1663d7..fb8d35e 100644 --- a/pyanaconda/language.py +++ b/pyanaconda/language.py @@ -28,7 +28,6 @@ import gettext from pyanaconda.constants import ROOT_PATH, DEFAULT_LANG import localeinfo from simpleconfig import SimpleConfigFile -import system_config_keyboard.keyboard as keyboard
import logging log = logging.getLogger("anaconda") @@ -165,17 +164,6 @@ class Language(object): def getCurrentLangSearchList(self): return localeinfo.expandLangs(self.systemLang) + ['C']
- def getDefaultKeyboard(self): - try: - return self.localeInfo[self.systemLang][3] - except KeyError: - try: - kbd = keyboard.Keyboard() - kbd.read(ROOT_PATH) - return kbd.get() - except: - return self.localeInfo[self._default][3] - def getDefaultTimeZone(self): try: return self.localeInfo[self.systemLang][4] diff --git a/tests/pyanaconda_test/language_test.py b/tests/pyanaconda_test/language_test.py index 5c895b2..2325a73 100644 --- a/tests/pyanaconda_test/language_test.py +++ b/tests/pyanaconda_test/language_test.py @@ -142,26 +142,6 @@ class LanguageTest(mock.TestCase): ret = lang.getCurrentLangSearchList() self.assertEqual(set(ret), set(['cs_CZ.UTF-8', 'cs_CZ', 'cs', 'C']))
- def get_default_keyboard_default_test(self): - import pyanaconda.language - lang = pyanaconda.language.Language() - ret = lang.getDefaultKeyboard() - self.assertEqual(ret, 'us') - - def get_default_keyboard_after_set_test(self): - import pyanaconda.language - lang = pyanaconda.language.Language() - lang.systemLang = 'cs' - ret = lang.getDefaultKeyboard() - self.assertEqual(ret, 'cz-lat2') - - def get_default_keyboard_with_cs_CZ_locale_test(self): - import pyanaconda.language - pyanaconda.language.os.environ = {'LANG': 'cs'} - lang = pyanaconda.language.Language() - ret = lang.getDefaultKeyboard() - self.assertEqual(ret, 'cz-lat2') - def get_default_time_zone_default_test(self): import pyanaconda.language pyanaconda.language.os.path.exists = mock.Mock(return_value=False)
@@ -901,8 +902,9 @@ if __name__ == "__main__": if opts.keymap: if not ksdata.keyboard.keyboard: ksdata.keyboard.keyboard = opts.keymap
anaconda.keyboard.set(opts.keymap)anaconda.keyboard.activate()
if ksdata.keyboard.keyboard:
keyboard.activate_console_keymap(ksdata.keyboard.keyboard)# now start the interface setupDisplay(anaconda, opts)
Could this be moved up to right under the following comment in anaconda:
# Some kickstart commands must be executed immediately, as they # affect how anaconda operates.
Also I guess at some point, we're going to need to have ksdata and command line opts play well with each other.
+def activate_console_keymap(keymap):
- """
- Try to setup a given keymap as a console keymap. If there is no such
- keymap, try to setup a basic variant (e.g. 'cz' instead of 'cz (qwerty)').
- @param keymap: a keymap
- @type keymap: string
- @raise KeyboardConfigError: if loadkeys command is not available
- @return: False if failed to activate both the given keymap and its basic
variant, True otherwise- """
- try:
#TODO: replace with calling systemd-localed methods once it can load# X layoutsret = iutil.execWithRedirect("loadkeys", [keymap], stdout="/dev/tty5",stderr="/dev/tty5")- except OSError as oserr:
msg = "'loadkeys' command not available (%s)" % oserr.strerrorraise KeyboardConfigError(msg)- if ret != 0:
log.error("Failed to activate keymap %s" % keymap)#failed to activate the given keymap, extract and try#the basic keymap -- e.g. 'cz-cp1250' -> 'cz'parts = re.split(r'[- _(]', keymap, 1)keymap = parts[0]
You should protect against parts being empty here.
- Chris
On Wed, 2012-07-25 at 12:24 -0400, Chris Lumens wrote:
@@ -901,8 +902,9 @@ if __name__ == "__main__": if opts.keymap: if not ksdata.keyboard.keyboard: ksdata.keyboard.keyboard = opts.keymap
anaconda.keyboard.set(opts.keymap)anaconda.keyboard.activate()
if ksdata.keyboard.keyboard:
keyboard.activate_console_keymap(ksdata.keyboard.keyboard)# now start the interface setupDisplay(anaconda, opts)
Could this be moved up to right under the following comment in anaconda:
# Some kickstart commands must be executed immediately, as they # affect how anaconda operates.
No problem.
Also I guess at some point, we're going to need to have ksdata and command line opts play well with each other.
Agreed. What will be the policy when the boot option and kickstart command for one thing are both used?
+def activate_console_keymap(keymap):
- """
- Try to setup a given keymap as a console keymap. If there is no such
- keymap, try to setup a basic variant (e.g. 'cz' instead of 'cz (qwerty)').
- @param keymap: a keymap
- @type keymap: string
- @raise KeyboardConfigError: if loadkeys command is not available
- @return: False if failed to activate both the given keymap and its basic
variant, True otherwise- """
- try:
#TODO: replace with calling systemd-localed methods once it can load# X layoutsret = iutil.execWithRedirect("loadkeys", [keymap], stdout="/dev/tty5",stderr="/dev/tty5")- except OSError as oserr:
msg = "'loadkeys' command not available (%s)" % oserr.strerrorraise KeyboardConfigError(msg)- if ret != 0:
log.error("Failed to activate keymap %s" % keymap)#failed to activate the given keymap, extract and try#the basic keymap -- e.g. 'cz-cp1250' -> 'cz'parts = re.split(r'[- _(]', keymap, 1)keymap = parts[0]You should protect against parts being empty here.
Good catch, I will add the check before pushing.
Apart from those two comments, do these two patches look alright?
Also I guess at some point, we're going to need to have ksdata and command line opts play well with each other.
Agreed. What will be the policy when the boot option and kickstart command for one thing are both used?
We should try to preserve whatever behavior we've kept in the past, so long as there is some consistency to it. I don't know what that behavior would be off the top of my head.
Apart from those two comments, do these two patches look alright?
Yep.
- Chris
It is used only in the text mode and it should be possible to replace it with calling 'loadkeys' command in the new text mode. --- pyanaconda/isys/__init__.py | 6 --- pyanaconda/isys/isys.c | 1 - pyanaconda/isys/lang.c | 108 ------------------------------------------- pyanaconda/isys/lang.h | 18 -------- 4 files changed, 133 deletions(-)
diff --git a/pyanaconda/isys/__init__.py b/pyanaconda/isys/__init__.py index fc766fc..63edbf6 100755 --- a/pyanaconda/isys/__init__.py +++ b/pyanaconda/isys/__init__.py @@ -171,12 +171,6 @@ def swapoff (path): def swapon (path): return _isys.swapon (path)
-## Load a keyboard layout for text mode installs. -# @param keymap The keyboard layout to load. This must be one of the values -# from rhpl.KeyboardModels. -def loadKeymap(keymap): - return _isys.loadKeymap (keymap) - def resetResolv(): return _isys.resetresolv()
diff --git a/pyanaconda/isys/isys.c b/pyanaconda/isys/isys.c index 8c4eb4b..35b2f33 100644 --- a/pyanaconda/isys/isys.c +++ b/pyanaconda/isys/isys.c @@ -126,7 +126,6 @@ static PyMethodDef isysModuleMethods[] = { { "resetresolv", (PyCFunction) doResetResolv, METH_VARARGS, NULL }, { "swapon", (PyCFunction) doSwapon, METH_VARARGS, NULL }, { "swapoff", (PyCFunction) doSwapoff, METH_VARARGS, NULL }, - { "loadKeymap", (PyCFunction) doLoadKeymap, METH_VARARGS, NULL }, { "vtActivate", (PyCFunction) doVtActivate, METH_VARARGS, NULL}, { "isPseudoTTY", (PyCFunction) doisPseudoTTY, METH_VARARGS, NULL}, { "isVioConsole", (PyCFunction) doisVioConsole, METH_NOARGS, NULL}, diff --git a/pyanaconda/isys/lang.c b/pyanaconda/isys/lang.c index 07dee56..c38b1bb 100644 --- a/pyanaconda/isys/lang.c +++ b/pyanaconda/isys/lang.c @@ -56,111 +56,3 @@ int isysSetUnicodeKeymap(void) { return 0; }
-/* the file pointer must be at the beginning of the section already! */ -int loadKeymap(gzFile stream) { - int console; - int kmap, key; - struct kbentry entry; - int keymaps[MAX_NR_KEYMAPS]; - int count = 0; - unsigned int magic; - short keymap[NR_KEYS]; - struct stat sb; - -#if defined (__s390__) || defined (__s390x__) - return 0; -#endif - if (isVioConsole()) - return 0; - - /* assume that if we're already on a pty loading a keymap is silly */ - fstat(0, &sb); - if (major(sb.st_rdev) == 3 || major(sb.st_rdev) == 136) - return 0; - - if (gzread(stream, &magic, sizeof(magic)) != sizeof(magic)) - return -EIO; - - if (magic != KMAP_MAGIC) return -EINVAL; - - if (gzread(stream, keymaps, sizeof(keymaps)) != sizeof(keymaps)) - return -EINVAL; - - console = open("/dev/tty0", O_RDWR); - if (console < 0) - return -EACCES; - - for (kmap = 0; kmap < MAX_NR_KEYMAPS; kmap++) { - if (!keymaps[kmap]) continue; - - if (gzread(stream, keymap, sizeof(keymap)) != sizeof(keymap)) { - close(console); - return -EIO; - } - - count++; - for (key = 0; key < NR_KEYS; key++) { - entry.kb_index = key; - entry.kb_table = kmap; - entry.kb_value = keymap[key]; - if (KTYP(entry.kb_value) != KT_SPEC) { - if (ioctl(console, KDSKBENT, &entry)) { - int ret = errno; - close(console); - return ret; - } - } - } - } - close(console); - return 0; -} - -int isysLoadKeymap(char * keymap) { - int num = -1; - int rc; - gzFile f; - struct kmapHeader hdr; - struct kmapInfo * infoTable; - char buf[16384]; /* I hope this is big enough */ - int i; - - f = gzopen("/etc/keymaps.gz", "r"); - if (!f) return -EACCES; - - if (gzread(f, &hdr, sizeof(hdr)) != sizeof(hdr)) { - gzclose(f); - return -EINVAL; - } - - i = hdr.numEntries * sizeof(*infoTable); - infoTable = alloca(i); - if (gzread(f, infoTable, i) != i) { - gzclose(f); - return -EIO; - } - - for (i = 0; i < hdr.numEntries; i++) - if (!strcmp(infoTable[i].name, keymap)) { - num = i; - break; - } - - if (num == -1) { - gzclose(f); - return -ENOENT; - } - - for (i = 0; i < num; i++) { - if (gzread(f, buf, infoTable[i].size) != infoTable[i].size) { - gzclose(f); - return -EIO; - } - } - - rc = loadKeymap(f); - - gzclose(f); - - return rc; -} diff --git a/pyanaconda/isys/lang.h b/pyanaconda/isys/lang.h index b5ff4da..aac821a 100644 --- a/pyanaconda/isys/lang.h +++ b/pyanaconda/isys/lang.h @@ -20,24 +20,6 @@ #ifndef ISYS_LANG_H #define ISYS_LANG_H
-#include <zlib.h> - -/* define ask johnsonm@redhat.com where this came from */ -#define KMAP_MAGIC 0x8B39C07F -#define KMAP_NAMELEN 40 /* including '\0' */ - -struct kmapHeader { - int magic; - int numEntries; -}; - -struct kmapInfo { - int size; - char name[KMAP_NAMELEN]; -}; - -int loadKeymap(gzFile stream); -int isysLoadKeymap(char * keymap); int isysSetUnicodeKeymap(void);
#endif
Could we drop console keymaps at all once systemd-localed gets patched to load X layouts supporting only X layouts and not the legacy keymaps?
I think we will always have to make sure the console keyboard layout is set to something, though. Think about this: you do an install and set your keyboard to whatever layout. Later during the install, you decide you want to switch over to tty2 and do something. However if we don't set the keyboard layout there, you'll have completely different settings. It would be pretty weird.
- Chris
On Wed, 2012-07-25 at 12:28 -0400, Chris Lumens wrote:
Could we drop console keymaps at all once systemd-localed gets patched to load X layouts supporting only X layouts and not the legacy keymaps?
I think we will always have to make sure the console keyboard layout is set to something, though. Think about this: you do an install and set your keyboard to whatever layout. Later during the install, you decide you want to switch over to tty2 and do something. However if we don't set the keyboard layout there, you'll have completely different settings. It would be pretty weird.
I've expressed myself badly. I meant we could drop keymaps from /lib/kbd/keymaps/i386/ and use only X layouts for both X and console, once systemd-localed is able to load X layouts as console layouts.
I've expressed myself badly. I meant we could drop keymaps from /lib/kbd/keymaps/i386/ and use only X layouts for both X and console, once systemd-localed is able to load X layouts as console layouts.
Ahh, I didn't know that was possible. If so, I'd certainly be fine with getting rid of the keymaps directory.
- Chris
anaconda-patches@lists.fedorahosted.org