Re: [libvirt] [PATCH] spec: require libvirt-wireshark from libvirt metapackage

On Thu, Feb 13, 2014 at 10:55:11AM +0000, Daniel P. Berrange wrote: > On Thu, Feb 13, 2014 at 10:17:26AM +0100, Michal Privoznik wrote: > > On 12.02.2014 21:28, Eric Blake wrote: > > >In general, the 'libvirt' metapackage should pull in all subpackages. > > >Fix this for the wireshark subpackage created in commit f9ada9f. > > > > > >* libvirt.spec.in (Requires): Add dependency. > > > > > >Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; > > >--- > > > libvirt.spec.in | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > >diff --git a/libvirt.spec.in b/libvirt.spec.in > > >index d3e6048..2d57c71 100644 > > >--- a/libvirt.spec.in > > >+++ b/libvirt.spec.in > > >@@ -428,6 +428,9 @@ Requires: libvirt-daemon-driver-storage = %{version}-%{release} > > > Requires: libvirt-daemon-driver-network = %{version}-%{release} > > > Requires: libvirt-daemon-driver-nodedev = %{version}-%{release} > > > %endif > > >+ %if %{with_wireshark} > > >+Requires: libvirt-wireshark = %{version}-%{release} > > >+ %endif > > > %endif > > > Requires: libvirt-client = %{version}-%{release} > > > > > > > > > Aah, I see you've already pushed this one. However I have doubts > > about it. The wireshark plugin is meant for developers, not ordinary > > users. With this patch: > > > > yum install libvirt > > > > drags wireshark into the dependencies. Me, as a libvirt developer, > > am comfortable with it. The ordinary libvirt users who just creates > > dozen virtual machines may be not. > > > > But I'm open to persuasion :) > > I tend to agree - I don't think we should be pulling in the wireshark > RPM, so I'd suggest we revert this patch. > I haven't realized that libvirt-wireshark requires wireshark; I was under the impression that it just installs the compiled dissector files. Similarly how for example logrotate rules should be installed but should not require logrotate to be installed (and if you want it, you can install it and it "Just Works"). That said, I don't know whether wireshark is needed for the installation of libvirt-wireshark, but if it is not, I'd rather see this in: diff --git a/libvirt.spec.in b/libvirt.spec.in index 2d57c71..a3f1170 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1137,7 +1137,6 @@ virtualization capabilities of recent versions of Linux (and other OSes). %package wireshark Summary: Wireshark dissector plugin for libvirt RPC transactions Group: Development/Libraries -Requires: wireshark Requires: %{name}-client = %{version}-%{release} %description wireshark