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