[PATCH] Revert "spec: Simplify setting features off by default"

[Neal Gompa]

On Tue, Oct 27, 2020 at 9:30 AM Andrea Bolognani <abologna at redhat.com> wrote:
>
> On Tue, 2020-10-27 at 14:05 +0100, Andrea Bolognani wrote:
> > I'm not convinced reverting this was the right call.
> >
> > The way RPM conditional macros work is that, if you have
> >
> >   %{!?macro:value}
> >
> > that will expand to 'value' if 'macro' is *not* defined, and to
> > nothing otherwise. So if you have for example
> >
> >   %define with_fuse          0%{!?_without_fuse:0}
> >
> > that means that, if you pass
> >
> >   --define '_without_fuse 1'
> >
> > to rpmbuild the line will expand to
> >
> >   %define with_fuse          0
> >
> > and if you don't pass the extra option to rpmbuild it will instead
> > expand to
> >
> >   %define with_fuse          00
> >
> > The two are clearly equivalent, so there is no point in keeping the
> > conditional macro - it merely obfuscates the logic.
> >
> > Of course things would be different if you had
> >
> >   %define with_fuse          0%{!?_without_fuse:1}
> >
> > instead: in this case, the line would expand to
> >
> >   %define with_fuse          0
> >
> > and
> >
> >   %define with_fuse          01
> >
> > respectively, which means the feature would be enabled by default but
> > could optionally be disabled by passing the correct argument to
> > rpmbuild, as expected. We have plenty similar macros in libvirt.spec,
> > and since they work just as intended 31d687a3218c didn't touch them.
> >
> > Note that in this case I've removed
> >
> >   # fuse is used to provide virtualized /proc for LXC
> >   %if %{with_lxc}
> >       %define with_fuse      0%{!?_without_fuse:1}
> >   %endif
> >
> > from the spec to make sure that the value for the 'fuse' option
> > passed to Meson depended solely on the value of the _without_fuse
> > macro, and then checked the rpmbuild output to compare.
>

Ugh, you're right, and those values need to be changed to 1.

> Also note that I'm aware you want to eventually push for adoption of
> the standard bcond macros, and I fully stand behind that desire! If
> this patch had been the first in a series that introduced bcond
> support and was clearing the path for that, I would have zero
> problems with it. As it is, however, you're simply reintroducing some
> of the obfuscation we had recently managed to get rid of, without
> getting anything in return.
>

Fixing this so that I can switch to bconds is going to be a massive
rewrite of how feature enablement works. That is not something I can
push for a 6.9.0 freeze break patch.

My in-progress rewrite is going to be a massive break in how this is managed...

-- 
真実はいつも一つ！/ Always, there's only one truth!