mbox series

[v6,00/21] Convert QAPI doc comments to generate rST instead of texinfo

Message ID 20200925162316.21205-1-peter.maydell@linaro.org
Headers show
Series Convert QAPI doc comments to generate rST instead of texinfo | expand

Message

Peter Maydell Sept. 25, 2020, 4:22 p.m. UTC
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.

I think I have now resolved all of Markus' issues raised in his
review of v5, and IMHO this is ready to commit.  If there are still
things needing fixing, it would be nice if we were able to commit
patches 1-5, which are the ones which add the new indent-sensitive
code to the QAPI parser.  That would put a stop to the steady trickle
of doc-comment changes which break the new rules...

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

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 chosen 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.

Changes from v5:
 * rebased (in particular, updated to meson build system)
 * new patch 1 fixes indent issues that hit master since v5
 * new patch 2 makes block-latency-histogram-set's use of Example
   sections match everybody else's, instead of special casing it
   in the parser
 * the .gitignore got pruned after meson conversion so we only
   need to change git.orderfile now
 * slightly reordered patches to bring the parser.py indent change nearer
   the start of the series in the hopes of being able to get at least
   that much of the series into master
 * we now tell Sphinx about all the json files for dependency info,
   so editing a json file correctly rebuilds the docs
 * added a test case for the bad-de-indent parser error
 * Adopted the various Python scripting suggestions from Markus
 * We don't insist on section headings being the only thing in their
   doc comment block any more (the existing "must be first line"
   requirement is sufficient)
 * added a test case for doc-generation that does a compare of
   the sphinx plain-text builder output against a reference file
 * Added the Python source files for Sphinx extensions (including
   the QAPI source files) to the dependency lists for the manuals,
   so that changes made to them correctly trigger a docs rebuild
 * qemu-ga-ref.rst and qemu-qmp-ref.rst now have a TODO note about
   making the manual licensing more visible to readers
 * fixed bug in reported file/line info for some errors in rST
   in doc comments when using Sphinx 1.6
 * don't insist on section headers being in their own freeform doc
   comment block; they're (after commit dcdc07a97cbe) always the
   first line in the comment block, so just handle the possibility
   of having text after that.

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

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

thanks
-- PMM


Peter Maydell (21):
  qapi: Fix doc comment indentation again
  qapi/block.json: Add newline after "Example:" for
    block-latency-histogram-set
  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
  qapi/machine.json: Escape a literal '*' in doc comment
  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
  tests/qapi-schema: Convert doc-good.json to rST-style strong/emphasis
  meson.build: Move SPHINX_ARGS to top level meson.build file
  meson.build: Make manuals depend on source to Sphinx extensions
  tests/qapi-schema: Add test of the rST QAPI doc-comment outputn
  scripts/qapi: Remove texinfo generation support
  docs/devel/qapi-code-gen.txt: Update to new rST backend conventions
  scripts/texi2pod: Delete unused script
  Remove Texinfo related line from git.orderfile
  configure: Drop texinfo requirement
  Remove texinfo dependency from docker and CI configs

 docs/conf.py                               |   6 +-
 docs/devel/qapi-code-gen.txt               | 106 ++--
 docs/index.html.in                         |   2 -
 docs/interop/conf.py                       |   4 +
 docs/interop/index.rst                     |   2 +
 docs/interop/qemu-ga-ref.rst               |   9 +
 docs/interop/qemu-ga-ref.texi              |  80 ---
 docs/interop/qemu-qmp-ref.rst              |   9 +
 docs/interop/qemu-qmp-ref.texi             |  80 ---
 docs/meson.build                           |  11 +-
 docs/sphinx/qapidoc.py                     | 550 +++++++++++++++++++++
 tests/qapi-schema/doc-good.rst             |   5 +
 configure                                  |  12 +-
 meson.build                                | 103 +---
 qapi/block-core.json                       |  20 +-
 qapi/block.json                            |  12 +-
 qapi/machine.json                          |   2 +-
 qapi/migration.json                        | 102 ++--
 qapi/qapi-schema.json                      |   6 +-
 qga/qapi-schema.json                       |   8 +-
 .travis.yml                                |   1 -
 MAINTAINERS                                |   3 +-
 qapi/meson.build                           |   4 +-
 qga/meson.build                            |   3 +-
 scripts/checkpatch.pl                      |   2 +-
 scripts/coverity-scan/coverity-scan.docker |   1 -
 scripts/git.orderfile                      |   1 -
 scripts/qapi-gen.py                        |   2 -
 scripts/qapi/doc.py                        | 301 -----------
 scripts/qapi/gen.py                        |   7 -
 scripts/qapi/parser.py                     | 105 +++-
 scripts/texi2pod.pl                        | 536 --------------------
 tests/docker/dockerfiles/debian10.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-bad-indent.err       |   1 +
 tests/qapi-schema/doc-bad-indent.json      |   8 +
 tests/qapi-schema/doc-bad-indent.out       |   0
 tests/qapi-schema/doc-good.json            |  27 +-
 tests/qapi-schema/doc-good.out             |  24 +-
 tests/qapi-schema/doc-good.texi            | 319 ------------
 tests/qapi-schema/doc-good.txt             | 288 +++++++++++
 tests/qapi-schema/meson.build              |  63 ++-
 45 files changed, 1223 insertions(+), 1607 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
 create mode 100644 tests/qapi-schema/doc-good.rst
 delete mode 100644 scripts/qapi/doc.py
 delete mode 100755 scripts/texi2pod.pl
 create mode 100644 tests/qapi-schema/doc-bad-indent.err
 create mode 100644 tests/qapi-schema/doc-bad-indent.json
 create mode 100644 tests/qapi-schema/doc-bad-indent.out
 delete mode 100644 tests/qapi-schema/doc-good.texi
 create mode 100644 tests/qapi-schema/doc-good.txt

