qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH 1/3] scripts: introduce buildconf.py


From: Eric Blake
Subject: Re: [Qemu-block] [PATCH 1/3] scripts: introduce buildconf.py
Date: Fri, 21 Jul 2017 09:00:22 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1

On 07/20/2017 10:47 PM, Cleber Rosa wrote:
> scripts/buildconf.py is a command line utility (but also can be used
> as a Python module) that introspects the build configuration.
> 
> It uses the generated host level config-host.mak to obtain the general
> build configuration, and optionally, also target specific
> config-target.mak and config-devices.mak.  It does not attempt to
> implement a Makefile parser, but instead relies on "make" itself to
> parse those files and output the queried variable.
> 
> It requires a build tree that has been both configured and built.  By
> default, for convenience, it will selected a default target, which can
> be displayed and overriden.  A few examples follow.

s/overriden/overridden/


> 
> And for checking a target different than the default one:
> 
>  $ ./scripts/buildconf.py -c CONFIG_PARALLEL arm-softmmu; echo $?
>  255

Gross.  exit status greater than 128 typically mean death due to signal,
meanwhile, 255 is special-cased by find to kill processing immediately;
it is very rare that someone intentionally returns a status of 255, and
more often, it is evidence that someone mistakenly used exit(-1).

> 
> Signed-off-by: Cleber Rosa <address@hidden>
> ---
>  scripts/buildconf.py | 278 
> +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 278 insertions(+)
>  create mode 100755 scripts/buildconf.py
> 

I don't feel comfortable reviewing the script in depth, but I will request:


> +                else:
> +                    sys.exit(-1)
> +            else:
> +                conf = get_build_conf(config, self.target)
> +                if conf:
> +                    print(conf)
> +                    sys.exit(0)
> +                else:
> +                    sys.exit(-1)

Please use sys.exit(1), not -1.

Overall, the idea is cool.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]