Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only set up default locale on FixMyStreet install. #24

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

dracos
Copy link
Member

@dracos dracos commented Jul 16, 2014

FixMyStreet uses the default locale in order to have working URIs with
floating point numbers. This could be moved to the FixMyStreet install
script with its next release and then removed from here.

FixMyStreet uses the default locale in order to have working URIs with
floating point numbers. This could be moved to the FixMyStreet install
script with its next release and then removed from here.
generate_locales
set_locale
if [ $SITE = "fixmystreet" ]; then
generate_locales
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably want to make the locales available to all systems so that the SSH session can pick the one it requires.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All generate_locales does is install language-pack-en (if present, so Ubuntu) and add an en_GB locale. I don't think it does anything that prevents making locales available to any system.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding (which could well be wrong!) is that you need to make the locales available through dpkg-reconfigure locales. Basically this step:

screen shot 2014-07-18 at 10 49 49

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Installation of the locales package runs locale-gen postinst, so any locale chosen at server set up is already available. I'm not sure it feels right for an install script to install every possible locale, just in case? Vagrant's puppetlabs/debian-7.4-64-nocm, puppetlabs/debian-6.0.9-64-nocm, and ubuntu/precise64 all have a default of en_US.UTF-8, for what it's worth. If the default is None, then yes, you need to make sure the locale passed by SSH is installed on the server in order for it to work. Perhaps this script should instead do the following:

  • Check what locale has come over SSH - if it's UTF-8, make sure that it is installed and that the server has no default, and then continue;
  • Otherwise, check if the server has a default, and that it's UTF-8, and if so continue;
  • Otherwise, set up en_GB.UTF-8 as now so that installation will proceed smoothly (e.g. in mapit's case, install_postgis requires a UTF-8 locale).
    Does that work?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds ideal.

Rather than always installing en_GB.UTF-8 and setting it as the default,
we instead do the following:
* If the current LANG (perhaps passed in via SSH) is UTF-8, make sure it
  is generated, and remove any default locale setup if different;
* If it isn't, but the default locale is UTF-8, use that;
* If not, set up en_GB.UTF-8 as before.
@dracos
Copy link
Member Author

dracos commented Jul 18, 2014

Have added commit to implement different locale setup. Would be good to have a number of eyes checking this @garethrees @mhl :)

@garethrees
Copy link
Member

Running against the puppetlabs/squeeze VM I get:

/vagrant/install.sh: line 53: warning: setlocale: LC_ALL: cannot change locale (en_GB.UTF-8)

Here's some more info: http://pastie.org/private/9ujduo5nfmqckjjoobsfxg

My terminal sends the locale I want

debug1: Sending env LANG = en_GB.UTF-8

and the script seems to generate it

Generating locale en_GB... done

but then we get the warning and the locale remains as en_US

/vagrant/install.sh: line 53: warning: setlocale: LC_ALL: cannot change locale (en_GB.UTF-8)
done
vagrant@debian-607-x64-vbox4210-nocm:~$ locale
LANG=en_US

After logging out and starting a new SSH session:

Linux debian-607-x64-vbox4210-nocm 2.6.32-5-amd64 #1 SMP Fri Feb 15 15:39:52 UTC 2013 x86_64

The programs included with the Debian GNU/Linux system are free software;
the exact distribution terms for each program are described in the
individual files in /usr/share/doc/*/copyright.

Debian GNU/Linux comes with ABSOLUTELY NO WARRANTY, to the extent
permitted by applicable law.
Last login: Mon Jul 21 11:16:39 2014 from 192.168.33.1
-bash: warning: setlocale: LC_ALL: cannot change locale (en_GB.UTF-8)
vagrant@debian-607-x64-vbox4210-nocm:~$ locale
locale: Cannot set LC_CTYPE to default locale: No such file or directory
locale: Cannot set LC_MESSAGES to default locale: No such file or directory
locale: Cannot set LC_ALL to default locale: No such file or directory
LANG=en_GB.UTF-8
LANGUAGE=
LC_CTYPE="en_GB.UTF-8"
LC_NUMERIC="en_GB.UTF-8"
LC_TIME="en_GB.UTF-8"
LC_COLLATE="en_GB.UTF-8"
LC_MONETARY="en_GB.UTF-8"
LC_MESSAGES="en_GB.UTF-8"
LC_PAPER="en_GB.UTF-8"
LC_NAME="en_GB.UTF-8"
LC_ADDRESS="en_GB.UTF-8"
LC_TELEPHONE="en_GB.UTF-8"
LC_MEASUREMENT="en_GB.UTF-8"
LC_IDENTIFICATION="en_GB.UTF-8"
LC_ALL=en_GB.UTF-8

@dracos
Copy link
Member Author

dracos commented Jul 21, 2014

@garethrees You didn't set DISTRIBUTION in your miniscript, so en_GB.UTF-8 wasn't generated; I think that's the issue with that output anyway. I've pushed a sh/bash difference fix that I think is unrelated but I came across whilst looking. I think e.g. https://vagrantcloud.com/puppetlabs/debian-6.0.9-64-nocm is more up to date than ones at puppetlabs.com (and has en_US.UTF-8 instead of en_US as the default, interestingly).

@dracos
Copy link
Member Author

dracos commented Jul 21, 2014

Separate issue - if LANG is an okay UTF-8, but not installed on the server, my first if will fail because you can't yet set LC_ALL in order to call locale and check its charmap. Also call to add_locale expects just first half of locale line, without UTF-8, which this doesn't do, and not sure its [ is correct. I guess it could just look for utf-8 in $LANG and strip/assume that's enough, seems likely that it would be?

if [ x"$(LC_ALL=$LANG locale -k LC_CTYPE | grep -c 'charmap="UTF-8"')" = x1 ]; then
# Current is UTF-8, make sure it's generated and use that
add_locale $LANG
if [ $default != $lang && -n "$default" ]; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't seen lowercase $lang defined anywhere, so not sure where that's coming from.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a typo, ta; that whole section needs work as per #24 (comment)

@garethrees
Copy link
Member

You didn't set DISTRIBUTION in your miniscript

Oops. Updated http://pastie.org/private/dnl3fu91emrgxdwl9dsq (including 975e309).

add_locale seems to be writing literal \n in to /etc/locale.gen:

$ tail -n 4 /etc/locale.gen
# zu_ZA ISO-8859-1
# zu_ZA.UTF-8 UTF-8
en_US ISO-8859-1
\nen_GB.UTF-8 UTF-8

@dracos
Copy link
Member Author

dracos commented Jul 23, 2014

Yes, that will happen if you run the install script with bash rather than sh as the instructions/Vagrantfile do, due to the differing behaviours of echo. That line is the same as it always has been, I guess it could use printf.

@garethrees
Copy link
Member

Ah, yeah, running through sh works. Sorry about that.

I notice it still needs a logout to pick up the new locale, but interestingly appending apt-get -y install postgres to the end of the script seems to install postgres with the correct encoding settings:

$ sudo su
# sudo -u postgres psql -c '\list'
                                  List of databases
   Name    |  Owner   | Encoding |  Collation  |    Ctype    |   Access privileges
-----------+----------+----------+-------------+-------------+-----------------------
 postgres  | postgres | UTF8     | en_GB.UTF-8 | en_GB.UTF-8 |
 template0 | postgres | UTF8     | en_GB.UTF-8 | en_GB.UTF-8 | =c/postgres
                                                             : postgres=CTc/postgres
 template1 | postgres | UTF8     | en_GB.UTF-8 | en_GB.UTF-8 | =c/postgres
                                                             : postgres=CTc/postgres
(3 rows)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants