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

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 %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 - %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 + %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/ %{_libdir}/%{name}/connection-driver/libvirt_driver_network.so %endif -- 1.8.3.1

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.
%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. I know you are gone for the weekend, but maybe someone else can test 'make rpm' with the attached patch in a similar environment. I am also hesitant to push this without a review. Regards, Jim

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 :-)
participants (2)
-
Jim Fehlig
-
Laine Stump