Bug 16872

Created attachment 17928 [details] [review]
Patch to fix the issue

Hello,

I think Igor's patch is incomplete as it doesn't add aliases for std_comma and comma everywhere.

This bug is probably a regression caused by the patch in bug 12377.

Regards,
Andrei

(In reply to comment #2)
> This bug is probably a regression caused by the patch in bug 12377.

I think you meant bug 13277.

Aaa, yes. Sorry, that was a typo.

Regards,
Andrei

Hello,

During the investigation of this issue I could not find
clear answers that would cover these questions:

- why add new aliases in 10 places for completeness?

and depending on the answer of the previous question:

- is it ok to add 'aliases' in symbol/xx file?

so instead of adding this several times in rules/base

 * ro(std_foo) = pc+ro(std)

this could be added in symbol/ro file:

partial alphanumeric_keys
xkb_symbols "std_foo" {
    include "ro(std)"
    name[Group1]="Romania - Standard (Foo)";
};

Thanks,
Igor Stirbu

> - why add new aliases in 10 places for completeness?
In order not to break existing configurations.

> and depending on the answer of the previous question:
> 
> - is it ok to add 'aliases' in symbol/xx file?
No.

Actually, the patch is not correct - it is modifying the resulting rules/base file instead of modifying the components it is built from.

I will gladly provide a better one, I just didn't know what the components are.

Regards,
Andrei

Created attachment 17954 [details] [review]
First version of parts patch

This patch modifies rules/compat/*.part files where
the romanian layout is specified. The resulted base
file seem to be correct but I am not able to load
any variant using alias. For example,

setxkbmap -layout ro -variant comma

works but this one fails:

$ setxkbmap -v -layout us,ro,ru -variant ,comma,phonetic
Warning! Multiple definitions of keyboard layout
         Using command line, ignoring X server
Warning! Multiple definitions of layout variant
         Using command line, ignoring X server
Trying to build keymap using the following components:
keycodes:   xfree86+aliases(qwerty)
types:      complete
compat:     complete
symbols:    pc+us+ro(basic):2+ro(comma):2+ru(phonetic):3+group(menu_toggle)+ctrl(nocaps)
geometry:   pc(pc104)
Error loading new keyboard description

where comma is an alias to basic variant. Are there any
other files that need to be updated besides parts?

Thanks,
Igor

Hello Igor,

I'm not sure this is supposed to work. According to setxkbmap(1):

-layout name
         Specifies  the  name  of  the layout used to determine the components
         which make up the keyboard description.  Only one layout may be
         specified on the command line.

[...]

-variant name
          Specifies which variant of the keyboard layout should be used to
          determine the components which make  up  the  keyboard description.
          Only one variant may be specified on the command line.

Regards,
Andrei

man page for setxkbmap is out of date.

Hello,

It would be nice to solve this bug for Debian Lenny, which is already in freeze. 

Are there any other files that need to be modified so the "comma" variant will work as an alias to "basic"?

Regards,
Andrei

First, thanks for your patch. I committed it. 
Second, after I really rebuilt everything, I am getting more interesting output for setxkbmap -v -model pc -layout us,ro,ru -variant ,comma,phonetic -print:

keycodes:   xfree86+aliases(qwerty)
types:      complete
compat:     complete
symbols:    pc+us+ro(basic):2+ro(comma):2+ru(phonetic):3
geometry:   pc(pc104)
xkb_keymap {
	xkb_keycodes  { include "xfree86+aliases(qwerty)"	};
	xkb_types     { include "complete"	};
	xkb_compat    { include "complete"	};
	xkb_symbols   { include "pc+us+ro(basic):2+ro(comma):2+ru(phonetic):3"	};
	xkb_geometry  { include "pc(pc104)"	};
};

Rather strange, isn't it? I will try to contact Daniel Stone, since he knows the rules engine internals better than me, he might give some insight...

I rebuilt the Debian package with Igor's patch and I get the same as you (if it helps to know). Please contact us if there is anything else to do.

Regards,
Andrei

Hello,

Another Debian specific issue that I've reported [1]
is that the symbols.dir is not updated when manually
building the xkb-data (xkeyboard-config) package.

After building a package with updated symbols.dir file
the alias variants are still not loading. I thought
this might be helpful.

[1] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=492973

Thanks,
Igor

> symbols.dir is not updated

This is perfectly valid bug report - but could you please report it separately?

Thanks

Hello,

(In reply to comment #15)
> > symbols.dir is not updated
> 
> This is perfectly valid bug report - but could you please report it separately?

Actually this bug is Debian specific and I've
mentioned it here because several people in CC
are Debian developers/contributors. This is not
a xkeyboard-config bug.

Sergey, you've mentioned that the last patch
was commited. I've pulled the latest changes
from the git tree and the patch for the commit
8e253e1bedfaf4beba3763c1e6b9ef4a706487a3 does
not contain all the changes, it updates only
rules/compat/variantRename.lst. Just wondering
because you did not mention about this fact.

Thanks,
Igor

Igor,

> Actually this bug is Debian specific and I've
No it is not:) I checked yesterday - the .dir files are not being regenerated properly.

> rules/compat/variantRename.lst. Just wondering
> because you did not mention about this fact.
If you look at rules/compat/Makefile.am, other files are generated from variantRename.lst.

Sergey,

(In reply to comment #17)
> > rules/compat/variantRename.lst. Just wondering
> > because you did not mention about this fact.
> If you look at rules/compat/Makefile.am, other files are generated from
> variantRename.lst.

How come the generated files from rules/compat are
kept in repository? The only part file that is not
generated is 'base.o_s.part'. This makes things
really confusing, especially with this patch business.

Thanks,
Igor

> How come the generated files from rules/compat are
> kept in repository? 
I'd say "historically". May be you're right, they should be removed. I'll think of it.

> The only part file that is not
> generated is 'base.o_s.part'. This makes things
> really confusing, especially with this patch business.
Well, may be it is confusing a bit. Though for the person who's able to read Makefile.am, it should be relatively easy to get the general idea...

Hello,

If the Debian maintainer were to fix this *and only this* issue, would it be enough to use the patch in 8e253e1bedfaf4beba3763c1e6b9ef4a706487a3 or is it also necessary to fix the .dir issue in bug 17008?

Regards,
Andrei

I'd say 17008 is pretty much irrelevant.

Ok, but what about the issue in comment #12? More interesting, it doesn't appear when using ro as the first layout:

$ setxkbmap -v 10 ro,de comma
Setting verbose level to 10
locale is C
Warning! Multiple definitions of keyboard layout
         Using command line, ignoring X server
Warning! Multiple definitions of layout variant
         Using command line, ignoring X server
Applied rules from xorg:
model:      pc105
layout:     ro,de
variant:    comma
options:    grp:alt_shift_toggle
Trying to build keymap using the following components:
keycodes:   xfree86+aliases(qwertz)
types:      complete
compat:     complete
symbols:    pc+ro+de:2+group(alt_shift_toggle)
geometry:   pc(pc105)

but

$ setxkbmap -v 10 de,ro ,comma
Setting verbose level to 10
locale is C
Warning! Multiple definitions of keyboard layout
         Using command line, ignoring X server
Warning! Multiple definitions of layout variant
         Using command line, ignoring X server
Applied rules from xorg:
model:      pc105
layout:     de,ro
variant:    ,comma
options:    grp:alt_shift_toggle
Trying to build keymap using the following components:
keycodes:   xfree86+aliases(qwertz)
types:      complete
compat:     complete
symbols:    pc+de+ro(basic):2+ro(comma):2+group(alt_shift_toggle)
geometry:   pc(pc105)
Error loading new keyboard description

Is this a different bug?

Regards,
Andrei

The actual issue is a reality, 100% reproducable. I'd say it an issue with setxkbmap (the way it handles rules). That is why I need #16975 fixed first - I do not see why setxkbmap behaves so strangely...

Hello,

I suppose this bug can be safely closed as the main
issue addressed in it was fixed in commit 
8e253e1bedfaf4beba3763c1e6b9ef4a706487a3.

Thanks,
Igor