This series switches all our QAPI doc comments over from
texinfo format to rST. It then removes all the texinfo
machinery, because this was the last user of texinfo.

This is largely just a rebase of patchset v4 to current master.
I've had to add three new patches at the front which fix
up newly added QAPI doc comment deviations from the
tightened-up indent rules that the rST conversion requires.
In a couple of places where Markus had already reviewed
patches with a 'reviewed-by with these minor commit message
fixes' I've made the fixes and added his r-by tag.

I've sent this out to the mailing list since I've done the
rebase-work anyway and it seems more useful to review this
than v4. I haven't bothered to re-generate the "here's what
the rendered HTML looks like" since v4, though I can do that
if useful.

Also available as a git branch at
https://git.linaro.org/people/peter.maydell/qemu-arm.git sphinx-conversions-v5

thanks
-- PMM


Below is the same old text from the v3 cover letter, just
for convenience:

The basic approach is somewhat similar to how we deal with kerneldoc
and hxtool: we have a custom Sphinx extension which is passed a
filename which is the json file it should run the QAPI parser over and
generate documentation for. Unlike 'kerneldoc' but somewhat like
hxtool, I have chosed to generate documentation by generating a tree
of docutils nodes, rather than by generating rST source that is then
fed to the rST parser to generate docutils nodes.  Individual lumps of
doc comment go to the rST parser, but the structured parts we render
directly. This makes it easier to get the structure and heading level
nesting correct.

Rather than trying to exactly handle all the existing comments I have
opted (as Markus suggested) to tweak them where this seemed more
sensible than contorting the rST generator to deal with
weirdnesses. The principal changes are:
 * whitespace is now significant, and multiline definitions must have
   their second and subsequent lines indented to match the first line
 * general rST format markup is permitted, not just the small set of
   markup the old texinfo generator handled. For most things (notably
   bulleted and itemized lists) the old format is the same as rST was.
 * Specific things that might trip people up:
   - instead of *bold* and _italic_ rST has **bold** and *italic*
   - lists need a preceding and following blank line
   - a lone literal '*' will need to be backslash-escaped to
     avoid a rST syntax error
 * the old leading '|' for example (literal text) blocks is replaced
   by the standard rST '::' literal block.
 * headings and subheadings must now be in a freeform documentation
   comment of their own
 * we support arbitrary levels of sub- and sub-sub-heading, not just a
   main and sub-heading like the old texinfo generator
 * as a special case, @foo is retained and is equivalent to ``foo``
Moving on to the actual code changes:
 * we start by changing the existing parser code to be more careful
   with leading whitespace: instead of stripping it all, it strips
   only the amount required for indented multiline definitions, and
   complains if it finds an unexpected de-indent. The texinfo
   generator code is updated to do whitespace stripping itself, so
   there is no change to the generated texi source.
 * then we add the new qapidoc Sphinx extension, which is not yet used
   by anything. This is a 500 line script, all added in one patch. I
   can split it if people think that would help, but I'm not sure I
   see a good split point.
 * then we can convert the two generated reference documents, one at a
   time. This is mostly just updating makefile rules and the like.
 * after that we can do some minor tweaks to doc comments that would
   have confused the texinfo parser: changing our two instances of
   '|'-markup literal blocks to rST '::' literal blocks, and adding
   some headings to the GA reference so the rST interop manual ToC
   looks better.
 * finally, we can delete the old texinfo machinery and update the
   markup docs in docs/devel/qapi-code-gen.txt

On headings:

Because the rST generator works by assembling a tree of docutils
nodes, it needs to know the structure of the document, in the
sense that it wants to know that there is a "section with a level
1 heading titled Foo", which contains "section with a level 2
heading titled Bar", which in turn contains the documentation for
commands Baz, Boz, Buz. This means we can't follow the texinfo
generator's approach of just treating '= Foo' as another kind
of markup to be turned into a '@section' texinfo and otherwise
just written out into the output stream. Instead we need to
be able to distinguish "this is a level 1 section heading"
from any other kind of doc-comment, and the user shouldn't be
able to insert directives specifying changes in the document
structure randomly in the middle of what would otherwise be a
lump of "just rST source to be fed to a rST parser".
The approach I've taken to letting the generator know the structure
is to special-case headings into "must be in their own freeform
doc-comment as a single line", like this:
 ##
 # = Foo
 ##
