* [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
* 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
* [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
* 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 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
* [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 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 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-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 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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.