[PATCH 11/20] qapi/schema: add doc_visible property to QAPISchemaDefinition

John Snow posted 20 patches 6 months, 2 weeks ago
Maintainers: Markus Armbruster <armbru@redhat.com>, Michael Roth <michael.roth@amd.com>, Peter Maydell <peter.maydell@linaro.org>, Eric Blake <eblake@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <anisinha@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Yanan Wang <wangyanan55@huawei.com>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, Jason Wang <jasowang@redhat.com>, Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>, Jiri Pirko <jiri@resnulli.us>, Stefan Berger <stefanb@linux.vnet.ibm.com>, Stefan Hajnoczi <stefanha@redhat.com>, Mads Ynddal <mads@ynddal.dk>, Lukas Straub <lukasstraub2@web.de>, Konstantin Kostiuk <kkostiuk@redhat.com>
[PATCH 11/20] qapi/schema: add doc_visible property to QAPISchemaDefinition
Posted by John Snow 6 months, 2 weeks ago
The intent here is to mark only certain definitions as visible in the
end-user docs.

All commands and events are inherently visible. Everything else is
visible only if it is a member (or a branch member) of a type that is
visible, or if it is named as a return type for a command.

Notably, this excludes arg_type for commands and events, and any
base_types specified for structures/unions. Those objects may still be
marked visible if they are named as members from a visible type.

This does not necessarily match the data revealed by introspection: in
this case, we want anything that we are cross-referencing in generated
documentation to be available to target.

Some internal and built-in types may be marked visible with this
approach, but if they do not have a documentation block, they'll be
skipped by the generator anyway. This includes array types and built-in
primitives which do not get their own documentation objects.

This information is not yet used by qapidoc, which continues to render
documentation exactly as it has. This information will be used by the
new qapidoc (the "transmogrifier"), to be introduced later. The new
generator verifies that all of the objects that should be rendered *are*
by failing if any cross-references are missing, verifying everything is
in place.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/schema.py | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index e15e64ea8cb..6025b4e9354 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -131,6 +131,7 @@ def __init__(
         self.doc = doc
         self._ifcond = ifcond or QAPISchemaIfCond()
         self.features = features or []
+        self.doc_visible = False
 
     def __repr__(self) -> str:
         return "<%s:%s at 0x%x>" % (type(self).__name__, self.name,
@@ -146,6 +147,10 @@ def check(self, schema: QAPISchema) -> None:
         for f in self.features:
             f.check_clash(self.info, seen)
 
+    def mark_visible(self, mark_self: bool = True) -> None:
+        if mark_self:
+            self.doc_visible = True
+
     def connect_doc(self, doc: Optional[QAPIDoc] = None) -> None:
         super().connect_doc(doc)
         doc = doc or self.doc
@@ -483,6 +488,10 @@ def check(self, schema: QAPISchema) -> None:
             self.info.defn_meta if self.info else None)
         assert not isinstance(self.element_type, QAPISchemaArrayType)
 
+    def mark_visible(self, mark_self: bool = True) -> None:
+        super().mark_visible(mark_self)
+        self.element_type.mark_visible()
+
     def set_module(self, schema: QAPISchema) -> None:
         self._set_module(schema, self.element_type.info)
 
@@ -607,6 +616,17 @@ def connect_doc(self, doc: Optional[QAPIDoc] = None) -> None:
         for m in self.local_members:
             m.connect_doc(doc)
 
+    def mark_visible(self, mark_self: bool = True) -> None:
+        # Mark this object and its members as visible in the user-facing docs.
+        if self.doc_visible:
+            return
+
+        super().mark_visible(mark_self)
+        for m in self.members:
+            m.type.mark_visible()
+        for var in self.branches or []:
+            var.type.mark_visible(False)
+
     def is_implicit(self) -> bool:
         # See QAPISchema._make_implicit_object_type(), as well as
         # _def_predefineds()
@@ -698,6 +718,11 @@ def check(self, schema: QAPISchema) -> None:
                         % (v.describe(self.info), types_seen[qt]))
                 types_seen[qt] = v.name
 
+    def mark_visible(self, mark_self: bool = True) -> None:
+        super().mark_visible(mark_self)
+        for var in self.alternatives:
+            var.type.mark_visible()
+
     def connect_doc(self, doc: Optional[QAPIDoc] = None) -> None:
         super().connect_doc(doc)
         doc = doc or self.doc
@@ -1056,6 +1081,13 @@ def check(self, schema: QAPISchema) -> None:
                         "command's 'returns' cannot take %s"
                         % self.ret_type.describe())
 
+    def mark_visible(self, mark_self: bool = True) -> None:
+        super().mark_visible(mark_self)
+        if self.arg_type:
+            self.arg_type.mark_visible(False)
+        if self.ret_type:
+            self.ret_type.mark_visible()
+
     def connect_doc(self, doc: Optional[QAPIDoc] = None) -> None:
         super().connect_doc(doc)
         doc = doc or self.doc
@@ -1112,6 +1144,11 @@ def check(self, schema: QAPISchema) -> None:
                     self.info,
                     "conditional event arguments require 'boxed': true")
 
+    def mark_visible(self, mark_self: bool = True) -> None:
+        super().mark_visible(mark_self)
+        if self.arg_type:
+            self.arg_type.mark_visible(False)
+
     def connect_doc(self, doc: Optional[QAPIDoc] = None) -> None:
         super().connect_doc(doc)
         doc = doc or self.doc
@@ -1488,6 +1525,9 @@ def check(self) -> None:
             ent.set_module(self)
         for doc in self.docs:
             doc.check()
