[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/6] iotests: add warning for rogue 'qemu' packages
From: |
Kevin Wolf |
Subject: |
Re: [PATCH 2/6] iotests: add warning for rogue 'qemu' packages |
Date: |
Thu, 23 Sep 2021 13:17:13 +0200 |
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?
>
> > + 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