[PATCH 0/2] Minor fixes to the RPM spec

As part of the work to add openSUSE support to the spec file, I discovered a couple of small fixes worth submitting separately. Neal Gompa (2): rpm: Simplify expression of supported platforms rpm: Drop unnecessary libiscsi runtime dependency libvirt.spec.in | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) -- 2.29.2

Stanzas like "0%{?fedora} && 0%{?fedora} >= %{min_fedora}" contain redundant definitions, as "0%{?fedora} >= %{min_fedora}" implies that "%fedora" is defined and has a value. Thus, we can simplify this. Signed-off-by: Neal Gompa <ngompa13@gmail.com> --- libvirt.spec.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index ce1a8d7078..0a8b0ebad4 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -6,7 +6,7 @@ %define min_rhel 7 %define min_fedora 31 -%if (0%{?fedora} && 0%{?fedora} >= %{min_fedora}) || (0%{?rhel} && 0%{?rhel} >= %{min_rhel}) +%if 0%{?fedora} >= %{min_fedora} || 0%{?rhel} >= %{min_rhel} %define supported_platform 1 %else %define supported_platform 0 -- 2.29.2

On Thu, Jan 07, 2021 at 09:58:08 -0500, Neal Gompa wrote:
Stanzas like "0%{?fedora} && 0%{?fedora} >= %{min_fedora}" contain redundant definitions, as "0%{?fedora} >= %{min_fedora}" implies that "%fedora" is defined and has a value. Thus, we can simplify this.
Signed-off-by: Neal Gompa <ngompa13@gmail.com> --- libvirt.spec.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in index ce1a8d7078..0a8b0ebad4 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -6,7 +6,7 @@ %define min_rhel 7 %define min_fedora 31
-%if (0%{?fedora} && 0%{?fedora} >= %{min_fedora}) || (0%{?rhel} && 0%{?rhel} >= %{min_rhel}) +%if 0%{?fedora} >= %{min_fedora} || 0%{?rhel} >= %{min_rhel} %define supported_platform 1 %else %define supported_platform 0
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

This is automatically picked up by the dependency generator, so there's no reason to have this here. Signed-off-by: Neal Gompa <ngompa13@gmail.com> --- libvirt.spec.in | 1 - 1 file changed, 1 deletion(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index 0a8b0ebad4..41e532102a 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -614,7 +614,6 @@ volumes using the host iscsi stack. Summary: Storage driver plugin for iscsi-direct Requires: libvirt-daemon-driver-storage-core = %{version}-%{release} Requires: libvirt-libs = %{version}-%{release} -Requires: libiscsi %description daemon-driver-storage-iscsi-direct The storage driver backend adding implementation of the storage APIs for iscsi -- 2.29.2

