On Tue, Jun 28, 2022 at 12:50:06PM +0200, Thomas Huth wrote: > On 28/06/2022 12.30, Michael S. Tsirkin wrote: > > 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? > > You don't end up with an inconsistent source tree in that case (which > happens with submodules quite easily, at least for me it happened a couple > of times already). Either the machine has an internet connection, so that > pip can install the missing bits, or it does not and the test has to be > skipped.
skipped tests are too easy to ignore ... > But if I copy the wrong state of a submodule around, things get > messed up quite easily in my experience. Ok, you could say that this is just > my special setup with rsync, but already given the fact that "git checkout" > creates an inconsistent state of your source tree until you run the script > for updating the submodules the next time is an indication that submodules > are rather a shaky thing (e.g. if you'd create a tarball for somebody else > from your source tree right after doing a "git checkout"). yea one has to remember to set submodule.recurse = true in .gitconfig I agree it's annoying, I guess they don't change it for compat reasons. > > > - 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? > > You don't get incomplete/non-working tarballs in that case. So skip the test ;) > > > - 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. > > Release tarballs don't include the ".git" folder infrastructur, so everybody > who downloads a tarball will simply never be able to run the test. I actually think I'm fine with that for this specific case. > > > > > - 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. > > Wait, I thought we were only talking about tappy here? The ISO binaries > should certainly *not* be bundled in the QEMU tarballs (they are too big > already anyway, we should rather think of moving the firmware binaries out > of the tarball instead). > > Thomas IIUC there are three things we are discussing - biosbits source - biosbits image - tappy -- MST