[PATCH 0/2] docs: Fix building with Sphinx 3.2

[PATCH 0/2] docs: Fix building with Sphinx 3.2

[Peter Maydell]

This patchseries fixes some issues with building our docs
with Sphinx 3.2:
 * kerneldoc was using the 'c:function' directive for both
   functions and macros, but Sphinx 3.2 wants 'c:macro' for
   macros and complains if the argument to 'c:function' isn't
   parseable as a function declaration
 * qemu-option-trace.rst.inc's use of option:: provokes a
   warning in Sphinx 3.2

We fix the first by making kerneldoc output the right directive
depending on the Sphinx version it's working with, and fix
the second by just dropping our usage of option:: in favour
of a simple definition-list markup.

This does mean our kernel-doc gets another patch that makes
it diverge a little from the kernel's version, but we already
have one of those (commit 152d1967f650f67b7e). I do want to
try to upstream these to the kernel, but that will require
more work I suspect since the kernel makes much more extensive
use of kernel-doc and probably also has other issues when
building with newer Sphinxes. For the moment I would like us
to release QEMU 5.2 with docs that build with all the Sphinxes
we know about.

Tested by building with Sphinx 1.6.1, 2.0, 2.4, 3.0 and 3.2.

thanks
-- PMM

Peter Maydell (2):
  scripts/kerneldoc: For Sphinx 3 use c:macro for macros with arguments
  qemu-option-trace.rst.inc: Don't use option:: markup

 docs/qemu-option-trace.rst.inc |  6 +++---
 scripts/kernel-doc             | 18 +++++++++++++++++-
 2 files changed, 20 insertions(+), 4 deletions(-)

-- 
2.20.1

[PATCH 1/2] scripts/kerneldoc: For Sphinx 3 use c:macro for macros with arguments

[Peter Maydell]

The kerneldoc script currently emits Sphinx markup for a macro with
arguments that uses the c:function directive. This is correct for
Sphinx versions earlier than Sphinx 3, where c:macro doesn't allow
documentation of macros with arguments and c:function is not picky
about the syntax of what it is passed. However, in Sphinx 3 the
c:macro directive was enhanced to support macros with arguments,
and c:function was made more picky about what syntax it accepted.

When kerneldoc is told that it needs to produce output for Sphinx
3 or later, make it emit c:function only for functions and c:macro
for macros with arguments. We assume that anything with a return
type is a function and anything without is a macro.

This fixes the Sphinx error:

/home/petmay01/linaro/qemu-from-laptop/qemu/docs/../include/qom/object.h:155:Error in declarator
If declarator-id with parameters (e.g., 'void f(int arg)'):
  Invalid C declaration: Expected identifier in nested name. [error at 25]
    DECLARE_INSTANCE_CHECKER ( InstanceType,  OBJ_NAME,  TYPENAME)
    -------------------------^
