Bug 478431

Summary: re-implementation (in python) for fedora-setup-keyboard which respects user settings
Product: [Fedora] Fedora Reporter: Alexander Kanevskiy <kad>
Component: xorg-x11-serverAssignee: Adam Jackson <ajax>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: high    
Version: 10CC: adel.gadllah, ajax, harald, ikaris.pt, kir, maurizio.antillon, peter.hutterer, xgl-maint
Target Milestone: ---Keywords: Patch
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: 1.5.3-13.fc10 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-02-10 19:00:56 EST Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Attachments:
Description Flags
Re-implementation of shell script fedora-setup-keyboard in Python with fixes.
none
Newer version none

Description Alexander Kanevskiy 2008-12-29 17:46:07 EST
Created attachment 327935 [details]
Re-implementation of shell script fedora-setup-keyboard in Python with fixes.

Description of problem:

Current version of /usr/bin/fedora-setup-keyboard always reset keyboard layout/options settings to something which is listed in rhpl.keyboard*.
So, if I'd like to have different key combination to switch layout's - I'm out of luck or need to override via /etc/hal/fdi/policy/10-mykeymap

I've fixed that behavior, so now fedora-setup-keyboard tries to set something which is present in /etc/sysconfig/keyboard, and then if not used, from rhpl.keyboard defaults.

Also, I've noticed comment: 
"
# Yes, this should really just be written in python.  If you can figure
# out how to make hal callouts written in python _work_, then please
# let me know.  In the meantime, we'll do this.
"
So, in attachment, my python implementation of the same thing.

Version-Release number of selected component (if applicable):
xorg-x11-server-Xorg-1.5.3-6.fc10.x86_64

How reproducible:
Always for non-us keyboards.

Steps to Reproduce:
1. set your keyboard map to something non-us via system-config-keyboard
e.g. Russian/Ukrainian/etc.
2. adjust key combination to switch layout in /etc/sysconfig/keyboard
E.g. change default for russian 
OPTIONS="grp:shifts_toggle,grp_led:scroll"
to 
OPTIONS="grp:ctrl_shift_toggle,grp_led:scroll"
3. remove and re-insert USB keyboard.
4. Notice, using lshal that your keyboard input.xkb.options still have "shifts_toggle"
  
Actual results:
fedora-setup-keyboard doesn't respect e.g. OPTIONS from /etc/sysconfig/keyboard while initializing keyboard via HAL

Expected results:
User settings from /etc/sysconfig/keyboard should have preference over defaults from rhpl.

Additional info:
Comment 1 Alexander Kanevskiy 2008-12-30 03:37:26 EST
Overnight I realized that user might want to set some variable to empty value, and it wouldn't work correct. Just replace line:

value = kbd_config.info.get(key.upper(), "") or result_dict[key]

to 

