* [PATCH 0/3] Use meson_options.txt in the configure script
@ 2021-08-29 17:32 Thomas Huth
2021-08-29 17:32 ` [PATCH 1/3] configure: Add the possibility to read options from meson_options.txt Thomas Huth
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Thomas Huth @ 2021-08-29 17:32 UTC (permalink / raw)
To: qemu-devel, Paolo Bonzini; +Cc: Marc-André Lureau
It's cumbersome to maintain the build options twice, one time in the
configure script and one time in meson_options.txt. Thus let's add some
logic to the configure script to look at the meson_options.txt file
instead of handling every option twice.
Thomas Huth (3):
configure: Add the possibility to read options from meson_options.txt
configure: Remove options that can be handled via meson_options.txt
instead
configure: Get help text from meson_options.txt
configure | 429 ++++++----------------------------------------
meson_options.txt | 2 +-
2 files changed, 53 insertions(+), 378 deletions(-)
--
2.27.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/3] configure: Add the possibility to read options from meson_options.txt
2021-08-29 17:32 [PATCH 0/3] Use meson_options.txt in the configure script Thomas Huth
@ 2021-08-29 17:32 ` Thomas Huth
2021-08-30 14:47 ` Eric Blake
2021-08-29 17:32 ` [PATCH 2/3] configure: Remove options that can be handled via meson_options.txt instead Thomas Huth
` (2 subsequent siblings)
3 siblings, 1 reply; 15+ messages in thread
From: Thomas Huth @ 2021-08-29 17:32 UTC (permalink / raw)
To: qemu-devel, Paolo Bonzini; +Cc: Marc-André Lureau
To avoid double maintenance between the configure script and
meson_options.txt, add some simple logic in the configure script
to read the options from meson_options.txt.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
configure | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/configure b/configure
index 9a79a004d7..b3e6d51916 100755
--- a/configure
+++ b/configure
@@ -836,6 +836,8 @@ fi
werror=""
+meson_options=""
+
for opt do
optarg=$(expr "x$opt" : 'x[^=]*=\(.*\)')
case "$opt" in
@@ -1581,6 +1583,26 @@ for opt do
;;
--disable-slirp-smbd) slirp_smbd=no
;;
+ --enable-*)
+ arg=$(echo "$opt" | sed -e "s/--enable-//" -e "s/-/_/g")
+ if ! grep -q "option('$arg', type[ ]*: 'feature'" \
+ $source_path/meson_options.txt; then
+ echo "ERROR: unknown option $opt"
+ echo "Try '$0 --help' for more information"
+ exit 1
+ fi
+ meson_options="$meson_options -D$arg=enabled"
+ ;;
+ --disable-*)
+ arg=$(echo "$opt" | sed -e "s/--disable-//" -e "s/-/_/g")
+ if ! grep -q "option('$arg', type[ ]*: 'feature'" \
+ $source_path/meson_options.txt; then
+ echo "ERROR: unknown option $opt"
+ echo "Try '$0 --help' for more information"
+ exit 1
+ fi
+ meson_options="$meson_options -D$arg=disabled"
+ ;;
*)
echo "ERROR: unknown option $opt"
echo "Try '$0 --help' for more information"
@@ -5211,7 +5233,7 @@ if test "$skip_meson" = no; then
-Dvhost_user_blk_server=$vhost_user_blk_server -Dmultiprocess=$multiprocess \
-Dfuse=$fuse -Dfuse_lseek=$fuse_lseek -Dguest_agent_msi=$guest_agent_msi -Dbpf=$bpf\
$(if test "$default_feature" = no; then echo "-Dauto_features=disabled"; fi) \
- -Dtcg_interpreter=$tcg_interpreter \
+ -Dtcg_interpreter=$tcg_interpreter $meson_options \
$cross_arg \
"$PWD" "$source_path"
--
2.27.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/3] configure: Remove options that can be handled via meson_options.txt instead
2021-08-29 17:32 [PATCH 0/3] Use meson_options.txt in the configure script Thomas Huth
2021-08-29 17:32 ` [PATCH 1/3] configure: Add the possibility to read options from meson_options.txt Thomas Huth
@ 2021-08-29 17:32 ` Thomas Huth
2021-08-30 15:06 ` Eric Blake
2021-08-29 17:32 ` [PATCH 3/3] configure: Get help text from meson_options.txt Thomas Huth
2021-08-29 21:22 ` [PATCH 0/3] Use meson_options.txt in the configure script Marc-André Lureau
3 siblings, 1 reply; 15+ messages in thread
From: Thomas Huth @ 2021-08-29 17:32 UTC (permalink / raw)
To: qemu-devel, Paolo Bonzini; +Cc: Marc-André Lureau
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(-)
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"
-curl="auto"
-iconv="auto"
-curses="auto"
-docs="auto"
fdt="auto"
netmap="no"
sdl="auto"
-sdl_image="auto"
coreaudio="auto"
-virtiofsd="auto"
-virtfs="auto"
-libudev="auto"
-mpath="auto"
-vnc="auto"
-sparse="auto"
vde="$default_feature"
-vnc_sasl="auto"
-vnc_jpeg="auto"
-vnc_png="auto"
-xkbcommon="auto"
xen=${default_feature:+disabled}
xen_ctrl_version="$default_feature"
-xen_pci_passthrough="auto"
linux_aio="$default_feature"
-linux_io_uring="auto"
-cap_ng="auto"
-attr="auto"
xfs="$default_feature"
tcg="enabled"
membarrier="$default_feature"
@@ -328,15 +308,8 @@ vhost_crypto="$default_feature"
vhost_scsi="$default_feature"
vhost_vsock="$default_feature"
vhost_user="no"
-vhost_user_blk_server="auto"
vhost_user_fs="$default_feature"
vhost_vdpa="$default_feature"
-bpf="auto"
-kvm="auto"
-hax="auto"
-hvf="auto"
-whpx="auto"
-nvmm="auto"
rdma="$default_feature"
pvrdma="$default_feature"
gprof="no"
@@ -362,7 +335,6 @@ bsd="no"
linux="no"
solaris="no"
profiler="no"
-cocoa="auto"
softmmu="yes"
linux_user="no"
bsd_user="no"
@@ -374,45 +346,23 @@ trace_backends="log"
trace_file="trace"
spice="$default_feature"
spice_protocol="auto"
-rbd="auto"
-smartcard="auto"
-u2f="auto"
-libusb="auto"
-usb_redir="auto"
opengl="$default_feature"
cpuid_h="no"
avx2_opt="$default_feature"
capstone="auto"
-lzo="auto"
-snappy="auto"
-bzip2="auto"
-lzfse="auto"
-zstd="auto"
guest_agent="$default_feature"
guest_agent_with_vss="no"
guest_agent_ntddscsi="no"
-guest_agent_msi="auto"
vss_win32_sdk="$default_feature"
win_sdk="no"
want_tools="$default_feature"
-libiscsi="auto"
-libnfs="auto"
coroutine=""
coroutine_pool="$default_feature"
debug_stack_usage="no"
crypto_afalg="no"
cfi="false"
cfi_debug="false"
-seccomp="auto"
-glusterfs="auto"
-gtk="auto"
tls_priority="NORMAL"
-gnutls="auto"
-nettle="auto"
-gcrypt="auto"
-auth_pam="auto"
-vte="auto"
-virglrenderer="auto"
tpm="$default_feature"
libssh="$default_feature"
live_block_migration=${default_feature:-yes}
@@ -428,25 +378,17 @@ vdi=${default_feature:-yes}
vvfat=${default_feature:-yes}
qed=${default_feature:-yes}
parallels=${default_feature:-yes}
-libxml2="auto"
debug_mutex="no"
-libpmem="auto"
default_devices="true"
plugins="$default_feature"
fuzzing="no"
rng_none="no"
secret_keyring="$default_feature"
-libdaxctl="auto"
meson=""
ninja=""
skip_meson=no
-gettext="auto"
-fuse="auto"
-fuse_lseek="auto"
-multiprocess="auto"
slirp_smbd="$default_feature"
-malloc_trim="auto"
gio="$default_feature"
# parse CC options second
@@ -982,38 +924,10 @@ for opt do
;;
--enable-sdl) sdl="enabled"
;;
- --disable-sdl-image) sdl_image="disabled"
- ;;
- --enable-sdl-image) sdl_image="enabled"
- ;;
--disable-qom-cast-debug) qom_cast_debug="no"
;;
--enable-qom-cast-debug) qom_cast_debug="yes"
;;
- --disable-virtfs) virtfs="disabled"
- ;;
- --enable-virtfs) virtfs="enabled"
- ;;
- --disable-libudev) libudev="disabled"
- ;;
- --enable-libudev) libudev="enabled"
- ;;
- --disable-virtiofsd) virtiofsd="disabled"
- ;;
- --enable-virtiofsd) virtiofsd="enabled"
- ;;
- --disable-mpath) mpath="disabled"
- ;;
- --enable-mpath) mpath="enabled"
- ;;
- --disable-vnc) vnc="disabled"
- ;;
- --enable-vnc) vnc="enabled"
- ;;
- --disable-gettext) gettext="disabled"
- ;;
- --enable-gettext) gettext="enabled"
- ;;
--oss-lib=*) oss_lib="$optarg"
;;
--audio-drv-list=*) audio_drv_list="$optarg"
@@ -1046,24 +960,8 @@ for opt do
;;
--disable-tsan) tsan="no"
;;
- --enable-sparse) sparse="enabled"
- ;;
- --disable-sparse) sparse="disabled"
- ;;
--disable-strip) strip_opt="no"
;;
- --disable-vnc-sasl) vnc_sasl="disabled"
- ;;
- --enable-vnc-sasl) vnc_sasl="enabled"
- ;;
- --disable-vnc-jpeg) vnc_jpeg="disabled"
- ;;
- --enable-vnc-jpeg) vnc_jpeg="enabled"
- ;;
- --disable-vnc-png) vnc_png="disabled"
- ;;
- --enable-vnc-png) vnc_png="enabled"
- ;;
--disable-slirp) slirp="disabled"
;;
--enable-slirp) slirp="enabled"
@@ -1084,51 +982,15 @@ for opt do
;;
--enable-xen) xen="enabled"
;;
- --disable-xen-pci-passthrough) xen_pci_passthrough="disabled"
- ;;
- --enable-xen-pci-passthrough) xen_pci_passthrough="enabled"
- ;;
- --disable-brlapi) brlapi="disabled"
- ;;
- --enable-brlapi) brlapi="enabled"
- ;;
- --disable-kvm) kvm="disabled"
- ;;
- --enable-kvm) kvm="enabled"
- ;;
- --disable-hax) hax="disabled"
- ;;
- --enable-hax) hax="enabled"
- ;;
- --disable-hvf) hvf="disabled"
- ;;
- --enable-hvf) hvf="enabled"
- ;;
- --disable-nvmm) nvmm="disabled"
- ;;
- --enable-nvmm) nvmm="enabled"
- ;;
- --disable-whpx) whpx="disabled"
- ;;
- --enable-whpx) whpx="enabled"
- ;;
--disable-tcg-interpreter) tcg_interpreter="false"
;;
--enable-tcg-interpreter) tcg_interpreter="true"
;;
- --disable-cap-ng) cap_ng="disabled"
- ;;
- --enable-cap-ng) cap_ng="enabled"
- ;;
--disable-tcg) tcg="disabled"
plugins="no"
;;
--enable-tcg) tcg="enabled"
;;
- --disable-malloc-trim) malloc_trim="disabled"
- ;;
- --enable-malloc-trim) malloc_trim="enabled"
- ;;
--disable-spice) spice="no"
;;
--enable-spice)
@@ -1141,20 +1003,8 @@ for opt do
;;
--enable-spice-protocol) spice_protocol="yes"
;;
- --disable-libiscsi) libiscsi="disabled"
- ;;
- --enable-libiscsi) libiscsi="enabled"
- ;;
- --disable-libnfs) libnfs="disabled"
- ;;
- --enable-libnfs) libnfs="enabled"
- ;;
--enable-profiler) profiler="yes"
;;
- --disable-cocoa) cocoa="disabled"
- ;;
- --enable-cocoa) cocoa="enabled"
- ;;
--disable-system) softmmu="no"
;;
--enable-system) softmmu="yes"
@@ -1202,18 +1052,6 @@ for opt do
;;
--disable-cfi-debug) cfi_debug="false"
;;
- --disable-curses) curses="disabled"
- ;;
- --enable-curses) curses="enabled"
- ;;
- --disable-iconv) iconv="disabled"
- ;;
- --enable-iconv) iconv="enabled"
- ;;
- --disable-curl) curl="disabled"
- ;;
- --enable-curl) curl="enabled"
- ;;
--disable-fdt) fdt="disabled"
;;
--enable-fdt) fdt="enabled"
@@ -1226,22 +1064,10 @@ for opt do
;;
--enable-linux-aio) linux_aio="yes"
;;
- --disable-linux-io-uring) linux_io_uring="disabled"
- ;;
- --enable-linux-io-uring) linux_io_uring="enabled"
- ;;
- --disable-attr) attr="disabled"
- ;;
- --enable-attr) attr="enabled"
- ;;
--disable-membarrier) membarrier="no"
;;
--enable-membarrier) membarrier="yes"
;;
- --disable-bpf) bpf="disabled"
- ;;
- --enable-bpf) bpf="enabled"
- ;;
--disable-blobs) blobs="false"
;;
--with-pkgversion=*) pkgversion="$optarg"
@@ -1258,10 +1084,6 @@ for opt do
;;
--disable-crypto-afalg) crypto_afalg="no"
;;
- --disable-docs) docs="disabled"
- ;;
- --enable-docs) docs="enabled"
- ;;
--disable-vhost-net) vhost_net="no"
;;
--enable-vhost-net) vhost_net="yes"
@@ -1278,10 +1100,6 @@ for opt do
;;
--enable-vhost-vsock) vhost_vsock="yes"
;;
- --disable-vhost-user-blk-server) vhost_user_blk_server="disabled"
- ;;
- --enable-vhost-user-blk-server) vhost_user_blk_server="enabled"
- ;;
--disable-vhost-user-fs) vhost_user_fs="no"
;;
--enable-vhost-user-fs) vhost_user_fs="yes"
@@ -1290,60 +1108,14 @@ for opt do
;;
--enable-opengl) opengl="yes"
;;
- --disable-rbd) rbd="disabled"
- ;;
- --enable-rbd) rbd="enabled"
- ;;
--disable-xfsctl) xfs="no"
;;
--enable-xfsctl) xfs="yes"
;;
- --disable-smartcard) smartcard="disabled"
- ;;
- --enable-smartcard) smartcard="enabled"
- ;;
- --disable-u2f) u2f="disabled"
- ;;
- --enable-u2f) u2f="enabled"
- ;;
- --disable-libusb) libusb="disabled"
- ;;
- --enable-libusb) libusb="enabled"
- ;;
- --disable-usb-redir) usb_redir="disabled"
- ;;
- --enable-usb-redir) usb_redir="enabled"
- ;;
- --disable-zlib-test)
- ;;
- --disable-lzo) lzo="disabled"
- ;;
- --enable-lzo) lzo="enabled"
- ;;
- --disable-snappy) snappy="disabled"
- ;;
- --enable-snappy) snappy="enabled"
- ;;
- --disable-bzip2) bzip2="disabled"
- ;;
- --enable-bzip2) bzip2="enabled"
- ;;
- --enable-lzfse) lzfse="enabled"
- ;;
- --disable-lzfse) lzfse="disabled"
- ;;
- --disable-zstd) zstd="disabled"
- ;;
- --enable-zstd) zstd="enabled"
- ;;
--enable-guest-agent) guest_agent="yes"
;;
--disable-guest-agent) guest_agent="no"
;;
- --enable-guest-agent-msi) guest_agent_msi="enabled"
- ;;
- --disable-guest-agent-msi) guest_agent_msi="disabled"
- ;;
--with-vss-sdk) vss_win32_sdk=""
;;
--with-vss-sdk=*) vss_win32_sdk="$optarg"
@@ -1360,12 +1132,6 @@ for opt do
;;
--disable-tools) want_tools="no"
;;
- --enable-seccomp) seccomp="enabled"
- ;;
- --disable-seccomp) seccomp="disabled"
- ;;
- --disable-glusterfs) glusterfs="disabled"
- ;;
--disable-avx2) avx2_opt="no"
;;
--enable-avx2) avx2_opt="yes"
@@ -1374,9 +1140,6 @@ for opt do
;;
--enable-avx512f) avx512f_opt="yes"
;;
-
- --enable-glusterfs) glusterfs="enabled"
- ;;
--disable-virtio-blk-data-plane|--enable-virtio-blk-data-plane)
echo "$0: $opt is obsolete, virtio-blk data-plane is always on" >&2
;;
@@ -1386,28 +1149,8 @@ for opt do
--enable-uuid|--disable-uuid)
echo "$0: $opt is obsolete, UUID support is always built" >&2
;;
- --disable-gtk) gtk="disabled"
- ;;
- --enable-gtk) gtk="enabled"
- ;;
--tls-priority=*) tls_priority="$optarg"
;;
- --disable-gnutls) gnutls="disabled"
- ;;
- --enable-gnutls) gnutls="enabled"
- ;;
- --disable-nettle) nettle="disabled"
- ;;
- --enable-nettle) nettle="enabled"
- ;;
- --disable-gcrypt) gcrypt="disabled"
- ;;
- --enable-gcrypt) gcrypt="enabled"
- ;;
- --disable-auth-pam) auth_pam="disabled"
- ;;
- --enable-auth-pam) auth_pam="enabled"
- ;;
--enable-rdma) rdma="yes"
;;
--disable-rdma) rdma="no"
@@ -1416,14 +1159,6 @@ for opt do
;;
--disable-pvrdma) pvrdma="no"
;;
- --disable-vte) vte="disabled"
- ;;
- --enable-vte) vte="enabled"
- ;;
- --disable-virglrenderer) virglrenderer="disabled"
- ;;
- --enable-virglrenderer) virglrenderer="enabled"
- ;;
--disable-tpm) tpm="no"
;;
--enable-tpm) tpm="yes"
@@ -1440,10 +1175,6 @@ for opt do
;;
--enable-numa) numa="yes"
;;
- --disable-libxml2) libxml2="disabled"
- ;;
- --enable-libxml2) libxml2="enabled"
- ;;
--disable-tcmalloc) tcmalloc="no"
;;
--enable-tcmalloc) tcmalloc="yes"
@@ -1525,14 +1256,6 @@ for opt do
;;
--disable-debug-mutex) debug_mutex=no
;;
- --enable-libpmem) libpmem="enabled"
- ;;
- --disable-libpmem) libpmem="disabled"
- ;;
- --enable-xkbcommon) xkbcommon="enabled"
- ;;
- --disable-xkbcommon) xkbcommon="disabled"
- ;;
--enable-plugins) if test "$mingw32" = "yes"; then
error_exit "TCG plugins not currently supported on Windows platforms"
else
@@ -1559,22 +1282,6 @@ for opt do
;;
--disable-keyring) secret_keyring="no"
;;
- --enable-libdaxctl) libdaxctl="enabled"
- ;;
- --disable-libdaxctl) libdaxctl="disabled"
- ;;
- --enable-fuse) fuse="enabled"
- ;;
- --disable-fuse) fuse="disabled"
- ;;
- --enable-fuse-lseek) fuse_lseek="enabled"
- ;;
- --disable-fuse-lseek) fuse_lseek="disabled"
- ;;
- --enable-multiprocess) multiprocess="enabled"
- ;;
- --disable-multiprocess) multiprocess="disabled"
- ;;
--enable-gio) gio=yes
;;
--disable-gio) gio=no
@@ -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 \
- -Dcurl=$curl -Dglusterfs=$glusterfs -Dbzip2=$bzip2 -Dlibiscsi=$libiscsi \
- -Dlibnfs=$libnfs -Diconv=$iconv -Dcurses=$curses -Dlibudev=$libudev\
- -Drbd=$rbd -Dlzo=$lzo -Dsnappy=$snappy -Dlzfse=$lzfse -Dlibxml2=$libxml2 \
- -Dlibdaxctl=$libdaxctl -Dlibpmem=$libpmem -Dlinux_io_uring=$linux_io_uring \
- -Dgnutls=$gnutls -Dnettle=$nettle -Dgcrypt=$gcrypt -Dauth_pam=$auth_pam \
- -Dzstd=$zstd -Dseccomp=$seccomp -Dvirtfs=$virtfs -Dcap_ng=$cap_ng \
- -Dattr=$attr -Ddefault_devices=$default_devices -Dvirglrenderer=$virglrenderer \
- -Ddocs=$docs -Dsphinx_build=$sphinx_build -Dinstall_blobs=$blobs \
- -Dvhost_user_blk_server=$vhost_user_blk_server -Dmultiprocess=$multiprocess \
- -Dfuse=$fuse -Dfuse_lseek=$fuse_lseek -Dguest_agent_msi=$guest_agent_msi -Dbpf=$bpf\
+ -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 \
$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')
option('vnc_jpeg', type : 'feature', value : 'auto',
description: 'JPEG lossy compression for VNC server')
--
2.27.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/3] configure: Get help text from meson_options.txt
2021-08-29 17:32 [PATCH 0/3] Use meson_options.txt in the configure script Thomas Huth
2021-08-29 17:32 ` [PATCH 1/3] configure: Add the possibility to read options from meson_options.txt Thomas Huth
2021-08-29 17:32 ` [PATCH 2/3] configure: Remove options that can be handled via meson_options.txt instead Thomas Huth
@ 2021-08-29 17:32 ` Thomas Huth
2021-08-30 15:30 ` Eric Blake
2021-08-29 21:22 ` [PATCH 0/3] Use meson_options.txt in the configure script Marc-André Lureau
3 siblings, 1 reply; 15+ messages in thread
From: Thomas Huth @ 2021-08-29 17:32 UTC (permalink / raw)
To: qemu-devel, Paolo Bonzini; +Cc: Marc-André Lureau
It's cumbersome to maintain the option help texts twice, once in the
"configure" script and once in meson_options.txt. So let's add some logic to
the configure script to read most of the help texts from meson_options.txt.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
configure | 89 ++++++++++++++++---------------------------------------
1 file changed, 25 insertions(+), 64 deletions(-)
diff --git a/configure b/configure
index cb125c3f84..8446b7b3ea 100755
--- a/configure
+++ b/configure
@@ -1549,7 +1549,6 @@ Advanced options (experts only):
Default:trace-<pid>
--disable-slirp disable SLIRP userspace network connectivity
--enable-tcg-interpreter enable TCI (TCG with bytecode interpreter, experimental and slow)
- --enable-malloc-trim enable libc malloc_trim() for memory optimization
--oss-lib path to OSS library
--cpu=CPU Build for host CPU [$cpu]
--with-coroutine=BACKEND coroutine backend. Supported options:
@@ -1576,16 +1575,13 @@ disabled with --disable-FEATURE, default is enabled if available
user supported user emulation targets
linux-user all linux usermode emulation targets
bsd-user all BSD usermode emulation targets
- docs build documentation
guest-agent build the QEMU Guest Agent
- guest-agent-msi build guest agent Windows MSI installation package
pie Position Independent Executables
modules modules support (non-Windows)
module-upgrades try to load modules from alternate paths for upgrades
debug-tcg TCG debugging (default is disabled)
debug-info debugging information
lto Enable Link-Time Optimization.
- sparse sparse checker
safe-stack SafeStack Stack Smash Protection. Depends on
clang/llvm >= 3.7 and requires coroutine backend ucontext.
cfi Enable Control-Flow Integrity for indirect function calls.
@@ -1595,85 +1591,33 @@ disabled with --disable-FEATURE, default is enabled if available
cfi-debug In case of a cfi violation, a message containing the line that
triggered the error is written to stderr. After the error,
QEMU is still terminated with SIGILL
- gnutls GNUTLS cryptography support
- nettle nettle cryptography support
- gcrypt libgcrypt cryptography support
- auth-pam PAM access control
- sdl SDL UI
- sdl-image SDL Image support for icons
- gtk gtk UI
- vte vte support for the gtk UI
- curses curses UI
- iconv font glyph conversion support
- vnc VNC UI support
- vnc-sasl SASL encryption for VNC server
- vnc-jpeg JPEG lossy compression for VNC server
- vnc-png PNG compression for VNC server
- cocoa Cocoa UI (Mac OS X only)
- virtfs VirtFS
- virtiofsd build virtiofs daemon (virtiofsd)
- libudev Use libudev to enumerate host devices
- mpath Multipath persistent reservation passthrough
- xen xen backend driver support
- xen-pci-passthrough PCI passthrough support for Xen
- brlapi BrlAPI (Braile)
- curl curl connectivity
membarrier membarrier system call (for Linux 4.14+ or Windows)
fdt fdt device tree
- kvm KVM acceleration support
- hax HAX acceleration support
- hvf Hypervisor.framework acceleration support
- nvmm NVMM acceleration support
- whpx Windows Hypervisor Platform acceleration support
rdma Enable RDMA-based migration
pvrdma Enable PVRDMA support
vde support for vde network
netmap support for netmap network
linux-aio Linux AIO support
- linux-io-uring Linux io_uring support
- cap-ng libcap-ng support
- attr attr and xattr support
vhost-net vhost-net kernel acceleration support
vhost-vsock virtio sockets device support
vhost-scsi vhost-scsi kernel target support
vhost-crypto vhost-user-crypto backend support
vhost-kernel vhost kernel backend support
vhost-user vhost-user backend support
- vhost-user-blk-server vhost-user-blk server support
vhost-vdpa vhost-vdpa kernel backend support
- bpf BPF kernel support
spice spice
spice-protocol spice-protocol
- rbd rados block device (rbd)
- libiscsi iscsi support
- libnfs nfs support
- smartcard smartcard support (libcacard)
- u2f U2F support (u2f-emu)
- libusb libusb (for usb passthrough)
live-block-migration Block migration in the main migration stream
- usb-redir usb network redirection support
- lzo support of lzo compression library
- snappy support of snappy compression library
- bzip2 support of bzip2 compression library
- (for reading bzip2-compressed dmg images)
- lzfse support of lzfse compression library
- (for reading lzfse-compressed dmg images)
- zstd support for zstd compression library
- (for migration compression and qcow2 cluster compression)
- seccomp seccomp support
coroutine-pool coroutine freelist (better performance)
- glusterfs GlusterFS backend
tpm TPM support
libssh ssh block device support
numa libnuma support
- libxml2 for Parallels image format
tcmalloc tcmalloc support
jemalloc jemalloc support
avx2 AVX2 optimization support
avx512f AVX512F optimization support
replication replication support
opengl opengl support
- virglrenderer virgl rendering support
xfsctl xfsctl support
qom-cast-debug cast debugging support
tools build qemu-io, qemu-nbd and qemu-img tools
@@ -1688,18 +1632,35 @@ disabled with --disable-FEATURE, default is enabled if available
crypto-afalg Linux AF_ALG crypto backend driver
capstone capstone disassembler support
debug-mutex mutex debugging support
- libpmem libpmem support
- xkbcommon xkbcommon support
rng-none dummy RNG, avoid using /dev/(u)random and getrandom()
- libdaxctl libdaxctl support
- fuse FUSE block device export
- fuse-lseek SEEK_HOLE/SEEK_DATA support for FUSE exports
- multiprocess Out of process device emulation support
gio libgio support
slirp-smbd use smbd (at path --smbd=*) in slirp networking
-
-NOTE: The object files are built at the place where configure is launched
EOF
+
+current_feature=""
+current_desc=""
+while read word1 word2 ; do
+ case "$word1" in
+ option*)
+ if echo "$word2" | grep -q "type[ ]*: 'feature'"; then
+ current_feature=$(echo $word1 | sed -e "s/option('//" -e "s/',.*$//")
+ else
+ current_feature=""
+ fi
+ ;;
+ description:)
+ if [ -n "$current_feature" ]; then
+ printf " %-15s %s\n" "$current_feature" \
+ "$(echo "$word2" | sed -e "s/^'//" -e "s/'.*$//")"
+ current_feature=""
+ fi
+ ;;
+ esac
+done < $source_path/meson_options.txt | sort
+
+echo
+echo "NOTE: The object files are built at the place where configure is launched"
+
exit 0
fi
--
2.27.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 0/3] Use meson_options.txt in the configure script
2021-08-29 17:32 [PATCH 0/3] Use meson_options.txt in the configure script Thomas Huth
` (2 preceding siblings ...)
2021-08-29 17:32 ` [PATCH 3/3] configure: Get help text from meson_options.txt Thomas Huth
@ 2021-08-29 21:22 ` Marc-André Lureau
2021-08-30 5:11 ` Thomas Huth
3 siblings, 1 reply; 15+ messages in thread
From: Marc-André Lureau @ 2021-08-29 21:22 UTC (permalink / raw)
To: Thomas Huth; +Cc: Paolo Bonzini, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 902 bytes --]
Hi Thomas
On Sun, Aug 29, 2021 at 9:32 PM Thomas Huth <thuth@redhat.com> wrote:
> It's cumbersome to maintain the build options twice, one time in the
> configure script and one time in meson_options.txt. Thus let's add some
> logic to the configure script to look at the meson_options.txt file
> instead of handling every option twice.
>
> Thomas Huth (3):
> configure: Add the possibility to read options from meson_options.txt
> configure: Remove options that can be handled via meson_options.txt
> instead
> configure: Get help text from meson_options.txt
>
> configure | 429 ++++++----------------------------------------
> meson_options.txt | 2 +-
> 2 files changed, 53 insertions(+), 378 deletions(-)
>
>
It looks similar to:
https://patchew.org/QEMU/20210107140039.467969-1-pbonzini@redhat.com/
Isn't it? (I haven't studied in details neither, I may be out of topic).
[-- Attachment #2: Type: text/html, Size: 1429 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/3] Use meson_options.txt in the configure script
2021-08-29 21:22 ` [PATCH 0/3] Use meson_options.txt in the configure script Marc-André Lureau
@ 2021-08-30 5:11 ` Thomas Huth
2021-08-30 9:12 ` Philippe Mathieu-Daudé
2021-08-31 12:49 ` Paolo Bonzini
0 siblings, 2 replies; 15+ messages in thread
From: Thomas Huth @ 2021-08-30 5:11 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: Paolo Bonzini, qemu-devel
On 29/08/2021 23.22, Marc-André Lureau wrote:
> Hi Thomas
>
> On Sun, Aug 29, 2021 at 9:32 PM Thomas Huth <thuth@redhat.com
> <mailto:thuth@redhat.com>> wrote:
>
> It's cumbersome to maintain the build options twice, one time in the
> configure script and one time in meson_options.txt. Thus let's add some
> logic to the configure script to look at the meson_options.txt file
> instead of handling every option twice.
>
> Thomas Huth (3):
> configure: Add the possibility to read options from meson_options.txt
> configure: Remove options that can be handled via meson_options.txt
> instead
> configure: Get help text from meson_options.txt
>
> configure | 429 ++++++----------------------------------------
> meson_options.txt | 2 +-
> 2 files changed, 53 insertions(+), 378 deletions(-)
>
>
> It looks similar to:
> https://patchew.org/QEMU/20210107140039.467969-1-pbonzini@redhat.com/
>
> Isn't it? (I haven't studied in details neither, I may be out of topic).
Oh, right, thanks for the pointer, I wasn't aware of that series yet. It's
indeed similar to patch 8/8 from Paolo's series. But while Paolo is using an
additional Perl-script and a json file for handling the configure options,
my series just uses some lines of shell code in the configure script.
Paolo, why did your patch get stalled? ... my way of parsing is certainly
way more fragile, but it's less complicated and seems to work as long as all
the options are written in the same way in meson_options.txt (e.g. as long
as nobody tries to use multi-line descriptions of the options there etc.)...
so maybe if the additional Perl script was too much, this could be a good
compromise?
Thomas
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/3] Use meson_options.txt in the configure script
2021-08-30 5:11 ` Thomas Huth
@ 2021-08-30 9:12 ` Philippe Mathieu-Daudé
2021-08-30 9:21 ` Peter Maydell
2021-08-31 12:49 ` Paolo Bonzini
1 sibling, 1 reply; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-08-30 9:12 UTC (permalink / raw)
To: Thomas Huth, Marc-André Lureau; +Cc: Paolo Bonzini, qemu-devel
On 8/30/21 7:11 AM, Thomas Huth wrote:
> On 29/08/2021 23.22, Marc-André Lureau wrote:
>> Hi Thomas
>>
>> On Sun, Aug 29, 2021 at 9:32 PM Thomas Huth <thuth@redhat.com
>> <mailto:thuth@redhat.com>> wrote:
>>
>> It's cumbersome to maintain the build options twice, one time in the
>> configure script and one time in meson_options.txt. Thus let's add
>> some
>> logic to the configure script to look at the meson_options.txt file
>> instead of handling every option twice.
>>
>> Thomas Huth (3):
>> configure: Add the possibility to read options from
>> meson_options.txt
>> configure: Remove options that can be handled via
>> meson_options.txt
>> instead
>> configure: Get help text from meson_options.txt
>>
>> configure | 429
>> ++++++----------------------------------------
>> meson_options.txt | 2 +-
>> 2 files changed, 53 insertions(+), 378 deletions(-)
>>
>>
>> It looks similar to:
>> https://patchew.org/QEMU/20210107140039.467969-1-pbonzini@redhat.com/
>> Isn't it? (I haven't studied in details neither, I may be out of topic).
>
> Oh, right, thanks for the pointer, I wasn't aware of that series yet.
> It's indeed similar to patch 8/8 from Paolo's series. But while Paolo is
> using an additional Perl-script and a json file for handling the
> configure options, my series just uses some lines of shell code in the
> configure script.
>
> Paolo, why did your patch get stalled? ... my way of parsing is
> certainly way more fragile, but it's less complicated and seems to work
> as long as all the options are written in the same way in
> meson_options.txt (e.g. as long as nobody tries to use multi-line
> descriptions of the options there etc.)... so maybe if the additional
> Perl script was too much, this could be a good compromise?
IIRC while Perl is perfect for parsing, the maintenance cost is too
high. The git-forge / meson switch proved next-gen contributors are
more at ease with Python. The few contributors who are fluent with
Perl are usually very busy maintainers.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/3] Use meson_options.txt in the configure script
2021-08-30 9:12 ` Philippe Mathieu-Daudé
@ 2021-08-30 9:21 ` Peter Maydell
0 siblings, 0 replies; 15+ messages in thread
From: Peter Maydell @ 2021-08-30 9:21 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Marc-André Lureau, Thomas Huth, qemu-devel, Paolo Bonzini
On Mon, 30 Aug 2021 at 10:14, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> IIRC while Perl is perfect for parsing, the maintenance cost is too
> high. The git-forge / meson switch proved next-gen contributors are
> more at ease with Python.
That seems a bit strong to me. But I would say that for code where
most people touching it are non-language-experts who need to just
occasionally dip in to it to fix a problem, rather than spending
most of their time working on that code, Python is easier to
understand than Perl is. Plus we have more Python than Perl; so we
should probably prefer Python in new scripts for the project.
-- PMM
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] configure: Add the possibility to read options from meson_options.txt
2021-08-29 17:32 ` [PATCH 1/3] configure: Add the possibility to read options from meson_options.txt Thomas Huth
@ 2021-08-30 14:47 ` Eric Blake
0 siblings, 0 replies; 15+ messages in thread
From: Eric Blake @ 2021-08-30 14:47 UTC (permalink / raw)
To: Thomas Huth; +Cc: Paolo Bonzini, qemu-devel, Marc-André Lureau
On Sun, Aug 29, 2021 at 07:32:08PM +0200, Thomas Huth wrote:
> To avoid double maintenance between the configure script and
> meson_options.txt, add some simple logic in the configure script
> to read the options from meson_options.txt.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> configure | 24 +++++++++++++++++++++++-
> 1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index 9a79a004d7..b3e6d51916 100755
> --- a/configure
> +++ b/configure
> @@ -836,6 +836,8 @@ fi
>
> werror=""
>
> +meson_options=""
> +
> for opt do
> optarg=$(expr "x$opt" : 'x[^=]*=\(.*\)')
> case "$opt" in
> @@ -1581,6 +1583,26 @@ for opt do
> ;;
> --disable-slirp-smbd) slirp_smbd=no
> ;;
> + --enable-*)
> + arg=$(echo "$opt" | sed -e "s/--enable-//" -e "s/-/_/g")
configure is a /bin/sh script, which means that using echo on
user-supplied arguments is unsafe (a malicious user may provide a
backslash, which not all 'echo' implementations treat the same across
different sh implementations). What's more, $opt starts with
"--enable", and 'echo --enable' may trigger echo to try and warn about
an unknown option. To be portable, you want:
arg=$(printf %s\\n "$opt" | sed...)
> + if ! grep -q "option('$arg', type[ ]*: 'feature'" \
> + $source_path/meson_options.txt; then
> + echo "ERROR: unknown option $opt"
> + echo "Try '$0 --help' for more information"
> + exit 1
> + fi
> + meson_options="$meson_options -D$arg=enabled"
> + ;;
> + --disable-*)
> + arg=$(echo "$opt" | sed -e "s/--disable-//" -e "s/-/_/g")
And again.
> + if ! grep -q "option('$arg', type[ ]*: 'feature'" \
> + $source_path/meson_options.txt; then
> + echo "ERROR: unknown option $opt"
> + echo "Try '$0 --help' for more information"
> + exit 1
> + fi
> + meson_options="$meson_options -D$arg=disabled"
> + ;;
> *)
> echo "ERROR: unknown option $opt"
> echo "Try '$0 --help' for more information"
> @@ -5211,7 +5233,7 @@ if test "$skip_meson" = no; then
> -Dvhost_user_blk_server=$vhost_user_blk_server -Dmultiprocess=$multiprocess \
> -Dfuse=$fuse -Dfuse_lseek=$fuse_lseek -Dguest_agent_msi=$guest_agent_msi -Dbpf=$bpf\
> $(if test "$default_feature" = no; then echo "-Dauto_features=disabled"; fi) \
> - -Dtcg_interpreter=$tcg_interpreter \
> + -Dtcg_interpreter=$tcg_interpreter $meson_options \
> $cross_arg \
> "$PWD" "$source_path"
Otherwise looks like a nice idea.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] configure: Remove options that can be handled via meson_options.txt instead
2021-08-29 17:32 ` [PATCH 2/3] configure: Remove options that can be handled via meson_options.txt instead Thomas Huth
@ 2021-08-30 15:06 ` Eric Blake
2021-08-30 15:33 ` Richard Henderson
2021-08-30 16:33 ` Thomas Huth
0 siblings, 2 replies; 15+ messages in thread
From: Eric Blake @ 2021-08-30 15:06 UTC (permalink / raw)
To: Thomas Huth; +Cc: Paolo Bonzini, qemu-devel, Marc-André Lureau
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] configure: Get help text from meson_options.txt
2021-08-29 17:32 ` [PATCH 3/3] configure: Get help text from meson_options.txt Thomas Huth
@ 2021-08-30 15:30 ` Eric Blake
2021-08-30 16:48 ` Thomas Huth
0 siblings, 1 reply; 15+ messages in thread
From: Eric Blake @ 2021-08-30 15:30 UTC (permalink / raw)
To: Thomas Huth; +Cc: Paolo Bonzini, qemu-devel, Marc-André Lureau
On Sun, Aug 29, 2021 at 07:32:10PM +0200, Thomas Huth wrote:
> It's cumbersome to maintain the option help texts twice, once in the
> "configure" script and once in meson_options.txt. So let's add some logic to
> the configure script to read most of the help texts from meson_options.txt.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> configure | 89 ++++++++++++++++---------------------------------------
> 1 file changed, 25 insertions(+), 64 deletions(-)
>
> diff --git a/configure b/configure
> index cb125c3f84..8446b7b3ea 100755
> --- a/configure
> +++ b/configure
> +
> +current_feature=""
> +current_desc=""
current_desc is unused below.
> +while read word1 word2 ; do
A bit misleading in the variable names. As a sample, you are parsing:
option('malloc_trim', type : 'feature', value : 'auto',
description: 'enable libc malloc_trim() for memory optimization')
which read then splits into:
word1="option('malloc_trim'," word2="type : 'feature', value : 'auto',"
or
word1="description:" word2="'enable libc malloc_trim() for memory optimization')"
Better might be the names $first and $rest. You could also play with
$IFS for more precise splitting, but your hack is good enough for the
current usage.
> + case "$word1" in
> + option*)
> + if echo "$word2" | grep -q "type[ ]*: 'feature'"; then
Unlike my complaint in patch 1 about using echo on user-supplied text,
here you are using it on test in meson_options.txt which is presumably
not going to contain \ or leading -.
> + current_feature=$(echo $word1 | sed -e "s/option('//" -e "s/',.*$//")
> + else
> + current_feature=""
> + fi
If desired, you can save some fork()ing by doing:
case "$word2" in *type*:*"'feature'")
current_feature=${word1%\'*}
current_feature=${current_feature#*\'}
*) current_feature=""
esac
> + ;;
> + description:)
> + if [ -n "$current_feature" ]; then
> + printf " %-15s %s\n" "$current_feature" \
> + "$(echo "$word2" | sed -e "s/^'//" -e "s/'.*$//")"
Similarly,
if [ "$current_feature" ]; then
word2=${word2%\'*}
printf " %-15s %s\n" "$current_feature" "${word2#\'}"
fi
> + current_feature=""
> + fi
> + ;;
> + esac
Missing a *) catchall case (probably a good idea to reset
current_feature back to "" on lines that don't match the two forms you
care about).
> +done < $source_path/meson_options.txt | sort
> +
> +echo
> +echo "NOTE: The object files are built at the place where configure is launched"
> +
> exit 0
> fi
>
> --
> 2.27.0
>
>
Overall pretty clever.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] configure: Remove options that can be handled via meson_options.txt instead
2021-08-30 15:06 ` Eric Blake
@ 2021-08-30 15:33 ` Richard Henderson
2021-08-30 16:33 ` Thomas Huth
1 sibling, 0 replies; 15+ messages in thread
From: Richard Henderson @ 2021-08-30 15:33 UTC (permalink / raw)
To: Eric Blake, Thomas Huth; +Cc: Paolo Bonzini, qemu-devel, Marc-André Lureau
On 8/30/21 8:06 AM, Eric Blake wrote:
> Does meson treat
> -Dbrlapi=auto and the absence of any mention of brlapi identically?
I believe the default is right there in meson_options.txt:
option('brlapi', type : 'feature', value : 'auto',
description: 'brlapi character device driver')
with value: 'auto'
r~
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] configure: Remove options that can be handled via meson_options.txt instead
2021-08-30 15:06 ` Eric Blake
2021-08-30 15:33 ` Richard Henderson
@ 2021-08-30 16:33 ` Thomas Huth
1 sibling, 0 replies; 15+ messages in thread
From: Thomas Huth @ 2021-08-30 16:33 UTC (permalink / raw)
To: Eric Blake; +Cc: Paolo Bonzini, qemu-devel, Marc-André Lureau
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] configure: Get help text from meson_options.txt
2021-08-30 15:30 ` Eric Blake
@ 2021-08-30 16:48 ` Thomas Huth
0 siblings, 0 replies; 15+ messages in thread
From: Thomas Huth @ 2021-08-30 16:48 UTC (permalink / raw)
To: Eric Blake; +Cc: Paolo Bonzini, qemu-devel, Marc-André Lureau
On 30/08/2021 17.30, Eric Blake wrote:
> On Sun, Aug 29, 2021 at 07:32:10PM +0200, Thomas Huth wrote:
>> It's cumbersome to maintain the option help texts twice, once in the
>> "configure" script and once in meson_options.txt. So let's add some logic to
>> the configure script to read most of the help texts from meson_options.txt.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>> configure | 89 ++++++++++++++++---------------------------------------
>> 1 file changed, 25 insertions(+), 64 deletions(-)
>>
>> diff --git a/configure b/configure
>> index cb125c3f84..8446b7b3ea 100755
>> --- a/configure
>> +++ b/configure
>
>> +
>> +current_feature=""
>> +current_desc=""
>
> current_desc is unused below.
>
>> +while read word1 word2 ; do
>
> A bit misleading in the variable names. As a sample, you are parsing:
>
> option('malloc_trim', type : 'feature', value : 'auto',
> description: 'enable libc malloc_trim() for memory optimization')
>
> which read then splits into:
>
> word1="option('malloc_trim'," word2="type : 'feature', value : 'auto',"
> or
> word1="description:" word2="'enable libc malloc_trim() for memory optimization')"
>
> Better might be the names $first and $rest. You could also play with
> $IFS for more precise splitting, but your hack is good enough for the
> current usage.
>
>> + case "$word1" in
>> + option*)
>> + if echo "$word2" | grep -q "type[ ]*: 'feature'"; then
>
> Unlike my complaint in patch 1 about using echo on user-supplied text,
> here you are using it on test in meson_options.txt which is presumably
> not going to contain \ or leading -.
>
>> + current_feature=$(echo $word1 | sed -e "s/option('//" -e "s/',.*$//")
>> + else
>> + current_feature=""
>> + fi
>
> If desired, you can save some fork()ing by doing:
>
> case "$word2" in *type*:*"'feature'")
> current_feature=${word1%\'*}
> current_feature=${current_feature#*\'}
> *) current_feature=""
> esac
>
>> + ;;
>> + description:)
>> + if [ -n "$current_feature" ]; then
>> + printf " %-15s %s\n" "$current_feature" \
>> + "$(echo "$word2" | sed -e "s/^'//" -e "s/'.*$//")"
>
> Similarly,
>
> if [ "$current_feature" ]; then
> word2=${word2%\'*}
> printf " %-15s %s\n" "$current_feature" "${word2#\'}"
> fi
>
>> + current_feature=""
>> + fi
>> + ;;
>> + esac
>
> Missing a *) catchall case (probably a good idea to reset
> current_feature back to "" on lines that don't match the two forms you
> care about).
Thanks for your review, Eric! Your shell script knowledge is always amazing! :-)
I'll wait for Paolo first to chime in to see whether he rather wants to
resume his variant with the Perl or Python script, but if not, I'll include
your suggestions in the next version of the patch.
Thomas
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/3] Use meson_options.txt in the configure script
2021-08-30 5:11 ` Thomas Huth
2021-08-30 9:12 ` Philippe Mathieu-Daudé
@ 2021-08-31 12:49 ` Paolo Bonzini
1 sibling, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2021-08-31 12:49 UTC (permalink / raw)
To: Thomas Huth; +Cc: Marc-André Lureau, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 944 bytes --]
Il lun 30 ago 2021, 07:11 Thomas Huth <thuth@redhat.com> ha scritto:
> Paolo, why did your patch get stalled?
It just wasn't worthwhile at the time given how few meson options were
there. I missed 6.1 and was going to send it out again for 6.2.
The reason for using Perl was because there's no guarantee of knowing the
path to the Python interpreter until after --python is parsed, so there
would be a risk of not being able to give a full help. I don't like Perl
very much, but the difference in obscurity between Perl and Eric-level sh
is small. :)
Paolo
... my way of parsing is certainly
> way more fragile, but it's less complicated and seems to work as long as
> all
> the options are written in the same way in meson_options.txt (e.g. as long
> as nobody tries to use multi-line descriptions of the options there
> etc.)...
> so maybe if the additional Perl script was too much, this could be a good
> compromise?
>
> Thomas
>
>
[-- Attachment #2: Type: text/html, Size: 1594 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2021-08-31 13:22 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-29 17:32 [PATCH 0/3] Use meson_options.txt in the configure script Thomas Huth
2021-08-29 17:32 ` [PATCH 1/3] configure: Add the possibility to read options from meson_options.txt Thomas Huth
2021-08-30 14:47 ` Eric Blake
2021-08-29 17:32 ` [PATCH 2/3] configure: Remove options that can be handled via meson_options.txt instead Thomas Huth
2021-08-30 15:06 ` Eric Blake
2021-08-30 15:33 ` Richard Henderson
2021-08-30 16:33 ` Thomas Huth
2021-08-29 17:32 ` [PATCH 3/3] configure: Get help text from meson_options.txt Thomas Huth
2021-08-30 15:30 ` Eric Blake
2021-08-30 16:48 ` Thomas Huth
2021-08-29 21:22 ` [PATCH 0/3] Use meson_options.txt in the configure script Marc-André Lureau
2021-08-30 5:11 ` Thomas Huth
2021-08-30 9:12 ` Philippe Mathieu-Daudé
2021-08-30 9:21 ` Peter Maydell
2021-08-31 12:49 ` Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).