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

Peter Maydell posted 3 patches 4 years, 6 months ago
Test docker-mingw@fedora passed
Test docker-quick@centos7 passed
Test checkpatch passed
Test FreeBSD passed
Test asan passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200411182934.28678-1-peter.maydell@linaro.org
configure                |  9 ++++++++-
Makefile                 |  2 +-
docs/sphinx/kerneldoc.py |  1 +
scripts/kernel-doc       | 18 ++++++++++++++++--
4 files changed, 26 insertions(+), 4 deletions(-)
[PATCH for-5.0? 0/3] Make docs build work with Sphinx 3
Posted by Peter Maydell 4 years, 6 months ago
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


Re: [PATCH for-5.0? 0/3] Make docs build work with Sphinx 3
Posted by Paolo Bonzini 4 years, 6 months ago
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)
Posted by John Snow 4 years, 6 months ago

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)
Posted by Peter Maydell 4 years, 6 months ago
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)
Posted by Peter Maydell 4 years, 6 months ago
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: Supported Sphinx Versions (was: Re: [PATCH for-5.0? 0/3] Make docs build work with Sphinx 3)
Posted by Peter Maydell 4 years, 6 months ago
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)
Posted by John Snow 4 years, 6 months ago

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
Posted by Markus Armbruster 4 years, 6 months ago
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


Supported Build Platforms (Again) (Was Re: Supported Sphinx Versions)
Posted by John Snow 4 years, 6 months ago

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)
Posted by Daniel P. Berrangé 4 years, 6 months ago
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)
Posted by Markus Armbruster 4 years, 6 months ago
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.