Re: [PATCH 2/2] configure: Check that sphinx-build is using Python 3

["Alex Bennée" <alex.bennee@linaro.org>]

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>

> ---
> At the moment our Sphinx extensions all work under Python 2;
> but the one for handling parsing QAPI docs out of the JSON is going
> to want to include some of the scripts/qapi Python which is more
> complicated and definitely now 3-only.  In any case it's nicer to
> fail cleanly rather than let users stumble into corner cases we don't
> test and don't want to support even if they happen to work today.
> ---
>  configure    | 12 ++++++++++--
>  docs/conf.py | 10 ++++++++++
>  2 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/configure b/configure
> index 830f325822a..95055f2e9dd 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.


-- 
Alex Bennée