[PATCH] Make sure TUI is readable for non-latin languages (#1182562)

Vratislav Podzimek vpodzime at redhat.com
Mon Jul 20 07:28:16 UTC 2015


On Fri, 2015-07-17 at 17:05 +0200, Martin Kolman wrote:
> Make sure the currently selected TUI language is displayable
> in the Linux console by checking its scripts, as reported by
> langtable.list_scripts()).
> (Currently supported scripts: 'Latn', 'Cyrl', 'Grek')
> 
> If the language is not displayable tehn fall back to English.
> 
> We also need to handle some edge cases such as Anaconda falling back
> to TUI after failed GUI startup and missing or corrupted console fonts.
> 
> Resolves: rhbz#1182562
> Signed-off-by: Martin Kolman <mkolman at redhat.com>
> ---
>  anaconda                            |  13 +++-
>  anaconda.spec.in                    |   2 +-
>  pyanaconda/anaconda.py              |   1 +
>  pyanaconda/localization.py          | 142 ++++++++++++++++++++++++++++++++++--
>  pyanaconda/ui/gui/spokes/welcome.py |   4 +-
>  5 files changed, 151 insertions(+), 11 deletions(-)
Generally looks good to me, but I have a few (inline) comments below. In
particular, I'd "degrade" all log messages.

