[libvirt] [PATCH] specfile: fix make rpm when with_driver_modules is 1

[Laine Stump]

On 12/14/2013 07:21 AM, Jim Fehlig wrote:
> Laine Stump wrote:
>> Commit ff76566 moved around things in the specfiles to put
>> driver-specific files into their appropriate sub-packages (when
>> with_driver_modules == 1), but accidentally changed things so that the
>> deamon-driver-network and daemon-config-network files were only
>> included in a package when with_driver_modules == 0. This broke "make
>> rpm" on fedora (where with_driver_modules == 1).
>>
>> This patch follows the pattern (already used for the files in other
>> sub-modules) of duplicating the files for the main package
>> (!with_driver_modules) and the sub-package (with_driver_modules). I
>> think this is asking for trouble, but the first priority is to fix the
>> build, then worry about removing redundancy later.
>> ---
>>
>> I would push this as a build breaker, but am unsure enough of my
>> specfile knowledge to be certain that I'm not breaking something else
>> (it does allow make rpm to complete successfully). Since I have to
>> leave for the weekend *right now*, I would appreciate if someone else
>> can push this for me if it's a proper fix.
>>
>> Thanks!
>>
>>  libvirt.spec.in | 20 ++++++++++++++------
>>  1 file changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/libvirt.spec.in b/libvirt.spec.in
>> index f615c62..d7d0e7e 100644
>> --- a/libvirt.spec.in
>> +++ b/libvirt.spec.in
>> @@ -1843,14 +1843,16 @@ exit 0
>>  
>>      %if ! %{with_driver_modules}
>>          %if %{with_network}
>> +# Files from the daemon-driver-network sub-package
>>  %dir %attr(0700, root, root) %{_sysconfdir}/libvirt/qemu/
>>  %dir %attr(0700, root, root) %{_sysconfdir}/libvirt/qemu/networks/
>>  %dir %attr(0700, root, root) %{_sysconfdir}/libvirt/qemu/networks/autostart
>>  %dir %{_datadir}/libvirt/networks/
>> -%{_datadir}/libvirt/networks/default.xml
>>  %ghost %dir %{_localstatedir}/run/libvirt/network/
>>  %dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/network/
>>  %dir %attr(0755, root, root) %{_localstatedir}/lib/libvirt/dnsmasq/
>> +# File from the daemon-config-network sub-package
>> +%{_datadir}/libvirt/networks/default.xml
>>   
> The daemon-config-network package is only conditional upon
> with_libvirtd, so this should be moved out of the with_driver_modules logic.
>
>>          %endif
>>          %if %{with_storage_disk}
>>  %attr(0755, root, root) %{_libexecdir}/libvirt_parthelper
>> @@ -1894,11 +1896,6 @@ exit 0
>>          %endif
>>      %endif # ! %{with_driver_modules}
>>  
>> -    %if %{with_network}
>> -%files daemon-config-network
>> -%defattr(-, root, root)
>> -    %endif
>> -
>>   
> I think this hunk should stay and default.xml should be listed here.
>
>>      %if %{with_nwfilter}
>>  %files daemon-config-nwfilter
>>  %defattr(-, root, root)
>> @@ -1913,8 +1910,19 @@ exit 0
>>          %endif
>>  
>>          %if %{with_network}
>> +%files daemon-config-network
>> +%defattr(-, root, root)
>> +%{_datadir}/libvirt/networks/default.xml
>> +
>>   
> This is in with_driver_modules. As above, daemon-config-network only
> depends on with_libvirtd.

Right. I had asked Dan about that on IRC and must have thought I was
asking about daemon-driver-network rather than daemon-config-network, as
he said "we only introduce those fine grained packages when we added
driver modules". But looking at the specfile where the packages are
declared, I see that you are correct.


>
>>  %files daemon-driver-network
>>  %defattr(-, root, root)
>> +%dir %attr(0700, root, root) %{_sysconfdir}/libvirt/qemu/
>> +%dir %attr(0700, root, root) %{_sysconfdir}/libvirt/qemu/networks/
>> +%dir %attr(0700, root, root) %{_sysconfdir}/libvirt/qemu/networks/autostart
>> +%dir %{_datadir}/libvirt/networks/
>> +%ghost %dir %{_localstatedir}/run/libvirt/network/
>> +%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/network/
>> +%dir %attr(0755, root, root) %{_localstatedir}/lib/libvirt/dnsmasq/
>>   
> This part looks correct, except I think '%dir
> %{_datadir}/libvirt/networks/' can be moved to daemon-config-network.

Actually, this comment caused me to look back at what is being installed
when and where again (I'm sure glad I didn't push my patch without
review!) and I realized that %{_datadir}/libvirt/networks is
/usr/share/libvirt/networks, which means that this copy of default.xml
is just an *example*, not actual config (the config file is
%{_sysconfdir}/libvirt/qemu/networks/default.xml). So I think both items
need to be included in daemon-driver-network (and also remain in the
main RPM when with_driver_modules is 0).

I've sent an updated patch, on a new thread so it isn't lost, but once
again am not pushing it without review even though the build is broken :-)