sphinx-build is the name of the script entry point from the sphinx
package itself. sphinx-build-3 is a pacakging convention by Linux
distributions. 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 of a virtual python
environment.

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

diff --git a/configure b/configure
index 233c671aaa..82143e8a41 100755
--- a/configure
+++ b/configure
@@ -928,13 +928,34 @@ do
     fi
 done
 
+# 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" -c "$source_path/docs" -b html "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 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
 
@@ -4928,24 +4949,17 @@ if check_include sys/kcov.h ; then
     kcov=yes
 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" -c "$source_path/docs" -b html "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 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, 15 Apr 2020 at 18:33, John Snow <jsnow@redhat.com> wrote:
>
> sphinx-build is the name of the script entry point from the sphinx
> package itself. sphinx-build-3 is a pacakging convention by Linux
> distributions. Prefer, where possible, the canonical package name.

This was Markus's code originally; cc'ing him.

(Incidentally I think when we say "Linux distributions" we
really mean "Red Hat"; Debian/Ubuntu don't use the "sphinx-build-3" name.)

thanks
-- PMM
(rest of email untrimmed for context)

> 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 of a virtual python
> environment.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  configure | 50 ++++++++++++++++++++++++++++++++------------------
>  1 file changed, 32 insertions(+), 18 deletions(-)
>
> diff --git a/configure b/configure
> index 233c671aaa..82143e8a41 100755
> --- a/configure
> +++ b/configure
> @@ -928,13 +928,34 @@ do
>      fi
>  done
>
> +# 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" -c "$source_path/docs" -b html "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 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
>
> @@ -4928,24 +4949,17 @@ if check_include sys/kcov.h ; then
>      kcov=yes
>  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" -c "$source_path/docs" -b html "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 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
> --

On 4/15/20 1:55 PM, Peter Maydell wrote:
> On Wed, 15 Apr 2020 at 18:33, John Snow <jsnow@redhat.com> wrote:
>>
>> sphinx-build is the name of the script entry point from the sphinx
>> package itself. sphinx-build-3 is a pacakging convention by Linux
>> distributions. Prefer, where possible, the canonical package name.
> 
> This was Markus's code originally; cc'ing him.
> 
> (Incidentally I think when we say "Linux distributions" we
> really mean "Red Hat"; Debian/Ubuntu don't use the "sphinx-build-3" name.)
> 

I'll take your word for it :)

> thanks
> -- PMM
> (rest of email untrimmed for context)
> 

My only goal here is that if you are using a virtual environment with
sphinx installed that it prefers that, so non-standard names need to
come last.

There's probably 10,000,000 ways to do that, hence the RFC.

--js

>> 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 of a virtual python
>> environment.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>  configure | 50 ++++++++++++++++++++++++++++++++------------------
>>  1 file changed, 32 insertions(+), 18 deletions(-)
>>
>> diff --git a/configure b/configure
>> index 233c671aaa..82143e8a41 100755
>> --- a/configure
>> +++ b/configure
>> @@ -928,13 +928,34 @@ do
>>      fi
>>  done
>>
>> +# 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" -c "$source_path/docs" -b html "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 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
>>
>> @@ -4928,24 +4949,17 @@ if check_include sys/kcov.h ; then
>>      kcov=yes
>>  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" -c "$source_path/docs" -b html "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 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
>> --
> 

John Snow <jsnow@redhat.com> writes:

> On 4/15/20 1:55 PM, Peter Maydell wrote:
>> On Wed, 15 Apr 2020 at 18:33, John Snow <jsnow@redhat.com> wrote:
>>>
>>> sphinx-build is the name of the script entry point from the sphinx
>>> package itself. sphinx-build-3 is a pacakging convention by Linux
>>> distributions. Prefer, where possible, the canonical package name.
>> 
>> This was Markus's code originally; cc'ing him.
>> 
>> (Incidentally I think when we say "Linux distributions" we
>> really mean "Red Hat"; Debian/Ubuntu don't use the "sphinx-build-3" name.)
>> 
>
> I'll take your word for it :)
>
>> thanks
>> -- PMM
>> (rest of email untrimmed for context)
>> 
>
> My only goal here is that if you are using a virtual environment with
> sphinx installed that it prefers that, so non-standard names need to
> come last.
>
> There's probably 10,000,000 ways to do that, hence the RFC.

My goal when I wrote the patch was to make the doc-building work out of
the box on systems where sphinx-build gets you a Python 2 Sphinx, which
does not work for us, and sphinx-build-3 gets you one that does.

I patterned the Sphinx check after the Python check: first the -3. and
only then the indeterminate version.

I won't object to a patch that makes things better for you without
making them worse for me :)

John Snow <jsnow@redhat.com> writes:

> On 4/15/20 1:55 PM, Peter Maydell wrote:
>> On Wed, 15 Apr 2020 at 18:33, John Snow <jsnow@redhat.com> wrote:
>>>
>>> sphinx-build is the name of the script entry point from the sphinx
>>> package itself. sphinx-build-3 is a pacakging convention by Linux
>>> distributions. Prefer, where possible, the canonical package name.
>> 
>> This was Markus's code originally; cc'ing him.
>> 
>> (Incidentally I think when we say "Linux distributions" we
>> really mean "Red Hat"; Debian/Ubuntu don't use the "sphinx-build-3" name.)
>> 
>
> I'll take your word for it :)
>
>> thanks
>> -- PMM
>> (rest of email untrimmed for context)
>> 
>
> My only goal here is that if you are using a virtual environment with
> sphinx installed that it prefers that, so non-standard names need to
> come last.
>
> There's probably 10,000,000 ways to do that, hence the RFC.

What's wrong with just passing --sphinx-build=sphinx-build in your
configure string? It will override whatever we auto-detect AFAICT.

-- 
Alex Bennée

On 4/16/20 8:31 AM, Alex Bennée wrote:
> 
> John Snow <jsnow@redhat.com> writes:
> 
>> On 4/15/20 1:55 PM, Peter Maydell wrote:
>>> On Wed, 15 Apr 2020 at 18:33, John Snow <jsnow@redhat.com> wrote:
>>>>
>>>> sphinx-build is the name of the script entry point from the sphinx
>>>> package itself. sphinx-build-3 is a pacakging convention by Linux
>>>> distributions. Prefer, where possible, the canonical package name.
>>>
>>> This was Markus's code originally; cc'ing him.
>>>
>>> (Incidentally I think when we say "Linux distributions" we
>>> really mean "Red Hat"; Debian/Ubuntu don't use the "sphinx-build-3" name.)
>>>
>>
>> I'll take your word for it :)
>>
>>> thanks
>>> -- PMM
>>> (rest of email untrimmed for context)
>>>
>>
>> My only goal here is that if you are using a virtual environment with
>> sphinx installed that it prefers that, so non-standard names need to
>> come last.
>>
>> There's probably 10,000,000 ways to do that, hence the RFC.
> 
> What's wrong with just passing --sphinx-build=sphinx-build in your
> configure string? It will override whatever we auto-detect AFAICT.
> 

My goal is to make virtual environments work out of the box.

I.e., if you run ./configure from inside a VENV, it should "just work."

--js

On Thu, 16 Apr 2020 at 19:22, John Snow <jsnow@redhat.com> wrote:
> My goal is to make virtual environments work out of the box.
>
> I.e., if you run ./configure from inside a VENV, it should "just work."

Yeah, this seems reasonable to me. If I understand your
patch correctly it ought to work without breaking
the setup Markus describes, because in that case
'sphinx-build' exists but will fail the test_sphinx_build
step (because it's a Python 2 sphinx-build) and we'll
then move on and use sphinx-build-3.

Patch looks good to me, but you'll need to rebase and update it
to take account of commits 516e8b7d4a and 988ae6c3a7
now in master.

thanks
-- PMM

On 4/16/20 3:16 PM, Peter Maydell wrote:
> On Thu, 16 Apr 2020 at 19:22, John Snow <jsnow@redhat.com> wrote:
>> My goal is to make virtual environments work out of the box.
>>
>> I.e., if you run ./configure from inside a VENV, it should "just work."
> 
> Yeah, this seems reasonable to me. If I understand your
> patch correctly it ought to work without breaking
> the setup Markus describes, because in that case
> 'sphinx-build' exists but will fail the test_sphinx_build
> step (because it's a Python 2 sphinx-build) and we'll
> then move on and use sphinx-build-3.
> 
> Patch looks good to me, but you'll need to rebase and update it
> to take account of commits 516e8b7d4a and 988ae6c3a7
> now in master.
> 
> thanks
> -- PMM
> 

OK, Done.

--js