Re: [PATCH 0/7] python: create installable package

[John Snow]

On 6/6/20 1:53 AM, Vladimir Sementsov-Ogievskiy wrote:
> For patches 05-07:
> 
> Reviewing such patch is a strange thing: Pipfile changes are obvious
> enough, just select some version (I can't be sure about correct version
> choice, just believe in your commit messages). But what for
> Pipfile.lock? I can state that it's about package set selecting,
> Pipfile.lock looks like what it should be, but I have idea about all
> these packages, their versions and hashes (and even, does it correspond
> really to Pipfile or not) :)
> 
> Ha, what I can check, is: does pipenv create almost same Pipfile.lock in
> my environment (with 3.7.5 python)
> OK, I've tried (pipenv lock --dev --python /usr/bin/python3), and yes,
> result is almost the same. For mypy and importlib-metadata packages I
> have newer version and different hashes (of course). Other packages are
> the same. Set of packages is the same.
> 
> Hmm, but in may generated Pipfile.lock there no "markers". They are
> about python version and looks like "markers": "python_version >=
> '3.5'".. Does pipenv follow them? Then why they are not generated for
> me, did you use some additional command/option to create them?
> Anyway, they don't look dangerous, so for last three patches:
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> 

Yes, from a blank repository you can do "pipenv install mypy>=0.730" --
it uses the same package specification syntax that pip uses.

(Or, you can edit the Pipfile manually, which is what I actually did.
The benefit of doing so is primarily about documenting our minimum
version requirements. I suppose in truth, those minimums should go into
setup.py too, actually.)

Thanks for taking a look!

--js

