mbox series

[RFC,0/3] Automatically convert configure options to meson build options

Message ID 20200913100534.22084-1-pbonzini@redhat.com
Headers show
Series Automatically convert configure options to meson build options | expand

Message

Paolo Bonzini Sept. 13, 2020, 10:05 a.m. UTC
Right now meson_options.txt lists less than a dozen options, but about
40 more could come as configure tests are converted and moved to
meson.build.  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 Python program, and using
Meson's introspection support to match those to meson_options.txt

The disadvantages are:

- because we parse command-line options before meson is available,
the introspection output is stored in the source tree.  The output
is slightly modified using the "jq" tool in order to ensure that it's
stable and that modifications to meson_buildoptions.txt do not cause
horrible conflicts.  This is the main reason for the unattractive
diffstat (the number of JSON lines added is higher than the number
of configure lines removed, though of course the latter are code
that must be maintained manually and the former is not).

- we now need Python to generate the full help, so if Python is
missing we can only print a partial message and direct the user
to specify the interpreter with --python.  It would be possible to fix
this by rewriting the script in Perl (at least on Fedora, JSON::PP is
always installed if Perl is, because it's a dependency for CPAN; I'd
have to check Ubuntu and the BSDs), or if we want to write it as a
Bourne shell script, to further massage the introspection output into
for example TAB-separated values.

Opinions are welcome on whether this is worthwhile and how to solve
the above doubts.

Paolo

Paolo Bonzini (3):
  configure: quote command line arguments in config.status
  configure: early test for Python
  configure: automatically parse command line for meson -D options

 Makefile                                |   6 ++
 configure                               | 107 ++++++++-----------
 docs/devel/build-system.rst             |  35 +------
 meson-buildoptions.json                 | 130 ++++++++++++++++++++++++
 scripts/configure-parse-buildoptions.py |  94 +++++++++++++++++
 5 files changed, 280 insertions(+), 92 deletions(-)
 create mode 100644 meson-buildoptions.json
 create mode 100644 scripts/configure-parse-buildoptions.py

Comments

Paolo Bonzini Sept. 13, 2020, 1:57 p.m. UTC | #1
On 13/09/20 12:15, 罗勇刚(Yonggang Luo) wrote:
> 
> I am confusing about this? we were converting configure to meson,
> and python is a force dependencies, why we need rewrite the script in
> Perl? If we wanna build qemu, the first thing we need to install is
> python+meson, so there is no need convert to Perl or bash script. And
> perl/bash will incur msys2/mingw user, the ideal way is we only 
> depends on python+meson to building qemu

The main issue is that the Python interpreter is any of python, python3,
python3.5.  Furthermore Meson's shebang python is not necessarily the
first "python3" in the path.

Perl is pretty much always "/usr/bin/env perl".  Note that Perl is
already needed to run "make check".  When/if it will be possible to only
build QEMU with python+meson there will be no configure script anymore,
so Perl is a reasonable choice for parsing the configure command line
options.  It wasn't my first choice, in fact this series includes a
Python implementation, but it does have some advantages.

Paolo
Daniel P. Berrangé Sept. 14, 2020, 9:12 a.m. UTC | #2
On Sun, Sep 13, 2020 at 12:05:31PM +0200, Paolo Bonzini wrote:
> Right now meson_options.txt lists less than a dozen options, but about
> 40 more could come as configure tests are converted and moved to
> meson.build.  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 Python program, and using
> Meson's introspection support to match those to meson_options.txt
> 
> The disadvantages are:
> 
> - because we parse command-line options before meson is available,
> the introspection output is stored in the source tree.  The output
> is slightly modified using the "jq" tool in order to ensure that it's
> stable and that modifications to meson_buildoptions.txt do not cause
> horrible conflicts.  This is the main reason for the unattractive
> diffstat (the number of JSON lines added is higher than the number
> of configure lines removed, though of course the latter are code
> that must be maintained manually and the former is not).
> 
> - we now need Python to generate the full help, so if Python is
> missing we can only print a partial message and direct the user
> to specify the interpreter with --python.  It would be possible to fix
> this by rewriting the script in Perl (at least on Fedora, JSON::PP is
> always installed if Perl is, because it's a dependency for CPAN; I'd
> have to check Ubuntu and the BSDs), or if we want to write it as a
> Bourne shell script, to further massage the introspection output into
> for example TAB-separated values.

