[PATCH 0/4] Python: Add 3.13 support, play linter whackamole

[PATCH 0/4] Python: Add 3.13 support, play linter whackamole

[John Snow]

Fix some regressions in check-python-tox that have crept in since Pylint
3.x, and add Python 3.13 support to the pipeline.

GitLab pipeline (before I fixed the missing DCO, but let's be honest, it
can't possibly be worth re-running so many tests for just that):
  https://gitlab.com/jsnow/qemu/-/pipelines/1349737188

John Snow (4):
  python: linter changes for pylint 3.x
  python: Do not use pylint 3.2.4 with python 3.8
  iotests: Change imports for Python 3.13
  python: enable testing for 3.13

 python/qemu/machine/machine.py         | 1 +
 python/qemu/utils/qemu_ga_client.py    | 2 +-
 python/setup.cfg                       | 4 +++-
 tests/docker/dockerfiles/python.docker | 1 +
 tests/qemu-iotests/testenv.py          | 7 ++++++-
 tests/qemu-iotests/testrunner.py       | 9 ++++++---
 6 files changed, 18 insertions(+), 6 deletions(-)

-- 
2.45.0

[PATCH 1/4] python: linter changes for pylint 3.x

[John Snow]

New bleeding edge versions, new nits to iron out. This addresses the
'check-python-tox' optional GitLab test, while 'check-python-minreqs'
saw no regressions, since it's frozen on an older version of pylint.

Fixes:
qemu/machine/machine.py:345:52: E0606: Possibly using variable 'sock' before assignment (possibly-used-before-assignment)
qemu/utils/qemu_ga_client.py:168:4: R1711: Useless return at end of function or method (useless-return)

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/qemu/machine/machine.py      | 1 +
 python/qemu/utils/qemu_ga_client.py | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
index f648f6af451..ebb58d5b68c 100644
--- a/python/qemu/machine/machine.py
+++ b/python/qemu/machine/machine.py
@@ -335,6 +335,7 @@ def binary(self) -> str:
 
     def _pre_launch(self) -> None:
         if self._qmp_set:
+            sock = None
             if self._monitor_address is None:
                 self._sock_pair = socket.socketpair()
                 os.set_inheritable(self._sock_pair[0].fileno(), True)
diff --git a/python/qemu/utils/qemu_ga_client.py b/python/qemu/utils/qemu_ga_client.py
index 9a665e6e990..cf0fcf9a8bb 100644
--- a/python/qemu/utils/qemu_ga_client.py
+++ b/python/qemu/utils/qemu_ga_client.py
@@ -174,7 +174,7 @@ def suspend(self, mode: str) -> None:
             # On error exception will raise
         except asyncio.TimeoutError:
             # On success command will timed out
-            return
+            pass
 
     def shutdown(self, mode: str = 'powerdown') -> None:
         if mode not in ['powerdown', 'halt', 'reboot']:
-- 
2.45.0

[PATCH 2/4] python: Do not use pylint 3.2.4 with python 3.8

[John Snow]

There is a bug in this version,
see: https://github.com/pylint-dev/pylint/issues/9751

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/setup.cfg | 1 +
 1 file changed, 1 insertion(+)

diff --git a/python/setup.cfg b/python/setup.cfg
index 48668609d3e..8ebd345d7ed 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -41,6 +41,7 @@ devel =
     isort >= 5.1.2
     mypy >= 1.4.0
     pylint >= 2.17.3
+    pylint != 3.2.4; python_version<"3.9"
     tox >= 3.18.0
     urwid >= 2.1.2
     urwid-readline >= 0.13
-- 
2.45.0

[PATCH 3/4] iotests: Change imports for Python 3.13

[John Snow]

Python 3.13 isn't out yet, but it's in beta and Fedora is ramping up to
make it the default system interpreter for Fedora 41.

They moved our cheese for where ContextManager lives; add a conditional
to locate it while we support both pre-3.9 and 3.13+.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/qemu-iotests/testenv.py    | 7 ++++++-
 tests/qemu-iotests/testrunner.py | 9 ++++++---
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index 588f30a4f14..96d69e56963 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -25,7 +25,12 @@
 import random
 import subprocess
 import glob
-from typing import List, Dict, Any, Optional, ContextManager
+from typing import List, Dict, Any, Optional
+
+if sys.version_info >= (3, 9):
+    from contextlib import AbstractContextManager as ContextManager
+else:
+    from typing import ContextManager
 
 DEF_GDB_OPTIONS = 'localhost:12345'
 
diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py
index 7b322272e92..2e236c8fa39 100644
--- a/tests/qemu-iotests/testrunner.py
+++ b/tests/qemu-iotests/testrunner.py
@@ -27,11 +27,14 @@
 import shutil
 import sys
 from multiprocessing import Pool
-from typing import List, Optional, Any, Sequence, Dict, \
-        ContextManager
-
+from typing import List, Optional, Any, Sequence, Dict
 from testenv import TestEnv
 
+if sys.version_info >= (3, 9):
+    from contextlib import AbstractContextManager as ContextManager
+else:
+    from typing import ContextManager
+
 
 def silent_unlink(path: Path) -> None:
     try:
-- 
2.45.0

[PATCH 4/4] python: enable testing for 3.13

[John Snow]

Python 3.13 is in beta and Fedora 41 is preparing to make it the default
system interpreter; enable testing for it.

(In the event problems develop prior to release, it should only impact
the check-python-tox job, which is not run by default and is allowed to
fail.)

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/setup.cfg                       | 3 ++-
 tests/docker/dockerfiles/python.docker | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/python/setup.cfg b/python/setup.cfg
index 8ebd345d7ed..3b4e2cc5501 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -19,6 +19,7 @@ classifiers =
     Programming Language :: Python :: 3.10
     Programming Language :: Python :: 3.11
     Programming Language :: Python :: 3.12
+    Programming Language :: Python :: 3.13
     Typing :: Typed
 
 [options]
@@ -184,7 +185,7 @@ multi_line_output=3
 # of python available on your system to run this test.
 
 [tox:tox]
-envlist = py38, py39, py310, py311, py312
+envlist = py38, py39, py310, py311, py312, py313
 skip_missing_interpreters = true
 
 [testenv]
diff --git a/tests/docker/dockerfiles/python.docker b/tests/docker/dockerfiles/python.docker
index a3c1321190c..8f0af9ef25f 100644
--- a/tests/docker/dockerfiles/python.docker
+++ b/tests/docker/dockerfiles/python.docker
@@ -14,6 +14,7 @@ ENV PACKAGES \
     python3.10 \
     python3.11 \
     python3.12 \
+    python3.13 \
     python3.8 \
     python3.9
 
-- 
2.45.0

Re: [PATCH 3/4] iotests: Change imports for Python 3.13

[John Snow]

[-- Attachment #1: Type: text/plain, Size: 2020 bytes --]

Ping - happy to merge this series myself but didn't wanna change iotests
without at least an ack from the lord of that castle.

On Wed, Jun 26, 2024, 7:22 PM John Snow <jsnow@redhat.com> wrote:

> Python 3.13 isn't out yet, but it's in beta and Fedora is ramping up to
> make it the default system interpreter for Fedora 41.
>
> They moved our cheese for where ContextManager lives; add a conditional
> to locate it while we support both pre-3.9 and 3.13+.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  tests/qemu-iotests/testenv.py    | 7 ++++++-
>  tests/qemu-iotests/testrunner.py | 9 ++++++---
>  2 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
> index 588f30a4f14..96d69e56963 100644
> --- a/tests/qemu-iotests/testenv.py
> +++ b/tests/qemu-iotests/testenv.py
> @@ -25,7 +25,12 @@
>  import random
>  import subprocess
>  import glob
> -from typing import List, Dict, Any, Optional, ContextManager
> +from typing import List, Dict, Any, Optional
> +
> +if sys.version_info >= (3, 9):
> +    from contextlib import AbstractContextManager as ContextManager
> +else:
> +    from typing import ContextManager
>
>  DEF_GDB_OPTIONS = 'localhost:12345'
>
> diff --git a/tests/qemu-iotests/testrunner.py
> b/tests/qemu-iotests/testrunner.py
> index 7b322272e92..2e236c8fa39 100644
> --- a/tests/qemu-iotests/testrunner.py
> +++ b/tests/qemu-iotests/testrunner.py
> @@ -27,11 +27,14 @@
>  import shutil
>  import sys
>  from multiprocessing import Pool
> -from typing import List, Optional, Any, Sequence, Dict, \
> -        ContextManager
> -
> +from typing import List, Optional, Any, Sequence, Dict
>  from testenv import TestEnv
>
> +if sys.version_info >= (3, 9):
> +    from contextlib import AbstractContextManager as ContextManager
> +else:
> +    from typing import ContextManager
> +
>
>  def silent_unlink(path: Path) -> None:
>      try:
> --
> 2.45.0
>
>

[-- Attachment #2: Type: text/html, Size: 2536 bytes --]

Re: [PATCH 3/4] iotests: Change imports for Python 3.13

[Nir Soffer]

On Thu, Jun 27, 2024 at 2:23 AM John Snow <jsnow@redhat.com> wrote:
>
> Python 3.13 isn't out yet, but it's in beta and Fedora is ramping up to
> make it the default system interpreter for Fedora 41.
>
> They moved our cheese for where ContextManager lives; add a conditional
> to locate it while we support both pre-3.9 and 3.13+.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  tests/qemu-iotests/testenv.py    | 7 ++++++-
>  tests/qemu-iotests/testrunner.py | 9 ++++++---
>  2 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
> index 588f30a4f14..96d69e56963 100644
> --- a/tests/qemu-iotests/testenv.py
> +++ b/tests/qemu-iotests/testenv.py
> @@ -25,7 +25,12 @@
>  import random
>  import subprocess
>  import glob
> -from typing import List, Dict, Any, Optional, ContextManager
> +from typing import List, Dict, Any, Optional
> +
> +if sys.version_info >= (3, 9):
> +    from contextlib import AbstractContextManager as ContextManager
> +else:
> +    from typing import ContextManager

It can be cleaner to add a compat module hiding the details so the
entire project
can have a single instance of this. Other code will just use:

    from compat import ContextManager

>
>  DEF_GDB_OPTIONS = 'localhost:12345'
>
> diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py
> index 7b322272e92..2e236c8fa39 100644
> --- a/tests/qemu-iotests/testrunner.py
> +++ b/tests/qemu-iotests/testrunner.py
> @@ -27,11 +27,14 @@
>  import shutil
>  import sys
>  from multiprocessing import Pool
> -from typing import List, Optional, Any, Sequence, Dict, \
> -        ContextManager
> -
> +from typing import List, Optional, Any, Sequence, Dict
>  from testenv import TestEnv
>
> +if sys.version_info >= (3, 9):
> +    from contextlib import AbstractContextManager as ContextManager
> +else:
> +    from typing import ContextManager
> +
>
>  def silent_unlink(path: Path) -> None:
>      try:
> --
> 2.45.0
>
>

Re: [PATCH 3/4] iotests: Change imports for Python 3.13

[John Snow]

[-- Attachment #1: Type: text/plain, Size: 1723 bytes --]

On Tue, Jul 2, 2024 at 7:52 AM Nir Soffer <nsoffer@redhat.com> wrote:

> On Thu, Jun 27, 2024 at 2:23 AM John Snow <jsnow@redhat.com> wrote:
> >
> > Python 3.13 isn't out yet, but it's in beta and Fedora is ramping up to
> > make it the default system interpreter for Fedora 41.
> >
> > They moved our cheese for where ContextManager lives; add a conditional
> > to locate it while we support both pre-3.9 and 3.13+.
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> >  tests/qemu-iotests/testenv.py    | 7 ++++++-
> >  tests/qemu-iotests/testrunner.py | 9 ++++++---
> >  2 files changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/tests/qemu-iotests/testenv.py
> b/tests/qemu-iotests/testenv.py
> > index 588f30a4f14..96d69e56963 100644
> > --- a/tests/qemu-iotests/testenv.py
> > +++ b/tests/qemu-iotests/testenv.py
> > @@ -25,7 +25,12 @@
> >  import random
> >  import subprocess
> >  import glob
> > -from typing import List, Dict, Any, Optional, ContextManager
> > +from typing import List, Dict, Any, Optional
> > +
> > +if sys.version_info >= (3, 9):
> > +    from contextlib import AbstractContextManager as ContextManager
> > +else:
> > +    from typing import ContextManager
>
> It can be cleaner to add a compat module hiding the details so the
> entire project
> can have a single instance of this. Other code will just use:
>
>     from compat import ContextManager
>

If there were more than two uses, I'd consider it. As it stands, a
compat.py module with just one import conditional in it doesn't seem worth
the hassle. Are there more cases of compatibility goop inside iotests that
need to be factored out to make it worth it?

--js

[-- Attachment #2: Type: text/html, Size: 2380 bytes --]

Re: [PATCH 3/4] iotests: Change imports for Python 3.13

[Nir Soffer]

[-- Attachment #1: Type: text/plain, Size: 1977 bytes --]


> On 2 Jul 2024, at 17:44, John Snow <jsnow@redhat.com> wrote:
> 
> 
> 
> On Tue, Jul 2, 2024 at 7:52 AM Nir Soffer <nsoffer@redhat.com <mailto:nsoffer@redhat.com>> wrote:
>> On Thu, Jun 27, 2024 at 2:23 AM John Snow <jsnow@redhat.com <mailto:jsnow@redhat.com>> wrote:
>> >
>> > Python 3.13 isn't out yet, but it's in beta and Fedora is ramping up to
>> > make it the default system interpreter for Fedora 41.
>> >
>> > They moved our cheese for where ContextManager lives; add a conditional
>> > to locate it while we support both pre-3.9 and 3.13+.
>> >
>> > Signed-off-by: John Snow <jsnow@redhat.com <mailto:jsnow@redhat.com>>
>> > ---
>> >  tests/qemu-iotests/testenv.py    | 7 ++++++-
>> >  tests/qemu-iotests/testrunner.py | 9 ++++++---
>> >  2 files changed, 12 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
>> > index 588f30a4f14..96d69e56963 100644
>> > --- a/tests/qemu-iotests/testenv.py
>> > +++ b/tests/qemu-iotests/testenv.py
>> > @@ -25,7 +25,12 @@
>> >  import random
>> >  import subprocess
>> >  import glob
>> > -from typing import List, Dict, Any, Optional, ContextManager
>> > +from typing import List, Dict, Any, Optional
>> > +
>> > +if sys.version_info >= (3, 9):
>> > +    from contextlib import AbstractContextManager as ContextManager
>> > +else:
>> > +    from typing import ContextManager
>> 
>> It can be cleaner to add a compat module hiding the details so the
>> entire project
>> can have a single instance of this. Other code will just use:
>> 
>>     from compat import ContextManager
> 
> If there were more than two uses, I'd consider it. As it stands, a compat.py module with just one import conditional in it doesn't seem worth the hassle. Are there more cases of compatibility goop inside iotests that need to be factored out to make it worth it?

I don’t about other. For me even one instance is ugly enough :-)


[-- Attachment #2: Type: text/html, Size: 3538 bytes --]

Re: [PATCH 1/4] python: linter changes for pylint 3.x

[Alex Bennée]

John Snow <jsnow@redhat.com> writes:

> New bleeding edge versions, new nits to iron out. This addresses the
> 'check-python-tox' optional GitLab test, while 'check-python-minreqs'
> saw no regressions, since it's frozen on an older version of pylint.
>
> Fixes:
> qemu/machine/machine.py:345:52: E0606: Possibly using variable 'sock' before assignment (possibly-used-before-assignment)
> qemu/utils/qemu_ga_client.py:168:4: R1711: Useless return at end of function or method (useless-return)
>
> Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 2/4] python: Do not use pylint 3.2.4 with python 3.8

[Alex Bennée]

John Snow <jsnow@redhat.com> writes:

> There is a bug in this version,
> see: https://github.com/pylint-dev/pylint/issues/9751
>
> Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 4/4] python: enable testing for 3.13

[Alex Bennée]

John Snow <jsnow@redhat.com> writes:

> Python 3.13 is in beta and Fedora 41 is preparing to make it the default
> system interpreter; enable testing for it.
>
> (In the event problems develop prior to release, it should only impact
> the check-python-tox job, which is not run by default and is allowed to
> fail.)
>
> Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 3/4] iotests: Change imports for Python 3.13

[John Snow]

[-- Attachment #1: Type: text/plain, Size: 2424 bytes --]

On Tue, Jul 2, 2024, 1:51 PM Nir Soffer <nsoffer@redhat.com> wrote:

>
> On 2 Jul 2024, at 17:44, John Snow <jsnow@redhat.com> wrote:
>
>
>
> On Tue, Jul 2, 2024 at 7:52 AM Nir Soffer <nsoffer@redhat.com> wrote:
>
>> On Thu, Jun 27, 2024 at 2:23 AM John Snow <jsnow@redhat.com> wrote:
>> >
>> > Python 3.13 isn't out yet, but it's in beta and Fedora is ramping up to
>> > make it the default system interpreter for Fedora 41.
>> >
>> > They moved our cheese for where ContextManager lives; add a conditional
>> > to locate it while we support both pre-3.9 and 3.13+.
>> >
>> > Signed-off-by: John Snow <jsnow@redhat.com>
>> > ---
>> >  tests/qemu-iotests/testenv.py    | 7 ++++++-
>> >  tests/qemu-iotests/testrunner.py | 9 ++++++---
>> >  2 files changed, 12 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/tests/qemu-iotests/testenv.py
>> b/tests/qemu-iotests/testenv.py
>> > index 588f30a4f14..96d69e56963 100644
>> > --- a/tests/qemu-iotests/testenv.py
>> > +++ b/tests/qemu-iotests/testenv.py
>> > @@ -25,7 +25,12 @@
>> >  import random
>> >  import subprocess
>> >  import glob
>> > -from typing import List, Dict, Any, Optional, ContextManager
>> > +from typing import List, Dict, Any, Optional
>> > +
>> > +if sys.version_info >= (3, 9):
>> > +    from contextlib import AbstractContextManager as ContextManager
>> > +else:
>> > +    from typing import ContextManager
>>
>> It can be cleaner to add a compat module hiding the details so the
>> entire project
>> can have a single instance of this. Other code will just use:
>>
>>     from compat import ContextManager
>>
>
> If there were more than two uses, I'd consider it. As it stands, a
> compat.py module with just one import conditional in it doesn't seem worth
> the hassle. Are there more cases of compatibility goop inside iotests that
> need to be factored out to make it worth it?
>
>
> I don’t about other. For me even one instance is ugly enough :-)
>

I was going to add it to qemu/utils, but then I remembered the
testenv/testrunner script here needs to operate without external
dependencies because part of the function of these modules is to *locate*
those dependencies.

Ehh. I'm going to say that repeating the import scaffolding in just two
places is fine enough for now and really not worth adding a compat.py for
*just* this. Let's just get the tests green.

--js

[-- Attachment #2: Type: text/html, Size: 4109 bytes --]

Re: [PATCH 0/4] Python: Add 3.13 support, play linter whackamole

[John Snow]

[-- Attachment #1: Type: text/plain, Size: 1128 bytes --]

On Wed, Jun 26, 2024, 7:22 PM John Snow <jsnow@redhat.com> wrote:

> Fix some regressions in check-python-tox that have crept in since Pylint
> 3.x, and add Python 3.13 support to the pipeline.
>
> GitLab pipeline (before I fixed the missing DCO, but let's be honest, it
> can't possibly be worth re-running so many tests for just that):
>   https://gitlab.com/jsnow/qemu/-/pipelines/1349737188
>
> John Snow (4):
>   python: linter changes for pylint 3.x
>   python: Do not use pylint 3.2.4 with python 3.8
>   iotests: Change imports for Python 3.13
>   python: enable testing for 3.13
>
>  python/qemu/machine/machine.py         | 1 +
>  python/qemu/utils/qemu_ga_client.py    | 2 +-
>  python/setup.cfg                       | 4 +++-
>  tests/docker/dockerfiles/python.docker | 1 +
>  tests/qemu-iotests/testenv.py          | 7 ++++++-
>  tests/qemu-iotests/testrunner.py       | 9 ++++++---
>  6 files changed, 18 insertions(+), 6 deletions(-)
>
> --
> 2.45.0
>

Staging under my Python branch, since I need to get tests green to roll
forward with some more substantial changes.

--js

>

[-- Attachment #2: Type: text/html, Size: 1858 bytes --]