From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6F806C433EF for ; Thu, 16 Sep 2021 17:49:08 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id C1EC461056 for ; Thu, 16 Sep 2021 17:49:07 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org C1EC461056 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=nongnu.org Received: from localhost ([::1]:47570 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mQvVS-000879-Mj for qemu-devel@archiver.kernel.org; Thu, 16 Sep 2021 13:49:06 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:44114) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mQvTB-0006gA-W8 for qemu-devel@nongnu.org; Thu, 16 Sep 2021 13:46:46 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:54663) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mQvT7-0000L1-5E for qemu-devel@nongnu.org; Thu, 16 Sep 2021 13:46:44 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1631814399; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=yszteKjXsUw+tEeK30ja5G95BQT5b27QdvetnjlzBRA=; b=cWpEbRc8DOLe/uuOrMGqgbhFDuc9kqo8rjyM5YC+tUeaHsjv4J4Lv3jeVvzLogK7fyh1ry 01zgN+rimIBF7v7Q7qzbrHQh3pwYhvSKRPdc3g//um3xptM7X/Cgmb39obfrHoWKLQAc5H zEe2U0CYHpzu4dULVzoewyfKeEMZvRc= Received: from mail-ot1-f71.google.com (mail-ot1-f71.google.com [209.85.210.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-500-pv183pUgMcaeLTmVstd4cQ-1; Thu, 16 Sep 2021 13:46:36 -0400 X-MC-Unique: pv183pUgMcaeLTmVstd4cQ-1 Received: by mail-ot1-f71.google.com with SMTP id i11-20020a9d53cb000000b00538e5ca17d6so34891302oth.18 for ; Thu, 16 Sep 2021 10:46:36 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=yszteKjXsUw+tEeK30ja5G95BQT5b27QdvetnjlzBRA=; b=8HExQxTWjvXqn630n0RDVVBrU2Sm1ebNKS4sKNv6Vx9t8eviUwoyuSzi87ZM1+8+fQ Wsm/iaZD3r3wtfCvGTSWH5L1GqiBr7ZgUuDO2d0UOnni22+vEC/ZS7PRlQEh0/CnB+hy GLJdccq3MXQCncaVLH04YYgK/A2cWfvgZsw2Z4YKWinpxnOKhghIleSy86mLiXTSo0fR IKXZozCEiBRQ94btOeOZ7GJ0rxaIjRHr7BToy7DF6ArEgkctmr3mGssH/YwhG6DZKRyU wEY2D26nVFwdRHRmerB5i5rdc0iR62o8L/OlzARJvJH0RundybWbhJQ67xNMeS1odZTQ EpDQ== X-Gm-Message-State: AOAM533ahBw+xNal6K+l+mCsl6OL1etEVUl7Dql64074dy7LomFwJ263 5e7kKWH/LJTvPsoPbgQ2sIjVtDHeyYA1+1vAgNxOx1GlQwDhN1WRlkdCD2vd10CVeU84xGgUl+4 n2S4kKaIThlGHE4d1m+wPB+5iuqdOOkw= X-Received: by 2002:a05:6830:2685:: with SMTP id l5mr2057446otu.129.1631814395838; Thu, 16 Sep 2021 10:46:35 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxlywDrcxKZgL0uSd89n1gjrEtH7Bwv/aCnbtYLfIwfo7dD2y3oTVYL7ll9Txop2dMcoU1X5PdxHo7nIN6XYaI= X-Received: by 2002:a05:6830:2685:: with SMTP id l5mr2057425otu.129.1631814395614; Thu, 16 Sep 2021 10:46:35 -0700 (PDT) MIME-Version: 1.0 References: <20210915154031.321592-1-jsnow@redhat.com> <20210915154031.321592-2-jsnow@redhat.com> In-Reply-To: From: John Snow Date: Thu, 16 Sep 2021 13:46:24 -0400 Message-ID: Subject: Re: [PATCH v3 1/1] python: Update for pylint 2.10 To: =?UTF-8?Q?Daniel_P=2E_Berrang=C3=A9?= Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=jsnow@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: multipart/alternative; boundary="00000000000063e8ba05cc206549" Received-SPF: pass client-ip=170.10.133.124; envelope-from=jsnow@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -31 X-Spam_score: -3.2 X-Spam_bar: --- X-Spam_report: (-3.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.392, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Peter Maydell , Eduardo Habkost , qemu-devel , G S Niteesh Babu , Cleber Rosa , Eric Blake Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --00000000000063e8ba05cc206549 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, Sep 16, 2021 at 11:40 AM John Snow wrote: > > > On Thu, Sep 16, 2021 at 11:39 AM Daniel P. Berrang=C3=A9 > 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=C3=A9 > > >> > 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 unspecifie= d >> > > > 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 platfor= m >> > > > default as a backup. >> > > > >> > > > Signed-off-by: John Snow >> > > > --- >> > > > 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 outp= ut >> > > > + # is defined by our locale. If indeterminate, use a >> platform >> > > default. >> > > > + _, encoding =3D locale.getlocale() >> > > > + if encoding is None: >> > > > + encoding =3D >> locale.getpreferredencoding(do_setlocale=3DFalse) >> > > >> > > 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=3DLC_CTYPE): >> > localename =3D _setlocale(category) >> > if category =3D=3D 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 =3D setlocale(category, locale)` where the category i= s >> 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 tur= n >> > calls normalize(localename). >> > Normalization looks quite involved, but has a fallback of returning th= e >> > 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 `Non= e` >> > 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 > So, a few things: (1) machine.py can also use TCP, it doesn't have to use UNIX sockets. (2) The FD passing is optional and already checks for the ability to do so before it is invoked (3) jsnow@scv ~/s/q/python (python-package-pylint-210)> LC_ALL=3D"C" python= 3 -c "import locale; print(locale.getlocale());" (None, None) (4) If you specify an encoding of "None", open() itself does exactly what I wrote: getpreferredencoding(do_setlocale=3DFalse) I'll drop the manual fallback -- but not because it wasn't actually necessary. It just so happens that it's handled for us. --js --00000000000063e8ba05cc206549 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Thu, Sep 16, 2021 at 11:40 AM John= Snow <jsnow@redhat.com> wrot= e:


