Am 23.09.2021 um 12:57 hat Kevin Wolf geschrieben:
> Am 23.09.2021 um 02:16 hat John Snow geschrieben:
> > Add a warning for when 'iotests' runs against a qemu namespace that
> > isn't the one in the source tree. This might occur if you have
> > (accidentally) installed the Python namespace package to your local
> > packages.
> >
> > (I'm not going to say that this is because I bit myself with this,
> > but. You can fill in the blanks.)
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> > tests/qemu-iotests/testenv.py | 21 ++++++++++++++++++++-
> > 1 file changed, 20 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
> > index 88104dace90..8a43b193af5 100644
> > --- a/tests/qemu-iotests/testenv.py
> > +++ b/tests/qemu-iotests/testenv.py
> > @@ -16,6 +16,8 @@
> > # along with this program. If not, see <http://www.gnu.org/licenses/>.
> > #
> >
> > +import importlib.util
> > +import logging
> > import os
> > import sys
> > import tempfile
> > @@ -25,7 +27,7 @@
> > import random
> > import subprocess
> > import glob
> > -from typing import List, Dict, Any, Optional, ContextManager
> > +from typing import List, Dict, Any, Optional, ContextManager, cast
> >
> > DEF_GDB_OPTIONS = 'localhost:12345'
> >
> > @@ -112,6 +114,22 @@ def init_directories(self) -> None:
> > # Path where qemu goodies live in this source tree.
> > qemu_srctree_path = Path(__file__, '../../../python').resolve()
> >
> > + # warn if we happen to be able to find 'qemu' packages from an
> > + # unexpected location (i.e. the package is already installed in
> > + # the user's environment)
> > + qemu_spec = importlib.util.find_spec('qemu.qmp')
> > + if qemu_spec:
> > + spec_path = Path(cast(str, qemu_spec.origin))
>
> You're casting Optional[str] to str here. The source type is not obvious
> from the local code, so unless you spend some time actively figuring it
> out, this could be casting anything to str. But even knowing this, just
> casting Optional away looks fishy anyway.
>
> Looking up the ModuleSpec docs, it seems okay to assume that it's never
> None in our case, but wouldn't it be much cleaner to just 'assert
> qemu_spec.origin' first and then use it without the cast?
>
OK. I suppose I was thinking: "It's always going to be a string, and if it isn't, something else will explode below, surely, so the assertion is redundant", but I don't want code that makes people wonder, so principle of least surprise it is.
> > + try:
> > + _ = spec_path.relative_to(qemu_srctree_path)
> > + except ValueError:
> > + self._logger.warning(
> > + "WARNING: 'qemu' package will be imported from outside "
> > + "the source tree!")
> > + self._logger.warning(
> > + "Importing from: '%s'",
> > + spec_path.parents[1])
> > +
> > self.pythonpath = os.getenv('PYTHONPATH')
> > self.pythonpath = os.pathsep.join(filter(None, (
> > self.source_iotests,
> > @@ -231,6 +249,7 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: str,
> >
> > self.build_root = os.path.join(self.build_iotests, '..', '..')
> >
> > + self._logger = logging.getLogger('qemu.iotests')
> > self.init_directories()
> > self.init_binaries()
>
> Looks good otherwise.
Well, actually there is a major problem: We'll pass the right PYTHONPATH
for the test scripts that we're calling, but this script itself doesn't
use it yet. So what I get is:
Traceback (most recent call last):
File "/home/kwolf/source/qemu/tests/qemu-iotests/build/check", line 120, in <module>
env = TestEnv(imgfmt=args.imgfmt, imgproto=args.imgproto,
File "/home/kwolf/source/qemu/tests/qemu-iotests/testenv.py", line 253, in __init__
self.init_directories()
File "/home/kwolf/source/qemu/tests/qemu-iotests/testenv.py", line 120, in init_directories
qemu_spec = importlib.util.find_spec('qemu.qmp')
File "/usr/lib64/python3.9/importlib/util.py", line 94, in find_spec
parent = __import__(parent_name, fromlist=['__path__'])
ModuleNotFoundError: No module named 'qemu'
Kevin
Tch. So, it turns out with namespace packages that once you remove the subpackages (pip uninstall qemu), it leaves the empty namespace folder behind. This is also the reason I directly use 'qemu.qmp' as a bellwether here, to make sure we're prodding a package and not just an empty namespace with nothing in it.
I'll fix these, thanks.