[PATCH 00/11] Python: packaging cleanups

[PATCH 00/11] Python: packaging cleanups

[John Snow]

This series is a collection of touchups and small fixes to the Python
packaging series that was initiated with commit ea1213b7.

It fixes a small handful of annoyances and adjusts some readme files,
help text, and so on.

I'd like to make sure this is included prior to the 6.1 RC testing
period.

John Snow (11):
  python: expose typing information via PEP 561
  python: Remove global pylint suppressions
  python: Re-lock pipenv at *oldest* supported versions
  python: README.rst touchups
  python: Add no-install usage instructions
  python: rename 'venv-check' target to 'check-pipenv'
  python: update help text for check-tox
  python: add 'make check-venv' invocation
  python: Update help text on 'make check', 'make develop'
  python: Update help text on 'make clean', 'make distclean'
  python: remove auto-generated pyproject.toml file

 python/README.rst              |  47 +++++++++++---
 .gitlab-ci.d/static_checks.yml |   2 +-
 python/Makefile                |  81 +++++++++++++++++------
 python/Pipfile.lock            | 113 ++++++++++++++++-----------------
 python/qemu/machine/machine.py |   3 +
 python/qemu/machine/py.typed   |   0
 python/qemu/machine/qtest.py   |   2 +
 python/qemu/qmp/py.typed       |   0
 python/qemu/utils/py.typed     |   0
 python/setup.cfg               |  12 ++--
 10 files changed, 167 insertions(+), 93 deletions(-)
 create mode 100644 python/qemu/machine/py.typed
 create mode 100644 python/qemu/qmp/py.typed
 create mode 100644 python/qemu/utils/py.typed

-- 
2.31.1

[PATCH 01/11] python: expose typing information via PEP 561

[John Snow]

https://www.python.org/dev/peps/pep-0561/#specification

Create 'py.typed' files in each subpackage that indicate to mypy that
this is a typed module, so that users of any of these packages can use
mypy to check their code as well.

Note: Theoretically it's possible to ditch MANIFEST.in in favor of using
package_data in setup.cfg, but I genuinely could not figure out how to
get it to include things from the *source root* into the *package root*;
only how to include things from each subpackage. I tried!

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/qemu/machine/py.typed | 0
 python/qemu/qmp/py.typed     | 0
 python/qemu/utils/py.typed   | 0
 python/setup.cfg             | 4 ++++
 4 files changed, 4 insertions(+)
 create mode 100644 python/qemu/machine/py.typed
 create mode 100644 python/qemu/qmp/py.typed
 create mode 100644 python/qemu/utils/py.typed

diff --git a/python/qemu/machine/py.typed b/python/qemu/machine/py.typed
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/python/qemu/qmp/py.typed b/python/qemu/qmp/py.typed
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/python/qemu/utils/py.typed b/python/qemu/utils/py.typed
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/python/setup.cfg b/python/setup.cfg
index 85cecbb41b..db1639c1f2 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -19,6 +19,7 @@ classifiers =
     Programming Language :: Python :: 3.8
     Programming Language :: Python :: 3.9
     Programming Language :: Python :: 3.10
+    Typing :: Typed
 
 [options]
 python_requires = >= 3.6
@@ -27,6 +28,9 @@ packages =
     qemu.machine
     qemu.utils
 
+[options.package_data]
+* = py.typed
+
 [options.extras_require]
 # Run `pipenv lock --dev` when changing these requirements.
 devel =
-- 
2.31.1

Re: [PATCH 01/11] python: expose typing information via PEP 561

[Willian Rampazzo]

On Fri, Jun 25, 2021 at 12:46 PM John Snow <jsnow@redhat.com> wrote:
>
> https://www.python.org/dev/peps/pep-0561/#specification
>
> Create 'py.typed' files in each subpackage that indicate to mypy that
> this is a typed module, so that users of any of these packages can use
> mypy to check their code as well.
>
> Note: Theoretically it's possible to ditch MANIFEST.in in favor of using
> package_data in setup.cfg, but I genuinely could not figure out how to
> get it to include things from the *source root* into the *package root*;
> only how to include things from each subpackage. I tried!
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  python/qemu/machine/py.typed | 0
>  python/qemu/qmp/py.typed     | 0
>  python/qemu/utils/py.typed   | 0
>  python/setup.cfg             | 4 ++++
>  4 files changed, 4 insertions(+)
>  create mode 100644 python/qemu/machine/py.typed
>  create mode 100644 python/qemu/qmp/py.typed
>  create mode 100644 python/qemu/utils/py.typed
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

Re: [PATCH 01/11] python: expose typing information via PEP 561

[Wainer dos Santos Moschetta]

Hi,

On 6/25/21 12:45 PM, John Snow wrote:
> https://www.python.org/dev/peps/pep-0561/#specification
> Create 'py.typed' files in each subpackage that indicate to mypy that
> this is a typed module, so that users of any of these packages can use
> mypy to check their code as well.
>
> Note: Theoretically it's possible to ditch MANIFEST.in in favor of using
> package_data in setup.cfg, but I genuinely could not figure out how to
> get it to include things from the *source root* into the *package root*;
> only how to include things from each subpackage. I tried!
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   python/qemu/machine/py.typed | 0
>   python/qemu/qmp/py.typed     | 0
>   python/qemu/utils/py.typed   | 0
>   python/setup.cfg             | 4 ++++
>   4 files changed, 4 insertions(+)
>   create mode 100644 python/qemu/machine/py.typed
>   create mode 100644 python/qemu/qmp/py.typed
>   create mode 100644 python/qemu/utils/py.typed

Just double-checking that you did not add a single py.typed on the root 
"python" dir, so that all modules inherit from it, because this is a 
namespace package. Right?

Anyway,

Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

>
> diff --git a/python/qemu/machine/py.typed b/python/qemu/machine/py.typed
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/python/qemu/qmp/py.typed b/python/qemu/qmp/py.typed
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/python/qemu/utils/py.typed b/python/qemu/utils/py.typed
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/python/setup.cfg b/python/setup.cfg
> index 85cecbb41b..db1639c1f2 100644
> --- a/python/setup.cfg
> +++ b/python/setup.cfg
> @@ -19,6 +19,7 @@ classifiers =
>       Programming Language :: Python :: 3.8
>       Programming Language :: Python :: 3.9
>       Programming Language :: Python :: 3.10
> +    Typing :: Typed
>   
>   [options]
>   python_requires = >= 3.6
> @@ -27,6 +28,9 @@ packages =
>       qemu.machine
>       qemu.utils
>   
> +[options.package_data]
> +* = py.typed
> +
>   [options.extras_require]
>   # Run `pipenv lock --dev` when changing these requirements.
>   devel =

Re: [PATCH 01/11] python: expose typing information via PEP 561

[John Snow]

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

On Mon, Jun 28, 2021 at 4:12 PM Wainer dos Santos Moschetta <
wainersm@redhat.com> wrote:

> Hi,
>
> On 6/25/21 12:45 PM, John Snow wrote:
> > https://www.python.org/dev/peps/pep-0561/#specification
> > Create 'py.typed' files in each subpackage that indicate to mypy that
> > this is a typed module, so that users of any of these packages can use
> > mypy to check their code as well.
> >
> > Note: Theoretically it's possible to ditch MANIFEST.in in favor of using
> > package_data in setup.cfg, but I genuinely could not figure out how to
> > get it to include things from the *source root* into the *package root*;
> > only how to include things from each subpackage. I tried!
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> >   python/qemu/machine/py.typed | 0
> >   python/qemu/qmp/py.typed     | 0
> >   python/qemu/utils/py.typed   | 0
> >   python/setup.cfg             | 4 ++++
> >   4 files changed, 4 insertions(+)
> >   create mode 100644 python/qemu/machine/py.typed
> >   create mode 100644 python/qemu/qmp/py.typed
> >   create mode 100644 python/qemu/utils/py.typed
>
> Just double-checking that you did not add a single py.typed on the root
> "python" dir, so that all modules inherit from it, because this is a
> namespace package. Right?
>

Yes, that's right. Each sub-package here can technically be typed or not
independently from one another, even though the packaging script I have
written at the moment will bundle all three of these packages together.
Complying with PEP 561 requires package-level information.

(So far as I am aware at-present, anyway.)


> Anyway,
>
> Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
>

Thanks!
--js

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

[PATCH 02/11] python: Remove global pylint suppressions

[John Snow]

