* [PATCH] qapi: pluggable backend code generators @ 2025-02-17 13:49 Daniel P. Berrangé 2025-02-17 14:34 ` Philippe Mathieu-Daudé 2025-02-20 7:58 ` Markus Armbruster 0 siblings, 2 replies; 4+ messages in thread From: Daniel P. Berrangé @ 2025-02-17 13:49 UTC (permalink / raw) To: qemu-devel Cc: Victor Toso, Michael Roth, Markus Armbruster, Daniel P. Berrangé The 'qapi.backend.QAPIBackend' class defines an API contract for code generators. The current generator is put into a new class 'qapi.backend.QAPICBackend' and made to be the default impl. A custom generator can be requested using the '-k' arg which takes a fully qualified python class name qapi-gen.py -k the.python.module.QAPIMyBackend Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- This is an impl of the idea I mentioned at: https://lists.nongnu.org/archive/html/qemu-devel/2025-02/msg03475.html With this change, it is possible for the Go generator code to live outside of qemu.git, invoked using: $ PYTHONPATH=/path/to/qemu.git/scripts \ python /path/to/qemu.git/scripts/qapi-gen.py \ -o somedir \ -k qapi.golang.golang.QAPIGoBackend \ /path/to/qemu.git/qga/qapi-schema.json The external app could just expect qemu.git to be checkedout somewhere convenient, or could use a git submodule to reference it. scripts/qapi/backend.py | 96 +++++++++++++++++++++++++++++++++++++++++ scripts/qapi/main.py | 65 ++++++++-------------------- 2 files changed, 113 insertions(+), 48 deletions(-) create mode 100644 scripts/qapi/backend.py diff --git a/scripts/qapi/backend.py b/scripts/qapi/backend.py new file mode 100644 index 0000000000..b6873fd2e3 --- /dev/null +++ b/scripts/qapi/backend.py @@ -0,0 +1,96 @@ +# This work is licensed under the terms of the GNU GPL, version 2 or later. +# See the COPYING file in the top-level directory. + +from abc import ABC, abstractmethod +from typing import Optional + +from .commands import gen_commands +from .common import must_match +from .events import gen_events +from .features import gen_features +from .introspect import gen_introspect +from .schema import QAPISchema +from .types import gen_types +from .visit import gen_visit + + +def invalid_prefix_char(prefix: str) -> Optional[str]: + match = must_match(r'([A-Za-z_.-][A-Za-z0-9_.-]*)?', prefix) + if match.end() != len(prefix): + return prefix[match.end()] + return None + + +class QAPIBackend(ABC): + + def run(self, + schema_file: str, + output_dir: str, + prefix: str, + unmask: bool = False, + builtins: bool = False, + gen_tracing: bool = False) -> None: + """ + Run the code generator for the given schema into 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. + """ + assert invalid_prefix_char(prefix) is None + + schema = QAPISchema(schema_file) + self.generate(schema, output_dir, prefix, unmask, builtins, gen_tracing) + + @abstractmethod + def generate(self, + schema: QAPISchema, + output_dir: str, + prefix: str, + unmask: bool, + builtins: bool, + gen_tracing: bool) -> None: + """ + Generate code for the given schema into the target directory. + + :param schema: The primary QAPI schema object. + :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. + """ + pass + + +class QAPICBackend(QAPIBackend): + + def generate(self, + schema: QAPISchema, + output_dir: str, + prefix: str, + unmask: bool, + builtins: bool, + gen_tracing: bool) -> None: + """ + Generate C code for the given schema into 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. + """ + gen_types(schema, output_dir, prefix, builtins) + gen_features(schema, output_dir, prefix) + gen_visit(schema, output_dir, prefix, builtins) + gen_commands(schema, output_dir, prefix, gen_tracing) + gen_events(schema, output_dir, prefix) + gen_introspect(schema, output_dir, prefix, unmask) diff --git a/scripts/qapi/main.py b/scripts/qapi/main.py index 324081b9fc..35552dffce 100644 --- a/scripts/qapi/main.py +++ b/scripts/qapi/main.py @@ -8,53 +8,18 @@ """ import argparse +from importlib import import_module import sys -from typing import Optional -from .commands import gen_commands -from .common import must_match from .error import QAPIError -from .events import gen_events -from .features import gen_features -from .introspect import gen_introspect -from .schema import QAPISchema -from .types import gen_types -from .visit import gen_visit +from .backend import invalid_prefix_char -def invalid_prefix_char(prefix: str) -> Optional[str]: - match = must_match(r'([A-Za-z_.-][A-Za-z0-9_.-]*)?', prefix) - if match.end() != len(prefix): - return prefix[match.end()] - return None - - -def generate(schema_file: str, - output_dir: str, - prefix: str, - unmask: bool = False, - builtins: bool = False, - gen_tracing: bool = False) -> None: - """ - Generate C code for the given schema into 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. - """ - assert invalid_prefix_char(prefix) is None - - schema = QAPISchema(schema_file) - gen_types(schema, output_dir, prefix, builtins) - gen_features(schema, output_dir, prefix) - gen_visit(schema, output_dir, prefix, builtins) - gen_commands(schema, output_dir, prefix, gen_tracing) - gen_events(schema, output_dir, prefix) - gen_introspect(schema, output_dir, prefix, unmask) +def import_class_from_string(path): + module_path, _, class_name = path.rpartition('.') + mod = import_module(module_path) + klass = getattr(mod, class_name) + return klass def main() -> int: @@ -77,6 +42,8 @@ def main() -> int: parser.add_argument('-u', '--unmask-non-abi-names', action='store_true', dest='unmask', help="expose non-ABI names in introspection") + parser.add_argument('-k', '--backend', default="qapi.backend.QAPICBackend", + help="Python module name for code generator") # Option --suppress-tracing exists so we can avoid solving build system # problems. TODO Drop it when we no longer need it. @@ -92,13 +59,15 @@ def main() -> int: print(f"{sys.argv[0]}: {msg}", file=sys.stderr) return 1 + backendclass = import_class_from_string(args.backend) try: - generate(args.schema, - output_dir=args.output_dir, - prefix=args.prefix, - unmask=args.unmask, - builtins=args.builtins, - gen_tracing=not args.suppress_tracing) + backend = backendclass() + backend.run(args.schema, + output_dir=args.output_dir, + prefix=args.prefix, + unmask=args.unmask, + builtins=args.builtins, + gen_tracing=not args.suppress_tracing) except QAPIError as err: print(err, file=sys.stderr) return 1 -- 2.47.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] qapi: pluggable backend code generators 2025-02-17 13:49 [PATCH] qapi: pluggable backend code generators Daniel P. Berrangé @ 2025-02-17 14:34 ` Philippe Mathieu-Daudé 2025-02-20 7:58 ` Markus Armbruster 1 sibling, 0 replies; 4+ messages in thread From: Philippe Mathieu-Daudé @ 2025-02-17 14:34 UTC (permalink / raw) To: Daniel P. Berrangé, qemu-devel Cc: Victor Toso, Michael Roth, Markus Armbruster On 17/2/25 14:49, Daniel P. Berrangé wrote: > The 'qapi.backend.QAPIBackend' class defines an API contract for > code generators. The current generator is put into a new class > 'qapi.backend.QAPICBackend' and made to be the default impl. > > A custom generator can be requested using the '-k' arg which takes > a fully qualified python class name > > qapi-gen.py -k the.python.module.QAPIMyBackend > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > > This is an impl of the idea I mentioned at: > > https://lists.nongnu.org/archive/html/qemu-devel/2025-02/msg03475.html > > With this change, it is possible for the Go generator code to live > outside of qemu.git, invoked using: > > $ PYTHONPATH=/path/to/qemu.git/scripts \ > python /path/to/qemu.git/scripts/qapi-gen.py \ > -o somedir \ > -k qapi.golang.golang.QAPIGoBackend \ > /path/to/qemu.git/qga/qapi-schema.json > > The external app could just expect qemu.git to be checkedout somewhere > convenient, or could use a git submodule to reference it. > > scripts/qapi/backend.py | 96 +++++++++++++++++++++++++++++++++++++++++ > scripts/qapi/main.py | 65 ++++++++-------------------- > 2 files changed, 113 insertions(+), 48 deletions(-) > create mode 100644 scripts/qapi/backend.py Good idea. Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] qapi: pluggable backend code generators 2025-02-17 13:49 [PATCH] qapi: pluggable backend code generators Daniel P. Berrangé 2025-02-17 14:34 ` Philippe Mathieu-Daudé @ 2025-02-20 7:58 ` Markus Armbruster 2025-02-25 12:11 ` Daniel P. Berrangé 1 sibling, 1 reply; 4+ messages in thread From: Markus Armbruster @ 2025-02-20 7:58 UTC (permalink / raw) To: Daniel P. Berrangé; +Cc: qemu-devel, Victor Toso, Michael Roth, John Snow Cc: John for advice on my somewhat nebulous mypy worries at the end. Daniel P. Berrangé <berrange@redhat.com> writes: > The 'qapi.backend.QAPIBackend' class defines an API contract for > code generators. The current generator is put into a new class > 'qapi.backend.QAPICBackend' and made to be the default impl. > > A custom generator can be requested using the '-k' arg which takes > a fully qualified python class name > > qapi-gen.py -k the.python.module.QAPIMyBackend I understand why that will be useful, but explaining it briefly in the commit message wouldn't hurt. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > > This is an impl of the idea I mentioned at: > > https://lists.nongnu.org/archive/html/qemu-devel/2025-02/msg03475.html > > With this change, it is possible for the Go generator code to live > outside of qemu.git, invoked using: > > $ PYTHONPATH=/path/to/qemu.git/scripts \ > python /path/to/qemu.git/scripts/qapi-gen.py \ > -o somedir \ > -k qapi.golang.golang.QAPIGoBackend \ > /path/to/qemu.git/qga/qapi-schema.json > > The external app could just expect qemu.git to be checkedout somewhere > convenient, or could use a git submodule to reference it. > > scripts/qapi/backend.py | 96 +++++++++++++++++++++++++++++++++++++++++ > scripts/qapi/main.py | 65 ++++++++-------------------- > 2 files changed, 113 insertions(+), 48 deletions(-) > create mode 100644 scripts/qapi/backend.py > > diff --git a/scripts/qapi/backend.py b/scripts/qapi/backend.py > new file mode 100644 > index 0000000000..b6873fd2e3 > --- /dev/null > +++ b/scripts/qapi/backend.py > @@ -0,0 +1,96 @@ > +# This work is licensed under the terms of the GNU GPL, version 2 or later. > +# See the COPYING file in the top-level directory. > + > +from abc import ABC, abstractmethod > +from typing import Optional > + > +from .commands import gen_commands > +from .common import must_match > +from .events import gen_events > +from .features import gen_features > +from .introspect import gen_introspect > +from .schema import QAPISchema > +from .types import gen_types > +from .visit import gen_visit > + > + > +def invalid_prefix_char(prefix: str) -> Optional[str]: > + match = must_match(r'([A-Za-z_.-][A-Za-z0-9_.-]*)?', prefix) > + if match.end() != len(prefix): > + return prefix[match.end()] > + return None > + > + > +class QAPIBackend(ABC): > + > + def run(self, > + schema_file: str, > + output_dir: str, > + prefix: str, > + unmask: bool = False, > + builtins: bool = False, > + gen_tracing: bool = False) -> None: > + """ > + Run the code generator for the given schema into 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. > + """ > + assert invalid_prefix_char(prefix) is None > + > + schema = QAPISchema(schema_file) Hmm. This makes the backend run the frontend. Could we keep this in main.py instead? > + self.generate(schema, output_dir, prefix, unmask, builtins, gen_tracing) > + > + @abstractmethod > + def generate(self, > + schema: QAPISchema, > + output_dir: str, > + prefix: str, > + unmask: bool, > + builtins: bool, > + gen_tracing: bool) -> None: > + """ > + Generate code for the given schema into the target directory. > + > + :param schema: The primary QAPI schema object. > + :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. > + """ > + pass pylint prefers not to pass: scripts/qapi/backend.py:68:8: W0107: Unnecessary pass statement (unnecessary-pass) > + > + > +class QAPICBackend(QAPIBackend): > + > + def generate(self, > + schema: QAPISchema, > + output_dir: str, > + prefix: str, > + unmask: bool, > + builtins: bool, > + gen_tracing: bool) -> None: > + """ > + Generate C code for the given schema into 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. > + """ > + gen_types(schema, output_dir, prefix, builtins) > + gen_features(schema, output_dir, prefix) > + gen_visit(schema, output_dir, prefix, builtins) > + gen_commands(schema, output_dir, prefix, gen_tracing) > + gen_events(schema, output_dir, prefix) > + gen_introspect(schema, output_dir, prefix, unmask) > diff --git a/scripts/qapi/main.py b/scripts/qapi/main.py > index 324081b9fc..35552dffce 100644 > --- a/scripts/qapi/main.py > +++ b/scripts/qapi/main.py > @@ -8,53 +8,18 @@ > """ > > import argparse > +from importlib import import_module > import sys > -from typing import Optional > > -from .commands import gen_commands > -from .common import must_match > from .error import QAPIError > -from .events import gen_events > -from .features import gen_features > -from .introspect import gen_introspect > -from .schema import QAPISchema > -from .types import gen_types > -from .visit import gen_visit > +from .backend import invalid_prefix_char isort wants you to put this above from .error, like this: from .backend import invalid_prefix_char from .error import QAPIError > > > -def invalid_prefix_char(prefix: str) -> Optional[str]: > - match = must_match(r'([A-Za-z_.-][A-Za-z0-9_.-]*)?', prefix) > - if match.end() != len(prefix): > - return prefix[match.end()] > - return None > - > - > -def generate(schema_file: str, > - output_dir: str, > - prefix: str, > - unmask: bool = False, > - builtins: bool = False, > - gen_tracing: bool = False) -> None: > - """ > - Generate C code for the given schema into 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. > - """ > - assert invalid_prefix_char(prefix) is None > - > - schema = QAPISchema(schema_file) > - gen_types(schema, output_dir, prefix, builtins) > - gen_features(schema, output_dir, prefix) > - gen_visit(schema, output_dir, prefix, builtins) > - gen_commands(schema, output_dir, prefix, gen_tracing) > - gen_events(schema, output_dir, prefix) > - gen_introspect(schema, output_dir, prefix, unmask) > +def import_class_from_string(path): > + module_path, _, class_name = path.rpartition('.') > + mod = import_module(module_path) > + klass = getattr(mod, class_name) > + return klass Lacks error handling, see appended test cases. Moreover, mypy points out scripts/qapi/main.py:18: error: Function is missing a type annotation [no-untyped-def] The argument is str, but what is returned? The function name suggests it returns a class. As coded, the function could return pretty much anything. The caller actually needs a QAPIBackend class. We need a checked cast to QAPIBackend class somewhere. If you put it in this function, it returns a QAPIBackend class, and its name should be adjusted accordingly. import_backend() maybe? > > > def main() -> int: > @@ -77,6 +42,8 @@ def main() -> int: > parser.add_argument('-u', '--unmask-non-abi-names', action='store_true', > dest='unmask', > help="expose non-ABI names in introspection") > + parser.add_argument('-k', '--backend', default="qapi.backend.QAPICBackend", Any particular reason for picking -k for --backend? -b is taken, but -B would be available. Break the line before default=, please. > + help="Python module name for code generator") > > # Option --suppress-tracing exists so we can avoid solving build system > # problems. TODO Drop it when we no longer need it. > @@ -92,13 +59,15 @@ def main() -> int: > print(f"{sys.argv[0]}: {msg}", file=sys.stderr) > return 1 > > + backendclass = import_class_from_string(args.backend) > try: > - generate(args.schema, > - output_dir=args.output_dir, > - prefix=args.prefix, > - unmask=args.unmask, > - builtins=args.builtins, > - gen_tracing=not args.suppress_tracing) > + backend = backendclass() > + backend.run(args.schema, > + output_dir=args.output_dir, > + prefix=args.prefix, > + unmask=args.unmask, > + builtins=args.builtins, > + gen_tracing=not args.suppress_tracing) > except QAPIError as err: > print(err, file=sys.stderr) > return 1 The connection to the backend moves to run-time: static import statements get replaced by a dynamic import_module(). Fine, it's what it takes to support pluggable backends. However, it might hide the bundled backend from tools like mypy. Would that be bad? I'm not sure. If it is, we could avoid it pretty easily: instead of defaulting the module name, so we dynamically load the bundled backend module by default, default the class, so we create the bundled backend class by default. The promised test cases: $ python3 scripts/qapi-gen.py -o /tmp/$$ -b -k noexistent qapi/qapi-schema.json Traceback (most recent call last): File "/work/armbru/qemu/scripts/qapi-gen.py", line 19, in <module> sys.exit(main.main()) ~~~~~~~~~^^ File "/work/armbru/qemu/scripts/qapi/main.py", line 62, in main backendclass = import_class_from_string(args.backend) File "/work/armbru/qemu/scripts/qapi/main.py", line 20, in import_class_from_string mod = import_module(module_path) File "/usr/lib64/python3.13/importlib/__init__.py", line 88, in import_module return _bootstrap._gcd_import(name[level:], package, level) ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "<frozen importlib._bootstrap>", line 1384, in _gcd_import File "<frozen importlib._bootstrap>", line 1298, in _sanity_check ValueError: Empty module name python3 scripts/qapi-gen.py -o /tmp/$$ -b -k noexistent.foo qapi/qapi-schema.json Traceback (most recent call last): File "/work/armbru/qemu/scripts/qapi-gen.py", line 19, in <module> sys.exit(main.main()) ~~~~~~~~~^^ File "/work/armbru/qemu/scripts/qapi/main.py", line 62, in main backendclass = import_class_from_string(args.backend) File "/work/armbru/qemu/scripts/qapi/main.py", line 20, in import_class_from_string mod = import_module(module_path) File "/usr/lib64/python3.13/importlib/__init__.py", line 88, in import_module return _bootstrap._gcd_import(name[level:], package, level) ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "<frozen importlib._bootstrap>", line 1387, in _gcd_import File "<frozen importlib._bootstrap>", line 1360, in _find_and_load File "<frozen importlib._bootstrap>", line 1324, in _find_and_load_unlocked ModuleNotFoundError: No module named 'noexistent' $ python3 scripts/qapi-gen.py -o /tmp/$$ -b -k qapi.backend.foo qapi/qapi-schema.json Traceback (most recent call last): File "/work/armbru/qemu/scripts/qapi-gen.py", line 19, in <module> sys.exit(main.main()) ~~~~~~~~~^^ File "/work/armbru/qemu/scripts/qapi/main.py", line 62, in main backendclass = import_class_from_string(args.backend) File "/work/armbru/qemu/scripts/qapi/main.py", line 21, in import_class_from_string klass = getattr(mod, class_name) AttributeError: module 'qapi.backend' has no attribute 'foo' $ python3 scripts/qapi-gen.py -o /tmp/$$ -b -k qapi.backend.QAPIBackend qapi/qapi-schema.json Traceback (most recent call last): File "/work/armbru/qemu/scripts/qapi-gen.py", line 19, in <module> sys.exit(main.main()) ~~~~~~~~~^^ File "/work/armbru/qemu/scripts/qapi/main.py", line 64, in main backend = backendclass() TypeError: Can't instantiate abstract class QAPIBackend without an implementation for abstract method 'generate' $ python3 scripts/qapi-gen.py -o /tmp/$$ -b -k qapi.common.Indentation qapi/qapi-schema.json Traceback (most recent call last): File "/work/armbru/qemu/scripts/qapi-gen.py", line 19, in <module> sys.exit(main.main()) ~~~~~~~~~^^ File "/work/armbru/qemu/scripts/qapi/main.py", line 65, in main backend.run(args.schema, ^^^^^^^^^^^ AttributeError: 'Indentation' object has no attribute 'run' ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] qapi: pluggable backend code generators 2025-02-20 7:58 ` Markus Armbruster @ 2025-02-25 12:11 ` Daniel P. Berrangé 0 siblings, 0 replies; 4+ messages in thread From: Daniel P. Berrangé @ 2025-02-25 12:11 UTC (permalink / raw) To: Markus Armbruster; +Cc: qemu-devel, Victor Toso, Michael Roth, John Snow On Thu, Feb 20, 2025 at 08:58:17AM +0100, Markus Armbruster wrote: > Cc: John for advice on my somewhat nebulous mypy worries at the end. > > Daniel P. Berrangé <berrange@redhat.com> writes: > > > The 'qapi.backend.QAPIBackend' class defines an API contract for > > code generators. The current generator is put into a new class > > 'qapi.backend.QAPICBackend' and made to be the default impl. > > > > A custom generator can be requested using the '-k' arg which takes > > a fully qualified python class name > > > > qapi-gen.py -k the.python.module.QAPIMyBackend > > I understand why that will be useful, but explaining it briefly in the > commit message wouldn't hurt. I expanded on this. > > +class QAPIBackend(ABC): > > + > > + def run(self, > > + schema_file: str, > > + output_dir: str, > > + prefix: str, > > + unmask: bool = False, > > + builtins: bool = False, > > + gen_tracing: bool = False) -> None: > > + """ > > + Run the code generator for the given schema into 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. > > + """ > > + assert invalid_prefix_char(prefix) is None > > + > > + schema = QAPISchema(schema_file) > > Hmm. This makes the backend run the frontend. Could we keep this in > main.py instead? Yes, that is better, and eliminates the need for the extra 'run' method to call 'generate'. > > + self.generate(schema, output_dir, prefix, unmask, builtins, gen_tracing) > > + > > + @abstractmethod > > + def generate(self, > > + schema: QAPISchema, > > + output_dir: str, > > + prefix: str, > > + unmask: bool, > > + builtins: bool, > > + gen_tracing: bool) -> None: > > + """ > > + Generate code for the given schema into the target directory. > > + > > + :param schema: The primary QAPI schema object. > > + :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. > > + """ > > + pass > > pylint prefers not to pass: > > scripts/qapi/backend.py:68:8: W0107: Unnecessary pass statement (unnecessary-pass) Fun, I expected it to be a syntax error to not have a statement in the method body, but pylint is right. > > diff --git a/scripts/qapi/main.py b/scripts/qapi/main.py > > index 324081b9fc..35552dffce 100644 > > --- a/scripts/qapi/main.py > > +++ b/scripts/qapi/main.py > > @@ -8,53 +8,18 @@ > > """ > > > > import argparse > > +from importlib import import_module > > import sys > > -from typing import Optional > > > > -from .commands import gen_commands > > -from .common import must_match > > from .error import QAPIError > > -from .events import gen_events > > -from .features import gen_features > > -from .introspect import gen_introspect > > -from .schema import QAPISchema > > -from .types import gen_types > > -from .visit import gen_visit > > +from .backend import invalid_prefix_char > > isort wants you to put this above from .error, like this: > > from .backend import invalid_prefix_char > from .error import QAPIError Yep, I forgot we sorted imports > > +def import_class_from_string(path): > > + module_path, _, class_name = path.rpartition('.') > > + mod = import_module(module_path) > > + klass = getattr(mod, class_name) > > + return klass > > Lacks error handling, see appended test cases. > > Moreover, mypy points out > > scripts/qapi/main.py:18: error: Function is missing a type annotation [no-untyped-def] > > The argument is str, but what is returned? > > The function name suggests it returns a class. > > As coded, the function could return pretty much anything. > > The caller actually needs a QAPIBackend class. > > We need a checked cast to QAPIBackend class somewhere. If you put it in > this function, it returns a QAPIBackend class, and its name should be > adjusted accordingly. import_backend() maybe? Yeah, with all the error checking needed for the scenarios you illustrate below, this method was better turned into a more specialized "create_backend" method. > > > > > > > def main() -> int: > > @@ -77,6 +42,8 @@ def main() -> int: > > parser.add_argument('-u', '--unmask-non-abi-names', action='store_true', > > dest='unmask', > > help="expose non-ABI names in introspection") > > + parser.add_argument('-k', '--backend', default="qapi.backend.QAPICBackend", > > Any particular reason for picking -k for --backend? There is a 'k' in 'backend' was the extent of my decision making:-) > -b is taken, but -B would be available. I've used -B now. > > @@ -92,13 +59,15 @@ def main() -> int: > > print(f"{sys.argv[0]}: {msg}", file=sys.stderr) > > return 1 > > > > + backendclass = import_class_from_string(args.backend) > > try: > > - generate(args.schema, > > - output_dir=args.output_dir, > > - prefix=args.prefix, > > - unmask=args.unmask, > > - builtins=args.builtins, > > - gen_tracing=not args.suppress_tracing) > > + backend = backendclass() > > + backend.run(args.schema, > > + output_dir=args.output_dir, > > + prefix=args.prefix, > > + unmask=args.unmask, > > + builtins=args.builtins, > > + gen_tracing=not args.suppress_tracing) > > except QAPIError as err: > > print(err, file=sys.stderr) > > return 1 > > The connection to the backend moves to run-time: static import > statements get replaced by a dynamic import_module(). Fine, it's what > it takes to support pluggable backends. > > However, it might hide the bundled backend from tools like mypy. Would > that be bad? I'm not sure. > > If it is, we could avoid it pretty easily: instead of defaulting the > module name, so we dynamically load the bundled backend module by > default, default the class, so we create the bundled backend class by > default. Yes, in v2 I've kept QAPICBackend as an explicit import so we avoid any questions about hiding from mypy. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-02-25 12:12 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-02-17 13:49 [PATCH] qapi: pluggable backend code generators Daniel P. Berrangé 2025-02-17 14:34 ` Philippe Mathieu-Daudé 2025-02-20 7:58 ` Markus Armbruster 2025-02-25 12:11 ` Daniel P. Berrangé
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).