← Back to team overview

cloud-init-dev team mailing list archive

Re: [Merge] ~raharper/cloud-init:default-lang-c-utf8 into cloud-init:master

 

Looks good, just some inline nits and questions

Diff comments:

> diff --git a/cloudinit/distros/debian.py b/cloudinit/distros/debian.py
> index abfb81f..b2904e8 100644
> --- a/cloudinit/distros/debian.py
> +++ b/cloudinit/distros/debian.py
> @@ -246,9 +250,17 @@ def apply_locale(locale, sys_path=LOCALE_CONF_FN, keyname='LANG'):
>                  keyname, sys_val, locale)
>              return
>  
> -    util.subp(['locale-gen', locale], capture=False)
> +    # Update system configuration (not found, or not the same)
>      util.subp(
>          ['update-locale', '--locale-file=' + sys_path,
>           '%s=%s' % (keyname, locale)], capture=False)
>  
> +    # special case for C.UTF-8, it does not need to be regenerated
> +    if locale.lower() == 'c.utf-8':

How does this not "break" if c.utf-8 is changes in the future in debian/ubuntu images? Is there nothing on the system we can query to determine that utf-8 is present?
minimally, I'd like to see us log this behavior so we can see we are "skip generating C.UTF-8" as we suspect it is already present. Maybe we could check for whether the directory /usr/lib/locale/C.UTF-8/  exists?

> +        return
> +
> +    # finally, trigger regeneration
> +    util.subp(['locale-gen', locale], capture=False)
> +
> +
>  # vi: ts=4 expandtab
> diff --git a/tests/unittests/test_handler/test_handler_locale.py b/tests/unittests/test_handler/test_handler_locale.py
> index e9a810c..3c860c5 100644
> --- a/tests/unittests/test_handler/test_handler_locale.py
> +++ b/tests/unittests/test_handler/test_handler_locale.py
> @@ -27,6 +29,9 @@ LOG = logging.getLogger(__name__)
>  
>  
>  class TestLocale(t_help.FilesystemMockingTestCase):
> +
> +    with_logs = True
> +
>      def setUp(self):
>          super(TestLocale, self).setUp()
>          self.new_root = tempfile.mkdtemp()

Not related to this branch, but can we move to self.tmp_dir() instead of tempfile.mkdtemp and drop the cleanUp.



-- 
https://code.launchpad.net/~raharper/cloud-init/+git/cloud-init/+merge/329152
Your team cloud-init commiters is requested to review the proposed merge of ~raharper/cloud-init:default-lang-c-utf8 into cloud-init:master.


References