Skip to content

Damaged format placeholders in the el and fa catalogues abort the build instead of warning #14664

Description

@katerynaKhar

Describe the bug

Three strings in the shipped catalogues have damaged str.format placeholders.
Because sphinx/config.py formats the translated message eagerly —
logger.warning(msg.format(...)) — the result is not a mangled warning but an
aborted build.

check_confval_types is a config-inited handler, so the exception is caught by
EventManager.emit and re-raised as

ExtensionError: Handler <function check_confval_types ...> for event 'config-inited' threw an exception

A user running under LANGUAGE=el or LANGUAGE=fa therefore gets a crash that
blames an extension, in place of the warning that their conf.py holds a value
of the wrong type.

The three strings:

locale msgid at what the translation has why it fails
el config.py:829 '{name]' … '[current__name__}' braces swapped for brackets; does not parse as a format string
el config.py:846 {current__name__}, {default__name__} the dots are gone, so the names do not exist → KeyError
fa config.py:846 {permitted} where {default.__name__} belongs permitted is not passed to this call → KeyError

The fa one looks borrowed from the sibling message at config.py:829, which
does take permitted.

I checked all 71 catalogues — 18,968 translated strings — and these are the only
three where the placeholders do not match their msgid. Everything else is clean.

doc/internals/contributing.rst says pull requests altering translation files
are not accepted, so I have not opened one; these need someone with Transifex
access. The second half, under Additional context, is code rather than
translation, and I would be glad to send a PR for it.

How to Reproduce

I have no Python interpreter on this machine, so this comes from reading the
shipped catalogues and the call sites rather than from a build I ran. The
catalogue half can be confirmed on its own, from a checkout, without building
anything:

import gettext

t = gettext.translation('sphinx', 'sphinx/locale', languages=['el'])
msg = t.gettext(
    "The config value `{name}' has type `{current.__name__}', "
    "defaults to `{default.__name__}'."
)
print(msg.format(name='x', current=int, default=str))

That is the same lookup and the same .format() call as sphinx/config.py:849,
with the same keyword arguments, and it raises.

For the build-level symptom: build any project whose conf.py holds a value of
the wrong type, under LANGUAGE=el or LANGUAGE=fa.

The damaged strings are present in the compiled .mo files, which are tracked in
the repository, so nothing needs recompiling to reproduce this.

Environment Information

Read from a checkout of sphinx-doc/sphinx at `e44a40e` (main), specifically
`sphinx/locale/*/LC_MESSAGES/sphinx.po` and the matching `.mo` files.

No `sphinx-build --bug-report` output: there is no Python interpreter on this
machine and I did not run Sphinx. Everything above comes from the catalogues and
the source, and the snippet under "How to Reproduce" is there so it can be
checked in a second rather than taken on trust.

Sphinx extensions

Additional context

Repairing the three strings is one thing; the exposure behind them is another.

.format() is called on a translated string in six places:

sphinx/config.py:801, 840, 849
sphinx/transforms/i18n.py:156
sphinx/_cli/__init__.py:73, 170

Each is evaluated eagerly, so a damaged placeholder in any locale, in any of
these messages, raises rather than degrades. Messages passed as
logger.warning(__('...%s'), arg) are safer, because the logging module catches
formatting errors itself — the .format() calls have no such backstop.

Two ways to close it, not mutually exclusive:

  1. Fall back to the untranslated message when formatting the translated one
    fails. The user then gets an English warning instead of a dead build.
  2. Check placeholders when the catalogues are compiled. utils/babel_runner.py compile already walks every .po; comparing the format fields of each
    msgstr against its msgid there would have caught all three of these when
    they were pulled from Transifex, and would keep catching the next ones.

Happy to send a PR for either.

Found with i18n-keeper, then
verified by hand against the catalogues and the call sites.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions