From 7137a96099644734cd6045313823840d4cecd5e8 Mon Sep 17 00:00:00 2001 From: John Snow Date: Fri, 9 Oct 2020 12:15:27 -0400 Subject: qapi: Prefer explicit relative imports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All of the QAPI include statements are changed to be package-aware, as explicit relative imports. A quirk of Python packages is that the name of the package exists only *outside* of the package. This means that to a module inside of the qapi folder, there is inherently no such thing as the "qapi" package. The reason these imports work is because the "qapi" package exists in the context of the caller -- the execution shim, where sys.path includes a directory that has a 'qapi' folder in it. When we write "from qapi import sibling", we are NOT referencing the folder 'qapi', but rather "any package named qapi in sys.path". If you should so happen to have a 'qapi' package in your path, it will use *that* package. When we write "from .sibling import foo", we always reference explicitly our sibling module; guaranteeing consistency in *where* we are importing these modules from. This can be useful when working with virtual environments and packages in development mode. In development mode, a package is installed as a series of symlinks that forwards to your same source files. The problem arises because code quality checkers will follow "import qapi.x" to the "installed" version instead of the sibling file and -- even though they are the same file -- they have different module paths, and this causes cyclic import problems, false positive type mismatch errors, and more. It can also be useful when dealing with hierarchical packages, e.g. if we allow qemu.core.qmp, qemu.qapi.parser, etc. Signed-off-by: John Snow Reviewed-by: Eduardo Habkost Reviewed-by: Cleber Rosa Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20201009161558.107041-6-jsnow@redhat.com> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- scripts/qapi/events.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'scripts/qapi/events.py') 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 @@ This work is licensed under the terms of the GNU GPL, version 2. 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): -- cgit 1.4.1 From 5af8263d40c698c47befd4c0bed3d6c452b56d82 Mon Sep 17 00:00:00 2001 From: John Snow Date: Fri, 9 Oct 2020 12:15:28 -0400 Subject: qapi: Remove wildcard includes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wildcard includes become hard to manage when refactoring and dealing with circular dependencies with strictly typed mypy. flake8 also flags each one as a warning, as it is not smart enough to know which names exist in the imported file. Remove them and include things explicitly by name instead. Signed-off-by: John Snow Reviewed-by: Eduardo Habkost Reviewed-by: Cleber Rosa Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20201009161558.107041-7-jsnow@redhat.com> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- scripts/qapi/commands.py | 2 +- scripts/qapi/events.py | 7 ++++++- scripts/qapi/gen.py | 12 +++++++++--- scripts/qapi/introspect.py | 7 ++++++- scripts/qapi/types.py | 8 +++++++- scripts/qapi/visit.py | 10 +++++++++- 6 files changed, 38 insertions(+), 8 deletions(-) (limited to 'scripts/qapi/events.py') diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py index 1f43a0a34e..e06c10afcd 100644 --- a/scripts/qapi/commands.py +++ b/scripts/qapi/commands.py @@ -13,7 +13,7 @@ This work is licensed under the terms of the GNU GPL, version 2. See the COPYING file in the top-level directory. """ -from .common import * +from .common import build_params, c_name, mcgen from .gen import QAPIGenCCode, QAPISchemaModularCVisitor, ifcontext diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py index 0467272438..6b3afa14d7 100644 --- a/scripts/qapi/events.py +++ b/scripts/qapi/events.py @@ -12,7 +12,12 @@ This work is licensed under the terms of the GNU GPL, version 2. See the COPYING file in the top-level directory. """ -from .common import * +from .common import ( + build_params, + c_enum_const, + c_name, + mcgen, +) from .gen import QAPISchemaModularCVisitor, ifcontext from .schema import QAPISchemaEnumMember from .types import gen_enum, gen_enum_lookup diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py index fc57fdca5b..1fed712b43 100644 --- a/scripts/qapi/gen.py +++ b/scripts/qapi/gen.py @@ -11,13 +11,19 @@ # This work is licensed under the terms of the GNU GPL, version 2. # See the COPYING file in the top-level directory. - +from contextlib import contextmanager import errno import os import re -from contextlib import contextmanager -from .common import * +from .common import ( + c_fname, + gen_endif, + gen_if, + guardend, + guardstart, + mcgen, +) from .schema import QAPISchemaVisitor diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py index 6c82d9d95f..42016a7e66 100644 --- a/scripts/qapi/introspect.py +++ b/scripts/qapi/introspect.py @@ -10,7 +10,12 @@ This work is licensed under the terms of the GNU GPL, version 2. See the COPYING file in the top-level directory. """ -from .common import * +from .common import ( + c_name, + gen_endif, + gen_if, + mcgen, +) from .gen import QAPISchemaMonolithicCVisitor from .schema import (QAPISchemaArrayType, QAPISchemaBuiltinType, QAPISchemaType) diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py index ca9a5aacb3..53b47f9e58 100644 --- a/scripts/qapi/types.py +++ b/scripts/qapi/types.py @@ -13,7 +13,13 @@ This work is licensed under the terms of the GNU GPL, version 2. # See the COPYING file in the top-level directory. """ -from .common import * +from .common import ( + c_enum_const, + c_name, + gen_endif, + gen_if, + mcgen, +) from .gen import QAPISchemaModularCVisitor, ifcontext from .schema import QAPISchemaEnumMember, QAPISchemaObjectType diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py index 7850f6e848..ea277e7704 100644 --- a/scripts/qapi/visit.py +++ b/scripts/qapi/visit.py @@ -13,7 +13,15 @@ This work is licensed under the terms of the GNU GPL, version 2. See the COPYING file in the top-level directory. """ -from .common import * +from .common import ( + c_enum_const, + c_name, + gen_endif, + gen_if, + mcgen, + pop_indent, + push_indent, +) from .gen import QAPISchemaModularCVisitor, ifcontext from .schema import QAPISchemaObjectType -- cgit 1.4.1 From e6a34cd7a440e6ba04251612aa6eb036d3c47d98 Mon Sep 17 00:00:00 2001 From: John Snow Date: Fri, 9 Oct 2020 12:15:39 -0400 Subject: qapi/common.py: move build_params into gen.py Including it in common.py creates a circular import dependency; schema relies on common, but common.build_params requires a type annotation from schema. To type this properly, it needs to be moved outside the cycle. Signed-off-by: John Snow Reviewed-by: Eduardo Habkost Reviewed-by: Cleber Rosa Message-Id: <20201009161558.107041-18-jsnow@redhat.com> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- scripts/qapi/commands.py | 9 +++++++-- scripts/qapi/common.py | 23 ----------------------- scripts/qapi/events.py | 9 ++------- scripts/qapi/gen.py | 29 +++++++++++++++++++++++++++-- 4 files changed, 36 insertions(+), 34 deletions(-) (limited to 'scripts/qapi/events.py') diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py index cde0c1e777..88ba11c40e 100644 --- a/scripts/qapi/commands.py +++ b/scripts/qapi/commands.py @@ -13,8 +13,13 @@ This work is licensed under the terms of the GNU GPL, version 2. See the COPYING file in the top-level directory. """ -from .common import build_params, c_name, mcgen -from .gen import QAPIGenCCode, QAPISchemaModularCVisitor, ifcontext +from .common import c_name, mcgen +from .gen import ( + QAPIGenCCode, + QAPISchemaModularCVisitor, + build_params, + ifcontext, +) def gen_command_decl(name, arg_type, boxed, ret_type): diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py index 669e3829b3..11b86beeab 100644 --- a/scripts/qapi/common.py +++ b/scripts/qapi/common.py @@ -210,26 +210,3 @@ def gen_endif(ifcond: Sequence[str]) -> str: #endif /* %(cond)s */ ''', cond=ifc) return ret - - -def build_params(arg_type, - boxed: bool, - extra: Optional[str] = None) -> str: - ret = '' - sep = '' - if boxed: - assert arg_type - ret += '%s arg' % arg_type.c_param_type() - sep = ', ' - elif arg_type: - assert not arg_type.variants - for memb in arg_type.members: - ret += sep - sep = ', ' - if memb.optional: - ret += 'bool has_%s, ' % c_name(memb.name) - ret += '%s %s' % (memb.type.c_param_type(), - c_name(memb.name)) - if extra: - ret += sep + extra - return ret if ret else 'void' diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py index 6b3afa14d7..f840a62ed9 100644 --- a/scripts/qapi/events.py +++ b/scripts/qapi/events.py @@ -12,13 +12,8 @@ This work is licensed under the terms of the GNU GPL, version 2. See the COPYING file in the top-level directory. """ -from .common import ( - build_params, - c_enum_const, - c_name, - mcgen, -) -from .gen import QAPISchemaModularCVisitor, ifcontext +from .common import c_enum_const, c_name, mcgen +from .gen import QAPISchemaModularCVisitor, build_params, ifcontext from .schema import QAPISchemaEnumMember from .types import gen_enum, gen_enum_lookup diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py index 1fed712b43..fff0c0acb6 100644 --- a/scripts/qapi/gen.py +++ b/scripts/qapi/gen.py @@ -2,7 +2,7 @@ # # QAPI code generation # -# Copyright (c) 2018-2019 Red Hat Inc. +# Copyright (c) 2015-2019 Red Hat Inc. # # Authors: # Markus Armbruster @@ -15,16 +15,18 @@ from contextlib import contextmanager import errno import os import re +from typing import Optional from .common import ( c_fname, + c_name, gen_endif, gen_if, guardend, guardstart, mcgen, ) -from .schema import QAPISchemaVisitor +from .schema import QAPISchemaObjectType, QAPISchemaVisitor class QAPIGen: @@ -90,6 +92,29 @@ def _wrap_ifcond(ifcond, before, after): return out +def build_params(arg_type: Optional[QAPISchemaObjectType], + boxed: bool, + extra: Optional[str] = None) -> str: + ret = '' + sep = '' + if boxed: + assert arg_type + ret += '%s arg' % arg_type.c_param_type() + sep = ', ' + elif arg_type: + assert not arg_type.variants + for memb in arg_type.members: + ret += sep + sep = ', ' + if memb.optional: + ret += 'bool has_%s, ' % c_name(memb.name) + ret += '%s %s' % (memb.type.c_param_type(), + c_name(memb.name)) + if extra: + ret += sep + extra + return ret if ret else 'void' + + class QAPIGenCCode(QAPIGen): def __init__(self, fname): -- cgit 1.4.1 From d1b21b393e7ae1a387a7c81f633d924a66d29408 Mon Sep 17 00:00:00 2001 From: John Snow Date: Fri, 9 Oct 2020 12:15:41 -0400 Subject: qapi/events.py: add type hint annotations Annotations do not change runtime behavior. This commit *only* adds annotations. Note: __init__ does not need its return type annotated, as it is special. https://mypy.readthedocs.io/en/stable/class_basics.html#annotating-init-methods Signed-off-by: John Snow Reviewed-by: Eduardo Habkost Reviewed-by: Cleber Rosa Message-Id: <20201009161558.107041-20-jsnow@redhat.com> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- scripts/qapi/events.py | 46 +++++++++++++++++++++++++++++++++++----------- scripts/qapi/mypy.ini | 5 ----- 2 files changed, 35 insertions(+), 16 deletions(-) (limited to 'scripts/qapi/events.py') diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py index f840a62ed9..57e0939e96 100644 --- a/scripts/qapi/events.py +++ b/scripts/qapi/events.py @@ -12,19 +12,31 @@ This work is licensed under the terms of the GNU GPL, version 2. See the COPYING file in the top-level directory. """ +from typing import List + from .common import c_enum_const, c_name, mcgen from .gen import QAPISchemaModularCVisitor, build_params, ifcontext -from .schema import QAPISchemaEnumMember +from .schema import ( + QAPISchema, + QAPISchemaEnumMember, + QAPISchemaFeature, + QAPISchemaObjectType, +) +from .source import QAPISourceInfo from .types import gen_enum, gen_enum_lookup -def build_event_send_proto(name, arg_type, boxed): +def build_event_send_proto(name: str, + arg_type: QAPISchemaObjectType, + boxed: bool) -> str: return 'void qapi_event_send_%(c_name)s(%(param)s)' % { 'c_name': c_name(name.lower()), 'param': build_params(arg_type, boxed)} -def gen_event_send_decl(name, arg_type, boxed): +def gen_event_send_decl(name: str, + arg_type: QAPISchemaObjectType, + boxed: bool) -> str: return mcgen(''' %(proto)s; @@ -33,7 +45,7 @@ def gen_event_send_decl(name, arg_type, boxed): # Declare and initialize an object 'qapi' using parameters from build_params() -def gen_param_var(typ): +def gen_param_var(typ: QAPISchemaObjectType) -> str: assert not typ.variants ret = mcgen(''' %(c_name)s param = { @@ -61,7 +73,11 @@ def gen_param_var(typ): return ret -def gen_event_send(name, arg_type, boxed, event_enum_name, event_emit): +def gen_event_send(name: str, + arg_type: QAPISchemaObjectType, + boxed: bool, + event_enum_name: str, + event_emit: str) -> str: # FIXME: Our declaration of local variables (and of 'errp' in the # parameter list) can collide with exploded members of the event's # data type passed in as parameters. If this collision ever hits in @@ -137,15 +153,15 @@ def gen_event_send(name, arg_type, boxed, event_enum_name, event_emit): class QAPISchemaGenEventVisitor(QAPISchemaModularCVisitor): - def __init__(self, prefix): + def __init__(self, prefix: str): super().__init__( prefix, 'qapi-events', ' * Schema-defined QAPI/QMP events', None, __doc__) self._event_enum_name = c_name(prefix + 'QAPIEvent', protect=False) - self._event_enum_members = [] + self._event_enum_members: List[QAPISchemaEnumMember] = [] self._event_emit_name = c_name(prefix + 'qapi_event_emit') - def _begin_user_module(self, name): + def _begin_user_module(self, name: str) -> None: events = self._module_basename('qapi-events', name) types = self._module_basename('qapi-types', name) visit = self._module_basename('qapi-visit', name) @@ -168,7 +184,7 @@ class QAPISchemaGenEventVisitor(QAPISchemaModularCVisitor): ''', types=types)) - def visit_end(self): + def visit_end(self) -> None: self._add_system_module('emit', ' * QAPI Events emission') self._genc.preamble_add(mcgen(''' #include "qemu/osdep.h" @@ -189,7 +205,13 @@ void %(event_emit)s(%(event_enum)s event, QDict *qdict); event_emit=self._event_emit_name, event_enum=self._event_enum_name)) - def visit_event(self, name, info, ifcond, features, arg_type, boxed): + def visit_event(self, + name: str, + info: QAPISourceInfo, + ifcond: List[str], + features: List[QAPISchemaFeature], + arg_type: QAPISchemaObjectType, + boxed: bool) -> None: with ifcontext(ifcond, self._genh, self._genc): self._genh.add(gen_event_send_decl(name, arg_type, boxed)) self._genc.add(gen_event_send(name, arg_type, boxed, @@ -200,7 +222,9 @@ void %(event_emit)s(%(event_enum)s event, QDict *qdict); self._event_enum_members.append(QAPISchemaEnumMember(name, None)) -def gen_events(schema, output_dir, prefix): +def gen_events(schema: QAPISchema, + output_dir: str, + prefix: str) -> None: vis = QAPISchemaGenEventVisitor(prefix) schema.visit(vis) vis.write(output_dir) diff --git a/scripts/qapi/mypy.ini b/scripts/qapi/mypy.ini index 00fac15dc6..5df11e53fd 100644 --- a/scripts/qapi/mypy.ini +++ b/scripts/qapi/mypy.ini @@ -14,11 +14,6 @@ disallow_untyped_defs = False disallow_incomplete_defs = False check_untyped_defs = False -[mypy-qapi.events] -disallow_untyped_defs = False -disallow_incomplete_defs = False -check_untyped_defs = False - [mypy-qapi.expr] disallow_untyped_defs = False disallow_incomplete_defs = False -- cgit 1.4.1 From 1a503761e451549ba7457848d3955bd3391438d6 Mon Sep 17 00:00:00 2001 From: John Snow Date: Fri, 9 Oct 2020 12:15:42 -0400 Subject: qapi/events.py: Move comments into docstrings Clarify them while we're here. Signed-off-by: John Snow Reviewed-by: Eduardo Habkost Reviewed-by: Cleber Rosa Message-Id: <20201009161558.107041-21-jsnow@redhat.com> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- scripts/qapi/events.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'scripts/qapi/events.py') diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py index 57e0939e96..599f3d1f56 100644 --- a/scripts/qapi/events.py +++ b/scripts/qapi/events.py @@ -44,8 +44,12 @@ def gen_event_send_decl(name: str, proto=build_event_send_proto(name, arg_type, boxed)) -# Declare and initialize an object 'qapi' using parameters from build_params() def gen_param_var(typ: QAPISchemaObjectType) -> str: + """ + Generate a struct variable holding the event parameters. + + Initialize it with the function arguments defined in `gen_event_send`. + """ assert not typ.variants ret = mcgen(''' %(c_name)s param = { -- cgit 1.4.1