sphinx-build is the name of the script entry point from the sphinx
package itself. sphinx-build-3 is a pacakging convention in the Fedora
distribution of Sphinx. Prefer, where possible, the canonical package
name.

In the event that this resolves to a python2 version, test the
suitability of the binary early in the configuration process, and
continue looking for sphinx-build-3 if necessary.

This prioritizes a virtual environment version of sphinx above any
distribution versions, if attempting to build out of a virtual python
environment, which can ease dependency difficulties on older
distributions, as well as allowing easy testing of specific sphinx
versions.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 configure | 66 +++++++++++++++++++++++++++++++++----------------------
 1 file changed, 40 insertions(+), 26 deletions(-)

diff --git a/configure b/configure
index 0d69c360c0..cda4b022e7 100755
--- a/configure
+++ b/configure
@@ -929,13 +929,42 @@ do
     fi
 done
 
+# If we're making warnings fatal, apply this to Sphinx runs as well
+sphinx_werror=""
+if test "$werror" = "yes"; then
+    sphinx_werror="-W"
+fi
+
+# Check we have a new enough version of sphinx-build
+test_sphinx_build() {
+    sphinx=$1
+    # This is a bit awkward but works: create a trivial document and
+    # try to run it with our configuration file (which enforces a
+    # version requirement). This will fail if either
+    # sphinx-build doesn't exist at all or if it is too old.
+    mkdir -p "$TMPDIR1/sphinx"
+    touch "$TMPDIR1/sphinx/index.rst"
+    "$sphinx" $sphinx_werror -c "$source_path/docs" \
+              -b html "$TMPDIR1/sphinx" \
+              "$TMPDIR1/sphinx/out" >> config.log 2>&1
+}
+
+# We require the python3 version of sphinx, but sphinx-build-3 is a
+# distro package. prefer 'sphinx-build' to find the venv version, if
+# any, but ensure it is a suitable version.
 sphinx_build=
-for binary in sphinx-build-3 sphinx-build
+sphinx_ok=
+for binary in sphinx-build sphinx-build-3
 do
     if has "$binary"
     then
-        sphinx_build=$(command -v "$binary")
-        break
+        sphinx_candidate=$(command -v "$binary")
+        if test_sphinx_build "$sphinx_candidate"
+        then
+            sphinx_build=$sphinx_candidate
+            sphinx_ok=yes
+            break
+        fi
     fi
 done
 
@@ -4929,32 +4958,17 @@ if check_include sys/kcov.h ; then
     kcov=yes
 fi
 
-# If we're making warnings fatal, apply this to Sphinx runs as well
-sphinx_werror=""
-if test "$werror" = "yes"; then
-    sphinx_werror="-W"
-fi
-
-# Check we have a new enough version of sphinx-build
-has_sphinx_build() {
-    # This is a bit awkward but works: create a trivial document and
-    # try to run it with our configuration file (which enforces a
-    # version requirement). This will fail if either
-    # sphinx-build doesn't exist at all or if it is too old.
-    mkdir -p "$TMPDIR1/sphinx"
-    touch "$TMPDIR1/sphinx/index.rst"
-    "$sphinx_build" $sphinx_werror -c "$source_path/docs" \
-                    -b html "$TMPDIR1/sphinx" \
-                    "$TMPDIR1/sphinx/out"  >> config.log 2>&1
-}
-
 # Check if tools are available to build documentation.
 if test "$docs" != "no" ; then
-  if has_sphinx_build; then
-    sphinx_ok=yes
-  else
-    sphinx_ok=no
+
+  if [ "$sphinx_ok" != "yes" ]; then
+    if test_sphinx_build "$sphinx_build"; then
+      sphinx_ok=yes
+    else
+      sphinx_ok=no
+    fi
   fi
+
   if has makeinfo && has pod2man && test "$sphinx_ok" = "yes"; then
     docs=yes
   else
-- 
2.21.1

On Wed, 13 May 2020 at 22:52, John Snow <jsnow@redhat.com> wrote:

> +# We require the python3 version of sphinx, but sphinx-build-3 is a
> +# distro package. prefer 'sphinx-build' to find the venv version, if
> +# any, but ensure it is a suitable version.
>  sphinx_build=
> -for binary in sphinx-build-3 sphinx-build
> +sphinx_ok=
> +for binary in sphinx-build sphinx-build-3
>  do
>      if has "$binary"
>      then
> -        sphinx_build=$(command -v "$binary")
> -        break
> +        sphinx_candidate=$(command -v "$binary")
> +        if test_sphinx_build "$sphinx_candidate"
> +        then
> +            sphinx_build=$sphinx_candidate
> +            sphinx_ok=yes
> +            break
> +        fi
>      fi
>  done

Here we look for a sphinx binary that works, and if
we find one then we set sphinx_build to that binary and
set sphinx_ok to 'yes'.

But then (in the part of configure between these two patch hunks)
if the user uses the command line --sphinx-build=foo option we
will reset sphinx_build but don't reset sphinx_ok...

>  # Check if tools are available to build documentation.
>  if test "$docs" != "no" ; then
> -  if has_sphinx_build; then
> -    sphinx_ok=yes
> -  else
> -    sphinx_ok=no
> +
> +  if [ "$sphinx_ok" != "yes" ]; then
> +    if test_sphinx_build "$sphinx_build"; then
> +      sphinx_ok=yes
> +    else
> +      sphinx_ok=no
> +    fi
>    fi

...which means we'll skip this check entirely and won't
test to see whether the user-specified sphinx-build binary works.

> +
>    if has makeinfo && has pod2man && test "$sphinx_ok" = "yes"; then
>      docs=yes
>    else

thanks
-- PMM

On 5/14/20 6:40 AM, Peter Maydell wrote:
> On Wed, 13 May 2020 at 22:52, John Snow <jsnow@redhat.com> wrote:
> 
>> +# We require the python3 version of sphinx, but sphinx-build-3 is a
>> +# distro package. prefer 'sphinx-build' to find the venv version, if
>> +# any, but ensure it is a suitable version.
>>  sphinx_build=
>> -for binary in sphinx-build-3 sphinx-build
>> +sphinx_ok=
>> +for binary in sphinx-build sphinx-build-3
>>  do
>>      if has "$binary"
>>      then
>> -        sphinx_build=$(command -v "$binary")
>> -        break
>> +        sphinx_candidate=$(command -v "$binary")
>> +        if test_sphinx_build "$sphinx_candidate"
>> +        then
>> +            sphinx_build=$sphinx_candidate
>> +            sphinx_ok=yes
>> +            break
>> +        fi
>>      fi
>>  done
> 
> Here we look for a sphinx binary that works, and if
> we find one then we set sphinx_build to that binary and
> set sphinx_ok to 'yes'.
> 
> But then (in the part of configure between these two patch hunks)
> if the user uses the command line --sphinx-build=foo option we
> will reset sphinx_build but don't reset sphinx_ok...
> 
>>  # Check if tools are available to build documentation.
>>  if test "$docs" != "no" ; then
>> -  if has_sphinx_build; then
>> -    sphinx_ok=yes
>> -  else
>> -    sphinx_ok=no
>> +
>> +  if [ "$sphinx_ok" != "yes" ]; then
>> +    if test_sphinx_build "$sphinx_build"; then
>> +      sphinx_ok=yes
>> +    else
>> +      sphinx_ok=no
>> +    fi
>>    fi
> 
> ...which means we'll skip this check entirely and won't
> test to see whether the user-specified sphinx-build binary works.
> 
>> +
>>    if has makeinfo && has pod2man && test "$sphinx_ok" = "yes"; then
>>      docs=yes
>>    else
> 
> thanks
> -- PMM
> 

Oops!

I've tried a new method of invoking sphinx instead of pursuing this, but
I see it has a build failure for one of our CI tests, so I'll keep
poking at it.

--js