On Thu, Jan 07, 2021 at 09:58:09 -0500, Neal Gompa wrote:
This is automatically picked up by the dependency generator, so there's no reason to have this here.
Signed-off-by: Neal Gompa <ngompa13@gmail.com> --- libvirt.spec.in | 1 - 1 file changed, 1 deletion(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in index 0a8b0ebad4..41e532102a 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -614,7 +614,6 @@ volumes using the host iscsi stack. Summary: Storage driver plugin for iscsi-direct Requires: libvirt-daemon-driver-storage-core = %{version}-%{release} Requires: libvirt-libs = %{version}-%{release} -Requires: libiscsi
The explicit dependency was added by Andrea 2.5 years ago, perhaps he had reasons to do so. Any comments Andrea? Jirka

On Thu, Jan 7, 2021 at 12:38 PM Jiri Denemark <jdenemar@redhat.com> wrote:
On Thu, Jan 07, 2021 at 09:58:09 -0500, Neal Gompa wrote:
This is automatically picked up by the dependency generator, so there's no reason to have this here.
Signed-off-by: Neal Gompa <ngompa13@gmail.com> --- libvirt.spec.in | 1 - 1 file changed, 1 deletion(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in index 0a8b0ebad4..41e532102a 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -614,7 +614,6 @@ volumes using the host iscsi stack. Summary: Storage driver plugin for iscsi-direct Requires: libvirt-daemon-driver-storage-core = %{version}-%{release} Requires: libvirt-libs = %{version}-%{release} -Requires: libiscsi
The explicit dependency was added by Andrea 2.5 years ago, perhaps he had reasons to do so. Any comments Andrea?
It most likely dates back to when Fedora had two providers of libiscsi sharing the same soname. That situation no longer exists today. Other distributions also don't have that issue. -- 真実はいつも一つ!/ Always, there's only one truth!

On Thu, 2021-01-07 at 13:48 -0500, Neal Gompa wrote:
On Thu, Jan 7, 2021 at 12:38 PM Jiri Denemark <jdenemar@redhat.com> wrote:
On Thu, Jan 07, 2021 at 09:58:09 -0500, Neal Gompa wrote:
+++ b/libvirt.spec.in @@ -614,7 +614,6 @@ volumes using the host iscsi stack. Summary: Storage driver plugin for iscsi-direct Requires: libvirt-daemon-driver-storage-core = %{version}-%{release} Requires: libvirt-libs = %{version}-%{release} -Requires: libiscsi
The explicit dependency was added by Andrea 2.5 years ago, perhaps he had reasons to do so. Any comments Andrea?
It most likely dates back to when Fedora had two providers of libiscsi sharing the same soname. That situation no longer exists today. Other distributions also don't have that issue.
I didn't offer much in the way of explanation for the change in commit fe5b35c6b29dc952babf4436ccba83c4a0ffa82e Author: Andrea Bolognani <abologna@redhat.com> Date: Tue Aug 14 14:31:35 2018 +0200 spec: Enable the iscsi-direct storage driver conditionally Most distributions we build RPMs on don't ship a recent enough version of libiscsi, so we can't enable the driver unconditionally. Add an explicit dependency on the runtime package while at it. Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com> so I can only guess that I was mistakenly thinking the runtime dependency would not be added automatically. Under the assumption that Neal has already verified the set of runtime dependencies is exactly the same, on all RPM-based targets, before and after this patch has been applied, Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization

On Mon, Jan 11, 2021 at 10:52 AM Andrea Bolognani <abologna@redhat.com> wrote:
On Thu, 2021-01-07 at 13:48 -0500, Neal Gompa wrote:
On Thu, Jan 7, 2021 at 12:38 PM Jiri Denemark <jdenemar@redhat.com> wrote:
On Thu, Jan 07, 2021 at 09:58:09 -0500, Neal Gompa wrote:
+++ b/libvirt.spec.in @@ -614,7 +614,6 @@ volumes using the host iscsi stack. Summary: Storage driver plugin for iscsi-direct Requires: libvirt-daemon-driver-storage-core = %{version}-%{release} Requires: libvirt-libs = %{version}-%{release} -Requires: libiscsi
The explicit dependency was added by Andrea 2.5 years ago, perhaps he had reasons to do so. Any comments Andrea?
It most likely dates back to when Fedora had two providers of libiscsi sharing the same soname. That situation no longer exists today. Other distributions also don't have that issue.
I didn't offer much in the way of explanation for the change in
commit fe5b35c6b29dc952babf4436ccba83c4a0ffa82e Author: Andrea Bolognani <abologna@redhat.com> Date: Tue Aug 14 14:31:35 2018 +0200
spec: Enable the iscsi-direct storage driver conditionally
Most distributions we build RPMs on don't ship a recent enough version of libiscsi, so we can't enable the driver unconditionally. Add an explicit dependency on the runtime package while at it.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
so I can only guess that I was mistakenly thinking the runtime dependency would not be added automatically.
Under the assumption that Neal has already verified the set of runtime dependencies is exactly the same, on all RPM-based targets, before and after this patch has been applied,
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
-- Andrea Bolognani / Red Hat / Virtualization
So apparently this wasn't actually pushed yet... Can we get this pushed now? -- 真実はいつも一つ!/ Always, there's only one truth!

On Tue, May 11, 2021 at 08:36:40PM -0400, Neal Gompa wrote:
On Mon, Jan 11, 2021 at 10:52 AM Andrea Bolognani <abologna@redhat.com> wrote:
On Thu, 2021-01-07 at 13:48 -0500, Neal Gompa wrote:
On Thu, Jan 7, 2021 at 12:38 PM Jiri Denemark <jdenemar@redhat.com> wrote:
On Thu, Jan 07, 2021 at 09:58:09 -0500, Neal Gompa wrote:
+++ b/libvirt.spec.in @@ -614,7 +614,6 @@ volumes using the host iscsi stack. Summary: Storage driver plugin for iscsi-direct Requires: libvirt-daemon-driver-storage-core = %{version}-%{release} Requires: libvirt-libs = %{version}-%{release} -Requires: libiscsi
The explicit dependency was added by Andrea 2.5 years ago, perhaps he had reasons to do so. Any comments Andrea?
It most likely dates back to when Fedora had two providers of libiscsi sharing the same soname. That situation no longer exists today. Other distributions also don't have that issue.
I didn't offer much in the way of explanation for the change in
commit fe5b35c6b29dc952babf4436ccba83c4a0ffa82e Author: Andrea Bolognani <abologna@redhat.com> Date: Tue Aug 14 14:31:35 2018 +0200
spec: Enable the iscsi-direct storage driver conditionally
Most distributions we build RPMs on don't ship a recent enough version of libiscsi, so we can't enable the driver unconditionally. Add an explicit dependency on the runtime package while at it.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
so I can only guess that I was mistakenly thinking the runtime dependency would not be added automatically.
Under the assumption that Neal has already verified the set of runtime dependencies is exactly the same, on all RPM-based targets, before and after this patch has been applied,
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
So apparently this wasn't actually pushed yet... Can we get this pushed now?
Pushed. -- Andrea Bolognani / Red Hat / Virtualization
participants (3)
-
Andrea Bolognani
-
Jiri Denemark
-
Neal Gompa