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
[PATCH RESEND v3 8/8] libvirt.spec: drop no longer required build dependencies
These are no longer required to build libvirt as they are used during
compilation only by meson to detect if some "auto" features should be
enabled or not but in spec file we explicitly enable/disable all libvirt
features.
Signed-off-by: Pavel Hrdina
---
libvirt.spec.in | 31 ---
1 file changed, 31 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index f421828d16..a49cd58e38 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -260,10 +260,6 @@ BuildRequires: sanlock-devel >= 2.4
BuildRequires: libpcap-devel >= 1.5.0
BuildRequires: libnl3-devel
BuildRequires: libselinux-devel
-BuildRequires: dnsmasq >= 2.41
-BuildRequires: iptables
-BuildRequires: radvd
-BuildRequires: ebtables
BuildRequires: module-init-tools
BuildRequires: cyrus-sasl-devel
BuildRequires: polkit >= 0.112
@@ -272,13 +268,7 @@ BuildRequires: util-linux
%if %{with_qemu}
# For managing ACLs
BuildRequires: libacl-devel
-# From QEMU RPMs
-BuildRequires: /usr/bin/qemu-img
%endif
-# For LVM drivers
-BuildRequires: lvm2
-# For pool type=iscsi
-BuildRequires: iscsi-initiator-utils
%if %{with_storage_iscsi_direct}
# For pool type=iscsi-direct
BuildRequires: libiscsi-devel
@@ -297,15 +287,6 @@ BuildRequires: librbd-devel
BuildRequires: glusterfs-api-devel >= 3.4.1
BuildRequires: glusterfs-devel >= 3.4.1
%endif
-%if %{with_storage_sheepdog}
-BuildRequires: sheepdog
-%endif
-%if %{with_storage_zfs}
-# Support any conforming implementation of zfs. On stock Fedora
-# this is zfs-fuse, but could be zfsonlinux upstream RPMs
-BuildRequires: /sbin/zfs
-BuildRequires: /sbin/zpool
-%endif
%if %{with_numactl}
# For QEMU/LXC numa info
BuildRequires: numactl-devel
@@ -330,21 +311,9 @@ BuildRequires: audit-libs-devel
# we need /usr/sbin/dtrace
BuildRequires: systemtap-sdt-devel
-# For mount/umount in FS driver
-BuildRequires: util-linux
-# For showmount in FS driver (netfs discovery)
-BuildRequires: nfs-utils
-
# Fedora build root suckage
BuildRequires: gawk
-# For storage wiping with different algorithms
-BuildRequires: scrub
-
-%if %{with_numad}
-BuildRequires: numad
-%endif
-
%if %{with_wireshark}
BuildRequires: wireshark-devel
%endif
--
2.31.1
