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 > >