Re: [libvirt PATCH 1/2] rpm: remove with_bash_completion condition

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Oct 28, 2020 at 04:49:43PM -0400, Neal Gompa wrote:
> On Wed, Oct 28, 2020 at 8:36 AM Daniel P. Berrangé <berrange@xxxxxxxxxx> wrote:
> >
> > The %meson macro sets "--auto-features=enabled", thus any feature in the
> > RPM which has a "with_XXX" condition, needs to explicitly pass a
> > "-DXXX=state" arg to %meson to override the auto features setting.
> >
> > The with_bash_completion condition is always set to 1, so rather than
> > adding an arg to %meson, just remove the condition.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> > ---
> >  libvirt.spec.in | 15 ---------------
> >  1 file changed, 15 deletions(-)
> >
> > diff --git a/libvirt.spec.in b/libvirt.spec.in
> > index 84515cc7de..47fb53c681 100644
> > --- a/libvirt.spec.in
> > +++ b/libvirt.spec.in
> > @@ -94,7 +94,6 @@
> >  %endif
> >
> >  # Other optional features
> > -%define with_bash_completion  0%{!?_without_bash_completion:1}
> >  %define with_numactl          0%{!?_without_numactl:1}
> >
> >  # A few optional bits off by default, we enable later
> > @@ -279,9 +278,7 @@ BuildRequires: glib2-devel >= 2.48
> >  BuildRequires: libxml2-devel
> >  BuildRequires: libxslt
> >  BuildRequires: readline-devel
> > -%if %{with_bash_completion}
> >  BuildRequires: bash-completion >= 2.0
> > -%endif
> >  BuildRequires: gettext
> >  BuildRequires: libtasn1-devel
> >  BuildRequires: gnutls-devel
> > @@ -897,9 +894,7 @@ Requires: %{name}-libs = %{version}-%{release}
> >  Requires: gettext
> >  # Needed by virt-pki-validate script.
> >  Requires: gnutls-utils
> > -%if %{with_bash_completion}
> >  Requires: %{name}-bash-completion = %{version}-%{release}
> > -%endif
> >
> >  %description client
> >  The client binaries needed to access the virtualization
> > @@ -919,20 +914,16 @@ Shared libraries for accessing the libvirt daemon.
> >  %package admin
> >  Summary: Set of tools to control libvirt daemon
> >  Requires: %{name}-libs = %{version}-%{release}
> > -%if %{with_bash_completion}
> >  Requires: %{name}-bash-completion = %{version}-%{release}
> > -%endif
> >
> >  %description admin
> >  The client side utilities to control the libvirt daemon.
> >
> > -%if %{with_bash_completion}
> >  %package bash-completion
> >  Summary: Bash completion script
> >
> >  %description bash-completion
> >  Bash completion script stub.
> > -%endif
> >
> >  %if %{with_wireshark}
> >  %package wireshark
> > @@ -1855,9 +1846,7 @@ exit 0
> >  %{_datadir}/systemtap/tapset/libvirt_qemu_probes*.stp
> >  %endif
> >
> > -%if %{with_bash_completion}
> >  %{_datadir}/bash-completion/completions/virsh
> > -%endif
> >
> >
> >  %{_unitdir}/libvirt-guests.service
> > @@ -1885,14 +1874,10 @@ exit 0
> >  %files admin
> >  %{_mandir}/man1/virt-admin.1*
> >  %{_bindir}/virt-admin
> > -%if %{with_bash_completion}
> >  %{_datadir}/bash-completion/completions/virt-admin
> > -%endif
> >
> > -%if %{with_bash_completion}
> >  %files bash-completion
> >  %{_datadir}/bash-completion/completions/vsh
> > -%endif
> >
> >  %if %{with_wireshark}
> >  %files wireshark
> > --
> > 2.26.2
> >
> 
> This doesn't make sense unless you're ripping out the conditional
> logic from Meson. The bug here would be that flipping the conditional
> does not flip the behavior in Meson.

The RPM spec should only have conditionals that are actually needed to tune
the build options for Fedora / RHEL distros. Since bash completion is always
on for Fedora / RHEL, there's no need for a conditional in the RPM spec.
Meson still wants the conditionals, because we keep all our features with
external deps as conditional in the build system.


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 :|




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux