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

[Thomas Huth <thuth@redhat.com>]

On 30/08/2021 17.06, Eric Blake wrote:
> 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?

I can confirm what you've already discovered later in your review already: 
The "auto" setting is used by default in meson_options.txt already.

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

Agreed, I think I'll best split it into a separate patch. For the context: I 
should have changed this already in 3a6a1256d46daa2121 ("configure: Allow 
vnc to get disabled with --without-default-features"), but I was not aware 
of the duplicated values between meson_options.txt and the configure script 
at that point in time yet.

  Thomas