| Message ID | 20200206173040.17337-3-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Convert QAPI doc comments to generate rST instead of texinfo | expand |
Peter Maydell <peter.maydell@linaro.org> writes: > Currently configure's has_sphinx_build() check simply runs a dummy > sphinx-build and either passes or fails. This means that "no > sphinx-build at all" and "sphinx-build exists but is too old" are > both reported the same way. > > Further, we want to assume that all the Python we write is running > with at least Python 3.5; configure checks that for our scripts, but > Sphinx extensions run with whatever Python version sphinx-build > itself is using. > > Add a check to our conf.py which makes sphinx-build fail if it would > be running our extensions with an old Python, and handle this > in configure so we can report failure helpfully to the user. > This will mean that configure --enable-docs will fail like this > if the sphinx-build provided is not suitable: > > Warning: sphinx-build exists but it is either too old or uses too old a Python version > > ERROR: User requested feature docs > configure was not able to find it. > Install texinfo, Perl/perl-podlators and a Python 3 version of python-sphinx > > (As usual, the default is to simply not build the docs, as we would > if sphinx-build wasn't present at all.) > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > Reviewed-by: Alex Bennée <alex.bennee@linaro.org> > Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com> > --- > configure | 12 ++++++++++-- > docs/conf.py | 10 ++++++++++ > 2 files changed, 20 insertions(+), 2 deletions(-) > > diff --git a/configure b/configure > index 0aceb8e50db..2c5cad13edd 100755 > --- a/configure > +++ b/configure Any particular reason for having $sphinx_build default to the indeterminate version sphinx-build rather than sphinx-build-3? It's here: pkg_config=query_pkg_config sdl2_config="${SDL2_CONFIG-${cross_prefix}sdl2-config}" sphinx_build=sphinx-build # If the user hasn't specified ARFLAGS, default to 'rv', just as make does. ARFLAGS="${ARFLAGS-rv}" > @@ -4808,11 +4808,19 @@ has_sphinx_build() { > > # Check if tools are available to build documentation. > if test "$docs" != "no" ; then > - if has makeinfo && has pod2man && has_sphinx_build; then > + if has_sphinx_build; then > + sphinx_ok=yes > + else > + sphinx_ok=no > + fi > + if has makeinfo && has pod2man && test "$sphinx_ok" = "yes"; then > docs=yes > else > if test "$docs" = "yes" ; then > - feature_not_found "docs" "Install texinfo, Perl/perl-podlators and python-sphinx" > + if has $sphinx_build && test "$sphinx_ok" != "yes"; then > + echo "Warning: $sphinx_build exists but it is either too old or uses too old a Python version" >&2 > + fi > + feature_not_found "docs" "Install texinfo, Perl/perl-podlators and a Python 3 version of python-sphinx" > fi > docs=no > fi > diff --git a/docs/conf.py b/docs/conf.py > index ee7faa6b4e7..7588bf192ee 100644 > --- a/docs/conf.py > +++ b/docs/conf.py > @@ -28,6 +28,16 @@ > > import os > import sys > +import sphinx > +from sphinx.errors import VersionRequirementError > + > +# Make Sphinx fail cleanly if using an old Python, rather than obscurely > +# failing because some code in one of our extensions doesn't work there. > +# Unfortunately this doesn't display very neatly (there's an unavoidable > +# Python backtrace) but at least the information gets printed... > +if sys.version_info < (3,5): > + raise VersionRequirementError( > + "QEMU requires a Sphinx that uses Python 3.5 or better\n") > > # The per-manual conf.py will set qemu_docdir for a single-manual build; > # otherwise set it here if this is an entire-manual-set build.
On Fri, 7 Feb 2020 at 16:18, Markus Armbruster <armbru@redhat.com> wrote: > > Peter Maydell <peter.maydell@linaro.org> writes: > > > Currently configure's has_sphinx_build() check simply runs a dummy > > sphinx-build and either passes or fails. This means that "no > > sphinx-build at all" and "sphinx-build exists but is too old" are > > both reported the same way. > > > > Further, we want to assume that all the Python we write is running > > with at least Python 3.5; configure checks that for our scripts, but > > Sphinx extensions run with whatever Python version sphinx-build > > itself is using. > > > > Add a check to our conf.py which makes sphinx-build fail if it would > > be running our extensions with an old Python, and handle this > > in configure so we can report failure helpfully to the user. > > This will mean that configure --enable-docs will fail like this > > if the sphinx-build provided is not suitable: > > > > Warning: sphinx-build exists but it is either too old or uses too old a Python version > > > > ERROR: User requested feature docs > > configure was not able to find it. > > Install texinfo, Perl/perl-podlators and a Python 3 version of python-sphinx > > > > (As usual, the default is to simply not build the docs, as we would > > if sphinx-build wasn't present at all.) > > > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > > Reviewed-by: Alex Bennée <alex.bennee@linaro.org> > > Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com> > > --- > > configure | 12 ++++++++++-- > > docs/conf.py | 10 ++++++++++ > > 2 files changed, 20 insertions(+), 2 deletions(-) > > > > diff --git a/configure b/configure > > index 0aceb8e50db..2c5cad13edd 100755 > > --- a/configure > > +++ b/configure > > Any particular reason for having $sphinx_build default to the > indeterminate version sphinx-build rather than sphinx-build-3? Because that's the binary we were using before this patch. "Allow the user to specify" shouldn't be tangled up with "and also change the default". It might be sphinx-build-3 on RH, but on Debian/Ubuntu it's just 'sphinx-build' assuming you installed the python3-sphinx and not the python2-sphinx, or you can run it directly out of /usr/share/sphinx/scripts/python3/sphinx-build, or (like me) you might have a locally installed 'sphinx-build' which is using Python 3. My assumption is that once the python2->3 transition has faded into the rear view mirror most distros will just have a /usr/bin/sphinx-build that's a Python 3 one. thanks -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On Fri, 7 Feb 2020 at 16:18, Markus Armbruster <armbru@redhat.com> wrote: >> >> Peter Maydell <peter.maydell@linaro.org> writes: >> >> > Currently configure's has_sphinx_build() check simply runs a dummy >> > sphinx-build and either passes or fails. This means that "no >> > sphinx-build at all" and "sphinx-build exists but is too old" are >> > both reported the same way. >> > >> > Further, we want to assume that all the Python we write is running >> > with at least Python 3.5; configure checks that for our scripts, but >> > Sphinx extensions run with whatever Python version sphinx-build >> > itself is using. >> > >> > Add a check to our conf.py which makes sphinx-build fail if it would >> > be running our extensions with an old Python, and handle this >> > in configure so we can report failure helpfully to the user. >> > This will mean that configure --enable-docs will fail like this >> > if the sphinx-build provided is not suitable: >> > >> > Warning: sphinx-build exists but it is either too old or uses too old a Python version >> > >> > ERROR: User requested feature docs >> > configure was not able to find it. >> > Install texinfo, Perl/perl-podlators and a Python 3 version of python-sphinx >> > >> > (As usual, the default is to simply not build the docs, as we would >> > if sphinx-build wasn't present at all.) >> > >> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >> > Reviewed-by: Alex Bennée <alex.bennee@linaro.org> >> > Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com> >> > --- >> > configure | 12 ++++++++++-- >> > docs/conf.py | 10 ++++++++++ >> > 2 files changed, 20 insertions(+), 2 deletions(-) >> > >> > diff --git a/configure b/configure >> > index 0aceb8e50db..2c5cad13edd 100755 >> > --- a/configure >> > +++ b/configure >> >> Any particular reason for having $sphinx_build default to the >> indeterminate version sphinx-build rather than sphinx-build-3? > > Because that's the binary we were using before this patch. > "Allow the user to specify" shouldn't be tangled up with > "and also change the default". > > It might be sphinx-build-3 on RH, but on Debian/Ubuntu it's > just 'sphinx-build' assuming you installed the python3-sphinx > and not the python2-sphinx, or you can run it directly out of > /usr/share/sphinx/scripts/python3/sphinx-build, or (like > me) you might have a locally installed 'sphinx-build' which > is using Python 3. My assumption is that once the python2->3 > transition has faded into the rear view mirror most distros > will just have a /usr/bin/sphinx-build that's a Python 3 one. Defaulting to sphinx-build-3 if it exists, else sphinx-build would be nicer for users on some common systems, and wouldn't hurt users on the other common systems. It's what we do for Python.
On Sat, 8 Feb 2020 at 07:51, Markus Armbruster <armbru@redhat.com> wrote: > Peter Maydell <peter.maydell@linaro.org> writes: > > It might be sphinx-build-3 on RH, but on Debian/Ubuntu it's > > just 'sphinx-build' assuming you installed the python3-sphinx > > and not the python2-sphinx, or you can run it directly out of > > /usr/share/sphinx/scripts/python3/sphinx-build, or (like > > me) you might have a locally installed 'sphinx-build' which > > is using Python 3. My assumption is that once the python2->3 > > transition has faded into the rear view mirror most distros > > will just have a /usr/bin/sphinx-build that's a Python 3 one. > > Defaulting to sphinx-build-3 if it exists, else sphinx-build would be > nicer for users on some common systems, and wouldn't hurt users on the > other common systems. It's what we do for Python. Feel free to send a followup patch :-) I have no systems where there is a 'sphinx-build-3' at all. thanks -- PMM
diff --git a/configure b/configure index 0aceb8e50db..2c5cad13edd 100755 --- a/configure +++ b/configure @@ -4808,11 +4808,19 @@ has_sphinx_build() { # Check if tools are available to build documentation. if test "$docs" != "no" ; then - if has makeinfo && has pod2man && has_sphinx_build; then + if has_sphinx_build; then + sphinx_ok=yes + else + sphinx_ok=no + fi + if has makeinfo && has pod2man && test "$sphinx_ok" = "yes"; then docs=yes else if test "$docs" = "yes" ; then - feature_not_found "docs" "Install texinfo, Perl/perl-podlators and python-sphinx" + if has $sphinx_build && test "$sphinx_ok" != "yes"; then + echo "Warning: $sphinx_build exists but it is either too old or uses too old a Python version" >&2 + fi + feature_not_found "docs" "Install texinfo, Perl/perl-podlators and a Python 3 version of python-sphinx" fi docs=no fi diff --git a/docs/conf.py b/docs/conf.py index ee7faa6b4e7..7588bf192ee 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -28,6 +28,16 @@ import os import sys +import sphinx +from sphinx.errors import VersionRequirementError + +# Make Sphinx fail cleanly if using an old Python, rather than obscurely +# failing because some code in one of our extensions doesn't work there. +# Unfortunately this doesn't display very neatly (there's an unavoidable +# Python backtrace) but at least the information gets printed... +if sys.version_info < (3,5): + raise VersionRequirementError( + "QEMU requires a Sphinx that uses Python 3.5 or better\n") # The per-manual conf.py will set qemu_docdir for a single-manual build; # otherwise set it here if this is an entire-manual-set build.
Currently configure's has_sphinx_build() check simply runs a dummy sphinx-build and either passes or fails. This means that "no sphinx-build at all" and "sphinx-build exists but is too old" are both reported the same way. Further, we want to assume that all the Python we write is running with at least Python 3.5; configure checks that for our scripts, but Sphinx extensions run with whatever Python version sphinx-build itself is using. Add a check to our conf.py which makes sphinx-build fail if it would be running our extensions with an old Python, and handle this in configure so we can report failure helpfully to the user. This will mean that configure --enable-docs will fail like this if the sphinx-build provided is not suitable: Warning: sphinx-build exists but it is either too old or uses too old a Python version ERROR: User requested feature docs configure was not able to find it. Install texinfo, Perl/perl-podlators and a Python 3 version of python-sphinx (As usual, the default is to simply not build the docs, as we would if sphinx-build wasn't present at all.) Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com> --- configure | 12 ++++++++++-- docs/conf.py | 10 ++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) -- 2.20.1