[PATCH 0/4] Python: Enable python3.12 support

[PATCH 0/4] Python: Enable python3.12 support

[John Snow]

A few mostly trivial fixes, one backport from the qemu.qmp repo, and
enabling the Python tests to run against Python3.12.

John Snow (4):
  Python/iotests: Add type hint for nbd module
  python/qmp: remove Server.wait_closed() call for Python 3.12
  configure: fix error message to say Python 3.8
  Python: Enable python3.12 support

 configure                              | 5 +++--
 python/qemu/qmp/protocol.py            | 1 -
 python/setup.cfg                       | 3 ++-
 tests/docker/dockerfiles/python.docker | 6 +++++-
 tests/qemu-iotests/tests/nbd-multiconn | 4 +++-
 5 files changed, 13 insertions(+), 6 deletions(-)

-- 
2.41.0

[PATCH 1/4] Python/iotests: Add type hint for nbd module

[John Snow]

The test bails gracefully if this module isn't installed, but linters
need a little help understanding that. It's enough to just declare the
type in this case.

(Fixes pylint complaining about use of an uninitialized variable because
it isn't wise enough to understand the notrun call is noreturn.)

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/qemu-iotests/tests/nbd-multiconn | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/tests/nbd-multiconn b/tests/qemu-iotests/tests/nbd-multiconn
index 478a1eaba2..7e686a786e 100755
--- a/tests/qemu-iotests/tests/nbd-multiconn
+++ b/tests/qemu-iotests/tests/nbd-multiconn
@@ -20,6 +20,8 @@
 
 import os
 from contextlib import contextmanager
+from types import ModuleType
+
 import iotests
 from iotests import qemu_img_create, qemu_io
 
@@ -28,7 +30,7 @@ disk = os.path.join(iotests.test_dir, 'disk')
 size = '4M'
 nbd_sock = os.path.join(iotests.sock_dir, 'nbd_sock')
 nbd_uri = 'nbd+unix:///{}?socket=' + nbd_sock
-
+nbd: ModuleType
 
 @contextmanager
 def open_nbd(export_name):
-- 
2.41.0

Re: [PATCH 1/4] Python/iotests: Add type hint for nbd module

[Vladimir Sementsov-Ogievskiy]

On 06.10.23 22:52, John Snow wrote:
> The test bails gracefully if this module isn't installed, but linters
> need a little help understanding that. It's enough to just declare the
> type in this case.
> 
> (Fixes pylint complaining about use of an uninitialized variable because
> it isn't wise enough to understand the notrun call is noreturn.)
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>


Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

-- 
Best regards,
Vladimir

[PATCH 2/4] python/qmp: remove Server.wait_closed() call for Python 3.12

[John Snow]

This patch is a backport from
https://gitlab.com/qemu-project/python-qemu-qmp/-/commit/e03a3334b6a477beb09b293708632f2c06fe9f61

According to Guido in https://github.com/python/cpython/issues/104344 ,
this call was never meant to wait for the server to shut down - that is
handled synchronously - but instead, this waits for all connections to
close. Or, it would have, if it wasn't broken since it was introduced.

3.12 fixes the bug, which now causes a hang in our code. The fix is just
to remove the wait.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/qemu/qmp/protocol.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/python/qemu/qmp/protocol.py b/python/qemu/qmp/protocol.py
index 753182131f..a4ffdfad51 100644
--- a/python/qemu/qmp/protocol.py
+++ b/python/qemu/qmp/protocol.py
@@ -495,7 +495,6 @@ async def _stop_server(self) -> None:
         try:
             self.logger.debug("Stopping server.")
             self._server.close()
-            await self._server.wait_closed()
             self.logger.debug("Server stopped.")
         finally:
             self._server = None
-- 
2.41.0

Re: [PATCH 2/4] python/qmp: remove Server.wait_closed() call for Python 3.12

[Vladimir Sementsov-Ogievskiy]

On 06.10.23 22:52, John Snow wrote:
> This patch is a backport from
> https://gitlab.com/qemu-project/python-qemu-qmp/-/commit/e03a3334b6a477beb09b293708632f2c06fe9f61
> 
> According to Guido in https://github.com/python/cpython/issues/104344 ,
> this call was never meant to wait for the server to shut down - that is
> handled synchronously - but instead, this waits for all connections to
> close. Or, it would have, if it wasn't broken since it was introduced.
> 
> 3.12 fixes the bug, which now causes a hang in our code. The fix is just
> to remove the wait.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>


-- 
Best regards,
Vladimir

[PATCH 3/4] configure: fix error message to say Python 3.8

[John Snow]

Signed-off-by: John Snow <jsnow@redhat.com>
---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index e08127045d..e9a921ffb0 100755
--- a/configure
+++ b/configure
@@ -944,7 +944,7 @@ then
     # If first_python is set, there was a binary somewhere even though
     # it was not suitable.  Use it for the error message.
     if test -n "$first_python"; then
-        error_exit "Cannot use '$first_python', Python >= 3.7 is required." \
+        error_exit "Cannot use '$first_python', Python >= 3.8 is required." \
             "Use --python=/path/to/python to specify a supported Python."
     else
         error_exit "Python not found. Use --python=/path/to/python"
-- 
2.41.0

Re: [PATCH 3/4] configure: fix error message to say Python 3.8

[Vladimir Sementsov-Ogievskiy]

On 06.10.23 22:52, John Snow wrote:
> Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>


-- 
Best regards,
Vladimir

[PATCH 4/4] Python: Enable python3.12 support

[John Snow]

Python 3.12 has released, so update the test infrastructure to test
against this version. Update the configure script to look for it when an
explicit Python interpreter isn't chosen.

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

diff --git a/configure b/configure
index e9a921ffb0..b480a3d6ae 100755
--- a/configure
+++ b/configure
@@ -561,7 +561,8 @@ first_python=
 if test -z "${PYTHON}"; then
     # A bare 'python' is traditionally python 2.x, but some distros
     # have it as python 3.x, so check in both places.
-    for binary in python3 python python3.11 python3.10 python3.9 python3.8; do
+    for binary in python3 python python3.12 python3.11 \
+                          python3.10 python3.9 python3.8; do
         if has "$binary"; then
             python=$(command -v "$binary")
             if check_py_version "$python"; then
diff --git a/python/setup.cfg b/python/setup.cfg
index 8c67dce457..48668609d3 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -18,6 +18,7 @@ classifiers =
     Programming Language :: Python :: 3.9
     Programming Language :: Python :: 3.10
     Programming Language :: Python :: 3.11
+    Programming Language :: Python :: 3.12
     Typing :: Typed
 
 [options]
@@ -182,7 +183,7 @@ multi_line_output=3
 # of python available on your system to run this test.
 
 [tox:tox]
-envlist = py38, py39, py310, py311
+envlist = py38, py39, py310, py311, py312
 skip_missing_interpreters = true
 
 [testenv]
diff --git a/tests/docker/dockerfiles/python.docker b/tests/docker/dockerfiles/python.docker
index 383ccbdc3a..a3c1321190 100644
--- a/tests/docker/dockerfiles/python.docker
+++ b/tests/docker/dockerfiles/python.docker
@@ -11,7 +11,11 @@ ENV PACKAGES \
     python3-pip \
     python3-tox \
     python3-virtualenv \
-    python3.10
+    python3.10 \
+    python3.11 \
+    python3.12 \
+    python3.8 \
+    python3.9
 
 RUN dnf install -y $PACKAGES
 RUN rpm -q $PACKAGES | sort > /packages.txt
-- 
2.41.0

Re: [PATCH 4/4] Python: Enable python3.12 support

[Vladimir Sementsov-Ogievskiy]

On 06.10.23 22:52, John Snow wrote:
> Python 3.12 has released, so update the test infrastructure to test
> against this version. Update the configure script to look for it when an
> explicit Python interpreter isn't chosen.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   configure                              | 3 ++-
>   python/setup.cfg                       | 3 ++-
>   tests/docker/dockerfiles/python.docker | 6 +++++-
>   3 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/configure b/configure
> index e9a921ffb0..b480a3d6ae 100755
> --- a/configure
> +++ b/configure
> @@ -561,7 +561,8 @@ first_python=
>   if test -z "${PYTHON}"; then
>       # A bare 'python' is traditionally python 2.x, but some distros
>       # have it as python 3.x, so check in both places.
> -    for binary in python3 python python3.11 python3.10 python3.9 python3.8; do
> +    for binary in python3 python python3.12 python3.11 \
> +                          python3.10 python3.9 python3.8; do
>           if has "$binary"; then
>               python=$(command -v "$binary")
>               if check_py_version "$python"; then
> diff --git a/python/setup.cfg b/python/setup.cfg
> index 8c67dce457..48668609d3 100644
> --- a/python/setup.cfg
> +++ b/python/setup.cfg
> @@ -18,6 +18,7 @@ classifiers =
>       Programming Language :: Python :: 3.9
>       Programming Language :: Python :: 3.10
>       Programming Language :: Python :: 3.11
> +    Programming Language :: Python :: 3.12
>       Typing :: Typed
>   
>   [options]
> @@ -182,7 +183,7 @@ multi_line_output=3
>   # of python available on your system to run this test.
>   
>   [tox:tox]
> -envlist = py38, py39, py310, py311
> +envlist = py38, py39, py310, py311, py312
>   skip_missing_interpreters = true
>   
>   [testenv]
> diff --git a/tests/docker/dockerfiles/python.docker b/tests/docker/dockerfiles/python.docker
> index 383ccbdc3a..a3c1321190 100644
> --- a/tests/docker/dockerfiles/python.docker
> +++ b/tests/docker/dockerfiles/python.docker
> @@ -11,7 +11,11 @@ ENV PACKAGES \
>       python3-pip \
>       python3-tox \
>       python3-virtualenv \
> -    python3.10
> +    python3.10 \
> +    python3.11 \
> +    python3.12 \
> +    python3.8 \
> +    python3.9

Hmm, interesting, how did it work before? Only 3.10 was tested?

>   
>   RUN dnf install -y $PACKAGES
>   RUN rpm -q $PACKAGES | sort > /packages.txt

weak, I'm unsure about how this all works, I just see that 3.12 is added like others in all hunks except python.docker, but I think adding several python versions to docker should be safe anyway:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

-- 
Best regards,
Vladimir

Re: [PATCH 4/4] Python: Enable python3.12 support

[Vladimir Sementsov-Ogievskiy]

On 06.10.23 23:39, Vladimir Sementsov-Ogievskiy wrote:
> On 06.10.23 22:52, John Snow wrote:
>> Python 3.12 has released, so update the test infrastructure to test
>> against this version. Update the configure script to look for it when an
>> explicit Python interpreter isn't chosen.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>   configure                              | 3 ++-
>>   python/setup.cfg                       | 3 ++-
>>   tests/docker/dockerfiles/python.docker | 6 +++++-
>>   3 files changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/configure b/configure
>> index e9a921ffb0..b480a3d6ae 100755
>> --- a/configure
>> +++ b/configure
>> @@ -561,7 +561,8 @@ first_python=
>>   if test -z "${PYTHON}"; then
>>       # A bare 'python' is traditionally python 2.x, but some distros
>>       # have it as python 3.x, so check in both places.
>> -    for binary in python3 python python3.11 python3.10 python3.9 python3.8; do
>> +    for binary in python3 python python3.12 python3.11 \
>> +                          python3.10 python3.9 python3.8; do
>>           if has "$binary"; then
>>               python=$(command -v "$binary")
>>               if check_py_version "$python"; then
>> diff --git a/python/setup.cfg b/python/setup.cfg
>> index 8c67dce457..48668609d3 100644
>> --- a/python/setup.cfg
>> +++ b/python/setup.cfg
>> @@ -18,6 +18,7 @@ classifiers =
>>       Programming Language :: Python :: 3.9
>>       Programming Language :: Python :: 3.10
>>       Programming Language :: Python :: 3.11
>> +    Programming Language :: Python :: 3.12
>>       Typing :: Typed
>>   [options]
>> @@ -182,7 +183,7 @@ multi_line_output=3
>>   # of python available on your system to run this test.
>>   [tox:tox]
>> -envlist = py38, py39, py310, py311
>> +envlist = py38, py39, py310, py311, py312
>>   skip_missing_interpreters = true
>>   [testenv]
>> diff --git a/tests/docker/dockerfiles/python.docker b/tests/docker/dockerfiles/python.docker
>> index 383ccbdc3a..a3c1321190 100644
>> --- a/tests/docker/dockerfiles/python.docker
>> +++ b/tests/docker/dockerfiles/python.docker
>> @@ -11,7 +11,11 @@ ENV PACKAGES \
>>       python3-pip \
>>       python3-tox \
>>       python3-virtualenv \
>> -    python3.10
>> +    python3.10 \
>> +    python3.11 \
>> +    python3.12 \
>> +    python3.8 \
>> +    python3.9
> 
> Hmm, interesting, how did it work before? Only 3.10 was tested?
> 
>>   RUN dnf install -y $PACKAGES
>>   RUN rpm -q $PACKAGES | sort > /packages.txt
> 
> weak, I'm unsure about how this all works, I just see that 3.12 is added like others in all hunks except python.docker, but I think adding several python versions to docker should be safe anyway:
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> 

I meant, r-b is weak, not the patch :)

-- 
Best regards,
Vladimir

Re: [PATCH 4/4] Python: Enable python3.12 support

[John Snow]

On Fri, Oct 6, 2023 at 4:40 PM Vladimir Sementsov-Ogievskiy
<vsementsov@yandex-team.ru> wrote:
>
> On 06.10.23 22:52, John Snow wrote:
> > Python 3.12 has released, so update the test infrastructure to test
> > against this version. Update the configure script to look for it when an
> > explicit Python interpreter isn't chosen.
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> >   configure                              | 3 ++-
> >   python/setup.cfg                       | 3 ++-
> >   tests/docker/dockerfiles/python.docker | 6 +++++-
> >   3 files changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/configure b/configure
> > index e9a921ffb0..b480a3d6ae 100755
> > --- a/configure
> > +++ b/configure
> > @@ -561,7 +561,8 @@ first_python=
> >   if test -z "${PYTHON}"; then
> >       # A bare 'python' is traditionally python 2.x, but some distros
> >       # have it as python 3.x, so check in both places.
> > -    for binary in python3 python python3.11 python3.10 python3.9 python3.8; do
> > +    for binary in python3 python python3.12 python3.11 \
> > +                          python3.10 python3.9 python3.8; do
> >           if has "$binary"; then
> >               python=$(command -v "$binary")
> >               if check_py_version "$python"; then
> > diff --git a/python/setup.cfg b/python/setup.cfg
> > index 8c67dce457..48668609d3 100644
> > --- a/python/setup.cfg
> > +++ b/python/setup.cfg
> > @@ -18,6 +18,7 @@ classifiers =
> >       Programming Language :: Python :: 3.9
> >       Programming Language :: Python :: 3.10
> >       Programming Language :: Python :: 3.11
> > +    Programming Language :: Python :: 3.12
> >       Typing :: Typed
> >
> >   [options]
> > @@ -182,7 +183,7 @@ multi_line_output=3
> >   # of python available on your system to run this test.
> >
> >   [tox:tox]
> > -envlist = py38, py39, py310, py311
> > +envlist = py38, py39, py310, py311, py312
> >   skip_missing_interpreters = true
> >
> >   [testenv]
> > diff --git a/tests/docker/dockerfiles/python.docker b/tests/docker/dockerfiles/python.docker
> > index 383ccbdc3a..a3c1321190 100644
> > --- a/tests/docker/dockerfiles/python.docker
> > +++ b/tests/docker/dockerfiles/python.docker
> > @@ -11,7 +11,11 @@ ENV PACKAGES \
> >       python3-pip \
> >       python3-tox \
> >       python3-virtualenv \
> > -    python3.10
> > +    python3.10 \
> > +    python3.11 \
> > +    python3.12 \
> > +    python3.8 \
> > +    python3.9
>
> Hmm, interesting, how did it work before? Only 3.10 was tested?

I was relying on dependencies to pull in other versions -- tox usually
pulls in all but the very latest version. I made it explicit instead.
I can explain this in the commit.

>
> >
> >   RUN dnf install -y $PACKAGES
> >   RUN rpm -q $PACKAGES | sort > /packages.txt
>
> weak, I'm unsure about how this all works, I just see that 3.12 is added like others in all hunks except python.docker, but I think adding several python versions to docker should be safe anyway:
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

Thanks -- and I see your patches for iotests. I'll try to look into
them shortly.

>
> --
> Best regards,
> Vladimir
>