The completion script is so tiny that it makes more sense to
potentially have two copies of it on disk than for it to have
its own package.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 libvirt.spec.in | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 0fec79c133..11abbb080e 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -430,9 +430,6 @@ Summary: Server side daemon and supporting files for libvirt library
 # The client side, i.e. shared libs are in a subpackage
 Requires: %{name}-libs = %{version}-%{release}
 
-# bash completion support for virt-admin
-Requires: %{name}-bash-completion = %{version}-%{release}
-
 # (client invokes 'nc' against the UNIX socket on the server)
 Requires: /usr/bin/nc
 
@@ -469,6 +466,7 @@ Requires: gettext
 # Ensure smooth upgrades
 Obsoletes: libvirt-admin < 7.3.0
 Provides: libvirt-admin
+Obsoletes: libvirt-bash-completion < 7.3.0
 
 %description daemon
 Server side daemon required to manage the virtualization capabilities
@@ -908,7 +906,9 @@ Summary: Client side utilities of the libvirt library
 Requires: %{name}-libs = %{version}-%{release}
 # Needed by virt-pki-validate script.
 Requires: gnutls-utils
-Requires: %{name}-bash-completion = %{version}-%{release}
+
+# Ensure smooth upgrades
+Obsoletes: libvirt-bash-completion < 7.3.0
 
 %description client
 The client binaries needed to access the virtualization
@@ -925,12 +925,6 @@ Requires: cyrus-sasl-gssapi
 %description libs
 Shared libraries for accessing the libvirt daemon.
 
-%package bash-completion
-Summary: Bash completion script
-
-%description bash-completion
-Bash completion script stub.
-
 %if %{with_wireshark}
 %package wireshark
 Summary: Wireshark dissector plugin for libvirt RPC transactions
@@ -1296,6 +1290,17 @@ mv $RPM_BUILD_ROOT%{_datadir}/systemtap/tapset/libvirt_qemu_probes.stp \
     %endif
 %endif
 
+# Turning these symlinks into regular files allows us to make the
+# -client and -daemon packages, both of which need bash-completion
+# support, completely independent by only paying a tiny price in
+# terms of additional disk usage
+for cmd in virsh virt-admin; do
+  rm -f $RPM_BUILD_ROOT%{_datadir}/bash-completion/completions/$cmd
+  cp $RPM_BUILD_ROOT%{_datadir}/bash-completion/completions/vsh \
+     $RPM_BUILD_ROOT%{_datadir}/bash-completion/completions/$cmd
+done
+rm -f $RPM_BUILD_ROOT%{_datadir}/bash-completion/completions/vsh
+
 %check
 # Building on slow archs, like emulated s390x in Fedora copr, requires
 # raising the test timeout
@@ -1912,9 +1917,6 @@ exit 0
 
 %{_datadir}/libvirt/test-screenshot.png
 
-%files bash-completion
-%{_datadir}/bash-completion/completions/vsh
-
 %if %{with_wireshark}
 %files wireshark
 %{wireshark_plugindir}/libvirt.so
-- 
2.26.3

On 4/20/21 2:30 PM, Andrea Bolognani wrote:
> The completion script is so tiny that it makes more sense to
> potentially have two copies of it on disk than for it to have
> its own package.
> 
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
>   libvirt.spec.in | 28 +++++++++++++++-------------
>   1 file changed, 15 insertions(+), 13 deletions(-)
> 

Don't forget to remove package description from 
docs/kbase/rpm-deployment.rst.

Michal

