this at least is true:

>>> from pyanaconda.keyboard import LocaledWrapper
>>> wrap = LocaledWrapper()
>>> wrap.convert_layout('ru')
''

I am not totally sure if it's a valid test, but I *think* it is.

Discussed during the 2016-05-09 blocker review meeting: [1]

Accepted as a blocker for F24 final due to violation of the following release criteria: 

"If a particular keyboard layout has been configured for the system, that keyboard layout must be used: ... When logging in at a console" [2]


[1] https://meetbot-raw.fedoraproject.org/fedora-blocker-review/2016-05-09/f24-blocker-review.2016-05-09-16.00.txt

[2] https://fedoraproject.org/wiki/Fedora_24_Final_Release_Criteria#Keyboard_layout_configuration

Created attachment 1158146 [details]
anaconda.log

Created attachment 1158147 [details]
dnf.log

Created attachment 1158148 [details]
dnf.rpm.log

Created attachment 1158149 [details]
journal.log

Created attachment 1158150 [details]
ks-script-5pepjbk3.log

Created attachment 1158151 [details]
ks-script-huo8ag6_.log

Created attachment 1158152 [details]
ks-script-xfr7kjrt.log

Created attachment 1158153 [details]
packaging.log

Created attachment 1158154 [details]
program.log

Created attachment 1158155 [details]
storage.log

Performed this test with F24 Beta 1.6 Workstation. After install, keyboard was defaulted to "us" layout, however, after going through initial setup (create user, root passwd, etc) and exiting Anaconda, the keyboard layout changed back to the original "ru" layout.

geoffrey: things are a little different between X and console for switched layouts. In X, two layouts should be configured by default, US and 'ru', with 'en' as the default and a switcher key combo for switching between the two of them. The US layout will only handle inputting Latin characters, the ru layout will only handle inputting Cyrillic characters, and the switcher key combo really does switch between these two, which are distinct separate layouts as far as X is concerned.

At the console the expected experience is broadly the same, but the way it works is not. You can only have a single console keyboard layout loaded at any given time, so for switched layouts like Russian, both US and native inputs and switching are implemented *within a single layout*; the console layout called 'ru' implements both Latin character input (with no modifiers active) and Cyrillic character input (with some modifier active) and has a 'switcher' combo which actually serves to lock the relevant modifier. So the result is a similar experience, but implemented differently. (See 'man keymaps' for more info on how console keyboard layouts work).

So for QA purposes at the console we need to check that 'ru' is the single loaded layout, but in graphical contexts we need to check both US and 'ru' are enabled and a switcher mechanism is in place, but US should be the default.

I can't quite tell if the case you described is as intended, I guess I'll check it out myself later.

Performed several more tests on F24 Beta 1.6 Workstation. Previous results remain the same with the addition that, after the install, when switched to a virtual terminal/console, the keymap is a standard US QWERTY layout. In X, the user has the option to switch between layouts through the use of the Gnome toolbar selector.

Summary:
When F24 Beta 1.6 Workstation Live is booted and installed from with 'ru' keymap selected, install progresses normally. After reboot, keymap changes from originally set 'ru' layout to 'en' layout in both the X window and the virutal terminal while the Anaconda installer finishes (select Location Services, add user account, set root password, etc). Once Anaconda has exited, the keymap in X switches from the 'en' layout back to the 'ru' layout, with the virtual terminal layout remaining as a standard US QWERTY layout.

The data in langtable for the "ru" layout seems OK:

