It simplifies the typing to say that _section is always a
QAPIDoc.Section().
To accommodate this change, we must allow for this object to evaluate to
False for functions like _end_section which behave differently based on
whether or not a Section has been started.
Signed-off-by: John Snow <jsnow@redhat.com>
---
Probably a better fix is to restructure the code to prevent
_end_section() from being called twice in a row, but that seemed like
more effort, but if you have suggestions for a tactical fix, I'm open to
it.
Signed-off-by: John Snow <jsnow@redhat.com>
---
scripts/qapi/parser.py | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index b3a468504fc..71e982bff57 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -456,6 +456,9 @@ def __init__(self, parser, name=None, indent=0):
# the expected indent level of the text of this section
self._indent = indent
+ def __bool__(self) -> bool:
+ return bool(self.name or self.text)
+
def append(self, line):
# Strip leading spaces corresponding to the expected indent level
# Blank lines are always OK.
@@ -722,7 +725,7 @@ def _end_section(self):
raise QAPIParseError(
self._parser,
"empty doc section '%s'" % self._section.name)
- self._section = None
+ self._section = QAPIDoc.Section(self._parser)
def _append_freeform(self, line):
match = re.match(r'(@\S+:)', line)
--
2.30.2
John Snow <jsnow@redhat.com> writes: > It simplifies the typing to say that _section is always a > QAPIDoc.Section(). > > To accommodate this change, we must allow for this object to evaluate to > False for functions like _end_section which behave differently based on > whether or not a Section has been started. > > Signed-off-by: John Snow <jsnow@redhat.com> > > --- > > Probably a better fix is to restructure the code to prevent > _end_section() from being called twice in a row, but that seemed like > more effort, but if you have suggestions for a tactical fix, I'm open to > it. First step is to find out how _end_section() can be called twice in a row. It isn't in all of "make check". Hmm.
On 5/20/21 10:42 AM, Markus Armbruster wrote:
> First step is to find out how _end_section() can be called twice in a
> row. It isn't in all of "make check". Hmm.
Ah, maybe not twice in a *row*. It does seem to be called when we have
an "empty section" sometimes, which arises from stuff like this:
Extension error:
/home/jsnow/src/qemu/docs/../qga/qapi-schema.json:1143:1: ending a
totally empty section
##
# @GuestExec:
# @pid: pid of child process in guest OS
#
# Since: 2.5
##
{ 'struct': 'GuestExec',
'data': { 'pid': 'int'} }
Without the newline there, it seems to get confused. There's a few like
this that could be fixed, but then some of the test cases break too.
No appetite for barking up this tree right now.
Can I fix the commit message and leave the patch in place? Maybe with a
#FIXME comment nearby?
--js
John Snow <jsnow@redhat.com> writes:
> On 5/20/21 10:42 AM, Markus Armbruster wrote:
>> First step is to find out how _end_section() can be called twice in a
>> row. It isn't in all of "make check". Hmm.
>
> Ah, maybe not twice in a *row*. It does seem to be called when we have
> an "empty section" sometimes, which arises from stuff like this:
>
> Extension error:
> /home/jsnow/src/qemu/docs/../qga/qapi-schema.json:1143:1: ending a
> totally empty section
>
> ##
> # @GuestExec:
> # @pid: pid of child process in guest OS
> #
> # Since: 2.5
> ##
> { 'struct': 'GuestExec',
> 'data': { 'pid': 'int'} }
>
> Without the newline there, it seems to get confused. There's a few
> like this that could be fixed, but then some of the test cases break
> too.
I still can't see it. I tried the obvious
diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index f03ba2cfec..263aeb5fc5 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -716,6 +716,7 @@ def _start_section(self, name=None, indent=0):
self.sections.append(self._section)
def _end_section(self):
+ assert self._section
if self._section:
text = self._section.text = self._section.text.strip()
if self._section.name and (not text or text.isspace()):
Does not fire for qga/qapi-schema.json. Can you help?
> No appetite for barking up this tree right now.
>
> Can I fix the commit message and leave the patch in place? Maybe with
> a #FIXME comment nearby?
I'd like to understand your analysis before I answer your question.
On 5/21/21 1:35 AM, Markus Armbruster wrote:
> Does not fire for qga/qapi-schema.json. Can you help?
Odd.
I did:
if self._section:
...
else:
raise QAPIWhicheverErrorItWas(...)
and then did a full build and found it to fail on QGA stuff. You may
need --enable-docs to make it happen.
It later failed on test cases, too.
--js
John Snow <jsnow@redhat.com> writes:
> On 5/21/21 1:35 AM, Markus Armbruster wrote:
>> Does not fire for qga/qapi-schema.json. Can you help?
>
> Odd.
>
> I did:
>
> if self._section:
> ...
> else:
> raise QAPIWhicheverErrorItWas(...)
>
> and then did a full build and found it to fail on QGA stuff. You may
> need --enable-docs to make it happen.
>
> It later failed on test cases, too.
PEBKAC: I tested with a --disable-docs tree. Disabled, because the
conversion to reST restored the "touch anything, rebuild everything" for
docs, which slows me down too much when I mess with the schema.
This snippet triggers the error:
##
# @GuestExec:
# @pid: pid of child process in guest OS
#
# Since: 2.5
##
{ 'struct': 'GuestExec',
'data': { 'pid': 'int'} }
This one doesn't:
##
# @GuestExec:
#
# @pid: pid of child process in guest OS
#
# Since: 2.5
##
{ 'struct': 'GuestExec',
'data': { 'pid': 'int'} }
The code dealing with sections is pretty impenetrable.
On 6/11/21 10:40 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
>
>> On 5/21/21 1:35 AM, Markus Armbruster wrote:
>>> Does not fire for qga/qapi-schema.json. Can you help?
>>
>> Odd.
>>
>> I did:
>>
>> if self._section:
>> ...
>> else:
>> raise QAPIWhicheverErrorItWas(...)
>>
>> and then did a full build and found it to fail on QGA stuff. You may
>> need --enable-docs to make it happen.
>>
>> It later failed on test cases, too.
>
> PEBKAC: I tested with a --disable-docs tree. Disabled, because the
> conversion to reST restored the "touch anything, rebuild everything" for
> docs, which slows me down too much when I mess with the schema.
>
> This snippet triggers the error:
>
> ##
> # @GuestExec:
> # @pid: pid of child process in guest OS
> #
> # Since: 2.5
> ##
> { 'struct': 'GuestExec',
> 'data': { 'pid': 'int'} }
>
> This one doesn't:
>
> ##
> # @GuestExec:
> #
> # @pid: pid of child process in guest OS
> #
> # Since: 2.5
> ##
> { 'struct': 'GuestExec',
> 'data': { 'pid': 'int'} }
>
> The code dealing with sections is pretty impenetrable.
>
Yeah, that's what I thought too. I might need (or want?) to touch this
soon to do the cross-reference Sphinx stuff, so I figured I'd be coming
back here "soon".
I could make a gitlab issue for me to track to remind myself to come
back to it if you think that's acceptable.
--js
© 2016 - 2026 Red Hat, Inc.