Comments

John Snow Sept. 25, 2020, 4:54 p.m. UTC | #1
On 9/25/20 12:22 PM, Peter Maydell wrote:
>   * 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

I suppose this answers my question if qmp commands were going to be able 
to be cross-referenced yet.

I'd love to get that working so I can make smart references in the 
bitmaps doc.

We could even amend the QMP code parser to insert cross-references to 
the function names, which would be *extremely* slick.

--js
Peter Maydell Sept. 25, 2020, 5:02 p.m. UTC | #2
On Fri, 25 Sep 2020 at 17:54, John Snow <jsnow@redhat.com> wrote:
>
> On 9/25/20 12:22 PM, Peter Maydell wrote:
> >   * 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
>
> I suppose this answers my question if qmp commands were going to be able
> to be cross-referenced yet.
>
> I'd love to get that working so I can make smart references in the
> bitmaps doc.
>
> We could even amend the QMP code parser to insert cross-references to
> the function names, which would be *extremely* slick.

If you'd like to have a closer look into Sphinx's indexing
support, be my guest :-)

-- PMM
John Snow Sept. 25, 2020, 5:09 p.m. UTC | #3
On 9/25/20 1:02 PM, Peter Maydell wrote:
> On Fri, 25 Sep 2020 at 17:54, John Snow <jsnow@redhat.com> wrote:
>>
>> On 9/25/20 12:22 PM, Peter Maydell wrote:
>>>    * 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
>>
>> I suppose this answers my question if qmp commands were going to be able
>> to be cross-referenced yet.
>>
>> I'd love to get that working so I can make smart references in the
>> bitmaps doc.
>>
>> We could even amend the QMP code parser to insert cross-references to
>> the function names, which would be *extremely* slick.
> 
> If you'd like to have a closer look into Sphinx's indexing
> support, be my guest :-)
> 
Yeah, I am *absolutely* not suggesting this for this series.

I did start taking a look at Sphinx's default_role. As an exercise for 
the Python cleanup, I looked at what would happen if I set our default 
role to "Any" to be able to cross-reference using `backticks`.

I used this to test my docstring consistency for the QAPI module 
cleanup. It involved me converting a few usages of single backticks in 
./docs proper to double-backticks, but it wasn't too bad.

(See the "DO-NOT-MERGE" commits on my part1 V3.)

I'll look into how we *add* reference targets; that part I still have no 
clue on. but adding a qmp reference scope would be *delightful*.

