On Fri, Nov 13, 2020 at 02:13:38PM +0100, Andrea Bolognani wrote:
On Fri, 2020-11-13 at 12:37 +0100, Pavel Hrdina wrote:On Thu, Nov 12, 2020 at 10:40:02PM +0100, Olaf Hering wrote: > Since meson does not support environment variables it seems the > only way to address this is to introduce an option in > meson_options.txt for each runtime executable. > > Will such change be accepted? This would be one way how to address runtime executables, currently we have a lot of them. These executables are checked in order to enable/disable some libvirt feature but is used only in runtime: parted, bhyve, bhyvectl, bhyveload, mount, umount, mkfs, pvcreate, vgcreate, lvcreate, pvremove, vgremove, lvremove, lvchange, vgchange, vgscan, pvs, vgs, lvs, (collie|dog), vstorage, vstorage-mount, zfs, zpool, numad This one is even more broken as if we don't find it during build time it is set to empty string and never checked in our code: showmount These are checked during build time but if not present they are set to known absolute path: qemu-bridge-helper, qemu-pr-helper, slirp-helper, dbus-daemon And we have a list of optional_programs that are checked during build time and if not present they are set to the name of the executable and resolved during runtime from $PATH. The last executable, pkcheck, is not used during build and in runtime. We only check its presence to enable/disable polkit support. We should be able to simply drop this check and figure out the presence of polkit using DBus only as we do for machined or logind and other DBus services that we use. All of this was copied from autoconf except for the fact that it is no longer possible to use ENV variables. I think we need to unify the process how we handle runtime executable dependencies and possibly add meson options for them.As another data point, Debian currently carries a patch[1] which allows us to enable the ZFS driver without installing the ZFS packages in the build environment: this is necessary because, due to its license, ZFS is kept outside of the main Debian repository. Being able to use something like -Dprog_zfs=/sbin/zfs -Dprog_zfspool=/sbin/zfspool to achieve the same result would allow us to drop that patch, which I would be *extremely* happy about :)
It sounds like most of the things are just caused by the binary being required at runtime while being checked during build time. Libvirt needs to be able to handle that missing binary at runtime anyway and we should just not require it at build time (or, if worse comes to worst, emit a non-fatal message about it missing) and that's it. When, and only when, someone needs to change the path to the executable we might have a discussion about dealing with that. Ideally we'd just get it fixed. We started with _some_ build-time requirements like that, but as far as I know nobody continued the work.
[1] https://salsa.debian.org/libvirt-team/libvirt/-/blob/debian/master/debian/patches/debian/Set-defaults-for-zfs-tools.patch -- Andrea Bolognani / Red Hat / Virtualization
Attachment:
signature.asc
Description: PGP signature
