Re: [PATCH 04/37] qapi: move generator entrypoint into module
John Snow writes: > On 9/16/20 7:54 AM, Markus Armbruster wrote: >> John Snow 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. [...]
Re: [PATCH 04/37] qapi: move generator entrypoint into module
On 9/16/20 7:54 AM, Markus Armbruster wrote: John Snow 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 --- 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. [...]
Re: [PATCH 04/37] qapi: move generator entrypoint into module
John Snow 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 > --- > 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. [...]
[PATCH 04/37] qapi: move generator entrypoint into module
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
---
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
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
