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


Reply via email to