Re: [PATCH 2/3] configure: Remove options that can be handled via meson_options.txt instead

[Eric Blake]

On Sun, Aug 29, 2021 at 07:32:09PM +0200, Thomas Huth wrote:
> These trivial options can now be handled via the new generic code
> that parses meson_options.txt
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  configure         | 316 +---------------------------------------------
>  meson_options.txt |   2 +-
>  2 files changed, 5 insertions(+), 313 deletions(-)

Picking on one example...

> 
> diff --git a/configure b/configure
> index b3e6d51916..cb125c3f84 100755
> --- a/configure
> +++ b/configure
> @@ -291,34 +291,14 @@ for opt do
>    esac
>  done
>  
> -brlapi="auto"

> -  --disable-brlapi) brlapi="disabled"
> -  ;;
> -  --enable-brlapi) brlapi="enabled"
> -  ;;

> @@ -5213,25 +4920,10 @@ if test "$skip_meson" = no; then
>          -Db_pie=$(if test "$pie" = yes; then echo true; else echo false; fi) 
> \
>          ${staticpic:+-Db_staticpic=$staticpic} \
>          -Db_coverage=$(if test "$gcov" = yes; then echo true; else echo 
> false; fi) \
> -        -Db_lto=$lto -Dcfi=$cfi -Dcfi_debug=$cfi_debug \
> -        -Dmalloc=$malloc -Dmalloc_trim=$malloc_trim -Dsparse=$sparse \
> -        -Dkvm=$kvm -Dhax=$hax -Dwhpx=$whpx -Dhvf=$hvf -Dnvmm=$nvmm \
> -        -Dxen=$xen -Dxen_pci_passthrough=$xen_pci_passthrough -Dtcg=$tcg \
> -        -Dcocoa=$cocoa -Dgtk=$gtk -Dmpath=$mpath -Dsdl=$sdl 
> -Dsdl_image=$sdl_image \
> -        -Dlibusb=$libusb -Dsmartcard=$smartcard -Dusb_redir=$usb_redir 
> -Dvte=$vte \
> -        -Dvnc=$vnc -Dvnc_sasl=$vnc_sasl -Dvnc_jpeg=$vnc_jpeg 
> -Dvnc_png=$vnc_png \
> -        -Dgettext=$gettext -Dxkbcommon=$xkbcommon -Du2f=$u2f 
> -Dvirtiofsd=$virtiofsd \
> -        -Dcapstone=$capstone -Dslirp=$slirp -Dfdt=$fdt -Dbrlapi=$brlapi \

Pre-patch, if you used neither --enable-brlapi nor --disable-brlapi,
we called $meson with -Dbrlapi=auto.

> +        -Db_lto=$lto -Dcfi=$cfi -Dcfi_debug=$cfi_debug -Dmalloc=$malloc \
> +        -Ddefault_devices=$default_devices -Dxen=$xen -Dtcg=$tcg -Dsdl=$sdl \
> +        -Dcapstone=$capstone -Dslirp=$slirp -Dfdt=$fdt \
> +        -Dsphinx_build=$sphinx_build -Dinstall_blobs=$blobs \
>          $(if test "$default_feature" = no; then echo 
> "-Dauto_features=disabled"; fi) \
>          -Dtcg_interpreter=$tcg_interpreter $meson_options \

Post-patch, if you use an option, $meson_options includes
-Dbrlapi=disabled or -Dbrlapi=enabled (per patch 1), but if you omit
an option, now we aren't passing anything.  Does meson treat
-Dbrlapi=auto and the absence of any mention of brlapi identically?
If so, then this patch is good; if not, you are stripping too much.

>          $cross_arg \
> diff --git a/meson_options.txt b/meson_options.txt
> index a9a9b8f4c6..2c89e79e8b 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -120,7 +120,7 @@ option('usb_redir', type : 'feature', value : 'auto',
>         description: 'libusbredir support')
>  option('virglrenderer', type : 'feature', value : 'auto',
>         description: 'virgl rendering support')
> -option('vnc', type : 'feature', value : 'enabled',
> +option('vnc', type : 'feature', value : 'auto',
>         description: 'VNC server')

Why is the default for this option changed?  It looks unrelated to the
mechanical changes in the rest of the patch, enough so to warrant its
own patch, or at least special mention in the commit message.

But I think I just answered my question above: in meson_options.txt,
we typically default an unspecified option to 'auto'.  So now it's
just making sure that all such options removed from configure in this
patch have a sane default.

/me goes reading...

Yep, every option deleted above appears with 'auto' in
meson_options.txt.  It's annoying that we did not have consistent
ordering in any of the three places; alphabetic might have been nice;
we even had --disable-glusterfs and --enable-glusterfs split by
unrelated options in the case statement.

Whether you split the patch or amend the commit message, I've now done
enough review that I'm happy if you add:

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org