diff mbox series

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

Message ID 20200203132533.21850-3-peter.maydell@linaro.org (mailing list archive)
State New, archived
Headers show
Series configure: Check that sphinx-build is using Python 3 | expand

Commit Message

Peter Maydell Feb. 3, 2020, 1:25 p.m. UTC
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>
---
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(-)

Comments

Alex Bennée Feb. 3, 2020, 6:09 p.m. UTC | #1
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.
Wainer dos Santos Moschetta Feb. 4, 2020, 1:58 p.m. UTC | #2
On 2/3/20 11:25 AM, Peter Maydell wrote:
> 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>
> ---
> 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(-)


Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>



>
> 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.
diff mbox series

Patch

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.