On Thu, Sep 23, 2021 at 2:32 PM Vladimir Sementsov-Ogievskiy <
vsement...@virtuozzo.com> wrote:
> 23.09.2021 21:07, John Snow wrote:
> > 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.)
> >
> > In the future, we will pivot to always preferring a specific installed
> > instance of qemu python packages managed directly by iotests. For now
> > simply warn if there is an ambiguity over which instance that iotests
> > might use.
> >
> > Example: If a user has navigated to ~/src/qemu/python and executed
> > `pip install .`, you will see output like this when running `./check`:
> >
> > WARNING: 'qemu' python packages will be imported from outside the source
> tree ('/home/jsnow/src/qemu/python')
> > Importing instead from
> '/home/jsnow/.local/lib/python3.9/site-packages/qemu'
> >
> > Signed-off-by: John Snow <js...@redhat.com>
> > ---
> > tests/qemu-iotests/testenv.py | 24 ++++++++++++++++++++++++
> > 1 file changed, 24 insertions(+)
> >
> > diff --git a/tests/qemu-iotests/testenv.py
> b/tests/qemu-iotests/testenv.py
> > index 99a57a69f3a..1c0f6358538 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
> > @@ -112,6 +114,27 @@ 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 namespace packages
> > + # (using qemu.qmp as a bellwether) from an unexpected location.
> > + # i.e. the package is already installed in the user's
> environment.
> > + try:
> > + qemu_spec = importlib.util.find_spec('qemu.qmp')
> > + except ModuleNotFoundError:
> > + qemu_spec = None
> > +
> > + if qemu_spec and qemu_spec.origin:
> > + spec_path = Path(qemu_spec.origin)
> > + try:
> > + _ = spec_path.relative_to(qemu_srctree_path)
>
> It took some time and looking at specification trying to understand what's
> going on here :)
>
> Could we just use:
>
> if not Path(qemu_spec.origin).is_relative_to(qemu_srctree_path):
> ... logging ...
>
>
Nope, that's 3.9+ only. (I made the same mistake.)
> > + except ValueError:
>
> + self._logger.warning(
> > + "WARNING: 'qemu' python packages will be imported
> from"
> > + " outside the source tree ('%s')",
> > + qemu_srctree_path)
> > + self._logger.warning(
> > + " Importing instead from '%s'",
> > + spec_path.parents[1])
> > +
>
> Also, I'd move this new chunk of code to a separate function (may be even
> out of class, as the only usage of self is self._logger, which you
> introduce with this patch. Still a method would be OK too). And then, just
> call it from __init__(). Just to keep init_directories() simpler. And with
> this new code we don't init any directories to pass to further test
> execution, it's just a check for runtime environment.
>
>
I wanted to keep the wiggly python import logic all in one place so that it
was harder to accidentally forget a piece of it if/when we adjust it. I can
create a standalone function for it, but I'd need to stash that
qemu_srctree_path variable somewhere if we want to call that runtime check
from somewhere else, because I don't want to compute it twice. Is it still
worth doing in your opinion if I just create a method/function and pass it
the qemu_srctree_path variable straight from init_directories()?
Not adding _logger is valid, though. I almost removed it myself. I'll
squash that in.
> > self.pythonpath = os.pathsep.join(filter(None, (
> > self.source_iotests,
> > str(qemu_srctree_path),
> > @@ -230,6 +253,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()
> >
> >
>
>
> --
> Best regards,
> Vladimir
>
>
