[PATCH] Make sure TUI is readable for non-latin languages (#1182562)
Martin Kolman
mkolman at redhat.com
Wed Jul 22 11:51:59 UTC 2015
On Mon, 2015-07-20 at 09:28 +0200, Vratislav Podzimek wrote:
> 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.
OK. :)
>
> > + :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.
Yep, you are of course right - dropping 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)
Fixing.
>
> > + 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.
OK!
>
> > +
> > + 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.
Yeah, even though the issues can be quite serious (you have requested
a language, but we have found out it can't be displayed), it is less of
na issue due to the fallback to English. So I'll demote those errors to
warnings. :)
>
> > + 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?
Sure, that should also make it easier to easier to comprehend that:
1) locale/env gets set only once per call
2) what gets actually set
>
> >
> > 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:...'
Fixing.
>
> > + :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.
Also fixing.
>
> > + :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.
I opt for debug calls, so that it can be more easily correlated with
the other console font related messages in the main anaconda log.
>
> > + 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]
>
More information about the anaconda-patches
mailing list