All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: marcandre.lureau@redhat.com, alex.bennee@linaro.org,
	qemu-devel@nongnu.org, stefanha@redhat.com
Subject: Re: [PATCH 8/8] configure: automatically parse command line for meson -D options
Date: Wed, 13 Jan 2021 10:31:43 +0000	[thread overview]
Message-ID: <20210113103143.GA1568240@redhat.com> (raw)
In-Reply-To: <20210107140039.467969-9-pbonzini@redhat.com>

On Thu, Jan 07, 2021 at 03:00:39PM +0100, Paolo Bonzini wrote:
> Right now meson_options.txt lists almost 60 options.  Each option
> needs code in configure to parse it and pass the option down to Meson as
> a -D command-line argument; in addition the default must be duplicated
> between configure and meson_options.txt.
> 
> This series tries to remove the code duplication by passing unknown
> --enable and --disable options to a Perl program, and using Meson's
> introspection support to match those to meson_options.txt.
> About 80% of the options can be handled completely by the new
> mechanism.  Five meson options are not of the --enable/--disable
> kind.  Six more need to be parsed in configure for various reasons
> documented in the patch, but they still have their help automatically
> generated.
> 
> The advantages are simple to explain, and are basically what you
> can expect from an introspection-based system:
> 
> - there is obviously much less code in configure.  About 1000 lines
>   of the script deal with command line parsing, and the patch removes
>   a quarter of them.
> 
> - the script is higher quality than the repetitive code in configure.
>   Help is generally more complete and useful, for example it consistently
>   lists defaults as well as the possible choices if they are not just
>   enabled/disabled/auto.  Parsing is more consistent too, for example
>   --enable-slirp and --enable-blobs were not supported.
> 
> - new Meson options do not need any change to the configure script.
>   This increases the attractiveness of converting options from
>   hand-crafted parsing and config-host.mak to Meson.
> 
> The disadvantages are:
> 
> - a few options change name: --enable-tcmalloc and --enable-jemalloc
>   become --enable-malloc={tcmalloc,jemalloc}; --disable-blobs becomes
>   --disable-install-blobs.
> 
> - because we need to run the script to generate the full help, we
>   cannot rely on the user supplying the path to a Python interpreter
>   with --python.  For this reason, the script is written in Perl.
>   Perl 5 is universally available as "/usr/bin/env perl", while
>   (even ignoring the Python 2/3 difference) some systems do not
>   have a "python" or "python3" binary on the path.

Can't we just use  "/usr/bin/env python3", and if that doesn't
exist in $PATH, simply show truncated --help output, with a
message requesting that they pass --python to see full help.

> 
> - because we parse command-line options before meson is available,
>   the introspection output is stored in the source tree.  This is
>   the reason for the unattractive diffstat; the number of JSON lines
>   added is higher than the number of configure lines removed.
>   Of course the latter are code that must be maintained manually and
>   the former is not.
> 
> Note that the output of "meson introspect --buildoptions" is massaged
> slightly, in order to make it more stable and avoid horrible conflicts
> on every modification to meson_options.txt.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  Makefile                                |  11 +
>  configure                               | 371 ++----------
>  docs/devel/build-system.rst             |  49 +-
>  meson-buildoptions.json                 | 717 ++++++++++++++++++++++++

I'm not a fan of seeing this file introduced as it has significant
overlap with meson_options.txt.    I feel like the latter has enough
information present to do an acceptable job for help output. After
all that's sufficient if we were using meson directly.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



  reply	other threads:[~2021-01-13 10:32 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-07 14:00 [RFC PATCH v2 0/8] Automatically convert configure options to meson build options Paolo Bonzini
2021-01-07 14:00 ` [PATCH 1/8] build-system: clean up TCG/TCI configury Paolo Bonzini
2021-01-07 15:01   ` Peter Maydell
2021-01-07 15:50     ` Paolo Bonzini
2021-01-07 16:06       ` Daniel P. Berrangé
2021-01-13 13:09         ` Philippe Mathieu-Daudé
2021-01-13 13:42           ` Helge Deller
2021-01-13 13:57             ` Daniel P. Berrangé
2021-01-13 14:23               ` Peter Maydell
2021-01-13 20:39                 ` Helge Deller
2021-01-13 14:23               ` Helge Deller
2021-01-13 14:34                 ` Paolo Bonzini
2021-01-13 15:37                   ` Helge Deller
2021-01-14  9:51                 ` John Paul Adrian Glaubitz
2021-01-13 14:02             ` John David Anglin
2021-01-13 15:00           ` John Paul Adrian Glaubitz
2021-01-07 14:00 ` [PATCH 2/8] cocoa: do not enable coreaudio automatically Paolo Bonzini
2021-01-07 14:00 ` [PATCH 3/8] gtk: remove CONFIG_GTK_GL Paolo Bonzini
2021-01-07 16:05   ` Gerd Hoffmann
2021-01-07 14:00 ` [PATCH 4/8] configure: move X11 detection to Meson Paolo Bonzini
2021-01-07 14:00 ` [PATCH 5/8] configure: move GTK+ " Paolo Bonzini
2021-01-07 14:00 ` [PATCH 6/8] configure: move Cocoa incompatibility checks " Paolo Bonzini
2021-01-07 14:00 ` [PATCH 7/8] configure: quote command line arguments in config.status Paolo Bonzini
2021-01-13 15:44   ` Eric Blake
2021-01-07 14:00 ` [PATCH 8/8] configure: automatically parse command line for meson -D options Paolo Bonzini
2021-01-13 10:31   ` Daniel P. Berrangé [this message]
2021-01-13 12:26     ` Paolo Bonzini
2021-01-13 14:04     ` Paolo Bonzini
2021-01-22  8:00       ` 罗勇刚(Yonggang Luo)
2021-01-22 20:43         ` Paolo Bonzini
2021-01-23  3:30           ` 罗勇刚(Yonggang Luo)
2021-01-23 18:00             ` 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=20210113103143.GA1568240@redhat.com \
    --to=berrange@redhat.com \
    --cc=alex.bennee@linaro.org \
    --cc=marcandre.lureau@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@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 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.