This method adds the options/preamble to each definition block. Notably,
:since: and :ifcond: are added, as are any "special features" such as
:deprecated: and :unstable:.
Signed-off-by: John Snow <jsnow@redhat.com>
---
docs/sphinx/qapidoc.py | 33 ++++++++++++++++++++++++++++++++-
1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
index 6f8f69077b1..85c7ce94564 100644
--- a/docs/sphinx/qapidoc.py
+++ b/docs/sphinx/qapidoc.py
@@ -38,7 +38,7 @@
from qapi.error import QAPIError, QAPISemError
from qapi.gen import QAPISchemaVisitor
from qapi.parser import QAPIDoc
-from qapi.schema import QAPISchema
+from qapi.schema import QAPISchema, QAPISchemaEntity
from qapi.source import QAPISourceInfo
from sphinx import addnodes
@@ -125,6 +125,37 @@ def ensure_blank_line(self) -> None:
# +2: correct for zero/one index, then increment by one.
self.add_line_raw("", fname, line + 2)
+ # Transmogrification helpers
+
+ def preamble(self, ent: QAPISchemaEntity) -> None:
+ """
+ Generate option lines for qapi entity directives.
+ """
+ if ent.doc and ent.doc.since:
+ assert ent.doc.since.tag == QAPIDoc.Tag.SINCE
+ # Generated from the entity's docblock; info location is exact.
+ self.add_line(f":since: {ent.doc.since.text}", ent.doc.since.info)
+
+ if ent.ifcond.is_present():
+ doc = ent.ifcond.docgen()
+ # Generated from entity definition; info location is approximate.
+ self.add_line(f":ifcond: {doc}", ent.info)
+
+ # Hoist special features such as :deprecated: and :unstable:
+ # into the options block for the entity. If, in the future, new
+ # special features are added, qapi-domain will chirp about
+ # unrecognized options and fail.
+ for feat in ent.features:
+ if feat.is_special():
+ # We don't expect special features to have an ifcond property.
+ # (Hello, intrepid developer in the future who changed that!)
+ # ((With luck, you are not me.))
+ assert not feat.ifcond.is_present()
+ # Generated from entity def; info location is approximate.
+ self.add_line(f":{feat.name}:", feat.info)
+
+ self.ensure_blank_line()
+
# Transmogrification core methods
def visit_module(self, path: str) -> None:
--
2.47.0John Snow <jsnow@redhat.com> writes:
> This method adds the options/preamble to each definition block. Notably,
> :since: and :ifcond: are added, as are any "special features" such as
> :deprecated: and :unstable:.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> docs/sphinx/qapidoc.py | 33 ++++++++++++++++++++++++++++++++-
> 1 file changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
> index 6f8f69077b1..85c7ce94564 100644
> --- a/docs/sphinx/qapidoc.py
> +++ b/docs/sphinx/qapidoc.py
> @@ -38,7 +38,7 @@
> from qapi.error import QAPIError, QAPISemError
> from qapi.gen import QAPISchemaVisitor
> from qapi.parser import QAPIDoc
> -from qapi.schema import QAPISchema
> +from qapi.schema import QAPISchema, QAPISchemaEntity
> from qapi.source import QAPISourceInfo
>
> from sphinx import addnodes
> @@ -125,6 +125,37 @@ def ensure_blank_line(self) -> None:
> # +2: correct for zero/one index, then increment by one.
> self.add_line_raw("", fname, line + 2)
>
> + # Transmogrification helpers
> +
> + def preamble(self, ent: QAPISchemaEntity) -> None:
> + """
> + Generate option lines for qapi entity directives.
> + """
> + if ent.doc and ent.doc.since:
> + assert ent.doc.since.tag == QAPIDoc.Tag.SINCE
> + # Generated from the entity's docblock; info location is exact.
> + self.add_line(f":since: {ent.doc.since.text}", ent.doc.since.info)
> +
> + if ent.ifcond.is_present():
> + doc = ent.ifcond.docgen()
> + # Generated from entity definition; info location is approximate.
> + self.add_line(f":ifcond: {doc}", ent.info)
> +
> + # Hoist special features such as :deprecated: and :unstable:
> + # into the options block for the entity. If, in the future, new
> + # special features are added, qapi-domain will chirp about
> + # unrecognized options and fail.
> + for feat in ent.features:
> + if feat.is_special():
> + # We don't expect special features to have an ifcond property.
> + # (Hello, intrepid developer in the future who changed that!)
> + # ((With luck, you are not me.))
> + assert not feat.ifcond.is_present()
Nope :)
The attempt to add a conditional special feature now fails with
Sphinx parallel build error:
AssertionError
If you want to outlaw conditional special features, reject them cleanly
in schema.py, document the restriction in docs/devel/qapi-code-gen.rst,
and explain why in the commit message. Recommend a separate commit, to
make it stand out in git-log.
> + # Generated from entity def; info location is approximate.
> + self.add_line(f":{feat.name}:", feat.info)
> +
> + self.ensure_blank_line()
> +
> # Transmogrification core methods
>
> def visit_module(self, path: str) -> None:
On Fri, Dec 20, 2024 at 9:15 AM Markus Armbruster <armbru@redhat.com> wrote:
> John Snow <jsnow@redhat.com> writes:
>
> > This method adds the options/preamble to each definition block. Notably,
> > :since: and :ifcond: are added, as are any "special features" such as
> > :deprecated: and :unstable:.
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> > docs/sphinx/qapidoc.py | 33 ++++++++++++++++++++++++++++++++-
> > 1 file changed, 32 insertions(+), 1 deletion(-)
> >
> > diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
> > index 6f8f69077b1..85c7ce94564 100644
> > --- a/docs/sphinx/qapidoc.py
> > +++ b/docs/sphinx/qapidoc.py
> > @@ -38,7 +38,7 @@
> > from qapi.error import QAPIError, QAPISemError
> > from qapi.gen import QAPISchemaVisitor
> > from qapi.parser import QAPIDoc
> > -from qapi.schema import QAPISchema
> > +from qapi.schema import QAPISchema, QAPISchemaEntity
> > from qapi.source import QAPISourceInfo
> >
> > from sphinx import addnodes
> > @@ -125,6 +125,37 @@ def ensure_blank_line(self) -> None:
> > # +2: correct for zero/one index, then increment by one.
> > self.add_line_raw("", fname, line + 2)
> >
> > + # Transmogrification helpers
> > +
> > + def preamble(self, ent: QAPISchemaEntity) -> None:
> > + """
> > + Generate option lines for qapi entity directives.
> > + """
> > + if ent.doc and ent.doc.since:
> > + assert ent.doc.since.tag == QAPIDoc.Tag.SINCE
> > + # Generated from the entity's docblock; info location is
> exact.
> > + self.add_line(f":since: {ent.doc.since.text}",
> ent.doc.since.info)
> > +
> > + if ent.ifcond.is_present():
> > + doc = ent.ifcond.docgen()
> > + # Generated from entity definition; info location is
> approximate.
> > + self.add_line(f":ifcond: {doc}", ent.info)
> > +
> > + # Hoist special features such as :deprecated: and :unstable:
> > + # into the options block for the entity. If, in the future, new
> > + # special features are added, qapi-domain will chirp about
> > + # unrecognized options and fail.
> > + for feat in ent.features:
> > + if feat.is_special():
> > + # We don't expect special features to have an ifcond
> property.
> > + # (Hello, intrepid developer in the future who changed
> that!)
> > + # ((With luck, you are not me.))
> > + assert not feat.ifcond.is_present()
>
> Nope :)
>
> The attempt to add a conditional special feature now fails with
>
> Sphinx parallel build error:
> AssertionError
>
> If you want to outlaw conditional special features, reject them cleanly
> in schema.py, document the restriction in docs/devel/qapi-code-gen.rst,
> and explain why in the commit message. Recommend a separate commit, to
> make it stand out in git-log.
>
Do you advocate this? I wasn't sure what it *meant* for a special feature
to be conditional; I couldn't conceive of what it meant to have an ifcond
for "deprecated" or "unstable", for instance. It sounds like it isn't well
defined, but we happen to not expressly forbid it.
I guard against it here because, similarly, I have no idea how to handle
the case where it's true.
I didn't realize we technically allow it, though ... would you like me to
move to expressly forbid it in the parser? (Failing that, I have no idea
how to display this information otherwise, so I'd need you to sketch
something out for me; so my inclination is to forbid it as you suggest.
Future developers can always lift the restriction once they have some
use-case in mind and a plan for how to display that information.)
--js
>
> > + # Generated from entity def; info location is
> approximate.
> > + self.add_line(f":{feat.name}:", feat.info)
> > +
> > + self.ensure_blank_line()
> > +
> > # Transmogrification core methods
> >
> > def visit_module(self, path: str) -> None:
>
>
John Snow <jsnow@redhat.com> writes:
> On Fri, Dec 20, 2024 at 9:15 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> John Snow <jsnow@redhat.com> writes:
>>
>> > This method adds the options/preamble to each definition block. Notably,
>> > :since: and :ifcond: are added, as are any "special features" such as
>> > :deprecated: and :unstable:.
>> >
>> > Signed-off-by: John Snow <jsnow@redhat.com>
>> > ---
>> > docs/sphinx/qapidoc.py | 33 ++++++++++++++++++++++++++++++++-
>> > 1 file changed, 32 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
>> > index 6f8f69077b1..85c7ce94564 100644
>> > --- a/docs/sphinx/qapidoc.py
>> > +++ b/docs/sphinx/qapidoc.py
>> > @@ -38,7 +38,7 @@
>> > from qapi.error import QAPIError, QAPISemError
>> > from qapi.gen import QAPISchemaVisitor
>> > from qapi.parser import QAPIDoc
>> > -from qapi.schema import QAPISchema
>> > +from qapi.schema import QAPISchema, QAPISchemaEntity
>> > from qapi.source import QAPISourceInfo
>> >
>> > from sphinx import addnodes
>> > @@ -125,6 +125,37 @@ def ensure_blank_line(self) -> None:
>> > # +2: correct for zero/one index, then increment by one.
>> > self.add_line_raw("", fname, line + 2)
>> >
>> > + # Transmogrification helpers
>> > +
>> > + def preamble(self, ent: QAPISchemaEntity) -> None:
>> > + """
>> > + Generate option lines for qapi entity directives.
>> > + """
>> > + if ent.doc and ent.doc.since:
>> > + assert ent.doc.since.tag == QAPIDoc.Tag.SINCE
>> > + # Generated from the entity's docblock; info location is exact.
>> > + self.add_line(f":since: {ent.doc.since.text}", ent.doc.since.info)
>> > +
>> > + if ent.ifcond.is_present():
>> > + doc = ent.ifcond.docgen()
>> > + # Generated from entity definition; info location is approximate.
>> > + self.add_line(f":ifcond: {doc}", ent.info)
>> > +
>> > + # Hoist special features such as :deprecated: and :unstable:
>> > + # into the options block for the entity. If, in the future, new
>> > + # special features are added, qapi-domain will chirp about
>> > + # unrecognized options and fail.
>> > + for feat in ent.features:
>> > + if feat.is_special():
>> > + # We don't expect special features to have an ifcond property.
>> > + # (Hello, intrepid developer in the future who changed that!)
>> > + # ((With luck, you are not me.))
>> > + assert not feat.ifcond.is_present()
>>
>> Nope :)
>>
>> The attempt to add a conditional special feature now fails with
>>
>> Sphinx parallel build error:
>> AssertionError
>>
>> If you want to outlaw conditional special features, reject them cleanly
>> in schema.py, document the restriction in docs/devel/qapi-code-gen.rst,
>> and explain why in the commit message. Recommend a separate commit, to
>> make it stand out in git-log.
>
> Do you advocate this? I wasn't sure what it *meant* for a special feature
> to be conditional; I couldn't conceive of what it meant to have an ifcond
> for "deprecated" or "unstable", for instance. It sounds like it isn't well
> defined, but we happen to not expressly forbid it.
Semantics are clear enough to me.
"Conditional special feature" combines "conditional feature" (which is a
special case of conditional thing) with special feature (which is a
special case of feature).
The QAPI schema language supports compile-time conditionals for certain
things. Generated code behaves as if the thing didn't exist unless its
condition is true.
QAPI schema features are strings exposed to clients in introspection.
Combine the two: a conditional feature is exposed if and only if its
condition is true.
Existing uses: dynamic-auto-read-only if CONFIG_POSIX, fdset if
CONFIG_BLKIO_VHOST_VDPA_FD.
A special feature is a feature that has special meaning to the
generator, i.e. we generate different code in places when it's present.
Combine: we enable different code for a conditional special feature only
when its condition is true.
No existing uses so far.
Implementation is straightforward, too.
Any code we generate for a conditional thing is guarded by #if
... #endif.
For features, we generate suitable data into qapi-introspect.c.
For special features, we generate a little additional code here and
there; look for .is_special() to find it.
Bug: this additional code lacks #if ... #endif. Simple oversight,
should be easy enough to fix.
> I guard against it here because, similarly, I have no idea how to handle
> the case where it's true.
>
> I didn't realize we technically allow it, though ... would you like me to
> move to expressly forbid it in the parser? (Failing that, I have no idea
> how to display this information otherwise, so I'd need you to sketch
> something out for me; so my inclination is to forbid it as you suggest.
> Future developers can always lift the restriction once they have some
> use-case in mind and a plan for how to display that information.)
I think we should first make a reasonable effort at figuring out how to
handle conditional special features. If we fail, we can add the
restriction instead.
How do you handle features in general, and special features in
particular?
How do you handle conditionals in general?
How do you combine feature and conditional?
How could you combine special feature and conditonal?
[...]
On Thu, Jan 9, 2025, 5:34 AM Markus Armbruster <armbru@redhat.com> wrote:
> John Snow <jsnow@redhat.com> writes:
>
> > On Fri, Dec 20, 2024 at 9:15 AM Markus Armbruster <armbru@redhat.com>
> wrote:
> >
> >> John Snow <jsnow@redhat.com> writes:
> >>
> >> > This method adds the options/preamble to each definition block.
> Notably,
> >> > :since: and :ifcond: are added, as are any "special features" such as
> >> > :deprecated: and :unstable:.
> >> >
> >> > Signed-off-by: John Snow <jsnow@redhat.com>
> >> > ---
> >> > docs/sphinx/qapidoc.py | 33 ++++++++++++++++++++++++++++++++-
> >> > 1 file changed, 32 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
> >> > index 6f8f69077b1..85c7ce94564 100644
> >> > --- a/docs/sphinx/qapidoc.py
> >> > +++ b/docs/sphinx/qapidoc.py
> >> > @@ -38,7 +38,7 @@
> >> > from qapi.error import QAPIError, QAPISemError
> >> > from qapi.gen import QAPISchemaVisitor
> >> > from qapi.parser import QAPIDoc
> >> > -from qapi.schema import QAPISchema
> >> > +from qapi.schema import QAPISchema, QAPISchemaEntity
> >> > from qapi.source import QAPISourceInfo
> >> >
> >> > from sphinx import addnodes
> >> > @@ -125,6 +125,37 @@ def ensure_blank_line(self) -> None:
> >> > # +2: correct for zero/one index, then increment by one.
> >> > self.add_line_raw("", fname, line + 2)
> >> >
> >> > + # Transmogrification helpers
> >> > +
> >> > + def preamble(self, ent: QAPISchemaEntity) -> None:
> >> > + """
> >> > + Generate option lines for qapi entity directives.
> >> > + """
> >> > + if ent.doc and ent.doc.since:
> >> > + assert ent.doc.since.tag == QAPIDoc.Tag.SINCE
> >> > + # Generated from the entity's docblock; info location is
> exact.
> >> > + self.add_line(f":since: {ent.doc.since.text}",
> ent.doc.since.info)
> >> > +
> >> > + if ent.ifcond.is_present():
> >> > + doc = ent.ifcond.docgen()
> >> > + # Generated from entity definition; info location is
> approximate.
> >> > + self.add_line(f":ifcond: {doc}", ent.info)
> >> > +
> >> > + # Hoist special features such as :deprecated: and :unstable:
> >> > + # into the options block for the entity. If, in the future,
> new
> >> > + # special features are added, qapi-domain will chirp about
> >> > + # unrecognized options and fail.
> >> > + for feat in ent.features:
> >> > + if feat.is_special():
> >> > + # We don't expect special features to have an ifcond
> property.
> >> > + # (Hello, intrepid developer in the future who
> changed that!)
> >> > + # ((With luck, you are not me.))
> >> > + assert not feat.ifcond.is_present()
> >>
> >> Nope :)
> >>
> >> The attempt to add a conditional special feature now fails with
> >>
> >> Sphinx parallel build error:
> >> AssertionError
> >>
> >> If you want to outlaw conditional special features, reject them cleanly
> >> in schema.py, document the restriction in docs/devel/qapi-code-gen.rst,
> >> and explain why in the commit message. Recommend a separate commit, to
> >> make it stand out in git-log.
> >
> > Do you advocate this? I wasn't sure what it *meant* for a special feature
> > to be conditional; I couldn't conceive of what it meant to have an ifcond
> > for "deprecated" or "unstable", for instance. It sounds like it isn't
> well
> > defined, but we happen to not expressly forbid it.
>
> Semantics are clear enough to me.
>
> "Conditional special feature" combines "conditional feature" (which is a
> special case of conditional thing) with special feature (which is a
> special case of feature).
>
> The QAPI schema language supports compile-time conditionals for certain
> things. Generated code behaves as if the thing didn't exist unless its
> condition is true.
>
> QAPI schema features are strings exposed to clients in introspection.
>
> Combine the two: a conditional feature is exposed if and only if its
> condition is true.
>
> Existing uses: dynamic-auto-read-only if CONFIG_POSIX, fdset if
> CONFIG_BLKIO_VHOST_VDPA_FD.
>
> A special feature is a feature that has special meaning to the
> generator, i.e. we generate different code in places when it's present.
>
> Combine: we enable different code for a conditional special feature only
> when its condition is true.
>
> No existing uses so far.
>
> Implementation is straightforward, too.
>
Mechanically, it's well defined... but that isn't my concern.
What I asked you was: "what would it mean for deprecated to be conditional?"
Without understanding that, I have no chance to design a system that
handles that information accordingly for the documentation.
> Any code we generate for a conditional thing is guarded by #if
> ... #endif.
>
> For features, we generate suitable data into qapi-introspect.c.
>
> For special features, we generate a little additional code here and
> there; look for .is_special() to find it.
>
> Bug: this additional code lacks #if ... #endif. Simple oversight,
> should be easy enough to fix.
>
> > I guard against it here because, similarly, I have no idea how to handle
> > the case where it's true.
> >
> > I didn't realize we technically allow it, though ... would you like me to
> > move to expressly forbid it in the parser? (Failing that, I have no idea
> > how to display this information otherwise, so I'd need you to sketch
> > something out for me; so my inclination is to forbid it as you suggest.
> > Future developers can always lift the restriction once they have some
> > use-case in mind and a plan for how to display that information.)
>
> I think we should first make a reasonable effort at figuring out how to
> handle conditional special features. If we fail, we can add the
> restriction instead.
>
I don't think I agree; I don't think it's worth spending time defining a
feature we don't use in our minds to then design around a hypothetical
future use.
Easier to admit we don't use it and save defining the semantics for the
future developer who stumbles across a reason for needing it.
> How do you handle features in general, and special features in
> particular?
>
Features: they go in the :feat: field list.
Special features: they go in the :feat: field list, and also receive a
special option flag to the directive to allow for special rendering of the
"signature bar" (the header line for a given entity definition in the
rendered documentation.)
e.g.
@deprecated will generate two things:
1) the definition block will get a :deprecated: option, which enables the
html renderer to (potentially) do special things like color the entry
differently, add a "warning pip"/eyecatch, etc. It's scoped to the whole
definition.
2) A :feat deprecated: line which is added to the features "field list" as
per normal for features.
> How do you handle conditionals in general?
>
In this series, I don't!
...except for top-level conditionals, which are currently passed as-is as
an argument to the :ifcond: directive option. The format of that argument
and what the HTML renderer actually does with it is currently TBD.
(In current prototypes I just print the conditional string in the signature
bar.)
> How do you combine feature and conditional?
>
Notably TBD. Unhandled both in this series as-is *and* in my larger WIP.
> How could you combine special feature and conditonal?
>
Bewilderingly uncertain.
Mechanically, it *could* be handled the same way :ifcond: directive options
are.
e.g.
```
.. qapi:command:: x-beware-of-the-leopard
:deprecated: {ifcond stuff here}
:unstable: {ifcond stuff here}
lorem ipsum dolor sit amet
:feat deprecated: blah blah (if: ...)
:feat unstable: blah blah (if: ...)
```
Semantically and in the rendered output, I have absolutely no clue
whatsoever; and doubt I could figure it out without a use case in front of
me to design around ... so I'd prefer to put up a traffic barrier for now.
i.o.w. the design issue arises specifically because special features are
semanticaly special and are "hoisted" to be block-level in the
transmogrified rST doc, and so syntax needs to be designed, implemented and
tested if they are to be conditional.
Yet, we have no such cases to facilitate the design, implementation and
testing.
So, I'd like to prohibit until such time as we have such a case.
> [...]
>
>
John Snow <jsnow@redhat.com> writes:
> On Thu, Jan 9, 2025, 5:34 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> John Snow <jsnow@redhat.com> writes:
>>
>> > On Fri, Dec 20, 2024 at 9:15 AM Markus Armbruster <armbru@redhat.com> wrote:
>> >
>> >> John Snow <jsnow@redhat.com> writes:
>> >>
>> >> > This method adds the options/preamble to each definition block. Notably,
>> >> > :since: and :ifcond: are added, as are any "special features" such as
>> >> > :deprecated: and :unstable:.
>> >> >
>> >> > Signed-off-by: John Snow <jsnow@redhat.com>
>> >> > ---
>> >> > docs/sphinx/qapidoc.py | 33 ++++++++++++++++++++++++++++++++-
>> >> > 1 file changed, 32 insertions(+), 1 deletion(-)
>> >> >
>> >> > diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
>> >> > index 6f8f69077b1..85c7ce94564 100644
>> >> > --- a/docs/sphinx/qapidoc.py
>> >> > +++ b/docs/sphinx/qapidoc.py
>> >> > @@ -38,7 +38,7 @@
>> >> > from qapi.error import QAPIError, QAPISemError
>> >> > from qapi.gen import QAPISchemaVisitor
>> >> > from qapi.parser import QAPIDoc
>> >> > -from qapi.schema import QAPISchema
>> >> > +from qapi.schema import QAPISchema, QAPISchemaEntity
>> >> > from qapi.source import QAPISourceInfo
>> >> >
>> >> > from sphinx import addnodes
>> >> > @@ -125,6 +125,37 @@ def ensure_blank_line(self) -> None:
>> >> > # +2: correct for zero/one index, then increment by one.
>> >> > self.add_line_raw("", fname, line + 2)
>> >> >
>> >> > + # Transmogrification helpers
>> >> > +
>> >> > + def preamble(self, ent: QAPISchemaEntity) -> None:
>> >> > + """
>> >> > + Generate option lines for qapi entity directives.
>> >> > + """
>> >> > + if ent.doc and ent.doc.since:
>> >> > + assert ent.doc.since.tag == QAPIDoc.Tag.SINCE
>> >> > + # Generated from the entity's docblock; info location is exact.
>> >> > + self.add_line(f":since: {ent.doc.since.text}", ent.doc.since.info)
>> >> > +
>> >> > + if ent.ifcond.is_present():
>> >> > + doc = ent.ifcond.docgen()
>> >> > + # Generated from entity definition; info location is approximate.
>> >> > + self.add_line(f":ifcond: {doc}", ent.info)
>> >> > +
>> >> > + # Hoist special features such as :deprecated: and :unstable:
>> >> > + # into the options block for the entity. If, in the future, new
>> >> > + # special features are added, qapi-domain will chirp about
>> >> > + # unrecognized options and fail.
>> >> > + for feat in ent.features:
>> >> > + if feat.is_special():
>> >> > + # We don't expect special features to have an ifcond property.
>> >> > + # (Hello, intrepid developer in the future who changed that!)
>> >> > + # ((With luck, you are not me.))
>> >> > + assert not feat.ifcond.is_present()
>> >>
>> >> Nope :)
>> >>
>> >> The attempt to add a conditional special feature now fails with
>> >>
>> >> Sphinx parallel build error:
>> >> AssertionError
>> >>
>> >> If you want to outlaw conditional special features, reject them cleanly
>> >> in schema.py, document the restriction in docs/devel/qapi-code-gen.rst,
>> >> and explain why in the commit message. Recommend a separate commit, to
>> >> make it stand out in git-log.
>> >
>> > Do you advocate this? I wasn't sure what it *meant* for a special feature
>> > to be conditional; I couldn't conceive of what it meant to have an ifcond
>> > for "deprecated" or "unstable", for instance. It sounds like it isn't well
>> > defined, but we happen to not expressly forbid it.
>>
>> Semantics are clear enough to me.
>>
>> "Conditional special feature" combines "conditional feature" (which is a
>> special case of conditional thing) with special feature (which is a
>> special case of feature).
>>
>> The QAPI schema language supports compile-time conditionals for certain
>> things. Generated code behaves as if the thing didn't exist unless its
>> condition is true.
>>
>> QAPI schema features are strings exposed to clients in introspection.
>>
>> Combine the two: a conditional feature is exposed if and only if its
>> condition is true.
>>
>> Existing uses: dynamic-auto-read-only if CONFIG_POSIX, fdset if
>> CONFIG_BLKIO_VHOST_VDPA_FD.
>>
>> A special feature is a feature that has special meaning to the
>> generator, i.e. we generate different code in places when it's present.
>>
>> Combine: we enable different code for a conditional special feature only
>> when its condition is true.
>>
>> No existing uses so far.
>>
>> Implementation is straightforward, too.
>>
>
> Mechanically, it's well defined... but that isn't my concern.
>
> What I asked you was: "what would it mean for deprecated to be conditional?"
>
> Without understanding that, I have no chance to design a system that
> handles that information accordingly for the documentation.
Special feature deprecated means the use of the thing that has it is
deprecated with this particular qemu-system-FOO.
Why "this particular"? Because it wasn't deprecated in some prior
version of QEMU. Features (special or not) are always, always about the
specific qemu-system-FOO binary you're talking to via QMP.
Conditionals are compile-time schema configuration. Either the
condition is true or not at compile time. If it's true, the thing that
has it is there, else it isn't.
Now combine the two. If the condition of special feature 'deprecate' is
true, the thing that has it is deprecated with this particular
qemu-system-FOO. Else it isn't.
>> Any code we generate for a conditional thing is guarded by #if
>> ... #endif.
>>
>> For features, we generate suitable data into qapi-introspect.c.
>>
>> For special features, we generate a little additional code here and
>> there; look for .is_special() to find it.
>>
>> Bug: this additional code lacks #if ... #endif. Simple oversight,
>> should be easy enough to fix.
>>
>> > I guard against it here because, similarly, I have no idea how to handle
>> > the case where it's true.
>> >
>> > I didn't realize we technically allow it, though ... would you like me to
>> > move to expressly forbid it in the parser? (Failing that, I have no idea
>> > how to display this information otherwise, so I'd need you to sketch
>> > something out for me; so my inclination is to forbid it as you suggest.
>> > Future developers can always lift the restriction once they have some
>> > use-case in mind and a plan for how to display that information.)
>>
>> I think we should first make a reasonable effort at figuring out how to
>> handle conditional special features. If we fail, we can add the
>> restriction instead.
>
> I don't think I agree; I don't think it's worth spending time defining a
> feature we don't use in our minds to then design around a hypothetical
> future use.
>
> Easier to admit we don't use it and save defining the semantics for the
> future developer who stumbles across a reason for needing it.
Semantics of "apply conditional to feature" don't need defining, because
they are simply the same as "apply conditional to any other thing."
When you have a definition for the general case that works for all the
special cases, then outlawing one special case *increases* complexity.
That's why I'm reluctant.
I figure your actual problem more concrete than "what does it even
mean?", namely how to present conditionals in generated documentation.
In that context, features *may* differ from any other thing!
Aside: the (mostly generated) QMP introspection is about this particular
qemu-system-FOO, like all of QMP, whereas the (mostly generated) QMP
documentation is about any qemu-system-FOO that could be built from this
particular working tree. May lead to doc generation problems that have
no match elsewhere.
>> How do you handle features in general, and special features in
>> particular?
>>
>
> Features: they go in the :feat: field list.
> Special features: they go in the :feat: field list, and also receive a
> special option flag to the directive to allow for special rendering of the
> "signature bar" (the header line for a given entity definition in the
> rendered documentation.)
>
> e.g.
>
> @deprecated will generate two things:
>
> 1) the definition block will get a :deprecated: option, which enables the
> html renderer to (potentially) do special things like color the entry
> differently, add a "warning pip"/eyecatch, etc. It's scoped to the whole
> definition.
>
> 2) A :feat deprecated: line which is added to the features "field list" as
> per normal for features.
>
>
>> How do you handle conditionals in general?
>>
>
> In this series, I don't!
>
> ...except for top-level conditionals, which are currently passed as-is as
> an argument to the :ifcond: directive option. The format of that argument
> and what the HTML renderer actually does with it is currently TBD.
>
> (In current prototypes I just print the conditional string in the signature
> bar.)
>
>
>> How do you combine feature and conditional?
>>
>
> Notably TBD. Unhandled both in this series as-is *and* in my larger WIP.
How do you combine definition and conditional? You answered that right
above: largely TBD.
How do you combine enum value / object type member / command argument /
event argument and conditional? Also answered right above: not yet.
What does that mean for the doc generator code? Does it simply ignore
conditionals there?
>> How could you combine special feature and conditonal?
>>
>
> Bewilderingly uncertain.
>
> Mechanically, it *could* be handled the same way :ifcond: directive options
> are.
>
> e.g.
>
> ```
> .. qapi:command:: x-beware-of-the-leopard
> :deprecated: {ifcond stuff here}
> :unstable: {ifcond stuff here}
>
> lorem ipsum dolor sit amet
>
> :feat deprecated: blah blah (if: ...)
> :feat unstable: blah blah (if: ...)
> ```
>
> Semantically and in the rendered output, I have absolutely no clue
> whatsoever; and doubt I could figure it out without a use case in front of
> me to design around ... so I'd prefer to put up a traffic barrier for now.
>
> i.o.w. the design issue arises specifically because special features are
> semanticaly special and are "hoisted" to be block-level in the
> transmogrified rST doc, and so syntax needs to be designed, implemented and
> tested if they are to be conditional.
You choose to make them special in the documentation. They haven't been
special there so far. I'm not telling you not to, I'm just pointing out
this is a problem we elect to have :)
> Yet, we have no such cases to facilitate the design, implementation and
> testing.
>
> So, I'd like to prohibit until such time as we have such a case.
I believe what you need isn't so much an explanation of semantics, it's
use cases. And that's fair!
Let me offer two.
1. Imagine we're trying to develop something big & complex enough to
make keeping it out of tree until done impractical. We feel even an
in-tree branch would be impractical. Instead, we commit it to
master, default off, configure --enable-complex-thing to get it. Not
exactly something we do all the time, but not outlandish, either.
Further imagine that the big & complex thing will involve some new
QMP commands replacing existing ones. As usual, we want to mark the
ones being replaced deprecated, so we can remove them after a grace
period.
But simply deprecating them in the schema would be premature! The
big & complex thing may fail, and if it does, we rip it out. If it
succeeds, we enable it unconditionally.
We can express what we're doing by making feature deprecated
conditional on CONFIG_COMPLEX_THING, which is defined by
--enable-complex-thing.
That way, the complex thing doesn't affect the QMP interface unless
we enable it. Good. And if we enable it, we do get the deprecation,
and can test management applications cope with it.
2. Imagine we turned allow-preconfig into a special feature. Further
imagine some whole-stack feature requires a certain command to have
allow-preconfig, and you got tasked with implementing it. Which you
duly did, to everybody's satisfaction. It is now days to the
release, rc4 has sailed, and some dude comes out of the woodwork to
report the command crashes in preconfig state. Weird! It worked
*fine* in your meticulous testing. After some back and forth, you
discover that the reporter builds with --enable-exotic-crap, which
you didn't test, because you weren't even aware it exists. You
enable it, and now the command crashes for you, too. Fixing this
after rc4 is out of the question, too much churn. You could,
however, make feature allow-preconfig conditional on not
CONFIG_EXOTIC_CRAP.
>> [...]
On Fri, Jan 10, 2025 at 7:19 AM Markus Armbruster <armbru@redhat.com> wrote:
> John Snow <jsnow@redhat.com> writes:
>
> > On Thu, Jan 9, 2025, 5:34 AM Markus Armbruster <armbru@redhat.com>
> wrote:
> >
> >> John Snow <jsnow@redhat.com> writes:
> >>
> >> > On Fri, Dec 20, 2024 at 9:15 AM Markus Armbruster <armbru@redhat.com>
> wrote:
> >> >
> >> >> John Snow <jsnow@redhat.com> writes:
> >> >>
> >> >> > This method adds the options/preamble to each definition block.
> Notably,
> >> >> > :since: and :ifcond: are added, as are any "special features" such
> as
> >> >> > :deprecated: and :unstable:.
> >> >> >
> >> >> > Signed-off-by: John Snow <jsnow@redhat.com>
> >> >> > ---
> >> >> > docs/sphinx/qapidoc.py | 33 ++++++++++++++++++++++++++++++++-
> >> >> > 1 file changed, 32 insertions(+), 1 deletion(-)
> >> >> >
> >> >> > diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
> >> >> > index 6f8f69077b1..85c7ce94564 100644
> >> >> > --- a/docs/sphinx/qapidoc.py
> >> >> > +++ b/docs/sphinx/qapidoc.py
> >> >> > @@ -38,7 +38,7 @@
> >> >> > from qapi.error import QAPIError, QAPISemError
> >> >> > from qapi.gen import QAPISchemaVisitor
> >> >> > from qapi.parser import QAPIDoc
> >> >> > -from qapi.schema import QAPISchema
> >> >> > +from qapi.schema import QAPISchema, QAPISchemaEntity
> >> >> > from qapi.source import QAPISourceInfo
> >> >> >
> >> >> > from sphinx import addnodes
> >> >> > @@ -125,6 +125,37 @@ def ensure_blank_line(self) -> None:
> >> >> > # +2: correct for zero/one index, then increment by
> one.
> >> >> > self.add_line_raw("", fname, line + 2)
> >> >> >
> >> >> > + # Transmogrification helpers
> >> >> > +
> >> >> > + def preamble(self, ent: QAPISchemaEntity) -> None:
> >> >> > + """
> >> >> > + Generate option lines for qapi entity directives.
> >> >> > + """
> >> >> > + if ent.doc and ent.doc.since:
> >> >> > + assert ent.doc.since.tag == QAPIDoc.Tag.SINCE
> >> >> > + # Generated from the entity's docblock; info location
> is exact.
> >> >> > + self.add_line(f":since: {ent.doc.since.text}",
> ent.doc.since.info)
> >> >> > +
> >> >> > + if ent.ifcond.is_present():
> >> >> > + doc = ent.ifcond.docgen()
> >> >> > + # Generated from entity definition; info location is
> approximate.
> >> >> > + self.add_line(f":ifcond: {doc}", ent.info)
> >> >> > +
> >> >> > + # Hoist special features such as :deprecated: and
> :unstable:
> >> >> > + # into the options block for the entity. If, in the
> future, new
> >> >> > + # special features are added, qapi-domain will chirp about
> >> >> > + # unrecognized options and fail.
> >> >> > + for feat in ent.features:
> >> >> > + if feat.is_special():
> >> >> > + # We don't expect special features to have an
> ifcond property.
> >> >> > + # (Hello, intrepid developer in the future who
> changed that!)
> >> >> > + # ((With luck, you are not me.))
> >> >> > + assert not feat.ifcond.is_present()
> >> >>
> >> >> Nope :)
> >> >>
> >> >> The attempt to add a conditional special feature now fails with
> >> >>
> >> >> Sphinx parallel build error:
> >> >> AssertionError
> >> >>
> >> >> If you want to outlaw conditional special features, reject them
> cleanly
> >> >> in schema.py, document the restriction in
> docs/devel/qapi-code-gen.rst,
> >> >> and explain why in the commit message. Recommend a separate commit,
> to
> >> >> make it stand out in git-log.
> >> >
> >> > Do you advocate this? I wasn't sure what it *meant* for a special
> feature
> >> > to be conditional; I couldn't conceive of what it meant to have an
> ifcond
> >> > for "deprecated" or "unstable", for instance. It sounds like it isn't
> well
> >> > defined, but we happen to not expressly forbid it.
> >>
> >> Semantics are clear enough to me.
> >>
> >> "Conditional special feature" combines "conditional feature" (which is a
> >> special case of conditional thing) with special feature (which is a
> >> special case of feature).
> >>
> >> The QAPI schema language supports compile-time conditionals for certain
> >> things. Generated code behaves as if the thing didn't exist unless its
> >> condition is true.
> >>
> >> QAPI schema features are strings exposed to clients in introspection.
> >>
> >> Combine the two: a conditional feature is exposed if and only if its
> >> condition is true.
> >>
> >> Existing uses: dynamic-auto-read-only if CONFIG_POSIX, fdset if
> >> CONFIG_BLKIO_VHOST_VDPA_FD.
> >>
> >> A special feature is a feature that has special meaning to the
> >> generator, i.e. we generate different code in places when it's present.
> >>
> >> Combine: we enable different code for a conditional special feature only
> >> when its condition is true.
> >>
> >> No existing uses so far.
> >>
> >> Implementation is straightforward, too.
> >>
> >
> > Mechanically, it's well defined... but that isn't my concern.
> >
> > What I asked you was: "what would it mean for deprecated to be
> conditional?"
> >
> > Without understanding that, I have no chance to design a system that
> > handles that information accordingly for the documentation.
>
> Special feature deprecated means the use of the thing that has it is
> deprecated with this particular qemu-system-FOO.
>
> Why "this particular"? Because it wasn't deprecated in some prior
> version of QEMU. Features (special or not) are always, always about the
> specific qemu-system-FOO binary you're talking to via QMP.
>
> Conditionals are compile-time schema configuration. Either the
> condition is true or not at compile time. If it's true, the thing that
> has it is there, else it isn't.
>
> Now combine the two. If the condition of special feature 'deprecate' is
> true, the thing that has it is deprecated with this particular
> qemu-system-FOO. Else it isn't.
>
> >> Any code we generate for a conditional thing is guarded by #if
> >> ... #endif.
> >>
> >> For features, we generate suitable data into qapi-introspect.c.
> >>
> >> For special features, we generate a little additional code here and
> >> there; look for .is_special() to find it.
> >>
> >> Bug: this additional code lacks #if ... #endif. Simple oversight,
> >> should be easy enough to fix.
> >>
> >> > I guard against it here because, similarly, I have no idea how to
> handle
> >> > the case where it's true.
> >> >
> >> > I didn't realize we technically allow it, though ... would you like
> me to
> >> > move to expressly forbid it in the parser? (Failing that, I have no
> idea
> >> > how to display this information otherwise, so I'd need you to sketch
> >> > something out for me; so my inclination is to forbid it as you
> suggest.
> >> > Future developers can always lift the restriction once they have some
> >> > use-case in mind and a plan for how to display that information.)
> >>
> >> I think we should first make a reasonable effort at figuring out how to
> >> handle conditional special features. If we fail, we can add the
> >> restriction instead.
> >
> > I don't think I agree; I don't think it's worth spending time defining a
> > feature we don't use in our minds to then design around a hypothetical
> > future use.
> >
> > Easier to admit we don't use it and save defining the semantics for the
> > future developer who stumbles across a reason for needing it.
>
> Semantics of "apply conditional to feature" don't need defining, because
> they are simply the same as "apply conditional to any other thing."
> When you have a definition for the general case that works for all the
> special cases, then outlawing one special case *increases* complexity.
> That's why I'm reluctant.
>
> I figure your actual problem more concrete than "what does it even
> mean?", namely how to present conditionals in generated documentation.
> In that context, features *may* differ from any other thing!
>
> Aside: the (mostly generated) QMP introspection is about this particular
> qemu-system-FOO, like all of QMP, whereas the (mostly generated) QMP
> documentation is about any qemu-system-FOO that could be built from this
> particular working tree. May lead to doc generation problems that have
> no match elsewhere.
>
> >> How do you handle features in general, and special features in
> >> particular?
> >>
> >
> > Features: they go in the :feat: field list.
> > Special features: they go in the :feat: field list, and also receive a
> > special option flag to the directive to allow for special rendering of
> the
> > "signature bar" (the header line for a given entity definition in the
> > rendered documentation.)
> >
> > e.g.
> >
> > @deprecated will generate two things:
> >
> > 1) the definition block will get a :deprecated: option, which enables the
> > html renderer to (potentially) do special things like color the entry
> > differently, add a "warning pip"/eyecatch, etc. It's scoped to the whole
> > definition.
> >
> > 2) A :feat deprecated: line which is added to the features "field list"
> as
> > per normal for features.
> >
> >
> >> How do you handle conditionals in general?
> >>
> >
> > In this series, I don't!
> >
> > ...except for top-level conditionals, which are currently passed as-is as
> > an argument to the :ifcond: directive option. The format of that argument
> > and what the HTML renderer actually does with it is currently TBD.
> >
> > (In current prototypes I just print the conditional string in the
> signature
> > bar.)
> >
> >
> >> How do you combine feature and conditional?
> >>
> >
> > Notably TBD. Unhandled both in this series as-is *and* in my larger WIP.
>
> How do you combine definition and conditional? You answered that right
> above: largely TBD.
>
> How do you combine enum value / object type member / command argument /
> event argument and conditional? Also answered right above: not yet.
>
> What does that mean for the doc generator code? Does it simply ignore
> conditionals there?
>
> >> How could you combine special feature and conditonal?
> >>
> >
> > Bewilderingly uncertain.
> >
> > Mechanically, it *could* be handled the same way :ifcond: directive
> options
> > are.
> >
> > e.g.
> >
> > ```
> > .. qapi:command:: x-beware-of-the-leopard
> > :deprecated: {ifcond stuff here}
> > :unstable: {ifcond stuff here}
> >
> > lorem ipsum dolor sit amet
> >
> > :feat deprecated: blah blah (if: ...)
> > :feat unstable: blah blah (if: ...)
> > ```
> >
> > Semantically and in the rendered output, I have absolutely no clue
> > whatsoever; and doubt I could figure it out without a use case in front
> of
> > me to design around ... so I'd prefer to put up a traffic barrier for
> now.
> >
> > i.o.w. the design issue arises specifically because special features are
> > semanticaly special and are "hoisted" to be block-level in the
> > transmogrified rST doc, and so syntax needs to be designed, implemented
> and
> > tested if they are to be conditional.
>
> You choose to make them special in the documentation. They haven't been
> special there so far. I'm not telling you not to, I'm just pointing out
> this is a problem we elect to have :)
>
> > Yet, we have no such cases to facilitate the design, implementation and
> > testing.
> >
> > So, I'd like to prohibit until such time as we have such a case.
>
> I believe what you need isn't so much an explanation of semantics, it's
> use cases. And that's fair!
>
> Let me offer two.
>
> 1. Imagine we're trying to develop something big & complex enough to
> make keeping it out of tree until done impractical. We feel even an
> in-tree branch would be impractical. Instead, we commit it to
> master, default off, configure --enable-complex-thing to get it. Not
> exactly something we do all the time, but not outlandish, either.
>
> Further imagine that the big & complex thing will involve some new
> QMP commands replacing existing ones. As usual, we want to mark the
> ones being replaced deprecated, so we can remove them after a grace
> period.
>
> But simply deprecating them in the schema would be premature! The
> big & complex thing may fail, and if it does, we rip it out. If it
> succeeds, we enable it unconditionally.
>
> We can express what we're doing by making feature deprecated
> conditional on CONFIG_COMPLEX_THING, which is defined by
> --enable-complex-thing.
>
> That way, the complex thing doesn't affect the QMP interface unless
> we enable it. Good. And if we enable it, we do get the deprecation,
> and can test management applications cope with it.
>
> 2. Imagine we turned allow-preconfig into a special feature. Further
> imagine some whole-stack feature requires a certain command to have
> allow-preconfig, and you got tasked with implementing it. Which you
> duly did, to everybody's satisfaction. It is now days to the
> release, rc4 has sailed, and some dude comes out of the woodwork to
> report the command crashes in preconfig state. Weird! It worked
> *fine* in your meticulous testing. After some back and forth, you
> discover that the reporter builds with --enable-exotic-crap, which
> you didn't test, because you weren't even aware it exists. You
> enable it, and now the command crashes for you, too. Fixing this
> after rc4 is out of the question, too much churn. You could,
> however, make feature allow-preconfig conditional on not
> CONFIG_EXOTIC_CRAP.
>
> >> [...]
>
>
Hm, alright. I think I'll just stub out conditionals with a TODO and circle
back to how I'll handle them; I need to do a pass on ifcond handling in
general, so I guess I'll get to it then. I still think it's a little weird,
but you don't, and you're the QAPI guy ;)
--js
John Snow <jsnow@redhat.com> writes:
> On Fri, Jan 10, 2025 at 7:19 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> John Snow <jsnow@redhat.com> writes:
>>
>> > On Thu, Jan 9, 2025, 5:34 AM Markus Armbruster <armbru@redhat.com> wrote:
>> >
>> >> John Snow <jsnow@redhat.com> writes:
>> >>
>> >> > On Fri, Dec 20, 2024 at 9:15 AM Markus Armbruster <armbru@redhat.com> wrote:
>> >> >
>> >> >> John Snow <jsnow@redhat.com> writes:
>> >> >>
>> >> >> > This method adds the options/preamble to each definition block. Notably,
>> >> >> > :since: and :ifcond: are added, as are any "special features" such as
>> >> >> > :deprecated: and :unstable:.
>> >> >> >
>> >> >> > Signed-off-by: John Snow <jsnow@redhat.com>
>> >> >> > ---
>> >> >> > docs/sphinx/qapidoc.py | 33 ++++++++++++++++++++++++++++++++-
>> >> >> > 1 file changed, 32 insertions(+), 1 deletion(-)
>> >> >> >
>> >> >> > diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
>> >> >> > index 6f8f69077b1..85c7ce94564 100644
>> >> >> > --- a/docs/sphinx/qapidoc.py
>> >> >> > +++ b/docs/sphinx/qapidoc.py
>> >> >> > @@ -38,7 +38,7 @@
>> >> >> > from qapi.error import QAPIError, QAPISemError
>> >> >> > from qapi.gen import QAPISchemaVisitor
>> >> >> > from qapi.parser import QAPIDoc
>> >> >> > -from qapi.schema import QAPISchema
>> >> >> > +from qapi.schema import QAPISchema, QAPISchemaEntity
>> >> >> > from qapi.source import QAPISourceInfo
>> >> >> >
>> >> >> > from sphinx import addnodes
>> >> >> > @@ -125,6 +125,37 @@ def ensure_blank_line(self) -> None:
>> >> >> > # +2: correct for zero/one index, then increment by one.
>> >> >> > self.add_line_raw("", fname, line + 2)
>> >> >> >
>> >> >> > + # Transmogrification helpers
>> >> >> > +
>> >> >> > + def preamble(self, ent: QAPISchemaEntity) -> None:
>> >> >> > + """
>> >> >> > + Generate option lines for qapi entity directives.
>> >> >> > + """
>> >> >> > + if ent.doc and ent.doc.since:
>> >> >> > + assert ent.doc.since.tag == QAPIDoc.Tag.SINCE
>> >> >> > + # Generated from the entity's docblock; info location is exact.
>> >> >> > + self.add_line(f":since: {ent.doc.since.text}", ent.doc.since.info)
>> >> >> > +
>> >> >> > + if ent.ifcond.is_present():
>> >> >> > + doc = ent.ifcond.docgen()
>> >> >> > + # Generated from entity definition; info location is approximate.
>> >> >> > + self.add_line(f":ifcond: {doc}", ent.info)
>> >> >> > +
>> >> >> > + # Hoist special features such as :deprecated: and :unstable:
>> >> >> > + # into the options block for the entity. If, in the future, new
>> >> >> > + # special features are added, qapi-domain will chirp about
>> >> >> > + # unrecognized options and fail.
>> >> >> > + for feat in ent.features:
>> >> >> > + if feat.is_special():
>> >> >> > + # We don't expect special features to have an ifcond property.
>> >> >> > + # (Hello, intrepid developer in the future who changed that!)
>> >> >> > + # ((With luck, you are not me.))
>> >> >> > + assert not feat.ifcond.is_present()
>> >> >>
>> >> >> Nope :)
>> >> >>
>> >> >> The attempt to add a conditional special feature now fails with
>> >> >>
>> >> >> Sphinx parallel build error:
>> >> >> AssertionError
>> >> >>
>> >> >> If you want to outlaw conditional special features, reject them cleanly
>> >> >> in schema.py, document the restriction in docs/devel/qapi-code-gen.rst,
>> >> >> and explain why in the commit message. Recommend a separate commit, to
>> >> >> make it stand out in git-log.
>> >> >
>> >> > Do you advocate this? I wasn't sure what it *meant* for a special feature
>> >> > to be conditional; I couldn't conceive of what it meant to have an ifcond
>> >> > for "deprecated" or "unstable", for instance. It sounds like it isn't well
>> >> > defined, but we happen to not expressly forbid it.
[...]
>> I believe what you need isn't so much an explanation of semantics, it's
>> use cases. And that's fair!
>>
>> Let me offer two.
>>
>> 1. Imagine we're trying to develop something big & complex enough to
>> make keeping it out of tree until done impractical. We feel even an
>> in-tree branch would be impractical. Instead, we commit it to
>> master, default off, configure --enable-complex-thing to get it. Not
>> exactly something we do all the time, but not outlandish, either.
>>
>> Further imagine that the big & complex thing will involve some new
>> QMP commands replacing existing ones. As usual, we want to mark the
>> ones being replaced deprecated, so we can remove them after a grace
>> period.
>>
>> But simply deprecating them in the schema would be premature! The
>> big & complex thing may fail, and if it does, we rip it out. If it
>> succeeds, we enable it unconditionally.
>>
>> We can express what we're doing by making feature deprecated
>> conditional on CONFIG_COMPLEX_THING, which is defined by
>> --enable-complex-thing.
>>
>> That way, the complex thing doesn't affect the QMP interface unless
>> we enable it. Good. And if we enable it, we do get the deprecation,
>> and can test management applications cope with it.
>>
>> 2. Imagine we turned allow-preconfig into a special feature. Further
>> imagine some whole-stack feature requires a certain command to have
>> allow-preconfig, and you got tasked with implementing it. Which you
>> duly did, to everybody's satisfaction. It is now days to the
>> release, rc4 has sailed, and some dude comes out of the woodwork to
>> report the command crashes in preconfig state. Weird! It worked
>> *fine* in your meticulous testing. After some back and forth, you
>> discover that the reporter builds with --enable-exotic-crap, which
>> you didn't test, because you weren't even aware it exists. You
>> enable it, and now the command crashes for you, too. Fixing this
>> after rc4 is out of the question, too much churn. You could,
>> however, make feature allow-preconfig conditional on not
>> CONFIG_EXOTIC_CRAP.
>>
>> >> [...]
>>
>>
> Hm, alright. I think I'll just stub out conditionals with a TODO and circle
> back to how I'll handle them; I need to do a pass on ifcond handling in
> general, so I guess I'll get to it then.
Makes sense (although I'd use FIXME rather than TODO).
If resolving them turns out to be hard, we can talk about adding
restrictions to help you over the hump.
> I still think it's a little weird,
> but you don't, and you're the QAPI guy ;)
I admit constructing believable use cases took a bit of thought.
Calling them a little weird can't offend me :)
© 2016 - 2025 Red Hat, Inc.