> 03.06.2020 03:15, John Snow wrote:
>> Based-on: 20200602214528.12107-1-jsnow@redhat.com
>>
>> This series factors the python/qemu directory as an installable
>> module. As a developer, you can install this to your virtual environment
>> and then always have access to the classes contained within without
>> needing to wrangle python import path problems.
>>
>> When developing, you could go to qemu/python/ and invoke `pipenv shell`
>> to activate a virtual environment within which you could type `pip
>> install -e .` to install a special development version of this package
>> to your virtual environment. This package will always reflect the most
>> recent version of the source files in the tree.
>>
>> When not developing, you could install a version of this package to your
>> environment outright to gain access to the QMP and QEMUMachine classes
>> for lightweight scripting and testing.
>>
>> This package is formatted in such a way that it COULD be uploaded to
>> https://pypi.org/project/qemu and installed independently of qemu.git
>> with `pip install qemu`, but of course that button remains unpushed.
>>
>> There are a few major questions to answer first:
>>
>> - What versioning scheme should we use? See patch 2.
>>
>> - Should we use a namespace package or not?
>>    - Namespaced: 'qemu.machine', 'qemu.monitor' etc may be separately
>>      versioned, packaged and distributed packages. Third party authors
>>      may register 'qemu.xxx' to create plugins within the namespace as
>>      they see fit.
>>
>>    - Non-namespaced: 'qemu' is one giant glob package, packaged and
>>      versioned in unison. We control this package exclusively.
>>
>> - How do we eliminate sys.path hacks from the rest of the QEMU tree?
>>    (Background: sys.path hacks generally impede the function of static
>>    code quality analysis tools like mypy and pylint.)
>>
>>    - Simplest: parent scripts (or developer) needs to set PYTHONPATH.
>>
>>    - Harder: Python scripts should all be written to assume package form,
>>      all tests and CI that use Python should execute within a VENV.
>>
>>    In either case, we lose the ability (for many scripts) to "just run" a
>>    script out of the source tree if it depends on other QEMU Python
>>    files. This is annoying, but as the complexity of the Python lib
>>    grows, it is unavoidable.
>>
>>    In the VENV case, we at least establish a simple paradigm: the scripts
>>    should work in their "installed" forms; and the rest of the build and
>>    test infrastructure should use this VENV to automatically handle
>>    dependencies and path requirements. This should allow us to move many
>>    of our existing python scripts with "hidden" dependencies into a
>>    proper python module hierarchy and test for regressions with mypy,
>>    flake8, pylint, etc.
>>
>>    (We could even establish e.g. Sphinx versions as a dependency for our
>>    build kit here and make sure it's installed to the VENV.)
>>
>>    Pros: Almost all scripts can be moved under python/qemu/* and checked
>>    with CQA tools. imports are written the same no matter where you are
>>    (Use the fully qualified names, e.g. qemu.core.qmp.QMPMessage).
>>    Regressions in scripts are caught *much* faster.
>>
>>    Downsides: Kind of annoying; most scripts now require you to install a
>>    devkit forwarder (pip3 install --user .) or be inside of an activated
>>    venv. Not too bad if you know python at all, but it's certainly less
>>    plug-n-play.
>>
>> - What's our backwards compatibility policy if we start shipping this?
>>
>>    Proposed: Attempt to maintain API compatibility (after stabilizing the
>>    library). Incompatible changes should probably cause a semver bump.
>>
>>    Each published release makes no attempt to support any version of QEMU
>>    other than the one it was released against. We publish this on the tin
>>    in big red letters.
>>
>> TESTING THIS PACKAGE OUT:
>>
>> 1. You can install to your local user's environment normally by
>> navigating to qemu/python/ and typing "pip3 install --user ."
>>
>> 2. If you are in a VENV, use "pip install ."
>>
>> 3. To install in development mode (Where the installed package always
>> reflects the most recent version of the files automatically), use "pip3
>> install -e ." or "pip install -e ." as appropriate (See above)
>>
>> John Snow (7):
>>    python/qemu: create qemu.lib module
>>    python/qemu: formalize as package
>>    python/qemu: add README.rst
>>    python/qemu: Add pipenv support
>>    python/qemu: add pylint to pipenv
>>    python/qemu: Add flake8 to pipenv
>>    python/qemu: add mypy to pipenv
>>
>>   python/README.rst                         |   6 +
>>   python/qemu/README.rst                    |   8 +
>>   python/Pipfile                            |  14 ++
>>   python/Pipfile.lock                       | 195 ++++++++++++++++++++++
>>   python/qemu/__init__.py                   |  11 --
>>   python/qemu/{ => core}/.flake8            |   0
>>   python/qemu/core/__init__.py              |  57 +++++++
>>   python/qemu/{ => core}/accel.py           |   0
>>   python/qemu/{ => core}/machine.py         |   0
>>   python/qemu/{ => core}/pylintrc           |   0
>>   python/qemu/{ => core}/qmp.py             |   0
>>   python/qemu/{ => core}/qtest.py           |   0
>>   python/setup.py                           |  50 ++++++
>>   scripts/device-crash-test                 |   2 +-
>>   scripts/qmp/qemu-ga-client                |   2 +-
>>   scripts/qmp/qmp                           |   2 +-
>>   scripts/qmp/qmp-shell                     |   2 +-
>>   scripts/qmp/qom-fuse                      |   2 +-
>>   scripts/qmp/qom-get                       |   2 +-
>>   scripts/qmp/qom-list                      |   2 +-
>>   scripts/qmp/qom-set                       |   2 +-
>>   scripts/qmp/qom-tree                      |   2 +-
>>   scripts/render_block_graph.py             |   6 +-
>>   scripts/simplebench/bench_block_job.py    |   4 +-
>>   tests/acceptance/avocado_qemu/__init__.py |   2 +-
>>   tests/acceptance/boot_linux.py            |   3 +-
>>   tests/acceptance/virtio_check_params.py   |   2 +-
>>   tests/acceptance/virtio_version.py        |   2 +-
>>   tests/migration/guestperf/engine.py       |   2 +-
>>   tests/qemu-iotests/235                    |   2 +-
>>   tests/qemu-iotests/297                    |   2 +-
>>   tests/qemu-iotests/iotests.py             |   4 +-
>>   tests/vm/basevm.py                        |   6 +-
>>   33 files changed, 355 insertions(+), 39 deletions(-)
>>   create mode 100644 python/README.rst
>>   create mode 100644 python/qemu/README.rst
>>   create mode 100644 python/Pipfile
>>   create mode 100644 python/Pipfile.lock
>>   delete mode 100644 python/qemu/__init__.py
>>   rename python/qemu/{ => core}/.flake8 (100%)
>>   create mode 100644 python/qemu/core/__init__.py
>>   rename python/qemu/{ => core}/accel.py (100%)
>>   rename python/qemu/{ => core}/machine.py (100%)
>>   rename python/qemu/{ => core}/pylintrc (100%)
>>   rename python/qemu/{ => core}/qmp.py (100%)
>>   rename python/qemu/{ => core}/qtest.py (100%)
>>   create mode 100755 python/setup.py
>>
> 
> 

-- 
—js