All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Burgener <dburgener@linux.microsoft.com>
To: Vit Mojzis <vmojzis@redhat.com>, selinux@vger.kernel.org
Subject: Re: [PATCH] gettext: handle unsupported languages properly
Date: Fri, 24 Jun 2022 22:09:56 -0400	[thread overview]
Message-ID: <7563a56c-5527-abaf-f33b-ab9d9f1487dc@linux.microsoft.com> (raw)
In-Reply-To: <a85f9664-aa08-fcc9-aa69-f944479eb0fd@redhat.com>

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@redhat.com>
>>> ---
>>>   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@linux.microsoft.com>

> 
>>
>> -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:
>>


  reply	other threads:[~2022-06-25  2:10 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-24 14:24 [PATCH] gettext: handle unsupported languages properly Vit Mojzis
2022-06-24 16:37 ` Daniel Burgener
2022-06-24 17:27   ` Vit Mojzis
2022-06-25  2:09     ` Daniel Burgener [this message]
2022-06-27  8:16       ` Petr Lautrbach
2022-06-29 13:52         ` Petr Lautrbach

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7563a56c-5527-abaf-f33b-ab9d9f1487dc@linux.microsoft.com \
    --to=dburgener@linux.microsoft.com \
    --cc=selinux@vger.kernel.org \
    --cc=vmojzis@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.