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(a)gmail.com&gt; > --- > libvirt.spec.in | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com&gt; and pushed.

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.