From: John Snow <email@example.com>
To: "Daniel P. Berrangé" <firstname.lastname@example.org>
Cc: Peter Maydell <email@example.com>,
Eduardo Habkost <firstname.lastname@example.org>,
G S Niteesh Babu <email@example.com>,
Cleber Rosa <firstname.lastname@example.org>, Eric Blake <email@example.com>
Subject: Re: [PATCH v3 1/1] python: Update for pylint 2.10
Date: Thu, 16 Sep 2021 11:40:34 -0400 [thread overview]
Message-ID: <CAFn=p-YX7jFqU0g_DAwD3zxC3LxDVQjPRpjdpRcQ6Fy7GeZJKA@mail.gmail.com> (raw)
[-- Attachment #1: Type: text/plain, Size: 4782 bytes --]
On Thu, Sep 16, 2021 at 11:39 AM Daniel P. Berrangé <firstname.lastname@example.org>
> On Thu, Sep 16, 2021 at 09:42:30AM -0400, John Snow wrote:
> > On Thu, Sep 16, 2021 at 8:59 AM Daniel P. Berrangé <email@example.com>
> > wrote:
> > > On Wed, Sep 15, 2021 at 11:40:31AM -0400, John Snow wrote:
> > > > A few new annoyances. Of note is the new warning for an unspecified
> > > > encoding when opening a text file, which actually does indicate a
> > > > potentially real problem; see
> > > > https://www.python.org/dev/peps/pep-0597/#motivation
> > > >
> > > > Use LC_CTYPE to determine an encoding to use for interpreting QEMU's
> > > > terminal output. Note that Python states: "language code and encoding
> > > > may be None if their values cannot be determined" -- use a platform
> > > > default as a backup.
> > > >
> > > > Signed-off-by: John Snow <firstname.lastname@example.org>
> > > > ---
> > > > python/qemu/machine/machine.py | 9 ++++++++-
> > > > python/setup.cfg | 1 +
> > > > 2 files changed, 9 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/python/qemu/machine/machine.py
> > > b/python/qemu/machine/machine.py
> > > > index a7081b1845..51b6e79a13 100644
> > > > --- a/python/qemu/machine/machine.py
> > > > +++ b/python/qemu/machine/machine.py
> > > > @@ -19,6 +19,7 @@
> > > >
> > > > import errno
> > > > from itertools import chain
> > > > +import locale
> > > > import logging
> > > > import os
> > > > import shutil
> > > > @@ -290,8 +291,14 @@ def get_pid(self) -> Optional[int]:
> > > > return self._subp.pid
> > > >
> > > > def _load_io_log(self) -> None:
> > > > + # Assume that the output encoding of QEMU's terminal output
> > > > + # is defined by our locale. If indeterminate, use a platform
> > > default.
> > > > + _, encoding = locale.getlocale()
> > > > + if encoding is None:
> > > > + encoding =
> > >
> > > Do we really need this getpreferredencoding ? IIUC, this is a sign
> > > that the application is buggy by not calling
> > >
> > > locale.setlocale(locale.LC_ALL, '')
> > >
> > > during its main() method, which I think we can just delegate to the
> > > code in question to fix. Missing setlocale will affect everything
> > > they run, so doing workarounds in only 1 place is not worth it IMHO
> > >
> > >
> > I genuinely don't know! (And, I try to keep the Python code free from
> > assuming Linux as much as I can help it.)
> > Python's getlocale documentation states: "language code and encoding may
> > None if their values cannot be determined."
> > https://docs.python.org/3/library/locale.html#locale.getlocale
> > But it is quiet as to the circumstances under which this may happen.
> > Browsing the cpython source code, (3.9ish):
> > ```
> > def getlocale(category=LC_CTYPE):
> > localename = _setlocale(category)
> > if category == LC_ALL and ';' in localename:
> > raise TypeError('category LC_ALL is not supported')
> > return _parse_localename(localename)
> > ```
> > _setlocale is ultimately a call to (I think) _localemodule.c's
> > PyLocale_setlocale(PyObject *self, PyObject *args) C function.
> > It calls `result = setlocale(category, locale)` where the category is
> > to be LC_CTYPE, so this should be equivalent to locale(3) (LC_CTYPE,
> > locale(3) says that "The return value is NULL if the request cannot be
> > honored."
> > Python parses that string according to _parse_localename, which in turn
> > calls normalize(localename).
> > Normalization looks quite involved, but has a fallback of returning the
> > string verbatim. If the normalized locale string is "C", we return the
> > tuple (None, None)!
> > So I figured there was a non-zero chance that we'd see a value of `None`
> > here.
> > Source code is in cpython/Lib/locale.py and
> > if you want to nose around yourself.
> > I also have no idea how this will all shake out on Windows, so I decided
> > add the fallback here just in case. (Does the Python package work on
> > Windows? I don't know, but I avoid assuming it won't EVER run there...
> > Certainly, I have an interest in having the QMP packages I am building
> > on all platforms.)
> > Thoughts?
> Well this machine.py is using UNIX domain sockets and FD passing,
> so Windows is out of the question.
> I'd be inclined to just keep it simple unless someone reports a
> real bug with it.
What about the case where getlocale finds "C" and can't/won't return an
encoding? Is that not a real circumstance?
[-- Attachment #2: Type: text/html, Size: 6411 bytes --]
next prev parent reply other threads:[~2021-09-16 15:58 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-15 15:40 [PATCH v3 0/1] Update check-python-tox test for pylint 2.10 John Snow
2021-09-15 15:40 ` [PATCH v3 1/1] python: Update " John Snow
2021-09-16 12:59 ` Daniel P. Berrangé
2021-09-16 13:42 ` John Snow
2021-09-16 15:39 ` Daniel P. Berrangé
2021-09-16 15:40 ` John Snow [this message]
2021-09-16 17:46 ` John Snow
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:
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
* 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.