On Fri, Nov 20, 2020 at 12:24:38PM +0100, Andrea Bolognani wrote:
On Fri, 2020-11-20 at 01:19 +0100, Pavel Hrdina wrote:On Thu, Nov 19, 2020 at 11:38:28PM +0100, Martin Kletzander wrote: > Right now, IMHO, all meson checks for binaries that are not needed at build time > should be removed. During runtime we can just use the name of the binary. I > don't know whether it used to be the case that it was thought that there might > be security issues with supplying different binary in a directory in $PATH, but > frankly, if you have (different-)user-writable directory in $PATH or non-root > access to modifying system-wide $PATH then you have bigger problems to deal > with. Even though I do not have anything to back this claim I think that > might've been the original reason. That was my take on the original reasoning as well. I completely agree here with Martin and vote for removing these runtime binary checks from meson completely. There would be also the benefit for testing purposes that you can simply change the path to use your own compiled binary without changing anything in libvirt.While I would love the simplification such an approach would yield, I have to point out that there is at least one advantage to checking for the availability of commands at build time, even if those commands will only ever be invoked at runtime: it makes it obvious those commands are going to be needed later on. Take the zfs example again: if there was no build time checking, one might very reasonably assume that the zfs storage driver is usable on its own (the same way the iscsi-direct driver, for example, is); the fact that this is not the case would only become apparent much later, as you attempt to perform some storage operation and get back a failure.
This should be handled by a package maintainer who would put such requirement into the package for libvirt-deamon-driver-storage-zfs (or similar). It would just be in Requires instead of BuildRequires. Or DEPEND instead of RDEPEND. And based on what you say that's actaully what debian does, just with a hackish workaround. I understand that if we add new functionality that depends on a new binary and it only fails at runtime then it might happen that the maintainer misses it and people will run into an issue. It's most probably not going to be a regression, though. And we need to document it somewhere anyway. And my understanding is that the maintainer should at least roughly look at the changes. Maybe if we had like a purpose built file in the repository that would document the building process and requirements so that it was easier to filter the changes between releases to figure out if the build needs changing. Oh, wait, we have libvirt.spec.in ;) Honestly, I would not be against having the same file in the repo for every distro that ships libvirt, but that's another discussion. But anyway, just checking for changes in the spec file is enough.
So while libvirt obviously needs to be able to cope gracefully with the commands that were detected at build time not being present at runtime (which would indicate a packaging bug), I'm not convinced that dropping the build time checks is the best solution. Making them overridable, as they were with autotools, sounds like a more solid approach; and for those commands where we don't already check at build time, such a check should probably be introduced.
I would just like to know if there is anyone who needs them overridable for any other reason than to disable the build-time check. Because if no, then we're just talking about adding pointless complexity.
-- Andrea Bolognani / Red Hat / Virtualization
Attachment:
signature.asc
Description: PGP signature