> -- PMM
>
no-reply@patchew.org Sept. 25, 2020, 7:25 p.m. UTC | #4
Patchew URL: https://patchew.org/QEMU/20200925162316.21205-1-peter.maydell@linaro.org/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===

 perl-Encode-devel                       x86_64  4:3.07-457.fc32                  updates   41 k
 perl-Env                                noarch  1.04-440.fc32                    fedora    19 k
 perl-Errno                              x86_64  1.30-456.fc32                    updates   24 k
 perl-Error                              noarch  1:0.17029-1.fc32                 fedora    42 k
 perl-Exporter                           noarch  5.74-2.fc32                      fedora    32 k
 perl-ExtUtils-CBuilder                  noarch  1:0.280234-2.fc32                fedora    47 k
 perl-ExtUtils-Command                   noarch  2:7.46-1.fc32                    updates   14 k
---
(648/845): perl-Digest-SHA-6.02-442.fc32.x86_64 647 kB/s |  64 kB     00:00    
(649/845): perl-Encode-Locale-1.05-15.fc32.noar 196 kB/s |  19 kB     00:00    
(650/845): perl-Env-1.04-440.fc32.noarch.rpm    166 kB/s |  19 kB     00:00    
(651/845): perl-Error-0.17029-1.fc32.noarch.rpm 371 kB/s |  42 kB     00:00    
(652/845): perl-Exporter-5.74-2.fc32.noarch.rpm 331 kB/s |  32 kB     00:00    
(653/845): perl-ExtUtils-CBuilder-0.280234-2.fc 478 kB/s |  47 kB     00:00    
(654/845): perl-ExtUtils-Manifest-1.72-440.fc32 358 kB/s |  34 kB     00:00    
---
  Installing       : perl-DB_File-1.853-2.fc32.x86_64                   302/869 
  Installing       : perl-Devel-Size-0.83-5.fc32.x86_64                 303/869 
  Installing       : perl-Env-1.04-440.fc32.noarch                      304/869 
  Installing       : perl-Error-1:0.17029-1.fc32.noarch                 305/869 
  Installing       : perl-IPC-SysV-2.07-442.fc32.x86_64                 306/869 
  Installing       : perl-IPC-System-Simple-1.30-1.fc32.noarch          307/869 
  Installing       : perl-autodie-2.32-2.fc32.noarch                    308/869 
---
  Verifying        : perl-Digest-SHA-1:6.02-442.fc32.x86_64             648/869 
  Verifying        : perl-Encode-Locale-1.05-15.fc32.noarch             649/869 
  Verifying        : perl-Env-1.04-440.fc32.noarch                      650/869 
  Verifying        : perl-Error-1:0.17029-1.fc32.noarch                 651/869 
  Verifying        : perl-Exporter-5.74-2.fc32.noarch                   652/869 
  Verifying        : perl-ExtUtils-CBuilder-1:0.280234-2.fc32.noarch    653/869 
  Verifying        : perl-ExtUtils-Install-2.14-441.fc32.noarch         654/869 
---
  perl-Encode-devel-4:3.07-457.fc32.x86_64                                      
  perl-Env-1.04-440.fc32.noarch                                                 
  perl-Errno-1.30-456.fc32.x86_64                                               
  perl-Error-1:0.17029-1.fc32.noarch                                            
  perl-Exporter-5.74-2.fc32.noarch                                              
  perl-ExtUtils-CBuilder-1:0.280234-2.fc32.noarch                               
  perl-ExtUtils-Command-2:7.46-1.fc32.noarch                                    
---
Host machine cpu: x86_64
Target machine cpu family: x86
Target machine cpu: x86_64
../src/meson.build:10: WARNING: Module unstable-keyval has no backwards or forwards compatibility and might not exist in future releases.
Program sh found: YES
Program python3 found: YES (/usr/bin/python3)
Configuring ninjatool using configuration
---
Generating trace-hw_alpha.h with a meson_exe.py custom command
In file included from ../src/qapi/qapi-schema.json:78:
../src/qapi/migration.json:1747:1: unexpected de-indent (expected at least 13 spaces)
make: *** [Makefile.ninja:39: CUSTOM_COMMAND@d09afa93bc2.stamp] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 709, in <module>
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--rm', '--label', 'com.qemu.instance.uuid=8e891e79f2d34224a1d3856f15796b12', '-u', '1001', '--security-opt', 'seccomp=unconfined', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-9zsv4zbd/src/docker-src.2020-09-25-15.22.55.20619:/var/tmp/qemu:z,ro', 'qemu/fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=8e891e79f2d34224a1d3856f15796b12
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-9zsv4zbd/src'
make: *** [docker-run-test-mingw@fedora] Error 2

