Re: [libvirt PATCH 15/28] build: add nft to the list of binaries we attempt to locate

On Thu, May 04, 2023 at 02:21:57PM -0400, Laine Stump wrote: > On 5/4/23 4:33 AM, Andrea Bolognani wrote: > > I don't think we need the BuildRequires, or the build time detection, > > at all. Just > > > > #define NFT "nft" > > > > in the relevant file and be done with it. We'll locate the binary at > > runtime, same as we're doing with most of them already. > > Are we? What's the huge list of "optional programs" in meson.build then? Leftovers, that I intend to clean up At Some Point™ :) > I don't have any problem with doing all binary-location at runtime, as long > as we don't think there's any potential security problem / bug that could > arise from having a different binary with the same name added in some place > earlier in $PATH If some malicious actor can alter root's $PATH, or inject binaries into it, it's pretty much game over already. > (is that why we started canonicalizing binary paths during > the build?) I think it was done more for feature detection purposes, e.g. only enable the network driver if ifconfig is present or something. But that gets in the way of packagers, who usually want to explicitly enable/disable features anyway and to build in a minimal environment. It also assumes same-host deployment, and locks the configuration too early (what if I install ifconfig after building libvirt?). Runtime detection has some drawbacks too, but overall is more flexible and we've been moving in that direction. > > Maybe we also want to turn the iptables dependency into a Recommends? > > That way you will be able to uninstall it for a pure nft-based setup. > > I was being ultra-conservative about the change, making it opt-in for the > distros for now at least. But I'm also fine with making it opt-out I believe Dan argued for the nft backend to be made the default where possible. I generally agree that we should adopt forward-looking defaults whenever that can be done without breaking existing users. Anyway, regardless of which one of the backends ends up being the default one, maybe *both* nft and iptables should be Recommends? That way you'll get both installed by default, but you'll be able to drop the one that you're not using if you're aiming for a minimal deployment.