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

Andrea Bolognani abologna at redhat.com
Tue Oct 27 13:05:43 UTC 2020


On Tue, 2020-10-27 at 11:07 +0100, Michal Privoznik wrote:
> On 10/26/20 10:53 PM, Neal Gompa wrote:
> > As it turns out, the rather complicated structure that is
> > currently used for enabling or disabling features in the libvirt
> > build does not cleanly map well to RPM's bcond feature.
> > 
> > Consequently, we need these back in order to support trivially
> > activating these features through extra macros as build inputs.
> > 
> > This reverts commit 31d687a3218c9072d7050dd608e013e063ca180f.
> > 
> > Signed-off-by: Neal Gompa <ngompa13 at gmail.com>
> > ---
> >   libvirt.spec.in | 16 ++++++++--------
> >   1 file changed, 8 insertions(+), 8 deletions(-)
> 
> Reviewed-by: Michal Privoznik <mprivozn at redhat.com>
> 
> and pushed.

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.

-- 
Andrea Bolognani / Red Hat / Virtualization




More information about the libvir-list mailing list