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 E11EBC433EF for ; Thu, 26 May 2022 14:22:51 +0000 (UTC) Received: from localhost ([::1]:48774 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1nuEO2-00067I-VK for qemu-devel@archiver.kernel.org; Thu, 26 May 2022 10:22:50 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:59622) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nuEGd-0000P3-Hp for qemu-devel@nongnu.org; Thu, 26 May 2022 10:15:11 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]:45726) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nuEGY-0000UW-Uw for qemu-devel@nongnu.org; Thu, 26 May 2022 10:15:09 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1653574505; 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=xp2UpmwVux/+/4VXJHs/qf1i8d4GnriKqGGVTHZVtzo=; b=iqNCXOAuOBImJzoiggaD+b2dcGK4i+03/8iU80omGTxb1SJLoiGTpIwous/JddYYDfzovb Cb8D/DsvWGnAt7TE2lhlH7jVAgAHXkTlKcvou1Z/N9fT04UVV8aFHHeu9iSOTGL5EtynlZ 0CNAvE1V3m9QdileG0/9+m4c4b7Jwag= Received: from mail-vk1-f198.google.com (mail-vk1-f198.google.com [209.85.221.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-319-rtQ6N20ANFiFAg-ORtmxuw-1; Thu, 26 May 2022 10:15:03 -0400 X-MC-Unique: rtQ6N20ANFiFAg-ORtmxuw-1 Received: by mail-vk1-f198.google.com with SMTP id m65-20020a1f5844000000b0035adfd2bc2dso52471vkb.8 for ; Thu, 26 May 2022 07:15:03 -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=xp2UpmwVux/+/4VXJHs/qf1i8d4GnriKqGGVTHZVtzo=; b=Z025Sncwxtrsd4RjGWf4HcvMk3HtgxgI4Rz7X4r1OGjaRCyZRmwxaNrDX+Iq1yIM8v dBUomhxCLCOG3TjuuTNw0v9R6J5kmDExxqxvcR6jla0aNf/WbXM7uLZVb3Ts0JDtINBX Eo/v6CSAdUWa1VLihSp6Eg/IZLHu3GMu7RbDGTXl0cr0cJl265rQqD3ogkj+fE/q94bi NgIgS+wUz2suzhOTyQwnVyuOrcZLwxNRQ4vVgsmrIUUE+cLhi6yJAy7SUKs1bpfeWq5Z NW9S6Yb1HXIMWzFZU4EiWndG2uU99WAjRStuhw6p5pauLXwgMkfI+HyOtFyEcQWPSUH0 IV0g== X-Gm-Message-State: AOAM5337LFcrlxH5//AatE7Nx7VLyqJjWLCjs5hTl4t1qoKaK52umysB nmT0LCwDJFX6E+SkRf2dm7RZwTzNuma/Z0Xnp/b3ZltoKsoPTtvnuLfTAHFMWZZ+F/1cCUT9TAh 7xMaPVSHbMIJ88JKEX4p01rBeQmKmEtc= X-Received: by 2002:ab0:2905:0:b0:369:1fcf:2ee9 with SMTP id v5-20020ab02905000000b003691fcf2ee9mr3054904uap.119.1653574503315; Thu, 26 May 2022 07:15:03 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwKDEmyroID8DjhTyM01eI5C83TqvPSHP9bwPt95CYRK0xa1YYnz5l5RP69sV1eF0vLZYu/WL9kGjrQtDEF7OM= X-Received: by 2002:ab0:2905:0:b0:369:1fcf:2ee9 with SMTP id v5-20020ab02905000000b003691fcf2ee9mr3054881uap.119.1653574503077; Thu, 26 May 2022 07:15:03 -0700 (PDT) MIME-Version: 1.0 References: <20220526000921.1581503-1-jsnow@redhat.com> <20220526000921.1581503-10-jsnow@redhat.com> <319df99e-42d8-64dd-fbe8-a6f9311f3630@redhat.com> In-Reply-To: <319df99e-42d8-64dd-fbe8-a6f9311f3630@redhat.com> From: John Snow Date: Thu, 26 May 2022 10:14:52 -0400 Message-ID: Subject: Re: [PATCH 9/9] tests: run 'device-crash-test' from tests/venv To: Paolo Bonzini Cc: qemu-devel , =?UTF-8?B?QWxleCBCZW5uw6ll?= , Cleber Rosa , Thomas Huth , =?UTF-8?Q?Philippe_Mathieu=2DDaud=C3=A9?= , Wainer dos Santos Moschetta , Daniel Berrange , Beraldo Leal Content-Type: multipart/alternative; boundary="000000000000ddb10205dfead0af" Received-SPF: pass client-ip=170.10.129.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" --000000000000ddb10205dfead0af Content-Type: text/plain; charset="UTF-8" On Thu, May 26, 2022, 8:14 AM Paolo Bonzini wrote: > On 5/26/22 02:09, John Snow wrote: > > Remove the sys.path hacking from device-crash-test, and add in a little > > user-friendly message for anyone who was used to running this script > > directly from the source tree. > > > > Modify the GitLab job recipes to create the tests/venv first, then run > > device-crash-test from that venv. > > > > Signed-off-by: John Snow > > --- > > .gitlab-ci.d/buildtest.yml | 8 +++++--- > > scripts/device-crash-test | 14 +++++++++++--- > > 2 files changed, 16 insertions(+), 6 deletions(-) > > > > diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml > > index e9620c30748..fde29c35aa3 100644 > > --- a/.gitlab-ci.d/buildtest.yml > > +++ b/.gitlab-ci.d/buildtest.yml > > @@ -110,7 +110,8 @@ crash-test-debian: > > IMAGE: debian-amd64 > > script: > > - cd build > > - - scripts/device-crash-test -q ./qemu-system-i386 > > + - make check-venv > > + - tests/venv/bin/python3 scripts/device-crash-test -q > ./qemu-system-i386 > > > > build-system-fedora: > > extends: .native_build_job_template > > @@ -155,8 +156,9 @@ crash-test-fedora: > > IMAGE: fedora > > script: > > - cd build > > - - scripts/device-crash-test -q ./qemu-system-ppc > > - - scripts/device-crash-test -q ./qemu-system-riscv32 > > + - make check-venv > > + - tests/venv/bin/python3 scripts/device-crash-test -q > ./qemu-system-ppc > > + - tests/venv/bin/python3 scripts/device-crash-test -q > ./qemu-system-riscv32 > > > > build-system-centos: > > extends: .native_build_job_template > > diff --git a/scripts/device-crash-test b/scripts/device-crash-test > > index a203b3fdea2..73bcb986937 100755 > > --- a/scripts/device-crash-test > > +++ b/scripts/device-crash-test > > @@ -33,10 +33,18 @@ import re > > import random > > import argparse > > from itertools import chain > > +from pathlib import Path > > > > -sys.path.append(os.path.join(os.path.dirname(__file__), '..', 'python')) > > -from qemu.machine import QEMUMachine > > -from qemu.qmp import ConnectError > > +try: > > + from qemu.machine import QEMUMachine > > + from qemu.qmp import ConnectError > > +except ModuleNotFoundError as exc: > > + path = Path(__file__).resolve() > > + print(f"Module '{exc.name}' not found.") > > + print(" Try 'make check-venv' from your build directory,") > > + print(" and then one way to run this script is like so:") > > + print(f' > $builddir/tests/venv/bin/python3 "{path}"') > > + sys.exit(1) > > > > logger = logging.getLogger('device-crash-test') > > dbg = logger.debug > > Reviewed-by: Paolo Bonzini > > Even though I'd still prefer the venv to be setup early (so the > check-venv change in buildtest.yml and the friendly message in the > script will go away), this is a step in the right direction. > > Paolo > Agree, figured I'd do baby steps before I wound up with a 40 patch series, and this gives Thomas et al a chance to find out if this ruins their workflow. (I'll probably keep the friendly message a little while more anyway, though; to catch anyone who runs this script manually for a release or so. I should add a section to our QEMU developer's guide and just link to it in the message and explain the many, many ways you might enter a venv or install the package.) --000000000000ddb10205dfead0af Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Thu, May 26, 2022, 8:14 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
On 5/26/22 02:09, John Snow wrote:
> Remove the sys.path hacking from device-crash-test, and add in a littl= e
> user-friendly message for anyone who was used to running this script > directly from the source tree.
>
> Modify the GitLab job recipes to create the tests/venv first, then run=
> device-crash-test from that venv.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>=C2=A0 =C2=A0.gitlab-ci.d/buildtest.yml |=C2=A0 8 +++++---
>=C2=A0 =C2=A0scripts/device-crash-test=C2=A0 | 14 +++++++++++---
>=C2=A0 =C2=A02 files changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml > index e9620c30748..fde29c35aa3 100644
> --- a/.gitlab-ci.d/buildtest.yml
> +++ b/.gitlab-ci.d/buildtest.yml
> @@ -110,7 +110,8 @@ crash-test-debian:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0IMAGE: debian-amd64
>=C2=A0 =C2=A0 =C2=A0script:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0- cd build
> -=C2=A0 =C2=A0 - scripts/device-crash-test -q ./qemu-system-i386
> +=C2=A0 =C2=A0 - make check-venv
> +=C2=A0 =C2=A0 - tests/venv/bin/python3 scripts/device-crash-test -q .= /qemu-system-i386
>=C2=A0 =C2=A0
>=C2=A0 =C2=A0build-system-fedora:
>=C2=A0 =C2=A0 =C2=A0extends: .native_build_job_template
> @@ -155,8 +156,9 @@ crash-test-fedora:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0IMAGE: fedora
>=C2=A0 =C2=A0 =C2=A0script:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0- cd build
> -=C2=A0 =C2=A0 - scripts/device-crash-test -q ./qemu-system-ppc
> -=C2=A0 =C2=A0 - scripts/device-crash-test -q ./qemu-system-riscv32 > +=C2=A0 =C2=A0 - make check-venv
> +=C2=A0 =C2=A0 - tests/venv/bin/python3 scripts/device-crash-test -q .= /qemu-system-ppc
> +=C2=A0 =C2=A0 - tests/venv/bin/python3 scripts/device-crash-test -q .= /qemu-system-riscv32
>=C2=A0 =C2=A0
>=C2=A0 =C2=A0build-system-centos:
>=C2=A0 =C2=A0 =C2=A0extends: .native_build_job_template
> diff --git a/scripts/device-crash-test b/scripts/device-crash-test
> index a203b3fdea2..73bcb986937 100755
> --- a/scripts/device-crash-test
> +++ b/scripts/device-crash-test
> @@ -33,10 +33,18 @@ import re
>=C2=A0 =C2=A0import random
>=C2=A0 =C2=A0import argparse
>=C2=A0 =C2=A0from itertools import chain
> +from pathlib import Path
>=C2=A0 =C2=A0
> -sys.path.append(os.path.join(os.path.dirname(__file__), '..',= 'python'))
> -from qemu.machine import QEMUMachine
> -from qemu.qmp import ConnectError
> +try:
> +=C2=A0 =C2=A0 from qemu.machine import QEMUMachine
> +=C2=A0 =C2=A0 from qemu.qmp import ConnectError
> +except ModuleNotFoundError as exc:
> +=C2=A0 =C2=A0 path =3D Path(__file__).resolve()
> +=C2=A0 =C2=A0 print(f"Module '{exc.name}' not found= .")
> +=C2=A0 =C2=A0 print("=C2=A0 Try 'make check-venv' from y= our build directory,")
> +=C2=A0 =C2=A0 print("=C2=A0 and then one way to run this script = is like so:")
> +=C2=A0 =C2=A0 print(f'=C2=A0 > $builddir/tests/venv/bin/python= 3 "{path}"')
> +=C2=A0 =C2=A0 sys.exit(1)
>=C2=A0 =C2=A0
>=C2=A0 =C2=A0logger =3D logging.getLogger('device-crash-test')<= br> >=C2=A0 =C2=A0dbg =3D logger.debug

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Even though I'd still prefer the venv to be setup early (so the
check-venv change in buildtest.yml and the friendly message in the
script will go away), this is a step in the right direction.

Paolo

Agree, figured I'd do baby steps before I wound up with a 40 patch= series, and this gives Thomas et al a chance to find out if this ruins the= ir workflow.

(I'll p= robably keep the friendly message a little while more anyway, though; to ca= tch anyone who runs this script manually for a release or so. I should add = a section to our QEMU developer's guide and just link to it in the mess= age and explain the many, many ways you might enter a venv or install the p= ackage.)

--000000000000ddb10205dfead0af--