python3
Python 3.4.3 (default, Mar 31 2016, 20:42:37) 
[GCC 5.3.1 20151207 (Red Hat 5.3.1-2)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import langtable
import langtable
>>> langtable.list_keyboards(languageId="ru")
langtable.list_keyboards(languageId="ru")
['ru']
>>> langtable.supports_ascii('ru')
langtable.supports_ascii('ru')
False
>>>

Under F23:
>>> wrap.convert_layout('ru')
Failed to set layouts: Failed to call SetX11Keyboard method on /org/freedesktop/locale1 with ('ru', '', '', '', true, false) arguments: GDBus.Error:org.freedesktop.DBus.Error.InteractiveAuthorizationRequired: Interactive authentication required.
Failed to set layouts: Failed to call SetX11Keyboard method on /org/freedesktop/locale1 with ('us', '', '', '', false, false) arguments: GDBus.Error:org.freedesktop.DBus.Error.InteractiveAuthorizationRequired: Interactive authentication required.
'us'

I think I ran my test as root (I believe anaconda runs as root, but not 100% sure).

I should probably note this doesn't work with F23 either, at least I'm pretty sure I tested and that's what happened (it was before I went on vacation). I'll double-check and also check F22 on Tuesday (tomorrow is a holiday here).

The result is '' when run under root on F23.

But that matches what localed seems to think:
$ localectl set-x11-keymap ru
$ localectl status|tail -n2
       VC Keymap: n/a
      X11 Layout: ru

But converting the other way around works:
$ localectl set-keymap ru
$ localectl status|tail -n4
       VC Keymap: ru
      X11 Layout: ru,us
       X11 Model: pc105
     X11 Options: terminate:ctrl_alt_bksp,grp:shifts_toggle,grp_led:scroll

The following also works:
$ localectl set-x11-keymap ru,us
$ localectl status|tail -n2
       VC Keymap: ru
      X11 Layout: ru,us

If somebody tells me that the first should also work (i.e. that X11 'ru' should convert to 'ru' on the console), I can work on adding that. But I won't even pretend that I understand what the rules for those conversions are.

The whole conversion system is really pretty odd and was, I'm fairly sure, initially written to work the *other way around* - to derive an xkb configuration from a user choice that was based on a list of kbd layouts. (this stuff all ultimately derives from the old system-config-keyboard / system-setup-keyboard , I think). All I'm willing to say for sure is this: when you do an install in Russian you should get the 'ru' console layout and both 'us' and 'ru' xkb layouts. I'm not gonna state definitively how that's supposed to be achieved because it's all pretty fuzzy.

Welp, we obviously didn't test this for a while - it's been broken since Fedora 21. It worked in F20. Dunno what changed, whether it's anaconda or systemd/localed.

OK, so I threw a couple of debug logs into convert_layout and confirmed what the issue is, I think:

DEBUG anaconda: convert_layout: called with ru
DEBUG anaconda: convert_layout: returning ['']

so as I thought, it seems, during a real install, convert_layout gets called with 'ru' and returns a list containing a single element, an empty string. This occurs during 'Configuring installed system'.

I really don't want to hazard a guess as to what the 'right way' to fix this is, though.

I also checked f20. In f20, 'convert_layout' did not exist, there was only 'set_and_convert_layout' (I think things were set up such that the code should always want to set and convert at the same time?) so I put the debug statements there, and got:

DEBUG anaconda: set_and_convert_layout: called with ru
DEBUG anaconda: set_and_convert_layout: returning ru

so in F20 it seems set_and_convert_layout returned a string (not a list) and when called with just 'ru', it returned 'ru' (which is what we want), not ''.

So in a stunning vindication of my habit of collecting ancient TCs (which will land me on an episode of Cyber Hoarders any time soon...), I managed to bisect this quite closely: it broke between F21 Beta TC3 and F21 Beta TC4. In anaconda terms, it broke between anaconda-21.48.9-1 and anaconda-21.48.10-1 . convert_layout existed in both of those. 21 Beta TC3:

DEBUG anaconda: convert_layout: called with ru
DEBUG anaconda: convert_layout: returning ru

21 Beta TC4:

DEBUG anaconda: convert_layout: called with ru
DEBUG anaconda: convert_layout: returning

So it seems that as of 21 Beta TC4 it returned just an empty string (not a list containing an empty string), but the effect was the same.

There was a single commit difference in keyboard.py between those two builds, specifically, this commit:

https://github.com/rhinstaller/anaconda/commit/ed4d1d1cd46d4b3ea1db7bf32ff91f26e60b8661

Per https://fedorahosted.org/rel-eng/ticket/6010 , 21 Beta TC3 came on 2014-10-10, 21 Beta TC4 came on 2014-10-17. Per the Branched reports for that date range, systemd did not change during that period, nor was any systemd build listed in the Beta TC4 compose request. I'll check that applying the keyboard.py change to Beta TC3 causes the bug to happen, just to verify, but it looks very much like it was the cause of this bug.

One possible issue I can see with that commit is that it may break the fallback commented:

            # nothing suggested by systemd-localed, but we may try to use the
            # same string for both VConsole keymap and X layout (will fail
            # safely if it doesn't work)

in some cases, when the "# activate VConsole keymap and get converted layout and variant" block is hit.

Before the commit, that block would result in `c_lay_var` being "", which is false-y, so later on, the "nothing suggested by systemd-localed..." block would trigger.

After the commit, the "activate VConsole keymap and get converted layout" block would result in `c_lays_vars` being [""] - a single-element list containing an empty string. This is *truth-y*, it is not false-y. So on this path, the "nothing suggested by systemd-localed..." block would *never trigger*.

I can't say for sure that's the cause of this bug, though, because the block in question is actually about doing the opposite (getting an X layout). But the logic paths in here are very weird and snake-y so it *is* possible it causes the bug somehow. Tomorrow I'll throw a bunch more debug statements in there and see if I can get to the bottom of things, if no-one else beats me to it.

Hmm, well, there goes my smoking gun...Beta TC3 still works with ed4d1d1cd46d4b3ea1db7bf32ff91f26e60b8661 applied via an updates.img! So it must be something else. I'll investigate more tomorrow.

Huh - so somehow, systemd *did* change between Beta TC3 and Beta TC4, though I could find no record of it. It went from 215-19 to 216-4.

I've verified that this is what broke it. 216-4 has been garbage collected, but 216-5 is still available. So I did an install of Beta TC3, then I verified that the bug does not happen with the repro steps from the first comment:

>>> from pyanaconda.keyboard import LocaledWrapper
>>> wrap = LocaledWrapper()
>>> wrap.convert_layout('ru')
'ru'

then I updated systemd to 216-5 (and initscripts, which had a conflict with systemd that needed an update to fix) and rebooted, changing nothing else. Now the bug does happen:

>>> from pyanaconda.keyboard import LocaledWrapper
>>> wrap = LocaledWrapper()
>>> wrap.convert_layout('ru')
''

so it broke between systemd-215-19.fc21 and systemd-216-5.fc21 . You can see what `convert_layout` does in anaconda's keyboard.py. Basically what happens is:

1. it stores the result of getting the localed "X11Layout" dbus property
2. it calls "SetX11Keyboard" with args derived from the requested layout and the 'convert' arg set to True
   I believe for the 'layout' "ru" it would set the first arg to "ru", the second to "" (this is hardcoded), the third to "", the fourth to "", the fifth to whatever indicates 'truth' (it's the 'convert' arg), and the sixth to whatever indicates 'false' (this is hardcoded)
3. It gets the "VConsoleKeymap" property at this point and stores it (this is what it will ultimately return)
4. It calls "SetX11Keyboard" again to restore the original configuration that it stored in step 1
5. It returns the value it read in step 3

So it seems like with systemd 215, localed would return 'ru' for the VConsoleKeymap property at step 3, but systemd 216 would return ''. I do not know what change in systemd caused this, yet.

So systemd-216-4 is a bit of a misnomer: looking at the spec file shows it was 577 commits ahead of 216, on https://cgit.freedesktop.org/systemd/systemd-stable/log/?h=v216-stable . Specifically, it was at this point:

https://cgit.freedesktop.org/systemd/systemd-stable/commit/?h=v216-stable&id=0fff82e5f867f9494ed631736964d9abfe672673

Similarly, 215-19 was way past 'v215' on the 'v215-stable' branch. Trying to pin down the actual delta between 215-19 and 216-4 is a bit tricky (I think? maybe someone with better git skillz knows a magic way), but I've got a hunch that the problem is in one of the post-v216 commits. One obvious suspect is:

https://cgit.freedesktop.org/systemd/systemd-stable/commit?h=v216-stable&id=78bd12a05a9252cf573da28394b23e2b891cbba8

so I'm gonna start bisecting through the giant patch set in systemd-216-4 and try to nail down the bug to a single commit.

OK, so after an afternoon of SUPER EXCITING bisecting, my diagnosis is that this is the offending commit:

https://cgit.freedesktop.org/systemd/systemd-stable/commit/src/locale?h=v216-stable&id=81fd105a5f9762fa2f2e42bc949876e32b3a126f - "localed: introduce helper function to simplify matching"

if I build with all patches up to that one but leave it out, the test passes (I get 'ru'). If I then add that patch to the build, the test fails (I get '').

I'm not gonna hazard a guess as to what's wrong with that, my C isn't good enough, but that's the problem, it seems.

http://koji.fedoraproject.org/koji/taskinfo?taskID=14252351 was supposed to be a scratch build for f24 with the changes from that commit hand reverted, so I can verify that that's enough to 'fix' the bug in f24, but it failed with something that looks weirdly like a bug in the kernel headers:

In file included from /usr/include/linux/if.h:31:0,
                 from src/shared/firewall-util.c:28:
/usr/include/linux/hdlc/ioctl.h:73:14: error: 'IFNAMSIZ' undeclared here (not in a function)
  char master[IFNAMSIZ]; /* Name of master FRAD device */

I'll try and get around that later or tomorrow, gotta run out for now.

OK, fixed the build problem thanks to jwb, and confirmed. http://koji.fedoraproject.org/koji/taskinfo?taskID=14263836 is a scratch build of current F24 systemd but with the changes from 81fd105a5f9762fa2f2e42bc949876e32b3a126f reverted. With that, the conversion works:

>>> from pyanaconda.keyboard import LocaledWrapper
>>> wrap = LocaledWrapper()
>>> wrap.convert_layout('ru')
'ru'

so the bug is in that startswith_comma change somehow.

My C may suck, but I *think* that commit somewhat changed the logic of the code it affected.

AIUI, throughout the code, c->x11_layout is the currently-set X11 layout (c is the context passed to the find_legacy_keymap function) and a[1] is the x11 value from the entry in SYSTEMD_KBD_MODEL_MAP we're currently comparing it against to try and find the best match.

The logic *before* the commit, AFAICS, was this:

1) if the two strings match perfectly, assign a score of 10 and move on
2) if not, take the currently-set layout only up to (not including) the first comma and see if that matches the MODEL_MAP entry perfectly; if so assign score 5 and move on
3) if not, take the MODEL_MAP entry only up to (not including) the first comma and see if that matches the currently-set layout only up to (not including) the first comma; if so assign score 1 and move on

