Comment 64 for bug 553162

Revision history for this message
Gunnar Hjalmarsson (gunnarhj) wrote :

Hi Martin,

Thanks for your comments and questions, both here and on the merge proposal!

On 2010-11-24 10:40, Martin Pitt wrote:
> I looked at your recent gdm patch,

Please note that the gdm patch, i.e. the Xsession code changes, is not part of my suggestion any longer. The reason why I still didn't remove the patch is that I wasn't sure about which route to take. My apologies if that caused some confusion.

As you know, I tried to 'sell' the idea upstream, but wasn't very successful...
https://bugzilla.gnome.org/show_bug.cgi?id=633295

As a result of that, I replaced the previously suggested Xsession changes with two new language-selector files:
/etc/profile.d/gdm-lang-unset.sh
/etc/X11/xinit/xinitrc.d/language-environment.sh

OTOH, since language_update() is now part of language-environment.sh, your questions and comments still apply.

> - in language_update(), why do we need the "en" special case?

That's an attempt to imitate what the LANGUAGE list would have looked like if the change had been made via the language tab in LanguageSelector. If a user picks 'en' in the GDM greeter, the whole LANGUAGE list should consist of nothing but 'en'.

> - in the second part of language_update(), why would $GDM_LANG not
> be a valid locale? this does some rather expensive things like
> locale -a and grep and iterating over all available locales. So far
> we trusted that $GDM_LANG was the format of a locale name, and also
> a valid locale (gdm already tests that); under that assumption,
> couldn't we simply set $LC_MESSAGES to $GDM_LANG?

I think that the reason why there is a need to ensure that LC_MESSAGES is assigned a valid locale is that ~/.dmrc and /var/cache/gdm/$USER/dmrc are now updated when LANGUAGE is set. Hence $GDM_LANG (or $GDM_LANGUAGE) may well be just 'en.utf8', 'sv.utf8' or 'de.utf8', none of which is a valid locale (at least not on my box).

As regards efficiency ("expensive things"), please note that the language_update() function is called only when the user changes the language from the GDM greeter.

> - in the new_language check at the bottom, I think we should
> additionally check that $GDM_LANG != $LANG; for people who already
> have $LANG set to $GDM_LANG (as we do in our installer by default,
> etc.) there is IMHO no reason to change anything in the environment
> or do any expensive operations.

No, no, please don't go back to mixing up LANG and LANGUAGE/LC_MESSAGES. ;-)

Assume a user with these ~/.profile settings:

export LANG="de_DE.utf8"
export LANGUAGE="en_GB:en"
export LC_MESSAGES="en_GB.utf8"

If s/he picks German (Germany) from the GDM greeter, i.e. changes the language from English to German, $GDM_LANG/$GDM_LANGUAGE would equal $LANG. But now, when we complete the separation of LANG from the translation language, that has nothing to do with it any longer, has it?

> - in the final patch I'd like to replace some external program
> calls (expr, grep) with internal shell constructs for efficiency;
> that's just a FYI, though

Ok. I did a few such replacements already, but there may well be further opportunities that I missed.

Please also see my comments on the merge proposal.

I'd like to raise a couple of Kubuntu related questions. The GNOME and KDE versions of language-selector are distributed in separate binary packages, while there is a third common container. The code that sets LC_MESSAGES is included in language-selector-common, but I suppose it can't hurt the KDE users. As regards the code that replaces the previously suggested changes to /etc/gdm/Xsession, it's put in the GNOME binary package to prevent possible conflicts with KDE. My first question is whether there is a need for code tweaking to prevent problems when both GNOME and KDE reside on the same machine. Secondly, OTOH, I wonder if the suggested changes may be useful to KDE users as well, and consequently should be included in the common binary instead of the GNOME binary?

Gunnar