[libvirt PATCH] rpm: more fixes for disabling features

Daniel P. Berrangé posted 1 patch 3 years, 4 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20201102124521.159433-1-berrange@redhat.com
libvirt.spec.in | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
[libvirt PATCH] rpm: more fixes for disabling features
Posted by Daniel P. Berrangé 3 years, 4 months ago
The %meson macro sets "--auto-features=enabled", so it is not enough to
disable the driver options, we must also disable any library options
which the drivers depend on.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 libvirt.spec.in | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 06afd0dab8..1b6a9596d8 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1026,15 +1026,15 @@ exit 1
 %endif
 
 %if %{with_esx}
-    %define arg_esx -Ddriver_esx=enabled
+    %define arg_esx -Ddriver_esx=enabled -Dcurl=enabled
 %else
-    %define arg_esx -Ddriver_esx=disabled
+    %define arg_esx -Ddriver_esx=disabled -Dcurl=disabled
 %endif
 
 %if %{with_hyperv}
-    %define arg_hyperv -Ddriver_hyperv=enabled
+    %define arg_hyperv -Ddriver_hyperv=enabled -Dopenwsman=enabled
 %else
-    %define arg_hyperv -Ddriver_hyperv=disabled
+    %define arg_hyperv -Ddriver_hyperv=disabled -Dopenwsman=disabled
 %endif
 
 %if %{with_vmware}
@@ -1056,9 +1056,9 @@ exit 1
 %endif
 
 %if %{with_storage_gluster}
-    %define arg_storage_gluster -Dstorage_gluster=enabled
+    %define arg_storage_gluster -Dstorage_gluster=enabled -Dglusterfs=enabled
 %else
-    %define arg_storage_gluster -Dstorage_gluster=disabled
+    %define arg_storage_gluster -Dstorage_gluster=disabled -Dglusterfs=disabled
 %endif
 
 %if %{with_storage_zfs}
@@ -1104,9 +1104,9 @@ exit 1
 %endif
 
 %if %{with_storage_iscsi_direct}
-    %define arg_storage_iscsi_direct -Dstorage_iscsi_direct=enabled
+    %define arg_storage_iscsi_direct -Dstorage_iscsi_direct=enabled -Dlibiscsi=enabled
 %else
-    %define arg_storage_iscsi_direct -Dstorage_iscsi_direct=disabled
+    %define arg_storage_iscsi_direct -Dstorage_iscsi_direct=disabled -Dlibiscsi=disabled
 %endif
 
 %if %{with_libssh}
-- 
2.28.0

Re: [libvirt PATCH] rpm: more fixes for disabling features
Posted by Pavel Hrdina 3 years, 4 months ago
On Mon, Nov 02, 2020 at 12:45:21PM +0000, Daniel P. Berrangé wrote:
> The %meson macro sets "--auto-features=enabled", so it is not enough to
> disable the driver options, we must also disable any library options
> which the drivers depend on.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  libvirt.spec.in | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index 06afd0dab8..1b6a9596d8 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -1026,15 +1026,15 @@ exit 1
>  %endif

This fixes the current issue with RPM dependencies so I'm willing to
give it an ACK, however, I would like to propose to get rid of the
library specific options if the library is used only by single driver,
the library is mandatory for that driver.

Historically we had all of the libraries with autoconf because it was
possible to provide a custom path to a library using the build option
in addition to enable/disable state. With meson it is no longer
possible and users have to use different ways to provide custom path
for libraries.

Pavel
Re: [libvirt PATCH] rpm: more fixes for disabling features
Posted by Daniel P. Berrangé 3 years, 4 months ago
On Mon, Nov 02, 2020 at 02:13:41PM +0100, Pavel Hrdina wrote:
> On Mon, Nov 02, 2020 at 12:45:21PM +0000, Daniel P. Berrangé wrote:
> > The %meson macro sets "--auto-features=enabled", so it is not enough to
> > disable the driver options, we must also disable any library options
> > which the drivers depend on.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  libvirt.spec.in | 16 ++++++++--------
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> > 
> > diff --git a/libvirt.spec.in b/libvirt.spec.in
> > index 06afd0dab8..1b6a9596d8 100644
> > --- a/libvirt.spec.in
> > +++ b/libvirt.spec.in
> > @@ -1026,15 +1026,15 @@ exit 1
> >  %endif
> 
> This fixes the current issue with RPM dependencies so I'm willing to
> give it an ACK, however, I would like to propose to get rid of the
> library specific options if the library is used only by single driver,
> the library is mandatory for that driver.
> 
> Historically we had all of the libraries with autoconf because it was
> possible to provide a custom path to a library using the build option
> in addition to enable/disable state. With meson it is no longer
> possible and users have to use different ways to provide custom path
> for libraries.

I tend to have a preference for being able to disable individual libraries,
while then letting the drivers them auto-select enabled/disabled based on
whether the libraries they need are present. IOW, I think the meson options
for libraries are good - they just clash with fact that RPM forces all
auto-options to enabled, so we need this explicit hammer to turn them off
again.

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

Re: [libvirt PATCH] rpm: more fixes for disabling features
Posted by Pavel Hrdina 3 years, 4 months ago
On Wed, Nov 04, 2020 at 09:44:39AM +0000, Daniel P. Berrangé wrote:
> On Mon, Nov 02, 2020 at 02:13:41PM +0100, Pavel Hrdina wrote:
> > On Mon, Nov 02, 2020 at 12:45:21PM +0000, Daniel P. Berrangé wrote:
> > > The %meson macro sets "--auto-features=enabled", so it is not enough to
> > > disable the driver options, we must also disable any library options
> > > which the drivers depend on.
> > > 
> > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > > ---
> > >  libvirt.spec.in | 16 ++++++++--------
> > >  1 file changed, 8 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/libvirt.spec.in b/libvirt.spec.in
> > > index 06afd0dab8..1b6a9596d8 100644
> > > --- a/libvirt.spec.in
> > > +++ b/libvirt.spec.in
> > > @@ -1026,15 +1026,15 @@ exit 1
> > >  %endif

I guess this will be longer discussion so

Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

to this patch to fix the current issue.

> > This fixes the current issue with RPM dependencies so I'm willing to
> > give it an ACK, however, I would like to propose to get rid of the
> > library specific options if the library is used only by single driver,
> > the library is mandatory for that driver.
> > 
> > Historically we had all of the libraries with autoconf because it was
> > possible to provide a custom path to a library using the build option
> > in addition to enable/disable state. With meson it is no longer
> > possible and users have to use different ways to provide custom path
> > for libraries.
> 
> I tend to have a preference for being able to disable individual libraries,
> while then letting the drivers them auto-select enabled/disabled based on
> whether the libraries they need are present. IOW, I think the meson options
> for libraries are good - they just clash with fact that RPM forces all
> auto-options to enabled, so we need this explicit hammer to turn them off
> again.

The situation with our options is a complete mess. For example as this
patch shows we have -Dglusterfs and -Dstorage_gluster options and both
controls if the storage gluster is compiled or not.

On the other hand for rbd storage we have only -Dstorage_rbd which uses
librbd and librados bud we don't have options for these libraries.

There are other cases where we are inconsistent. The question is what we
want to expose as build options because currently we have these cases:

    - options only for libraries which enable some libvirt features

    - options only for libvirt features which require some libraries

    - options for both libvirt features and the required libraries

    - optional libraries/features without any options

We should decide what we want to have, fix the build system and write
some guidelines for future additions of new libraries/features.

Pavel