*After* the commit, AFAICS, the logic is this:

1) if the two strings match perfectly, assign a score of 10 and move on
2) if not, take the currently-set layout and see if it starts with the MODEL_MAP entry and the next character is a comma; if so, assign score 5 and move on
3) if not, take the MODEL_MAP entry up to the first comma and see if the currently-set layout starts with that text and the next character is a comma

Both 2) and 3) in the new setup do not quite match the previous behaviour. For instance, take these cases:

1. Current layout 'ru,us,jp' : MODEL_MAP 'ru,us' - this would get a 5 with the new code, but a 1 with the old code

This one isn't very harmful I don't think and is probably 'correct'. But consider our case!

2. Current layout 'ru', MODEL_MAP 'ru,us'

This gets a 0 with the new code, but it got a 1 with the old code. This is because the behaviour of 3) changed.

With the old step 3, we take the MODEL_MAP value up to the first comma (so 'ru') and compare it against the current layout up to the first comma (also 'ru', since there's no comma); that's a match, so we score a 1.

With the new step 3, we take the MODEL_MAP value up to the first comma (so 'ru') and see if the currently-set layout starts with that string (yep, it does) *and if the next character in the currently-set layout is a comma* (nope, it isn't, so no pass, we don't score a 1).

Hmm, a thought strikes me. Part of the step 3 conditional in the old code is slightly odd:

                                if (x > 0 && x == w &&
                                    memcmp(c->x11_layout, a[1], x) == 0)

the odd part is the 'x > 0' bit. In the old code, 'x' is the result of 'strcspn(c->x11_layout, ",")'. I'm not sure if the 'x > 0' there is only supposed to be a check on whether we have a c->x11_layout at all, or if the original code misunderstood how strcspn works.

Perhaps it thought strcspn would return 0 or null or something else false-y if the second string was not in the first string at all? In that case, the intention of the 'x > 0' condition would be to only pass if the currently set layout actually contained a ',' meaning 2) and 3) would only ever pass if the currently-set layout had a comma in it - in which case the new 3) and the old 3) would indeed be functionally identical.