+        for ent in self._entity_list:
+            if isinstance(ent, (QAPISchemaCommand, QAPISchemaEvent)):
+                ent.mark_visible()
 
     def visit(self, visitor: QAPISchemaVisitor) -> None:
         visitor.visit_begin(self)
-- 
2.44.0
Re: [PATCH 11/20] qapi/schema: add doc_visible property to QAPISchemaDefinition
Posted by Markus Armbruster 5 months, 1 week ago
John Snow <jsnow@redhat.com> writes:

> The intent here is to mark only certain definitions as visible in the
> end-user docs.
>
> All commands and events are inherently visible. Everything else is
> visible only if it is a member (or a branch member) of a type that is
> visible, or if it is named as a return type for a command.
>
> Notably, this excludes arg_type for commands and events, and any
> base_types specified for structures/unions. Those objects may still be
> marked visible if they are named as members from a visible type.

Why?  I figure the answer is "because the transmogrifier inlines the
things excluded".  Correct?

> This does not necessarily match the data revealed by introspection: in
> this case, we want anything that we are cross-referencing in generated
> documentation to be available to target.

I don't get the part after the colon.

> Some internal and built-in types may be marked visible with this
> approach, but if they do not have a documentation block, they'll be
> skipped by the generator anyway. This includes array types and built-in
> primitives which do not get their own documentation objects.
>
> This information is not yet used by qapidoc, which continues to render
> documentation exactly as it has. This information will be used by the
> new qapidoc (the "transmogrifier"), to be introduced later. The new
> generator verifies that all of the objects that should be rendered *are*
> by failing if any cross-references are missing, verifying everything is
> in place.

So... we decide "doc should be visible" here, and then the
transmogrifier decides again, and we check the two decisions match?

> Signed-off-by: John Snow <jsnow@redhat.com>
Re: [PATCH 11/20] qapi/schema: add doc_visible property to QAPISchemaDefinition
Posted by John Snow 5 months, 1 week ago
On Fri, Jun 14, 2024, 5:40 AM Markus Armbruster <armbru@redhat.com> wrote:

> John Snow <jsnow@redhat.com> writes:
>
> > The intent here is to mark only certain definitions as visible in the
> > end-user docs.
> >
> > All commands and events are inherently visible. Everything else is
> > visible only if it is a member (or a branch member) of a type that is
> > visible, or if it is named as a return type for a command.
> >
> > Notably, this excludes arg_type for commands and events, and any
> > base_types specified for structures/unions. Those objects may still be
> > marked visible if they are named as members from a visible type.
>
> Why?  I figure the answer is "because the transmogrifier inlines the
> things excluded".  Correct?
>

Yes. If a type is only ever used as a branch type or a base type and never
named as a field type, it can be safely omitted from the rendered docs.

(If a type is not named in any fashion by and command or event or recursive
structure therein, it's an internal type not used in QMP at all and can
also be excluded safely.)


> > This does not necessarily match the data revealed by introspection: in
> > this case, we want anything that we are cross-referencing in generated
> > documentation to be available to target.
>
> I don't get the part after the colon.
>

What I mean to say is that anywhere in the generated documentation where we
need to cross-reference a type name should be "doc visible".

This means, currently:

- All commands
- All events
- All member field types (including branch field types) of commands/events
or nested fields therein (i.e. CommandA takes an arg "foo" of type "FooA"
which in turn has a field "bar" of type "BarB"; CommandA, FooA and BarB are
all doc visible types.)
- All return types and nested types therein; identical to the input rules
above (You've expressed a desire to change this. Future work.)
- All alternate types (which are never inlined but instead are referenced.
This can be changed later but it's just not how it works currently.)

but excludes:

- Arg types for events/commands
- Inherited object types
- Any data type that does not appear as a field/member type for a
command/event input/output
- Array/List types: only the primitive types are documented with a section,
not the List/Array container. This is in contrast to Alternates, where we
do not perform a similar destructuring.

So, if you look at the WIP render on gitlab.io, any (type) parenthetical
links to a "qapi object" (Sphinx parlance: an indexable qapi domain
*thingie*) that exists as its own section with a name and docs attached.

Those are the ones that are "doc visible".

This differs from introspection in that some (but not all) of the shared
types get sections in the HTML output, but these are not API and not
inherently stable with regards to their name(s) or factorings.

Introspection by contrast strips more of the names out than we do here.


> > Some internal and built-in types may be marked visible with this
> > approach, but if they do not have a documentation block, they'll be
> > skipped by the generator anyway. This includes array types and built-in
> > primitives which do not get their own documentation objects.
> >
> > This information is not yet used by qapidoc, which continues to render
> > documentation exactly as it has. This information will be used by the
> > new qapidoc (the "transmogrifier"), to be introduced later. The new
> > generator verifies that all of the objects that should be rendered *are*
> > by failing if any cross-references are missing, verifying everything is
> > in place.
>
> So... we decide "doc should be visible" here, and then the
> transmogrifier decides again, and we check the two decisions match?
>

Not quite; in the generator, we visit only entities that *have docs*, so
the true visibility check is (pseudocode):

ent.doc_visible() && ent.has_docs()

but we guarantee this does not cull anything inappropriate because any
hanging reference is a warning that will cause a build failure. This should
not be possible to trigger in the doc builder under normal circumstances.

I just mean to say: "I believe the algorithm in this patch is correct
because the build would not tolerate a hanging reference, and the doc
generator does in fact succeed."

Notably, the doc_visible algorithm may mark some built-in types as
"visible", but they're still excluded from the html render because they
simply have no docs to show. (The reference checker will not yelp for these
built-in types.)


> > Signed-off-by: John Snow <jsnow@redhat.com>
>
>