On Thu, Sep 23, 2021 at 7:17 AM Kevin Wolf <kw...@redhat.com> wrote:
> 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 <js...@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.
