[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-commits] [qemu/qemu] 3404e5: qapi/parser: Don't try to handle file
|
From: |
Peter Maydell |
|
Subject: |
[Qemu-commits] [qemu/qemu] 3404e5: qapi/parser: Don't try to handle file errors |
|
Date: |
Fri, 21 May 2021 01:54:52 -0700 |
Branch: refs/heads/master
Home: https://github.com/qemu/qemu
Commit: 3404e57410b80734e6961b6574d6683d9c3d9c14
https://github.com/qemu/qemu/commit/3404e57410b80734e6961b6574d6683d9c3d9c14
Author: John Snow <jsnow@redhat.com>
Date: 2021-05-20 (Thu, 20 May 2021)
Changed paths:
M scripts/qapi/parser.py
M scripts/qapi/schema.py
M scripts/qapi/source.py
M tests/qapi-schema/test-qapi.py
Log Message:
-----------
qapi/parser: Don't try to handle file errors
Fixes: f5d4361cda
Fixes: 52a474180a
Fixes: 46f49468c6
Remove the try/except block that handles file-opening errors in
QAPISchemaParser.__init__() and add one each to
QAPISchemaParser._include() and QAPISchema.__init__() respectively.
This simultaneously fixes the typing of info.fname (f5d4361cda), A
static typing violation in test-qapi (46f49468c6), and a regression of
an error message (52a474180a).
The short-ish version of what motivates this patch is:
- It's hard to write a good error message in the init method,
because we need to determine the context of our caller to do so.
It's easier to just let the caller write the message.
- We don't want to allow QAPISourceInfo(None, None, None) to exist. The
typing introduced by commit f5d4361cda types the 'fname' field as
(non-optional) str, which was premature until the removal of this
construct.
- Errors made using such an object are currently incorrect (since
52a474180a)
- It's not technically a semantic error if we cannot open the schema.
- There are various typing constraints that make mixing these two cases
undesirable for a single special case.
- test-qapi's code handling an fname of 'None' is now dead, drop it.
Additionally, Not all QAPIError objects have an 'info' field (since
46f49468), so deleting this stanza corrects a typing oversight in
test-qapi introduced by that commit.
Other considerations:
- open() is moved to a 'with' block to ensure file pointers are
cleaned up deterministically.
- Python 3.3 deprecated IOError and made it a synonym for OSError.
Avoid the misleading perception these exception handlers are
narrower than they really are.
The long version:
The error message here is incorrect (since commit 52a474180a):
> python3 qapi-gen.py 'fake.json'
qapi-gen.py: qapi-gen.py: can't read schema file 'fake.json': No such file or
directory
In pursuing it, we find that QAPISourceInfo has a special accommodation
for when there's no filename. Meanwhile, the intent when QAPISourceInfo
was typed (f5d4361cda) was non-optional 'str'. This usage was
overlooked.
To remove this, I'd want to avoid having a "fake" QAPISourceInfo
object. I also don't want to explicitly begin accommodating
QAPISourceInfo itself being None, because we actually want to eventually
prove that this can never happen -- We don't want to confuse "The file
isn't open yet" with "This error stems from a definition that wasn't
defined in any file".
(An earlier series tried to create a dummy info object, but it was tough
to prove in review that it worked correctly without creating new
regressions. This patch avoids that distraction. We would like to first
prove that we never raise QAPISemError for any built-in object before we
add "special" info objects. We aren't ready to do that yet.)
So, which way out of the labyrinth?
Here's one way: Don't try to handle errors at a level with "mixed"
semantic contexts; i.e. don't mix inclusion errors (should report a
source line where the include was triggered) and command line errors
(where we specified a file we couldn't read).
Remove the error handling from the initializer of the parser. Pythonic!
Now it's the caller's job to figure out what to do about it. Handle the
error in QAPISchemaParser._include() instead, where we can write a
targeted error message where we are guaranteed to have an 'info' context
to report with.
The root level error can similarly move to QAPISchema.__init__(), where
we know we'll never have an info context to report with, so we use a
more abstract error type.
Now the error looks sensible again:
> python3 qapi-gen.py 'fake.json'
qapi-gen.py: can't read schema file 'fake.json': No such file or directory
With these error cases separated, QAPISourceInfo can be solidified as
never having placeholder arguments that violate our desired types. Clean
up test-qapi along similar lines.
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210519183951.3946870-2-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Commit: 334c3cd58a202d082703a1ae175b4230f4157f65
https://github.com/qemu/qemu/commit/334c3cd58a202d082703a1ae175b4230f4157f65
Author: John Snow <jsnow@redhat.com>
Date: 2021-05-20 (Thu, 20 May 2021)
Changed paths:
M tests/qapi-schema/meson.build
A tests/qapi-schema/missing-schema.err
A tests/qapi-schema/missing-schema.out
Log Message:
-----------
qapi: Add test for nonexistent schema file
This tests the error-return pathway introduced in the previous commit.
(Thanks to Paolo for the help with the Meson magic.)
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210519183951.3946870-3-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Commit: b2b31fdf9bc66a82718c9e6ede2f364b0005728a
https://github.com/qemu/qemu/commit/b2b31fdf9bc66a82718c9e6ede2f364b0005728a
Author: John Snow <jsnow@redhat.com>
Date: 2021-05-20 (Thu, 20 May 2021)
Changed paths:
M scripts/qapi/parser.py
M scripts/qapi/source.py
Log Message:
-----------
qapi/source: Remove line number from QAPISourceInfo initializer
With the QAPISourceInfo(None, None, None) construct gone, there's no
longer any reason to have to specify that a file starts on the first
line. Remove it from the initializer and default it to 1.
Remove the last vestiges where we check for 'line' being unset, that
can't happen, now.
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210519183951.3946870-4-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Commit: 16ff40acc9c1fc871c2c835b3b20e374d6daed98
https://github.com/qemu/qemu/commit/16ff40acc9c1fc871c2c835b3b20e374d6daed98
Author: John Snow <jsnow@redhat.com>
Date: 2021-05-20 (Thu, 20 May 2021)
Changed paths:
M scripts/qapi/parser.py
Log Message:
-----------
qapi/parser: factor parsing routine into method
For the sake of keeping __init__ smaller (and treating it more like a
gallery of what state variables we can expect to see), put the actual
parsing action into a parse method. It remains invoked from the init
method to reduce churn.
To accomplish this, @previously_included becomes the private data
member ._included, and the filename is stashed as ._fname.
Add any missing declarations to the init method, and group them by
function so they can be understood quickly at a glance.
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210519183951.3946870-5-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Commit: 7c610ce6a9950a49148fc3d37ed353958ca8d776
https://github.com/qemu/qemu/commit/7c610ce6a9950a49148fc3d37ed353958ca8d776
Author: John Snow <jsnow@redhat.com>
Date: 2021-05-20 (Thu, 20 May 2021)
Changed paths:
M scripts/qapi/parser.py
Log Message:
-----------
qapi/parser: Assert lexer value is a string
The type checker can't narrow the type of the token value to string,
because it's only loosely correlated with the return token.
We know that a token of '#' should always have a "str" value.
Add an assertion.
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210519183951.3946870-6-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Commit: 9cd0205d553bc27a66454782dfc5d7e8d2324e34
https://github.com/qemu/qemu/commit/9cd0205d553bc27a66454782dfc5d7e8d2324e34
Author: John Snow <jsnow@redhat.com>
Date: 2021-05-20 (Thu, 20 May 2021)
Changed paths:
M scripts/qapi/parser.py
M tests/qapi-schema/non-objects.err
M tests/qapi-schema/quoted-structural-chars.err
Log Message:
-----------
qapi/parser: enforce all top-level expressions must be dict in _parse()
Instead of using get_expr nested=False, allow get_expr to always return
any expression. In exchange, add a new error message to the top-level
parser that explains the semantic error: Top-level expressions must
always be JSON objects.
This helps mypy understand the rest of this function which assumes that
get_expr did indeed return a dict.
The exception type changes from QAPIParseError to QAPISemError as a
result, and the error message in two tests now changes.
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210519183951.3946870-7-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Commit: 234dce2c2d93cfff7433c0fd244ef207c7eace2b
https://github.com/qemu/qemu/commit/234dce2c2d93cfff7433c0fd244ef207c7eace2b
Author: John Snow <jsnow@redhat.com>
Date: 2021-05-20 (Thu, 20 May 2021)
Changed paths:
M scripts/qapi/parser.py
Log Message:
-----------
qapi/parser: assert object keys are strings
The single quote token implies the value is a string. Assert this to be
the case, to allow us to write an accurate return type for get_members.
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210519183951.3946870-8-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Commit: 43b1be65f07c57ef2a4a6012e263677cf812c7e1
https://github.com/qemu/qemu/commit/43b1be65f07c57ef2a4a6012e263677cf812c7e1
Author: John Snow <jsnow@redhat.com>
Date: 2021-05-20 (Thu, 20 May 2021)
Changed paths:
M scripts/qapi/parser.py
Log Message:
-----------
qapi/parser: Use @staticmethod where appropriate
No self, no thank you!
(Quiets pylint warnings.)
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210519183951.3946870-9-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Commit: e0e8a0ac2e60fdebd7ff0f831250c849f22af35d
https://github.com/qemu/qemu/commit/e0e8a0ac2e60fdebd7ff0f831250c849f22af35d
Author: John Snow <jsnow@redhat.com>
Date: 2021-05-20 (Thu, 20 May 2021)
Changed paths:
M scripts/qapi/common.py
M scripts/qapi/main.py
M scripts/qapi/parser.py
Log Message:
-----------
qapi: add must_match helper
Mypy cannot generally understand that these regex functions cannot
possibly fail. Add a "must_match" helper that makes this clear for
mypy.
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210519183951.3946870-10-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Commit: c256263f3df0eaf9011405cdaee354380beb6dc5
https://github.com/qemu/qemu/commit/c256263f3df0eaf9011405cdaee354380beb6dc5
Author: John Snow <jsnow@redhat.com>
Date: 2021-05-20 (Thu, 20 May 2021)
Changed paths:
M scripts/qapi/parser.py
M tests/qapi-schema/meson.build
A tests/qapi-schema/missing-array-rsqb.err
A tests/qapi-schema/missing-array-rsqb.json
A tests/qapi-schema/missing-array-rsqb.out
A tests/qapi-schema/missing-object-member-element.err
A tests/qapi-schema/missing-object-member-element.json
A tests/qapi-schema/missing-object-member-element.out
Log Message:
-----------
qapi/parser: Fix token membership tests when token can be None
When the token can be None (EOF), we can't use 'x in "abc"' style
membership tests to group types of tokens together, because 'None in
"abc"' is a TypeError.
Easy enough to fix. (Use a tuple: It's neither a static typing error nor
a runtime error to check for None in Tuple[str, ...])
Add tests to prevent a regression. (Note: they cannot be added prior to
this fix, as the unhandled stack trace will not match test output in the
CI system.)
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210519183951.3946870-11-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Commit: 03386200b90c68953e217baedd3716cdee9ed169
https://github.com/qemu/qemu/commit/03386200b90c68953e217baedd3716cdee9ed169
Author: John Snow <jsnow@redhat.com>
Date: 2021-05-20 (Thu, 20 May 2021)
Changed paths:
M scripts/qapi/parser.py
Log Message:
-----------
qapi/parser: Rework _check_pragma_list_of_str as a TypeGuard
TypeGuards wont exist in Python proper until 3.10. Ah well. We can hack
up our own by declaring this function to return the type we claim it
checks for and using this to safely downcast object -> List[str].
In so doing, I bring this function under _pragma so it can use the
'info' object in its closure. Having done this, _pragma also now no
longer needs to take a 'self' parameter, so drop it.
To help with line-length, and with the context evident from its new
scope, rename the function to the shorter check_list_str().
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210519183951.3946870-12-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Commit: 810aff8f29dedbf4568f36462d2bfc3ef47f11e8
https://github.com/qemu/qemu/commit/810aff8f29dedbf4568f36462d2bfc3ef47f11e8
Author: John Snow <jsnow@redhat.com>
Date: 2021-05-20 (Thu, 20 May 2021)
Changed paths:
M scripts/qapi/parser.py
Log Message:
-----------
qapi/parser: add type hint annotations
Annotations do not change runtime behavior.
This commit *only* adds annotations.
(Annotations for QAPIDoc are in a forthcoming commit.)
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210519183951.3946870-13-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Commit: 013a3aceb5b94710ee686cfa448458b1f170d78c
https://github.com/qemu/qemu/commit/013a3aceb5b94710ee686cfa448458b1f170d78c
Author: John Snow <jsnow@redhat.com>
Date: 2021-05-20 (Thu, 20 May 2021)
Changed paths:
M scripts/qapi/parser.py
Log Message:
-----------
qapi/parser: Remove superfluous list comprehension
A generator suffices (and quiets a pylint warning).
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210519183951.3946870-14-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Commit: 9b91e76b3a2cf8ced6e9ce0d29b556d963409b3f
https://github.com/qemu/qemu/commit/9b91e76b3a2cf8ced6e9ce0d29b556d963409b3f
Author: John Snow <jsnow@redhat.com>
Date: 2021-05-20 (Thu, 20 May 2021)
Changed paths:
M scripts/qapi/pylintrc
Log Message:
-----------
qapi/parser: allow 'ch' variable name
We can have a two-letter variable name, as a treat.
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210519183951.3946870-15-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Commit: d4092ffa2604e07b2e1bb5b1f7b2651bc1edda80
https://github.com/qemu/qemu/commit/d4092ffa2604e07b2e1bb5b1f7b2651bc1edda80
Author: John Snow <jsnow@redhat.com>
Date: 2021-05-20 (Thu, 20 May 2021)
Changed paths:
M scripts/qapi/parser.py
Log Message:
-----------
qapi/parser: add docstrings
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210519183951.3946870-16-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Doc string spacing tweaked slightly]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Commit: 0b5acf89c1b197fc8f36db0896f652a4c577352f
https://github.com/qemu/qemu/commit/0b5acf89c1b197fc8f36db0896f652a4c577352f
Author: Peter Maydell <peter.maydell@linaro.org>
Date: 2021-05-20 (Thu, 20 May 2021)
Changed paths:
M scripts/qapi/common.py
M scripts/qapi/main.py
M scripts/qapi/parser.py
M scripts/qapi/pylintrc
M scripts/qapi/schema.py
M scripts/qapi/source.py
M tests/qapi-schema/meson.build
A tests/qapi-schema/missing-array-rsqb.err
A tests/qapi-schema/missing-array-rsqb.json
A tests/qapi-schema/missing-array-rsqb.out
A tests/qapi-schema/missing-object-member-element.err
A tests/qapi-schema/missing-object-member-element.json
A tests/qapi-schema/missing-object-member-element.out
A tests/qapi-schema/missing-schema.err
A tests/qapi-schema/missing-schema.out
M tests/qapi-schema/non-objects.err
M tests/qapi-schema/quoted-structural-chars.err
M tests/qapi-schema/test-qapi.py
Log Message:
-----------
Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2021-05-20' into
staging
QAPI patches patches for 2021-05-20
# gpg: Signature made Thu 20 May 2021 16:10:21 BST
# gpg: using RSA key 354BC8B3D7EB2A6B68674E5F3870B400EB918653
# gpg: issuer "armbru@redhat.com"
# gpg: Good signature from "Markus Armbruster <armbru@redhat.com>" [full]
# gpg: aka "Markus Armbruster <armbru@pond.sub.org>" [full]
# Primary key fingerprint: 354B C8B3 D7EB 2A6B 6867 4E5F 3870 B400 EB91 8653
* remotes/armbru/tags/pull-qapi-2021-05-20:
qapi/parser: add docstrings
qapi/parser: allow 'ch' variable name
qapi/parser: Remove superfluous list comprehension
qapi/parser: add type hint annotations
qapi/parser: Rework _check_pragma_list_of_str as a TypeGuard
qapi/parser: Fix token membership tests when token can be None
qapi: add must_match helper
qapi/parser: Use @staticmethod where appropriate
qapi/parser: assert object keys are strings
qapi/parser: enforce all top-level expressions must be dict in _parse()
qapi/parser: Assert lexer value is a string
qapi/parser: factor parsing routine into method
qapi/source: Remove line number from QAPISourceInfo initializer
qapi: Add test for nonexistent schema file
qapi/parser: Don't try to handle file errors
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Compare: https://github.com/qemu/qemu/compare/972e848b5397...0b5acf89c1b1