However, this is not how strcspn works. strcspn only returns 0 in two cases: if the first string is empty, or if the first occurrence of a character from the second string is at position 0. So 'strcspn("", ",")' returns 0, and so does 'strcspn(",", ",")'. But if the second string is not present in the first at all, strcspn simply returns the length of the first string - so strcspn('ru', ',') returns 2, not 0. I tested this, and it's true.

So I think my summary is that the new form of the code actually implements what the old code *thought* it was doing (and what the comments indicate it's supposed to do), but that's not what the old code *actually* did, and the difference happens to break this case. Arguably the old behaviour, while unintentional, was actually *better*, because a current layout of 'ru' probably should be counted as a weak match for an entry 'ru,us'.

I could of course be wrong, but that's my reading =)

Note that the new 2) and the old 2) really aren't very similar at all, and the new one doesn't really match what the comment says. I'd say that's a bug.

https://github.com/rhinstaller/anaconda/pull/648 should fix this. At least, I tested and it *does* fix exactly this case: if you do a Russian install with an updates.img with that change, you get KEYMAP="ru" in the installed system's /etc/vconsole.conf. Based on my reading of the localed code I *believe* this is a safe change as it will never fail to find a keymap where the old code would have succeeded, but I think it'd be good for others to double-check that.

I still kinda think it would make sense for systemd to change this a bit, though. I don't think the current logic makes a lot of sense. It doesn't make sense to me that a layout of 'ru' will not match a table entry for 'ru,us' but a layout of 'ru,foo,bar' *will* match a table entry for 'ru,us', which - if I read the current code right - is currently the case. I'm working on a little PoC which just isolates the relevant localed code and lets you easily test it from the command line.

