[libvirt PATCH 0/3] spec: Bump min_fedora and min_rhel
Plus a couple additional cleanups. Test pipeline: https://gitlab.com/abologna/libvirt/-/pipelines/297950317 Andrea Bolognani (3): spec: Don't disable LTO in Fedora 34 spec: Bump min_fedora and min_rhel spec: Drop supported_platform variable libvirt.spec.in | 121 ++++++++---------------------------------- mingw-libvirt.spec.in | 14 ++--- 2 files changed, 25 insertions(+), 110 deletions(-) -- 2.26.3
The bug that caused this to be added https://bugzilla.redhat.com/show_bug.cgi?id=1889763 has since been resolved. Reverts: a16c0402bae3138c8c6c6da5bbe1bb4ad2c4dc06 Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- libvirt.spec.in | 7 ------- 1 file changed, 7 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index da7af2824e..27de42f822 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -985,13 +985,6 @@ Libvirt plugin for NSS for translating domain names into IP addresses. %autosetup -S git_am %build - -%if 0%{?fedora} == 34 - # binutils change in F34 broke linking of tests - # https://bugzilla.redhat.com/show_bug.cgi?id=1889763 - %define _lto_cflags %{nil} -%endif - %if ! %{supported_platform} echo "This RPM requires either Fedora >= %{min_fedora} or RHEL >= %{min_rhel}" exit 1 -- 2.26.3
According to our platform support policy https://libvirt.org/platforms.html RHEL 7 and all versions of Fedora older than 33 are going to be out of scope by the time libvirt 7.4.0 is released. Dropping RHEL 7 in particular allows us to greatly simplify many parts of the spec file. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- libvirt.spec.in | 102 ++++++++---------------------------------- mingw-libvirt.spec.in | 2 +- 2 files changed, 19 insertions(+), 85 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index 27de42f822..013c7742a2 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -3,8 +3,8 @@ # This spec file assumes you are building on a Fedora or RHEL version # that's still supported by the vendor. It may work on other distros # or versions, but no effort will be made to ensure that going forward. -%define min_rhel 7 -%define min_fedora 31 +%define min_rhel 8 +%define min_fedora 33 %if 0%{?fedora} >= %{min_fedora} || 0%{?rhel} >= %{min_rhel} %define supported_platform 1 @@ -12,18 +12,9 @@ %define supported_platform 0 %endif -# On RHEL 7 and older macro _vpath_builddir is not defined. -%if 0%{?rhel} && 0%{?rhel} <= 7 - %define _vpath_builddir %{_target_platform} -%endif - %define arches_qemu_kvm %{ix86} x86_64 %{power64} %{arm} aarch64 s390x %if 0%{?rhel} - %if 0%{?rhel} <= 8 - %define arches_qemu_kvm x86_64 %{power64} aarch64 s390x - %else - %define arches_qemu_kvm x86_64 aarch64 s390x - %endif + %define arches_qemu_kvm x86_64 aarch64 s390x %endif %define arches_64bit x86_64 %{power64} aarch64 s390x riscv64 @@ -77,25 +68,20 @@ %define with_storage_gluster 0%{!?_without_storage_gluster:1} %ifnarch %{arches_qemu_kvm} - # gluster is only built where qemu driver is enabled on RHEL 8 - %if 0%{?rhel} >= 8 + # gluster is only built where qemu driver is enabled on RHEL + %if 0%{?rhel} %define with_storage_gluster 0 %endif %endif -# F25+ has zfs-fuse +# Fedora has zfs-fuse %if 0%{?fedora} %define with_storage_zfs 0%{!?_without_storage_zfs:1} %else %define with_storage_zfs 0 %endif -# We need a recent enough libiscsi (>= 1.18.0) -%if 0%{?fedora} || 0%{?rhel} > 7 - %define with_storage_iscsi_direct 0%{!?_without_storage_iscsi_direct:1} -%else - %define with_storage_iscsi_direct 0 -%endif +%define with_storage_iscsi_direct 0%{!?_without_storage_iscsi_direct:1} # Other optional features %define with_numactl 0%{!?_without_numactl:1} @@ -130,9 +116,7 @@ %define with_storage_rbd 0 %endif -# RHEL doesn't ship OpenVZ, VBox, PowerHypervisor, -# VMware, libxenlight (Xen 4.1 and newer), -# or HyperV. +# RHEL doesn't ship many hypervisor drivers %if 0%{?rhel} %define with_openvz 0 %define with_vbox 0 @@ -140,15 +124,10 @@ %define with_libxl 0 %define with_hyperv 0 %define with_vz 0 - - %if 0%{?rhel} > 7 - %define with_lxc 0 - %endif + %define with_lxc 0 %endif -%if 0%{?fedora} || 0%{?rhel} > 7 - %define with_firewalld_zone 0%{!?_without_firewalld_zone:1} -%endif +%define with_firewalld_zone 0%{!?_without_firewalld_zone:1} %if (0%{?fedora} && 0%{?fedora} < 34) || (0%{?rhel} && 0%{?rhel} < 9) %define with_netcf 0%{!?_without_netcf:1} @@ -176,16 +155,12 @@ %define with_libssh2 0%{!?_without_libssh2:1} %endif -# Enable wireshark plugins for all distros except RHEL-7 -%if 0%{?fedora} || 0%{?rhel} > 7 - %define with_wireshark 0%{!?_without_wireshark:1} - %define wireshark_plugindir %(pkg-config --variable plugindir wireshark)/epan -%endif +# Enable wireshark plugins for all distros +%define with_wireshark 0%{!?_without_wireshark:1} +%define wireshark_plugindir %(pkg-config --variable plugindir wireshark)/epan -# Enable libssh transport for new enough distros -%if 0%{?fedora} || 0%{?rhel} > 7 - %define with_libssh 0%{!?_without_libssh:1} -%endif +# Enable libssh transport for all distros +%define with_libssh 0%{!?_without_libssh:1} %if %{with_qemu} || %{with_lxc} # numad is used to manage the CPU and memory placement dynamically, @@ -213,11 +188,7 @@ %define enable_werror -Dwerror=false %endif -%if 0%{?rhel} == 7 - %define tls_priority "NORMAL" -%else - %define tls_priority "@LIBVIRT,SYSTEM" -%endif +%define tls_priority "@LIBVIRT,SYSTEM" Summary: Library providing a simple virtualization API @@ -262,20 +233,12 @@ Requires: libvirt-libs = %{version}-%{release} # All build-time requirements. Run-time requirements are # listed against each sub-RPM -%if 0%{?rhel} == 7 -BuildRequires: python36-docutils -%else BuildRequires: python3-docutils -%endif BuildRequires: gcc BuildRequires: meson >= 0.54.0 BuildRequires: ninja-build BuildRequires: git -%if 0%{?fedora} || 0%{?rhel} > 7 BuildRequires: perl-interpreter -%else -BuildRequires: perl -%endif BuildRequires: python3 BuildRequires: systemd-units %if %{with_libxl} @@ -333,13 +296,8 @@ BuildRequires: device-mapper-devel # For XFS reflink clone support BuildRequires: xfsprogs-devel %if %{with_storage_rbd} - %if 0%{?fedora} || 0%{?rhel} > 7 BuildRequires: librados-devel BuildRequires: librbd-devel - %else -BuildRequires: librados2-devel -BuildRequires: librbd1-devel - %endif %endif %if %{with_storage_gluster} BuildRequires: glusterfs-api-devel >= 3.4.1 @@ -401,11 +359,7 @@ BuildRequires: wireshark-devel BuildRequires: libssh-devel >= 0.7.0 %endif -# On RHEL-7 rpcgen is still part of glibc-common package -%if 0%{?fedora} || 0%{?rhel} > 7 BuildRequires: rpcgen -%endif - BuildRequires: libtirpc-devel # Needed for the firewalld_reload macro @@ -440,12 +394,10 @@ Requires: /usr/bin/nc # for modprobe of pci devices Requires: module-init-tools -# for /sbin/ip & /sbin/tc +# for /sbin/ip Requires: iproute -# tc is provided by iproute-tc since at least Fedora 26 -%if 0%{?fedora} || 0%{?rhel} > 7 +# for /sbin/tc Requires: iproute-tc -%endif Requires: polkit >= 0.112 %if %{with_dmidecode} @@ -529,9 +481,7 @@ Requires: libvirt-libs = %{version}-%{release} # needed for device enumeration Requires: systemd >= 185 # For managing persistent mediated devices -%if 0%{?fedora} || 0%{?rhel} > 7 Requires: mdevctl -%endif %description daemon-driver-nodedev The nodedev driver plugin for the libvirtd daemon, providing @@ -747,12 +697,8 @@ Requires: gzip Requires: bzip2 Requires: lzop Requires: xz - %if 0%{?fedora} || 0%{?rhel} > 7 Requires: systemd-container - %endif - %if 0%{?fedora} || 0%{?rhel} > 7 Requires: swtpm-tools - %endif %description daemon-driver-qemu The qemu driver plugin for the libvirtd daemon, providing @@ -768,9 +714,7 @@ Requires: libvirt-daemon = %{version}-%{release} Requires: libvirt-libs = %{version}-%{release} # There really is a hard cross-driver dependency here Requires: libvirt-daemon-driver-network = %{version}-%{release} - %if 0%{?fedora} || 0%{?rhel} > 7 Requires: systemd-container - %endif %description daemon-driver-lxc The LXC driver plugin for the libvirtd daemon, providing @@ -1292,16 +1236,6 @@ mv $RPM_BUILD_ROOT%{_datadir}/systemtap/tapset/libvirt_qemu_probes.stp \ # raising the test timeout VIR_TEST_DEBUG=1 %meson_test --no-suite syntax-check --timeout-multiplier 10 -%post libs -%if 0%{?rhel} == 7 -/sbin/ldconfig -%endif - -%postun libs -%if 0%{?rhel} == 7 -/sbin/ldconfig -%endif - %pre daemon # 'libvirt' group is just to allow password-less polkit access to # libvirtd. The uid number is irrelevant, so we use dynamic allocation diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in index 288f533d52..84b8998f74 100644 --- a/mingw-libvirt.spec.in +++ b/mingw-libvirt.spec.in @@ -3,7 +3,7 @@ # This spec file assumes you are building on a Fedora version # that's still supported by the vendor. It may work on other distros # or versions, but no effort will be made to ensure that going forward. -%define min_fedora 31 +%define min_fedora 33 %if 0%{?fedora} && 0%{?fedora} >= %{min_fedora} %define supported_platform 1 -- 2.26.3
It's only used in one place, and it's nicer to keep the error message close to the check that causes it to be emitted. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- libvirt.spec.in | 12 +++--------- mingw-libvirt.spec.in | 12 +++--------- 2 files changed, 6 insertions(+), 18 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index 013c7742a2..9dea6c6787 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -6,12 +6,6 @@ %define min_rhel 8 %define min_fedora 33 -%if 0%{?fedora} >= %{min_fedora} || 0%{?rhel} >= %{min_rhel} - %define supported_platform 1 -%else - %define supported_platform 0 -%endif - %define arches_qemu_kvm %{ix86} x86_64 %{power64} %{arm} aarch64 s390x %if 0%{?rhel} %define arches_qemu_kvm x86_64 aarch64 s390x @@ -929,9 +923,9 @@ Libvirt plugin for NSS for translating domain names into IP addresses. %autosetup -S git_am %build -%if ! %{supported_platform} -echo "This RPM requires either Fedora >= %{min_fedora} or RHEL >= %{min_rhel}" -exit 1 +%if (0%{?fedora} && 0%{?fedora} < %{min_fedora}) || (0%{?rhel} && 0%{?rhel} < %{min_rhel}) + echo "This RPM requires either Fedora >= %{min_fedora} or RHEL >= %{min_rhel}" + exit 1 %endif %if %{with_qemu} diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in index 84b8998f74..61a4843fb3 100644 --- a/mingw-libvirt.spec.in +++ b/mingw-libvirt.spec.in @@ -5,12 +5,6 @@ # or versions, but no effort will be made to ensure that going forward. %define min_fedora 33 -%if 0%{?fedora} && 0%{?fedora} >= %{min_fedora} - %define supported_platform 1 -%else - %define supported_platform 0 -%endif - Name: mingw-libvirt Version: @VERSION@ Release: 1%{?dist} @@ -95,9 +89,9 @@ MinGW Windows libvirt virtualization library. %setup -q -n libvirt-%{version} %build -%if ! %{supported_platform} -echo "This RPM requires Fedora >= %{min_fedora}" -exit 1 +%if 0%{?fedora} < %{min_fedora} + echo "This RPM requires Fedora >= %{min_fedora}" + exit 1 %endif %mingw_meson \ -- 2.26.3
On Wed, May 5, 2021 at 4:12 PM Andrea Bolognani <abologna@redhat.com> wrote:
It's only used in one place, and it's nicer to keep the error message close to the check that causes it to be emitted.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- libvirt.spec.in | 12 +++--------- mingw-libvirt.spec.in | 12 +++--------- 2 files changed, 6 insertions(+), 18 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in index 013c7742a2..9dea6c6787 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -6,12 +6,6 @@ %define min_rhel 8 %define min_fedora 33
-%if 0%{?fedora} >= %{min_fedora} || 0%{?rhel} >= %{min_rhel} - %define supported_platform 1 -%else - %define supported_platform 0 -%endif - %define arches_qemu_kvm %{ix86} x86_64 %{power64} %{arm} aarch64 s390x %if 0%{?rhel} %define arches_qemu_kvm x86_64 aarch64 s390x @@ -929,9 +923,9 @@ Libvirt plugin for NSS for translating domain names into IP addresses. %autosetup -S git_am
%build -%if ! %{supported_platform} -echo "This RPM requires either Fedora >= %{min_fedora} or RHEL >= %{min_rhel}" -exit 1 +%if (0%{?fedora} && 0%{?fedora} < %{min_fedora}) || (0%{?rhel} && 0%{?rhel} < %{min_rhel}) + echo "This RPM requires either Fedora >= %{min_fedora} or RHEL >= %{min_rhel}" + exit 1 %endif
This broke my builds locally, because now the libvirt spec throws an error on Fedora and RHEL. Your conditional logic will always trigger an error because you're now checking to see if *either* Fedora < 33 or RHEL < 7, and that will always be true and fail the build. :( I'm going to send a fixup for this.
%if %{with_qemu} diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in index 84b8998f74..61a4843fb3 100644 --- a/mingw-libvirt.spec.in +++ b/mingw-libvirt.spec.in @@ -5,12 +5,6 @@ # or versions, but no effort will be made to ensure that going forward. %define min_fedora 33
-%if 0%{?fedora} && 0%{?fedora} >= %{min_fedora} - %define supported_platform 1 -%else - %define supported_platform 0 -%endif - Name: mingw-libvirt Version: @VERSION@ Release: 1%{?dist} @@ -95,9 +89,9 @@ MinGW Windows libvirt virtualization library. %setup -q -n libvirt-%{version}
%build -%if ! %{supported_platform} -echo "This RPM requires Fedora >= %{min_fedora}" -exit 1 +%if 0%{?fedora} < %{min_fedora} + echo "This RPM requires Fedora >= %{min_fedora}" + exit 1 %endif
%mingw_meson \ -- 2.26.3
-- 真実はいつも一つ！/ Always, there's only one truth!
On Tue, May 11, 2021 at 09:12:41AM -0400, Neal Gompa wrote:
On Wed, May 5, 2021 at 4:12 PM Andrea Bolognani <abologna@redhat.com> wrote:
-%if 0%{?fedora} >= %{min_fedora} || 0%{?rhel} >= %{min_rhel} - %define supported_platform 1 -%else - %define supported_platform 0 -%endif
-%if ! %{supported_platform} -echo "This RPM requires either Fedora >= %{min_fedora} or RHEL >= %{min_rhel}" -exit 1 +%if (0%{?fedora} && 0%{?fedora} < %{min_fedora}) || (0%{?rhel} && 0%{?rhel} < %{min_rhel}) + echo "This RPM requires either Fedora >= %{min_fedora} or RHEL >= %{min_rhel}" + exit 1 %endif
This broke my builds locally, because now the libvirt spec throws an error on Fedora and RHEL. Your conditional logic will always trigger an error because you're now checking to see if *either* Fedora < 33 or RHEL < 7, and that will always be true and fail the build. :(
I'm going to send a fixup for this.
I'm surprised by this, because the CI pipeline is green and we do RPM builds in there. Looking again at the check, it seems correct: "if we're on Fedora and the version of Fedora is smaller than the minimum supported one, or if we're on RHEL and the version of RHEL is smaller than the minimum supported one, then error out". We have check that's basically identical, earlier in the spec file, to decide whether or not to netcf support should be enabled. What am I missing? -- Andrea Bolognani / Red Hat / Virtualization
On Tue, May 11, 2021 at 07:22:26AM -0700, Andrea Bolognani wrote:
On Tue, May 11, 2021 at 09:12:41AM -0400, Neal Gompa wrote:
On Wed, May 5, 2021 at 4:12 PM Andrea Bolognani <abologna@redhat.com> wrote:
-%if 0%{?fedora} >= %{min_fedora} || 0%{?rhel} >= %{min_rhel} - %define supported_platform 1 -%else - %define supported_platform 0 -%endif
-%if ! %{supported_platform} -echo "This RPM requires either Fedora >= %{min_fedora} or RHEL >= %{min_rhel}" -exit 1 +%if (0%{?fedora} && 0%{?fedora} < %{min_fedora}) || (0%{?rhel} && 0%{?rhel} < %{min_rhel}) + echo "This RPM requires either Fedora >= %{min_fedora} or RHEL >= %{min_rhel}" + exit 1 %endif
This broke my builds locally, because now the libvirt spec throws an error on Fedora and RHEL. Your conditional logic will always trigger an error because you're now checking to see if *either* Fedora < 33 or RHEL < 7, and that will always be true and fail the build. :(
I'm going to send a fixup for this.
I'm surprised by this, because the CI pipeline is green and we do RPM builds in there.
Looking again at the check, it seems correct: "if we're on Fedora and the version of Fedora is smaller than the minimum supported one, or if we're on RHEL and the version of RHEL is smaller than the minimum supported one, then error out".
We have check that's basically identical, earlier in the spec file, to decide whether or not to netcf support should be enabled.
What am I missing?
Not the issue that Neal was mentioning, but I think the condition you added is not equivalent to the original condition. The new test only runs on either Fedora or RHEL. The previous test ran on all distros. ie, attempting an RPM build on SUSE would report the error originally, but no longer does. I think this needs fixing. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Tue, May 11, 2021 at 03:36:46PM +0100, Daniel P. Berrangé wrote:
On Tue, May 11, 2021 at 07:22:26AM -0700, Andrea Bolognani wrote:
On Tue, May 11, 2021 at 09:12:41AM -0400, Neal Gompa wrote:
On Wed, May 5, 2021 at 4:12 PM Andrea Bolognani <abologna@redhat.com> wrote:
-%if 0%{?fedora} >= %{min_fedora} || 0%{?rhel} >= %{min_rhel} - %define supported_platform 1 -%else - %define supported_platform 0 -%endif
-%if ! %{supported_platform} -echo "This RPM requires either Fedora >= %{min_fedora} or RHEL >= %{min_rhel}" -exit 1 +%if (0%{?fedora} && 0%{?fedora} < %{min_fedora}) || (0%{?rhel} && 0%{?rhel} < %{min_rhel}) + echo "This RPM requires either Fedora >= %{min_fedora} or RHEL >= %{min_rhel}" + exit 1 %endif
This broke my builds locally, because now the libvirt spec throws an error on Fedora and RHEL. Your conditional logic will always trigger an error because you're now checking to see if *either* Fedora < 33 or RHEL < 7, and that will always be true and fail the build. :(
I'm going to send a fixup for this.
I'm surprised by this, because the CI pipeline is green and we do RPM builds in there.
Looking again at the check, it seems correct: "if we're on Fedora and the version of Fedora is smaller than the minimum supported one, or if we're on RHEL and the version of RHEL is smaller than the minimum supported one, then error out".
We have check that's basically identical, earlier in the spec file, to decide whether or not to netcf support should be enabled.
What am I missing?
Not the issue that Neal was mentioning, but I think the condition you added is not equivalent to the original condition.
The new test only runs on either Fedora or RHEL.
The previous test ran on all distros.
ie, attempting an RPM build on SUSE would report the error originally, but no longer does. I think this needs fixing.
Yeah, good point. I might just revert the original commit and only move the definition of supported_platform down to where it's actually used, to avoid making the check even more unwieldy. -- Andrea Bolognani / Red Hat / Virtualization
On 5/5/21 10:11 PM, Andrea Bolognani wrote:
Plus a couple additional cleanups.
Test pipeline: https://gitlab.com/abologna/libvirt/-/pipelines/297950317
Andrea Bolognani (3): spec: Don't disable LTO in Fedora 34 spec: Bump min_fedora and min_rhel spec: Drop supported_platform variable
libvirt.spec.in | 121 ++++++++---------------------------------- mingw-libvirt.spec.in | 14 ++--- 2 files changed, 25 insertions(+), 110 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (4)
-
Andrea Bolognani -
Daniel P. Berrangé -
Michal Prívozník -
Neal Gompa