Re: [PATCH] gettext: handle unsupported languages properly

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Daniel Burgener <dburgener@xxxxxxxxxxxxxxxxxxx> writes:

> On 6/24/2022 1:27 PM, Vit Mojzis wrote:
>> 
>> 
>> On 6/24/22 18:37, Daniel Burgener wrote:
>>> On 6/24/2022 10:24 AM, Vit Mojzis wrote:
>>>> With "fallback=True" gettext.translation behaves the same as
>>>> gettext.install and uses NullTranslations in case the
>>>> translation file for given language was not found (as opposed to
>>>> throwing an exception).
>>>>
>>>> Fixes:
>>>>    # LANG is set to any "unsupported" language, e.g. en_US.UTF-8
>>>>    $ chcat --help
>>>>    Traceback (most recent call last):
>>>>    File "/usr/bin/chcat", line 39, in <module>
>>>>      t = gettext.translation(PROGNAME,
>>>>    File "/usr/lib64/python3.9/gettext.py", line 592, in translation
>>>>      raise FileNotFoundError(ENOENT,
>>>>    FileNotFoundError: [Errno 2] No translation file found for domain: 
>>>> 'selinux-python'
>>>>
>>>> Signed-off-by: Vit Mojzis <vmojzis@xxxxxxxxxx>
>>>> ---
>>>>   gui/booleansPage.py                          | 3 ++-
>>>>   gui/domainsPage.py                           | 3 ++-
>>>>   gui/fcontextPage.py                          | 3 ++-
>>>>   gui/loginsPage.py                            | 3 ++-
>>>>   gui/modulesPage.py                           | 3 ++-
>>>>   gui/polgengui.py                             | 3 ++-
>>>>   gui/portsPage.py                             | 3 ++-
>>>>   gui/semanagePage.py                          | 3 ++-
>>>>   gui/statusPage.py                            | 3 ++-
>>>>   gui/system-config-selinux.py                 | 3 ++-
>>>>   gui/usersPage.py                             | 3 ++-
>>>>   python/chcat/chcat                           | 5 +++--
>>>>   python/semanage/semanage                     | 3 ++-
>>>>   python/semanage/seobject.py                  | 3 ++-
>>>>   python/sepolgen/src/sepolgen/sepolgeni18n.py | 4 +++-
>>>>   python/sepolicy/sepolicy.py                  | 3 ++-
>>>>   python/sepolicy/sepolicy/__init__.py         | 3 ++-
>>>>   python/sepolicy/sepolicy/generate.py         | 3 ++-
>>>>   python/sepolicy/sepolicy/gui.py              | 3 ++-
>>>>   python/sepolicy/sepolicy/interface.py        | 3 ++-
>>>>   sandbox/sandbox                              | 3 ++-
>>>>   21 files changed, 44 insertions(+), 22 deletions(-)
>>>>
>>>> diff --git a/gui/booleansPage.py b/gui/booleansPage.py
>>>> index 5beec58b..ad11a9b2 100644
>>>> --- a/gui/booleansPage.py
>>>> +++ b/gui/booleansPage.py
>>>> @@ -46,7 +46,8 @@ try:
>>>>           kwargs['unicode'] = True
>>>>       t = gettext.translation(PROGNAME,
>>>>                       localedir="/usr/share/locale",
>>>> -                    **kwargs)
>>>> +                    **kwargs,
>>>> +                    fallback=True)
>>>>       _ = t.gettext
>>>>   except:
>>>>       try:
>>>> diff --git a/gui/domainsPage.py b/gui/domainsPage.py
>>>> index e08f34b4..e6eadd61 100644
>>>> --- a/gui/domainsPage.py
>>>> +++ b/gui/domainsPage.py
>>>> @@ -38,7 +38,8 @@ try:
>>>>           kwargs['unicode'] = True
>>>>       t = gettext.translation(PROGNAME,
>>>>                       localedir="/usr/share/locale",
>>>> -                    **kwargs)
>>>> +                    **kwargs,
>>>> +                    fallback=True)
>>>>       _ = t.gettext
>>>>   except:
>>>>       try:
>>>> diff --git a/gui/fcontextPage.py b/gui/fcontextPage.py
>>>> index bac2bec3..767664f2 100644
>>>> --- a/gui/fcontextPage.py
>>>> +++ b/gui/fcontextPage.py
>>>> @@ -55,7 +55,8 @@ try:
>>>>           kwargs['unicode'] = True
>>>>       t = gettext.translation(PROGNAME,
>>>>                       localedir="/usr/share/locale",
>>>> -                    **kwargs)
>>>> +                    **kwargs,
>>>> +                    fallback=True)
>>>>       _ = t.gettext
>>>>   except:
>>>>       try:
>>>> diff --git a/gui/loginsPage.py b/gui/loginsPage.py
>>>> index 18b93d8c..7e08232a 100644
>>>> --- a/gui/loginsPage.py
>>>> +++ b/gui/loginsPage.py
>>>> @@ -37,7 +37,8 @@ try:
>>>>           kwargs['unicode'] = True
>>>>       t = gettext.translation(PROGNAME,
>>>>                       localedir="/usr/share/locale",
>>>> -                    **kwargs)
>>>> +                    **kwargs,
>>>> +                    fallback=True)
>>>>       _ = t.gettext
>>>>   except:
>>>>       try:
>>>> diff --git a/gui/modulesPage.py b/gui/modulesPage.py
>>>> index c546d455..02b79f15 100644
>>>> --- a/gui/modulesPage.py
>>>> +++ b/gui/modulesPage.py
>>>> @@ -38,7 +38,8 @@ try:
>>>>           kwargs['unicode'] = True
>>>>       t = gettext.translation(PROGNAME,
>>>>                       localedir="/usr/share/locale",
>>>> -                    **kwargs)
>>>> +                    **kwargs,
>>>> +                    fallback=True)
>>>>       _ = t.gettext
>>>>   except:
>>>>       try:
>>>> diff --git a/gui/polgengui.py b/gui/polgengui.py
>>>> index a18f1cba..7a3ecd50 100644
>>>> --- a/gui/polgengui.py
>>>> +++ b/gui/polgengui.py
>>>> @@ -71,7 +71,8 @@ try:
>>>>           kwargs['unicode'] = True
>>>>       t = gettext.translation(PROGNAME,
>>>>                       localedir="/usr/share/locale",
>>>> -                    **kwargs)
>>>> +                    **kwargs,
>>>> +                    fallback=True)
>>>>       _ = t.gettext
>>>>   except:
>>>>       try:
>>>> diff --git a/gui/portsPage.py b/gui/portsPage.py
>>>> index 54aa80de..bee2bdf1 100644
>>>> --- a/gui/portsPage.py
>>>> +++ b/gui/portsPage.py
>>>> @@ -43,7 +43,8 @@ try:
>>>>           kwargs['unicode'] = True
>>>>       t = gettext.translation(PROGNAME,
>>>>                       localedir="/usr/share/locale",
>>>> -                    **kwargs)
>>>> +                    **kwargs,
>>>> +                    fallback=True)
>>>>       _ = t.gettext
>>>>   except:
>>>>       try:
>>>> diff --git a/gui/semanagePage.py b/gui/semanagePage.py
>>>> index 1371d4e7..efad14d9 100644
>>>> --- a/gui/semanagePage.py
>>>> +++ b/gui/semanagePage.py
>>>> @@ -30,7 +30,8 @@ try:
>>>>           kwargs['unicode'] = True
>>>>       t = gettext.translation(PROGNAME,
>>>>                       localedir="/usr/share/locale",
>>>> -                    **kwargs)
>>>> +                    **kwargs,
>>>> +                    fallback=True)
>>>>       _ = t.gettext
>>>>   except:
>>>>       try:
>>>> diff --git a/gui/statusPage.py b/gui/statusPage.py
>>>> index c241ef83..832849e6 100644
>>>> --- a/gui/statusPage.py
>>>> +++ b/gui/statusPage.py
>>>> @@ -43,7 +43,8 @@ try:
>>>>           kwargs['unicode'] = True
>>>>       t = gettext.translation(PROGNAME,
>>>>                       localedir="/usr/share/locale",
>>>> -                    **kwargs)
>>>> +                    **kwargs,
>>>> +                    fallback=True)
>>>>       _ = t.gettext
>>>>   except:
>>>>       try:
>>>> diff --git a/gui/system-config-selinux.py b/gui/system-config-selinux.py
>>>> index 1b460c99..9f53b7fe 100644
>>>> --- a/gui/system-config-selinux.py
>>>> +++ b/gui/system-config-selinux.py
>>>> @@ -53,7 +53,8 @@ try:
>>>>           kwargs['unicode'] = True
>>>>       t = gettext.translation(PROGNAME,
>>>>                       localedir="/usr/share/locale",
>>>> -                    **kwargs)
>>>> +                    **kwargs,
>>>> +                    fallback=True)
>>>>       _ = t.gettext
>>>>   except:
>>>>       try:
>>>> diff --git a/gui/usersPage.py b/gui/usersPage.py
>>>> index d51bd968..9acd3b84 100644
>>>> --- a/gui/usersPage.py
>>>> +++ b/gui/usersPage.py
>>>> @@ -37,7 +37,8 @@ try:
>>>>           kwargs['unicode'] = True
>>>>       t = gettext.translation(PROGNAME,
>>>>                       localedir="/usr/share/locale",
>>>> -                    **kwargs)
>>>> +                    **kwargs,
>>>> +                    fallback=True)
>>>>       _ = t.gettext
>>>>   except:
>>>>       try:
>>>> diff --git a/python/chcat/chcat b/python/chcat/chcat
>>>> index e779fcc6..952cb818 100755
>>>> --- a/python/chcat/chcat
>>>> +++ b/python/chcat/chcat
>>>> @@ -38,9 +38,10 @@ try:
>>>>           kwargs['unicode'] = True
>>>>       t = gettext.translation(PROGNAME,
>>>>                       localedir="/usr/share/locale",
>>>> -                    **kwargs)
>>>> +                    **kwargs,
>>>> +                    fallback=True)
>>>>       _ = t.gettext
>>>> -except ImportError:
>>>> +except:
>>>
>>> Isn't the point of the overall change that gettext.translation() 
>>> doesn't throw an exception anymore?  So we don't need to handle 
>>> OSError/IOError here once fallback=True.  I see that this standardizes 
>>> with the other call sites, but I wonder if standardizing on the more 
>>> specific exceptions (or just leaving as is) wouldn't be better?
>> 
>> Yes, we do not need to handle OSError/IOError exceptions any more.
>> I agree that standardizing on the more specific exception handling would 
>> be more proper. However, translation handling is probably the least 
>> important feature of this tool (most people probably use it in English 
>> and those who don't wish they did) -- we don't want people to be unable 
>> to use the tool at all because of some translation issue and "catch all" 
>> exception handling is the easiest way to avoid that.
>> As this bug showed, not settling on a single approach is probably the 
>> worst alternative (I tested the previous patch with multiple tools, but 
>> apparently missed the one that was different).
>> 
>> That being said, either approach is fine by me. Please let me know if I 
>> should change the patch.
>> Vit
>
> Well, I'll leave questions of what you *should* do up to the 
> maintainers, but I think as far as I'm concerned your point that we 
> really don't want to fail hard on translations makes a lot of sense (and 
> I definitely agree that inconsistency is a pretty bad alternative.  So 
> I'm on board with this approach.  I was initially worried about "what if 
> in some future version gettext adds an exception?", but to your point, 
> we want the same fallback logic in that case, so the general except 
> seems reasonable.
>
> Reviewed-by: Daniel Burgener <dburgener@xxxxxxxxxxxxxxxxxxx>
>

Thanks!

Acked-by: Petr Lautrbach <plautrba@xxxxxxxxxx>




>> 
>>>
>>> -Daniel
>>>
>>>>       try:
>>>>           import builtins
>>>>           builtins.__dict__['_'] = str
>>>> diff --git a/python/semanage/semanage b/python/semanage/semanage
>>>> index 1d828128..4e8d64d6 100644
>>>> --- a/python/semanage/semanage
>>>> +++ b/python/semanage/semanage
>>>> @@ -38,7 +38,8 @@ try:
>>>>           kwargs['unicode'] = True
>>>>       t = gettext.translation(PROGNAME,
>>>>                       localedir="/usr/share/locale",
>>>> -                    **kwargs)
>>>> +                    **kwargs,
>>>> +                    fallback=True)
>>>>       _ = t.gettext
>>>>   except:
>>>>       try:
>>>> diff --git a/python/semanage/seobject.py b/python/semanage/seobject.py
>>>> index ff8f4e9c..0782c082 100644
>>>> --- a/python/semanage/seobject.py
>>>> +++ b/python/semanage/seobject.py
>>>> @@ -42,7 +42,8 @@ try:
>>>>           kwargs['unicode'] = True
>>>>       t = gettext.translation(PROGNAME,
>>>>                       localedir="/usr/share/locale",
>>>> -                    **kwargs)
>>>> +                    **kwargs,
>>>> +                    fallback=True)
>>>>       _ = t.gettext
>>>>   except:
>>>>       try:
>>>> diff --git a/python/sepolgen/src/sepolgen/sepolgeni18n.py 
>>>> b/python/sepolgen/src/sepolgen/sepolgeni18n.py
>>>> index 56ebd807..1ff307d9 100644
>>>> --- a/python/sepolgen/src/sepolgen/sepolgeni18n.py
>>>> +++ b/python/sepolgen/src/sepolgen/sepolgeni18n.py
>>>> @@ -19,7 +19,9 @@
>>>>     try:
>>>>       import gettext
>>>> -    t = gettext.translation( 'selinux-python' )
>>>> +    t = gettext.translation("selinux-python",
>>>> +                        localedir="/usr/share/locale",
>>>> +                        fallback=True)
>>>>       _ = t.gettext
>>>>   except:
>>>>       def _(str):
>>>> diff --git a/python/sepolicy/sepolicy.py b/python/sepolicy/sepolicy.py
>>>> index 7ebe0efa..c7a70e09 100755
>>>> --- a/python/sepolicy/sepolicy.py
>>>> +++ b/python/sepolicy/sepolicy.py
>>>> @@ -36,7 +36,8 @@ try:
>>>>           kwargs['unicode'] = True
>>>>       t = gettext.translation(PROGNAME,
>>>>                       localedir="/usr/share/locale",
>>>> -                    **kwargs)
>>>> +                    **kwargs,
>>>> +                    fallback=True)
>>>>       _ = t.gettext
>>>>   except:
>>>>       try:
>>>> diff --git a/python/sepolicy/sepolicy/__init__.py 
>>>> b/python/sepolicy/sepolicy/__init__.py
>>>> index 7208234b..9c3caa05 100644
>>>> --- a/python/sepolicy/sepolicy/__init__.py
>>>> +++ b/python/sepolicy/sepolicy/__init__.py
>>>> @@ -31,7 +31,8 @@ try:
>>>>           kwargs['unicode'] = True
>>>>       t = gettext.translation(PROGNAME,
>>>>                       localedir="/usr/share/locale",
>>>> -                    **kwargs)
>>>> +                    **kwargs,
>>>> +                    fallback=True)
>>>>       _ = t.gettext
>>>>   except:
>>>>       try:
>>>> diff --git a/python/sepolicy/sepolicy/generate.py 
>>>> b/python/sepolicy/sepolicy/generate.py
>>>> index 67189fc3..3717d5d4 100644
>>>> --- a/python/sepolicy/sepolicy/generate.py
>>>> +++ b/python/sepolicy/sepolicy/generate.py
>>>> @@ -56,7 +56,8 @@ try:
>>>>           kwargs['unicode'] = True
>>>>       t = gettext.translation(PROGNAME,
>>>>                       localedir="/usr/share/locale",
>>>> -                    **kwargs)
>>>> +                    **kwargs,
>>>> +                    fallback=True)
>>>>       _ = t.gettext
>>>>   except:
>>>>       try:
>>>> diff --git a/python/sepolicy/sepolicy/gui.py 
>>>> b/python/sepolicy/sepolicy/gui.py
>>>> index b0263740..5bdbfeba 100644
>>>> --- a/python/sepolicy/sepolicy/gui.py
>>>> +++ b/python/sepolicy/sepolicy/gui.py
>>>> @@ -49,7 +49,8 @@ try:
>>>>           kwargs['unicode'] = True
>>>>       t = gettext.translation(PROGNAME,
>>>>                       localedir="/usr/share/locale",
>>>> -                    **kwargs)
>>>> +                    **kwargs,
>>>> +                    fallback=True)
>>>>       _ = t.gettext
>>>>   except:
>>>>       try:
>>>> diff --git a/python/sepolicy/sepolicy/interface.py 
>>>> b/python/sepolicy/sepolicy/interface.py
>>>> index 599f97fd..43f86443 100644
>>>> --- a/python/sepolicy/sepolicy/interface.py
>>>> +++ b/python/sepolicy/sepolicy/interface.py
>>>> @@ -38,7 +38,8 @@ try:
>>>>           kwargs['unicode'] = True
>>>>       t = gettext.translation(PROGNAME,
>>>>                       localedir="/usr/share/locale",
>>>> -                    **kwargs)
>>>> +                    **kwargs,
>>>> +                    fallback=True)
>>>>       _ = t.gettext
>>>>   except:
>>>>       try:
>>>> diff --git a/sandbox/sandbox b/sandbox/sandbox
>>>> index cd5709fb..789621e1 100644
>>>> --- a/sandbox/sandbox
>>>> +++ b/sandbox/sandbox
>>>> @@ -45,7 +45,8 @@ try:
>>>>           kwargs['unicode'] = True
>>>>       t = gettext.translation(PROGNAME,
>>>>                       localedir="/usr/share/locale",
>>>> -                    **kwargs)
>>>> +                    **kwargs,
>>>> +                    fallback=True)
>>>>       _ = t.gettext
>>>>   except:
>>>>       try:
>>>




[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux