From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 586D0C433F5 for ; Fri, 13 May 2022 15:42:31 +0000 (UTC) Received: from localhost ([::1]:49290 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1npXR0-0007vm-GR for qemu-devel@archiver.kernel.org; Fri, 13 May 2022 11:42:30 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:45480) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1npXOQ-0002pF-Kf for qemu-devel@nongnu.org; Fri, 13 May 2022 11:39:54 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:32057) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1npXON-0007XE-3P for qemu-devel@nongnu.org; Fri, 13 May 2022 11:39:49 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1652456386; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=Bi5iPD8TXT86bfRWPULvkgVvAyZG//e6pNA9P91oy04=; b=CjANYgVZh/zbHQ5ErL5rFhx0xXJKMwpyRqyjqHF2Q17mvzwRoSIclc0NJy1bX8fJFuecQu g862MX1XV11/iB7lDglAukdnuWHS6YSelYxagg66D6w5lEGKbsLMfJGL8v8Fixq5LZosqs w3BJ5jrKspWHGGQiW7qkn2O5bbiiTdE= Received: from mail-ua1-f71.google.com (mail-ua1-f71.google.com [209.85.222.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-112-cFAY9PgiODidkGMOcGNvNQ-1; Fri, 13 May 2022 11:39:44 -0400 X-MC-Unique: cFAY9PgiODidkGMOcGNvNQ-1 Received: by mail-ua1-f71.google.com with SMTP id 66-20020a9f2048000000b003688fbd5e19so360775uam.14 for ; Fri, 13 May 2022 08:39:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Bi5iPD8TXT86bfRWPULvkgVvAyZG//e6pNA9P91oy04=; b=1+f4sWJLSjus2M7adekvCnE2TmX7eOHjJ/dwdN8mX43KjxFADGAaMbmL8LVhb/mhLe Oyu88VpxvNo+1tNGB3zt9ZlFXPQ/bk/yAtJX63bkh6Ck56R+KzZLqYJFx07teFXmLV12 hex0Woom5C16NZzgxnSakIHobFDO3dCsDMS0Z8oR3c+omLHCGaqUL127rCmD11gEiaVQ sczE3bC7A3GtqKigM/QORmBDRzAQxNyf5CIhQr08wN2BZ6x/rniwP41naaew5Xpy3PSM 29khRY+ZGH8Shx/1Env1zIFRH5WzKscDSdiZzkpmkaO3n8s+bH7QsG+oLENMaNI7c57B Lseg== X-Gm-Message-State: AOAM532/30L1BOFrwTEa7NxWAk9aq4S98QN31QuvJ4m4lDtIvPTqdg/S cuWed7NZheexeD4ZEQGpw8Hh0jSb0yX1zWX7sFYKTD4RLVxasMGmKA0WXKJc26sODyqaSUFh+RD Bm2B6TZuws63yW0esSj2ygVDmF/imx3A= X-Received: by 2002:a05:6102:320c:b0:32c:ffc1:ff1c with SMTP id r12-20020a056102320c00b0032cffc1ff1cmr2328573vsf.35.1652456384386; Fri, 13 May 2022 08:39:44 -0700 (PDT) X-Google-Smtp-Source: ABdhPJybIvX8e+RFJWN9lkDTC5PRDamGgAs+TyjEYlKJnYc0iCUT0bWKU+EAJZI2r8L+j7YJT2by50Gl4/oG7IMLLLw= X-Received: by 2002:a05:6102:320c:b0:32c:ffc1:ff1c with SMTP id r12-20020a056102320c00b0032cffc1ff1cmr2328556vsf.35.1652456384163; Fri, 13 May 2022 08:39:44 -0700 (PDT) MIME-Version: 1.0 References: <20220513000609.197906-1-jsnow@redhat.com> <0248b5df-dbc6-48bf-b5c8-e5c73decc1c1@redhat.com> In-Reply-To: <0248b5df-dbc6-48bf-b5c8-e5c73decc1c1@redhat.com> From: John Snow Date: Fri, 13 May 2022 11:39:33 -0400 Message-ID: Subject: Re: [RFC PATCH 0/9] tests: run python tests under the build/tests/venv environment To: Paolo Bonzini Cc: qemu-devel , Qemu-block , Cleber Rosa , =?UTF-8?B?QWxleCBCZW5uw6ll?= , Hanna Reitz , Thomas Huth , Daniel Berrange , Kevin Wolf , =?UTF-8?Q?Philippe_Mathieu=2DDaud=C3=A9?= , Wainer dos Santos Moschetta , Beraldo Leal Content-Type: multipart/alternative; boundary="000000000000c904b105dee67bef" Received-SPF: pass client-ip=170.10.133.124; envelope-from=jsnow@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -28 X-Spam_score: -2.9 X-Spam_bar: -- X-Spam_report: (-2.9 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.082, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_LOW=-0.7, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --000000000000c904b105dee67bef Content-Type: text/plain; charset="UTF-8" On Fri, May 13, 2022, 6:21 AM Paolo Bonzini wrote: > On 5/13/22 02:06, John Snow wrote: > > The only downside I am really frowning at is that I will have to > > replicate some "update the venv if it's outdated" logic that is usually > > handled by the Make system in the venv bootstrapper. Still, I think it's > > probably the only way to hit all of the requirements here without trying > > to concoct a fairly complex Makefile. > > > > any thoughts? If not, I'll just move on to trying to hack up that > > version next instead. > > I would not even bother with keeping the venv up to date. Just initialize > I'm worried about this idea being very inconvenient for iterative development of the python code. it in configure, this is exactly what configure remains useful for in the > Meson-based world: > > - add configure options --enable-python-qemu={enabled,system,internal,pip, > auto}/--disable-python-qemu (auto means system>internal>pip>disabled; > enabled means > system>internal>pip>error) and matching CONFIG_PYTHON_QEMU=y to > config-host.mak > I'm not sure this makes sense. python/qemu will continue to exist in-tree and will only ever be "internal" in that sense. It won't be something you can wholesale install from pip. i.e. I plan to continue to break off pieces and upstream them, but I intend to leave several modules as internal only. So I'm not sure "internal" vs "pip" makes sense config-wise, it's intended to be a mixture of both, really. But, I suppose this is how you'd like to address different venv setup behaviors to accommodate spec builds vs dev builds - with a configure flag of some kind. (I suppose you'd then like to see configure error out if it doesn't have the necessary requisites given the venv-style chosen?) - use CONFIG_PYTHON_QEMU to enable/disable iotests in > tests/qemu-iotests/meson.build > So it's just skipped if you don't have the reqs to make the venv? (Not an error?) > - add a configure option --enable-avocado= > {system,pip,auto,enabled}/--disable-avocado and matching > CONFIG_AVOCADO=y to config-host.mak > > - use it to enable/disable acceptance tests in tests/Makefile.include > And this is how you propose eliminating the need for an always-present avocado builddep. > - build the venv in configure and use the options to pick the right pip > install > commands, like > > has_python_module() { > $python -c "import $1" > /dev/null 2>&1 > } > > # do_pip VENV-PATH VAR PACKAGE [PATH] -- PIP-OPTIONS > do_pip() { > local num_args source > num_args=5 > test $4 = '--' && num_args=4 > eval source=\$$2 > # Try to resolve the package using a system install > case $source in > enabled|auto|system) > if has_python_module $3; then > source=system > elif test $source = system; then > error_exit "Python package $3 not installed" > fi > esac > # See if a bundled copy is present > case $source in > enabled|auto|internal) > if test $num_args = 5 && test -f $4/setup.cfg; then > source=internal > elif test $source = internal; then > error_exit "Sources for Python package $3 not found in the QEMU > source tree" > fi > esac > # Install the bundled copy or let pip download the package > case $source in > internal) > # The Pip error message should be clear enough > (cd $1 && . bin/activate && pip install "$@") || exit 1 > ;; > enabled|auto|pip) > shift $num_args > if (cd $1 && . bin/activate && pip install "$@"); then > source=pip > elif test $source = auto; then > source=disabled > else > # The Pip error message should be clear enough > exit 1 > fi > ;; > esac > eval $2=\$source > } > > rm -rf venv/ > $python -m venv venv/ > do_pip venv/ enable_python_qemu qemu.qmp python/qemu -- qemu.qmp > do_pip venv/ enable_avocado avocado -- -r tests/requirements.txt > Won't this rebuild the venv like *all of the time*, basically whenever you see the "configuration is stale" message? That both seems like way too often, *and* it wouldn't cover cases when it really ought to be refreshed. --000000000000c904b105dee67bef Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Fri, May 13, 2022, 6:21 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
On 5/13/22 02:06, John Snow wrote:
> The only downside I am really frowning at is that I will have to
> replicate some "update the venv if it's outdated" logic = that is usually
> handled by the Make system in the venv bootstrapper. Still, I think it= 's
> probably the only way to hit all of the requirements here without tryi= ng
> to concoct a fairly complex Makefile.
>
> any thoughts? If not, I'll just move on to trying to hack up that<= br> > version next instead.

