qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Thomas Huth <thuth@redhat.com>
Cc: "Paolo Bonzini" <pbonzini@redhat.com>,
	qemu-devel@nongnu.org,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>
Subject: Re: [PATCH 3/3] configure: Get help text from meson_options.txt
Date: Mon, 30 Aug 2021 10:30:22 -0500	[thread overview]
Message-ID: <20210830153022.3yywyrpynoiss6nr@redhat.com> (raw)
In-Reply-To: <20210829173210.39562-4-thuth@redhat.com>

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



  reply	other threads:[~2021-08-30 15:32 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210830153022.3yywyrpynoiss6nr@redhat.com \
    --to=eblake@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).