All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: John Snow <jsnow@redhat.com>
Cc: qemu-devel <qemu-devel@nongnu.org>,
	Qemu-block <qemu-block@nongnu.org>,
	"Cleber Rosa" <crosa@redhat.com>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	"Hanna Reitz" <hreitz@redhat.com>,
	"Thomas Huth" <thuth@redhat.com>, "Kevin Wolf" <kwolf@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"Wainer dos Santos Moschetta" <wainersm@redhat.com>,
	"Beraldo Leal" <bleal@redhat.com>
Subject: Re: [RFC PATCH 0/9] tests: run python tests under the build/tests/venv environment
Date: Fri, 13 May 2022 17:07:26 +0100	[thread overview]
Message-ID: <Yn6CPm3VosPfcK7j@redhat.com> (raw)
In-Reply-To: <CAFn=p-Y77=F=qjdwWRycFafxiS7Rjxag4gLmPK0ERqEiyT19ig@mail.gmail.com>

On Fri, May 13, 2022 at 11:55:18AM -0400, John Snow wrote:
> On Fri, May 13, 2022, 6:29 AM Daniel P. Berrangé <berrange@redhat.com>
> wrote:
> 
> > On Fri, May 13, 2022 at 09:35:23AM +0100, Daniel P. Berrangé wrote:
> > > On Thu, May 12, 2022 at 08:06:00PM -0400, John Snow wrote:
> > > > RFC: This is a very early, crude attempt at switching over to an
> > > > external Python package dependency for QMP. This series does not
> > > > actually make the switch in and of itself, but instead just switches to
> > > > the paradigm of using a venv in general to install the QEMU python
> > > > packages instead of using PYTHONPATH to load them from the source tree.
> > > >
> > > > (By installing the package, we can process dependencies.)
> > > >
> > > > I'm sending it to the list so I can show you some of what's ugly so far
> > > > and my notes on how I might make it less ugly.
> > > >
> > > > (1) This doesn't trigger venv creation *from* iotests, it merely prints
> > > > a friendly error message if "make check-venv" has not been run
> > > > first. Not the greatest.
> > >
> > > So if we run the sequence
> > >
> > >   mkdir build
> > >   cd build
> > >   ../configure
> > >   make
> > >   ./tests/qemu-iotests/check 001
> > >
> > > It won't work anymore, until we 'make check-venv' (or simply
> > > 'make check') ?
> > >
> > > I'm somewhat inclined to say that venv should be created
> > > unconditionally by default. ie a plain 'make' should always
> > > everything needed to be able to invoke the tests directly.
> > >
> > > > (2) This isn't acceptable for SRPM builds, because it uses PyPI to
> > fetch
> > > > packages just-in-time. My thought is to use an environment variable
> > like
> > > > QEMU_CHECK_NO_INTERNET that changes the behavior of the venv setup
> > > > process. We can use "--system-site-packages" as an argument to venv
> > > > creation and "--no-index" as an argument to pip installation to achieve
> > > > good behavior in SRPM building scenarios. It'd be up to the spec-writer
> > > > to opt into that behavior.
> > >
> > > I think I'd expect --system-site-packages to be the default behaviour.
> > > We expect QEMU to be compatible with the packages available in the
> > > distros that we're targetting. So if the dev has the python packages
> > > installed from their distro, we should be using them preferentially.
> > >
> > > This is similar to how we bundle slirp/capstone/etc, but will
> > > preferentially use the distro version if it is available.
> >
> > AFAICT from testing it, when '--system-site-packages' is set
> > for the venv, then 'pip install' appears to end up being a
> > no-op if the package is already present in the host, but
> > installs it if missing.
> >
> > IOW, if we default to --system-site-packages, but still
> > also run 'pip install', we should "do the right thing".
> > It'll use any  distro packages that are available, and
> > augment with stuff from pip. In the no-op case, pip will
> > still try to consult the pipy servers to fetch version
> > info IIUC, so we need to be able to stop that. So I'd
> > suggest a  --python-env arg taking three values
> >
> >  * "auto" (the default) - add --system-site-packages, but
> >    also run 'pip install'. Good for developers day-to-day
> >
> 
> Sounds like a decent balance...
> 
> ...My only concern is that the system packages might be very old and it's
> possible that the qemu packages will be "too new" or have conflicts with
> the system deps.
> 
> I'll just have to test this.
> 
> e.g. what if I want to require mypy >= 0.900 for testing, but you have a
> system package that requires mypy < 0.700?

I would expect us to not require packages that are not present in
the distros implied by 

  https://www.qemu.org/docs/master/about/build-platforms.html

if that was absolutely a must have, then gracefully skip tests
if the system version wasn't new enough. The user could always
pass --python-env=pip if they want to force new enough