real    2m43.820s
user    0m21.216s


The full log is available at
http://patchew.org/logs/20200925162316.21205-1-peter.maydell@linaro.org/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
no-reply@patchew.org Sept. 25, 2020, 7:25 p.m. UTC | #5
Patchew URL: https://patchew.org/QEMU/20200925162316.21205-1-peter.maydell@linaro.org/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

C linker for the host machine: cc ld.bfd 2.27-43
Host machine cpu family: x86_64
Host machine cpu: x86_64
../src/meson.build:10: WARNING: Module unstable-keyval has no backwards or forwards compatibility and might not exist in future releases.
Program sh found: YES
Program python3 found: YES (/usr/bin/python3)
Configuring ninjatool using configuration
---
Generating trace-scsi.h with a meson_exe.py custom command
In file included from ../src/qapi/qapi-schema.json:78:
../src/qapi/migration.json:1747:1: unexpected de-indent (expected at least 13 spaces)
make: *** [CUSTOM_COMMAND@d09afa93bc2.stamp] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 709, in <module>
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--rm', '--label', 'com.qemu.instance.uuid=a2209dd2d512478b8d26c94a5b889ce1', '-u', '1003', '--security-opt', 'seccomp=unconfined', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-tvzj2v5u/src/docker-src.2020-09-25-15.22.42.19873:/var/tmp/qemu:z,ro', 'qemu/centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=a2209dd2d512478b8d26c94a5b889ce1
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-tvzj2v5u/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    3m9.538s
user    0m19.483s


The full log is available at
http://patchew.org/logs/20200925162316.21205-1-peter.maydell@linaro.org/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Peter Maydell Sept. 25, 2020, 9:37 p.m. UTC | #6
On Fri, 25 Sep 2020 at 20:25, <no-reply@patchew.org> wrote:

> In file included from ../src/qapi/qapi-schema.json:78:
> ../src/qapi/migration.json:1747:1: unexpected de-indent (expected at least 13 spaces)

This is yet another mis-indented line in a change to the QAPI
doc-comments (commit 4c437254b807). It hit master in the
latest migration pull after I'd sent out this patchseries
but before patchew got round to testing..

thanks
-- PMM
Markus Armbruster Sept. 28, 2020, 1:04 p.m. UTC | #7
Peter Maydell <peter.maydell@linaro.org> writes:

> On Fri, 25 Sep 2020 at 20:25, <no-reply@patchew.org> wrote:
>
>> In file included from ../src/qapi/qapi-schema.json:78:
>> ../src/qapi/migration.json:1747:1: unexpected de-indent (expected at least 13 spaces)
>
> This is yet another mis-indented line in a change to the QAPI
> doc-comments (commit 4c437254b807). It hit master in the
> latest migration pull after I'd sent out this patchseries
> but before patchew got round to testing..

Obvious fixup for your PATCH 01:

diff --git a/qapi/migration.json b/qapi/migration.json
index 7d9342c064..7f5e6fd681 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1744,9 +1744,9 @@
 # Information about current dirty page rate of vm.
 #
 # @dirty-rate: @dirtyrate describing the dirty page rate of vm
-#          in units of MB/s.
-#          If this field returns '-1', it means querying has not
-#          yet started or completed.
+#              in units of MB/s.
+#              If this field returns '-1', it means querying has not
+#              yet started or completed.
 #
 # @status: status containing dirtyrate query status includes
 #          'unstarted' or 'measuring' or 'measured'

