qemu-devel
[Top][All Lists]
Advanced

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

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


From: Cleber Rosa
Subject: Re: [Qemu-devel] [PATCH 1/3] scripts: introduce buildconf.py
Date: Fri, 21 Jul 2017 10:07:24 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1


On 07/21/2017 10:00 AM, Eric Blake wrote:
> 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/
> 
> 

Oops, thanks for spotting it.

>>
>> 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).
> 

Yuck! Yes, it was a clear mistake.

>>
>> 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.
> 

Sure, I 'll fix that.

> Overall, the idea is cool.
> 

Thanks.  I mean, there's one big issue about the general approach, which
is to require a build root.  Let's see how this specific problem gets a
solution.

-- 
Cleber Rosa
[ Sr Software Engineer - Virtualization Team - Red Hat ]
[ Avocado Test Framework - avocado-framework.github.io ]
[  7ABB 96EB 8B46 B94D 5E0F  E9BB 657E 8D33 A5F2 09F3  ]

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

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