Re: [PATCH 7/8] python/qapi: move scripts/qapi to python/qemu/qapi

[Daniel P . Berrangé]

On Fri, Aug 30, 2024 at 01:20:35PM +0200, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
> 
> > This is being done for the sake of unifying the linting and static type
> > analysis configurations between scripts/qapi and python/qemu/*.
> >
> > With this change, the qapi module will now be checked by mypy, flake8,
> > pylint, isort etc under all python versions from 3.8 through 3.13 under
> > a variety of different dependency configurations in the GitLab testing
> > pipelines.
> >
> > The tests can be run locally, as always:
> >
> >> cd qemu.git/python
> >> make check-minreqs
> >> make check-tox
> >> make check-dev
> >
> > "check-minreqs" is the must-pass GitLab test.
> > "check-tox" is the optional allowed-to-fail GitLab test.
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> 
> I don't understand why we have to keep Python code in its own directory
> just to get it checked.  We wouldn't do that say for Rust code, would
> we?  Anyway, if it's the price of checking, I'll pay[*].

The 'check-tox' target is defined in python/Makefile, and is
written to only check code below that location, which is a
somewhat arbitrary choice.

Having this in "make" at all is a bit outdated. Ideally all
the targets in python/Makefile should be converted into meson
targets and/or tests, in a "python" suite.

IOW, we should make 'check-tox' a target in meson.build at the
top level, and have it check any .py file in the source tree,
with an exclude list for files we know are not "clean" yet,
so we don't have to move stuff around as we clean up individual
files.

> 
> [...]
> 
> > diff --git a/scripts/qapi-gen.py b/scripts/qapi-gen.py
> > index f3518d29a54..42912c91716 100644
> > --- a/scripts/qapi-gen.py
> > +++ b/scripts/qapi-gen.py
> > @@ -11,9 +11,11 @@
> >  execution environment.
> >  """
> >  
> > +import os
> >  import sys
> >  
> > -from qapi import main
> > +sys.path.append(os.path.join(os.path.dirname(__file__), '..', 'python'))
> > +from qemu.qapi import main
> >  
> >  if __name__ == '__main__':
> >      sys.exit(main.main())
> 
> Suggest to use the opportunity to rename to just qapi-gen (no .py) and
> chmod +x, possibly in a separate patch.
> 
> [...]
> 
> 
> [*] Grudgingly.
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|