Happy to fix it up in my tree.
Peter Maydell Sept. 28, 2020, 1:05 p.m. UTC | #8
On Mon, 28 Sep 2020 at 14:04, Markus Armbruster <armbru@redhat.com> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > On Fri, 25 Sep 2020 at 20:25, <no-reply@patchew.org> wrote:
> >
> >> In file included from ../src/qapi/qapi-schema.json:78:
> >> ../src/qapi/migration.json:1747:1: unexpected de-indent (expected at least 13 spaces)
> >
> > This is yet another mis-indented line in a change to the QAPI
> > doc-comments (commit 4c437254b807). It hit master in the
> > latest migration pull after I'd sent out this patchseries
> > but before patchew got round to testing..
>
> Obvious fixup for your PATCH 01:
>
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 7d9342c064..7f5e6fd681 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -1744,9 +1744,9 @@
>  # Information about current dirty page rate of vm.
>  #
>  # @dirty-rate: @dirtyrate describing the dirty page rate of vm
> -#          in units of MB/s.
> -#          If this field returns '-1', it means querying has not
> -#          yet started or completed.
> +#              in units of MB/s.
> +#              If this field returns '-1', it means querying has not
> +#              yet started or completed.
>  #
>  # @status: status containing dirtyrate query status includes
>  #          'unstarted' or 'measuring' or 'measured'
>
> Happy to fix it up in my tree.

Yes, please.

thanks
-- PMM
Markus Armbruster Sept. 29, 2020, 1:31 p.m. UTC | #9
Peter Maydell <peter.maydell@linaro.org> writes:

> 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.
>
> I think I have now resolved all of Markus' issues raised in his
> review of v5, and IMHO this is ready to commit.  If there are still
> things needing fixing, it would be nice if we were able to commit
> patches 1-5, which are the ones which add the new indent-sensitive
> code to the QAPI parser.  That would put a stop to the steady trickle
> of doc-comment changes which break the new rules...

I found several small things to improve.  I'll now try to address them
in my tree.  If I fail, I'll take PATCH 01-05 now, and ask for a respin
of the rest.

[...]
Markus Armbruster Sept. 29, 2020, 3:26 p.m. UTC | #10
Peter Maydell <peter.maydell@linaro.org> writes:

> On Mon, 28 Sep 2020 at 14:04, Markus Armbruster <armbru@redhat.com> wrote:
>>
>> Peter Maydell <peter.maydell@linaro.org> writes:
>>
>> > On Fri, 25 Sep 2020 at 20:25, <no-reply@patchew.org> wrote:
>> >
>> >> In file included from ../src/qapi/qapi-schema.json:78:
>> >> ../src/qapi/migration.json:1747:1: unexpected de-indent (expected at least 13 spaces)
>> >
>> > This is yet another mis-indented line in a change to the QAPI
>> > doc-comments (commit 4c437254b807). It hit master in the
>> > latest migration pull after I'd sent out this patchseries
>> > but before patchew got round to testing..
>>
>> Obvious fixup for your PATCH 01:
>>
>> diff --git a/qapi/migration.json b/qapi/migration.json
>> index 7d9342c064..7f5e6fd681 100644
>> --- a/qapi/migration.json
>> +++ b/qapi/migration.json
>> @@ -1744,9 +1744,9 @@
>>  # Information about current dirty page rate of vm.
>>  #
>>  # @dirty-rate: @dirtyrate describing the dirty page rate of vm
>> -#          in units of MB/s.
>> -#          If this field returns '-1', it means querying has not
>> -#          yet started or completed.
>> +#              in units of MB/s.
>> +#              If this field returns '-1', it means querying has not
>> +#              yet started or completed.
>>  #
>>  # @status: status containing dirtyrate query status includes
>>  #          'unstarted' or 'measuring' or 'measured'
>>
>> Happy to fix it up in my tree.
>
> Yes, please.

One more issue:

    /work/armbru/qemu/docs/../qapi/machine.json:1000: WARNING: Unexpected indentation.
    /work/armbru/qemu/docs/../qapi/machine.json:1000: WARNING: Block quote ends without a blank line; unexpected unindent.

Line 1000 is at the beginning of a comment block.  Suboptimal.

After a bit of guessing, I arrived at this fix:

diff --git a/qapi/machine.json b/qapi/machine.json
index 7c9e69a9f5..756dacb06f 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1001,9 +1001,11 @@
 #
 # Request the balloon driver to change its balloon size.
 #
-# @value: the target logical size of the VM in bytes
+# @value: the target logical size of the VM in bytes.
 #         We can deduce the size of the balloon using this formula:
+#
 #            logical_vm_size = vm_ram_size - balloon_size
+#
 #         From it we have: balloon_size = vm_ram_size - @value
 #
 # Returns: - Nothing on success