IMHO we should stay as far away from Perl as possible, and I say this as
someone who enjoys writing Perl scripts !  Perl is a significant barrier
to entry for potential contributors, because it just doesn't have the
wide knowledge base that it did in the past. You can expect most people
to have some familiarity with Python, or be able to pick it up fairly
easily in a way that just isn't possible in Perl.

As for shell, we have a never ending stream of bugs due to the wide
range of different shell impls which make portable coding very hard.

In general I think our overall goal should be to focus on getting down to
use of a single scripting language in QEMU, and that language clearly has
to be python3. We shouldn't introduce any new usage of Perl or Shell
in QEMU IMHO. Even if we think the usage will only be short term temporary
workaround, short term hacks in QEMU have a habit of living for years longer
than we expect.

> Opinions are welcome on whether this is worthwhile and how to solve
> the above doubts.

IIUC, the motivation of this series is just to remove some duplication
of defaults / args, nothing more than that ?

If correct, then I'd say it probably isn't worth the hassle. I'd say we
should focus effort on just converting more of configure to meson, as
quickly as practical, and not try to add much more magic that's only
relevant for the transition time.


Regards,
Daniel
罗勇刚(Yonggang Luo) Sept. 14, 2020, 10 a.m. UTC | #3
On Mon, Sep 14, 2020 at 5:58 PM Stefan Hajnoczi <stefanha@gmail.com> wrote:
>
> On Sun, Sep 13, 2020 at 12:05:31PM +0200, Paolo Bonzini wrote:
> > - because we parse command-line options before meson is available,
> > the introspection output is stored in the source tree.  The output
> > is slightly modified using the "jq" tool in order to ensure that it's
> > stable and that modifications to meson_buildoptions.txt do not cause
> > horrible conflicts.  This is the main reason for the unattractive
> > diffstat (the number of JSON lines added is higher than the number
> > of configure lines removed, though of course the latter are code
> > that must be maintained manually and the former is not).
>
> Does this add a build dependency on jq? Is it possible to use a Python
> script or even a command-line one-liner instead?
What's jq stands for, is that a perl script?



--
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo
<div dir="ltr"><br><br>On Mon, Sep 14, 2020 at 5:58 PM Stefan Hajnoczi &lt;<a href="mailto:stefanha@gmail.com">stefanha@gmail.com</a>&gt; wrote:<br>&gt;<br>&gt; On Sun, Sep 13, 2020 at 12:05:31PM +0200, Paolo Bonzini wrote:<br>&gt; &gt; - because we parse command-line options before meson is available,<br>&gt; &gt; the introspection output is stored in the source tree.  The output<br>&gt; &gt; is slightly modified using the &quot;jq&quot; tool in order to ensure that it&#39;s<br>&gt; &gt; stable and that modifications to meson_buildoptions.txt do not cause<br>&gt; &gt; horrible conflicts.  This is the main reason for the unattractive<br>&gt; &gt; diffstat (the number of JSON lines added is higher than the number<br>&gt; &gt; of configure lines removed, though of course the latter are code<br>&gt; &gt; that must be maintained manually and the former is not).<br>&gt;<br>&gt; Does this add a build dependency on jq? Is it possible to use a Python<br>&gt; script or even a command-line one-liner instead?<div>What&#39;s jq stands for, is that a perl script?<br><br><br><br>--<br>         此致<br>礼<br>罗勇刚<br>Yours<br>    sincerely,<br>Yonggang Luo</div></div>
Paolo Bonzini Sept. 14, 2020, 10:27 a.m. UTC | #4
On 14/09/20 11:57, Stefan Hajnoczi wrote:
> On Sun, Sep 13, 2020 at 12:05:31PM +0200, Paolo Bonzini wrote:
>> - because we parse command-line options before meson is available,
>> the introspection output is stored in the source tree.  The output
>> is slightly modified using the "jq" tool in order to ensure that it's
>> stable and that modifications to meson_buildoptions.txt do not cause
>> horrible conflicts.  This is the main reason for the unattractive
>> diffstat (the number of JSON lines added is higher than the number
>> of configure lines removed, though of course the latter are code
>> that must be maintained manually and the former is not).
> 
> Does this add a build dependency on jq? Is it possible to use a Python
> script or even a command-line one-liner instead?