This is easy to spot in the 'freeform' method, and matches how
we already mark up headings in almost all cases. An alternative
approach would be to have parser.py detect heading markup, so
that instead of
        for doc in schema.docs:
            if doc.symbol:
                vis.symbol(doc, schema.lookup_entity(doc.symbol))
            else:
                vis.freeform(doc)
(ie "everything the parser gives you is either documenting
a symbol, or it is a freefrom comment") we have:
        for doc in schema.docs:
            if doc.symbol:
                vis.symbol(doc, schema.lookup_entity(doc.symbol))
            else if doc.is_section_header:
                vis.new_section(doc.heading_text, doc.heading_level)
            else:
                vis.freeform(doc)
(ie "everything the parser gives you is either documenting
a symbol, or a notification about the structure of the document,
or a freeform comment".) I feel that would be less simple than
we currently have, though.

There are a few things I have left out of this initial series:
 * unlike the texinfo, there is no generation of index entries
   or an index in the HTML docs
 * although there are HTML anchors on all the command/object/etc
   headings, they are not stable but just serial-number based
   tags like '#qapidoc-35', so not suitable for trying to link
   to from other parts of the docs
 * unlike the old texinfo generation, we make no attempt to regression
   test the rST generation in 'make check'. This is trickier than
   the texinfo equivalent, because we never generate rST source
   that we could compare against a golden reference. Comparing
   against generated HTML is liable to break with new Sphinx
   versions; trying to compare the data structure of docutils nodes
   would be a bit more robust but would require a bunch of code to
   mock up running Sphinx somehow.

My view is that we can add niceties like this later; the series
already seems big enough to me.




Peter Maydell (20):
  qapi/migration.json: Fix indentation
  qapi: Fix indentation, again
  qapi/block-core.json: Fix nbd-server-start docs
  qapi/qapi-schema.json: Put headers in their own doc-comment blocks
  qapi/machine.json: Escape a literal '*' in doc comment
  tests/qapi/doc-good.json: Prepare for qapi-doc Sphinx extension
  scripts/qapi: Move doc-comment whitespace stripping to doc.py
  scripts/qapi/parser.py: improve doc comment indent handling
  docs/sphinx: Add new qapi-doc Sphinx extension
  docs/interop: Convert qemu-ga-ref to rST
  docs/interop: Convert qemu-qmp-ref to rST
  qapi: Use rST markup for literal blocks
  qga/qapi-schema.json: Add some headings
  scripts/qapi: Remove texinfo generation support
  docs/devel/qapi-code-gen.txt: Update to new rST backend conventions
  Makefile: Remove redundant Texinfo related rules
  scripts/texi2pod: Delete unused script
  Remove Texinfo related files from .gitignore and git.orderfile
  configure: Drop texinfo requirement
  Remove texinfo dependency from docker and CI configs

 docs/conf.py                               |   6 +-
 docs/devel/qapi-code-gen.txt               |  90 ++--
 docs/index.html.in                         |   2 -
 docs/interop/conf.py                       |   4 +
 docs/interop/index.rst                     |   2 +
 docs/interop/qemu-ga-ref.rst               |   4 +
 docs/interop/qemu-ga-ref.texi              |  80 ---
 docs/interop/qemu-qmp-ref.rst              |   4 +
 docs/interop/qemu-qmp-ref.texi             |  80 ---
 docs/sphinx/qapidoc.py                     | 504 +++++++++++++++++++
 configure                                  |  12 +-
 Makefile                                   |  86 +---
 rules.mak                                  |  14 +-
 qapi/audio.json                            |  12 +-
 qapi/block-core.json                       |  30 +-
 qapi/control.json                          |   4 +-
 qapi/machine.json                          |   8 +-
 qapi/migration.json                        |  68 +--
 qapi/misc.json                             |   4 +-
 qapi/net.json                              |   2 +-
 qapi/qapi-schema.json                      |  18 +-
 qga/qapi-schema.json                       |  12 +-
 .gitignore                                 |  15 -
 .travis.yml                                |   1 -
 MAINTAINERS                                |   3 +-
 scripts/checkpatch.pl                      |   2 +-
 scripts/git.orderfile                      |   1 -
 scripts/qapi-gen.py                        |   2 -
 scripts/qapi/doc.py                        | 301 ------------
 scripts/qapi/gen.py                        |   7 -
 scripts/qapi/parser.py                     |  93 +++-
 scripts/texi2pod.pl                        | 536 ---------------------
 tests/Makefile.include                     |  15 +-
 tests/docker/dockerfiles/debian10.docker   |   1 -
 tests/docker/dockerfiles/debian9.docker    |   1 +
 tests/docker/dockerfiles/fedora.docker     |   1 -
 tests/docker/dockerfiles/ubuntu.docker     |   1 -
 tests/docker/dockerfiles/ubuntu1804.docker |   1 -
 tests/docker/dockerfiles/ubuntu2004.docker |   1 -
 tests/qapi-schema/doc-good.json            |  25 +-
 tests/qapi-schema/doc-good.out             |  22 +-
 tests/qapi-schema/doc-good.texi            | 319 ------------
 42 files changed, 784 insertions(+), 1610 deletions(-)
 create mode 100644 docs/interop/qemu-ga-ref.rst
 delete mode 100644 docs/interop/qemu-ga-ref.texi
 create mode 100644 docs/interop/qemu-qmp-ref.rst
 delete mode 100644 docs/interop/qemu-qmp-ref.texi
 create mode 100644 docs/sphinx/qapidoc.py
 delete mode 100644 scripts/qapi/doc.py
 delete mode 100755 scripts/texi2pod.pl
 delete mode 100644 tests/qapi-schema/doc-good.texi

-- 
2.20.1

On Mon, 10 Aug 2020 at 20:50, Peter Maydell <peter.maydell@linaro.org> wrote:
> This series switches all our QAPI doc comments over from
> texinfo format to rST. It then removes all the texinfo
> machinery, because this was the last user of texinfo.
>
> This is largely just a rebase of patchset v4 to current master.

> There are a few things I have left out of this initial series:

I realized there is something I forgot to add to this "left out" list:

Sphinx needs to know what all the input files which go into
a document are, as it builds up dependencies to tell it whether
to rebuild the output or not. The docs/sphinx/qapidoc.py
plugin adds such a dependency on the file that the .rst
docs reference (eg qapi/qapi-schema.json) but it does not
have a mechanism for adding dependencies when that .json
file uses an 'include' to pull in other .json files.

I'm not sure whether the scripts/qapi code supports telling
a consumer of the parsed info about this -- is it sufficient
for QAPISchemaGenRSTVisitor to implement the 'visit_include'
method, find the path to the included .qapi file from the
arguments and call Sphinx's env.notedependency(), or do we
need to do something more complicated to get the list of
all the included .qapi files ?

thanks
-- PMM

Peter Maydell <peter.maydell@linaro.org> writes:

> On Mon, 10 Aug 2020 at 20:50, Peter Maydell <peter.maydell@linaro.org> wrote:
>> This series switches all our QAPI doc comments over from
>> texinfo format to rST. It then removes all the texinfo
>> machinery, because this was the last user of texinfo.
>>
>> This is largely just a rebase of patchset v4 to current master.

It needs another rebase now, for the Meson build system.

>> There are a few things I have left out of this initial series:
>
> I realized there is something I forgot to add to this "left out" list:
>
> Sphinx needs to know what all the input files which go into
> a document are, as it builds up dependencies to tell it whether
> to rebuild the output or not. The docs/sphinx/qapidoc.py
> plugin adds such a dependency on the file that the .rst
> docs reference (eg qapi/qapi-schema.json)

In QAPIDocDirective.run():

        # Tell sphinx of the dependency
        env.note_dependency(os.path.abspath(qapifile))


>                                           but it does not
> have a mechanism for adding dependencies when that .json
> file uses an 'include' to pull in other .json files.
>
> I'm not sure whether the scripts/qapi code supports telling
> a consumer of the parsed info about this -- is it sufficient
> for QAPISchemaGenRSTVisitor to implement the 'visit_include'
> method, find the path to the included .qapi file from the
> arguments and call Sphinx's env.notedependency(), or do we
> need to do something more complicated to get the list of
> all the included .qapi files ?

Visitors can implement visit_include() to see include directives.
QAPISchemaModularCVisitor does, to generate #include that mirror the
source schema.  This is not what your want.

You want visit_module().  The appended hack makes qapi-gen.py spit out
the modules when it generates types, e.g.:

    $ python3 -B scripts/qapi-gen.py -o scratch tests/qapi-schema/qapi-schema-test.json 
    ### None
    ### 'qapi-schema-test.json'
    ### 'include/sub-module.json'
    ### 'sub-sub-module.json'

As you can see, the module names are file names relative to the main
module's directory.

Module None is for built-ins.

Unfortunately, your qapidoc.py bypasses the regular schema.visit() just
like old doc.py does, and for the same reason: it wants to iterate over
doc comments, not definitions.  Doc comments were bolted on later, and
it still shows.

I figure the clean solution is making schema.visit() pass doc comments
just like it passes entities.  Gets rid of the need to bypass
schema.visit().  Requires surgery to QAPISchema and QAPISchemaParser.

A quick hack: use a trivial QAPISchemaVisitor just to collect the
modules.

Questions?



diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index 3ad33af4ee..cec89e199c 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -274,6 +274,10 @@ class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor):
         # gen_object() is recursive, ensure it doesn't visit the empty type
         objects_seen.add(schema.the_empty_object_type.name)
 
+    def visit_module(self, name):
+        print('### %r' % name)
+        super().visit_module(name)
+
     def _gen_type_cleanup(self, name):
         self._genh.add(gen_type_cleanup_decl(name))
         self._genc.add(gen_type_cleanup(name))

On Fri, 4 Sep 2020 at 15:34, Markus Armbruster <armbru@redhat.com> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
> > I'm not sure whether the scripts/qapi code supports telling
> > a consumer of the parsed info about this -- is it sufficient
> > for QAPISchemaGenRSTVisitor to implement the 'visit_include'
> > method, find the path to the included .qapi file from the
> > arguments and call Sphinx's env.notedependency(), or do we
> > need to do something more complicated to get the list of
> > all the included .qapi files ?
>
> Visitors can implement visit_include() to see include directives.
> QAPISchemaModularCVisitor does, to generate #include that mirror the
> source schema.  This is not what your want.

Why not? Surely "see include directives" is exactly what I want?
Any time the QAPI parser opens a file that's different from the
initial one we want to know about it.

> You want visit_module().  The appended hack makes qapi-gen.py spit out
> the modules when it generates types, e.g.:
>
>     $ python3 -B scripts/qapi-gen.py -o scratch tests/qapi-schema/qapi-schema-test.json
>     ### None
>     ### 'qapi-schema-test.json'
>     ### 'include/sub-module.json'
>     ### 'sub-sub-module.json'

What's a "module" here ? Does this still produce output if the
top level .json file includes a sub-json file that doesn't actually
have any contents ? (We still want to generate the dependency
then, so we update the docs if the included file is edited to
add content.)

thanks
-- PMM

Peter Maydell <peter.maydell@linaro.org> writes:

> On Fri, 4 Sep 2020 at 15:34, Markus Armbruster <armbru@redhat.com> wrote:
>> Peter Maydell <peter.maydell@linaro.org> writes:
>> > I'm not sure whether the scripts/qapi code supports telling
>> > a consumer of the parsed info about this -- is it sufficient
>> > for QAPISchemaGenRSTVisitor to implement the 'visit_include'
>> > method, find the path to the included .qapi file from the
>> > arguments and call Sphinx's env.notedependency(), or do we
>> > need to do something more complicated to get the list of
>> > all the included .qapi files ?
>>
>> Visitors can implement visit_include() to see include directives.
>> QAPISchemaModularCVisitor does, to generate #include that mirror the
>> source schema.  This is not what your want.
>
> Why not? Surely "see include directives" is exactly what I want?
> Any time the QAPI parser opens a file that's different from the
> initial one we want to know about it.
>
>> You want visit_module().  The appended hack makes qapi-gen.py spit out
>> the modules when it generates types, e.g.:
>>
>>     $ python3 -B scripts/qapi-gen.py -o scratch tests/qapi-schema/qapi-schema-test.json
>>     ### None
>>     ### 'qapi-schema-test.json'
>>     ### 'include/sub-module.json'
>>     ### 'sub-sub-module.json'
>
> What's a "module" here ?

Initially, the include directive was just that: include another file's
contents right here.

Back in 2018, we switched from generating monolithic code to generating
modular code.  What does that mean?

Instead of generating the kitchen sink into a single qapi-types.c, we
split out the stuff generated for each FOO.json included by
qapi-schema.json into qapi-types-FOO.c.

Same for qapi-types.h, but with #include stitching that mirrors the
schema's include directives.  So, if FOO.json includes SUB.json, then
qapi-types-FOO.h will include qapi-types-SUB.h.

Same for qapi-{commands,events,visit}.{c,h}.

The qapi-schema.json (rather: the file you pass to qapi-gen.py) is the
main module.

The .json it includes are the sub-modules.

visit_module() lets you see the modules.

visit_include() lets you see the includes.  The same module can be
included multiple times.  Having to filter that out would be annoying.

>                          Does this still produce output if the
> top level .json file includes a sub-json file that doesn't actually
> have any contents ? (We still want to generate the dependency
> then, so we update the docs if the included file is edited to
> add content.)

If it doesn't, I'd be willing to call it a bug.  I dimly remember fixing
this (or a similar bug) before.

On Fri, 4 Sep 2020 at 16:54, Markus Armbruster <armbru@redhat.com> wrote:
> Initially, the include directive was just that: include another file's
> contents right here.
>
> Back in 2018, we switched from generating monolithic code to generating
> modular code.  What does that mean?
>
> Instead of generating the kitchen sink into a single qapi-types.c, we
> split out the stuff generated for each FOO.json included by
> qapi-schema.json into qapi-types-FOO.c.
>
> Same for qapi-types.h, but with #include stitching that mirrors the
> schema's include directives.  So, if FOO.json includes SUB.json, then
> qapi-types-FOO.h will include qapi-types-SUB.h.
>
> Same for qapi-{commands,events,visit}.{c,h}.
>
> The qapi-schema.json (rather: the file you pass to qapi-gen.py) is the
> main module.
>
> The .json it includes are the sub-modules.
>
> visit_module() lets you see the modules.
>
> visit_include() lets you see the includes.  The same module can be
> included multiple times.  Having to filter that out would be annoying.

I don't think you'd need to filter it out for this use case -- I
assume Sphinx would just ignore the unnecessary extra dependency
information. But if visit_module() does what we want and you
recommend it I'll look at that.

thanks
-- PMM

On Fri, 4 Sep 2020 at 17:05, Peter Maydell <peter.maydell@linaro.org> wrote:
> On Fri, 4 Sep 2020 at 16:54, Markus Armbruster <armbru@redhat.com> wrote:
> > visit_module() lets you see the modules.
> >
> > visit_include() lets you see the includes.  The same module can be
> > included multiple times.  Having to filter that out would be annoying.
>
> I don't think you'd need to filter it out for this use case -- I
> assume Sphinx would just ignore the unnecessary extra dependency
> information. But if visit_module() does what we want and you
> recommend it I'll look at that.

visit_module() seems to work. I notice it gets called with None
before it's called with the filename of the top level qapi-schema.json,
but it's easy enough to ignore the 'None' call...

thanks
-- PMM

Peter Maydell <peter.maydell@linaro.org> writes:

> On Fri, 4 Sep 2020 at 17:05, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On Fri, 4 Sep 2020 at 16:54, Markus Armbruster <armbru@redhat.com> wrote:
>> > visit_module() lets you see the modules.
>> >
>> > visit_include() lets you see the includes.  The same module can be
>> > included multiple times.  Having to filter that out would be annoying.
>>
>> I don't think you'd need to filter it out for this use case -- I
>> assume Sphinx would just ignore the unnecessary extra dependency
>> information. But if visit_module() does what we want and you
>> recommend it I'll look at that.
>
> visit_module() seems to work. I notice it gets called with None
> before it's called with the filename of the top level qapi-schema.json,
> but it's easy enough to ignore the 'None' call...

None stands for the (nameless) module containing built-in stuff.