Re: [PATCH RESEND v3 8/8] libvirt.spec: drop no longer required build dependencies
On Tue, May 25, 2021 at 10:19:28PM -0400, Neal Gompa wrote: > On Tue, May 25, 2021 at 11:34 AM Andrea Bolognani wrote: > > On Tue, May 25, 2021 at 05:06:41PM +0200, Michal Prívozník wrote: > > > Wasn't the whole idea to drop dependencies? > > > > The point was to drop **build** dependencies, specifically > > problematic ones such as ZFS. When it comes to runtime dependencies, > > our RPMs generally try to be as featureful as possible, so adding a > > dependency on scrub is the right thing to do IMO. > > Why is ZFS problematic? The zfs-fuse package is totally fine for us to > depend on... I'm not too familiar with the ZFS situation in Fedora / RHEL, but at least in Debian we're patching meson.build specifically to enable ZFS support without needing a ZFS implementation available at build time. In general, using the build time availability of programs to guess whether features should be enabled is a good, user-friendly behavior, but we shouldn't second-guess the information provided by the user: if they passed -Dstorage_zfs=enabled to Meson, then we should enable ZFS support whether or not ZFS happens to be currently installed. -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH RESEND v3 8/8] libvirt.spec: drop no longer required build dependencies
On Tue, May 25, 2021 at 11:34 AM Andrea Bolognani wrote: > > On Tue, May 25, 2021 at 05:06:41PM +0200, Michal Prívozník wrote: > > On 5/25/21 12:00 PM, Andrea Bolognani wrote: > > > On Tue, May 25, 2021 at 10:00:47AM +0200, Pavel Hrdina wrote: > > >> -# For storage wiping with different algorithms > > >> -BuildRequires: scrub > > > > > > Turns out we're actually missing the runtime dependency on scrub! Can > > > you please take care of addressing that issue in a separate patch? > > > > Are we? scrub is needed if and only if you want to pass a special > > algorithm to virStorageVolWipePattern(). Does that justify a runtime > > dependency? We fail gracefully if scrub isn't installed. We aren't even > > requiring qemu binary for daemon-driver-qemu package. > > > > Wasn't the whole idea to drop dependencies? > > The point was to drop **build** dependencies, specifically > problematic ones such as ZFS. When it comes to runtime dependencies, > our RPMs generally try to be as featureful as possible, so adding a > dependency on scrub is the right thing to do IMO. > Why is ZFS problematic? The zfs-fuse package is totally fine for us to depend on... -- 真実はいつも一つ！/ Always, there's only one truth!
Re: [PATCH RESEND v3 8/8] libvirt.spec: drop no longer required build dependencies
On Tue, May 25, 2021 at 05:06:41PM +0200, Michal Prívozník wrote: > On 5/25/21 12:00 PM, Andrea Bolognani wrote: > > On Tue, May 25, 2021 at 10:00:47AM +0200, Pavel Hrdina wrote: > >> -# For storage wiping with different algorithms > >> -BuildRequires: scrub > > > > Turns out we're actually missing the runtime dependency on scrub! Can > > you please take care of addressing that issue in a separate patch? > > Are we? scrub is needed if and only if you want to pass a special > algorithm to virStorageVolWipePattern(). Does that justify a runtime > dependency? We fail gracefully if scrub isn't installed. We aren't even > requiring qemu binary for daemon-driver-qemu package. > > Wasn't the whole idea to drop dependencies? The point was to drop **build** dependencies, specifically problematic ones such as ZFS. When it comes to runtime dependencies, our RPMs generally try to be as featureful as possible, so adding a dependency on scrub is the right thing to do IMO. -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH RESEND v3 8/8] libvirt.spec: drop no longer required build dependencies
On 5/25/21 12:00 PM, Andrea Bolognani wrote: > On Tue, May 25, 2021 at 10:00:47AM +0200, Pavel Hrdina wrote: >> -# For mount/umount in FS driver >> -BuildRequires: util-linux > > This BuildRequires is duplicated, so please drop the other instance > as well. > >> -# For storage wiping with different algorithms >> -BuildRequires: scrub > > Turns out we're actually missing the runtime dependency on scrub! Can > you please take care of addressing that issue in a separate patch? Are we? scrub is needed if and only if you want to pass a special algorithm to virStorageVolWipePattern(). Does that justify a runtime dependency? We fail gracefully if scrub isn't installed. We aren't even requiring qemu binary for daemon-driver-qemu package. Wasn't the whole idea to drop dependencies? Michal
Re: [PATCH RESEND v3 8/8] libvirt.spec: drop no longer required build dependencies
On Tue, May 25, 2021 at 10:00:47AM +0200, Pavel Hrdina wrote: > -# For mount/umount in FS driver > -BuildRequires: util-linux This BuildRequires is duplicated, so please drop the other instance as well. > -# For storage wiping with different algorithms > -BuildRequires: scrub Turns out we're actually missing the runtime dependency on scrub! Can you please take care of addressing that issue in a separate patch? With the second BuildRequires on util-linux removed, Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization
