[PATCH for-5.0? 0/3] Make docs build work with Sphinx 3

[PATCH for-5.0? 0/3] Make docs build work with Sphinx 3

[Peter Maydell]

Our current docs don't build with Sphinx 3, as noted in
https://bugs.launchpad.net/bugs/1872113 -- this is a combination of:
 (1) we are using the sphinx-build -W option so warnings are treated
     as errors
 (3) a kernel-doc script bug meant it was omitting a close-paren
     when a function parameter was a function pointer; older Sphinx
     ignored this but Sphinx 3 parses the function declaration and
     warns about it; and because of (1) this is fatal to the QEMU build
 (2) Sphinx 3 makes a breaking change in how it wants C structs
     to be marked up (moving from 'c:type:: struct Foo' to
     'c:struct:: Foo'); our use of the old syntax provokes a
     warning, which again because of point (1) is fatal

Patch 1 extends configure's --disable-werror to cover Sphinx as
well as the C compiler, so that at least there is a workaround
(which will be automatic for release builds).

Patch 2 fixes the trivial kernel-doc bug.

Patch 3 adds and uses a new --sphinx-version option to kernel-doc,
so that our Sphinx plugin can pass the Sphinx version down and
the script can then choose the right syntax.

I've marked this up as 'for-5.0?' because I think it would be
nice if at least patch 1 went in. Patch 2 seems uncontroversial
(though I guess we should forward it up to the kernel folks
since kernel-doc is from them originally). Patch 3 is the
expedient change, but you could argue about whether this is
the best way to tell kernel-doc what to do.

thanks
-- PMM

Peter Maydell (3):
  configure: Honour --disable-werror for Sphinx
  scripts/kernel-doc: Add missing close-paren in c:function directives
  kernel-doc: Use c:struct for Sphinx 3.0 and later

 configure                |  9 ++++++++-
 Makefile                 |  2 +-
 docs/sphinx/kerneldoc.py |  1 +
 scripts/kernel-doc       | 18 ++++++++++++++++--
 4 files changed, 26 insertions(+), 4 deletions(-)

-- 
2.20.1

[PATCH for-5.0? 1/3] configure: Honour --disable-werror for Sphinx

[Peter Maydell]

If we are not making warnings fatal for compilation, make them
non-fatal when building the Sphinx documentation also.  (For instance
Sphinx 3.0 warns about some constructs that older versions were happy
with, which is a build failure if we use the warnings-as-errors
flag.)

This provides a workaround at least for LP:1872113.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 configure | 9 ++++++++-
 Makefile  | 2 +-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 22870f38672..7b9ae0418d0 100755
--- a/configure
+++ b/configure
@@ -4928,6 +4928,12 @@ if check_include sys/kcov.h ; then
     kcov=yes
 fi
 
