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/expr.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'scripts/qapi/expr.py') diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index a15c1fb474..bb4dc55f56 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, -- cgit 1.4.1 From 67fea575023a9b2871414857770aafd334a6e39d Mon Sep 17 00:00:00 2001 From: John Snow Date: Fri, 9 Oct 2020 12:15:29 -0400 Subject: qapi: enforce import order/styling with isort While we're mucking around with imports, we might as well formalize the style we use. Let's use isort to do it for us. lines_after_imports=2: Use two lines after imports, to match PEP8's desire to have "two lines before and after" class definitions, which are likely to start immediately after imports. force_sort_within_sections: Intermingles "from x" and "import x" style statements, such that sorting is always performed strictly on the module name itself. force_grid_wrap=4: Four or more imports from a single module will force the one-per-line style that's more git-friendly. This will generally happen for 'typing' imports. multi_line_output=3: Uses the one-per-line indented style for long imports. include_trailing_comma: Adds a comma to the last import in a group, which makes git conflicts nicer to deal with, generally. line_length: 72 is chosen to match PEP8's "docstrings and comments" line length limit. If you have a single line import that exceeds 72 characters, your names are too long! Suggested-by: Cleber Rosa Signed-off-by: John Snow Reviewed-by: Cleber Rosa Tested-by: Cleber Rosa Reviewed-by: Markus Armbruster Message-Id: <20201009161558.107041-8-jsnow@redhat.com> Signed-off-by: Markus Armbruster --- scripts/qapi/.isort.cfg | 7 +++++++ scripts/qapi/expr.py | 3 ++- scripts/qapi/introspect.py | 7 +++++-- scripts/qapi/parser.py | 2 +- scripts/qapi/schema.py | 2 +- 5 files changed, 16 insertions(+), 5 deletions(-) create mode 100644 scripts/qapi/.isort.cfg (limited to 'scripts/qapi/expr.py') diff --git a/scripts/qapi/.isort.cfg b/scripts/qapi/.isort.cfg new file mode 100644 index 0000000000..643caa1fbd --- /dev/null +++ b/scripts/qapi/.isort.cfg @@ -0,0 +1,7 @@ +[settings] +force_grid_wrap=4 +force_sort_within_sections=True +include_trailing_comma=True +line_length=72 +lines_after_imports=2 +multi_line_output=3 diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index bb4dc55f56..2fcaaa2497 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -14,8 +14,9 @@ # This work is licensed under the terms of the GNU GPL, version 2. # See the COPYING file in the top-level directory. -import re from collections import OrderedDict +import re + from .common import c_name from .error import QAPISemError diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py index 42016a7e66..fafec94e02 100644 --- a/scripts/qapi/introspect.py +++ b/scripts/qapi/introspect.py @@ -17,8 +17,11 @@ from .common import ( mcgen, ) from .gen import QAPISchemaMonolithicCVisitor -from .schema import (QAPISchemaArrayType, QAPISchemaBuiltinType, - QAPISchemaType) +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 7298f5dbd1..e7b9d670ad 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -14,9 +14,9 @@ # This work is licensed under the terms of the GNU GPL, version 2. # See the COPYING file in the top-level directory. +from collections import OrderedDict import os import re -from collections import OrderedDict from .error import QAPIParseError, QAPISemError from .source import QAPISourceInfo diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index 676185d1a7..71ebb1e396 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -14,9 +14,9 @@ # TODO catching name collisions in generated code would be nice +from collections import OrderedDict import os import re -from collections import OrderedDict from .common import c_name, pointer_suffix from .error import QAPIError, QAPISemError -- cgit 1.4.1