I would not even bother with keeping the venv up to date.=C2=A0 Just initia= lize

I'm worried about this idea being very inconvenient for iterative d= evelopment of the python code.

it in configure, this is exactly what configure remains useful for in the Meson-based world:

- add configure options --enable-python-qemu=3D{enabled,system,internal,pip= ,
auto}/--disable-python-qemu (auto means system>internal>pip>disabl= ed; enabled means
system>internal>pip>error) and matching CONFIG_PYTHON_QEMU=3Dy to<= br> config-host.mak

I'm not sure this makes sense. python/qemu will continue= to exist in-tree and will only ever be "internal" in that sense.= It won't be something you can wholesale install from pip.

i.e. I plan to continue to break off= pieces and upstream them, but I intend to leave several modules as interna= l only.

So I'm not s= ure "internal" vs "pip" makes sense config-wise, it'= ;s intended to be a mixture of both, really.

But, I suppose this is how you'd like to address d= ifferent venv setup behaviors to accommodate spec builds vs dev builds - wi= th a configure flag of some kind.

(I suppose you'd then like to see configure error out if it d= oesn't have the necessary requisites given the venv-style chosen?)

<= blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1px= #ccc solid;padding-left:1ex"> - use CONFIG_PYTHON_QEMU to enable/disable iotests in tests/qemu-iotests/me= son.build

