Re: [gentoo-dev] [PATCH 1/4] distutils-r1.eclass: Add distutils_enable_sphinx helper function

[Patrick McLean]

Hi Michał,

Thanks for doing this work, it's always better to have standardized
ways of doing things.

On Wed, 20 Nov 2019 15:21:55 +0100
Michał Górny <mgorny@g.o> wrote:

<snip>

> +distutils_enable_sphinx() {
> +	debug-print-function ${FUNCNAME} "${@}"
> +	[[ ${#} -ge 1 ]] || die "${FUNCNAME} takes at least one arg: <subdir>"
> +
> +	_DISTUTILS_SPHINX_SUBDIR=${1}
> +	shift
> +	_DISTUTILS_SPHINX_PLUGINS=( "${@}" )
> +
> +	local deps autodoc=1 d
> +	for d; do
> +		if [[ ${d} == --no-autodoc ]]; then
> +			autodoc=
> +		else
> +			deps+="
> +				${d}[\${PYTHON_USEDEP}]"
> +		fi
> +	done
> +
> +	if [[ ! ${autodoc} && -n ${deps} ]]; then
> +		die "${FUNCNAME}: do not pass --no-autodoc if external plugins are used"
> +	fi
> +	if [[ ${autodoc} ]]; then
> +		deps="$(python_gen_any_dep "
> +			dev-python/sphinx[\${PYTHON_USEDEP}]
> +			${deps}")"
> +
> +		python_check_deps() {
> +			use doc || return 0
> +			local p
> +			for p in "${_DISTUTILS_SPHINX_PLUGINS[@]}"; do
> +				has_version "${p}[${PYTHON_USEDEP}]" || return 1
> +			done
> +		}

I think it would be better to put this code in sphinx_check_deps
(or some such) and define a python_check_deps that just calls
sphinx_check_deps. That would allow ebuilds that need to do more than
the sphinx stuff to not have to reimplement this.

> +	else
> +		deps="dev-python/sphinx"
> +	fi
> +
> +	python_compile_all() {
> +		use doc || return
> +
> +		cd "${_DISTUTILS_SPHINX_SUBDIR}" || die
> +		[[ -f conf.py ]] ||
> +			die "conf.py not found, distutils_enable_sphinx call wrong"
> +
> +		if [[ ${_DISTUTILS_SPHINX_PLUGINS[0]} == --no-autodoc ]]; then
> +			if grep -q 'sphinx\.ext\.autodoc' "conf.py"; then

Since this is just searching for a fixed string maybe
"grep -F -q 'sphinx.ext.autodoc'" is a bit nicer.

> +				die "distutils_enable_sphinx: --no-autodoc passed but sphinx.ext.autodoc found in conf.py"
> +			fi
> +		else
> +			if ! grep -q 'sphinx\.ext\.autodoc' "conf.py"; then
> +				die "distutils_enable_sphinx: sphinx.ext.autodoc not found in conf.py, pass --no-autodoc"
> +			fi
> +		fi
> +
> +		# disable intersphinx (internet use)
> +		sed -e 's:^intersphinx_mapping:disabled_&:' -i conf.py || die
> +		# not all packages include the Makefile in pypi tarball
> +		sphinx-build -b html -d _build/doctrees . _build/html || die
> +
> +		HTML_DOCS+=( "${_DISTUTILS_SPHINX_SUBDIR}/_build/html/." )
> +	}

Same as above, I think it would be better to define this as
sphinx_compile, and define a python_compile_all that just calls it.
That way if an ebuild defines it's own python_compile_all it can call
sphinx_compile to get this functionality.

> +
> +	IUSE+=" doc"
> +	if [[ ${EAPI} == [56] ]]; then
> +		DEPEND+=" doc? ( ${deps} )"
> +	else
> +		BDEPEND+=" doc? ( ${deps} )"
> +	fi
> +
> +	# we need to ensure successful return in case we're called last,
> +	# otherwise Portage may wrongly assume sourcing failed
> +	return 0
> +}
> +
>  # @FUNCTION: distutils_enable_tests
>  # @USAGE: <test-runner>
>  # @DESCRIPTION: