[libvirt] RPM spec file patch

[Chris Picton]

On Wed, 2011-11-16 at 15:31 -0700, Eric Blake wrote:
> On 11/15/2011 11:42 PM, Chris Picton wrote:
> > Hi
> > 
> > Please accept the following patch to the rpm spec file.

> 
> Thanks for the report.  However, I'm not yet quite convinced that your
> proposed patch is the best approach.  I'll explain, focusing on just the
> nwfilter macros as an example.
> 
[snip]

> Perhaps it would be simpler to do something like this, which 1) fixes
> client_only to allow user override on appropriate platforms, and 2) sets
> default values based on inspecting _both_ with and without variants.
> This is not a complete patch, so much as a request for further
> discussion.  The idea is to demonstrate how defining the defaults for
> with_libvirtd and with_polkit should consider the _with_ variant in
> addition to the _without_ variant; again with the end result being 0 to
> disable, nonzero (whether 010, 011, or 001) to enable.


In my case, I was trying to override the behaviour of the spec file
which is to (in the case of with_openvz)
1) set sane defaults for variables near the top
%define with_openvz        0%{!?_without_openvz:%{server_drivers}}

2) override these with platform specific values later on
%if 0%{?rhel}
%define with_openvz 0
...
%endif

3) use the overridden values. ignoring the user-supplied options

In your patch, it still does not change the above behaviour, as the
defaults are set near the top, but still force overridden further down


The following may be a better way of expressing the dependencies
1) set sane defaults for variables near the top
%define with_openvz        0%{!?_without_openvz:%{server_drivers}}

2) override these with platform specific values later on (but taking
cognisance of the user supplied options

%if 0%{?rhel}
%define with_openvz 0%{?_with_openvz:1}
...
%endif

This is already used in some cases where certain features are turned on
on specific platforms:

%if 0%{?fedora} >= 13 || 0%{?rhel} >= 6
%define with_dtrace 0%{!?_without_dtrace:1}
%endif

So the bulk of the changes would be updating the spec file to replace
%define with_xxx 0

with

%define with_xxx 0{?_with_xxx:1}

And then also updating for the client_only settings as shown below as
well

Regards
Chris






> 
> diff --git i/libvirt.spec.in w/libvirt.spec.in
> index 89f710c..d085feb 100644
> --- i/libvirt.spec.in
> +++ w/libvirt.spec.in
> @@ -11,7 +11,7 @@
>  # A client only build will create a libvirt.so only containing
>  # the generic RPC driver, and test driver and no libvirtd
>  # Default to a full server + client build
> -%define client_only        0
> +%{!?client_only:%define client_only        0}
> 
>  # Now turn off server build in certain cases
> 
> @@ -34,7 +34,7 @@
>  # of any particular OS
> 
>  # First the daemon itself
> -%define with_libvirtd      0%{!?_without_libvirtd:%{server_drivers}}
> +%define with_libvirtd
> 0%{!?_without_libvirtd:%{server_drivers}%{?_with_libvirtd:1}}
>  %define with_avahi         0%{!?_without_avahi:%{server_drivers}}
> 
>  # Then the hypervisor drivers that run on local host
> @@ -64,7 +64,7 @@
>  %define with_selinux       0%{!?_without_selinux:%{server_drivers}}
> 
>  # A few optional bits off by default, we enable later
> -%define with_polkit        0%{!?_without_polkit:0}
> +%define with_polkit        0%{!?_without_polkit:0}%{?_with_polkit:1}
>  %define with_capng         0%{!?_without_capng:0}
>  %define with_netcf         0%{!?_without_netcf:0}
>  %define with_udev          0%{!?_without_udev:0}
> 
>