Created attachment 1162291 [details]
test script for checking scores with old and new logic

Attaching my tester. It isolates both the 'old' and 'new' matching logic and lets you run it against arbitrary values. You pass it two values: the first is the layout string you want to test the score for, the second is the value you want to compare it against (so it should be one of the X11 layout strings from KBD_MODEL_MAP). For e.g., to demonstrate the bug here:

[adamw@adam tmp]$ ./legacy_test ru ru,us
Old score: 1
New score: 0

shows that with the 'old' logic, given a layout value of 'ru', localed would score the KBD_MODEL_MAP entry with X11 layout value 'ru,us' as a 1 (weak match) with the old logic, but scores it a 0 (no match) with the new logic. To demonstrate the difference in the 5 score code:

[adamw@adam tmp]$ ./legacy_test ru,us,foo ru,us
Old score: 1
New score: 5

Thanks a lot for looking into this. I'm started working on some fixes in systemd (and also on adding some tests so that we don't regress again), but unfortunately I run out of time today.

For the record, I like the new 2) but the old 3) better.

systemd-229-8.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-5f8a34340d

systemd-229-8.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-5f8a34340d

The updated systemd does indeed appear to fix this:

>>> from pyanaconda.keyboard import LocaledWrapper
>>> wrap = LocaledWrapper()
>>> wrap.convert_layout('ru')
'ru'

with current anaconda f24-branch and systemd 229-8. From the anaconda end, my PR to send all X11 layouts to localed when converting was merged to master, and that setup seems to work with the fixed systemd as well:

>>> from pyanaconda.keyboard import LocaledWrapper
>>> wrap = LocaledWrapper()
>>> wrap.convert_layouts(['ru'])
'ru'
>>> wrap.convert_layouts(['ru', 'us'])
'ru'

so I'd say we're looking shiny for this case, let's hope nothing else broke. :)

systemd-229-8.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report.