value = kbd_config.info.get(key.upper(), result_dict[key])
Comment 2 Kir Kolyshkin 2009-01-24 11:44:20 EST
I was very disappointed then I upgraded my F9 to F10 and keyboard switching stopped working miserably (I'm using XFCE, Xkb options set in xorg.conf). Had to add HAL rule to fix that.

I vote for kad's script. Ways better than the one shipped in Fedora!

Current implementation of fedora-setup-keyboard is ugly because
(1) everything is hardcoded, no way to change any xkb option
(2) in case of unknown KEYTABLE it fails with undecryptable diagnostics:

[kir@kir ~]$ cat /etc/sysconfig/keyboard 
KEYBOARDTYPE="pc"
KEYTABLE="asp_custom"
[kir@kir ~]$ /usr/bin/fedora-setup-keyboard 
Traceback (most recent call last):
  File "<string>", line 1, in <module>
KeyError: 'asp_custom'

Alexandr, couple of comments.

(1) IMHO using KEYTABLE is a bit wrong, since it used to be the option for text console only and can differ from X setup. The fact that fedora-setup-keyboard is using it doesn't mean it's the way to go.

(1) What about adding parsing of xorg.conf (left from older Fedora install) as a fallback? Should be pretty easy to implement, we can stay backward-compatible, and print a warning that such and such options should now be set in /etc/sysconfig/keyboard?

[kir@kir ~]$ grep Xkb /etc/X11/xorg.conf
	Option	    "XkbModel" "inet"
	Option	    "XkbLayout" "us,ru(winkeys)"
	Option	    "XkbOptions" "grp:ctrl_shift_toggle,grp_led:scroll"
Comment 3 Alexander Kanevskiy 2009-01-25 10:14:25 EST
Created attachment 329939 [details]
Newer version

This version has changes that I described in comment #1 and would work correctly for scenarios where KEYTABLE is something custom or empty. It's used as initial reference set by system-config-keyboard, but everything which it affects to, can be overridden by rest of variables in /etc/sysconfig/keyboard.
Comment 4 Alexander Kanevskiy 2009-01-25 10:18:47 EST
(In reply to comment #2)
> Alexandr, couple of comments.
> 
> (1) IMHO using KEYTABLE is a bit wrong, since it used to be the option for text
> console only and can differ from X setup. The fact that fedora-setup-keyboard
> is using it doesn't mean it's the way to go.

I've uploaded new version, which minimizes usage of this variable. 
It's the best possible hint to start with. This variable is set by system-config-keyboard.
The reset of variables can be overridden by the rest of variables in /etc/sysconfig/keyboard

> (1) What about adding parsing of xorg.conf (left from older Fedora install) as
> a fallback? Should be pretty easy to implement, we can stay
> backward-compatible, and print a warning that such and such options should now
> be set in /etc/sysconfig/keyboard?
> 
> [kir@kir ~]$ grep Xkb /etc/X11/xorg.conf
>  Option     "XkbModel" "inet"
>  Option     "XkbLayout" "us,ru(winkeys)"
>  Option     "XkbOptions" "grp:ctrl_shift_toggle,grp_led:scroll"

I think the best place for such parser would be %postinst which will read /etc/X11/xorg.conf and move needed variables to /etc/sysconfig/keyboard

fedora-setup-keyboard script is meant to be used on each device plug/un-plug (quite frequently), so I don't think it would be beneficial to open many files each time.
Comment 5 Alexei Podtelezhnikov 2009-01-27 10:49:35 EST
I saw a koji build of xorg-server referring to this bug report. 
http://koji.fedoraproject.org/koji/buildinfo?buildID=80390
Comment 6 Matěj Cepl 2009-01-27 17:38:09 EST
*** Bug 476028 has been marked as a duplicate of this bug. ***
Comment 7 Harald Hoyer 2009-01-28 08:07:31 EST
Can you please tell me why /usr/bin/fedora-setup-keyboard has to be a python script and why it is called two times per boot process?

This is the only thing that uses python in the boot process in a standard installation.
Comment 8 Kir Kolyshkin 2009-01-28 09:58:11 EST
(In reply to comment #7)
> Can you please tell me why /usr/bin/fedora-setup-keyboard has to be a python
> script

Take a look at the original version of the script (shipped in Fedora 10). It says:

# Yes, this should really just be written in python.  If you can figure
# out how to make hal callouts written in python _work_, then please
# let me know.  In the meantime, we'll do this.

This was the first reason I guess.

The second one is the script uses sorta database that links a console layout name (like 'dvorak' or 'ru' or 'fi') to a list of parameters (keyboard name, list of xkb layouts (first one being always 'us'), xkb keyboard model (mostly 'pc105'), xkb layout variant, xkb options (like how to switch layouts). This database is expressed as python dict and is embedded into a python module named 'keyboard_models' which is a part of package called rhpl.

So this is the second reason -- at least some part of script has to be written in Python in order to access this database.

> This is the only thing that uses python in the boot process in a standard
> installation.

Personally I see no problem to rewrite it in bash, except for the above DB thing.
Comment 9 Harald Hoyer 2009-01-28 11:10:11 EST
As I said, this is the only tool, that brings in the whole python interpreter stack at boot time and even executed twice :-(.
Comment 10 Kir Kolyshkin 2009-01-28 11:20:38 EST
Actually, the biggest problem is the approach chosen.

In Fedora 9 and earlier there were a few methods of setting up XKB settings in X Window keyboard:
(1) using KDE
(2) using GNOME
(3) using xorg.conf (options for InputDevice section where Driver is 'kbd').
(4) using setxkbmap, say from ~/.xinitrc

People that do not use neither GNOME nor KDE are left with option (3) or (4).

Now, in Fedora 9, there are six(!) methods:
(1) using KDE
(2) using GNOME
(3) using xorg.conf (no longer works w/o disabling HAL telling X about input devices)
(4) using setxkbmap
(5) using HAL (putting some cryptic XML into /etc/hal/fdi/policy/10-keymap.fdi)
(6) using fedora-setup-keyboard

Now, the problem is people who used option (3) in Fedora 9 are now left helpless. There is no transition mechanism, the settings from xorg.conf just stop working.

More to say, there is a new mechanism (6) which tries to use CONSOLE keyboard settings (the name of keyboard layout file) to mock up some XKB settings. The problem is XKB settings differ much from console keyboard settings, so they can't be compared.

Console settings: a keytable map file which sets all keycodes for all the layouts, switch keys etc.

XKB settings: a list of layouts, ways of switching between those, various other options.

What fedora-config-keyboard is trying to do is to guess XKB settings from a filename used for console keymap. To my mind this is very wrong and shouldn't be done at all.

What can be done is:
1. While upgrading, migrate XKB settings from xorg.conf to some other place.
2. Modify system-setup-keyboard to choose both console and X keyboard settings.
3. Take X keyboard settings (which were put there by system-setup-keyboard) from /etc/sysconfig/keyboard.

Sorry for the long comment, just trying to rectify things.
Comment 11 Alexander Kanevskiy 2009-01-28 17:06:12 EST
(In reply to comment #10)
> What fedora-config-keyboard is trying to do is to guess XKB settings from a
> filename used for console keymap. To my mind this is very wrong and shouldn't
> be done at all.

Kir, you're not entirely right. It tries to guess sensible XKB default parameters from DB, if they are not defined in /etc/sysconfig/keyboard  
At least in my version :)

> What can be done is:
> 1. While upgrading, migrate XKB settings from xorg.conf to some other place.
that could be good approach to populate content of /etc/sysconfig/keyboard during upgrade.

> 2. Modify system-setup-keyboard to choose both console and X keyboard settings.
It's already doing that.  KEYTABLE is used only to search default values, if and only if, all other variables are not defined in /etc/sysconfig/keyboard

> 3. Take X keyboard settings (which were put there by system-setup-keyboard)
> from /etc/sysconfig/keyboard.
The idea of fedora-setup-keyboard, as I understood was exactly about that.
And put them to each new plugged device.

For example, my /etc/sysconfig/keyboard looks like that (a bit modified after system-config-keyboard):

KEYBOARDTYPE="pc"
KEYTABLE="ru"
LAYOUT="us,ru"
MODEL="pc105"
OPTIONS="grp:shifts_toggle,grp:switch,grp_led:scroll"
VARIANT=""


Maybe, it would be good idea, to extend somehow syntax of /etc/sysconfig/keyboard 
to use different xkb parameters for different keyboards which might be plugged on-fly
(e.g. KVM keyboard might use different group switch or a bit different layout) but this is place to improve in future. :)
Comment 12 Alexander Kanevskiy 2009-01-28 17:14:31 EST
(In reply to comment #7)
> Can you please tell me why /usr/bin/fedora-setup-keyboard has to be a python
> script and why it is called two times per boot process?

It's called by HAL as many times as many devices you have which provide compatibility input.keymap 

see, /usr/share/hal/fdi/policy/10osvendor/10-keymap.fdi 

    <match key="info.capabilities" contains="input.keymap">
      <append key="info.callouts.add" type="strlist">hal-setup-keymap</append>
    </match>

It's might be not the best choice, but that's how HAL works with spawning callouts.

> This is the only thing that uses python in the boot process in a standard
> installation.

Maybe. I've just re-wrote it in clean python because original version was much worse.

It was shell script which was spawning python, and then, 4 times it was spawning 
/usr/bin/hal-set-property. So, in total, 1 shell, 1 python, 4 hal-set-property. 
I think trade-off of calling python once per input.keymap capable device is better than 6 processes...

Theoretically it would be better to have that tool in plain C which will parse /etc/sysconfig/keyboard and work with HAL... It would be fastest solution, but it's harder to maintain C code in a long run.
Comment 13 Kir Kolyshkin 2009-01-29 08:47:55 EST
(In reply to comment #11)
> (In reply to comment #10)
> > 2. Modify system-setup-keyboard to choose both console and X keyboard settings.
> It's already doing that.  KEYTABLE is used only to search default values, if
> and only if, all other variables are not defined in /etc/sysconfig/keyboard

Right. The only problem is it does the same, i.e. substitute those defaults from the rhpl keyboard_models database. Which is wrong; but it's a subject of another bug.
Comment 14 Fedora Update System 2009-01-29 18:03:36 EST
xorg-x11-server-1.5.3-9.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update xorg-x11-server'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-1101
Comment 15 Adel Gadllah 2009-02-03 13:45:46 EST
(In reply to comment #9)
> As I said, this is the only tool, that brings in the whole python interpreter
> stack at boot time and even executed twice :-(.

This should solve the python issue: https://bugzilla.redhat.com/show_bug.cgi?id=483817
Comment 16 Fedora Update System 2009-02-04 21:16:59 EST
xorg-x11-server-1.5.3-10.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update xorg-x11-server'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-1308
Comment 17 Adel Gadllah 2009-02-05 09:24:06 EST
(In reply to comment #12)
> (In reply to comment #7)
> > Can you please tell me why /usr/bin/fedora-setup-keyboard has to be a python
> > script and why it is called two times per boot process?
> 
> It's called by HAL as many times as many devices you have which provide
> compatibility input.keymap 
> 
> see, /usr/share/hal/fdi/policy/10osvendor/10-keymap.fdi 
> 
>     <match key="info.capabilities" contains="input.keymap">
>       <append key="info.callouts.add" type="strlist">hal-setup-keymap</append>
>     </match>
> 
> It's might be not the best choice, but that's how HAL works with spawning
> callouts.

This should fix it https://bugzilla.redhat.com/show_bug.cgi?id=484217
Comment 18 Fedora Update System 2009-02-06 00:19:06 EST
xorg-x11-server-1.5.3-11.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update xorg-x11-server'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-1390
Comment 19 Peter Hutterer 2009-02-10 19:00:56 EST
I'm closing this as a duplicate of 483817.

It's not really a dupe, but this bug is originally about the reimplementation in python and this has been committed.
For the C implementation, please refer to bug 483817.

https://bugzilla.redhat.com/show_bug.cgi?id=483817

*** This bug has been marked as a duplicate of bug 483817 ***
Comment 20 Fedora Update System 2009-02-24 15:45:44 EST
xorg-x11-server-1.5.3-13.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 21 ikaris.pt 2009-03-01 17:21:33 EST
(related to comment #10 From  Kir Kolyshkin)

i did an upgrade (Fedora 10) and keyboard stop working... some googling ... found this bug report ...

now the confs are working ... but needed to put keyboard stuff in:
/etc/X11/xorg.conf
/etc/sysconfig/keyboard
/etc/hal/fdi/policy/10-keymap.fdi

if i alter/remove any option ... stop's working ...

my point is, why the "clever" way of guessing my keyboard layout :(

P.S. sorry, a bit frustrated, i did a lot of guessing
Comment 22 Peter Hutterer 2009-03-01 17:27:14 EST
Please open a new bug report for your issue and submit your /var/log/Xorg.0.log file.