+# If we're making warnings fatal, apply this to Sphinx runs as well
+sphinx_werror=""
+if test "$werror" = "yes"; then
+    sphinx_werror="-W"
+fi
+
 # Check we have a new enough version of sphinx-build
 has_sphinx_build() {
     # This is a bit awkward but works: create a trivial document and
@@ -4936,7 +4942,7 @@ has_sphinx_build() {
     # sphinx-build doesn't exist at all or if it is too old.
     mkdir -p "$TMPDIR1/sphinx"
     touch "$TMPDIR1/sphinx/index.rst"
-    "$sphinx_build" -c "$source_path/docs" -b html "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 2>&1
+    "$sphinx_build" $sphinx_werror -c "$source_path/docs" -b html "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 2>&1
 }
 
 # Check if tools are available to build documentation.
@@ -7631,6 +7637,7 @@ echo "INSTALL_PROG=$install -c -m 0755" >> $config_host_mak
 echo "INSTALL_LIB=$install -c -m 0644" >> $config_host_mak
 echo "PYTHON=$python" >> $config_host_mak
 echo "SPHINX_BUILD=$sphinx_build" >> $config_host_mak
+echo "SPHINX_WERROR=$sphinx_werror" >> $config_host_mak
 echo "GENISOIMAGE=$genisoimage" >> $config_host_mak
 echo "CC=$cc" >> $config_host_mak
 if $iasl -h > /dev/null 2>&1; then
diff --git a/Makefile b/Makefile
index 84ef8816000..8a9113e6663 100644
--- a/Makefile
+++ b/Makefile
@@ -1076,7 +1076,7 @@ sphinxdocs: $(MANUAL_BUILDDIR)/devel/index.html \
 # Note the use of different doctree for each (manual, builder) tuple;
 # this works around Sphinx not handling parallel invocation on
 # a single doctree: https://github.com/sphinx-doc/sphinx/issues/2946
-build-manual = $(call quiet-command,CONFDIR="$(qemu_confdir)" $(SPHINX_BUILD) $(if $(V),,-q) -W -b $2 -D version=$(VERSION) -D release="$(FULL_VERSION)" -d .doctrees/$1-$2 $(SRC_PATH)/docs/$1 $(MANUAL_BUILDDIR)/$1 ,"SPHINX","$(MANUAL_BUILDDIR)/$1")
+build-manual = $(call quiet-command,CONFDIR="$(qemu_confdir)" $(SPHINX_BUILD) $(if $(V),,-q) $(SPHINX_WERROR) -b $2 -D version=$(VERSION) -D release="$(FULL_VERSION)" -d .doctrees/$1-$2 $(SRC_PATH)/docs/$1 $(MANUAL_BUILDDIR)/$1 ,"SPHINX","$(MANUAL_BUILDDIR)/$1")
 # We assume all RST files in the manual's directory are used in it
 manual-deps = $(wildcard $(SRC_PATH)/docs/$1/*.rst $(SRC_PATH)/docs/$1/*/*.rst) \
               $(SRC_PATH)/docs/defs.rst.inc \
-- 
2.20.1

[PATCH for-5.0? 2/3] scripts/kernel-doc: Add missing close-paren in c:function directives

[Peter Maydell]

When kernel-doc generates a 'c:function' directive for a function
one of whose arguments is a function pointer, it fails to print
the close-paren after the argument list of the function pointer
argument, for instance:
  .. c:function:: void memory_region_init_resizeable_ram (MemoryRegion * mr, struct Object * owner, const char * name, uint64_t size, uint64_t max_size, void (*resized) (const char*, uint64_t length, void *host, Error ** errp)

which should have a ')' after the 'void *host' which is the
last argument to 'resized'.

Older versions of Sphinx don't try to parse the argumnet
to c:function, but Sphinx 3.0 does do this and will complain:

  /home/petmay01/linaro/qemu-from-laptop/qemu/docs/../include/exec/memory.h:834: WARNING: Error in declarator or parameters
  Invalid C declaration: Expecting "," or ")" in parameters, got "EOF". [error at 208]
    void memory_region_init_resizeable_ram (MemoryRegion * mr, struct Object * owner, const char * name, uint64_t size, uint64_t max_size, void (*resized) (const char*, uint64_t length, void *host, Error ** errp)
    ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------^

Add the missing close-paren.

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

diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index af470eb3211..8dc30e01e58 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -853,7 +853,7 @@ sub output_function_rst(%) {
 
 	if ($type =~ m/([^\(]*\(\*)\s*\)\s*\(([^\)]*)\)/) {
 	    # pointer-to-function
-	    print $1 . $parameter . ") (" . $2;
+	    print $1 . $parameter . ") (" . $2 . ")";
 	} else {
 	    print $type . " " . $parameter;
 	}
-- 
2.20.1

[PATCH for-5.0? 3/3] kernel-doc: Use c:struct for Sphinx 3.0 and later

[Peter Maydell]

The kernel-doc Sphinx plugin and associated script currently emit
'c:type' directives for "struct foo" documentation.

Sphinx 3.0 warns about this:
  /home/petmay01/linaro/qemu-from-laptop/qemu/docs/../include/exec/memory.h:3: WARNING: Type must be either just a name or a typedef-like declaration.
  If just a name:
    Error in declarator or parameters
    Invalid C declaration: Expected identifier in nested name, got keyword: struct [error at 6]
      struct MemoryListener
      ------^
  If typedef-like declaration:
    Error in declarator or parameters
    Invalid C declaration: Expected identifier in nested name. [error at 21]
      struct MemoryListener
      ---------------------^

because it wants us to use the new-in-3.0 'c:struct' instead.

Plumb the Sphinx version through to the kernel-doc script
and use it to select 'c:struct' for newer versions than 3.0.

Fixes: LP:1872113
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 docs/sphinx/kerneldoc.py |  1 +
 scripts/kernel-doc       | 16 +++++++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/docs/sphinx/kerneldoc.py b/docs/sphinx/kerneldoc.py
index 1159405cb92..3e879402064 100644
--- a/docs/sphinx/kerneldoc.py
+++ b/docs/sphinx/kerneldoc.py
@@ -99,6 +99,7 @@ class KernelDocDirective(Directive):
                 env.note_dependency(os.path.abspath(f))
                 cmd += ['-export-file', f]
 
+        cmd += ['-sphinx-version', sphinx.__version__]
         cmd += [filename]
 
         try:
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 8dc30e01e58..030b5c8691f 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -71,6 +71,8 @@ Output selection (mutually exclusive):
 			DOC: sections. May be specified multiple times.
 
 Output selection modifiers:
+  -sphinx-version VER   Generate rST syntax for the specified Sphinx version.
+                        Only works with reStructuredTextFormat.
   -no-doc-sections	Do not output DOC: sections.
   -enable-lineno        Enable output of #define LINENO lines. Only works with
                         reStructuredText format.
@@ -286,6 +288,7 @@ use constant {
 };
 my $output_selection = OUTPUT_ALL;
 my $show_not_found = 0;	# No longer used
+my $sphinx_version = "0.0"; # if not specified, assume old
 
 my @export_file_list;
 
@@ -436,6 +439,8 @@ while ($ARGV[0] =~ m/^--?(.*)/) {
 	    $enable_lineno = 1;
     } elsif ($cmd eq 'show-not-found') {
 	$show_not_found = 1;  # A no-op but don't fail
+    } elsif ($cmd eq 'sphinx-version') {
+        $sphinx_version = shift @ARGV;
     } else {
 	# Unknown argument
         usage();
@@ -963,7 +968,16 @@ sub output_struct_rst(%) {
     my $oldprefix = $lineprefix;
     my $name = $args{'type'} . " " . $args{'struct'};
 
-    print "\n\n.. c:type:: " . $name . "\n\n";
+    # Sphinx 3.0 and up will emit warnings for "c:type:: struct Foo".
+    # It wants to see "c:struct:: Foo" (and will add the word 'struct' in
+    # the rendered output).
+    if ((split(/\./, $sphinx_version))[0] >= 3) {
+        my $sname = $name;
+        $sname =~ s/^struct //;
+        print "\n\n.. c:struct:: " . $sname . "\n\n";
+    } else {
+        print "\n\n.. c:type:: " . $name . "\n\n";
+    }
     print_lineno($declaration_start_line);
     $lineprefix = "   ";
     output_highlight_rst($args{'purpose'});
-- 
2.20.1

Re: [PATCH for-5.0? 1/3] configure: Honour --disable-werror for Sphinx

[Richard Henderson]

On 4/11/20 11:29 AM, Peter Maydell wrote:
> If we are not making warnings fatal for compilation, make them
> non-fatal when building the Sphinx documentation also.  (For instance
> Sphinx 3.0 warns about some constructs that older versions were happy
> with, which is a build failure if we use the warnings-as-errors
> flag.)
> 
> This provides a workaround at least for LP:1872113.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  configure | 9 ++++++++-
>  Makefile  | 2 +-
>  2 files changed, 9 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH for-5.0? 2/3] scripts/kernel-doc: Add missing close-paren in c:function directives

[Richard Henderson]

On 4/11/20 11:29 AM, Peter Maydell wrote:
> When kernel-doc generates a 'c:function' directive for a function
> one of whose arguments is a function pointer, it fails to print
> the close-paren after the argument list of the function pointer
> argument, for instance:
>   .. c:function:: void memory_region_init_resizeable_ram (MemoryRegion * mr, struct Object * owner, const char * name, uint64_t size, uint64_t max_size, void (*resized) (const char*, uint64_t length, void *host, Error ** errp)
> 
> which should have a ')' after the 'void *host' which is the
> last argument to 'resized'.
> 
> Older versions of Sphinx don't try to parse the argumnet
> to c:function, but Sphinx 3.0 does do this and will complain:
> 
>   /home/petmay01/linaro/qemu-from-laptop/qemu/docs/../include/exec/memory.h:834: WARNING: Error in declarator or parameters
>   Invalid C declaration: Expecting "," or ")" in parameters, got "EOF". [error at 208]
>     void memory_region_init_resizeable_ram (MemoryRegion * mr, struct Object * owner, const char * name, uint64_t size, uint64_t max_size, void (*resized) (const char*, uint64_t length, void *host, Error ** errp)
>     ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------^
> 
> Add the missing close-paren.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  scripts/kernel-doc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH for-5.0? 0/3] Make docs build work with Sphinx 3

[Paolo Bonzini]

On 11/04/20 20:29, Peter Maydell wrote:
> 
> I've marked this up as 'for-5.0?' because I think it would be
> nice if at least patch 1 went in. Patch 2 seems uncontroversial
> (though I guess we should forward it up to the kernel folks
> since kernel-doc is from them originally). Patch 3 is the
> expedient change, but you could argue about whether this is
> the best way to tell kernel-doc what to do.

I agree---I would say this is ok for 5.0 as long as the last two patches
are forwarded to the kernel and any changes integrated back.

Patch 1 is clever. :)

Paolo

> 
> thanks
> -- PMM
> 
> Peter Maydell (3):
>   configure: Honour --disable-werror for Sphinx
>   scripts/kernel-doc: Add missing close-paren in c:function directives
>   kernel-doc: Use c:struct for Sphinx 3.0 and later

Supported Sphinx Versions (was: Re: [PATCH for-5.0? 0/3] Make docs build work with Sphinx 3)

[John Snow]

On 4/11/20 2:29 PM, Peter Maydell wrote:
> Our current docs don't build with Sphinx 3, as noted in
> https://bugs.launchpad.net/bugs/1872113 -- this is a combination of:
>  (1) we are using the sphinx-build -W option so warnings are treated
>      as errors
>  (3) a kernel-doc script bug meant it was omitting a close-paren
>      when a function parameter was a function pointer; older Sphinx
>      ignored this but Sphinx 3 parses the function declaration and
>      warns about it; and because of (1) this is fatal to the QEMU build
>  (2) Sphinx 3 makes a breaking change in how it wants C structs
>      to be marked up (moving from 'c:type:: struct Foo' to
>      'c:struct:: Foo'); our use of the old syntax provokes a
>      warning, which again because of point (1) is fatal
> 
> Patch 1 extends configure's --disable-werror to cover Sphinx as
> well as the C compiler, so that at least there is a workaround
> (which will be automatic for release builds).
> 
> Patch 2 fixes the trivial kernel-doc bug.
> 
> Patch 3 adds and uses a new --sphinx-version option to kernel-doc,
> so that our Sphinx plugin can pass the Sphinx version down and
> the script can then choose the right syntax.
> 
> I've marked this up as 'for-5.0?' because I think it would be
> nice if at least patch 1 went in. Patch 2 seems uncontroversial
> (though I guess we should forward it up to the kernel folks
> since kernel-doc is from them originally). Patch 3 is the
> expedient change, but you could argue about whether this is
> the best way to tell kernel-doc what to do.
> 
> thanks
> -- PMM
> 

I was curious about our actual version compatibility, so I did some testing.

I modified configure to prefer 'sphinx-build' over 'sphinx-build-3' so
it would use my venv version, and then;

From my qemu build directory (~/src/qemu/bin/git):

> python3 -m venv 200
> source ./200/bin/activate.fish
> pip install sphinx==2.0.0
> ../../configure --target-list='x86_64-softmmu' --enable-debug
--enable-docs; and cat config-host.mak | grep sphinx; and make html;


Repeat the process for the major versions we believe that we support in
the abstract (1.3.x through 2.4.x):

1.3: Can't even pass the configure check with a blank document.

Exception occurred:
  File
"/home/jhuston/src/qemu/bin/git/130/lib64/python3.7/site-packages/sphinx/environment.py",
line 146, in __init__
    FileInput.__init__(self, *args, **kwds)
TypeError: __init__() got an unexpected keyword argument 'handle_io_errors'
The full traceback has been saved in /tmp/sphinx-err-owwisn63.log, if
you want to report the issue to the developers.

No idea.


1.4 - 1.4.9: Fails to build.

Warning, treated as error:
/home/jsnow/src/qemu/docs/system/images.rst:4: SEVERE: Duplicate ID:
"cmdoption-qcow2-arg-encrypt".

It doesn't seem to like the "encrypt.FOO" section names here and
considers them duplicates, cutting off at the '.'.

Not clear if there's a fix, or if it's worth caring about.


1.5 - 1.5.6: Fails to build.

Warning, treated as error:
/home/jsnow/src/qemu/docs/system/invocation.rst:544: WARNING: Malformed
option description '[enable=]PATTERN', should look like "opt", "-opt
args", "--opt args", "/opt args" or "+opt args"

... I actually don't know where this one goes wrong; that's not a valid
line number there. device-url-syntax.rst.inc isn't that long either, so
I don't know what this correlates to, unfortunately.

1.6.1 through 2.2.2 all appear to work just fine, but produce a lot of
warnings about a coming incompatibility with Docutils > 0.16.

2.3.0 - 2.4.4 work silently with Docutils 0.16.

3.0.0: Notably fails, as is the subject of this patch. :)




Conclusion:

Required: >= 1.6.1
Recommended: >= 2.3.0



We can make this a little easier by using python virtual environments as
part of our build tree; we can freeze version dependencies if we want to
get more reproducible python builds.

We might also begin "installing" the QAPI generator module into such a
virtual environment such that the include statements are written in a
more formal manner, which will assist for pylint and mypy gating, but
that's another email.


I want to send patches to:

1. Change configure to try and prefer a virtualenv version of
sphinx-build, when found

2. Change sphinx conf.py to require >= 1.6.1 so that the requirement is
documented

3. Update documentation (somewhere?) explaining our sphinx dependency
and which versions are required and why ("Because 1.5.x does not work
with our tree.")

--js

Re: Supported Sphinx Versions (was: Re: [PATCH for-5.0? 0/3] Make docs build work with Sphinx 3)

[Peter Maydell]

On Mon, 13 Apr 2020 at 19:08, John Snow <jsnow@redhat.com> wrote:
> I was curious about our actual version compatibility, so I did some testing.

Thanks for doing the testing.

> 1.6.1 through 2.2.2 all appear to work just fine, but produce a lot of
> warnings about a coming incompatibility with Docutils > 0.16.

FWIW, I don't get this warning with the stock Ubuntu
1.6.7. The only time I did see it was when I'd managed
to accidentally install half of Sphinx 3 to my ~/.local
directory and I think it was the system Sphinx and an
upgraded docutils or some other weird combo.

> Conclusion:
>
> Required: >= 1.6.1
> Recommended: >= 2.3.0

I think that what we actually care about is the usual thing:
do we build OK with the version of sphinx-build shipped by
every distro on our support list?

thanks
-- PMM

Re: Supported Sphinx Versions (was: Re: [PATCH for-5.0? 0/3] Make docs build work with Sphinx 3)

[John Snow]

On 4/13/20 2:22 PM, Peter Maydell wrote:
> On Mon, 13 Apr 2020 at 19:08, John Snow <jsnow@redhat.com> wrote:
>> I was curious about our actual version compatibility, so I did some testing.
> 
> Thanks for doing the testing.
> 
>> 1.6.1 through 2.2.2 all appear to work just fine, but produce a lot of
>> warnings about a coming incompatibility with Docutils > 0.16.
> 
> FWIW, I don't get this warning with the stock Ubuntu
> 1.6.7. The only time I did see it was when I'd managed
> to accidentally install half of Sphinx 3 to my ~/.local
> directory and I think it was the system Sphinx and an
> upgraded docutils or some other weird combo.
> 

Yeah, it depends on what versions you pull in. I am using `pip` to
install sphinx straight from PyPI, and the version dependency resolution
opts for "the latest that isn't prohibited by the repository", which
means that I am using (very likely) some cutting edge dependencies for
an older version of sphinx.

That's OK, it works just fine -- just a note, is all. It likely works
completely quietly if you scoot back down to Docutils 0.15.

(The requirements specify only Docutils >= 0.12. Eventually, older
sphinx installations may break when Docutils 0.17 comes out unless you
start pinning versions manually.)

>> Conclusion:
>>
>> Required: >= 1.6.1
>> Recommended: >= 2.3.0
> 
> I think that what we actually care about is the usual thing:
> do we build OK with the version of sphinx-build shipped by
> every distro on our support list?

Sure; if any distro ships a version that's outside of what I laid out
above it would be good to fix and check.

We can also tighten and document the versions so if we do fall outside
of that by accident, we'll catch it during RC testing phase.

I'm using this to make a quick assessment:
https://repology.org/project/python:sphinx/versions

Fedora:
    30: 1.8.4
    31: 2.1.2

OpenSUSE:
    15.1: 1.7.6

Ubuntu:
    19.10: 1.8.5
    20.04/LTS: 1.8.5

Debian:
    8/Jessie: We don't support this anymore AFAIUI.
    9/Stretch: 1.4.8 -- Broken at present!
    10/Buster: 1.8.3

Ubuntu LTS:
    16.04: Dropped
    18.04: 1.6.7
    20.04: 1.8.5

RHEL:
    EPEL7: 1.2.3 -- way, way too old!
    RHEL8: 1.7.6 [via CentOS8]



We *might* need to do some surgery to support Stretch, and EPEL7 fell
off the wagon entirely if repology is to believe -- it doesn't support
our stated minimum of simply having the "Alabaster" theme, which comes
in 1.3.

For RHEL7 we *could* start using a virtual environment, which would help
alleviate the wide version spread.

...are we opposed to this kind of thing? Has there been a discussion before?

Targeting the native repo versions is nice (and we should continue to
make a best effort there), but we *could* start offering a virtual
python environment for the builds that grabs very precise versions.

--js

Re: Supported Sphinx Versions

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> On 4/13/20 2:22 PM, Peter Maydell wrote:
>> On Mon, 13 Apr 2020 at 19:08, John Snow <jsnow@redhat.com> wrote:
>>> I was curious about our actual version compatibility, so I did some testing.
>> 
>> Thanks for doing the testing.
>> 
>>> 1.6.1 through 2.2.2 all appear to work just fine, but produce a lot of
>>> warnings about a coming incompatibility with Docutils > 0.16.
>> 
>> FWIW, I don't get this warning with the stock Ubuntu
>> 1.6.7. The only time I did see it was when I'd managed
>> to accidentally install half of Sphinx 3 to my ~/.local
>> directory and I think it was the system Sphinx and an
>> upgraded docutils or some other weird combo.
>> 
>
> Yeah, it depends on what versions you pull in. I am using `pip` to
> install sphinx straight from PyPI, and the version dependency resolution
> opts for "the latest that isn't prohibited by the repository", which
> means that I am using (very likely) some cutting edge dependencies for
> an older version of sphinx.
>
> That's OK, it works just fine -- just a note, is all. It likely works
> completely quietly if you scoot back down to Docutils 0.15.
>
> (The requirements specify only Docutils >= 0.12. Eventually, older
> sphinx installations may break when Docutils 0.17 comes out unless you
> start pinning versions manually.)
>
>>> Conclusion:
>>>
>>> Required: >= 1.6.1
>>> Recommended: >= 2.3.0
>> 
>> I think that what we actually care about is the usual thing:
>> do we build OK with the version of sphinx-build shipped by
>> every distro on our support list?
>
> Sure; if any distro ships a version that's outside of what I laid out
> above it would be good to fix and check.
>
> We can also tighten and document the versions so if we do fall outside
> of that by accident, we'll catch it during RC testing phase.
>
> I'm using this to make a quick assessment:
> https://repology.org/project/python:sphinx/versions
>
> Fedora:
>     30: 1.8.4
>     31: 2.1.2
>
> OpenSUSE:
>     15.1: 1.7.6
>
> Ubuntu:
>     19.10: 1.8.5
>     20.04/LTS: 1.8.5
>
> Debian:
>     8/Jessie: We don't support this anymore AFAIUI.

Correct.

docs/system/build-platforms.rst:

    For distributions with long-lifetime releases, the project will aim
    to support the most recent major version at all times.  Support for
    the previous major version will be dropped 2 years after the new
    major version is released, or when it reaches "end of life".

Debian 8 reached end of life in 2018, one year after 9's release.

>     9/Stretch: 1.4.8 -- Broken at present!

Will reach end of life this summer, one year after 10's release.

>     10/Buster: 1.8.3
>
> Ubuntu LTS:
>     16.04: Dropped
>     18.04: 1.6.7
>     20.04: 1.8.5
>
> RHEL:
>     EPEL7: 1.2.3 -- way, way too old!
>     RHEL8: 1.7.6 [via CentOS8]
>
>
>
> We *might* need to do some surgery to support Stretch, and EPEL7 fell
> off the wagon entirely if repology is to believe -- it doesn't support
> our stated minimum of simply having the "Alabaster" theme, which comes
> in 1.3.
>
> For RHEL7 we *could* start using a virtual environment, which would help
> alleviate the wide version spread.
>
> ...are we opposed to this kind of thing? Has there been a discussion before?
>
> Targeting the native repo versions is nice (and we should continue to
> make a best effort there), but we *could* start offering a virtual
> python environment for the builds that grabs very precise versions.
>
> --js

Re: Supported Sphinx Versions (was: Re: [PATCH for-5.0? 0/3] Make docs build work with Sphinx 3)

[Peter Maydell]

On Mon, 13 Apr 2020 at 19:08, John Snow <jsnow@redhat.com> wrote:
> 1.5 - 1.5.6: Fails to build.
>
> Warning, treated as error:
> /home/jsnow/src/qemu/docs/system/invocation.rst:544: WARNING: Malformed
> option description '[enable=]PATTERN', should look like "opt", "-opt
> args", "--opt args", "/opt args" or "+opt args"
>
> ... I actually don't know where this one goes wrong; that's not a valid
> line number there. device-url-syntax.rst.inc isn't that long either, so
> I don't know what this correlates to, unfortunately.

https://github.com/sphinx-doc/sphinx/issues/3366
is probably relevant here: the regex controlling what Sphinx
thinks is a valid option string was relaxed.

The "1.4 fails with duplicate ID warnings" part is probably
https://github.com/sphinx-doc/sphinx/issues/646
which is a similar earlier issue where the option string
was relaxed to include the '.' character. Without that
Sphinx misparses all the "encrypt.format", "encrypt.key-secret", etc
options of the qcow2 block driver as being a single "encrypt" option
with an argument ".format", ".key-secret", etc and thinks they're
duplicates.

> I want to send patches to:

> 2. Change sphinx conf.py to require >= 1.6.1 so that the requirement is
> documented

I think we should do this for 5.0. My guess is that most users
building QEMU on older distros aren't actually interested in
building the documentation, so they'll be better off having
configure automatically disable docs building and getting a
working binary rather than having it try to build the docs
and then failing. If they really want the docs they'll have
a clear reason why which will point them in the right direction.

PS: why 1.6.1 and not 1.6.0? You don't list 1.6.0 in your set of
things you tested, was it just that you couldn't find it?

thanks
-- PMM

Re: Supported Sphinx Versions (was: Re: [PATCH for-5.0? 0/3] Make docs build work with Sphinx 3)

[Peter Maydell]

On Tue, 14 Apr 2020 at 11:28, Peter Maydell <peter.maydell@linaro.org> wrote:
> PS: why 1.6.1 and not 1.6.0? You don't list 1.6.0 in your set of
> things you tested, was it just that you couldn't find it?

Aha, I have just discovered that the Sphinx changelog
explains that 1.6.0 was "not released (because of package
script error)" so 1.6.1 was the first real version in the
1.6.x series.

thanks
-- PMM

Re: [PATCH for-5.0? 3/3] kernel-doc: Use c:struct for Sphinx 3.0 and later

[Peter Maydell]

On Sat, 11 Apr 2020 at 19:29, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> The kernel-doc Sphinx plugin and associated script currently emit
> 'c:type' directives for "struct foo" documentation.
>
> Sphinx 3.0 warns about this:
>   /home/petmay01/linaro/qemu-from-laptop/qemu/docs/../include/exec/memory.h:3: WARNING: Type must be either just a name or a typedef-like declaration.
>   If just a name:
>     Error in declarator or parameters
>     Invalid C declaration: Expected identifier in nested name, got keyword: struct [error at 6]
>       struct MemoryListener
>       ------^
>   If typedef-like declaration:
>     Error in declarator or parameters
>     Invalid C declaration: Expected identifier in nested name. [error at 21]
>       struct MemoryListener
>       ---------------------^
>
> because it wants us to use the new-in-3.0 'c:struct' instead.
>
> Plumb the Sphinx version through to the kernel-doc script
> and use it to select 'c:struct' for newer versions than 3.0.
>
> Fixes: LP:1872113
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  docs/sphinx/kerneldoc.py |  1 +
>  scripts/kernel-doc       | 16 +++++++++++++++-
>  2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/docs/sphinx/kerneldoc.py b/docs/sphinx/kerneldoc.py
> index 1159405cb92..3e879402064 100644
> --- a/docs/sphinx/kerneldoc.py
> +++ b/docs/sphinx/kerneldoc.py
> @@ -99,6 +99,7 @@ class KernelDocDirective(Directive):
>                  env.note_dependency(os.path.abspath(f))
>                  cmd += ['-export-file', f]
>
> +        cmd += ['-sphinx-version', sphinx.__version__]

Using sphinx.version() might perhaps be better: it gives you
a tuple of 5 elements rather than a string. OTOH passing the
tuple through to the Perl script without reformulating the
string and re-parsing it in the Perl isn't easy...

thanks
-- PMM

Re: [PATCH for-5.0? 1/3] configure: Honour --disable-werror for Sphinx

[Alex Bennée]

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

> If we are not making warnings fatal for compilation, make them
> non-fatal when building the Sphinx documentation also.  (For instance
> Sphinx 3.0 warns about some constructs that older versions were happy
> with, which is a build failure if we use the warnings-as-errors
> flag.)
>
> This provides a workaround at least for LP:1872113.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  configure | 9 ++++++++-
>  Makefile  | 2 +-
>  2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/configure b/configure
> index 22870f38672..7b9ae0418d0 100755
> --- a/configure
> +++ b/configure
> @@ -4928,6 +4928,12 @@ if check_include sys/kcov.h ; then
>      kcov=yes
>  fi
>  
> +# If we're making warnings fatal, apply this to Sphinx runs as well
> +sphinx_werror=""
> +if test "$werror" = "yes"; then
> +    sphinx_werror="-W"
> +fi
> +

I wonder if this would have caught the failure I was seeing on debian9 +
mxe images?

>  # Check we have a new enough version of sphinx-build
>  has_sphinx_build() {
>      # This is a bit awkward but works: create a trivial document and
> @@ -4936,7 +4942,7 @@ has_sphinx_build() {
>      # sphinx-build doesn't exist at all or if it is too old.
>      mkdir -p "$TMPDIR1/sphinx"
>      touch "$TMPDIR1/sphinx/index.rst"
> -    "$sphinx_build" -c "$source_path/docs" -b html "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 2>&1
> +    "$sphinx_build" $sphinx_werror -c "$source_path/docs" -b html
>  "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 2>&1

Note this will clash with the simple configure change I made to redirect
the output to config.log.

Anyway:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée

Re: [PATCH for-5.0? 2/3] scripts/kernel-doc: Add missing close-paren in c:function directives

[Alex Bennée]

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

> When kernel-doc generates a 'c:function' directive for a function
> one of whose arguments is a function pointer, it fails to print
> the close-paren after the argument list of the function pointer
> argument, for instance:

for instance in the memory API documentation:

?

>   .. c:function:: void memory_region_init_resizeable_ram (MemoryRegion * mr, struct Object * owner, const char * name, uint64_t size, uint64_t max_size, void (*resized) (const char*, uint64_t length, void *host, Error ** errp)
>
> which should have a ')' after the 'void *host' which is the
> last argument to 'resized'.
>
> Older versions of Sphinx don't try to parse the argumnet
> to c:function, but Sphinx 3.0 does do this and will complain:
>
>   /home/petmay01/linaro/qemu-from-laptop/qemu/docs/../include/exec/memory.h:834: WARNING: Error in declarator or parameters
>   Invalid C declaration: Expecting "," or ")" in parameters, got "EOF". [error at 208]
>     void memory_region_init_resizeable_ram (MemoryRegion * mr, struct Object * owner, const char * name, uint64_t size, uint64_t max_size, void (*resized) (const char*, uint64_t length, void *host, Error ** errp)
>     ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------^
>
> Add the missing close-paren.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Anyway:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée

Re: [PATCH for-5.0? 3/3] kernel-doc: Use c:struct for Sphinx 3.0 and later

[Alex Bennée]

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

> The kernel-doc Sphinx plugin and associated script currently emit
> 'c:type' directives for "struct foo" documentation.
>
> Sphinx 3.0 warns about this:
>   /home/petmay01/linaro/qemu-from-laptop/qemu/docs/../include/exec/memory.h:3: WARNING: Type must be either just a name or a typedef-like declaration.
>   If just a name:
>     Error in declarator or parameters
>     Invalid C declaration: Expected identifier in nested name, got keyword: struct [error at 6]
>       struct MemoryListener
>       ------^
>   If typedef-like declaration:
>     Error in declarator or parameters
>     Invalid C declaration: Expected identifier in nested name. [error at 21]
>       struct MemoryListener
>       ---------------------^
>
> because it wants us to use the new-in-3.0 'c:struct' instead.
>
> Plumb the Sphinx version through to the kernel-doc script
> and use it to select 'c:struct' for newer versions than 3.0.
>
> Fixes: LP:1872113
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Looks reasonable although I don't have a way of testing it on my system.
Any idea what systems have the latest sphinx 3 on them? I tried fedora
but that still has 1.8.4 so it's not that bleeding edge.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée

Re: [PATCH for-5.0? 3/3] kernel-doc: Use c:struct for Sphinx 3.0 and later

[Peter Maydell]

On Tue, 14 Apr 2020 at 16:54, Alex Bennée <alex.bennee@linaro.org> wrote:
> Looks reasonable although I don't have a way of testing it on my system.
> Any idea what systems have the latest sphinx 3 on them? I tried fedora
> but that still has 1.8.4 so it's not that bleeding edge.

I tested using a sphinx 3 install in a python virtualenv:

 cd
 python3 -m venv python-env
 . ~/python-env/bin/activate
 pip install sphinx
 deactivate

and then tell configure to use ~/python-env/bin/sphinx-build
(or don't deactivate, and then that will be the first
sphinx-build on the PATH.)

thanks
-- PMM

Supported Build Platforms (Again) (Was Re: Supported Sphinx Versions)

[John Snow]

On 4/14/20 3:53 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
> 

>> Debian:
>>     8/Jessie: We don't support this anymore AFAIUI.
> 
> Correct.
> 
> docs/system/build-platforms.rst:
> 
>     For distributions with long-lifetime releases, the project will aim
>     to support the most recent major version at all times.  Support for
>     the previous major version will be dropped 2 years after the new
>     major version is released, or when it reaches "end of life".
> 
> Debian 8 reached end of life in 2018, one year after 9's release.
> 

Debian 8 has "long-term support" until 2020-06-30. I only bring this
point up because we still list "Debian" under the "long-lifetime
releases" section, but are excluding the version of Debian that has
"Long-term" in the name.

Pedantic, yes.

Is it worth clarifying that we treat Debian as a "long-lifetime" release
distro, but we do not count their "long-term" support for purposes of
calculating EOL?

Re: Supported Build Platforms (Again) (Was Re: Supported Sphinx Versions)

[Daniel P. Berrangé]

On Wed, Apr 15, 2020 at 12:43:01PM -0400, John Snow wrote:
> 
> 
> On 4/14/20 3:53 AM, Markus Armbruster wrote:
> > John Snow <jsnow@redhat.com> writes:
> > 
> 
> >> Debian:
> >>     8/Jessie: We don't support this anymore AFAIUI.
> > 
> > Correct.
> > 
> > docs/system/build-platforms.rst:
> > 
> >     For distributions with long-lifetime releases, the project will aim
> >     to support the most recent major version at all times.  Support for
> >     the previous major version will be dropped 2 years after the new
> >     major version is released, or when it reaches "end of life".
> > 
> > Debian 8 reached end of life in 2018, one year after 9's release.
> > 
> 
> Debian 8 has "long-term support" until 2020-06-30. I only bring this
> point up because we still list "Debian" under the "long-lifetime
> releases" section, but are excluding the version of Debian that has
> "Long-term" in the name.
> 
> Pedantic, yes.
> 
> Is it worth clarifying that we treat Debian as a "long-lifetime" release
> distro, but we do not count their "long-term" support for purposes of
> calculating EOL?

Yes, the listing of Debian as a LTS section is a mistake I made in the
original drafting, which is overdue to clarify/correct.


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: Supported Build Platforms (Again) (Was Re: Supported Sphinx Versions)

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> On 4/14/20 3:53 AM, Markus Armbruster wrote:
>> John Snow <jsnow@redhat.com> writes:
>> 
>
>>> Debian:
>>>     8/Jessie: We don't support this anymore AFAIUI.
>> 
>> Correct.
>> 
>> docs/system/build-platforms.rst:
>> 
>>     For distributions with long-lifetime releases, the project will aim
>>     to support the most recent major version at all times.  Support for
>>     the previous major version will be dropped 2 years after the new
>>     major version is released, or when it reaches "end of life".
>> 
>> Debian 8 reached end of life in 2018, one year after 9's release.
>> 
>
> Debian 8 has "long-term support" until 2020-06-30. I only bring this
> point up because we still list "Debian" under the "long-lifetime
> releases" section, but are excluding the version of Debian that has
> "Long-term" in the name.
>
> Pedantic, yes.
>
> Is it worth clarifying that we treat Debian as a "long-lifetime" release
> distro, but we do not count their "long-term" support for purposes of
> calculating EOL?

LTS is a separate project under the Debian umbrella.  Our list of
distributions with long-lifetime releases says "Debian", not "Debian
LTS".  It does say "Ubuntu LTS".  Pedantic?  Yes.  Worth clarifying?
Probably.

Putting Debian under short-lifetime distributions would stretch "short"
to roughly three years.