Re: [PATCH 2/6] iotests: add warning for rogue 'qemu' packages

[Kevin Wolf <kwolf@redhat.com>]

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