Re: [libvirt PATCH 3/3] spec: Drop supported_platform variable

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(a)redhat.com&gt; 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.