Re: [PATCH 4/4] scripts/qmp: Fix QEMU Python scripts path

[John Snow <jsnow@redhat.com>]

On 5/2/20 1:54 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
> 
>> On 4/30/20 1:04 AM, Markus Armbruster wrote:
>>> John Snow <jsnow@redhat.com> writes:
>>>
>>>> On 4/21/20 5:42 AM, Philippe Mathieu-Daudé wrote:
>>>>> QEMU Python scripts have been moved in commit 8f8fd9edba4 ("Introduce
>>>>> Python module structure"). Use the same sys.path modification used
>>>>> in the referenced commit to be able to use these scripts again.
>>>>>
>>>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>>> ---
>>>>>  scripts/qmp/qmp      | 4 +++-
>>>>>  scripts/qmp/qom-fuse | 4 +++-
>>>>>  scripts/qmp/qom-get  | 4 +++-
>>>>>  scripts/qmp/qom-list | 4 +++-
>>>>>  scripts/qmp/qom-set  | 4 +++-
>>>>>  scripts/qmp/qom-tree | 4 +++-
>>>>>  6 files changed, 18 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/scripts/qmp/qmp b/scripts/qmp/qmp
>>>>> index 0625fc2aba..8e52e4a54d 100755
>>>>> --- a/scripts/qmp/qmp
>>>>> +++ b/scripts/qmp/qmp
>>>>> @@ -11,7 +11,9 @@
>>>>>  # See the COPYING file in the top-level directory.
>>>>>  
>>>>>  import sys, os
>>>>> -from qmp import QEMUMonitorProtocol
>>>>> +
>>>>> +sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
>>>>> +from qemu.qmp import QEMUMonitorProtocol
>>>>>  
>>>>
>>>> Try to avoid using sys.path hacks; they don't work in pylint or mypy and
>>>> it provides an active barrier to CQA work here.
>>>> (They also tend to be quite fragile.)
>>>>
>>>> We can discuss the right way to do this; one of those ways is to create
>>>> an installable package that we can install locally in a virtual environment.
>>>>
>>>> Another way is perhaps to set PYTHONPATH in the calling environment so
>>>> that standard "import" directives will work.
>>>>
>>>> Both ultimately involve changing the environment of the user to
>>>> accommodate the script.
>>>
>>> For what it's worth, tests/Makefile.involve does the latter for
>>> tests/qapi-schema/test-qapi.py.  Simple enough, but makes manual
>>> invocation inconvenient.
>>>
>>> Not necessary for scripts/qapi-gen.py, because its "import qmp.FOO"
>>> finds qmp right in scripts/qmp/.
>>>
>>
>> Yes, using "proper" package hierarchies often means the loss of being
>> able to invoke the scripts directly, unless you are careful to organize
>> the package such that the scripts can run both in an "unpackaged" and a
>> "packaged" mode.
>>
>> It can be done, but it's tricky and can be prone to error. Let's take a
>> look at how to do it!
>>
>> Let's imagine we have an honest-to-goodness QAPI parser module. In
>> isolation, the layout for such a package would probably look like this:
>>
>> qapi.git/
>>   setup.py
>>   qapi-gen.py
>>   README.rst
>>   qapi/
>>     __init__.py
>>     parser.py
>>     schema.py
>>     ...etc
>>
>>
>> Now, anything inside of qapi/ is considered the "qapi module" and you
>> will be unable to directly execute anything inside of this folder,
>> unless it does not depend on anything else in the "qapi module".
>>
>> i.e. "import qapi.x" will work, but only from the executing context of a
>> thread that understands how to find "qapi". If you are IN this
>> directory, you do not have that context, so those directives will not work.
>>
>> Python imports are always handled relative to the importing file, not
>> the imported file.
>>
>> qapi-gen in the parent directory, however, can use "from qapi import
>> parser" without any problem, because if you are executing it directly,
>> it will be able to see the "qapi module" as a folder.
> 
> Hmm...
> 
>     $ git-grep '^from.*schema' scripts/
>     scripts/qapi-gen.py:from qapi.schema import QAPIError, QAPISchema
>     scripts/qapi/events.py:from qapi.schema import QAPISchemaEnumMember
>     scripts/qapi/gen.py:from qapi.schema import QAPISchemaVisitor
>     scripts/qapi/introspect.py:from qapi.schema import (QAPISchemaArrayType, QAPISchemaBuiltinType,
>     scripts/qapi/types.py:from qapi.schema import QAPISchemaEnumMember, QAPISchemaObjectType
>     scripts/qapi/visit.py:from qapi.schema import QAPISchemaObjectType
> 
> How come importing from qapi. works in scripts/qapi/*.py, too?
> 
> [...]
> 

Because they're being consumed from the POV of the entry point, which is
scripts/qapi-gen.py.

That is to say, if in scripts/qapi/ you create a new file:

helloworld.py:
```
#!/usr/bin/env python3

from qapi.schema import QAPIError
```

And then execute it:

jsnow@probe ~/s/q/s/qapi ((v5.0.0))> ./helloworld.py
Traceback (most recent call last):
  File "./helloworld.py", line 3, in <module>
    from qapi.schema import QAPIError
ModuleNotFoundError: No module named 'qapi'


It doesn't know what qapi is. Python modules do not carry the
information or metadata themselves inherently to know that they are "in
a module."

That information is known only to qapi-gen.py, and files it brings in
use the namespace set up by qapi-gen.py -- not their own.

This is what I was trying to explain in the last mail about how to lay
out a python module -- and why it's tricky to write imports in such a
way that they preserve "run as script" semantics while also providing
"run as installed package semantics".

Put another way:

qapi-gen.py is considered a /client of/ the qapi module when it is in a
folder above/outside of the QAPI module. If you refactor the module to
include such a script, by moving it into that module's directory -- you
lose your "client" and you no longer have an entry point. (i.e. you
cannot just run the script in the folder any more.)

You can remedy this in several ways:

1. Set PYTHONPATH to include the folder that the `qapi` folder is in, so
that even when executing e.g. somedir/qapi/scripts.py, it knows to look
in "somedir" when evaluating imports.

2. Create a new shim that exists outside of the module proper that does
nothing but import and then execute the script. E.g., "somedir/shim.py"
that just loads the script from qapi/scripts.py and then runs it. This
is basically equivalent to #1, but doesn't require a special ENV for the
user.

3. Use the package installation facilities (setuptools) to define a
"script entry point", which in practice just uses setuptools to
automatically generate a shim for you that gets placed in e.g.
~/.local/bin/ and makes the script available on the command-line.


So I just want to point out that packaging python modules is possible,
and it's a good thing to do, but "runnable scripts" present a problem
WRT lookup paths depending on how they are written!

--js