cloud-init-dev team mailing list archive

[Steve Langasek <steve.langasek@xxxxxxxxxxxxx>]

Review: Needs Fixing

technical review of the changes, inline:

Diff comments:

> diff --git a/tools/Z99-cloud-locale-test.sh b/tools/Z99-cloud-locale-test.sh
> index 4978d87..050ee76 100644
> --- a/tools/Z99-cloud-locale-test.sh
> +++ b/tools/Z99-cloud-locale-test.sh
> @@ -73,28 +68,20 @@ locale_warn() {
>      done
>      pkgs=${pkgs# }
>  
> -    if [ -n "${pkgs}" ]; then
> -        printf "   sudo apt-get install ${pkgs# }\n"
> -        printf "     or\n"
> -        printf "   sudo locale-gen ${to_gen# }\n"
> -        printf "\n"
> -    fi
> -    for bad in ${invalid}; do
> -        printf "WARNING: '${bad}' is an invalid locale\n"
> -    done
> -
> -    printf "To see all available language packs, run:\n"
> -    printf "   apt-cache search \"^language-pack-[a-z][a-z]$\"\n"
> -    printf "To disable this message for all users, run:\n"
> -    printf "   sudo touch /var/lib/cloud/instance/locale-check.skip\n"
> -    printf "_____________________________________________________________________\n\n"
> -
> -    # only show the message once
> -    : > ~/.cloud-locale-test.skip 2>/dev/null || :
> +    echo "Your provided locale (${bad_kv}) is not available and has been"
> +    echo "changed to C.UTF-8 (LC_ALL=C.UTF-8). To install native locales:"
> +    echo "  sudo apt-get install locales${pkgs:+ ${pkgs}}"
> +    return 1
>  }
>  
> -[ -f ~/.cloud-locale-test.skip -o -f /var/lib/cloud/instance/locale-check.skip ] ||
> -    locale 2>&1 | locale_warn
> +if [ ! -f ~/.cloud-locale-test.skip ] &&
> +   [ ! -f /var/lib/cloud/instance/locale-check.skip ]; then

This means that we are only detecting and fixing up the invalid locale the first time the script is invoked; subsequent logins will have a broken locale again.

> +   if locale 2>&1 | locale_warn; then
> +      LC_ALL=C.UTF-8

This means that if a user has a mix of valid and invalid locale settings, we are clobbering all of them by way of LC_ALL rather than just clobbering the bits that are invalid.  I don't think that's ideal.

> +      # only show the message once
> +      : > ~/.cloud-locale-test.skip 2>/dev/null || :
> +   fi
> +fi
>  
>  unset locale_warn
>  # vi: ts=4 expandtab


-- 
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/348065
Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:fix/1134036-locale-message-change into cloud-init:master.