On Thu, Sep 16, 2021 at 11:39 AM Daniel P. Berrangé <[email protected]> wrote:
> 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 protected]> > > 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 <[email protected]> > > > > --- > > > > 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 = > locale.getpreferredencoding(do_setlocale=False) > > > > > > 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 > be > > 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 > going > > to be LC_CTYPE, so this should be equivalent to locale(3) (LC_CTYPE, > NULL). > > > > 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 > cpython/Modules/_localemodule.c > > if you want to nose around yourself. > > > > I also have no idea how this will all shake out on Windows, so I decided > to > > 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 > work > > 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? --js
