On 10/8/20 2:51 AM, Markus Armbruster wrote:
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 0if __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.
I assume they don't care about setting that return code manually. By default it's going to be 0 on normal exit, and non-zero if it raises or aborts.
We catch the error to pretty-print it to console instead, and want to avoid the stack trace so we return a status code instead.
I assume that's the only difference, really. --js