No, only developers need jq and only if they add configure options.
Using Python would certainly be possible, though probably it wouldn't be
a one-liner as for jq.

Paolo
Paolo Bonzini Sept. 14, 2020, 10:49 a.m. UTC | #5
On 14/09/20 11:12, Daniel P. Berrangé wrote:
> IMHO we should stay as far away from Perl as possible, and I say this as
> someone who enjoys writing Perl scripts !  Perl is a significant barrier
> to entry for potential contributors, because it just doesn't have the
> wide knowledge base that it did in the past. You can expect most people
> to have some familiarity with Python, or be able to pick it up fairly
> easily in a way that just isn't possible in Perl.
> 
> As for shell, we have a never ending stream of bugs due to the wide
> range of different shell impls which make portable coding very hard.
> 
> In general I think our overall goal should be to focus on getting down to
> use of a single scripting language in QEMU, and that language clearly has
> to be python3. We shouldn't introduce any new usage of Perl or Shell
> in QEMU IMHO. Even if we think the usage will only be short term temporary
> workaround, short term hacks in QEMU have a habit of living for years longer
> than we expect.

I totally agree.  This would not be a short term workaround, I expect it
to stay for years; I don't expect the transition of the configure script
to Meson to be very short.

This can be both a pro and a con.  On one hand it means this is "more
magic" that is here to stay.  On the other hand it makes transition
patches smaller and easier to review.

I agree that in principle all scripting should be Python.  On the other
hand Perl is quite suitable for "configure tasks that are too
complicated for shell" and are not going to be rewritten in Meson down
the road.  And I say that as someone who has written exactly 3 serious
Perl programs in his life, two of which are part of QEMU. :)

In this case shell scripting could also be a plausible alternative; I
didn't try only because I first tried the obvious choice of Python.

Or we can just decide that an incomplete help below a "please use
--python" error is good enough.

>> Opinions are welcome on whether this is worthwhile and how to solve
>> the above doubts.
> 
> IIUC, the motivation of this series is just to remove some duplication
> of defaults / args, nothing more than that ?

Yes, though it's quite some duplication: right now an option needs

- a default in configure

- command line parsing in configure

- a Meson -D argument in configure

- an help message in configure

- a declaration (including default and help) in meson_options.txt

so this patch reduces an option from 8-9 lines to 2.

There are a couple secondary goals too.  First, making sure all the
policy is in meson_options.txt and not configure; this avoids surprises
when/if we can start invoking meson directly.  Second, removing
variables completely from the configure script gives a better idea of
interdependencies between the configure tests.

Thanks,

Paolo

> I'd say we should focus effort on just converting more of configure
> to meson, as quickly as practical, and not try to add much more magic
> that's only relevant for the transition time.
Eric Blake Sept. 14, 2020, 7:17 p.m. UTC | #6
On 9/13/20 5:05 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

> ---

>   configure | 6 +++++-

>   1 file changed, 5 insertions(+), 1 deletion(-)

> 

> diff --git a/configure b/configure

> index 53723ace57..beae010e39 100755

> --- a/configure

> +++ b/configure