If parenthesis in noptr-declarator (e.g., 'void (*f(int arg))(double)'):
  Error in declarator or parameters
  Invalid C declaration: Expecting "(" in parameters. [error at 39]
    DECLARE_INSTANCE_CHECKER ( InstanceType,  OBJ_NAME,  TYPENAME)
    ---------------------------------------^

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 scripts/kernel-doc | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 0ff62bb6a2d..4fbaaa05e38 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -839,7 +839,23 @@ sub output_function_rst(%) {
 	output_highlight_rst($args{'purpose'});
 	$start = "\n\n**Syntax**\n\n  ``";
     } else {
-	print ".. c:function:: ";
+        if ((split(/\./, $sphinx_version))[0] >= 3) {
+            # Sphinx 3 and later distinguish macros and functions and
+            # complain if you use c:function with something that's not
+            # syntactically valid as a function declaration.
+            # We assume that anything with a return type is a function
+            # and anything without is a macro.
+            if ($args{'functiontype'} ne "") {
+                print ".. c:function:: ";
+            } else {
+                print ".. c:macro:: ";
+            }
+        } else {
+            # Older Sphinx don't support documenting macros that take
+            # arguments with c:macro, and don't complain about the use
+            # of c:function for this.
+            print ".. c:function:: ";
+        }
     }
     if ($args{'functiontype'} ne "") {
 	$start .= $args{'functiontype'} . " " . $args{'function'} . " (";
-- 
2.20.1

Re: [PATCH 1/2] scripts/kerneldoc: For Sphinx 3 use c:macro for macros with arguments

[Daniel P. Berrangé]

On Fri, Oct 30, 2020 at 05:46:59PM +0000, Peter Maydell wrote:
> The kerneldoc script currently emits Sphinx markup for a macro with
> arguments that uses the c:function directive. This is correct for
> Sphinx versions earlier than Sphinx 3, where c:macro doesn't allow
> documentation of macros with arguments and c:function is not picky
> about the syntax of what it is passed. However, in Sphinx 3 the
> c:macro directive was enhanced to support macros with arguments,
> and c:function was made more picky about what syntax it accepted.
> 
> When kerneldoc is told that it needs to produce output for Sphinx
> 3 or later, make it emit c:function only for functions and c:macro
> for macros with arguments. We assume that anything with a return
> type is a function and anything without is a macro.
> 
> This fixes the Sphinx error:
> 
> /home/petmay01/linaro/qemu-from-laptop/qemu/docs/../include/qom/object.h:155:Error in declarator
> If declarator-id with parameters (e.g., 'void f(int arg)'):
>   Invalid C declaration: Expected identifier in nested name. [error at 25]
>     DECLARE_INSTANCE_CHECKER ( InstanceType,  OBJ_NAME,  TYPENAME)
>     -------------------------^
> If parenthesis in noptr-declarator (e.g., 'void (*f(int arg))(double)'):
>   Error in declarator or parameters
>   Invalid C declaration: Expecting "(" in parameters. [error at 39]
>     DECLARE_INSTANCE_CHECKER ( InstanceType,  OBJ_NAME,  TYPENAME)
>     ---------------------------------------^
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  scripts/kernel-doc | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 1/2] scripts/kerneldoc: For Sphinx 3 use c:macro for macros with arguments

[Stefan Hajnoczi]

On Fri, Oct 30, 2020 at 5:47 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> The kerneldoc script currently emits Sphinx markup for a macro with
> arguments that uses the c:function directive. This is correct for
> Sphinx versions earlier than Sphinx 3, where c:macro doesn't allow
> documentation of macros with arguments and c:function is not picky
> about the syntax of what it is passed. However, in Sphinx 3 the
> c:macro directive was enhanced to support macros with arguments,
> and c:function was made more picky about what syntax it accepted.
>
> When kerneldoc is told that it needs to produce output for Sphinx
> 3 or later, make it emit c:function only for functions and c:macro
> for macros with arguments. We assume that anything with a return
> type is a function and anything without is a macro.
>
> This fixes the Sphinx error:
>
> /home/petmay01/linaro/qemu-from-laptop/qemu/docs/../include/qom/object.h:155:Error in declarator
> If declarator-id with parameters (e.g., 'void f(int arg)'):
>   Invalid C declaration: Expected identifier in nested name. [error at 25]
>     DECLARE_INSTANCE_CHECKER ( InstanceType,  OBJ_NAME,  TYPENAME)
>     -------------------------^
> If parenthesis in noptr-declarator (e.g., 'void (*f(int arg))(double)'):
>   Error in declarator or parameters
>   Invalid C declaration: Expecting "(" in parameters. [error at 39]
>     DECLARE_INSTANCE_CHECKER ( InstanceType,  OBJ_NAME,  TYPENAME)
>     ---------------------------------------^
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  scripts/kernel-doc | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)

Tested-by: Stefan Hajnoczi <stefanha@redhat.com>

[PATCH 2/2] qemu-option-trace.rst.inc: Don't use option:: markup

[Peter Maydell]

Sphinx 3.2 is pickier than earlier versions about the option:: markup,
and complains about our usage in qemu-option-trace.rst:

../../docs/qemu-option-trace.rst.inc:4:Malformed option description
  '[enable=]PATTERN', should look like "opt", "-opt args", "--opt args",
  "/opt args" or "+opt args"

