On Tue, Oct 27, 2020 at 9:30 AM Andrea Bolognani <abologna@xxxxxxxxxx> 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!