On Thu, Sep 16, 2021 at 11:39 AM Daniel P. Berrang= =C3=A9 <berrang= e@redhat.com> 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=C3=A9 <berrange@redhat.com><= br> > 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 unsp= ecified
> > > encoding when opening a text file, which actually does indic= ate a
> > > potentially real problem; see
> > > https://www.python.org/dev/peps/p= ep-0597/#motivation
> > >
> > > Use LC_CTYPE to determine an encoding to use for interpretin= g QEMU's
> > > terminal output. Note that Python states: "language cod= e and encoding
> > > may be None if their values cannot be determined" -- us= e a platform
> > > default as a backup.
> > >
> > > Signed-off-by: John Snow <jsnow@redhat.com>
> > > ---
> > >=C2=A0 python/qemu/machine/machine.py | 9 ++++++++-
> > >=C2=A0 python/setup.cfg=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0| 1 +
> > >=C2=A0 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 @@
> > >
> > >=C2=A0 import errno
> > >=C2=A0 from itertools import chain
> > > +import locale
> > >=C2=A0 import logging
> > >=C2=A0 import os
> > >=C2=A0 import shutil
> > > @@ -290,8 +291,14 @@ def get_pid(self) -> Optional[int]:<= br> > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return self._subp.pid
> > >
> > >=C2=A0 =C2=A0 =C2=A0 def _load_io_log(self) -> None:
> > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # Assume that the output encodi= ng of QEMU's terminal output
> > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # is defined by our locale. If = indeterminate, use a platform
> > default.
> > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 _, encoding =3D locale.getlocal= e()
> > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if encoding is None:
> > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 encoding =3D loca= le.getpreferredencoding(do_setlocale=3DFalse)
> >
> > Do we really need this getpreferredencoding ?=C2=A0 IIUC, this is= a sign
> > that the application is buggy by not calling
> >
> >=C2=A0 =C2=A0locale.setlocale(locale.LC_ALL, '')
> >
> > during its main() method, which I think we can just delegate to t= he
> > code in question to fix. Missing setlocale will affect everything=
> > they run, so doing workarounds in only 1 place is not worth it IM= HO
> >
> >
> I genuinely don't know! (And, I try to keep the Python code free f= rom
> assuming Linux as much as I can help it.)
>
> Python's getlocale documentation states: "language code and e= ncoding 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=3DLC_CTYPE):
>=C2=A0 =C2=A0 =C2=A0localename =3D _setlocale(category)
>=C2=A0 =C2=A0 =C2=A0if category =3D=3D LC_ALL and ';' in locale= name:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0raise TypeError('category LC_ALL = is not supported')
>=C2=A0 =C2=A0 =C2=A0return _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 =3D setlocale(category, locale)` where the category i= s going
> to be LC_CTYPE, so this should be equivalent to locale(3) (LC_CTYPE, N= ULL).
>
> locale(3) says that "The return value is NULL if the request cann= ot be
> honored."
>
> Python parses that string according to _parse_localename, which in tur= n
> calls normalize(localename).
> Normalization looks quite involved, but has a fallback of returning th= e
> 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/_localemod= ule.c
> if you want to nose around yourself.
>
> I also have no idea how this will all shake out on Windows, so I decid= ed 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 fi= nds "C" and can't/won't return an encoding? Is that not a= real circumstance?

--js

So, a few things:

(1)= machine.py can also use TCP, it doesn't have to use UNIX sockets.
<= /div>
(2) The FD passing is optional and already checks for the ability= to do so before it is invoked
(3) jsnow@scv ~/s/q/python (python= -package-pylint-210)> LC_ALL=3D"C" python3 -c "import loc= ale; print(locale.getlocale());"
(None, None)

(4) If you specify an encoding of "None", open() itself d= oes exactly what I wrote: getpreferredencoding(do_setlocale=3DFalse)
<= div>
I'll drop the manual fallback -- but not because it = wasn't actually necessary. It just so happens that it's handled for= us.

--js
--00000000000063e8ba05cc206549--