John Snow <js...@redhat.com> writes:
> On 10/7/20 4:07 AM, Markus Armbruster wrote:
>> John Snow <js...@redhat.com> writes:
>>
>>> This is a minor re-work of the entrypoint script. It isolates a
>>> generate() method from the actual command-line mechanism.
>>>
>>> Signed-off-by: John Snow <js...@redhat.com>
>>> Reviewed-by: Eduardo Habkost <ehabk...@redhat.com>
>>> Reviewed-by: Cleber Rosa <cr...@redhat.com>
>>> Tested-by: Cleber Rosa <cr...@redhat.com>
>>> ---
>>> scripts/qapi-gen.py | 85 +++++++++++++++++++++++++++++++++------------
>>> 1 file changed, 62 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/scripts/qapi-gen.py b/scripts/qapi-gen.py
>>> index 541e8c1f55d..117b396a595 100644
>>> --- a/scripts/qapi-gen.py
>>> +++ b/scripts/qapi-gen.py
>>> @@ -1,30 +1,77 @@
>>> #!/usr/bin/env python3
>>> -# QAPI generator
>>> -#
>>> +
>>> # This work is licensed under the terms of the GNU GPL, version 2 or
>>> later.
>>> # See the COPYING file in the top-level directory.
>>> +"""
>>> +QAPI Generator
>>> +
>>> +This script is the main entry point for generating C code from the QAPI
>>> schema.
>>> +"""
>>> import argparse
>>> import re
>>> import sys
>>> from qapi.commands import gen_commands
>>> +from qapi.error import QAPIError
>>> from qapi.events import gen_events
>>> from qapi.introspect import gen_introspect
>>> -from qapi.schema import QAPIError, QAPISchema
>>> +from qapi.schema import QAPISchema
>>> from qapi.types import gen_types
>>> from qapi.visit import gen_visit
>>>
>>> -def main(argv):
>>> +DEFAULT_OUTPUT_DIR = ''
>>> +DEFAULT_PREFIX = ''
>>> +
>>> +
>>> +def generate(schema_file: str,
>>> + output_dir: str,
>>> + prefix: str,
>>> + unmask: bool = False,
>>> + builtins: bool = False) -> None:
>>> + """
>>> + generate uses a given schema to produce C code in the target directory.
>>> +
>>> + :param schema_file: The primary QAPI schema file.
>>> + :param output_dir: The output directory to store generated code.
>>> + :param prefix: Optional C-code prefix for symbol names.
>>> + :param unmask: Expose non-ABI names through introspection?
>>> + :param builtins: Generate code for built-in types?
>>> +
>>> + :raise QAPIError: On failures.
>>> + """
>>> + match = re.match(r'([A-Za-z_.-][A-Za-z0-9_.-]*)?', prefix)
>>> + if match.end() != len(prefix):
>>> + msg = "funny character '{:s}' in prefix '{:s}'".format(
>>> + prefix[match.end()], prefix)
>>> + raise QAPIError('', None, msg)
>>> +
>>> + schema = QAPISchema(schema_file)
>>> + gen_types(schema, output_dir, prefix, builtins)
>>> + gen_visit(schema, output_dir, prefix, builtins)
>>> + gen_commands(schema, output_dir, prefix)
>>> + gen_events(schema, output_dir, prefix)
>>> + gen_introspect(schema, output_dir, prefix, unmask)
>>> +
>>> +
>>> +def main() -> int:
>>> + """
>>> + gapi-gen shell script entrypoint.
>> What's a "shell script entrypoint"?
>> Python docs talk about "when [...] run as a script":
>> https://docs.python.org/3/library/__main__.html
>> Similar:
>> https://docs.python.org/3/tutorial/modules.html#executing-modules-as-scripts
>>
>
> "entrypoint" is Python garble for a function that can be registered as
> a callable from the command line.
>
> So in a theoretical setup.py, you'd do something like:
>
> 'entry_points': {
> 'console_scripts': [
> 'qapi-gen = qapi.main:main',
> ]
> }
>
> so when I say "shell script entrypoint", I am referring to a shell
> script (I mean: it has a shebang and can be executed by an interactive
> shell process) that calls the entrypoint.
It can be executed by any process. See execve(2):
pathname must be either a binary executable, or a script starting with
a line of the form:
#!interpreter [optional-arg]
For details of the latter case, see "Interpreter scripts" below.
"Entry point" makes sense in Python context, "script entry point" also
makes sense (since every Python program is a script, script is
redundant, but not wrong). "Shell script entry point" is misleading.
> Once (if) QAPI migrates to ./python/qemu/qapi, it will be possible to
> just generate that script.
>
> (i.e. doing `pip install qemu.qapi` will install a 'qapi-gen' CLI
> script for you. this is how packages like sphinx create the
> 'sphinx-build' script, etc.)
>
>>> + Expects arguments via sys.argv, see --help for details.
>>> +
>>> + :return: int, 0 on success, 1 on failure.
>>> + """
>>> parser = argparse.ArgumentParser(
>>> description='Generate code from a QAPI schema')
>>> parser.add_argument('-b', '--builtins', action='store_true',
>>> help="generate code for built-in types")
>>> - parser.add_argument('-o', '--output-dir', action='store', default='',
>>> + parser.add_argument('-o', '--output-dir', action='store',
>>> + default=DEFAULT_OUTPUT_DIR,
>>> help="write output to directory OUTPUT_DIR")
>>> - parser.add_argument('-p', '--prefix', action='store', default='',
>>> + parser.add_argument('-p', '--prefix', action='store',
>>> + default=DEFAULT_PREFIX,
>>> help="prefix for symbols")
>>> parser.add_argument('-u', '--unmask-non-abi-names',
>>> action='store_true',
>>> dest='unmask',
>>> @@ -32,25 +79,17 @@ def main(argv):
>>> parser.add_argument('schema', action='store')
>>> args = parser.parse_args()
>>> - match = re.match(r'([A-Za-z_.-][A-Za-z0-9_.-]*)?',
>>> args.prefix)
>>> - if match.end() != len(args.prefix):
>>> - print("%s: 'funny character '%s' in argument of --prefix"
>>> - % (sys.argv[0], args.prefix[match.end()]),
>>> - file=sys.stderr)
>>> - sys.exit(1)
>>> -
>>> try:
>>> - schema = QAPISchema(args.schema)
>>> + generate(args.schema,
>>> + output_dir=args.output_dir,
>>> + prefix=args.prefix,
>>> + unmask=args.unmask,
>>> + builtins=args.builtins)
>>> except QAPIError as err:
>>> - print(err, file=sys.stderr)
>>> - exit(1)
>>> -
>>> - gen_types(schema, args.output_dir, args.prefix, args.builtins)
>>> - gen_visit(schema, args.output_dir, args.prefix, args.builtins)
>>> - gen_commands(schema, args.output_dir, args.prefix)
>>> - gen_events(schema, args.output_dir, args.prefix)
>>> - gen_introspect(schema, args.output_dir, args.prefix, args.unmask)
>>> + print(f"{sys.argv[0]}: {str(err)}", file=sys.stderr)
>>> + return 1
>>> + return 0
>>>
>>> if __name__ == '__main__':
>>> - main(sys.argv)
>>> + sys.exit(main())
>> What does sys.exit() really buy us here? I'm asking because both
>> spots
>> in the Python docs I referenced above do without.
>>
>
> It just pushes the sys.exit out of the main function so it can be
> invoked by other machinery. (And takes the return code from main and
> turns it into the return code for the process.)
>
> I don't think it winds up mattering for simple "console_script" entry
> points, but you don't want the called function to exit and deny the
> caller the chance to do their own tidying post-call.
>
> You've already offered a "YAGNI", but it's just the convention I tend
> to stick to for how to structure entry points.
I'm not questioning the conventional if __name__ == '__main__' menuett.
I wonder why *we* need sys.exit() where the examples in the Python docs
don't.
