[PATCH] docs/conf.py: Raise ConfigError for bad Sphinx Python version

Peter Maydell posted 1 patch 4 years, 1 month 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/20200313163616.30674-1-peter.maydell@linaro.org
Maintainers: Peter Maydell <peter.maydell@linaro.org>
docs/conf.py | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
[PATCH] docs/conf.py: Raise ConfigError for bad Sphinx Python version
Posted by Peter Maydell 4 years, 1 month ago
Raise ConfigError rather than VersionRequirementError when we detect
that the Python being used by Sphinx is too old.

Currently the way we flag the Python version problem up to the user
causes Sphinx to print an unnecessary Python stack trace as well as
the information about the problem; in most versions of Sphinx this is
unavoidable.

The upstream Sphinx developers kindly added a feature to allow
conf.py to report errors to the user without the backtrace:
  https://github.com/sphinx-doc/sphinx/commit/be608ca2313fc08eb842f3dc19d0f5d2d8227d08
but the exception type they chose for this was ConfigError.

Switch to ConfigError, which won't make any difference with currently
deployed Sphinx versions, but will be prettier one day when the user
is using a Sphinx version with the new feature.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
I don't think that commit is in a released Sphinx yet.
---
 docs/conf.py | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/docs/conf.py b/docs/conf.py
index 960043cb860..87b5f07f589 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -29,14 +29,15 @@
 import os
 import sys
 import sphinx
-from sphinx.errors import VersionRequirementError
+from sphinx.errors import ConfigError
 
 # 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...
+# In newer versions of Sphinx this will display nicely; in older versions
+# Sphinx will also produce a Python backtrace but at least the information
+# gets printed...
 if sys.version_info < (3,5):
-    raise VersionRequirementError(
+    raise ConfigError(
         "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;
-- 
2.20.1


Re: [PATCH] docs/conf.py: Raise ConfigError for bad Sphinx Python version
Posted by John Snow 4 years, 1 month ago

On 3/13/20 12:36 PM, Peter Maydell wrote:
> Raise ConfigError rather than VersionRequirementError when we detect
> that the Python being used by Sphinx is too old.
> 
> Currently the way we flag the Python version problem up to the user
> causes Sphinx to print an unnecessary Python stack trace as well as
> the information about the problem; in most versions of Sphinx this is
> unavoidable.
> 
> The upstream Sphinx developers kindly added a feature to allow
> conf.py to report errors to the user without the backtrace:
>   https://github.com/sphinx-doc/sphinx/commit/be608ca2313fc08eb842f3dc19d0f5d2d8227d08
> but the exception type they chose for this was ConfigError.
> 
> Switch to ConfigError, which won't make any difference with currently
> deployed Sphinx versions, but will be prettier one day when the user
> is using a Sphinx version with the new feature.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I don't think that commit is in a released Sphinx yet.
> ---
>  docs/conf.py | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/docs/conf.py b/docs/conf.py
> index 960043cb860..87b5f07f589 100644
> --- a/docs/conf.py
> +++ b/docs/conf.py
> @@ -29,14 +29,15 @@
>  import os
>  import sys
>  import sphinx
> -from sphinx.errors import VersionRequirementError
> +from sphinx.errors import ConfigError
>  
>  # 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...
> +# In newer versions of Sphinx this will display nicely; in older versions
> +# Sphinx will also produce a Python backtrace but at least the information
> +# gets printed...
>  if sys.version_info < (3,5):
> -    raise VersionRequirementError(
> +    raise ConfigError(
>          "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;
> 

When was ConfigError introduced, and what's our minimum Sphinx version?
(Hm, looks like it's not versioned, so I'll trust it's been around a while.)

Reviewed-by: John Snow <jsnow@redhat.com>


Re: [PATCH] docs/conf.py: Raise ConfigError for bad Sphinx Python version
Posted by Peter Maydell 4 years, 1 month ago
On Fri, 13 Mar 2020 at 22:30, John Snow <jsnow@redhat.com> wrote:
> When was ConfigError introduced, and what's our minimum Sphinx version?
> (Hm, looks like it's not versioned, so I'll trust it's been around a while.)

Yeah, it's been around a long time; our minimum Sphinx version is 1.3.

thanks
-- PMM