Looks good?
Peter Maydell Sept. 29, 2020, 3:43 p.m. UTC | #11
On Tue, 29 Sep 2020 at 16:26, Markus Armbruster <armbru@redhat.com> wrote:
> One more issue:
>
>     /work/armbru/qemu/docs/../qapi/machine.json:1000: WARNING: Unexpected indentation.
>     /work/armbru/qemu/docs/../qapi/machine.json:1000: WARNING: Block quote ends without a blank line; unexpected unindent.
>
> Line 1000 is at the beginning of a comment block.  Suboptimal.

Yes. There's a comment in qapidoc.py about that:

+            # The reported line number will always be that of the start line
+            # of the doc comment, rather than the actual location of the error.
+            # Being more precise would require overhaul of the QAPIDoc class
+            # to track lines more exactly within all the sub-parts of the doc
+            # comment, as well as counting lines here.

(That is, within qapidoc.py we can count the offset of each line within
this doc comment fragment from the start of the fragment (here, from
the "@value:" line), but only the scripts/qapi code is in a position to
know the offset from the start of the block comment on line 1000 to
the @value line, and it doesn't currently track that.)

> After a bit of guessing, I arrived at this fix:
>
> diff --git a/qapi/machine.json b/qapi/machine.json
> index 7c9e69a9f5..756dacb06f 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -1001,9 +1001,11 @@
>  #
>  # Request the balloon driver to change its balloon size.
>  #
> -# @value: the target logical size of the VM in bytes
> +# @value: the target logical size of the VM in bytes.
>  #         We can deduce the size of the balloon using this formula:
> +#
>  #            logical_vm_size = vm_ram_size - balloon_size
> +#
>  #         From it we have: balloon_size = vm_ram_size - @value
>  #
>  # Returns: - Nothing on success
>
> Looks good?

That will work: it will render the indented text as a block quote.

If you want the formula to be rendered as a fixed-width code
example, you could additionally do s/formula:/formula::/ to
create a rST literal block. But then you'd probably want to
do similar with the equation on the following line, for consistency.

thanks
-- PMM
Markus Armbruster Sept. 29, 2020, 8:17 p.m. UTC | #12
Peter Maydell <peter.maydell@linaro.org> writes:

> 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.
>
> I think I have now resolved all of Markus' issues raised in his
> review of v5, and IMHO this is ready to commit.  If there are still
> things needing fixing, it would be nice if we were able to commit
> patches 1-5, which are the ones which add the new indent-sensitive
> code to the QAPI parser.  That would put a stop to the steady trickle
> of doc-comment changes which break the new rules...
>
> Also available as a git branch at
> https://git.linaro.org/people/peter.maydell/qemu-arm.git sphinx-conversions-v6
>
> 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 chosen 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.
>
> Changes from v5:
>  * rebased (in particular, updated to meson build system)
>  * new patch 1 fixes indent issues that hit master since v5
>  * new patch 2 makes block-latency-histogram-set's use of Example
>    sections match everybody else's, instead of special casing it
>    in the parser
>  * the .gitignore got pruned after meson conversion so we only
>    need to change git.orderfile now
>  * slightly reordered patches to bring the parser.py indent change nearer
>    the start of the series in the hopes of being able to get at least
>    that much of the series into master
>  * we now tell Sphinx about all the json files for dependency info,
>    so editing a json file correctly rebuilds the docs
>  * added a test case for the bad-de-indent parser error
>  * Adopted the various Python scripting suggestions from Markus
>  * We don't insist on section headings being the only thing in their
>    doc comment block any more (the existing "must be first line"
>    requirement is sufficient)
>  * added a test case for doc-generation that does a compare of
>    the sphinx plain-text builder output against a reference file
>  * Added the Python source files for Sphinx extensions (including
>    the QAPI source files) to the dependency lists for the manuals,
>    so that changes made to them correctly trigger a docs rebuild
>  * qemu-ga-ref.rst and qemu-qmp-ref.rst now have a TODO note about
>    making the manual licensing more visible to readers
>  * fixed bug in reported file/line info for some errors in rST
>    in doc comments when using Sphinx 1.6
>  * don't insist on section headers being in their own freeform doc
>    comment block; they're (after commit dcdc07a97cbe) always the
>    first line in the comment block, so just handle the possibility
>    of having text after that.
>
> 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
>
> My view is that we can add niceties like this later; the series
> already seems big enough to me.

Queued, thanks!