These suppressions only apply to a small handful of places. Instead of
disabling them globally, disable them just in the cases where we
need. The design of the machine class grew quite organically with tons
of constructor and class instance variables -- there's little chance of
meaningfully refactoring it in the near term, so just suppress the
warnings for that class.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/qemu/machine/machine.py | 3 +++
 python/qemu/machine/qtest.py   | 2 ++
 python/setup.cfg               | 4 +---
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
index b62435528e..e3345dfa1b 100644
--- a/python/qemu/machine/machine.py
+++ b/python/qemu/machine/machine.py
@@ -84,6 +84,7 @@ class QEMUMachine:
             ...
         # vm is guaranteed to be shut down here
     """
+    # pylint: disable=too-many-instance-attributes, too-many-public-methods
 
     def __init__(self,
                  binary: str,
@@ -111,6 +112,8 @@ def __init__(self,
         @param console_log: (optional) path to console log file
         @note: Qemu process is not started until launch() is used.
         '''
+        # pylint: disable=too-many-arguments
+
         # Direct user configuration
 
         self._binary = binary
diff --git a/python/qemu/machine/qtest.py b/python/qemu/machine/qtest.py
index 93700684d1..d6d9c6a34a 100644
--- a/python/qemu/machine/qtest.py
+++ b/python/qemu/machine/qtest.py
@@ -116,6 +116,8 @@ def __init__(self,
                  base_temp_dir: str = "/var/tmp",
                  socket_scm_helper: Optional[str] = None,
                  sock_dir: Optional[str] = None):
+        # pylint: disable=too-many-arguments
+
         if name is None:
             name = "qemu-%d" % os.getpid()
         if sock_dir is None:
diff --git a/python/setup.cfg b/python/setup.cfg
index db1639c1f2..524789d6e0 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -87,9 +87,7 @@ ignore_missing_imports = True
 # --enable=similarities". If you want to run only the classes checker, but have
 # no Warning level messages displayed, use "--disable=all --enable=classes
 # --disable=W".
-disable=too-many-arguments,
-        too-many-instance-attributes,
-        too-many-public-methods,
+disable=
 
 [pylint.basic]
 # Good variable names which should always be accepted, separated by a comma.
-- 
2.31.1

Re: [PATCH 02/11] python: Remove global pylint suppressions

[Willian Rampazzo]

On Fri, Jun 25, 2021 at 12:46 PM John Snow <jsnow@redhat.com> wrote:
>
> These suppressions only apply to a small handful of places. Instead of
> disabling them globally, disable them just in the cases where we
> need. The design of the machine class grew quite organically with tons
> of constructor and class instance variables -- there's little chance of
> meaningfully refactoring it in the near term, so just suppress the
> warnings for that class.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  python/qemu/machine/machine.py | 3 +++
>  python/qemu/machine/qtest.py   | 2 ++
>  python/setup.cfg               | 4 +---
>  3 files changed, 6 insertions(+), 3 deletions(-)
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

Re: [PATCH 02/11] python: Remove global pylint suppressions

[Wainer dos Santos Moschetta]

On 6/25/21 12:45 PM, John Snow wrote:
> These suppressions only apply to a small handful of places. Instead of
> disabling them globally, disable them just in the cases where we
> need. The design of the machine class grew quite organically with tons
> of constructor and class instance variables -- there's little chance of
> meaningfully refactoring it in the near term, so just suppress the
> warnings for that class.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   python/qemu/machine/machine.py | 3 +++
>   python/qemu/machine/qtest.py   | 2 ++
>   python/setup.cfg               | 4 +---
>   3 files changed, 6 insertions(+), 3 deletions(-)
Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
>
> diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
> index b62435528e..e3345dfa1b 100644
> --- a/python/qemu/machine/machine.py
> +++ b/python/qemu/machine/machine.py
> @@ -84,6 +84,7 @@ class QEMUMachine:
>               ...
>           # vm is guaranteed to be shut down here
>       """
> +    # pylint: disable=too-many-instance-attributes, too-many-public-methods
>   
>       def __init__(self,
>                    binary: str,
> @@ -111,6 +112,8 @@ def __init__(self,
>           @param console_log: (optional) path to console log file
>           @note: Qemu process is not started until launch() is used.
>           '''
> +        # pylint: disable=too-many-arguments
> +
>           # Direct user configuration
>   
>           self._binary = binary
> diff --git a/python/qemu/machine/qtest.py b/python/qemu/machine/qtest.py
> index 93700684d1..d6d9c6a34a 100644
> --- a/python/qemu/machine/qtest.py
> +++ b/python/qemu/machine/qtest.py
> @@ -116,6 +116,8 @@ def __init__(self,
>                    base_temp_dir: str = "/var/tmp",
>                    socket_scm_helper: Optional[str] = None,
>                    sock_dir: Optional[str] = None):
> +        # pylint: disable=too-many-arguments
> +
>           if name is None:
>               name = "qemu-%d" % os.getpid()
>           if sock_dir is None:
> diff --git a/python/setup.cfg b/python/setup.cfg
> index db1639c1f2..524789d6e0 100644
> --- a/python/setup.cfg
> +++ b/python/setup.cfg
> @@ -87,9 +87,7 @@ ignore_missing_imports = True
>   # --enable=similarities". If you want to run only the classes checker, but have
>   # no Warning level messages displayed, use "--disable=all --enable=classes
>   # --disable=W".
> -disable=too-many-arguments,
> -        too-many-instance-attributes,
> -        too-many-public-methods,
> +disable=
>   
>   [pylint.basic]
>   # Good variable names which should always be accepted, separated by a comma.

[PATCH 03/11] python: Re-lock pipenv at *oldest* supported versions

[John Snow]

tox is already testing the most recent versions. Let's use pipenv to
test the oldest versions we claim to support. This matches the stylistic
choice to have pipenv always test our oldest supported Python version, 3.6.

The effect of this is that the python-check-pipenv CI job on gitlab will
now test against much older versions of these linters, which will help
highlight incompatible changes that might otherwise go unnoticed.

Update instructions for adding and bumping versions in setup.cfg. The
reason for deleting the line that gets added to Pipfile is largely just
to avoid having the version minimums specified in multiple places in
config checked into the tree.

(This patch was written by deleting Pipfile and Pipfile.lock, then
explicitly installing each dependency manually at a specific
version. Then, I restored the prior Pipfile and re-ran `pipenv lock
--dev --keep-outdated` to re-add the qemu dependency back to the pipenv
environment while keeping the "old" packages. It's annoying, yes, but I
think the improvement to test coverage is worthwhile.)

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/Pipfile.lock | 113 +++++++++++++++++++++-----------------------
 python/setup.cfg    |   4 +-
 2 files changed, 56 insertions(+), 61 deletions(-)

diff --git a/python/Pipfile.lock b/python/Pipfile.lock
index 5bb3f1b635..8ab41a3f60 100644
--- a/python/Pipfile.lock
+++ b/python/Pipfile.lock
@@ -31,19 +31,19 @@
         },
         "astroid": {
             "hashes": [
-                "sha256:4db03ab5fc3340cf619dbc25e42c2cc3755154ce6009469766d7143d1fc2ee4e",
-                "sha256:8a398dfce302c13f14bab13e2b14fe385d32b73f4e4853b9bdfb64598baa1975"
+                "sha256:09bdb456e02564731f8b5957cdd0c98a7f01d2db5e90eb1d794c353c28bfd705",
+                "sha256:6a8a51f64dae307f6e0c9db752b66a7951e282389d8362cc1d39a56f3feeb31d"
             ],
             "markers": "python_version ~= '3.6'",
-            "version": "==2.5.6"
+            "version": "==2.6.0"
         },
         "avocado-framework": {
             "hashes": [
-                "sha256:42aa7962df98d6b78d4efd9afa2177226dc630f3d83a2a7d5baf7a0a7da7fa1b",
-                "sha256:d96ae343abf890e1ef3b3a6af5ce49e35f6bded0715770c4acb325bca555c515"
+                "sha256:3fca7226d7d164f124af8a741e7fa658ff4345a0738ddc32907631fd688b38ed",
+                "sha256:48ac254c0ae2ef0c0ceeb38e3d3df0388718eda8f48b3ab55b30b252839f42b1"
             ],
-            "markers": "python_version >= '3.6'",
-            "version": "==88.1"
+            "index": "pypi",
+            "version": "==87.0"
         },
         "distlib": {
             "hashes": [
@@ -61,25 +61,27 @@
         },
         "flake8": {
             "hashes": [
-                "sha256:07528381786f2a6237b061f6e96610a4167b226cb926e2aa2b6b1d78057c576b",
-                "sha256:bf8fd333346d844f616e8d47905ef3a3384edae6b4e9beb0c5101e25e3110907"
+                "sha256:6a35f5b8761f45c5513e3405f110a86bea57982c3b75b766ce7b65217abe1670",
+                "sha256:c01f8a3963b3571a8e6bd7a4063359aff90749e160778e03817cd9b71c9e07d2"
             ],
-            "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3, 3.4'",
-            "version": "==3.9.2"
+            "index": "pypi",
+            "version": "==3.6.0"
         },
         "fusepy": {
             "hashes": [
-                "sha256:72ff783ec2f43de3ab394e3f7457605bf04c8cf288a2f4068b4cde141d4ee6bd"
+                "sha256:10f5c7f5414241bffecdc333c4d3a725f1d6605cae6b4eaf86a838ff49cdaf6c",
+                "sha256:a9f3a3699080ddcf0919fd1eb2cf743e1f5859ca54c2018632f939bdfac269ee"
             ],
-            "version": "==3.0.1"
+            "index": "pypi",
+            "version": "==2.0.4"
         },
         "importlib-metadata": {
             "hashes": [
-                "sha256:8c501196e49fb9df5df43833bdb1e4328f64847763ec8a50703148b73784d581",
-                "sha256:d7eb1dea6d6a6086f8be21784cc9e3bcfa55872b52309bc5fad53a8ea444465d"
+                "sha256:90bb658cdbbf6d1735b6341ce708fc7024a3e14e99ffdc5783edea9f9b077f83",
+                "sha256:dc15b2969b4ce36305c51eebe62d418ac7791e9a157911d58bfb1f9ccd8e2070"
             ],
             "markers": "python_version < '3.8'",
-            "version": "==4.0.1"
+            "version": "==1.7.0"
         },
         "importlib-resources": {
             "hashes": [
@@ -91,11 +93,11 @@
         },
         "isort": {
             "hashes": [
-                "sha256:0a943902919f65c5684ac4e0154b1ad4fac6dcaa5d9f3426b732f1c8b5419be6",
-                "sha256:2bb1680aad211e3c9944dbce1d4ba09a989f04e238296c87fe2139faa26d655d"
+                "sha256:408e4d75d84f51b64d0824894afee44469eba34a4caee621dc53799f80d71ccc",
+                "sha256:64022dea6a06badfa09b300b4dfe8ba968114a737919e8ed50aea1c288f078aa"
             ],
-            "markers": "python_version >= '3.6' and python_version < '4.0'",
-            "version": "==5.8.0"
+            "index": "pypi",
+            "version": "==5.1.2"
         },
         "lazy-object-proxy": {
             "hashes": [
@@ -134,31 +136,23 @@
         },
         "mypy": {
             "hashes": [
-                "sha256:0d0a87c0e7e3a9becdfbe936c981d32e5ee0ccda3e0f07e1ef2c3d1a817cf73e",
-                "sha256:25adde9b862f8f9aac9d2d11971f226bd4c8fbaa89fb76bdadb267ef22d10064",
-                "sha256:28fb5479c494b1bab244620685e2eb3c3f988d71fd5d64cc753195e8ed53df7c",
-                "sha256:2f9b3407c58347a452fc0736861593e105139b905cca7d097e413453a1d650b4",
-                "sha256:33f159443db0829d16f0a8d83d94df3109bb6dd801975fe86bacb9bf71628e97",
-                "sha256:3f2aca7f68580dc2508289c729bd49ee929a436208d2b2b6aab15745a70a57df",
-                "sha256:499c798053cdebcaa916eef8cd733e5584b5909f789de856b482cd7d069bdad8",
-                "sha256:4eec37370483331d13514c3f55f446fc5248d6373e7029a29ecb7b7494851e7a",
-                "sha256:552a815579aa1e995f39fd05dde6cd378e191b063f031f2acfe73ce9fb7f9e56",
-                "sha256:5873888fff1c7cf5b71efbe80e0e73153fe9212fafdf8e44adfe4c20ec9f82d7",
-                "sha256:61a3d5b97955422964be6b3baf05ff2ce7f26f52c85dd88db11d5e03e146a3a6",
-                "sha256:674e822aa665b9fd75130c6c5f5ed9564a38c6cea6a6432ce47eafb68ee578c5",
-                "sha256:7ce3175801d0ae5fdfa79b4f0cfed08807af4d075b402b7e294e6aa72af9aa2a",
-                "sha256:9743c91088d396c1a5a3c9978354b61b0382b4e3c440ce83cf77994a43e8c521",
-                "sha256:9f94aac67a2045ec719ffe6111df543bac7874cee01f41928f6969756e030564",
-                "sha256:a26f8ec704e5a7423c8824d425086705e381b4f1dfdef6e3a1edab7ba174ec49",
-                "sha256:abf7e0c3cf117c44d9285cc6128856106183938c68fd4944763003decdcfeb66",
-                "sha256:b09669bcda124e83708f34a94606e01b614fa71931d356c1f1a5297ba11f110a",
-                "sha256:cd07039aa5df222037005b08fbbfd69b3ab0b0bd7a07d7906de75ae52c4e3119",
-                "sha256:d23e0ea196702d918b60c8288561e722bf437d82cb7ef2edcd98cfa38905d506",
-                "sha256:d65cc1df038ef55a99e617431f0553cd77763869eebdf9042403e16089fe746c",
-                "sha256:d7da2e1d5f558c37d6e8c1246f1aec1e7349e4913d8fb3cb289a35de573fe2eb"
+                "sha256:15b948e1302682e3682f11f50208b726a246ab4e6c1b39f9264a8796bb416aa2",
+                "sha256:219a3116ecd015f8dca7b5d2c366c973509dfb9a8fc97ef044a36e3da66144a1",
+                "sha256:3b1fc683fb204c6b4403a1ef23f0b1fac8e4477091585e0c8c54cbdf7d7bb164",
+                "sha256:3beff56b453b6ef94ecb2996bea101a08f1f8a9771d3cbf4988a61e4d9973761",
+                "sha256:7687f6455ec3ed7649d1ae574136835a4272b65b3ddcf01ab8704ac65616c5ce",
+                "sha256:7ec45a70d40ede1ec7ad7f95b3c94c9cf4c186a32f6bacb1795b60abd2f9ef27",
+                "sha256:86c857510a9b7c3104cf4cde1568f4921762c8f9842e987bc03ed4f160925754",
+                "sha256:8a627507ef9b307b46a1fea9513d5c98680ba09591253082b4c48697ba05a4ae",
+                "sha256:8dfb69fbf9f3aeed18afffb15e319ca7f8da9642336348ddd6cab2713ddcf8f9",
+                "sha256:a34b577cdf6313bf24755f7a0e3f3c326d5c1f4fe7422d1d06498eb25ad0c600",
+                "sha256:a8ffcd53cb5dfc131850851cc09f1c44689c2812d0beb954d8138d4f5fc17f65",
+                "sha256:b90928f2d9eb2f33162405f32dde9f6dcead63a0971ca8a1b50eb4ca3e35ceb8",
+                "sha256:c56ffe22faa2e51054c5f7a3bc70a370939c2ed4de308c690e7949230c995913",
+                "sha256:f91c7ae919bbc3f96cd5e5b2e786b2b108343d1d7972ea130f7de27fdd547cf3"
             ],
-            "markers": "python_version >= '3.5'",
-            "version": "==0.812"
+            "index": "pypi",
+            "version": "==0.770"
         },
         "mypy-extensions": {
             "hashes": [
@@ -193,27 +187,26 @@
         },
         "pycodestyle": {
             "hashes": [
-                "sha256:514f76d918fcc0b55c6680472f0a37970994e07bbb80725808c17089be302068",
-                "sha256:c389c1d06bf7904078ca03399a4816f974a1d590090fecea0c63ec26ebaf1cef"
+                "sha256:74abc4e221d393ea5ce1f129ea6903209940c1ecd29e002e8c6933c2b21026e0",
+                "sha256:cbc619d09254895b0d12c2c691e237b2e91e9b2ecf5e84c26b35400f93dcfb83",
+                "sha256:cbfca99bd594a10f674d0cd97a3d802a1fdef635d4361e1a2658de47ed261e3a"
             ],
-            "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3'",
-            "version": "==2.7.0"
+            "version": "==2.4.0"
         },
         "pyflakes": {
             "hashes": [
-                "sha256:7893783d01b8a89811dd72d7dfd4d84ff098e5eed95cfa8905b22bbffe52efc3",
-                "sha256:f5bc8ecabc05bb9d291eb5203d6810b49040f6ff446a756326104746cc00c1db"
+                "sha256:9a7662ec724d0120012f6e29d6248ae3727d821bba522a0e6b356eff19126a49",
+                "sha256:f661252913bc1dbe7fcfcbf0af0db3f42ab65aabd1a6ca68fe5d466bace94dae"
             ],
-            "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3'",
-            "version": "==2.3.1"
+            "version": "==2.0.0"
         },
         "pylint": {
             "hashes": [
-                "sha256:586d8fa9b1891f4b725f587ef267abe2a1bad89d6b184520c7f07a253dd6e217",
-                "sha256:f7e2072654a6b6afdf5e2fb38147d3e2d2d43c89f648637baab63e026481279b"
+                "sha256:082a6d461b54f90eea49ca90fff4ee8b6e45e8029e5dbd72f6107ef84f3779c0",
+                "sha256:a01cd675eccf6e25b3bdb42be184eb46aaf89187d612ba0fb5f93328ed6b0fd5"
             ],
-            "markers": "python_version ~= '3.6'",
-            "version": "==2.8.2"
+            "index": "pypi",
+            "version": "==2.8.0"
         },
         "pyparsing": {
             "hashes": [
@@ -245,11 +238,11 @@
         },
         "tox": {
             "hashes": [
-                "sha256:307a81ddb82bd463971a273f33e9533a24ed22185f27db8ce3386bff27d324e3",
-                "sha256:b0b5818049a1c1997599d42012a637a33f24c62ab8187223fdd318fa8522637b"
+                "sha256:c60692d92fe759f46c610ac04c03cf0169432d1ff8e981e8ae63e068d0954fc3",
+                "sha256:f179cb4043d7dc1339425dd49ab1dd8c916246b0d9173143c1b0af7498a03ab0"
             ],
-            "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3, 3.4'",
-            "version": "==3.23.1"
+            "index": "pypi",
+            "version": "==3.18.0"
         },
         "typed-ast": {
             "hashes": [
diff --git a/python/setup.cfg b/python/setup.cfg
index 524789d6e0..e730f208d3 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -32,7 +32,9 @@ packages =
 * = py.typed
 
 [options.extras_require]
-# Run `pipenv lock --dev` when changing these requirements.
+# For the devel group, When adding new dependencies or bumping the minimum
+# version, use e.g. "pipenv install --dev pylint==3.0.0".
+# Subsequently, edit 'Pipfile' to remove e.g. 'pylint = "==3.0.0'.
 devel =
     avocado-framework >= 87.0
     flake8 >= 3.6.0
-- 
2.31.1

Re: [PATCH 03/11] python: Re-lock pipenv at *oldest* supported versions

[Willian Rampazzo]

On Fri, Jun 25, 2021 at 12:46 PM John Snow <jsnow@redhat.com> wrote:
>
> tox is already testing the most recent versions. Let's use pipenv to
> test the oldest versions we claim to support. This matches the stylistic
> choice to have pipenv always test our oldest supported Python version, 3.6.
>
> The effect of this is that the python-check-pipenv CI job on gitlab will
> now test against much older versions of these linters, which will help
> highlight incompatible changes that might otherwise go unnoticed.
>
> Update instructions for adding and bumping versions in setup.cfg. The
> reason for deleting the line that gets added to Pipfile is largely just
> to avoid having the version minimums specified in multiple places in
> config checked into the tree.
>
> (This patch was written by deleting Pipfile and Pipfile.lock, then
> explicitly installing each dependency manually at a specific
> version. Then, I restored the prior Pipfile and re-ran `pipenv lock
> --dev --keep-outdated` to re-add the qemu dependency back to the pipenv
> environment while keeping the "old" packages. It's annoying, yes, but I
> think the improvement to test coverage is worthwhile.)
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  python/Pipfile.lock | 113 +++++++++++++++++++++-----------------------
>  python/setup.cfg    |   4 +-
>  2 files changed, 56 insertions(+), 61 deletions(-)
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

Re: [PATCH 03/11] python: Re-lock pipenv at *oldest* supported versions

[Wainer dos Santos Moschetta]

On 6/25/21 12:45 PM, John Snow wrote:
> tox is already testing the most recent versions. Let's use pipenv to
> test the oldest versions we claim to support. This matches the stylistic
> choice to have pipenv always test our oldest supported Python version, 3.6.
Currently tox is testing with 3.6 too. Should we remove that version 
from the test matrix?
>
> The effect of this is that the python-check-pipenv CI job on gitlab will
> now test against much older versions of these linters, which will help
> highlight incompatible changes that might otherwise go unnoticed.
>
> Update instructions for adding and bumping versions in setup.cfg. The
> reason for deleting the line that gets added to Pipfile is largely just
> to avoid having the version minimums specified in multiple places in
> config checked into the tree.
>
> (This patch was written by deleting Pipfile and Pipfile.lock, then
> explicitly installing each dependency manually at a specific
> version. Then, I restored the prior Pipfile and re-ran `pipenv lock
> --dev --keep-outdated` to re-add the qemu dependency back to the pipenv
> environment while keeping the "old" packages. It's annoying, yes, but I
> think the improvement to test coverage is worthwhile.)
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   python/Pipfile.lock | 113 +++++++++++++++++++++-----------------------
>   python/setup.cfg    |   4 +-
>   2 files changed, 56 insertions(+), 61 deletions(-)

Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

>
> diff --git a/python/Pipfile.lock b/python/Pipfile.lock
> index 5bb3f1b635..8ab41a3f60 100644
> --- a/python/Pipfile.lock
> +++ b/python/Pipfile.lock
> @@ -31,19 +31,19 @@
>           },
>           "astroid": {
>               "hashes": [
> -                "sha256:4db03ab5fc3340cf619dbc25e42c2cc3755154ce6009469766d7143d1fc2ee4e",
> -                "sha256:8a398dfce302c13f14bab13e2b14fe385d32b73f4e4853b9bdfb64598baa1975"
> +                "sha256:09bdb456e02564731f8b5957cdd0c98a7f01d2db5e90eb1d794c353c28bfd705",
> +                "sha256:6a8a51f64dae307f6e0c9db752b66a7951e282389d8362cc1d39a56f3feeb31d"
>               ],
>               "markers": "python_version ~= '3.6'",
> -            "version": "==2.5.6"
> +            "version": "==2.6.0"
>           },
>           "avocado-framework": {
>               "hashes": [
> -                "sha256:42aa7962df98d6b78d4efd9afa2177226dc630f3d83a2a7d5baf7a0a7da7fa1b",
> -                "sha256:d96ae343abf890e1ef3b3a6af5ce49e35f6bded0715770c4acb325bca555c515"
> +                "sha256:3fca7226d7d164f124af8a741e7fa658ff4345a0738ddc32907631fd688b38ed",
> +                "sha256:48ac254c0ae2ef0c0ceeb38e3d3df0388718eda8f48b3ab55b30b252839f42b1"
>               ],
> -            "markers": "python_version >= '3.6'",
> -            "version": "==88.1"
> +            "index": "pypi",
> +            "version": "==87.0"
>           },
>           "distlib": {
>               "hashes": [
> @@ -61,25 +61,27 @@
>           },
>           "flake8": {
>               "hashes": [
> -                "sha256:07528381786f2a6237b061f6e96610a4167b226cb926e2aa2b6b1d78057c576b",
> -                "sha256:bf8fd333346d844f616e8d47905ef3a3384edae6b4e9beb0c5101e25e3110907"
> +                "sha256:6a35f5b8761f45c5513e3405f110a86bea57982c3b75b766ce7b65217abe1670",
> +                "sha256:c01f8a3963b3571a8e6bd7a4063359aff90749e160778e03817cd9b71c9e07d2"
>               ],
> -            "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3, 3.4'",
> -            "version": "==3.9.2"
> +            "index": "pypi",
> +            "version": "==3.6.0"
>           },
>           "fusepy": {
>               "hashes": [
> -                "sha256:72ff783ec2f43de3ab394e3f7457605bf04c8cf288a2f4068b4cde141d4ee6bd"
> +                "sha256:10f5c7f5414241bffecdc333c4d3a725f1d6605cae6b4eaf86a838ff49cdaf6c",
> +                "sha256:a9f3a3699080ddcf0919fd1eb2cf743e1f5859ca54c2018632f939bdfac269ee"
>               ],
> -            "version": "==3.0.1"
> +            "index": "pypi",
> +            "version": "==2.0.4"
>           },
>           "importlib-metadata": {
>               "hashes": [
> -                "sha256:8c501196e49fb9df5df43833bdb1e4328f64847763ec8a50703148b73784d581",
> -                "sha256:d7eb1dea6d6a6086f8be21784cc9e3bcfa55872b52309bc5fad53a8ea444465d"
> +                "sha256:90bb658cdbbf6d1735b6341ce708fc7024a3e14e99ffdc5783edea9f9b077f83",
> +                "sha256:dc15b2969b4ce36305c51eebe62d418ac7791e9a157911d58bfb1f9ccd8e2070"
>               ],
>               "markers": "python_version < '3.8'",
> -            "version": "==4.0.1"
> +            "version": "==1.7.0"
>           },
>           "importlib-resources": {
>               "hashes": [
> @@ -91,11 +93,11 @@
>           },
>           "isort": {
>               "hashes": [
> -                "sha256:0a943902919f65c5684ac4e0154b1ad4fac6dcaa5d9f3426b732f1c8b5419be6",
> -                "sha256:2bb1680aad211e3c9944dbce1d4ba09a989f04e238296c87fe2139faa26d655d"
> +                "sha256:408e4d75d84f51b64d0824894afee44469eba34a4caee621dc53799f80d71ccc",
> +                "sha256:64022dea6a06badfa09b300b4dfe8ba968114a737919e8ed50aea1c288f078aa"
>               ],
> -            "markers": "python_version >= '3.6' and python_version < '4.0'",
> -            "version": "==5.8.0"
> +            "index": "pypi",
> +            "version": "==5.1.2"
>           },
>           "lazy-object-proxy": {
>               "hashes": [
> @@ -134,31 +136,23 @@
>           },
>           "mypy": {
>               "hashes": [
> -                "sha256:0d0a87c0e7e3a9becdfbe936c981d32e5ee0ccda3e0f07e1ef2c3d1a817cf73e",
> -                "sha256:25adde9b862f8f9aac9d2d11971f226bd4c8fbaa89fb76bdadb267ef22d10064",
> -                "sha256:28fb5479c494b1bab244620685e2eb3c3f988d71fd5d64cc753195e8ed53df7c",
> -                "sha256:2f9b3407c58347a452fc0736861593e105139b905cca7d097e413453a1d650b4",
> -                "sha256:33f159443db0829d16f0a8d83d94df3109bb6dd801975fe86bacb9bf71628e97",
> -                "sha256:3f2aca7f68580dc2508289c729bd49ee929a436208d2b2b6aab15745a70a57df",
> -                "sha256:499c798053cdebcaa916eef8cd733e5584b5909f789de856b482cd7d069bdad8",
> -                "sha256:4eec37370483331d13514c3f55f446fc5248d6373e7029a29ecb7b7494851e7a",
> -                "sha256:552a815579aa1e995f39fd05dde6cd378e191b063f031f2acfe73ce9fb7f9e56",
> -                "sha256:5873888fff1c7cf5b71efbe80e0e73153fe9212fafdf8e44adfe4c20ec9f82d7",
> -                "sha256:61a3d5b97955422964be6b3baf05ff2ce7f26f52c85dd88db11d5e03e146a3a6",
> -                "sha256:674e822aa665b9fd75130c6c5f5ed9564a38c6cea6a6432ce47eafb68ee578c5",
> -                "sha256:7ce3175801d0ae5fdfa79b4f0cfed08807af4d075b402b7e294e6aa72af9aa2a",
> -                "sha256:9743c91088d396c1a5a3c9978354b61b0382b4e3c440ce83cf77994a43e8c521",
> -                "sha256:9f94aac67a2045ec719ffe6111df543bac7874cee01f41928f6969756e030564",
> -                "sha256:a26f8ec704e5a7423c8824d425086705e381b4f1dfdef6e3a1edab7ba174ec49",
> -                "sha256:abf7e0c3cf117c44d9285cc6128856106183938c68fd4944763003decdcfeb66",
> -                "sha256:b09669bcda124e83708f34a94606e01b614fa71931d356c1f1a5297ba11f110a",
> -                "sha256:cd07039aa5df222037005b08fbbfd69b3ab0b0bd7a07d7906de75ae52c4e3119",
> -                "sha256:d23e0ea196702d918b60c8288561e722bf437d82cb7ef2edcd98cfa38905d506",
> -                "sha256:d65cc1df038ef55a99e617431f0553cd77763869eebdf9042403e16089fe746c",
> -                "sha256:d7da2e1d5f558c37d6e8c1246f1aec1e7349e4913d8fb3cb289a35de573fe2eb"
> +                "sha256:15b948e1302682e3682f11f50208b726a246ab4e6c1b39f9264a8796bb416aa2",
> +                "sha256:219a3116ecd015f8dca7b5d2c366c973509dfb9a8fc97ef044a36e3da66144a1",
> +                "sha256:3b1fc683fb204c6b4403a1ef23f0b1fac8e4477091585e0c8c54cbdf7d7bb164",
> +                "sha256:3beff56b453b6ef94ecb2996bea101a08f1f8a9771d3cbf4988a61e4d9973761",
> +                "sha256:7687f6455ec3ed7649d1ae574136835a4272b65b3ddcf01ab8704ac65616c5ce",
> +                "sha256:7ec45a70d40ede1ec7ad7f95b3c94c9cf4c186a32f6bacb1795b60abd2f9ef27",
> +                "sha256:86c857510a9b7c3104cf4cde1568f4921762c8f9842e987bc03ed4f160925754",
> +                "sha256:8a627507ef9b307b46a1fea9513d5c98680ba09591253082b4c48697ba05a4ae",
> +                "sha256:8dfb69fbf9f3aeed18afffb15e319ca7f8da9642336348ddd6cab2713ddcf8f9",
> +                "sha256:a34b577cdf6313bf24755f7a0e3f3c326d5c1f4fe7422d1d06498eb25ad0c600",
> +                "sha256:a8ffcd53cb5dfc131850851cc09f1c44689c2812d0beb954d8138d4f5fc17f65",
> +                "sha256:b90928f2d9eb2f33162405f32dde9f6dcead63a0971ca8a1b50eb4ca3e35ceb8",
> +                "sha256:c56ffe22faa2e51054c5f7a3bc70a370939c2ed4de308c690e7949230c995913",
> +                "sha256:f91c7ae919bbc3f96cd5e5b2e786b2b108343d1d7972ea130f7de27fdd547cf3"
>               ],
> -            "markers": "python_version >= '3.5'",
> -            "version": "==0.812"
> +            "index": "pypi",
> +            "version": "==0.770"
>           },
>           "mypy-extensions": {
>               "hashes": [
> @@ -193,27 +187,26 @@
>           },
>           "pycodestyle": {
>               "hashes": [
> -                "sha256:514f76d918fcc0b55c6680472f0a37970994e07bbb80725808c17089be302068",
> -                "sha256:c389c1d06bf7904078ca03399a4816f974a1d590090fecea0c63ec26ebaf1cef"
> +                "sha256:74abc4e221d393ea5ce1f129ea6903209940c1ecd29e002e8c6933c2b21026e0",
> +                "sha256:cbc619d09254895b0d12c2c691e237b2e91e9b2ecf5e84c26b35400f93dcfb83",
> +                "sha256:cbfca99bd594a10f674d0cd97a3d802a1fdef635d4361e1a2658de47ed261e3a"
>               ],
> -            "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3'",
> -            "version": "==2.7.0"
> +            "version": "==2.4.0"
>           },
>           "pyflakes": {
>               "hashes": [
> -                "sha256:7893783d01b8a89811dd72d7dfd4d84ff098e5eed95cfa8905b22bbffe52efc3",
> -                "sha256:f5bc8ecabc05bb9d291eb5203d6810b49040f6ff446a756326104746cc00c1db"
> +                "sha256:9a7662ec724d0120012f6e29d6248ae3727d821bba522a0e6b356eff19126a49",
> +                "sha256:f661252913bc1dbe7fcfcbf0af0db3f42ab65aabd1a6ca68fe5d466bace94dae"
>               ],
> -            "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3'",
> -            "version": "==2.3.1"
> +            "version": "==2.0.0"
>           },
>           "pylint": {
>               "hashes": [
> -                "sha256:586d8fa9b1891f4b725f587ef267abe2a1bad89d6b184520c7f07a253dd6e217",
> -                "sha256:f7e2072654a6b6afdf5e2fb38147d3e2d2d43c89f648637baab63e026481279b"
> +                "sha256:082a6d461b54f90eea49ca90fff4ee8b6e45e8029e5dbd72f6107ef84f3779c0",
> +                "sha256:a01cd675eccf6e25b3bdb42be184eb46aaf89187d612ba0fb5f93328ed6b0fd5"
>               ],
> -            "markers": "python_version ~= '3.6'",
> -            "version": "==2.8.2"
> +            "index": "pypi",
> +            "version": "==2.8.0"
>           },
>           "pyparsing": {
>               "hashes": [
> @@ -245,11 +238,11 @@
>           },
>           "tox": {
>               "hashes": [
> -                "sha256:307a81ddb82bd463971a273f33e9533a24ed22185f27db8ce3386bff27d324e3",
> -                "sha256:b0b5818049a1c1997599d42012a637a33f24c62ab8187223fdd318fa8522637b"
> +                "sha256:c60692d92fe759f46c610ac04c03cf0169432d1ff8e981e8ae63e068d0954fc3",
> +                "sha256:f179cb4043d7dc1339425dd49ab1dd8c916246b0d9173143c1b0af7498a03ab0"
>               ],
> -            "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3, 3.4'",
> -            "version": "==3.23.1"
> +            "index": "pypi",
> +            "version": "==3.18.0"
>           },
>           "typed-ast": {
>               "hashes": [
> diff --git a/python/setup.cfg b/python/setup.cfg
> index 524789d6e0..e730f208d3 100644
> --- a/python/setup.cfg
> +++ b/python/setup.cfg
> @@ -32,7 +32,9 @@ packages =
>   * = py.typed
>   
>   [options.extras_require]
> -# Run `pipenv lock --dev` when changing these requirements.
> +# For the devel group, When adding new dependencies or bumping the minimum
> +# version, use e.g. "pipenv install --dev pylint==3.0.0".
> +# Subsequently, edit 'Pipfile' to remove e.g. 'pylint = "==3.0.0'.
>   devel =
>       avocado-framework >= 87.0
>       flake8 >= 3.6.0

Re: [PATCH 03/11] python: Re-lock pipenv at *oldest* supported versions

[John Snow]

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

On Mon, Jun 28, 2021 at 4:31 PM Wainer dos Santos Moschetta <
wainersm@redhat.com> wrote:

>
> On 6/25/21 12:45 PM, John Snow wrote:
> > tox is already testing the most recent versions. Let's use pipenv to
> > test the oldest versions we claim to support. This matches the stylistic
> > choice to have pipenv always test our oldest supported Python version,
> 3.6.
> Currently tox is testing with 3.6 too. Should we remove that version
> from the test matrix?
>

Tox will test with 3.6, but against the very latest pylint/mypy. This test
will also use 3.6, but against the oldest pylint/mypy we claim to support
in the package metadata. I think they are both useful.

The pipenv test will make sure we don't accidentally increase our
dependency requirements without noticing. This has some value for other
QEMU developers that are used to running and managing their own linters,
such as we did for a long time. I don't want to break their environments by
accident. (It also makes sure that no 3.7+ isms sneak into the code by
accident, too. This has historically been, and I anticipate it will
continue to be, a prominent problem in the Python library.)

The tox test gives us a heads up if there are incompatible changes arriving
in the bleeding edge for pylint/mypy et al that I will need to be aware of
before they are rolled out and wind up on random developer's systems and
start causing 'make check' to fail.

I think it's worth keeping both for now, provided the run cost isn't too
great. At the moment it seems rather minimal.


> >
> > The effect of this is that the python-check-pipenv CI job on gitlab will
> > now test against much older versions of these linters, which will help
> > highlight incompatible changes that might otherwise go unnoticed.
> >
> > Update instructions for adding and bumping versions in setup.cfg. The
> > reason for deleting the line that gets added to Pipfile is largely just
> > to avoid having the version minimums specified in multiple places in
> > config checked into the tree.
> >
> > (This patch was written by deleting Pipfile and Pipfile.lock, then
> > explicitly installing each dependency manually at a specific
> > version. Then, I restored the prior Pipfile and re-ran `pipenv lock
> > --dev --keep-outdated` to re-add the qemu dependency back to the pipenv
> > environment while keeping the "old" packages. It's annoying, yes, but I
> > think the improvement to test coverage is worthwhile.)
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> >   python/Pipfile.lock | 113 +++++++++++++++++++++-----------------------
> >   python/setup.cfg    |   4 +-
> >   2 files changed, 56 insertions(+), 61 deletions(-)
>
> Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
>

Thanks!

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

Re: [PATCH 03/11] python: Re-lock pipenv at *oldest* supported versions

[Wainer dos Santos Moschetta]

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


On 6/28/21 5:38 PM, John Snow wrote:
>
>
> On Mon, Jun 28, 2021 at 4:31 PM Wainer dos Santos Moschetta 
> <wainersm@redhat.com <mailto:wainersm@redhat.com>> wrote:
>
>
>     On 6/25/21 12:45 PM, John Snow wrote:
>     > tox is already testing the most recent versions. Let's use pipenv to
>     > test the oldest versions we claim to support. This matches the
>     stylistic
>     > choice to have pipenv always test our oldest supported Python
>     version, 3.6.
>     Currently tox is testing with 3.6 too. Should we remove that version
>     from the test matrix?
>
>
> Tox will test with 3.6, but against the very latest pylint/mypy. This 
> test will also use 3.6, but against the oldest pylint/mypy we claim to 
> support in the package metadata. I think they are both useful.
>
> The pipenv test will make sure we don't accidentally increase our 
> dependency requirements without noticing. This has some value for 
> other QEMU developers that are used to running and managing their own 
> linters, such as we did for a long time. I don't want to break their 
> environments by accident. (It also makes sure that no 3.7+ isms sneak 
> into the code by accident, too. This has historically been, and I 
> anticipate it will continue to be, a prominent problem in the Python 
> library.)
>
> The tox test gives us a heads up if there are incompatible changes 
> arriving in the bleeding edge for pylint/mypy et al that I will need 
> to be aware of before they are rolled out and wind up on random 
> developer's systems and start causing 'make check' to fail.
>
> I think it's worth keeping both for now, provided the run cost isn't 
> too great. At the moment it seems rather minimal.


While I was reviewing the next patches I started to understand the tox 
and pipenv testing strategies. I also think they are useful.

Thanks for the details explanation above.

- Wainer

>     >
>     > The effect of this is that the python-check-pipenv CI job on
>     gitlab will
>     > now test against much older versions of these linters, which
>     will help
>     > highlight incompatible changes that might otherwise go unnoticed.
>     >
>     > Update instructions for adding and bumping versions in
>     setup.cfg. The
>     > reason for deleting the line that gets added to Pipfile is
>     largely just
>     > to avoid having the version minimums specified in multiple places in
>     > config checked into the tree.
>     >
>     > (This patch was written by deleting Pipfile and Pipfile.lock, then
>     > explicitly installing each dependency manually at a specific
>     > version. Then, I restored the prior Pipfile and re-ran `pipenv lock
>     > --dev --keep-outdated` to re-add the qemu dependency back to the
>     pipenv
>     > environment while keeping the "old" packages. It's annoying,
>     yes, but I
>     > think the improvement to test coverage is worthwhile.)
>     >
>     > Signed-off-by: John Snow <jsnow@redhat.com
>     <mailto:jsnow@redhat.com>>
>     > ---
>     >   python/Pipfile.lock | 113
>     +++++++++++++++++++++-----------------------
>     >   python/setup.cfg    |   4 +-
>     >   2 files changed, 56 insertions(+), 61 deletions(-)
>
>     Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com
>     <mailto:wainersm@redhat.com>>
>
>
> Thanks!

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

[PATCH 04/11] python: README.rst touchups

[John Snow]

Clarifying a few points; removing the reference to 'setuptools' because
it referenced anywhere else in this document and doesn't really provide
any useful information to a Python newcomer.

Adjusting the language elsewhere to be less ambiguous and have fewer
run-on sentences.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/README.rst | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/python/README.rst b/python/README.rst
index dcf993819d..107786ffdc 100644
--- a/python/README.rst
+++ b/python/README.rst
@@ -7,8 +7,7 @@ then by package (e.g. ``qemu/machine``, ``qemu/qmp``, etc).
 
 ``setup.py`` is used by ``pip`` to install this tooling to the current
 environment. ``setup.cfg`` provides the packaging configuration used by
-``setup.py`` in a setuptools specific format. You will generally invoke
-it by doing one of the following:
+``setup.py``. You will generally invoke it by doing one of the following:
 
 1. ``pip3 install .`` will install these packages to your current
    environment. If you are inside a virtual environment, they will
@@ -17,12 +16,13 @@ it by doing one of the following:
 
 2. ``pip3 install --user .`` will install these packages to your user's
    local python packages. If you are inside of a virtual environment,
-   this will fail; you likely want the first invocation above.
+   this will fail; you want the first invocation above.
 
-If you append the ``-e`` argument, pip will install in "editable" mode;
-which installs a version of the package that installs a forwarder
-pointing to these files, such that the package always reflects the
-latest version in your git tree.
+If you append the ``--editable`` or ``-e`` argument to either invocation
+above, pip will install in "editable" mode. This installs the package as
+a forwarder ("qemu.egg-link") that points to the source tree. In so
+doing, the installed package always reflects the latest version in your
+source tree.
 
 Installing ".[devel]" instead of "." will additionally pull in required
 packages for testing this package. They are not runtime requirements,
@@ -30,6 +30,7 @@ and are not needed to simply use these libraries.
 
 Running ``make develop`` will pull in all testing dependencies and
 install QEMU in editable mode to the current environment.
+(It is a shortcut for ``pip3 install -e .[devel]``.)
 
 See `Installing packages using pip and virtual environments
 <https://packaging.python.org/guides/installing-using-pip-and-virtual-environments/>`_
@@ -39,7 +40,7 @@ for more information.
 Files in this directory
 -----------------------
 
-- ``qemu/`` Python package source directory.
+- ``qemu/`` Python 'qemu' namespace package source directory.
 - ``tests/`` Python package tests directory.
 - ``avocado.cfg`` Configuration for the Avocado test-runner.
   Used by ``make check`` et al.
-- 
2.31.1

Re: [PATCH 04/11] python: README.rst touchups

[Willian Rampazzo]

On Fri, Jun 25, 2021 at 12:46 PM John Snow <jsnow@redhat.com> wrote:
>
> Clarifying a few points; removing the reference to 'setuptools' because
> it referenced anywhere else in this document and doesn't really provide
> any useful information to a Python newcomer.
>
> Adjusting the language elsewhere to be less ambiguous and have fewer
> run-on sentences.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  python/README.rst | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

Re: [PATCH 04/11] python: README.rst touchups

[John Snow]

On 6/25/21 2:23 PM, Willian Rampazzo wrote:
> On Fri, Jun 25, 2021 at 12:46 PM John Snow <jsnow@redhat.com> wrote:
>>
>> Clarifying a few points; removing the reference to 'setuptools' because
>> it referenced anywhere else in this document and doesn't really provide
      ^ isn't

Noticed the typo when adding your RB.

>> any useful information to a Python newcomer.
>>
>> Adjusting the language elsewhere to be less ambiguous and have fewer
>> run-on sentences.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>   python/README.rst | 17 +++++++++--------
>>   1 file changed, 9 insertions(+), 8 deletions(-)
>>
> 
> Reviewed-by: Willian Rampazzo <willianr@redhat.com>
> 

Thanks! I hope the new packaging has been (or will be?) helpful to 
avocado folks.

--js

Re: [PATCH 04/11] python: README.rst touchups

[Wainer dos Santos Moschetta]

On 6/25/21 12:45 PM, John Snow wrote:
> Clarifying a few points; removing the reference to 'setuptools' because
> it referenced anywhere else in this document and doesn't really provide
> any useful information to a Python newcomer.
>
> Adjusting the language elsewhere to be less ambiguous and have fewer
> run-on sentences.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   python/README.rst | 17 +++++++++--------
>   1 file changed, 9 insertions(+), 8 deletions(-)
Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
>
> diff --git a/python/README.rst b/python/README.rst
> index dcf993819d..107786ffdc 100644
> --- a/python/README.rst
> +++ b/python/README.rst
> @@ -7,8 +7,7 @@ then by package (e.g. ``qemu/machine``, ``qemu/qmp``, etc).
>   
>   ``setup.py`` is used by ``pip`` to install this tooling to the current
>   environment. ``setup.cfg`` provides the packaging configuration used by
> -``setup.py`` in a setuptools specific format. You will generally invoke
> -it by doing one of the following:
> +``setup.py``. You will generally invoke it by doing one of the following:
>   
>   1. ``pip3 install .`` will install these packages to your current
>      environment. If you are inside a virtual environment, they will
> @@ -17,12 +16,13 @@ it by doing one of the following:
>   
>   2. ``pip3 install --user .`` will install these packages to your user's
>      local python packages. If you are inside of a virtual environment,
> -   this will fail; you likely want the first invocation above.
> +   this will fail; you want the first invocation above.
>   
> -If you append the ``-e`` argument, pip will install in "editable" mode;
> -which installs a version of the package that installs a forwarder
> -pointing to these files, such that the package always reflects the
> -latest version in your git tree.
> +If you append the ``--editable`` or ``-e`` argument to either invocation
> +above, pip will install in "editable" mode. This installs the package as
> +a forwarder ("qemu.egg-link") that points to the source tree. In so
> +doing, the installed package always reflects the latest version in your
> +source tree.
>   
>   Installing ".[devel]" instead of "." will additionally pull in required
>   packages for testing this package. They are not runtime requirements,
> @@ -30,6 +30,7 @@ and are not needed to simply use these libraries.
>   
>   Running ``make develop`` will pull in all testing dependencies and
>   install QEMU in editable mode to the current environment.
> +(It is a shortcut for ``pip3 install -e .[devel]``.)
>   
>   See `Installing packages using pip and virtual environments
>   <https://packaging.python.org/guides/installing-using-pip-and-virtual-environments/>`_
> @@ -39,7 +40,7 @@ for more information.
>   Files in this directory
>   -----------------------
>   
> -- ``qemu/`` Python package source directory.
> +- ``qemu/`` Python 'qemu' namespace package source directory.
>   - ``tests/`` Python package tests directory.
>   - ``avocado.cfg`` Configuration for the Avocado test-runner.
>     Used by ``make check`` et al.

[PATCH 05/11] python: Add no-install usage instructions

[John Snow]

It's not encouraged, but it's legitimate to want to know how to do.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/README.rst | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/python/README.rst b/python/README.rst
index 107786ffdc..d4502fdb60 100644
--- a/python/README.rst
+++ b/python/README.rst
@@ -37,6 +37,34 @@ See `Installing packages using pip and virtual environments
 for more information.
 
 
+Using these packages without installing them
+--------------------------------------------
+
+These packages may be used without installing them first, by using one
+of two tricks:
+
+1. Set your PYTHONPATH environment variable to include this source
+   directory, e.g. ``~/src/qemu/python``. See
+   https://docs.python.org/3/using/cmdline.html#envvar-PYTHONPATH
+
+2. Inside a Python script, use ``sys.path`` to forcibly include a search
+   path prior to importing the ``qemu`` namespace. See
+   https://docs.python.org/3/library/sys.html#sys.path
+
+A strong downside to both approaches is that they generally interfere
+with static analysis tools being able to locate and analyze the code
+being imported.
+
+Package installation also normally provides executable console scripts,
+so that tools like ``qmp-shell`` are always available via $PATH. To
+invoke them without installation, you can invoke e.g.:
+
+``> PYTHONPATH=~/src/qemu/python python3 -m qemu.qmp.qmp_shell``
+
+The mappings between console script name and python module path can be
+found in ``setup.cfg``.
+
+
 Files in this directory
 -----------------------
 
-- 
2.31.1

Re: [PATCH 05/11] python: Add no-install usage instructions

[Willian Rampazzo]

On Fri, Jun 25, 2021 at 12:46 PM John Snow <jsnow@redhat.com> wrote:
>
> It's not encouraged, but it's legitimate to want to know how to do.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  python/README.rst | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

Re: [PATCH 05/11] python: Add no-install usage instructions

[Wainer dos Santos Moschetta]

On 6/25/21 12:45 PM, John Snow wrote:
> It's not encouraged, but it's legitimate to want to know how to do.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   python/README.rst | 28 ++++++++++++++++++++++++++++
>   1 file changed, 28 insertions(+)
Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
>
> diff --git a/python/README.rst b/python/README.rst
> index 107786ffdc..d4502fdb60 100644
> --- a/python/README.rst
> +++ b/python/README.rst
> @@ -37,6 +37,34 @@ See `Installing packages using pip and virtual environments
>   for more information.
>   
>   
> +Using these packages without installing them
> +--------------------------------------------
> +
> +These packages may be used without installing them first, by using one
> +of two tricks:
> +
> +1. Set your PYTHONPATH environment variable to include this source
> +   directory, e.g. ``~/src/qemu/python``. See
> +   https://docs.python.org/3/using/cmdline.html#envvar-PYTHONPATH
> +
> +2. Inside a Python script, use ``sys.path`` to forcibly include a search
> +   path prior to importing the ``qemu`` namespace. See
> +   https://docs.python.org/3/library/sys.html#sys.path
> +
> +A strong downside to both approaches is that they generally interfere
> +with static analysis tools being able to locate and analyze the code
> +being imported.
> +
> +Package installation also normally provides executable console scripts,
> +so that tools like ``qmp-shell`` are always available via $PATH. To
> +invoke them without installation, you can invoke e.g.:
> +
> +``> PYTHONPATH=~/src/qemu/python python3 -m qemu.qmp.qmp_shell``
> +
> +The mappings between console script name and python module path can be
> +found in ``setup.cfg``.
> +
> +
>   Files in this directory
>   -----------------------
>   

[PATCH 06/11] python: rename 'venv-check' target to 'check-pipenv'

[John Snow]

Well, Cleber was right, this is a better name.

In preparation for adding a different kind of virtual environment check
(One that simply uses whichever version of Python you happen to have),
rename this test 'check-pipenv' so that it matches the CI job
'check-python-pipenv'.

Remove the "If you don't know which test to run" hint, because it's not
actually likely you have Python 3.6 installed to be able to run the
test. It's still the test I'd most prefer you to run, but it's not the
test you are most likely to be able to run.

Rename the 'venv' target to 'pipenv' as well, and move the more
pertinent help text under the 'check-pipenv' target.
Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/README.rst              |  2 +-
 .gitlab-ci.d/static_checks.yml |  2 +-
 python/Makefile                | 21 +++++++++++----------
 3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/python/README.rst b/python/README.rst
index d4502fdb60..9c1fceaee7 100644
--- a/python/README.rst
+++ b/python/README.rst
@@ -79,7 +79,7 @@ Files in this directory
 - ``PACKAGE.rst`` is used as the README file that is visible on PyPI.org.
 - ``Pipfile`` is used by Pipenv to generate ``Pipfile.lock``.
 - ``Pipfile.lock`` is a set of pinned package dependencies that this package
-  is tested under in our CI suite. It is used by ``make venv-check``.
+  is tested under in our CI suite. It is used by ``make check-pipenv``.
 - ``README.rst`` you are here!
 - ``VERSION`` contains the PEP-440 compliant version used to describe
   this package; it is referenced by ``setup.cfg``.
diff --git a/.gitlab-ci.d/static_checks.yml b/.gitlab-ci.d/static_checks.yml
index 7e685c6a65..8c7dc41abd 100644
--- a/.gitlab-ci.d/static_checks.yml
+++ b/.gitlab-ci.d/static_checks.yml
@@ -33,7 +33,7 @@ check-python-pipenv:
   stage: test
   image: $CI_REGISTRY_IMAGE/qemu/python:latest
   script:
-    - make -C python venv-check
+    - make -C python check-pipenv
   variables:
     GIT_DEPTH: 1
   needs:
diff --git a/python/Makefile b/python/Makefile
index b5621b0d54..07ad73ccd0 100644
--- a/python/Makefile
+++ b/python/Makefile
@@ -1,15 +1,13 @@
-.PHONY: help venv venv-check check clean distclean develop
+.PHONY: help pipenv check-pipenv check clean distclean develop
 
 help:
 	@echo "python packaging help:"
 	@echo ""
-	@echo "make venv:       Create pipenv's virtual environment."
-	@echo "    NOTE: Requires Python 3.6 and pipenv."
-	@echo "          Will download packages from PyPI."
-	@echo "    Hint: (On Fedora): 'sudo dnf install python36 pipenv'"
-	@echo ""
-	@echo "make venv-check: run linters using pipenv's virtual environment."
-	@echo "    Hint: If you don't know which test to run, run this one!"
+	@echo "make check-pipenv:"
+	@echo "    Run tests in pipenv's virtual environment."
+	@echo "    These tests use the oldest dependencies."
+	@echo "    Requires: Python 3.6 and pipenv."
+	@echo "    Hint (Fedora): 'sudo dnf install python3.6 pipenv'"
 	@echo ""
 	@echo "make develop:    Install deps for 'make check', and"
 	@echo "                 the qemu libs in editable/development mode."
@@ -18,18 +16,21 @@ help:
 	@echo ""
 	@echo "make check-tox:  run linters using multiple python versions."
 	@echo ""
+	@echo "make pipenv"
+	@echo "    Creates pipenv's virtual environment (.venv)"
+	@echo ""
 	@echo "make clean:      remove package build output."
 	@echo ""
 	@echo "make distclean:  remove venv files, qemu package forwarder,"
 	@echo "                 built distribution files, and everything"
 	@echo "                 from 'make clean'."
 
-venv: .venv
+pipenv: .venv
 .venv: Pipfile.lock
 	@PIPENV_VENV_IN_PROJECT=1 pipenv sync --dev --keep-outdated
 	@touch .venv
 
-venv-check: venv
+check-pipenv: pipenv
 	@pipenv run make check
 
 develop:
-- 
2.31.1

Re: [PATCH 06/11] python: rename 'venv-check' target to 'check-pipenv'

[Willian Rampazzo]

On Fri, Jun 25, 2021 at 12:46 PM John Snow <jsnow@redhat.com> wrote:
>
> Well, Cleber was right, this is a better name.
>
> In preparation for adding a different kind of virtual environment check
> (One that simply uses whichever version of Python you happen to have),
> rename this test 'check-pipenv' so that it matches the CI job
> 'check-python-pipenv'.
>
> Remove the "If you don't know which test to run" hint, because it's not
> actually likely you have Python 3.6 installed to be able to run the
> test. It's still the test I'd most prefer you to run, but it's not the
> test you are most likely to be able to run.
>
> Rename the 'venv' target to 'pipenv' as well, and move the more
> pertinent help text under the 'check-pipenv' target.
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  python/README.rst              |  2 +-
>  .gitlab-ci.d/static_checks.yml |  2 +-
>  python/Makefile                | 21 +++++++++++----------
>  3 files changed, 13 insertions(+), 12 deletions(-)
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

Re: [PATCH 06/11] python: rename 'venv-check' target to 'check-pipenv'

[Wainer dos Santos Moschetta]

On 6/25/21 12:45 PM, John Snow wrote:
> Well, Cleber was right, this is a better name.
>
> In preparation for adding a different kind of virtual environment check
> (One that simply uses whichever version of Python you happen to have),
> rename this test 'check-pipenv' so that it matches the CI job
> 'check-python-pipenv'.
>
> Remove the "If you don't know which test to run" hint, because it's not
> actually likely you have Python 3.6 installed to be able to run the
> test. It's still the test I'd most prefer you to run, but it's not the
> test you are most likely to be able to run.
>
> Rename the 'venv' target to 'pipenv' as well, and move the more
> pertinent help text under the 'check-pipenv' target.
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   python/README.rst              |  2 +-
>   .gitlab-ci.d/static_checks.yml |  2 +-
>   python/Makefile                | 21 +++++++++++----------
>   3 files changed, 13 insertions(+), 12 deletions(-)

I also agree pipenv/check-pipenvit is a better name.

Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

>
> diff --git a/python/README.rst b/python/README.rst
> index d4502fdb60..9c1fceaee7 100644
> --- a/python/README.rst
> +++ b/python/README.rst
> @@ -79,7 +79,7 @@ Files in this directory
>   - ``PACKAGE.rst`` is used as the README file that is visible on PyPI.org.
>   - ``Pipfile`` is used by Pipenv to generate ``Pipfile.lock``.
>   - ``Pipfile.lock`` is a set of pinned package dependencies that this package
> -  is tested under in our CI suite. It is used by ``make venv-check``.
> +  is tested under in our CI suite. It is used by ``make check-pipenv``.
>   - ``README.rst`` you are here!
>   - ``VERSION`` contains the PEP-440 compliant version used to describe
>     this package; it is referenced by ``setup.cfg``.
> diff --git a/.gitlab-ci.d/static_checks.yml b/.gitlab-ci.d/static_checks.yml
> index 7e685c6a65..8c7dc41abd 100644
> --- a/.gitlab-ci.d/static_checks.yml
> +++ b/.gitlab-ci.d/static_checks.yml
> @@ -33,7 +33,7 @@ check-python-pipenv:
>     stage: test
>     image: $CI_REGISTRY_IMAGE/qemu/python:latest
>     script:
> -    - make -C python venv-check
> +    - make -C python check-pipenv
>     variables:
>       GIT_DEPTH: 1
>     needs:
> diff --git a/python/Makefile b/python/Makefile
> index b5621b0d54..07ad73ccd0 100644
> --- a/python/Makefile
> +++ b/python/Makefile
> @@ -1,15 +1,13 @@
> -.PHONY: help venv venv-check check clean distclean develop
> +.PHONY: help pipenv check-pipenv check clean distclean develop
>   
>   help:
>   	@echo "python packaging help:"
>   	@echo ""
> -	@echo "make venv:       Create pipenv's virtual environment."
> -	@echo "    NOTE: Requires Python 3.6 and pipenv."
> -	@echo "          Will download packages from PyPI."
> -	@echo "    Hint: (On Fedora): 'sudo dnf install python36 pipenv'"
> -	@echo ""
> -	@echo "make venv-check: run linters using pipenv's virtual environment."
> -	@echo "    Hint: If you don't know which test to run, run this one!"
> +	@echo "make check-pipenv:"
> +	@echo "    Run tests in pipenv's virtual environment."
> +	@echo "    These tests use the oldest dependencies."
> +	@echo "    Requires: Python 3.6 and pipenv."
> +	@echo "    Hint (Fedora): 'sudo dnf install python3.6 pipenv'"
>   	@echo ""
>   	@echo "make develop:    Install deps for 'make check', and"
>   	@echo "                 the qemu libs in editable/development mode."
> @@ -18,18 +16,21 @@ help:
>   	@echo ""
>   	@echo "make check-tox:  run linters using multiple python versions."
>   	@echo ""
> +	@echo "make pipenv"
> +	@echo "    Creates pipenv's virtual environment (.venv)"
> +	@echo ""
>   	@echo "make clean:      remove package build output."
>   	@echo ""
>   	@echo "make distclean:  remove venv files, qemu package forwarder,"
>   	@echo "                 built distribution files, and everything"
>   	@echo "                 from 'make clean'."
>   
> -venv: .venv
> +pipenv: .venv
>   .venv: Pipfile.lock
>   	@PIPENV_VENV_IN_PROJECT=1 pipenv sync --dev --keep-outdated
>   	@touch .venv
>   
> -venv-check: venv
> +check-pipenv: pipenv
>   	@pipenv run make check
>   
>   develop:

[PATCH 07/11] python: update help text for check-tox

[John Snow]

Move it up near the check-pipenv help text, and update it to suggest parity.

(At the time I first added it, I wasn't sure if I would be keeping it,
but I've come to appreciate it as it has actually helped uncover bugs I
would not have noticed without it. It should stay.)
Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/Makefile | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/python/Makefile b/python/Makefile
index 07ad73ccd0..5cb8378b81 100644
--- a/python/Makefile
+++ b/python/Makefile
@@ -9,13 +9,17 @@ help:
 	@echo "    Requires: Python 3.6 and pipenv."
 	@echo "    Hint (Fedora): 'sudo dnf install python3.6 pipenv'"
 	@echo ""
+	@echo "make check-tox:"
+	@echo "    Run tests against multiple python versions."
+	@echo "    These tests use the newest dependencies."
+	@echo "    Requires: Python 3.6-3.10 and tox."
+	@echo "    Hint (Fedora): 'sudo dnf install python3-tox python3.10'"
+	@echo ""
 	@echo "make develop:    Install deps for 'make check', and"
 	@echo "                 the qemu libs in editable/development mode."
 	@echo ""
 	@echo "make check:      run linters using the current environment."
 	@echo ""
-	@echo "make check-tox:  run linters using multiple python versions."
-	@echo ""
 	@echo "make pipenv"
 	@echo "    Creates pipenv's virtual environment (.venv)"
 	@echo ""
-- 
2.31.1

Re: [PATCH 07/11] python: update help text for check-tox

[Willian Rampazzo]

On Fri, Jun 25, 2021 at 12:46 PM John Snow <jsnow@redhat.com> wrote:
>
> Move it up near the check-pipenv help text, and update it to suggest parity.
>
> (At the time I first added it, I wasn't sure if I would be keeping it,
> but I've come to appreciate it as it has actually helped uncover bugs I
> would not have noticed without it. It should stay.)
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  python/Makefile | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

Re: [PATCH 07/11] python: update help text for check-tox

[Wainer dos Santos Moschetta]

Hi,

On 6/25/21 12:45 PM, John Snow wrote:
> Move it up near the check-pipenv help text, and update it to suggest parity.
>
> (At the time I first added it, I wasn't sure if I would be keeping it,
> but I've come to appreciate it as it has actually helped uncover bugs I
> would not have noticed without it. It should stay.)
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   python/Makefile | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/python/Makefile b/python/Makefile
> index 07ad73ccd0..5cb8378b81 100644
> --- a/python/Makefile
> +++ b/python/Makefile
> @@ -9,13 +9,17 @@ help:
>   	@echo "    Requires: Python 3.6 and pipenv."
>   	@echo "    Hint (Fedora): 'sudo dnf install python3.6 pipenv'"
>   	@echo ""
> +	@echo "make check-tox:"
> +	@echo "    Run tests against multiple python versions."
> +	@echo "    These tests use the newest dependencies."
> +	@echo "    Requires: Python 3.6-3.10 and tox."

Nit: I'm not sure it is clear '3.6-3.10' is a range rather than a 
specific version.

- Wainer

> +	@echo "    Hint (Fedora): 'sudo dnf install python3-tox python3.10'"
> +	@echo ""
>   	@echo "make develop:    Install deps for 'make check', and"
>   	@echo "                 the qemu libs in editable/development mode."
>   	@echo ""
>   	@echo "make check:      run linters using the current environment."
>   	@echo ""
> -	@echo "make check-tox:  run linters using multiple python versions."
> -	@echo ""
>   	@echo "make pipenv"
>   	@echo "    Creates pipenv's virtual environment (.venv)"
>   	@echo ""

Re: [PATCH 07/11] python: update help text for check-tox

[John Snow]

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

On Mon, Jun 28, 2021 at 5:16 PM Wainer dos Santos Moschetta <
wainersm@redhat.com> wrote:

> Hi,
>
> On 6/25/21 12:45 PM, John Snow wrote:
> > Move it up near the check-pipenv help text, and update it to suggest
> parity.
> >
> > (At the time I first added it, I wasn't sure if I would be keeping it,
> > but I've come to appreciate it as it has actually helped uncover bugs I
> > would not have noticed without it. It should stay.)
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> >   python/Makefile | 8 ++++++--
> >   1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/python/Makefile b/python/Makefile
> > index 07ad73ccd0..5cb8378b81 100644
> > --- a/python/Makefile
> > +++ b/python/Makefile
> > @@ -9,13 +9,17 @@ help:
> >       @echo "    Requires: Python 3.6 and pipenv."
> >       @echo "    Hint (Fedora): 'sudo dnf install python3.6 pipenv'"
> >       @echo ""
> > +     @echo "make check-tox:"
> > +     @echo "    Run tests against multiple python versions."
> > +     @echo "    These tests use the newest dependencies."
> > +     @echo "    Requires: Python 3.6-3.10 and tox."
>
> Nit: I'm not sure it is clear '3.6-3.10' is a range rather than a
> specific version.
>

It was meant to be a range. Would you prefer "3.6 - 3.10" instead, then?


>
>
- Wainer
>

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

Re: [PATCH 07/11] python: update help text for check-tox

[Wainer dos Santos Moschetta]

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


On 6/29/21 12:40 PM, John Snow wrote:
>
>
> On Mon, Jun 28, 2021 at 5:16 PM Wainer dos Santos Moschetta 
> <wainersm@redhat.com <mailto:wainersm@redhat.com>> wrote:
>
>     Hi,
>
>     On 6/25/21 12:45 PM, John Snow wrote:
>     > Move it up near the check-pipenv help text, and update it to
>     suggest parity.
>     >
>     > (At the time I first added it, I wasn't sure if I would be
>     keeping it,
>     > but I've come to appreciate it as it has actually helped uncover
>     bugs I
>     > would not have noticed without it. It should stay.)
>     > Signed-off-by: John Snow <jsnow@redhat.com
>     <mailto:jsnow@redhat.com>>
>     > ---
>     >   python/Makefile | 8 ++++++--
>     >   1 file changed, 6 insertions(+), 2 deletions(-)
>     >
>     > diff --git a/python/Makefile b/python/Makefile
>     > index 07ad73ccd0..5cb8378b81 100644
>     > --- a/python/Makefile
>     > +++ b/python/Makefile
>     > @@ -9,13 +9,17 @@ help:
>     >       @echo "    Requires: Python 3.6 and pipenv."
>     >       @echo "    Hint (Fedora): 'sudo dnf install python3.6 pipenv'"
>     >       @echo ""
>     > +     @echo "make check-tox:"
>     > +     @echo "    Run tests against multiple python versions."
>     > +     @echo "    These tests use the newest dependencies."
>     > +     @echo "    Requires: Python 3.6-3.10 and tox."
>
>     Nit: I'm not sure it is clear '3.6-3.10' is a range rather than a
>     specific version.
>
>
> It was meant to be a range. Would you prefer "3.6 - 3.10" instead, then?

I think it's better.

- Wainer

>
>     - Wainer
>

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

[PATCH 08/11] python: add 'make check-venv' invocation

[John Snow]

This is a *third* way to run the Python tests. Unlike the first two
(check-pipenv, check-tox), this version does not require any specific
interpreter version -- making it a lot easier to tell people to run it
as a quick smoketest prior to submission to GitLab CI.

Summary:

  Checked via GitLab CI:
    - check-pipenv: tests our oldest python & dependencies
    - check-tox: tests newest dependencies on all non-EOL python versions
  Executed only incidentally:
    - check-venv: tests newest dependencies on whichever python version

('make check' does not set up any environment at all, it just runs the
tests in your current environment. All four invocations perform the
exact same tests, just in different execution environments.)

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/Makefile | 35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/python/Makefile b/python/Makefile
index 5cb8378b81..76bb24e671 100644
--- a/python/Makefile
+++ b/python/Makefile
@@ -1,4 +1,6 @@
-.PHONY: help pipenv check-pipenv check clean distclean develop
+.PHONY: help pipenv venv check-venv check-pipenv check clean distclean develop
+
+QEMU_VENV_DIR=~/.cache/qemu-pyvenv
 
 help:
 	@echo "python packaging help:"
@@ -15,6 +17,11 @@ help:
 	@echo "    Requires: Python 3.6-3.10 and tox."
 	@echo "    Hint (Fedora): 'sudo dnf install python3-tox python3.10'"
 	@echo ""
+	@echo "make check-venv:"
+	@echo "    Run tests in a venv against your default python3 version."
+	@echo "    These tests use the newest dependencies."
+	@echo "    Requires: Python 3.x"
+	@echo ""
 	@echo "make develop:    Install deps for 'make check', and"
 	@echo "                 the qemu libs in editable/development mode."
 	@echo ""
@@ -23,6 +30,9 @@ help:
 	@echo "make pipenv"
 	@echo "    Creates pipenv's virtual environment (.venv)"
 	@echo ""
+	@echo "make venv"
+	@echo "    Creates a simple venv for check-venv. ($(QEMU_VENV_DIR))"
+	@echo ""
 	@echo "make clean:      remove package build output."
 	@echo ""
 	@echo "make distclean:  remove venv files, qemu package forwarder,"
@@ -37,8 +47,27 @@ pipenv: .venv
 check-pipenv: pipenv
 	@pipenv run make check
 
+venv: $(QEMU_VENV_DIR) $(QEMU_VENV_DIR)/bin/activate
+$(QEMU_VENV_DIR) $(QEMU_VENV_DIR)/bin/activate: setup.cfg
+	@echo "VENV $(QEMU_VENV_DIR)"
+	@python3 -m venv $(QEMU_VENV_DIR)
+	@(							\
+		echo "ACTIVATE $(QEMU_VENV_DIR)";		\
+		. $(QEMU_VENV_DIR)/bin/activate;		\
+		echo "INSTALL qemu[devel] $(QEMU_VENV_DIR)";	\
+		make develop 1>/dev/null;			\
+	)
+	@touch $(QEMU_VENV_DIR)
+
+check-venv: venv
+	@(							\
+		echo "ACTIVATE $(QEMU_VENV_DIR)";		\
+		. $(QEMU_VENV_DIR)/bin/activate;		\
+		make check;					\
+	)
+
 develop:
-	pip3 install -e .[devel]
+	pip3 install --disable-pip-version-check -e .[devel]
 
 check:
 	@avocado --config avocado.cfg run tests/
@@ -50,4 +79,4 @@ clean:
 	python3 setup.py clean --all
 
 distclean: clean
-	rm -rf qemu.egg-info/ .venv/ .tox/ dist/
+	rm -rf qemu.egg-info/ .venv/ .tox/ $(QEMU_VENV_DIR) dist/
-- 
2.31.1

Re: [PATCH 08/11] python: add 'make check-venv' invocation

[Willian Rampazzo]

On Fri, Jun 25, 2021 at 12:46 PM John Snow <jsnow@redhat.com> wrote:
>
> This is a *third* way to run the Python tests. Unlike the first two
> (check-pipenv, check-tox), this version does not require any specific
> interpreter version -- making it a lot easier to tell people to run it
> as a quick smoketest prior to submission to GitLab CI.
>
> Summary:
>
>   Checked via GitLab CI:
>     - check-pipenv: tests our oldest python & dependencies
>     - check-tox: tests newest dependencies on all non-EOL python versions
>   Executed only incidentally:
>     - check-venv: tests newest dependencies on whichever python version
>
> ('make check' does not set up any environment at all, it just runs the
> tests in your current environment. All four invocations perform the
> exact same tests, just in different execution environments.)
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  python/Makefile | 35 ++++++++++++++++++++++++++++++++---
>  1 file changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/python/Makefile b/python/Makefile
> index 5cb8378b81..76bb24e671 100644
> --- a/python/Makefile
> +++ b/python/Makefile
> @@ -1,4 +1,6 @@
> -.PHONY: help pipenv check-pipenv check clean distclean develop
> +.PHONY: help pipenv venv check-venv check-pipenv check clean distclean develop
> +
> +QEMU_VENV_DIR=~/.cache/qemu-pyvenv
>
>  help:
>         @echo "python packaging help:"
> @@ -15,6 +17,11 @@ help:
>         @echo "    Requires: Python 3.6-3.10 and tox."
>         @echo "    Hint (Fedora): 'sudo dnf install python3-tox python3.10'"
>         @echo ""
> +       @echo "make check-venv:"

Maybe, it may confuse people using `make check-venv` under `tests`.
Anyway, I'm not opposed to it.

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

> +       @echo "    Run tests in a venv against your default python3 version."
> +       @echo "    These tests use the newest dependencies."
> +       @echo "    Requires: Python 3.x"
> +       @echo ""
>         @echo "make develop:    Install deps for 'make check', and"
>         @echo "                 the qemu libs in editable/development mode."
>         @echo ""
> @@ -23,6 +30,9 @@ help:
>         @echo "make pipenv"
>         @echo "    Creates pipenv's virtual environment (.venv)"
>         @echo ""
> +       @echo "make venv"
> +       @echo "    Creates a simple venv for check-venv. ($(QEMU_VENV_DIR))"
> +       @echo ""
>         @echo "make clean:      remove package build output."
>         @echo ""
>         @echo "make distclean:  remove venv files, qemu package forwarder,"
> @@ -37,8 +47,27 @@ pipenv: .venv
>  check-pipenv: pipenv
>         @pipenv run make check
>
> +venv: $(QEMU_VENV_DIR) $(QEMU_VENV_DIR)/bin/activate
> +$(QEMU_VENV_DIR) $(QEMU_VENV_DIR)/bin/activate: setup.cfg
> +       @echo "VENV $(QEMU_VENV_DIR)"
> +       @python3 -m venv $(QEMU_VENV_DIR)
> +       @(                                                      \
> +               echo "ACTIVATE $(QEMU_VENV_DIR)";               \
> +               . $(QEMU_VENV_DIR)/bin/activate;                \
> +               echo "INSTALL qemu[devel] $(QEMU_VENV_DIR)";    \
> +               make develop 1>/dev/null;                       \
> +       )
> +       @touch $(QEMU_VENV_DIR)
> +
> +check-venv: venv
> +       @(                                                      \
> +               echo "ACTIVATE $(QEMU_VENV_DIR)";               \
> +               . $(QEMU_VENV_DIR)/bin/activate;                \
> +               make check;                                     \
> +       )
> +
>  develop:
> -       pip3 install -e .[devel]
> +       pip3 install --disable-pip-version-check -e .[devel]
>
>  check:
>         @avocado --config avocado.cfg run tests/
> @@ -50,4 +79,4 @@ clean:
>         python3 setup.py clean --all
>
>  distclean: clean
> -       rm -rf qemu.egg-info/ .venv/ .tox/ dist/
> +       rm -rf qemu.egg-info/ .venv/ .tox/ $(QEMU_VENV_DIR) dist/
> --
> 2.31.1
>

Re: [PATCH 08/11] python: add 'make check-venv' invocation

[John Snow]

On 6/25/21 2:36 PM, Willian Rampazzo wrote:
> Maybe, it may confuse people using `make check-venv` under `tests`.
> Anyway, I'm not opposed to it.
> 
> Reviewed-by: Willian Rampazzo<willianr@redhat.com>

I have to admit there's much about Python packaging that is confusing :)

Can you elaborate on your point for me, though?

--js

Re: [PATCH 08/11] python: add 'make check-venv' invocation

[Willian Rampazzo]

On Fri, Jun 25, 2021 at 3:38 PM John Snow <jsnow@redhat.com> wrote:
>
> On 6/25/21 2:36 PM, Willian Rampazzo wrote:
> > Maybe, it may confuse people using `make check-venv` under `tests`.
> > Anyway, I'm not opposed to it.
> >
> > Reviewed-by: Willian Rampazzo<willianr@redhat.com>
>
> I have to admit there's much about Python packaging that is confusing :)
>

Oh, the comment was not so related to packaging, but to QEMU itself.

> Can you elaborate on your point for me, though?

Under the `tests` folder, `make check-venv` creates the Python venv
for the tests. It does not run the tests. The `make check-venv` under
the `pyhton` folder proposed here will actually run the tests in the
venv. My comment was related to people already used to the behavior of
`make` under the `tests` folder.

I don't think it is this patch fault and I think this makes more sense
than what we currently have under the `tests` folder. Maybe it is just
a matter of organizing the `tests` folder `make` command.

>
> --js
>

Re: [PATCH 08/11] python: add 'make check-venv' invocation

[John Snow]

On 6/25/21 3:01 PM, Willian Rampazzo wrote:
> On Fri, Jun 25, 2021 at 3:38 PM John Snow <jsnow@redhat.com> wrote:
>>
>> On 6/25/21 2:36 PM, Willian Rampazzo wrote:
>>> Maybe, it may confuse people using `make check-venv` under `tests`.
>>> Anyway, I'm not opposed to it.
>>>
>>> Reviewed-by: Willian Rampazzo<willianr@redhat.com>
>>
>> I have to admit there's much about Python packaging that is confusing :)
>>
> 
> Oh, the comment was not so related to packaging, but to QEMU itself.
> 
>> Can you elaborate on your point for me, though?
> 
> Under the `tests` folder, `make check-venv` creates the Python venv
> for the tests. It does not run the tests. The `make check-venv` under
> the `pyhton` folder proposed here will actually run the tests in the
> venv. My comment was related to people already used to the behavior of
> `make` under the `tests` folder.
> 
> I don't think it is this patch fault and I think this makes more sense
> than what we currently have under the `tests` folder. Maybe it is just
> a matter of organizing the `tests` folder `make` command.
> 

OH, I see what you're saying. It's the ambiguity between:

1. make [the] 'check' venv
2. make check, [with the] venv

I chose one semantic and tests/ chose another. Valid concern.

I suppose if someone does get it mixed up it won't hurt too much though, 
they'll certainly notice pretty quickly that 'check-venv' runs tests.

Cleber, my #1 co-maintainer, do you have any preferences here?

--js

Re: [PATCH 08/11] python: add 'make check-venv' invocation

[Wainer dos Santos Moschetta]

On 6/25/21 12:45 PM, John Snow wrote:
> This is a *third* way to run the Python tests. Unlike the first two
> (check-pipenv, check-tox), this version does not require any specific
> interpreter version -- making it a lot easier to tell people to run it
> as a quick smoketest prior to submission to GitLab CI.
>
> Summary:
>
>    Checked via GitLab CI:
>      - check-pipenv: tests our oldest python & dependencies
>      - check-tox: tests newest dependencies on all non-EOL python versions
>    Executed only incidentally:
>      - check-venv: tests newest dependencies on whichever python version
>
> ('make check' does not set up any environment at all, it just runs the
> tests in your current environment. All four invocations perform the
> exact same tests, just in different execution environments.)
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   python/Makefile | 35 ++++++++++++++++++++++++++++++++---
>   1 file changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/python/Makefile b/python/Makefile
> index 5cb8378b81..76bb24e671 100644
> --- a/python/Makefile
> +++ b/python/Makefile
> @@ -1,4 +1,6 @@
> -.PHONY: help pipenv check-pipenv check clean distclean develop
> +.PHONY: help pipenv venv check-venv check-pipenv check clean distclean develop
> +
btw, check-tox is missed here ^
> +QEMU_VENV_DIR=~/.cache/qemu-pyvenv

A few suggestions:

1. For the sake of consistence with others temporary directories 
created, use QEMU_VENV_DIR=<path-to-qemu-src>/.devvenv

2. Reword to 'devvenv' or 'dev-venv' (or something similar), instead of 
'venv', the directories and Make targets. IMHO it will make the purpose 
of the targets a bit clear.

What do you think John?

- Wainer

>   
>   help:
>   	@echo "python packaging help:"
> @@ -15,6 +17,11 @@ help:
>   	@echo "    Requires: Python 3.6-3.10 and tox."
>   	@echo "    Hint (Fedora): 'sudo dnf install python3-tox python3.10'"
>   	@echo ""
> +	@echo "make check-venv:"
> +	@echo "    Run tests in a venv against your default python3 version."
> +	@echo "    These tests use the newest dependencies."
> +	@echo "    Requires: Python 3.x"
> +	@echo ""
>   	@echo "make develop:    Install deps for 'make check', and"
>   	@echo "                 the qemu libs in editable/development mode."
>   	@echo ""
> @@ -23,6 +30,9 @@ help:
>   	@echo "make pipenv"
>   	@echo "    Creates pipenv's virtual environment (.venv)"
>   	@echo ""
> +	@echo "make venv"
> +	@echo "    Creates a simple venv for check-venv. ($(QEMU_VENV_DIR))"
> +	@echo ""
>   	@echo "make clean:      remove package build output."
>   	@echo ""
>   	@echo "make distclean:  remove venv files, qemu package forwarder,"
> @@ -37,8 +47,27 @@ pipenv: .venv
>   check-pipenv: pipenv
>   	@pipenv run make check
>   
> +venv: $(QEMU_VENV_DIR) $(QEMU_VENV_DIR)/bin/activate
> +$(QEMU_VENV_DIR) $(QEMU_VENV_DIR)/bin/activate: setup.cfg
> +	@echo "VENV $(QEMU_VENV_DIR)"
> +	@python3 -m venv $(QEMU_VENV_DIR)
> +	@(							\
> +		echo "ACTIVATE $(QEMU_VENV_DIR)";		\
> +		. $(QEMU_VENV_DIR)/bin/activate;		\
> +		echo "INSTALL qemu[devel] $(QEMU_VENV_DIR)";	\
> +		make develop 1>/dev/null;			\
> +	)
> +	@touch $(QEMU_VENV_DIR)
> +
> +check-venv: venv
> +	@(							\
> +		echo "ACTIVATE $(QEMU_VENV_DIR)";		\
> +		. $(QEMU_VENV_DIR)/bin/activate;		\
> +		make check;					\
> +	)
> +
>   develop:
> -	pip3 install -e .[devel]
> +	pip3 install --disable-pip-version-check -e .[devel]
>   
>   check:
>   	@avocado --config avocado.cfg run tests/
> @@ -50,4 +79,4 @@ clean:
>   	python3 setup.py clean --all
>   
>   distclean: clean
> -	rm -rf qemu.egg-info/ .venv/ .tox/ dist/
> +	rm -rf qemu.egg-info/ .venv/ .tox/ $(QEMU_VENV_DIR) dist/

Re: [PATCH 08/11] python: add 'make check-venv' invocation

[John Snow]

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

On Mon, Jun 28, 2021 at 5:40 PM Wainer dos Santos Moschetta <
wainersm@redhat.com> wrote:

>
> On 6/25/21 12:45 PM, John Snow wrote:
> > This is a *third* way to run the Python tests. Unlike the first two
> > (check-pipenv, check-tox), this version does not require any specific
> > interpreter version -- making it a lot easier to tell people to run it
> > as a quick smoketest prior to submission to GitLab CI.
> >
> > Summary:
> >
> >    Checked via GitLab CI:
> >      - check-pipenv: tests our oldest python & dependencies
> >      - check-tox: tests newest dependencies on all non-EOL python
> versions
> >    Executed only incidentally:
> >      - check-venv: tests newest dependencies on whichever python version
> >
> > ('make check' does not set up any environment at all, it just runs the
> > tests in your current environment. All four invocations perform the
> > exact same tests, just in different execution environments.)
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> >   python/Makefile | 35 ++++++++++++++++++++++++++++++++---
> >   1 file changed, 32 insertions(+), 3 deletions(-)
> >
> > diff --git a/python/Makefile b/python/Makefile
> > index 5cb8378b81..76bb24e671 100644
> > --- a/python/Makefile
> > +++ b/python/Makefile
> > @@ -1,4 +1,6 @@
> > -.PHONY: help pipenv check-pipenv check clean distclean develop
> > +.PHONY: help pipenv venv check-venv check-pipenv check clean distclean
> develop
> > +
> btw, check-tox is missed here ^
>

Oops, thanks! I am not sure it winds up mattering, but writing Makefiles
feels like religion. I just follow the same steps and pray at the altar.


> > +QEMU_VENV_DIR=~/.cache/qemu-pyvenv
>
> A few suggestions:
>
> 1. For the sake of consistence with others temporary directories
> created, use QEMU_VENV_DIR=<path-to-qemu-src>/.devvenv
>
>
Sure, straight in this folder, like .tox and .venv you mean?

(I'd rename .venv to .pipenv, but pipenv doesn't let you name this folder,
annoyingly ...)


> 2. Reword to 'devvenv' or 'dev-venv' (or something similar), instead of
> 'venv', the directories and Make targets. IMHO it will make the purpose
> of the targets a bit clear.
>
>
Sure, I'll do that -- .dev-venv works for me.

>
> What do you think John?
>
>
I think that it's hot outside :)


> - Wainer
>

Thanks!
--js

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

[PATCH 09/11] python: Update help text on 'make check', 'make develop'

[John Snow]

Update for visual parity with the other targets.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/Makefile | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/python/Makefile b/python/Makefile
index 76bb24e671..4ed37c29f0 100644
--- a/python/Makefile
+++ b/python/Makefile
@@ -22,10 +22,14 @@ help:
 	@echo "    These tests use the newest dependencies."
 	@echo "    Requires: Python 3.x"
 	@echo ""
-	@echo "make develop:    Install deps for 'make check', and"
-	@echo "                 the qemu libs in editable/development mode."
+	@echo "make check:"
+	@echo "    Run tests in your *current environment*."
+	@echo "    Performs no environment setup of any kind."
 	@echo ""
-	@echo "make check:      run linters using the current environment."
+	@echo "make develop:"
+	@echo "    Install deps needed for for 'make check',"
+	@echo "    and install the qemu package in editable mode."
+	@echo "    (Can be used in or outside of a venv.)"
 	@echo ""
 	@echo "make pipenv"
 	@echo "    Creates pipenv's virtual environment (.venv)"
-- 
2.31.1

Re: [PATCH 09/11] python: Update help text on 'make check', 'make develop'

[Willian Rampazzo]

On Fri, Jun 25, 2021 at 12:46 PM John Snow <jsnow@redhat.com> wrote:
>
> Update for visual parity with the other targets.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  python/Makefile | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

Re: [PATCH 09/11] python: Update help text on 'make check', 'make develop'

[Wainer dos Santos Moschetta]

On 6/25/21 12:45 PM, John Snow wrote:
> Update for visual parity with the other targets.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   python/Makefile | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
>
> diff --git a/python/Makefile b/python/Makefile
> index 76bb24e671..4ed37c29f0 100644
> --- a/python/Makefile
> +++ b/python/Makefile
> @@ -22,10 +22,14 @@ help:
>   	@echo "    These tests use the newest dependencies."
>   	@echo "    Requires: Python 3.x"
>   	@echo ""
> -	@echo "make develop:    Install deps for 'make check', and"
> -	@echo "                 the qemu libs in editable/development mode."
> +	@echo "make check:"
> +	@echo "    Run tests in your *current environment*."
> +	@echo "    Performs no environment setup of any kind."
>   	@echo ""
> -	@echo "make check:      run linters using the current environment."
> +	@echo "make develop:"
> +	@echo "    Install deps needed for for 'make check',"
> +	@echo "    and install the qemu package in editable mode."
> +	@echo "    (Can be used in or outside of a venv.)"
>   	@echo ""
>   	@echo "make pipenv"
>   	@echo "    Creates pipenv's virtual environment (.venv)"

[PATCH 10/11] python: Update help text on 'make clean', 'make distclean'

[John Snow]

Just for visual parity with everything else.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/Makefile | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/python/Makefile b/python/Makefile
index 4ed37c29f0..06f78f760a 100644
--- a/python/Makefile
+++ b/python/Makefile
@@ -37,11 +37,14 @@ help:
 	@echo "make venv"
 	@echo "    Creates a simple venv for check-venv. ($(QEMU_VENV_DIR))"
 	@echo ""
-	@echo "make clean:      remove package build output."
+	@echo "make clean:"
+	@echo "    Remove package build output."
 	@echo ""
-	@echo "make distclean:  remove venv files, qemu package forwarder,"
-	@echo "                 built distribution files, and everything"
-	@echo "                 from 'make clean'."
+	@echo "make distclean:"
+	@echo "    remove pipenv/venv files, qemu package forwarder,"
+	@echo "    built distribution files, and everything from 'make clean'."
+	@echo ""
+	@echo -e "Have a nice day ^_^\n"
 
 pipenv: .venv
 .venv: Pipfile.lock
-- 
2.31.1

Re: [PATCH 10/11] python: Update help text on 'make clean', 'make distclean'

[Willian Rampazzo]

On Fri, Jun 25, 2021 at 12:46 PM John Snow <jsnow@redhat.com> wrote:
>
> Just for visual parity with everything else.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  python/Makefile | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

Re: [PATCH 10/11] python: Update help text on 'make clean', 'make distclean'

[Wainer dos Santos Moschetta]

On 6/25/21 12:45 PM, John Snow wrote:
> Just for visual parity with everything else.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   python/Makefile | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/python/Makefile b/python/Makefile
> index 4ed37c29f0..06f78f760a 100644
> --- a/python/Makefile
> +++ b/python/Makefile
> @@ -37,11 +37,14 @@ help:
>   	@echo "make venv"
>   	@echo "    Creates a simple venv for check-venv. ($(QEMU_VENV_DIR))"
>   	@echo ""
> -	@echo "make clean:      remove package build output."
> +	@echo "make clean:"
> +	@echo "    Remove package build output."
>   	@echo ""
> -	@echo "make distclean:  remove venv files, qemu package forwarder,"
> -	@echo "                 built distribution files, and everything"
> -	@echo "                 from 'make clean'."
> +	@echo "make distclean:"
> +	@echo "    remove pipenv/venv files, qemu package forwarder,"
> +	@echo "    built distribution files, and everything from 'make clean'."
> +	@echo ""
> +	@echo -e "Have a nice day ^_^\n"

Devs will like the last message. ;)

Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

>   
>   pipenv: .venv
>   .venv: Pipfile.lock

[PATCH 11/11] python: remove auto-generated pyproject.toml file

[John Snow]

For reasons that at-present escape me, pipenv insists on creating a stub
pyproject.toml file. This file is a nuisance, because its mere presence
changes the behavior of various tools.

For instance, this stub file will cause "pip install --user -e ." to
fail in spectacular fashion with misleading errors. "pip install -e ."
works okay, but for some reason pip does not support editable installs
to the user directory when using PEP517.

References:
  https://github.com/pypa/pip/pull/9990
  https://github.com/pypa/pip/issues/7953

As outlined in ea1213b7ccc, it is still too early for us to consider
moving to a PEP-517 exclusive package. We must support support older
distributions, so squash the annoyance for now. (Python 3.6 shipped Dec
2016, PEP517 support showed up in pip sometime in 2019 or so.)

Add 'pyproject.toml' to the 'make clean' target, and also delete it
after every pipenv invocation issued by the Makefile.

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

diff --git a/python/Makefile b/python/Makefile
index 06f78f760a..758ce2c8d8 100644
--- a/python/Makefile
+++ b/python/Makefile
@@ -49,6 +49,7 @@ help:
 pipenv: .venv
 .venv: Pipfile.lock
 	@PIPENV_VENV_IN_PROJECT=1 pipenv sync --dev --keep-outdated
+	rm -f pyproject.toml
 	@touch .venv
 
 check-pipenv: pipenv
@@ -84,6 +85,7 @@ check-tox:
 
 clean:
 	python3 setup.py clean --all
+	rm -f pyproject.toml
 
 distclean: clean
 	rm -rf qemu.egg-info/ .venv/ .tox/ $(QEMU_VENV_DIR) dist/
-- 
2.31.1

Re: [PATCH 11/11] python: remove auto-generated pyproject.toml file

[Willian Rampazzo]

On Fri, Jun 25, 2021 at 12:46 PM John Snow <jsnow@redhat.com> wrote:
>
> For reasons that at-present escape me, pipenv insists on creating a stub
> pyproject.toml file. This file is a nuisance, because its mere presence
> changes the behavior of various tools.
>
> For instance, this stub file will cause "pip install --user -e ." to
> fail in spectacular fashion with misleading errors. "pip install -e ."
> works okay, but for some reason pip does not support editable installs
> to the user directory when using PEP517.
>
> References:
>   https://github.com/pypa/pip/pull/9990
>   https://github.com/pypa/pip/issues/7953
>
> As outlined in ea1213b7ccc, it is still too early for us to consider
> moving to a PEP-517 exclusive package. We must support support older

s/support support/support/

> distributions, so squash the annoyance for now. (Python 3.6 shipped Dec
> 2016, PEP517 support showed up in pip sometime in 2019 or so.)
>
> Add 'pyproject.toml' to the 'make clean' target, and also delete it
> after every pipenv invocation issued by the Makefile.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  python/Makefile | 2 ++
>  1 file changed, 2 insertions(+)
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

Re: [PATCH 11/11] python: remove auto-generated pyproject.toml file

[Wainer dos Santos Moschetta]

On 6/25/21 12:45 PM, John Snow wrote:
> For reasons that at-present escape me, pipenv insists on creating a stub
> pyproject.toml file. This file is a nuisance, because its mere presence
> changes the behavior of various tools.
>
> For instance, this stub file will cause "pip install --user -e ." to
> fail in spectacular fashion with misleading errors. "pip install -e ."
> works okay, but for some reason pip does not support editable installs
> to the user directory when using PEP517.
>
> References:
>    https://github.com/pypa/pip/pull/9990
>    https://github.com/pypa/pip/issues/7953
>
> As outlined in ea1213b7ccc, it is still too early for us to consider
> moving to a PEP-517 exclusive package. We must support support older
> distributions, so squash the annoyance for now. (Python 3.6 shipped Dec
> 2016, PEP517 support showed up in pip sometime in 2019 or so.)
>
> Add 'pyproject.toml' to the 'make clean' target, and also delete it
> after every pipenv invocation issued by the Makefile.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   python/Makefile | 2 ++
>   1 file changed, 2 insertions(+)
Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
>
> diff --git a/python/Makefile b/python/Makefile
> index 06f78f760a..758ce2c8d8 100644
> --- a/python/Makefile
> +++ b/python/Makefile
> @@ -49,6 +49,7 @@ help:
>   pipenv: .venv
>   .venv: Pipfile.lock
>   	@PIPENV_VENV_IN_PROJECT=1 pipenv sync --dev --keep-outdated
> +	rm -f pyproject.toml
>   	@touch .venv
>   
>   check-pipenv: pipenv
> @@ -84,6 +85,7 @@ check-tox:
>   
>   clean:
>   	python3 setup.py clean --all
> +	rm -f pyproject.toml
>   
>   distclean: clean
>   	rm -rf qemu.egg-info/ .venv/ .tox/ $(QEMU_VENV_DIR) dist/