Re: [PATCH v4 3/4] meson: Move the detection logic for sphinx to meson

[Paolo Bonzini]

Here is a better way to write the meson test:

if get_option('sphinx_build') == ''
  sphinx_build = find_program(['sphinx-build-3', 'sphinx-build'],
                              required: get_option('docs'))
else
  sphinx_build = find_program(get_option('sphinx_build'),
                              required: get_option('docs'))
endif

SPHINX_ARGS = [sphinx_build]
# If we're making warnings fatal, apply this to Sphinx runs as well
if get_option('werror')
  SPHINX_TEST_ARGS += [ '-W' ]
endif

# Check if tools are available to build documentation.
build_docs = false
if sphinx_build.found()
  # 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 sphinx-build is too old.
  run_command('mkdir', ['-p', tmpdir / 'sphinx'])
  run_command('touch', [tmpdir / 'sphinx/index.rst'])
  sphinx_build_test_out = run_command(SPHINX_ARGS + [
    '-c', meson.current_source_dir() / 'docs',
    '-b', 'html', tmpdir / 'sphinx',
    tmpdir / 'sphinx/out'])
  build_docs = (sphinx_build_test_out.returncode() == 0)
endif

if not build_docs
  if sphinx_build_option != ''
    warning('@0@ exists but it is either too old or uses too old a Python 
version'.format(sphinx_build_option))
  endif
  if get_option('docs').enabled()
    error('Install a Python 3 version of python-sphinx')
  endif
endif



On 16/10/20 00:06, Yonggang Luo wrote:
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> ---
>  configure                     | 59 +++-------------------------------
>  docs/meson.build              |  4 +--
>  meson.build                   | 60 +++++++++++++++++++++++++++++++----
>  meson_options.txt             |  5 ++-
>  tests/qapi-schema/meson.build |  2 +-
>  5 files changed, 64 insertions(+), 66 deletions(-)
> 
> diff --git a/configure b/configure
> index 1ce31f97b4..ff593a8542 100755
> --- a/configure
> +++ b/configure
> @@ -297,7 +297,7 @@ brlapi=""
>  curl=""
>  iconv="auto"
>  curses="auto"
> -docs=""
> +docs="auto"
>  fdt="auto"
>  netmap="no"
>  sdl="auto"
> @@ -822,15 +822,6 @@ do
>      fi
>  done
>  
> -sphinx_build=
> -for binary in sphinx-build-3 sphinx-build
> -do
> -    if has "$binary"
> -    then
> -        sphinx_build=$(command -v "$binary")
> -        break
> -    fi
> -done
>  
>  # Check for ancillary tools used in testing
>  genisoimage=
> @@ -1226,9 +1217,9 @@ for opt do
>    ;;
>    --disable-crypto-afalg) crypto_afalg="no"
>    ;;
> -  --disable-docs) docs="no"
> +  --disable-docs) docs="disabled"
>    ;;
> -  --enable-docs) docs="yes"
> +  --enable-docs) docs="enabled"
>    ;;
>    --disable-vhost-net) vhost_net="no"
>    ;;
> @@ -4413,45 +4404,6 @@ if check_include linux/btrfs.h ; then
>      btrfs=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
> -  fi
> -  if test "$sphinx_ok" = "yes"; then
> -    docs=yes
> -  else
> -    if test "$docs" = "yes" ; then
> -      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 a Python 3 version of python-sphinx"
> -    fi
> -    docs=no
> -  fi
> -fi
> -
>  # Search for bswap_32 function
>  byteswap_h=no
>  cat > $TMPC << EOF
> @@ -6087,9 +6039,6 @@ qemu_version=$(head $source_path/VERSION)
>  echo "PKGVERSION=$pkgversion" >>$config_host_mak
>  echo "SRC_PATH=$source_path" >> $config_host_mak
>  echo "TARGET_DIRS=$target_list" >> $config_host_mak
> -if [ "$docs" = "yes" ] ; then
> -  echo "BUILD_DOCS=yes" >> $config_host_mak
> -fi
>  if test "$modules" = "yes"; then
>    # $shacmd can generate a hash started with digit, which the compiler 
> doesn't
>    # like as an symbol. So prefix it with an underscore
> @@ -6794,7 +6743,6 @@ fi
>  echo "ROMS=$roms" >> $config_host_mak
>  echo "MAKE=$make" >> $config_host_mak
>  echo "PYTHON=$python" >> $config_host_mak
> -echo "SPHINX_BUILD=$sphinx_build" >> $config_host_mak
>  echo "GENISOIMAGE=$genisoimage" >> $config_host_mak
>  echo "MESON=$meson" >> $config_host_mak
>  echo "CC=$cc" >> $config_host_mak
> @@ -7076,6 +7024,7 @@ NINJA=${ninja:-$PWD/ninjatool} $meson setup \
>          -Dgettext=$gettext -Dxkbcommon=$xkbcommon -Du2f=$u2f \
>          -Dcapstone=$capstone -Dslirp=$slirp -Dfdt=$fdt \
>          -Diconv=$iconv -Dcurses=$curses \
> +        -Ddocs=$docs -Dsphinx_build=$sphinx_build \
>          $cross_arg \
>          "$PWD" "$source_path"
>  
> diff --git a/docs/meson.build b/docs/meson.build
> index 0340d489ac..f566809a6a 100644
> --- a/docs/meson.build
> +++ b/docs/meson.build
> @@ -37,7 +37,7 @@ if build_docs
>                  input: [files('conf.py'), files(manual / 'conf.py')],
>                  depfile: manual + '.d',
>                  depend_files: sphinx_extn_depends,
> -                command: [SPHINX_ARGS, '-Ddepfile=@DEPFILE@',
> +                command: SPHINX_ARGS + ['-Ddepfile=@DEPFILE@',
>                            '-Ddepfile_stamp=@OUTPUT0@',
>                            '-b', 'html', '-d', private_dir,
>                            input_dir, output_dir])
> @@ -59,7 +59,7 @@ if build_docs
>                           input: this_manual,
>                           install: build_docs,
>                           install_dir: install_dirs,
> -                         command: [SPHINX_ARGS, '-b', 'man', '-d', 
> private_dir,
> +                         command: SPHINX_ARGS + ['-b', 'man', '-d', 
> private_dir,
>                                     input_dir, meson.current_build_dir()])
>      endif
>    endforeach
> diff --git a/meson.build b/meson.build
> index 8156df8b71..8940468208 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -17,7 +17,13 @@ cc = meson.get_compiler('c')
>  config_host = keyval.load(meson.current_build_dir() / 'config-host.mak')
>  enable_modules = 'CONFIG_MODULES' in config_host
>  enable_static = 'CONFIG_STATIC' in config_host
> -build_docs = 'BUILD_DOCS' in config_host
> +
> +# Temporary directory used for files created while
> +# configure runs. Since it is in the build directory
> +# we can safely blow away any previous version of it
> +# (and we need not jump through hoops to try to delete
> +# it when configure exits.)
> +tmpdir = meson.current_build_dir() / 'config-temp'
>  
>  if get_option('qemu_suffix').startswith('/')
>    error('qemu_suffix cannot start with a /')
> @@ -1235,12 +1241,52 @@ foreach d : hx_headers
>  endforeach
>  genh += hxdep
>  
> -SPHINX_ARGS = [config_host['SPHINX_BUILD'],
> -               '-Dversion=' + meson.project_version(),
> -               '-Drelease=' + config_host['PKGVERSION']]
> +sphinx_build = not_found
> +sphinx_build_option = get_option('sphinx_build')
> +if sphinx_build_option == ''
> +  sphinx_build = find_program('sphinx-build', required: false)
> +else
> +  sphinx_build = find_program(sphinx_build_option, required: false)
> +endif
>  
> +SPHINX_TEST_ARGS = []
> +# If we're making warnings fatal, apply this to Sphinx runs as well
>  if get_option('werror')
> -  SPHINX_ARGS += [ '-W' ]
> +  SPHINX_TEST_ARGS += [ '-W' ]
> +endif
> +
> +build_docs = false
> +docs_option = get_option('docs')
> +# Check if tools are available to build documentation.
> +if sphinx_build.found() and not docs_option.disabled()
> +    # 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.
> +    run_command('mkdir', ['-p', tmpdir / 'sphinx'])
> +    run_command('touch', [tmpdir / 'sphinx/index.rst'])
> +    sphinx_build_test_out = run_command(sphinx_build, SPHINX_TEST_ARGS + [
> +      '-c', meson.current_source_dir() / 'docs',
> +      '-b', 'html', tmpdir / 'sphinx',
> +      tmpdir / 'sphinx/out'])
> +    if sphinx_build_test_out.returncode() == 0
> +      build_docs = true
> +    else
> +      if docs_option.enabled()
> +        if sphinx_build_option != ''
> +          warning('Warning: @0@ exists but it is either too old or uses too 
> old a Python version'.format(sphinx_build_option))
> +        endif
> +        error('Warning: Install a Python 3 version of python-sphinx')
> +      endif
> +      # do not building docs
> +    endif
> +endif
> +
> +SPHINX_ARGS = []
> +if build_docs
> +  SPHINX_ARGS = [sphinx_build]
> +  SPHINX_ARGS += SPHINX_TEST_ARGS
> +  SPHINX_ARGS += ['-Dversion=' + meson.project_version(), '-Drelease=' + 
> config_host['PKGVERSION']]
>  endif
>  
>  sphinx_extn_depends = [ meson.source_root() / 'docs/sphinx/depfile.py',
> @@ -1918,7 +1964,7 @@ summary_info += {'QEMU_CFLAGS':       
> config_host['QEMU_CFLAGS']}
>  summary_info += {'QEMU_LDFLAGS':      config_host['QEMU_LDFLAGS']}
>  summary_info += {'make':              config_host['MAKE']}
>  summary_info += {'python':            '@0@ (version: 
> @1@)'.format(python.full_path(), python.language_version())}
> -summary_info += {'sphinx-build':      config_host['SPHINX_BUILD']}
> +summary_info += {'sphinx-build':      sphinx_build.found()}
>  summary_info += {'genisoimage':       config_host['GENISOIMAGE']}
>  # TODO: add back version
>  summary_info += {'slirp support':     slirp_opt == 'disabled' ? false : 
> slirp_opt}
> @@ -1986,7 +2032,7 @@ if config_host.has_key('CONFIG_XEN_BACKEND')
>    summary_info += {'xen ctrl version':  
> config_host['CONFIG_XEN_CTRL_INTERFACE_VERSION']}
>  endif
>  summary_info += {'brlapi support':    config_host.has_key('CONFIG_BRLAPI')}
> -summary_info += {'Documentation':     config_host.has_key('BUILD_DOCS')}
> +summary_info += {'Documentation':     build_docs}
>  summary_info += {'PIE':               get_option('b_pie')}
>  summary_info += {'vde support':       config_host.has_key('CONFIG_VDE')}
>  summary_info += {'netmap support':    config_host.has_key('CONFIG_NETMAP')}
> diff --git a/meson_options.txt b/meson_options.txt
> index e6cb1e589b..0e3fa7136b 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -1,8 +1,11 @@
>  option('qemu_suffix', type : 'string', value: 'qemu',
>         description: 'Suffix for QEMU data/modules/config directories (can be 
> empty)')
> +option('docs', type : 'feature', value : 'auto',
> +       description: 'Documentations build support')
>  option('docdir', type : 'string', value : 'doc',
>         description: 'Base directory for documentation installation (can be 
> empty)')
> -
> +option('sphinx_build', type : 'string', value : '',
> +       description: 'Use specified sphinx-build [$sphinx_build] for building 
> document (default to be empty)')
>  option('gettext', type : 'boolean', value : true,
>         description: 'Localization of the GTK+ user interface')
>  option('sparse', type : 'feature', value : 'auto',
> diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
> index 1f222a7a13..961b8367a9 100644
> --- a/tests/qapi-schema/meson.build
> +++ b/tests/qapi-schema/meson.build
> @@ -237,7 +237,7 @@ qapi_doc_out = custom_target('QAPI rST doc',
>                               # we want it to always really run the QAPI doc
>                               # generation code. It also means we don't
>                               # clutter up the build dir with the cache.
> -                             command: [SPHINX_ARGS,
> +                             command:  SPHINX_ARGS + [
>                                         '-b', 'text', '-E',
>                                         '-c', meson.source_root() / 'docs',
>                                         '-D', 'master_doc=doc-good',
>