All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Thomas Huth <thuth@redhat.com>
Cc: "Ani Sinha" <ani@anisinha.ca>,
	"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: Why we should avoid new submodules if possible
Date: Tue, 28 Jun 2022 06:30:16 -0400	[thread overview]
Message-ID: <20220628062551-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <d7a7b28f-a665-2567-0fb6-e31e7ecbb5c8@redhat.com>

On Tue, Jun 28, 2022 at 12:21:39PM +0200, Thomas Huth wrote:
> On 28/06/2022 12.03, Michael S. Tsirkin wrote:
> [...]
> > 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.
> 
> As far as I know, you can also specify the version when using pip, can't
> you? So that's not really an advantage here.

But of course if you do you do not get updates ;) You do
however rely on a 3rd party to faithfully provide you
correct code based on the version, and host it forever.

> On the contrary, submodules have a couple of disadvantages that I really
> dislike:
> 
> - submodules do not get updated automatically when doing a "git checkout",
> we have to update them via a script instead. This causes e.g. trouble if you
> rsync your source tree to a machine that has no access to the internet and
> you forgot to update the submodule before the sync

how is pip better?

> - the content of submodules is not added to the tarballs that get created on
> the git forges automatically. There were lots of requests from users in the
> past that tried to download a tarball from github and then wondered why they
> couldn't compile QEMU.

how is pip better here?

> - we include the submodule content in our release tarballs, so people get
> the impression that hte submodule content is part of the QEMU sources. This
> has two disadvantages:
>  * We already got bug reports for the code in the submodule,
>    where people did not understand that they should report that
>    rather to the original project instead (i.e. you ship it - you
>    own it)
>  * People get the impression that QEMU is a huge monster
>    application if they count the number of code lines, run
>    their code scanner tools on the tarball contents, etc.
>    Remember "nemu", for example, where one of the main complaints
>    was that QEMU has too many lines of code?

I think we can skip the checkout in the tarball if we like.
If people want to run the test they can checkout then.


> - If programs includes code via submodules, this gets a higher
>   burder for distro maintainers, since they have to patch each
>   and every package when there is a bug, instead of being able to
>   fix it in one central place.

Come on, this is just a test. We *really* don't care if an ISO
we use to test ACPI is using an exploitable version of grub.

> So in my opinion we should avoid new submodules if there is an alternative.
> 
>  Thomas

Interesting take generally, but I don't see how the logic applies in this
case. Would appreciate hearing your answers to the above.

-- 
MST



  reply	other threads:[~2022-06-28 10:38 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
2022-06-28 10:21                                       ` Why we should avoid new submodules if possible Thomas Huth
2022-06-28 10:30                                         ` Michael S. Tsirkin [this message]
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=20220628062551-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 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.