> The python dep compatibility matrix I try to enforce and support for
> testing is already feeling overly wide. This might force me to support an
> even wider matrix, which I think is the precisely wrong direction for venvs
> where we want tighter control as a rule.

As above, from a QEMU POV we have clearly defined our targetted
platform matrix. Splitting off python packages isn't a reason
to change QEMU's platform matrix IMHO.

> >  * "system" - add --system-site-packages but never run
> >    'pip install'.  Good for formal package builds.
> >
> 
> We still have to install the in-tree qemu ns package, but we can use
> --no-index to do it. It'll fail if the deps aren't met.

> >  * "pip"  - don't add --system-site-packages, always run
> >    'pip install'. Good for testing compatibility with
> >    bleeding edge python versions, or if explicit full
> >    independance from host python install is desired.
> >
> 
> as arguments to configure, this spread of options makes sense to me than
> paolo's version, but I've still got some doubt on mixing system and venv
> packages.
> 
> I am also as of yet not sold on building the venv *from* configure, see my
> response to Paolo on that topic.
> 
> I'll keep plugging away for now, but the big picture is still a tad murky
> in my head.
> 
> --js

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



  reply	other threads:[~2022-05-13 16:36 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-13  0:06 [RFC PATCH 0/9] tests: run python tests under the build/tests/venv environment John Snow
2022-05-13  0:06 ` [RFC PATCH 1/9] python: update for mypy 0.950 John Snow
2022-05-13  8:42   ` Paolo Bonzini
2022-05-13 14:09     ` John Snow
2022-05-13  0:06 ` [RFC PATCH 2/9] tests: add "TESTS_PYTHON" variable to Makefile John Snow
2022-05-13  8:23   ` Paolo Bonzini
2022-05-13  0:06 ` [RFC PATCH 3/9] tests: install "qemu" namespace package into venv John Snow
2022-05-13  8:26   ` Paolo Bonzini
2022-05-13 14:01     ` John Snow
2022-05-13  0:06 ` [RFC PATCH 4/9] tests: silence pip upgrade warnings during venv creation John Snow
2022-05-13  8:27   ` Paolo Bonzini
2022-05-13 14:02     ` John Snow
2022-05-13  0:06 ` [RFC PATCH 5/9] tests: use tests/venv to run basevm.py-based scripts John Snow
2022-05-13  8:33   ` Paolo Bonzini
2022-05-13  0:06 ` [RFC PATCH 6/9] tests: add check-venv as a dependency of check and check-block John Snow
2022-05-13  8:41   ` Paolo Bonzini
2022-05-13 14:12     ` John Snow
2022-05-13 15:34       ` Paolo Bonzini
2022-05-13 16:08         ` John Snow
2022-05-13  0:06 ` [RFC PATCH 7/9] tests: add check-venv to build-tcg-disabled CI recipe John Snow
2022-05-13  8:41   ` Paolo Bonzini
2022-05-13  0:06 ` [RFC PATCH 8/9] iotests: fix source directory location John Snow
2022-05-13  8:35   ` Paolo Bonzini
2022-05-13  0:06 ` [RFC PATCH 9/9] iotests: use tests/venv for running tests John Snow
2022-05-13  8:38   ` Paolo Bonzini
2022-05-13 14:38     ` John Snow
2022-05-13 15:33       ` Paolo Bonzini
2022-05-13 16:00         ` John Snow
2022-05-14 15:55         ` John Snow
2022-05-16  7:41           ` Paolo Bonzini
2022-05-17 23:51             ` John Snow
2022-05-18 10:38               ` Paolo Bonzini
2022-05-13  8:35 ` [RFC PATCH 0/9] tests: run python tests under the build/tests/venv environment Daniel P. Berrangé
2022-05-13  9:45   ` Paolo Bonzini
2022-05-13 10:29   ` Daniel P. Berrangé
2022-05-13 15:55     ` John Snow
2022-05-13 16:07       ` Daniel P. Berrangé [this message]
2022-05-13 16:49         ` Paolo Bonzini
2022-05-13 19:09           ` John Snow
2022-05-13 12:57   ` Kevin Wolf
2022-05-13 15:25   ` John Snow
2022-05-13 10:20 ` Paolo Bonzini
2022-05-13 15:39   ` John Snow
2022-05-13 16:07     ` Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Yn6CPm3VosPfcK7j@redhat.com \
    --to=berrange@redhat.com \
    --cc=alex.bennee@linaro.org \
    --cc=bleal@redhat.com \
    --cc=crosa@redhat.com \
    --cc=f4bug@amsat.org \
    --cc=hreitz@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=thuth@redhat.com \
    --cc=wainersm@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.