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@xxxxxxxxxx> 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