So it's just skipped if you don't have the reqs to make t= he venv? (Not an error?)


- add a configure option --enable-avocado=3D
{system,pip,auto,enabled}/--disable-avocado and matching
CONFIG_AVOCADO=3Dy to config-host.mak

- use it to enable/disable acceptance tests in tests/Makefile.include

And th= is is how you propose eliminating the need for an always-present avocado bu= ilddep.


- build the venv in configure and use the options to pick the right pip ins= tall
commands, like

has_python_module() {
=C2=A0 =C2=A0$python -c "import $1" > /dev/null 2>&1 }

# do_pip VENV-PATH VAR PACKAGE [PATH] -- PIP-OPTIONS
do_pip() {
=C2=A0 =C2=A0 =C2=A0local num_args source
=C2=A0 =C2=A0 =C2=A0num_args=3D5
=C2=A0 =C2=A0 =C2=A0test $4 =3D '--' && num_args=3D4
=C2=A0 =C2=A0 =C2=A0eval source=3D\$$2
=C2=A0 =C2=A0 =C2=A0# Try to resolve the package using a system install
=C2=A0 =C2=A0 =C2=A0case $source in
=C2=A0 =C2=A0 =C2=A0 =C2=A0enabled|auto|system)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if has_python_module $3; then
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0source=3Dsystem
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0elif test $source =3D system; then
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0error_exit "Python package $3= not installed"
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0fi
=C2=A0 =C2=A0 =C2=A0esac
=C2=A0 =C2=A0 =C2=A0# See if a bundled copy is present
=C2=A0 =C2=A0 =C2=A0case $source in
=C2=A0 =C2=A0 =C2=A0 =C2=A0enabled|auto|internal)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if test $num_args =3D 5 && test -= f $4/setup.cfg; then
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0source=3Dinternal
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0elif test $source =3D internal; then
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0error_exit "Sources for Pytho= n package $3 not found in the QEMU source tree"
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0fi
=C2=A0 =C2=A0 =C2=A0esac
=C2=A0 =C2=A0 =C2=A0# Install the bundled copy or let pip download the pack= age
=C2=A0 =C2=A0 =C2=A0case $source in
=C2=A0 =C2=A0 =C2=A0 =C2=A0internal)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0# The Pip error message should be clear e= nough
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(cd $1 && . bin/activate &&am= p; pip install "$@") || exit 1
=C2=A0 =C2=A0 =C2=A0 =C2=A0;;
=C2=A0 =C2=A0 =C2=A0 =C2=A0enabled|auto|pip)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0shift $num_args
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (cd $1 && . bin/activate &= & pip install "$@"); then
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0source=3Dpip
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0elif test $source =3D auto; then
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0source=3Ddisabled
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0else
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0# The Pip error message should be = clear enough
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0exit 1
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0fi
=C2=A0 =C2=A0 =C2=A0 =C2=A0;;
=C2=A0 =C2=A0 =C2=A0esac
=C2=A0 =C2=A0 =C2=A0eval $2=3D\$source
}

rm -rf venv/
$python -m venv venv/
do_pip venv/ enable_python_qemu qemu.qmp python/qemu -- qemu.qmp
do_pip venv/ enable_avocado avocado -- -r tests/requirements.txt

Won't t= his rebuild the venv like *all of the time*, basically whenever you see the= "configuration is stale" message?

That both seems like way too often, *and* it wouldn= 9;t cover cases when it really ought to be refreshed.

--000000000000c904b105dee67bef--