qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Ani Sinha <ani@anisinha.ca>
Cc: "Thomas Huth" <thuth@redhat.com>,
	"Daniel P. Berrangé" <berrange@redhat.com>,
	"John Snow" <jsnow@redhat.com>,
	"Laurent Vivier" <lvivier@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	imammedo@redhat.com, qemu-devel@nongnu.org
Subject: Re: venv for python qtest bits? (was: Re: [PATCH 11/12] acpi/tests/bits: add README file for bits qtests)
Date: Tue, 28 Jun 2022 06:03:56 -0400	[thread overview]
Message-ID: <20220628060210-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <CAARzgwxcjppQuO65aFzyzNBaFvJer7JEWoJeALaoKON=3XAQhg@mail.gmail.com>

On Tue, Jun 28, 2022 at 02:19:41PM +0530, Ani Sinha wrote:
> 
> 
> On Tue, Jun 28, 2022 at 14:05 Ani Sinha <ani@anisinha.ca> wrote:
> 
>     On Tue, Jun 28, 2022 at 1:58 PM Thomas Huth <thuth@redhat.com> wrote:
>     >
>     > On 28/06/2022 10.23, Daniel P. Berrangé wrote:
>     > > On Tue, Jun 28, 2022 at 01:21:35PM +0530, Ani Sinha wrote:
>     > >> On Tue, Jun 28, 2022 at 1:19 PM Daniel P. Berrangé <
>     berrange@redhat.com> wrote:
>     > >>>
>     > >>> On Tue, Jun 28, 2022 at 09:25:35AM +0200, Thomas Huth wrote:
>     > >>>> On 28/06/2022 09.10, Michael S. Tsirkin wrote:
>     > >>>>> On Tue, Jun 28, 2022 at 09:03:33AM +0200, Thomas Huth wrote:
>     > >>>>>>>>>>>> No problem with that. So that's venv. But do we need pip and
>     pulling
>     > >>>>>>>>>>>> packages from the net during testing?
>     > >>>>>>>>>>>
>     > >>>>>>>>>>> We do that too. See requirements.txt in tests/
>     > >>>>>>>>>>> Following two are downloaded:
>     > >>>>>>>>>>> avocado-framework==88.1
>     > >>>>>>>>>>> pycdlib==1.11.0
>     > >>>>>>>>>>>
>     > >>>>>>>>>>> Also see this line in Makefie.include:
>     > >>>>>>>>>>>
>     > >>>>>>>>>>> $(call quiet-venv-pip,install -r $(TESTS_VENV_REQ))
>     > >>>>>>>>>>
>     > >>>>>>>>>> Right but that's avocado since it pulls lots of stuff from
>     > >>>>>>>>>> the net anyway.
>     > >>>>>>>>>> Are the libraries in question not packaged on major distros?
>     > >>>>>>>>>
>     > >>>>>>>>> Currently I only need this:
>     > >>>>>>>>> https://github.com/python-tap/tappy
>     > >>>>>>>>> which is the basic TAP processing library for python.
>     > >>>>>>>>>
>     > >>>>>>>>> It seems its only installed through pip:
>     > >>>>>>>>> https://tappy.readthedocs.io/en/latest/
>     > >>>>>>>>>
>     > >>>>>>>>> I do not think this is packaged by default. It's such a basic
>     library
>     > >>>>>>>>> for parsing test output that maybe we can keep this somewhere
>     within
>     > >>>>>>>>> the python src tree? Not sure ...
>     > >>>>>>>>
>     > >>>>>>>> It's pretty small for sure. Another submodule?
>     > >>>>>>>
>     > >>>>>>> Unlike BITS, this one is likely going to be maintained for a
>     while and
>     > >>>>>>> will receive new releases through
>     > >>>>>>> https://pypi.org/project/tap.py/
>     > >>>>>>> so forking is OK but someone has to keep this updated.
>     > >>>>>>>
>     > >>>>>>> I am open to anything. Whatever feels right is fine to me.
>     > >>>>>>
>     > >>>>>> John Snow is currently working on the "Pythonification" of various
>     QEMU
>     > >>>>>> bits, I think you should loop him into this discussion, too.
>     > >>>>>>
>     > >>>>>>    Thomas
>     > >>>>>
>     > >>>>> submodule does not mean we fork necessarily. We could have
>     > >>>>> all options: check for the module and use it if there, if not
>     > >>>>> use one from system if not there install with pip ..
>     > >>>>> But yea, I'm not sure what's best either.
>     > >>>>
>     > >>>> submodules create a dependency on an internet connection, too. So
>     before you
>     > >>>> add yet another submodule (which have a couple of other
>     disadvantages), I
>     > >>>> think you could also directly use the venv here.
>     > >>>
>     > >>> Definitely not submodules.
>     > >>>
>     > >>> We need to get out of the mindset that submodules are needed for
>     every new
>     > >>> dependancy we add. Submodules are only appropriate if the external
>     project
>     > >>> is designed to be used as a copylib (eg the keycodemapdb tool), or if
>     we
>     > >>> need to bundle in order to prevent a regression for previously
>     deployed
>     > >>> QEMU installs where the dependancy is known not to exist on all our
>     > >>> supported platforms.
>     > >>>
>     > >>> This does not apply in this case, because the proposed use of tappy
>     is
>     > >>> merely for a test case. Meson just needs to check if tappy exists and
>     if
>     > >>> it does, then use it, otherwise skip the tests that need it. The user
>     can
>     > >>> arrange to install tappy, as they do with the majority of other deps.
>     > >>>
>     > >>> If John's venv stuff is relevant, then we don't even need the meson
>     checks,
>     > >>> just delegate to the venv setup.
>     > >>>
>     > >>> Regardless, no submodules are needed or desirable.
>     > >>
>     > >> What about keeping biosbits stuff? Source or pre-built.
>     > >
>     > > Shipping them as pre-built binaries in QEMU is not a viable option
>     > > IMHO, especially for grub as a GPL'd project we need to be extremely
>     > > clear about the exact corresponding source and build process for any
>     > > binary.
>     > >
>     > > For this kind of thing I would generally expect the distro to provide
>     > > packages that we consume. Looking at biosbits I see it is itself
>     > > bundling a bunch more 3rd party projects, libffi, grub2, and including
>     > > even an ancient version of python as a submodule.
>     > >
>     > > So bundling a pre-built biosbits in QEMU appears to mean that we're in
>     > > turn going to unexpectedly bundle a bunch of other 3rd party projects
>     > > too, all with dubious license compliance. I don't think this looks like
>     > > something we should have in qemu.git or qemu tarballs. It will also
>     > > make it challenging for the distro to take biosbits at all, unless
>     > > those 3rd party bundles can be eliminated in favour of using existing
>     > > builds their have packaged for grub, python, libffi, etc.
>     >
>     > So if this depends on some third party binary bits, I think this is
>     pretty
>     > similar to the tests in the avocado directory ... there we download third
>     > party binaries, too... Wouldn't it make sense to adapt your tests to that
>     > framework?
> 
>     I do not want to bring in the whole avocado framework because it would
>     unnecessarily make things complicated. I just need the qemu machine
>     python library and that is enough. For downloading third party stuff,
> 
>     we can simply wget things from somewhere.
> 
> 
> https://pypi.org/project/wget/
> 
> That get_asset() call is an overkill for downloading two archives. 
> 