> @@ -89,6 +89,10 @@ printf " '%s'" "$0" "$@" >> config.log

>   echo >> config.log

>   echo "#" >> config.log

>   

> +quote_sh() {

> +    printf "'%s'" "$(echo "$1" | sed "s,','\\',")"


This is unsafe if $1 starts with - or contains \.  Better is using 
printf.  It also eats any trailing newlines in $1, although that may be 
less of a concern.

> +}

> +

>   print_error() {

>       (echo

>       echo "ERROR: $1"

> @@ -8061,7 +8065,7 @@ preserve_env WINDRES

>   

>   printf "exec" >>config.status

>   for i in "$0" "$@"; do

> -  test "$i" = --skip-meson || printf " '%s'" "$i" >>config.status

> +  test "$i" = --skip-meson || printf " %s" "$(quote_sh $i)" >>config.status


And this unquoted use of $i is wrong.

>   done

>   echo ' "$@"' >>config.status

>   chmod +x config.status

> 


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org
Stefan Hajnoczi Sept. 16, 2020, 11:24 a.m. UTC | #7
On Mon, Sep 14, 2020 at 06:00:11PM +0800, 罗勇刚(Yonggang Luo) wrote:
> On Mon, Sep 14, 2020 at 5:58 PM Stefan Hajnoczi <stefanha@gmail.com> wrote:

> >

> > On Sun, Sep 13, 2020 at 12:05:31PM +0200, Paolo Bonzini wrote:

> > > - because we parse command-line options before meson is available,

> > > the introspection output is stored in the source tree.  The output

> > > is slightly modified using the "jq" tool in order to ensure that it's

> > > stable and that modifications to meson_buildoptions.txt do not cause

> > > horrible conflicts.  This is the main reason for the unattractive

> > > diffstat (the number of JSON lines added is higher than the number

> > > of configure lines removed, though of course the latter are code

> > > that must be maintained manually and the former is not).

> >

> > Does this add a build dependency on jq? Is it possible to use a Python

> > script or even a command-line one-liner instead?

> What's jq stands for, is that a perl script?


https://stedolan.github.io/jq/
Stefan Hajnoczi Sept. 16, 2020, 11:25 a.m. UTC | #8
On Mon, Sep 14, 2020 at 12:27:59PM +0200, Paolo Bonzini wrote:
> On 14/09/20 11:57, Stefan Hajnoczi wrote:
> > On Sun, Sep 13, 2020 at 12:05:31PM +0200, Paolo Bonzini wrote:
> >> - because we parse command-line options before meson is available,
> >> the introspection output is stored in the source tree.  The output
> >> is slightly modified using the "jq" tool in order to ensure that it's
> >> stable and that modifications to meson_buildoptions.txt do not cause
> >> horrible conflicts.  This is the main reason for the unattractive
> >> diffstat (the number of JSON lines added is higher than the number
> >> of configure lines removed, though of course the latter are code
> >> that must be maintained manually and the former is not).
> > 
> > Does this add a build dependency on jq? Is it possible to use a Python
> > script or even a command-line one-liner instead?
> 
> No, only developers need jq and only if they add configure options.
> Using Python would certainly be possible, though probably it wouldn't be
> a one-liner as for jq.

I see. Avoiding the dependency would be nice but only if the alternative
is reasonable.

Stefan
Paolo Bonzini Sept. 16, 2020, 1:15 p.m. UTC | #9
On 16/09/20 13:25, Stefan Hajnoczi wrote:
>> No, only developers need jq and only if they add configure options.

>> Using Python would certainly be possible, though probably it wouldn't be

>> a one-liner as for jq.

> I see. Avoiding the dependency would be nice but only if the alternative

> is reasonable.


I guess this counts as reasonable:
 
python3 -c 'import json, sys; print(
  json.dumps(sorted([x for x in json.loads(sys.stdin.read()) 
                    if x["section"] == "user"], key=lambda x: x["name"]),
             indent=2))'

Paolo