In this file, we're really trying to document the different parts of
the top-level --trace option, which qemu-nbd.rst and qemu-img.rst
have already introduced with an option:: markup.  So it's not right
to use option:: here anyway.  Switch to a different markup
(definition lists) which gives about the same formatted output.

(Unlike option::, this markup doesn't produce index entries; but
at the moment we don't do anything much with indexes anyway, and
in any case I think it doesn't make much sense to have individual
index entries for the sub-parts of the --trace option.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 docs/qemu-option-trace.rst.inc | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/docs/qemu-option-trace.rst.inc b/docs/qemu-option-trace.rst.inc
index 7e09773a9c5..d7acbe67f73 100644
--- a/docs/qemu-option-trace.rst.inc
+++ b/docs/qemu-option-trace.rst.inc
@@ -1,7 +1,7 @@
 
 Specify tracing options.
 
-.. option:: [enable=]PATTERN
+``[enable=]PATTERN``
 
   Immediately enable events matching *PATTERN*
   (either event name or a globbing pattern).  This option is only
@@ -11,7 +11,7 @@ Specify tracing options.
 
   Use :option:`-trace help` to print a list of names of trace points.
 
-.. option:: events=FILE
+``events=FILE``
 
   Immediately enable events listed in *FILE*.
   The file must contain one event name (as listed in the ``trace-events-all``
@@ -19,7 +19,7 @@ Specify tracing options.
   available if QEMU has been compiled with the ``simple``, ``log`` or
   ``ftrace`` tracing backend.
 
-.. option:: file=FILE
+``file=FILE``
 
   Log output traces to *FILE*.
   This option is only available if QEMU has been compiled with
-- 
2.20.1

Re: [PATCH 2/2] qemu-option-trace.rst.inc: Don't use option:: markup

[Daniel P. Berrangé]

On Fri, Oct 30, 2020 at 05:47:00PM +0000, Peter Maydell wrote:
> Sphinx 3.2 is pickier than earlier versions about the option:: markup,
> and complains about our usage in qemu-option-trace.rst:
> 
> ../../docs/qemu-option-trace.rst.inc:4:Malformed option description
>   '[enable=]PATTERN', should look like "opt", "-opt args", "--opt args",
>   "/opt args" or "+opt args"
> 
> In this file, we're really trying to document the different parts of
> the top-level --trace option, which qemu-nbd.rst and qemu-img.rst
> have already introduced with an option:: markup.  So it's not right
> to use option:: here anyway.  Switch to a different markup
> (definition lists) which gives about the same formatted output.
> 
> (Unlike option::, this markup doesn't produce index entries; but
> at the moment we don't do anything much with indexes anyway, and
> in any case I think it doesn't make much sense to have individual
> index entries for the sub-parts of the --trace option.)
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  docs/qemu-option-trace.rst.inc | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 2/2] qemu-option-trace.rst.inc: Don't use option:: markup

[Stefan Hajnoczi]

On Fri, Oct 30, 2020 at 5:48 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> Sphinx 3.2 is pickier than earlier versions about the option:: markup,
> and complains about our usage in qemu-option-trace.rst:
>
> ../../docs/qemu-option-trace.rst.inc:4:Malformed option description
>   '[enable=]PATTERN', should look like "opt", "-opt args", "--opt args",
>   "/opt args" or "+opt args"
>
> In this file, we're really trying to document the different parts of
> the top-level --trace option, which qemu-nbd.rst and qemu-img.rst
> have already introduced with an option:: markup.  So it's not right
> to use option:: here anyway.  Switch to a different markup
> (definition lists) which gives about the same formatted output.
>
> (Unlike option::, this markup doesn't produce index entries; but
> at the moment we don't do anything much with indexes anyway, and
> in any case I think it doesn't make much sense to have individual
> index entries for the sub-parts of the --trace option.)
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  docs/qemu-option-trace.rst.inc | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Tested-by: Stefan Hajnoczi <stefanha@redhat.com>

Re: [PATCH 0/2] docs: Fix building with Sphinx 3.2

[Paolo Bonzini]

On 30/10/20 18:46, Peter Maydell wrote:
> 
> This does mean our kernel-doc gets another patch that makes
> it diverge a little from the kernel's version, but we already
> have one of those (commit 152d1967f650f67b7e). I do want to
> try to upstream these to the kernel, but that will require
> more work I suspect since the kernel makes much more extensive
> use of kernel-doc and probably also has other issues when
> building with newer Sphinxes. For the moment I would like us
> to release QEMU 5.2 with docs that build with all the Sphinxes
> we know about.

FWIW I've sent to Linux our other two local patches, and if you are okay
with it I can also do the sync in the other direction before 5.2 (the
plan was to do it afterwards).

Paolo

Re: [PATCH 0/2] docs: Fix building with Sphinx 3.2

[Peter Maydell]

On Fri, 30 Oct 2020 at 18:20, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 30/10/20 18:46, Peter Maydell wrote:
> >
> > This does mean our kernel-doc gets another patch that makes
> > it diverge a little from the kernel's version, but we already
> > have one of those (commit 152d1967f650f67b7e). I do want to
> > try to upstream these to the kernel, but that will require
> > more work I suspect since the kernel makes much more extensive
> > use of kernel-doc and probably also has other issues when
> > building with newer Sphinxes. For the moment I would like us
> > to release QEMU 5.2 with docs that build with all the Sphinxes
> > we know about.
>
> FWIW I've sent to Linux our other two local patches, and if you are okay
> with it I can also do the sync in the other direction before 5.2 (the
> plan was to do it afterwards).

Probably safer to sync afterwards. Do you have a link to the
patches you've sent in the Linux direction ?

thanks
-- PMM

Re: [PATCH 0/2] docs: Fix building with Sphinx 3.2

[Peter Maydell]

On Fri, 30 Oct 2020 at 19:12, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Fri, 30 Oct 2020 at 18:20, Paolo Bonzini <pbonzini@redhat.com> wrote:
> >
> > On 30/10/20 18:46, Peter Maydell wrote:
> > >
> > > This does mean our kernel-doc gets another patch that makes
> > > it diverge a little from the kernel's version, but we already
> > > have one of those (commit 152d1967f650f67b7e). I do want to
> > > try to upstream these to the kernel, but that will require
> > > more work I suspect since the kernel makes much more extensive
> > > use of kernel-doc and probably also has other issues when
> > > building with newer Sphinxes. For the moment I would like us
> > > to release QEMU 5.2 with docs that build with all the Sphinxes
> > > we know about.
> >
> > FWIW I've sent to Linux our other two local patches, and if you are okay
> > with it I can also do the sync in the other direction before 5.2 (the
> > plan was to do it afterwards).
>
> Probably safer to sync afterwards. Do you have a link to the
> patches you've sent in the Linux direction ?

Having actually looked at the current state of the kernel's
kernel-doc script I see somebody has already done the
necessary updates for Sphinx 3 compatibility. So we have
two choices for 5.2:
 * take this patch 1 as a minimal fix
 * do the sync of the kernel's version of the script

I'll have a look at how painful or otherwise the sync is, I guess.
We only use the kerneldoc stuff in the 'devel' manual which
is not really userfacing anyway.

thanks
-- PMM

Re: [PATCH 0/2] docs: Fix building with Sphinx 3.2

[Paolo Bonzini]

On 30/10/20 21:02, Peter Maydell wrote:
> Having actually looked at the current state of the kernel's
> kernel-doc script I see somebody has already done the
> necessary updates for Sphinx 3 compatibility. So we have
> two choices for 5.2:
>  * take this patch 1 as a minimal fix
>  * do the sync of the kernel's version of the script
> 
> I'll have a look at how painful or otherwise the sync is, I guess.
> We only use the kerneldoc stuff in the 'devel' manual which
> is not really userfacing anyway.

Oh nice! I had a slightly older Linux checkout and it wasn't there yet.
 I think you can go ahead with these two patches, I'll take care of the
sync for 6.0.

Here are our two patches.

https://lore.kernel.org/linux-doc/20201030144713.201372-1-pbonzini@redhat.com/T/#t