For biosbits if we are going this route then I feel a submodule is much
better.  It records which version exactly each qemu version wants.


> 
> 
>     >
>     >   Thomas
>     >
> 



  reply	other threads:[~2022-06-28 10:07 UTC|newest]

Thread overview: 118+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-27  7:28 [PATCH 00/12] Introduce new acpi/smbios qtests using biosbits Ani Sinha
2022-06-27  7:28 ` [PATCH 01/12] qtest: meson.build changes required to integrate python based qtests Ani Sinha
2022-06-27  7:28 ` [PATCH 04/12] acpi/tests/bits: initial commit of test scripts that are run by biosbits Ani Sinha
2022-06-28  7:24   ` Thomas Huth
2022-06-28  9:52     ` Michael S. Tsirkin
2022-06-27  7:28 ` [PATCH 05/12] acpi/tests/bits: disable acpi PSS tests that are failing in biosbits Ani Sinha
2022-06-27  7:28 ` [PATCH 06/12] acpi/tests/bits: add smilatency test suite from bits in order to disable it Ani Sinha
2022-06-27  7:28 ` [PATCH 07/12] acpi/tests/bits: disable smilatency test since it does not pass everytime Ani Sinha
2022-06-27  7:28 ` [PATCH 08/12] acpi/tests/bits: add biosbits config file for running bios tests Ani Sinha
2022-06-27  7:28 ` [PATCH 09/12] acpi/tests/bits: add acpi and smbios python tests that uses biosbits Ani Sinha
2022-06-28  7:20   ` Thomas Huth
2022-06-28  7:26     ` Ani Sinha
2022-06-28  7:36       ` Thomas Huth
2022-06-28  9:55       ` Michael S. Tsirkin
2022-06-28 10:00         ` Thomas Huth
2022-06-27  7:28 ` [PATCH 10/12] acpi/tests/bits: add acpi bits qtest directory in meson for running tests Ani Sinha
2022-06-27  7:28 ` [PATCH 11/12] acpi/tests/bits: add README file for bits qtests Ani Sinha
2022-06-27 22:26   ` Michael S. Tsirkin
2022-06-28  4:57     ` Ani Sinha
2022-06-28  6:06       ` Michael S. Tsirkin
2022-06-28  6:16         ` Ani Sinha
2022-06-28  6:20           ` Michael S. Tsirkin
2022-06-28  6:36             ` Ani Sinha
2022-06-28  6:50               ` Michael S. Tsirkin
2022-06-28  6:57                 ` Ani Sinha
2022-06-28  7:03                   ` venv for python qtest bits? (was: Re: [PATCH 11/12] acpi/tests/bits: add README file for bits qtests) Thomas Huth
2022-06-28  7:10                     ` Michael S. Tsirkin
2022-06-28  7:25                       ` Thomas Huth
2022-06-28  7:48                         ` Daniel P. Berrangé
2022-06-28  7:51                           ` Ani Sinha
2022-06-28  8:23                             ` Daniel P. Berrangé
2022-06-28  8:28                               ` Thomas Huth
2022-06-28  8:35                                 ` Ani Sinha
2022-06-28  8:49                                   ` Ani Sinha
2022-06-28 10:03                                     ` Michael S. Tsirkin [this message]
2022-06-28 10:21                                       ` Why we should avoid new submodules if possible Thomas Huth
2022-06-28 10:30                                         ` Michael S. Tsirkin
2022-06-28 10:43                                           ` Peter Maydell
2022-06-28 11:00                                             ` Michael S. Tsirkin
2022-06-28 14:54                                               ` Warner Losh
2022-09-28 20:48                                               ` Michal Suchánek
2022-09-28 21:07                                                 ` Michael S. Tsirkin
2022-09-28 21:43                                                   ` Michal Suchánek
2022-06-28 10:50                                           ` Thomas Huth
2022-06-28 11:14                                             ` Michael S. Tsirkin
2022-06-28 12:39                                               ` Thomas Huth
2022-06-28 14:45                                                 ` Michael S. Tsirkin
2022-06-28 15:54                                                 ` Ani Sinha
2022-06-28 16:15                                                   ` Daniel P. Berrangé
2022-06-28 18:00                                                     ` Michael S. Tsirkin
2022-06-29  6:28                                                       ` Ani Sinha
2022-07-01  3:34                                                         ` Thomas Huth
2022-07-02  0:05                                                           ` Philippe Mathieu-Daudé via
2022-09-28  9:26                                         ` Michael S. Tsirkin
2022-09-28  9:33                                           ` Thomas Huth
2022-09-28  9:47                                             ` Michael S. Tsirkin
2022-09-28  9:55                                               ` Thomas Huth
2022-09-28  9:37                                           ` Daniel P. Berrangé
2022-09-28  9:53                                             ` Michael S. Tsirkin
2022-09-28  9:57                                               ` Daniel P. Berrangé
2022-09-28 10:07                                                 ` Michael S. Tsirkin
2022-09-28 13:15                                                 ` Warner Losh
2022-09-28 13:22                                                   ` Michael S. Tsirkin
2022-09-28 10:13                                             ` Michael S. Tsirkin
2022-09-28 10:18                                               ` Daniel P. Berrangé
2022-09-28 13:12                                                 ` Michael S. Tsirkin
2022-09-28 15:07                                                   ` Peter Maydell
2022-09-28 19:59                                                     ` Michael S. Tsirkin
2022-09-28 13:06                                               ` Warner Losh
2022-06-28 10:04                                   ` venv for python qtest bits? (was: Re: [PATCH 11/12] acpi/tests/bits: add README file for bits qtests) Daniel P. Berrangé
2022-06-28 10:07                                     ` Michael S. Tsirkin
2022-06-28 10:18                                       ` Daniel P. Berrangé
2022-06-28 10:25                                         ` Michael S. Tsirkin
2022-06-28 10:41                                         ` Ani Sinha
2022-06-28 10:28                                       ` Ani Sinha
2022-06-28 10:42                                         ` Daniel P. Berrangé
2022-06-28 11:18                                           ` Michael S. Tsirkin
2022-06-28 11:28                                           ` Michael S. Tsirkin
2022-06-28 12:10                                             ` Peter Maydell
2022-06-28 12:36                                               ` Ani Sinha
2022-06-28 12:42                                                 ` Thomas Huth
2022-06-28 12:55                                                 ` Daniel P. Berrangé
2022-06-28 13:22                                                   ` Ani Sinha
2022-06-28 13:44                                                     ` Peter Maydell
2022-06-28 13:53                                                       ` Ani Sinha
2022-06-28 13:55                                                         ` Peter Maydell
2022-07-01  4:12                                                         ` Thomas Huth
2022-07-01  6:53                                                           ` Michael S. Tsirkin
2022-07-01  7:28                                                             ` Ani Sinha
2022-07-01  7:38                                                               ` Michael S. Tsirkin
2022-07-01  7:50                                                                 ` Ani Sinha
2022-07-01  9:42                                                                   ` Michael S. Tsirkin
2022-07-01 10:14                                                                     ` Ani Sinha
2022-07-01 12:54                                                                       ` Michael S. Tsirkin
2022-07-04 13:32                                                                         ` Ani Sinha
2022-07-05 13:48                                                                           ` Ani Sinha
2022-07-07 12:49                                                                 ` Ani Sinha
2022-06-28 14:41                                                     ` Michael S. Tsirkin
2022-06-28 14:38                                                   ` Michael S. Tsirkin
2022-06-28 10:14                                 ` Daniel P. Berrangé
2022-06-28 10:21                                   ` Michael S. Tsirkin
2022-06-28 10:30                                     ` Thomas Huth
2022-06-28 10:30                                   ` Ani Sinha
2022-06-28 10:49                                     ` Ani Sinha
2022-06-28 10:12                               ` Michael S. Tsirkin
2022-06-28 10:16                                 ` Daniel P. Berrangé
2022-06-28 10:00                           ` Michael S. Tsirkin
2022-06-28  7:49                         ` Ani Sinha
2022-06-28  7:53                           ` Thomas Huth
2022-06-28  9:53                         ` Michael S. Tsirkin
2022-06-28  7:05                   ` [PATCH 11/12] acpi/tests/bits: add README file for bits qtests Ani Sinha
2022-06-27  7:28 ` [PATCH 12/12] MAINTAINERS: add myself as the maintainer for acpi biosbits qtests Ani Sinha
2022-06-28  8:09 ` [PATCH 00/12] Introduce new acpi/smbios qtests using biosbits Daniel P. Berrangé
2022-06-28  8:33   ` Ani Sinha
2022-06-28 10:06     ` Daniel P. Berrangé
2022-06-28 10:16       ` Michael S. Tsirkin
2022-06-28 10:21         ` Daniel P. Berrangé
2022-06-28 10:35           ` Michael S. Tsirkin

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=20220628060210-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=ani@anisinha.ca \
    --cc=berrange@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=lvivier@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=thuth@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).