[libvirt PATCH 0/2] Fix disabling of libssh/libssh2 in RPM build
RHEL builds disable libssh2, and build root lack libssh2-devel. The RPM build fails though because meson looks for libssh2 and has it set to "enabled", not "auto" or "disabled". Daniel P. Berrangé (2): rpm: remove with_bash_completion condition rpm: tell meson whether to use libssh or libssh2 explicitly libvirt.spec.in | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) -- 2.26.2
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@redhat.com> --- 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
On Wed, Oct 28, 2020 at 8:36 AM Daniel P. Berrangé <berrange@redhat.com> 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@redhat.com> --- 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. -- 真実はいつも一つ！/ Always, there's only one truth!
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@redhat.com> 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@redhat.com> --- 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 :|
On Thu, Oct 29, 2020 at 5:40 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
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@redhat.com> 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@redhat.com> --- 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.
Fine, I guess... Reviewed-by: Neal Gompa <ngompa13@gmail.com> -- 真実はいつも一つ！/ Always, there's only one truth!
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_libssh and with_libssh2 conditions were not exposed to meson, so if either was set disabled, then meson would fail the build if the -devel packages were not found. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- libvirt.spec.in | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/libvirt.spec.in b/libvirt.spec.in index 47fb53c681..8faa8c656f 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1109,6 +1109,18 @@ exit 1 %define arg_storage_iscsi_direct -Dstorage_iscsi_direct=disabled %endif +%if %{with_libssh} + %define arg_libssh -Dlibssh=enabled +%else + %define arg_libssh -Dlibssh=disabled +%endif + +%if %{with_libssh2} + %define arg_libssh2 -Dlibssh2=enabled +%else + %define arg_libssh2 -Dlibssh2=disabled +%endif + %define when %(date +"%%F-%%T") %define where %(hostname) %define who %{?packager}%{!?packager:Unknown} -- 2.26.2
On Wed, Oct 28, 2020 at 12:30:37PM +0000, Daniel P. Berrangé 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_libssh and with_libssh2 conditions were not exposed to meson, so if either was set disabled, then meson would fail the build if the -devel packages were not found.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- libvirt.spec.in | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/libvirt.spec.in b/libvirt.spec.in index 47fb53c681..8faa8c656f 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1109,6 +1109,18 @@ exit 1 %define arg_storage_iscsi_direct -Dstorage_iscsi_direct=disabled %endif
+%if %{with_libssh} + %define arg_libssh -Dlibssh=enabled +%else + %define arg_libssh -Dlibssh=disabled +%endif + +%if %{with_libssh2} + %define arg_libssh2 -Dlibssh2=enabled +%else + %define arg_libssh2 -Dlibssh2=disabled +%endif + %define when %(date +"%%F-%%T") %define where %(hostname) %define who %{?packager}%{!?packager:Unknown}
Opps, failed to commit the second hunk. This is what I'll squash in: -Dfirewalld=enabled \ %{?arg_firewalld_zone} \ %{?arg_wireshark} \ + %{?arg_libssh} \ + %{?arg_libssh2} \ -Dpm_utils=disabled \ -Dnss=enabled \ %{arg_packager} \ 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 Wed, Oct 28, 2020 at 12:30:35PM +0000, Daniel P. Berrangé wrote:
RHEL builds disable libssh2, and build root lack libssh2-devel. The RPM build fails though because meson looks for libssh2 and has it set to "enabled", not "auto" or "disabled".
Daniel P. Berrangé (2): rpm: remove with_bash_completion condition rpm: tell meson whether to use libssh or libssh2 explicitly
libvirt.spec.in | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
participants (3)
-
Daniel P. Berrangé -
Neal Gompa -
Pavel Hrdina