qemu-devel
[Top][All Lists]
Advanced

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

Re: [PATCH v5 03/36] qapi-gen: Separate arg-parsing from generation


From: John Snow
Subject: Re: [PATCH v5 03/36] qapi-gen: Separate arg-parsing from generation
Date: Thu, 8 Oct 2020 12:37:22 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0

On 10/8/20 2:51 AM, Markus Armbruster wrote:
John Snow <jsnow@redhat.com> writes:

On 10/7/20 4:07 AM, Markus Armbruster wrote:
John Snow <jsnow@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 <jsnow@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@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.


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




reply via email to

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