[PATCH 0/4] Fixes for the --without-default-features configure switch

Thomas Huth posted 4 patches 4 years, 6 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210713093155.677589-1-thuth@redhat.com
configure   | 8 +++++---
meson.build | 2 +-
2 files changed, 6 insertions(+), 4 deletions(-)
[PATCH 0/4] Fixes for the --without-default-features configure switch
Posted by Thomas Huth 4 years, 6 months ago
Many features do not get properly disabled when the user runs the
configure script with --without-default-features. Let's fix that now.

Thomas Huth (4):
  configure: Fix --without-default-features propagation to meson
  configure: Allow vnc to get disabled with --without-default-features
  configure: Fix the default setting of the "xen" feature
  configure: Let --without-default-features disable vhost-kernel and
    vhost-vdpa

 configure   | 8 +++++---
 meson.build | 2 +-
 2 files changed, 6 insertions(+), 4 deletions(-)

-- 
2.27.0


Re: [PATCH 0/4] Fixes for the --without-default-features configure switch
Posted by Cole Robinson 4 years, 6 months ago
On 7/13/21 5:31 AM, Thomas Huth wrote:
> Many features do not get properly disabled when the user runs the
> configure script with --without-default-features. Let's fix that now.
> 
> Thomas Huth (4):
>   configure: Fix --without-default-features propagation to meson
>   configure: Allow vnc to get disabled with --without-default-features
>   configure: Fix the default setting of the "xen" feature
>   configure: Let --without-default-features disable vhost-kernel and
>     vhost-vdpa
> 

Patches look fine and fix some issues but others persist
(--disable-system isn't triggered). IMO this needs an audit, but more
importantly 'configure' should be rearranged a bit to make this less
likely to regress:

* move all the --enable/--disable variable init into one section with
nothing else mixed in

* convert the values to all use
$default_yes/no/auto/enabled/disabled/... variable syntax so visually
it's consistent, and if a default is ever changed like $default_no ->
$default_yes then we behave correctly (as opposed to 'no' -> 'yes').

Thanks,
Cole


Re: [PATCH 0/4] Fixes for the --without-default-features configure switch
Posted by Paolo Bonzini 4 years, 6 months ago
On 13/07/21 16:54, Cole Robinson wrote:
> Patches look fine and fix some issues but others persist
> (--disable-system isn't triggered).

I wouldn't say --disable-system counts as a feature, since it's really a 
shortcut for choosing a subset of the targets.  Likewise for linux_user 
and bsd_user.

> IMO this needs an audit, but more
> importantly 'configure' should be rearranged a bit to make this less
> likely to regress:
> 
> * move all the --enable/--disable variable init into one section with
> nothing else mixed in
> 
> * convert the values to all use
> $default_yes/no/auto/enabled/disabled/... variable syntax so visually
> it's consistent, and if a default is ever changed like $default_no ->
> $default_yes then we behave correctly (as opposed to 'no' -> 'yes').

This is a nice idea.  We should only have default_yes/no/auto, plus 
"auto" for Meson options.

Also there's the idea of parsing --enable/--disable options for Meson 
options automatically from the introspection data.  This has the 
advantage that you get the default automatically from meson_options.txt 
and -Dauto_features, without any code in configure.

Paolo


Re: [PATCH 0/4] Fixes for the --without-default-features configure switch
Posted by Paolo Bonzini 4 years, 6 months ago
On 13/07/21 11:31, Thomas Huth wrote:
> Many features do not get properly disabled when the user runs the
> configure script with --without-default-features. Let's fix that now.
> 
> Thomas Huth (4):
>    configure: Fix --without-default-features propagation to meson
>    configure: Allow vnc to get disabled with --without-default-features
>    configure: Fix the default setting of the "xen" feature
>    configure: Let --without-default-features disable vhost-kernel and
>      vhost-vdpa
> 
>   configure   | 8 +++++---
>   meson.build | 2 +-
>   2 files changed, 6 insertions(+), 4 deletions(-)
> 

Queued, thanks.

Paolo