sets $LANG to a language name

[Colin Watson]

As of the last oneiric upgrade I performed (I'm not certain, but I think this was from lightdm 0.9.7-0ubuntu2 to 1.0.0-0ubuntu3), my locale has been broken; it is now set to en_GB rather than to en_GB.UTF-8. I've attached lightdm.log, of which the salient lines seem to be:

  [+8.46s] DEBUG: Launching process 2603: /usr/sbin/lightdm-session 'gnome-session --session=ubuntu-2d'
  [+8.46s] DEBUG: pam_setcred(0x930a6c8, PAM_ESTABLISH_CRED) -> 0 (Success)
  [+8.46s] DEBUG: PAM returns environment 'GNOME_KEYRING_CONTROL=/tmp/keyring-JAW17p GNOME_KEYRING_PID=2592 PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games LANG=en_GB.UTF-8 LANGUAGE=en_GB.UTF-8:en'
  [+8.46s] DEBUG: Registering session with bus path /org/freedesktop/DisplayManager/Session0
  [+8.47s] DEBUG: Using locale en_GB for language en_GB

Please fix this urgently; it's bad to have regressed to a legacy encoding. We should be using UTF-8 for everything unless explicitly configured otherwise, and we definitely shouldn't be ignoring explicit configuration of UTF-8.

[Colin Watson (cjwatson)]

The code seems to be matching against the output of 'locale -a'. This is wrong because it fails to handle locale aliases; it's perfectly legitimate to have either en_GB.utf8 or en_GB.UTF-8 configured, despite what some people think.

[Colin Watson (cjwatson)]

My .dmrc reads:

[Desktop]
Session=ubuntu-2d
Layout=gb
Language=en_GB.UTF-8
Langlist=en_GB:en
LCMess=en_GB.UTF-8

[Gunnar Hjalmarsson (gunnarhj)]

Hi Colin,
Even if I wasn't able to reproduce the behavior you describe exactly, I noticed two things that contribute to incorrect results:
* locale settings happen after /etc/default/locale and ~/.profile have been sourced
* LANG is populated with a language value, which is currently not correct in Ubuntu, since LANG is used for regional formats.

I don't think that ~/.dmrc has anything to do with it; it's rather the Language value in AccountsService's keyfile that is fetched.

Anyway, I made a MP with a quick-and-dirty fix that ought to fix this bug for now.

[Robert Ancell (robert-ancell)]

Colin, yes, your locale should be overridden in ~/.profile. It is set by lightdm only for desktops that do not have a method of setting the language for a user.

[Martin Pitt (pitti)]

The problem is that accountsservice's Language property says "en_GB", which is wrong.

For oneiric, could we please drop all the `locale -a` stuff and asking accountsservice for the locale? accountsservice already writes the selected language into ~/.profile, so simply using that ought to be enough, and has worked fine with gdm as well.

So for applications which DO use accountsservice (language-selector) the locale is already set by ~/.profile. For derivatives without a langugage selector which don't use accountsservice, there is no point asking it either.

Note that for P onwards accountsservice will change ~/.pam_environment instead (bug 866062), but that's merely cleanup and not directly related to this bug.

[Launchpad Janitor (janitor)]

This bug was fixed in the package lightdm - 1.0.1-0ubuntu1

---------------
lightdm (1.0.1-0ubuntu1) oneiric; urgency=low

  * New upstream release:
    - GTK greeter now remembers last user
    - GTK greeter now initializes i18n (LP: #862427)
    - Start authentication for automatically selected user in GTK greeter
    - Link liblightdm-qt against QtGui
    - Fix liblightdm-qt crashing when face images are installed (LP: #850095)
    - Set correct permissions on session log files (LP: #863119)
    - Prefer a locale with a codeset over one without for setting LANG
      (LP: #864618)
    - Introduce a lightdm-guest-session-wrapper session command which MAC
      systems like AppArmor and SELinux can use for attaching a restrictive
      policy to guest sessions.
    - Provide an AppArmor profile for guest session lockdown.
  * debian/patches/01_guest_session_lockdown.patch:
    - Applied upstream
 -- Robert Ancell <email address hidden> Tue, 04 Oct 2011 19:58:25 +1100

[Martin Pitt (pitti)]

Reopening, lightdm 1.0.1 just gets it differently wrong, please see above.

[Gunnar Hjalmarsson (gunnarhj)]

On 2011-10-04 12:03, Martin Pitt wrote:
> The problem is that accountsservice's Language property says "en_GB",
> which is wrong.

Yes and no. Yes, it's not in accordance with the definition of the property. But personally I'd see advantages with making it right by changing the definition. But that depends on where all this land in the P cycle.

Btw, it's fetched from my Perl script, so how could it possibly be wrong? ;-)

[Gunnar Hjalmarsson (gunnarhj)]

One effect of Colin's British latin1 locale (discussed in #ubuntu-devel) is that the language chooser in the User Accounts UI states [ISO-8859-1] next to the en_GB option. If the option is selected, LC_MESSAGES is still assigned 'en_GB.UTF-8' as it should be, so the UI is misleading. I made a g-c-c MP that fixes this issue.

[Martin Pitt (pitti)]

Oh, hang on, I think I thought about this all wrong. "en_GB" *is* a correct value, but not for $LANG, but for $LANGUAGE; What you select in control-center is a language, _not_ a locale. So lightdm should not assing the "Language" accountsservice property to $LANG, which is what is causing all this trouble, and will be even more wrong for languages without a country specific dialect, i. e. all languages except English, Spanish, and Portugese.

So lightdm should either drop that set_language() call completely (I'm still convinced that this is the most correct and simplest solution), or change that to assing the "Language" property to $LANGUAGE, not $LANG.

[Martin Pitt (pitti)]

Robert and I just had a long discussion and created http://pad.ubuntu.com/locale-handling to explain the concepts and design.

Unfortunately it is not trivial to only disable the "set $LANG from accountsservice" part, but keeping the "set $LANG from .dmrc" part (which we need for backwards compat and derivatives). But anyway, the current set_language() method is just wrong, and we'll disable it for oneiric final to fix this bug. We will then need to SRU a slightly more intrusive patch to revive reading ~/.dmrc for backwards compatibility.

[Martin Pitt (pitti)]

In other words, I'll use Gunnar's original merge proposal.

[Martin Pitt (pitti)]

I filed bug 868149 about the "does not evaluate .dmrc" part, which now gets opened due to that quickfix.

[Launchpad Janitor (janitor)]

This bug was fixed in the package lightdm - 1.0.1-0ubuntu2

---------------
lightdm (1.0.1-0ubuntu2) oneiric; urgency=low

  * debian/patches/04_language_not_to_LANG.patch:
    Locale names based on AccountsService's "Language" key may not
    go to $LANG, as that property is a language name, not a locale.
    (LP: #864618).
 -- Gunnar Hjalmarsson <email address hidden> Wed, 05 Oct 2011 09:49:15 +0200

[Gunnar Hjalmarsson (gunnarhj)]

On 2011-10-05 08:39, Martin Pitt wrote:
> "en_GB" *is* a correct value, but not for $LANG, but for $LANGUAGE;
> What you select in control-center is a language, _not_ a locale.

Precisely.

I like http://pad.ubuntu.com/locale-handling, and will add some stuff to
it later on. Language Support Help (ghelp:language-selector) includes a
similar but less technical description of the concept.

[Gunnar Hjalmarsson (gunnarhj)]

Hi Rodrigo,

As you asked me on #ubuntu-desktop, I built g-c-c without the Ubuntu specific patch 52_ubuntu_language_list_mods.patch, in order to check out if the whole patch or part of it could be dropped in Oneiric.

I chose to respond through this long (hopefully not lengthy) bug comment. With this I'd like to say that we ought to talk more to each other on the locale/language topic; more about that in the end of this comment.

This is how the patch was motivated in the changelog:

  * debian/patches/52_ubuntu_language_list_mods.patch:
    - Change the list of options, when setting language from User
      Accounts, to items representing available translations, and with
      that make it similar to the language list in language-selector.
    - Make items representing language @variants be displayed as such.

To me it looks like nothing significant has been changed upstream that makes the patch less motivated than when it was uploaded.

With the build ex patch 52 installed, the language chooser UI consists of two lists. For me, the first of them does not show all installed translations, but it includes languages that I have not installed. It also shows two "English" items without disclosing which country specific translations they represent.

When I click "Other..." I see a list of all UTF-8 locales on my box, most of which are irrelevant considering that the number of translations are just a fraction of the number of locales. Also, in the case of @variants no info is displayed that lets you know which of the items are @variants. These are issues that g-c-c seems to have inherited from gdm, but which were resolved in the Natty gdm package as regards Ubuntu.

With patch 52 included, there is one list only, and its items represent the translations available on my computer, no more and no less. Each item has a label that unambigously distinguishes it from the other items, and you see if it represents a @variant.

So, I really can't see what good it would do to drop the patch in Oneiric and expose the users to all those bugs.

In both language-selector and the Ubuntu variant of the User Accounts UI the actual values that are dealt with in the language lists are language values such as 'es_ES' or 'de', while upstream g-c-c deals with locale names.

Our discussion started with https://code.launchpad.net/~gunnarhj/gnome-control-center/non-utf8-confusion/+merge/78140, where I propose that the call in gdm-languages.c for the language_name_get_codeset_details() function is commented. That change would hopefully make Colin, with his latin1 locale installed, happy, at least. :)

If you like you can blame the issue with installed non-utf8 locales on patch 52, since language_name_get_codeset_details() does not change anything if you pass a valid locale name to it. But that way of looking at it isn't fair, since the code that messes with codesets shouldn't be there in the first place, considering that we only deal with UTF-8 locales via the UI. In any case, dropping patch 52 the day before the Oneiric release candidate, as a replacement for disabling that irrelevant codeset code, can't reasonably be justified.

To make the intentions in https://lists.ubun...

[Martin Pitt (pitti)]

As far as I understand, the control-center change is not a release blocker, but more like cleanup, so it can be fixed in Precise.

[Rodrigo Moya (rodrigo-moya)]

The fix for g-c-c was in the bzr branch, so since I am updating g-c-c to 3.2.1, I'll include it, so this needs to be SRU'ed right?

[Martin Pitt (pitti)]

+- language_name_get_codeset_details (name, &langinfo_codeset, &is_utf8);
++// language_name_get_codeset_details (name, &langinfo_codeset, &is_utf8);

This change wasn't meant to be SRUed, it was just queued in the development bzr for precise. What is the impact for stable users, how does it change behaviour, what is the test case? Did you verify that this doesn't leave the two variables uninitialized?

[Rodrigo Moya (rodrigo-moya)]

Martin, my plan was to fix the issues that that patch tries to solve in g-c-c 3.2.1 and then completely remove this patch from the package. I could just fix a couple, since the others seemed a bit risky for a SRU, so the patch from Gunnar fixes this bug really for Oneiric users. There shouldn't be any impact for stable users, since we are getting the list of languages from another place anyway, so commenting out that call doesn't really affect much. My plan is to completely remove this Ubuntu-specific patch for Precise and, if possible, in a late SRU for Oneiric.

So, this should be safe, has been tested for a couple of weeks now. The 2 variables are initialized at the beginning of the function, so no problem with that.

[Martin Pitt (pitti)]

Hello Colin, or anyone else affected,

Accepted gnome-control-center into oneiric-proposed, the package will build now and be available in a few hours. Please test and give feedback here. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

[Launchpad Janitor (janitor)]

This bug was fixed in the package gnome-control-center - 1:3.2.1-0ubuntu1

---------------
gnome-control-center (1:3.2.1-0ubuntu1) oneiric-proposed; urgency=low

  [ Gunnar Hjalmarsson ]
  * debian/patches/52_ubuntu_language_list_mods.patch:
    Don't mess with non-utf8 locales, or else the language chooser in
    the User Accounts UI may display confusing info (LP: #864618).

  [ Rodrigo Moya ]
  * New upstream release
  * debian/patches/00git_show_search_view.patch:
  * debian/patches/00git_show_section_labels.patch:
  * debian/patches/00git_use_correct_sleep_timeout_when_disabled.patch:
  * debian/patches/00git_use_gtk_widget_show.patch:
    - Remove upstreamed patches
  * debian/patches/08_dont_crash_on_null_territories.patch:
    - Don't crash on locales with no territory code (LP: #841280)
  * debian/patches/12_add_never_turn_screen_off.patch:
    - Remove translations from patch, as they are now imported to LP
  * debian/patches/52_ubuntu_language_list_mods.patch:
    - Rebased patch
 -- Rodrigo Moya <email address hidden> Tue, 18 Oct 2011 11:39:14 +0200

[Gunnar Hjalmarsson (gunnarhj)]

Hi, I can confirm that the misleading '[ISO-8859-1]' in the User Accounts UI does not show up after upgrading to the oneiric-proposed version.

[Launchpad Janitor (janitor)]

This bug was fixed in the package gnome-control-center - 1:3.2.1-0ubuntu1

---------------
gnome-control-center (1:3.2.1-0ubuntu1) oneiric-proposed; urgency=low

  [ Gunnar Hjalmarsson ]
  * debian/patches/52_ubuntu_language_list_mods.patch:
    Don't mess with non-utf8 locales, or else the language chooser in
    the User Accounts UI may display confusing info (LP: #864618).

  [ Rodrigo Moya ]
  * New upstream release
  * debian/patches/00git_show_search_view.patch:
  * debian/patches/00git_show_section_labels.patch:
  * debian/patches/00git_use_correct_sleep_timeout_when_disabled.patch:
  * debian/patches/00git_use_gtk_widget_show.patch:
    - Remove upstreamed patches
  * debian/patches/08_dont_crash_on_null_territories.patch:
    - Don't crash on locales with no territory code (LP: #841280)
  * debian/patches/12_add_never_turn_screen_off.patch:
    - Remove translations from patch, as they are now imported to LP
  * debian/patches/52_ubuntu_language_list_mods.patch:
    - Rebased patch
 -- Rodrigo Moya <email address hidden> Tue, 18 Oct 2011 11:39:14 +0200