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.
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.
--
Andrea Bolognani / Red Hat / Virtualization
