| Message ID | 20200915224027.2529813-5-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qapi: static typing conversion, pt1 | expand |
John Snow <jsnow@redhat.com> writes: > As part of delinting and adding type hints to the QAPI generator, it's > helpful for the entrypoint to be part of the module, only leaving a very > tiny entrypoint shim outside of the module. > > As a result, all of the include statements are reworked to be module-aware, > as explicit relative imports. Should this be split into two commits, one for each of the paragraphs above? PEP 8 recommends absolute imports, with one exception: However, explicit relative imports are an acceptable alternative to absolute imports, especially when dealing with complex package layouts where using absolute imports would be unnecessarily verbose: from . import sibling from .sibling import example Standard library code should avoid complex package layouts and always use absolute imports. Do you think it covers your use of relative imports? > This is done primarily for the benefit of python tooling (pylint, mypy, > flake8, et al) which otherwise has trouble consistently resolving > "qapi.x" to mean "a sibling file in this folder." Can you give me an example of some tool having troube? > Signed-off-by: John Snow <jsnow@redhat.com> > --- > scripts/qapi-gen.py | 94 +++----------------------------------- > scripts/qapi/commands.py | 4 +- > scripts/qapi/doc.py | 2 +- > scripts/qapi/events.py | 8 ++-- > scripts/qapi/expr.py | 4 +- > scripts/qapi/gen.py | 4 +- > scripts/qapi/introspect.py | 8 ++-- > scripts/qapi/parser.py | 4 +- > scripts/qapi/schema.py | 8 ++-- > scripts/qapi/script.py | 91 ++++++++++++++++++++++++++++++++++++ > scripts/qapi/types.py | 6 +-- > scripts/qapi/visit.py | 6 +-- > 12 files changed, 124 insertions(+), 115 deletions(-) > create mode 100644 scripts/qapi/script.py Naming is hard... main.py? > > diff --git a/scripts/qapi-gen.py b/scripts/qapi-gen.py > index 59becba3e1..e649f8dd44 100644 > --- a/scripts/qapi-gen.py > +++ b/scripts/qapi-gen.py > @@ -1,97 +1,15 @@ > #!/usr/bin/env python3 > - > -# This work is licensed under the terms of the GNU GPL, version 2 or later. > -# See the COPYING file in the top-level directory. > - Keep the license blurb. [...]
On 9/16/20 7:54 AM, Markus Armbruster wrote: > John Snow <jsnow@redhat.com> writes: > >> As part of delinting and adding type hints to the QAPI generator, it's >> helpful for the entrypoint to be part of the module, only leaving a very >> tiny entrypoint shim outside of the module. >> >> As a result, all of the include statements are reworked to be module-aware, >> as explicit relative imports. > > Should this be split into two commits, one for each of the paragraphs > above? > Hmm ... I hadn't considered it was possible, but actually ... I guess I can split those out, yeah. > PEP 8 recommends absolute imports, with one exception: > > However, explicit relative imports are an acceptable alternative to > absolute imports, especially when dealing with complex package > layouts where using absolute imports would be unnecessarily verbose: > > from . import sibling > from .sibling import example > > Standard library code should avoid complex package layouts and > always use absolute imports. > > Do you think it covers your use of relative imports? > Admittedly I am going by trial and error; my experience is that the relative imports behave the nicest. There is a historical fear of explicit relative imports because they are "new" and years of Python2 compatibility rendered many afraid of them. It is advice safely ignored in my opinion. Using absolute imports (e.g. from qapi.sibling import foo) gets messy in virtual environments when you have *installed* the module in question: it becomes ambiguous as to *which* qapi you meant: the one in this folder, or the one installed to the environment? Python, mypy, pylint, flake8 etc disagree sometimes, or can get confused; thinking there are two copies of each module. Just my experience that relative imports seem to give me the least trouble. >> This is done primarily for the benefit of python tooling (pylint, mypy, >> flake8, et al) which otherwise has trouble consistently resolving >> "qapi.x" to mean "a sibling file in this folder." > > Can you give me an example of some tool having troube? > I'd have to code up some examples. I have some hobby code unrelated to QEMU where I struggled to get flake8, mypy, and pylint all cooperating with an import regime until I gave up and used explicit relative imports. >> Signed-off-by: John Snow <jsnow@redhat.com> >> --- >> scripts/qapi-gen.py | 94 +++----------------------------------- >> scripts/qapi/commands.py | 4 +- >> scripts/qapi/doc.py | 2 +- >> scripts/qapi/events.py | 8 ++-- >> scripts/qapi/expr.py | 4 +- >> scripts/qapi/gen.py | 4 +- >> scripts/qapi/introspect.py | 8 ++-- >> scripts/qapi/parser.py | 4 +- >> scripts/qapi/schema.py | 8 ++-- >> scripts/qapi/script.py | 91 ++++++++++++++++++++++++++++++++++++ >> scripts/qapi/types.py | 6 +-- >> scripts/qapi/visit.py | 6 +-- >> 12 files changed, 124 insertions(+), 115 deletions(-) >> create mode 100644 scripts/qapi/script.py > > Naming is hard... main.py? > I was thinking of changing this myself, so this convinced me. >> >> diff --git a/scripts/qapi-gen.py b/scripts/qapi-gen.py >> index 59becba3e1..e649f8dd44 100644 >> --- a/scripts/qapi-gen.py >> +++ b/scripts/qapi-gen.py >> @@ -1,97 +1,15 @@ >> #!/usr/bin/env python3 >> - >> -# This work is licensed under the terms of the GNU GPL, version 2 or later. >> -# See the COPYING file in the top-level directory. >> - > > Keep the license blurb. > This is a mistake. I tried to convince git to "move" the old file and then add a "new" file to preserve history, but of course that's not how git manages file histories, so it didn't work. TLDR: I didn't delete the license blurb, I just didn't "add" it again. I'll "fix" that. > [...] >
John Snow <jsnow@redhat.com> writes: > On 9/16/20 7:54 AM, Markus Armbruster wrote: >> John Snow <jsnow@redhat.com> writes: >> >>> As part of delinting and adding type hints to the QAPI generator, it's >>> helpful for the entrypoint to be part of the module, only leaving a very >>> tiny entrypoint shim outside of the module. >>> >>> As a result, all of the include statements are reworked to be module-aware, >>> as explicit relative imports. >> Should this be split into two commits, one for each of the >> paragraphs >> above? >> > > Hmm ... I hadn't considered it was possible, but actually ... I guess > I can split those out, yeah. > >> PEP 8 recommends absolute imports, with one exception: >> However, explicit relative imports are an acceptable >> alternative to >> absolute imports, especially when dealing with complex package >> layouts where using absolute imports would be unnecessarily verbose: >> from . import sibling >> from .sibling import example >> Standard library code should avoid complex package layouts and >> always use absolute imports. >> Do you think it covers your use of relative imports? >> > > Admittedly I am going by trial and error; my experience is that the > relative imports behave the nicest. > > There is a historical fear of explicit relative imports because they > are "new" and years of Python2 compatibility Having to struggle with that for years was bound to damage brains. > rendered many afraid of > them. It is advice safely ignored in my opinion. > > Using absolute imports (e.g. from qapi.sibling import foo) gets messy > in virtual environments when you have *installed* the module in > question: it becomes ambiguous as to *which* qapi you meant: the one > in this folder, or the one installed to the environment? > > Python, mypy, pylint, flake8 etc disagree sometimes, or can get > confused; thinking there are two copies of each module. > > Just my experience that relative imports seem to give me the least trouble. > >>> This is done primarily for the benefit of python tooling (pylint, mypy, >>> flake8, et al) which otherwise has trouble consistently resolving >>> "qapi.x" to mean "a sibling file in this folder." >> Can you give me an example of some tool having troube? >> > > I'd have to code up some examples. I have some hobby code unrelated to > QEMU where I struggled to get flake8, mypy, and pylint all cooperating > with an import regime until I gave up and used explicit relative imports. I'd make the switch when we actually do run into trouble. But I'm willing to take your advice and switch now. [...]
diff --git a/scripts/qapi-gen.py b/scripts/qapi-gen.py index 59becba3e1..e649f8dd44 100644 --- a/scripts/qapi-gen.py +++ b/scripts/qapi-gen.py @@ -1,97 +1,15 @@ #!/usr/bin/env python3 - -# 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 +QAPI code generation execution shim. -This script is the main entry point for generating C code from the QAPI schema. +This file exists primarily to facilitate the running of the QAPI code +generator without needing to install the python module to the current +execution environment. """ -import argparse -import re import sys -from qapi.commands import gen_commands -from qapi.doc import gen_doc -from qapi.error import QAPIError -from qapi.events import gen_events -from qapi.introspect import gen_introspect -from qapi.schema import QAPISchema -from qapi.types import gen_types -from qapi.visit import gen_visit - - -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 and 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) - gen_doc(schema, output_dir, prefix) - - -def main() -> int: - """ - gapi-gen shell script entrypoint. - 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=DEFAULT_OUTPUT_DIR, - help="write output to directory OUTPUT_DIR") - 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', - help="expose non-ABI names in introspection") - parser.add_argument('schema', action='store') - args = parser.parse_args() - - try: - generate(args.schema, - output_dir=args.output_dir, - prefix=args.prefix, - unmask=args.unmask, - builtins=args.builtins) - except QAPIError as err: - print(f"{sys.argv[0]}: {str(err)}", file=sys.stderr) - return 1 - return 0 - +from qapi import script if __name__ == '__main__': - sys.exit(main()) + sys.exit(script.main()) diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py index 3cf9e1110b..ce5926146a 100644 --- a/scripts/qapi/commands.py +++ b/scripts/qapi/commands.py @@ -13,8 +13,8 @@ See the COPYING file in the top-level directory. """ -from qapi.common import * -from qapi.gen import QAPIGenCCode, QAPISchemaModularCVisitor, ifcontext +from .common import * +from .gen import QAPIGenCCode, QAPISchemaModularCVisitor, ifcontext def gen_command_decl(name, arg_type, boxed, ret_type): diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py index 92f584edcf..cbf7076ed9 100644 --- a/scripts/qapi/doc.py +++ b/scripts/qapi/doc.py @@ -5,7 +5,7 @@ """This script produces the documentation of a qapi schema in texinfo format""" import re -from qapi.gen import QAPIGenDoc, QAPISchemaVisitor +from .gen import QAPIGenDoc, QAPISchemaVisitor MSG_FMT = """ diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py index b544af5a1c..0467272438 100644 --- a/scripts/qapi/events.py +++ b/scripts/qapi/events.py @@ -12,10 +12,10 @@ See the COPYING file in the top-level directory. """ -from qapi.common import * -from qapi.gen import QAPISchemaModularCVisitor, ifcontext -from qapi.schema import QAPISchemaEnumMember -from qapi.types import gen_enum, gen_enum_lookup +from .common import * +from .gen import QAPISchemaModularCVisitor, ifcontext +from .schema import QAPISchemaEnumMember +from .types import gen_enum, gen_enum_lookup def build_event_send_proto(name, arg_type, boxed): diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index 2942520399..03b31ecfc1 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -16,8 +16,8 @@ import re from collections import OrderedDict -from qapi.common import c_name -from qapi.error import QAPISemError +from .common import c_name +from .error import QAPISemError # Names must be letters, numbers, -, and _. They must start with letter, diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py index bf5552a4e7..8df19a0df0 100644 --- a/scripts/qapi/gen.py +++ b/scripts/qapi/gen.py @@ -17,8 +17,8 @@ import re from contextlib import contextmanager -from qapi.common import * -from qapi.schema import QAPISchemaVisitor +from .common import * +from .schema import QAPISchemaVisitor class QAPIGen: diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py index 23652be810..2a34cd1e8e 100644 --- a/scripts/qapi/introspect.py +++ b/scripts/qapi/introspect.py @@ -10,10 +10,10 @@ See the COPYING file in the top-level directory. """ -from qapi.common import * -from qapi.gen import QAPISchemaMonolithicCVisitor -from qapi.schema import (QAPISchemaArrayType, QAPISchemaBuiltinType, - QAPISchemaType) +from .common import * +from .gen import QAPISchemaMonolithicCVisitor +from .schema import (QAPISchemaArrayType, QAPISchemaBuiltinType, + QAPISchemaType) def _make_tree(obj, ifcond, features, extra=None): diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index 165925ca72..327cf05736 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -18,8 +18,8 @@ import re from collections import OrderedDict -from qapi.error import QAPIParseError, QAPISemError -from qapi.source import QAPISourceInfo +from .error import QAPIParseError, QAPISemError +from .source import QAPISourceInfo class QAPISchemaParser: diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index 78309a00f0..a835ee6fde 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -18,10 +18,10 @@ import re from collections import OrderedDict -from qapi.common import c_name, pointer_suffix -from qapi.error import QAPIError, QAPISemError -from qapi.expr import check_exprs -from qapi.parser import QAPISchemaParser +from .common import c_name, pointer_suffix +from .error import QAPIError, QAPISemError +from .expr import check_exprs +from .parser import QAPISchemaParser class QAPISchemaEntity: diff --git a/scripts/qapi/script.py b/scripts/qapi/script.py new file mode 100644 index 0000000000..3f8338ade8 --- /dev/null +++ b/scripts/qapi/script.py @@ -0,0 +1,91 @@ +# 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 is the main entry point for generating C code from the QAPI schema. +""" + +import argparse +import re +import sys + +from .commands import gen_commands +from .doc import gen_doc +from .error import QAPIError +from .events import gen_events +from .introspect import gen_introspect +from .schema import QAPISchema +from .types import gen_types +from .visit import gen_visit + + +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 and 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) + gen_doc(schema, output_dir, prefix) + + +def main() -> int: + """ + gapi-gen shell script entrypoint. + 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=DEFAULT_OUTPUT_DIR, + help="write output to directory OUTPUT_DIR") + 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', + help="expose non-ABI names in introspection") + parser.add_argument('schema', action='store') + args = parser.parse_args() + + try: + generate(args.schema, + output_dir=args.output_dir, + prefix=args.prefix, + unmask=args.unmask, + builtins=args.builtins) + except QAPIError as err: + print(f"{sys.argv[0]}: {str(err)}", file=sys.stderr) + return 1 + return 0 diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py index 3640f17cd6..ca9a5aacb3 100644 --- a/scripts/qapi/types.py +++ b/scripts/qapi/types.py @@ -13,9 +13,9 @@ # See the COPYING file in the top-level directory. """ -from qapi.common import * -from qapi.gen import QAPISchemaModularCVisitor, ifcontext -from qapi.schema import QAPISchemaEnumMember, QAPISchemaObjectType +from .common import * +from .gen import QAPISchemaModularCVisitor, ifcontext +from .schema import QAPISchemaEnumMember, QAPISchemaObjectType # variants must be emitted before their container; track what has already diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py index cdabc5fa28..7850f6e848 100644 --- a/scripts/qapi/visit.py +++ b/scripts/qapi/visit.py @@ -13,9 +13,9 @@ See the COPYING file in the top-level directory. """ -from qapi.common import * -from qapi.gen import QAPISchemaModularCVisitor, ifcontext -from qapi.schema import QAPISchemaObjectType +from .common import * +from .gen import QAPISchemaModularCVisitor, ifcontext +from .schema import QAPISchemaObjectType def gen_visit_decl(name, scalar=False):
As part of delinting and adding type hints to the QAPI generator, it's helpful for the entrypoint to be part of the module, only leaving a very tiny entrypoint shim outside of the module. As a result, all of the include statements are reworked to be module-aware, as explicit relative imports. This is done primarily for the benefit of python tooling (pylint, mypy, flake8, et al) which otherwise has trouble consistently resolving "qapi.x" to mean "a sibling file in this folder." Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi-gen.py | 94 +++----------------------------------- scripts/qapi/commands.py | 4 +- scripts/qapi/doc.py | 2 +- scripts/qapi/events.py | 8 ++-- scripts/qapi/expr.py | 4 +- scripts/qapi/gen.py | 4 +- scripts/qapi/introspect.py | 8 ++-- scripts/qapi/parser.py | 4 +- scripts/qapi/schema.py | 8 ++-- scripts/qapi/script.py | 91 ++++++++++++++++++++++++++++++++++++ scripts/qapi/types.py | 6 +-- scripts/qapi/visit.py | 6 +-- 12 files changed, 124 insertions(+), 115 deletions(-) create mode 100644 scripts/qapi/script.py