Re: [PATCH 04/37] qapi: move generator entrypoint into module

[John Snow <jsnow@redhat.com>]

On 9/16/20 7:54 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
> 
>> As part of delinting and adding type hints to the QAPI generator, it's
>> helpful for the entrypoint to be part of the module, only leaving a very
>> tiny entrypoint shim outside of the module.
>>
>> As a result, all of the include statements are reworked to be module-aware,
>> as explicit relative imports.
> 
> Should this be split into two commits, one for each of the paragraphs
> above?
> 

Hmm ... I hadn't considered it was possible, but actually ... I guess I 
can split those out, yeah.

> PEP 8 recommends absolute imports, with one exception:
> 
>      However, explicit relative imports are an acceptable alternative to
>      absolute imports, especially when dealing with complex package
>      layouts where using absolute imports would be unnecessarily verbose:
> 
>          from . import sibling
>          from .sibling import example
> 
>      Standard library code should avoid complex package layouts and
>      always use absolute imports.
> 
> Do you think it covers your use of relative imports?
> 

Admittedly I am going by trial and error; my experience is that the 
relative imports behave the nicest.

There is a historical fear of explicit relative imports because they are 
"new" and years of Python2 compatibility rendered many afraid of them. 
It is advice safely ignored in my opinion.

Using absolute imports (e.g. from qapi.sibling import foo) gets messy in 
virtual environments when you have *installed* the module in question: 
it becomes ambiguous as to *which* qapi you meant: the one in this 
folder, or the one installed to the environment?

Python, mypy, pylint, flake8 etc disagree sometimes, or can get 
confused; thinking there are two copies of each module.

Just my experience that relative imports seem to give me the least trouble.

>> This is done primarily for the benefit of python tooling (pylint, mypy,
>> flake8, et al) which otherwise has trouble consistently resolving
>> "qapi.x" to mean "a sibling file in this folder."
> 
> Can you give me an example of some tool having troube?
> 

I'd have to code up some examples. I have some hobby code unrelated to 
QEMU where I struggled to get flake8, mypy, and pylint all cooperating 
with an import regime until I gave up and used explicit relative imports.

>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>   scripts/qapi-gen.py        | 94 +++-----------------------------------
>>   scripts/qapi/commands.py   |  4 +-
>>   scripts/qapi/doc.py        |  2 +-
>>   scripts/qapi/events.py     |  8 ++--
>>   scripts/qapi/expr.py       |  4 +-
>>   scripts/qapi/gen.py        |  4 +-
>>   scripts/qapi/introspect.py |  8 ++--
>>   scripts/qapi/parser.py     |  4 +-
>>   scripts/qapi/schema.py     |  8 ++--
>>   scripts/qapi/script.py     | 91 ++++++++++++++++++++++++++++++++++++
>>   scripts/qapi/types.py      |  6 +--
>>   scripts/qapi/visit.py      |  6 +--
>>   12 files changed, 124 insertions(+), 115 deletions(-)
>>   create mode 100644 scripts/qapi/script.py
> 
> Naming is hard...  main.py?
> 

I was thinking of changing this myself, so this convinced me.

>>
>> diff --git a/scripts/qapi-gen.py b/scripts/qapi-gen.py
>> index 59becba3e1..e649f8dd44 100644
>> --- a/scripts/qapi-gen.py
>> +++ b/scripts/qapi-gen.py
>> @@ -1,97 +1,15 @@
>>   #!/usr/bin/env python3
>> -
>> -# This work is licensed under the terms of the GNU GPL, version 2 or later.
>> -# See the COPYING file in the top-level directory.
>> -
> 
> Keep the license blurb.
> 

This is a mistake. I tried to convince git to "move" the old file and 
then add a "new" file to preserve history, but of course that's not how 
git manages file histories, so it didn't work.

TLDR: I didn't delete the license blurb, I just didn't "add" it again.
I'll "fix" that.

> [...]
>