> 
> diff --git a/anaconda b/anaconda
> index 7b5d386..b868ac3 100755
> --- a/anaconda
> +++ b/anaconda
> @@ -681,6 +681,9 @@ def setupDisplay(anaconda, options, addons=None):
>  
>      anaconda.instClass.configure(anaconda)
>  
> +    # report if starting the GUI failed
> +    anaconda.gui_startup_failed = bool(graphical_failed)
> +
>  def prompt_for_ssh():
>      # Do some work here to get the ip addr / hostname to pass
>      # to the user.
> @@ -1122,7 +1125,7 @@ if __name__ == "__main__":
>      # first, try to load firmware language if nothing is already set in
>      # the environment
>      if "LANG" not in os.environ:
> -        localization.load_firmware_language(ksdata.lang)
> +        localization.load_firmware_language(ksdata.lang, anaconda.displayMode != "g")
>  
>      # If command line options or kickstart set LANG, override the environment
>      if opts.lang:
> @@ -1145,7 +1148,7 @@ if __name__ == "__main__":
>          log.error("Invalid locale '%s' given on command line or in kickstart", os.environ["LANG"])
>          os.environ["LANG"] = constants.DEFAULT_LANG # pylint: disable=environment-modify
>  
> -    localization.setup_locale(os.environ["LANG"], ksdata.lang)
> +    localization.setup_locale(os.environ["LANG"], ksdata.lang, anaconda.displayMode != "g")
>  
>      import blivet
>      blivet.enable_installer_mode()
> @@ -1158,6 +1161,12 @@ if __name__ == "__main__":
>  
>      # now start the interface
>      setupDisplay(anaconda, opts, addon_paths)
> +    if anaconda.gui_startup_failed:
> +        # we need to reinitialize the locale if GUI startup failed,
> +        # as we might now be in text mode, which might not be able to display
> +        # the characters from our current locale
> +        log.warning("reinitializing locale due to failed attempt to start the GUI")
> +        localization.setup_locale(os.environ["LANG"], ksdata.lang, anaconda.displayMode != "g")
>  
>      # we now know in which mode we are going to run so store the information
>      from pykickstart.constants import DISPLAY_MODE_GRAPHICAL, DISPLAY_MODE_CMDLINE, DISPLAY_MODE_TEXT
> diff --git a/anaconda.spec.in b/anaconda.spec.in
> index a698c24..37a46dd 100644
> --- a/anaconda.spec.in
> +++ b/anaconda.spec.in
> @@ -38,7 +38,7 @@ Source0: %{name}-%{version}.tar.bz2
>  %define iscsiver 6.2.0.870-3
>  %define rpmver 4.10.0
>  %define libarchivever 3.0.4
> -%define langtablever 0.0.18-1
> +%define langtablever 0.0.31-3
>  %define libxklavierver 5.4
>  %define libtimezonemapver 0.4.1-2
>  
> diff --git a/pyanaconda/anaconda.py b/pyanaconda/anaconda.py
> index 6baf48a..8e9e777 100644
> --- a/pyanaconda/anaconda.py
> +++ b/pyanaconda/anaconda.py
> @@ -51,6 +51,7 @@ class Anaconda(object):
>          self.desktop = desktop.Desktop()
>          self.dir = None
>          self.displayMode = None
> +        self.gui_startup_failed = False
>          self.id = None
>          self._instClass = None
>          self._intf = None
> diff --git a/pyanaconda/localization.py b/pyanaconda/localization.py
> index 30112f9..5a4659f 100644
> --- a/pyanaconda/localization.py
> +++ b/pyanaconda/localization.py
> @@ -29,13 +29,15 @@ import glob
>  from collections import namedtuple
>  
>  from pyanaconda import constants
> -from pyanaconda.iutil import upcase_first_letter, setenv
> +from pyanaconda.iutil import upcase_first_letter, setenv, execWithRedirect
>  
>  import logging
>  log = logging.getLogger("anaconda")
>  
>  LOCALE_CONF_FILE_PATH = "/etc/locale.conf"
>  
> +SCRIPTS_SUPPORTED_BY_CONSOLE = {'Latn', 'Cyrl', 'Grek'}
> +
>  #e.g. 'SR_RS.UTF-8 at latin'
>  LANGCODE_RE = re.compile(r'(?P<language>[A-Za-z]+)'
>                           r'(_(?P<territory>[A-Za-z]+))?'
> @@ -92,6 +94,24 @@ def is_supported_locale(locale):
>      en_name = get_english_name(locale)
>      return bool(en_name)
>  
> +def locale_supported_in_console(locale):
> +    """
> +    Function that tells if the given locale can be displayed by the Linux console.
> +
> +    The Linux console can display Latin, Cyrillic and Greek characters reliably,
> +    but others such as Japanese, can't be correctly installed.
> +
> +    :param locale: locale to test
> +    :type locale: str
The type could be moved to the :param... line.

> +    :return: whether the given locale is supported by the console or not
> +    :rtype: bool
> +    :raise InvalidLocaleSpec: if an invalid locale is given (see LANGCODE_RE)
> +
> +    """
> +
> +    locale_scripts = get_locale_scripts(locale)
> +    return bool(set(locale_scripts).issubset(SCRIPTS_SUPPORTED_BY_CONSOLE))
I'd expect issubset() to return a boolean value. Is that really not the
case? If it is, please drop the bool() call.

> +
>  def langcode_matches_locale(langcode, locale):
>      """
>      Function that tells if the given langcode matches the given locale. I.e. if
> @@ -181,7 +201,7 @@ def find_best_locale_match(locale, langcodes):
>      else:
>          return None
>  
> -def setup_locale(locale, lang=None):
> +def setup_locale(locale, lang=None, text_mode=False):
>      """
>      Procedure setting the system to use the given locale and store it in to the
>      ksdata.lang object (if given). DOES NOT PERFORM ANY CHECKS OF THE GIVEN
> @@ -190,9 +210,14 @@ def setup_locale(locale, lang=None):
>      $LANG must be set by the caller in order to set the language used by gettext.
>      Doing this in a thread-safe way is up to the caller.
>  
> +    We also try to set a proper console font for the locale in textmode.
^ text mode (two words)

> +    If the font for the locale can't be displayed in the Linux console,
> +    we fall back to the English locale.
> +
>      :param locale: locale to setup
>      :type locale: str
>      :param lang: ksdata.lang object or None
> +    :param bool text_mode: if the locale is being setup for textmode
>      :return: None
>      :rtype: None
>  
> @@ -201,8 +226,55 @@ def setup_locale(locale, lang=None):
>      if lang:
>          lang.lang = locale
>  
> -    setenv("LANG", locale)
> -    locale_mod.setlocale(locale_mod.LC_ALL, locale)
> +    # not all locales might be displayable in text mode
> +    if text_mode:
> +        # check if the script corresponding to the locale/language
> +        # can be displayed by the Linux console
> +        # * all scripts for the given locale/language need to be
> +        #   supported by the linux console
> +        # * otherwise users might get a screen full of white rectangles
> +        #   (also known as "tofu") in text mode
> +        # then we also need to check if we have information about what
> +        # font to use for correctly displaying the given language/locale
> +
> +        script_supported = locale_supported_in_console(locale)
> +        log.info("scripts found for locale %s: %s", locale, get_locale_scripts(locale))
> +
> +        console_fonts = get_locale_console_fonts(locale)
> +        log.info("console fonts found for locale %s: %s", locale, console_fonts)
These should be turned into debug messages, I think.

> +
> +        font_set = False
> +        if script_supported and console_fonts:
> +            # set the locale
> +            setenv("LANG", locale)
> +            locale_mod.setlocale(locale_mod.LC_ALL, locale)
This change may be overridden by the 'if not font_set' branch below. See
the comment at the end of this function for the suggestion. 

> +            # try to set console font
> +            for font in console_fonts:
> +                if set_console_font(font):
> +                    # console font set successfully, skip the rest
> +                    font_set = True
> +                    break
> +
> +        if not font_set:
> +            log.error("can't set console font for locale %s", locale)
> +            # report what exactly went wrong
> +            if not(script_supported):
> +                log.error("script not supported by console for locale %s", locale)
> +            if not(console_fonts):  # no fonts known for locale
> +                log.error("no console font found for locale %s", locale)
> +            if script_supported and console_fonts:
> +                log.error("none of the suggested fonts can be set for locale %s", locale)
> +            log.error("falling back to the English locale")
I think the above are more like warnings than errors.

> +            default_locale = constants.DEFAULT_LANG
> +            os.environ["LANG"] = default_locale
> +            setenv("LANG", default_locale)
> +            locale_mod.setlocale(locale_mod.LC_ALL, default_locale)
> +
> +    # we can use any locale supported by langtable in the GUI
> +    else:
> +        # set the locale
> +        setenv("LANG", locale)
> +        locale_mod.setlocale(locale_mod.LC_ALL, locale)
Generally I'm quite a bit nervous about the amount of setenv() and
setlocale() calls in this function, especially when thinking about the
issues we had with the *env() functions. Could you please set the
'locale' variable accordingly in the code branches and then do the
setenv(), setlocale() and os.environ modifications at the very end in
one place?

>  
>  def get_english_name(locale):
>      """
> @@ -377,6 +449,46 @@ def get_locale_territory(locale):
>  
>      return parts.get("territory", None)
>  
> +def get_locale_console_fonts(locale):
> +    """
> +    Function returning preferred console fonts for the given locale.
> +
> +    :param locale: locale string (see LANGCODE_RE)
> +    :type locale: str
Again, this could be reduced to ':param str locale:...'

> +    :return: list of preferred console fonts
> +    :rtype: list of strings
> +    :raise InvalidLocaleSpec: if an invalid locale is given (see LANGCODE_RE)
> +
> +    """
> +
> +    parts = parse_langcode(locale)
> +    if "language" not in parts:
> +        raise InvalidLocaleSpec("'%s' is not a valid locale" % locale)
> +
> +    return langtable.list_consolefonts(languageId=parts["language"],
> +                                       territoryId=parts.get("territory", ""),
> +                                       scriptId=parts.get("script", ""))
> +
> +def get_locale_scripts(locale):
> +    """
> +    Function returning preferred scripts (writing systems) for the given locale.
> +
> +    :param locale: locale string (see LANGCODE_RE)
> +    :type locale: str
Same here.

> +    :return: list of preferred scripts
> +    :rtype: list of strings
> +    :raise InvalidLocaleSpec: if an invalid locale is given (see LANGCODE_RE)
> +
> +    """
> +
> +    parts = parse_langcode(locale)
> +    if "language" not in parts:
> +        raise InvalidLocaleSpec("'%s' is not a valid locale" % locale)
> +
> +    return langtable.list_scripts(languageId=parts["language"],
> +                                  territoryId=parts.get("territory", ""),
> +                                  scriptId=parts.get("script", ""))
> +
>  def get_xlated_timezone(tz_spec_part):
>      """
>      Function returning translated name of a region, city or complete timezone
> @@ -417,7 +529,7 @@ def write_language_configuration(lang, root):
>          msg = "Cannot write language configuration file: %s" % ioerr.strerror
>          raise LocalizationConfigError(msg)
>  
> -def load_firmware_language(lang):
> +def load_firmware_language(lang, text_mode=False):
>      """
>      Procedure that loads firmware language information (if any). It stores the
>      information in the given ksdata.lang object and sets the $LANG environment
> @@ -474,7 +586,7 @@ def load_firmware_language(lang):
>          return
>  
>      log.debug("Using UEFI PlatformLang '%s' ('%s') as our language.", d, locales[0])
> -    setup_locale(locales[0], lang)
> +    setup_locale(locales[0], lang, text_mode)
>  
>      os.environ["LANG"] = locales[0] # pylint: disable=environment-modify
>  
> @@ -563,3 +675,21 @@ def resolve_date_format(year, month, day, fail_safe=True):
>              # if this call fails too, something is going terribly wrong and we
>              # should be informed about it
>              return order_terms_formats(FAIL_SAFE_DEFAULT)
> +
> +def set_console_font(font):
> +    """
> +    Try to set console font to the given value.
> +
> +    :param str font: console font name
> +    :returns: True on success, False on failure
> +    :rtype: Bool
> +
> +    """
> +    log.info("setting console font to %s", font)
> +    rc = execWithRedirect("setfont", [font])
> +    if rc == 0:
> +        log.info("console font set successfully to %s", font)
These two log.info() calls should either be dropped (we can see the
action in the program.log) or at least changed to log.debug() calls as
these are debug messages.

> +        return True
> +    else:
> +        log.error("setting console font to %s failed", font)
> +        return False
> diff --git a/pyanaconda/ui/gui/spokes/welcome.py b/pyanaconda/ui/gui/spokes/welcome.py
> index 1dbe527..91b2b3a 100644
> --- a/pyanaconda/ui/gui/spokes/welcome.py
> +++ b/pyanaconda/ui/gui/spokes/welcome.py
> @@ -67,7 +67,7 @@ class WelcomeLanguageSpoke(LangLocaleHandler, StandaloneSpoke):
>  
>          locale = store[itr][1]
>          self._set_lang(locale)
> -        localization.setup_locale(locale, self.data.lang)
> +        localization.setup_locale(locale, self.data.lang, text_mode=False)
>  
>          # Skip timezone and keyboard default setting for kickstart installs.
>          # The user may have provided these values via kickstart and if not, we
> @@ -197,7 +197,7 @@ class WelcomeLanguageSpoke(LangLocaleHandler, StandaloneSpoke):
>          # use default
>          if not langs_with_translations:
>              self._set_lang(DEFAULT_LANG)
> -            localization.setup_locale(DEFAULT_LANG, self.data.lang)
> +            localization.setup_locale(DEFAULT_LANG, self.data.lang, text_mode=False)
>              lang_itr, _locale_itr = self._select_locale(self.data.lang.lang)
>              langs_with_translations[DEFAULT_LANG] = lang_itr
>              locales = [DEFAULT_LANG]

-- 
Vratislav Podzimek

Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic



More information about the anaconda-patches mailing list