On Tue, Apr 20, 2021 at 02:30:33PM +0200, Andrea Bolognani wrote:
> The completion script is so tiny that it makes more sense to
> potentially have two copies of it on disk than for it to have
> its own package.
> 
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
>  libvirt.spec.in | 28 +++++++++++++++-------------
>  1 file changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index 0fec79c133..11abbb080e 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -430,9 +430,6 @@ Summary: Server side daemon and supporting files for libvirt library
>  # The client side, i.e. shared libs are in a subpackage
>  Requires: %{name}-libs = %{version}-%{release}
>  
> -# bash completion support for virt-admin
> -Requires: %{name}-bash-completion = %{version}-%{release}
> -
>  # (client invokes 'nc' against the UNIX socket on the server)
>  Requires: /usr/bin/nc
>  
> @@ -469,6 +466,7 @@ Requires: gettext
>  # Ensure smooth upgrades
>  Obsoletes: libvirt-admin < 7.3.0
>  Provides: libvirt-admin
> +Obsoletes: libvirt-bash-completion < 7.3.0
>  
>  %description daemon
>  Server side daemon required to manage the virtualization capabilities
> @@ -908,7 +906,9 @@ Summary: Client side utilities of the libvirt library
>  Requires: %{name}-libs = %{version}-%{release}
>  # Needed by virt-pki-validate script.
>  Requires: gnutls-utils
> -Requires: %{name}-bash-completion = %{version}-%{release}
> +
> +# Ensure smooth upgrades
> +Obsoletes: libvirt-bash-completion < 7.3.0
>  
>  %description client
>  The client binaries needed to access the virtualization
> @@ -925,12 +925,6 @@ Requires: cyrus-sasl-gssapi
>  %description libs
>  Shared libraries for accessing the libvirt daemon.
>  
> -%package bash-completion
> -Summary: Bash completion script
> -
> -%description bash-completion
> -Bash completion script stub.
> -
>  %if %{with_wireshark}
>  %package wireshark
>  Summary: Wireshark dissector plugin for libvirt RPC transactions
> @@ -1296,6 +1290,17 @@ mv $RPM_BUILD_ROOT%{_datadir}/systemtap/tapset/libvirt_qemu_probes.stp \
>      %endif
>  %endif
>  
> +# Turning these symlinks into regular files allows us to make the
> +# -client and -daemon packages, both of which need bash-completion
> +# support, completely independent by only paying a tiny price in
> +# terms of additional disk usage
> +for cmd in virsh virt-admin; do
> +  rm -f $RPM_BUILD_ROOT%{_datadir}/bash-completion/completions/$cmd
> +  cp $RPM_BUILD_ROOT%{_datadir}/bash-completion/completions/vsh \
> +     $RPM_BUILD_ROOT%{_datadir}/bash-completion/completions/$cmd
> +done
> +rm -f $RPM_BUILD_ROOT%{_datadir}/bash-completion/completions/vsh

IMHO this is not good. It results in a "virt-admin" completion
file that installs completions for "virsh", and vica-verca.

If we're going to separate them, then  do it properly in the
source build, not the RPM.

> +
>  %check
>  # Building on slow archs, like emulated s390x in Fedora copr, requires
>  # raising the test timeout
> @@ -1912,9 +1917,6 @@ exit 0
>  
>  %{_datadir}/libvirt/test-screenshot.png
>  
> -%files bash-completion
> -%{_datadir}/bash-completion/completions/vsh
> -
>  %if %{with_wireshark}
>  %files wireshark
>  %{wireshark_plugindir}/libvirt.so
> -- 
> 2.26.3
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

On Tue, 2021-04-20 at 15:18 +0100, Daniel P. Berrangé wrote:
> On Tue, Apr 20, 2021 at 02:30:33PM +0200, Andrea Bolognani wrote:
> > +# Turning these symlinks into regular files allows us to make the
> > +# -client and -daemon packages, both of which need bash-completion
> > +# support, completely independent by only paying a tiny price in
> > +# terms of additional disk usage
> > +for cmd in virsh virt-admin; do
> > +  rm -f $RPM_BUILD_ROOT%{_datadir}/bash-completion/completions/$cmd
> > +  cp $RPM_BUILD_ROOT%{_datadir}/bash-completion/completions/vsh \
> > +     $RPM_BUILD_ROOT%{_datadir}/bash-completion/completions/$cmd
> > +done
> > +rm -f $RPM_BUILD_ROOT%{_datadir}/bash-completion/completions/vsh
> 
> IMHO this is not good. It results in a "virt-admin" completion
> file that installs completions for "virsh", and vica-verca.
> 
> If we're going to separate them, then  do it properly in the
> source build, not the RPM.

The current situation is exactly as you describe: the bash completion
file for both virsh and virt-admin also defines completion rules for
the other command, regardless of whether it's actually installed, so
this change wouldn't make the situation any worse.

But you're right, if we're touching it anyway might as well try to
make it better :)

-- 
Andrea Bolognani / Red Hat / Virtualization