[gentoo-python] Re: [PATCH] Validate PYTHON_COMPAT, support disabling implementations.

[Mike Gilbert]

On 1/14/2013 6:03 AM, Michał Górny wrote:
> Now eclasses die with invalid implementations in PYTHON_COMPAT (well,
> except for python-any-r1 which barely looks at PYTHON_COMPAT and more
> relies on _PYTHON_ALL_IMPLS) and can omit the implementations which will
> be marked as 'removed' (for upcoming pypy1.8 cleanup).
> ---
>  gx86/eclass/python-any-r1.eclass    | 11 ++++-----
>  gx86/eclass/python-r1.eclass        | 49 +++++++++++++++++++++++++++++--------
>  gx86/eclass/python-single-r1.eclass | 39 +++++++++++++++++++----------
>  gx86/eclass/python-utils-r1.eclass  | 31 +++++++++++++++++++++++
>  4 files changed, 101 insertions(+), 29 deletions(-)
>
> diff --git a/gx86/eclass/python-any-r1.eclass b/gx86/eclass/python-any-r1.eclass
> index 199602a..492ab27 100644
> --- a/gx86/eclass/python-any-r1.eclass
> +++ b/gx86/eclass/python-any-r1.eclass
> @@ -134,13 +134,13 @@ _python_build_set_globals() {
>  }
>  _python_build_set_globals
>
> -# @FUNCTION: _python_impl_supported
> +# @FUNCTION: _python_EPYTHON_supported
>  # @USAGE: <epython>
>  # @INTERNAL
>  # @DESCRIPTION:
>  # Check whether the specified implementation is supported by package
>  # (specified in PYTHON_COMPAT).
> -_python_impl_supported() {
> +_python_EPYTHON_supported() {
>  	debug-print-function ${FUNCNAME} "${@}"
>
>  	local i=${1/./_}
> @@ -153,6 +153,7 @@ _python_impl_supported() {
>  			;;
>  		*)
>  			ewarn "Invalid EPYTHON: ${EPYTHON}"
> +			return 1
>  			;;
>  	esac
>
> @@ -173,7 +174,7 @@ python-any-r1_pkg_setup() {
>
>  	# first, try ${EPYTHON}... maybe it's good enough for us.
>  	if [[ ${EPYTHON} ]]; then
> -		if _python_impl_supported "${EPYTHON}"; then
> +		if _python_EPYTHON_supported "${EPYTHON}"; then
>  			python_export EPYTHON PYTHON
>  			return
>  		fi
> @@ -187,7 +188,7 @@ python-any-r1_pkg_setup() {
>  		if [[ ! ${i} ]]; then
>  			# no eselect-python?
>  			break
> -		elif _python_impl_supported "${i}"; then
> +		elif _python_EPYTHON_supported "${i}"; then
>  			python_export "${i}" EPYTHON PYTHON
>  			return
>  		fi
> @@ -206,8 +207,6 @@ python-any-r1_pkg_setup() {
>  		python_export "${i}" PYTHON_PKG_DEP EPYTHON PYTHON
>  		ROOT=/ has_version "${PYTHON_PKG_DEP}" && return
>  	done
> -
> -	die $EPYTHON
>  }
>
>  _PYTHON_ANY_R1=1
> diff --git a/gx86/eclass/python-r1.eclass b/gx86/eclass/python-r1.eclass
> index 8557727..5067931 100644
> --- a/gx86/eclass/python-r1.eclass
> +++ b/gx86/eclass/python-r1.eclass
> @@ -132,7 +132,23 @@ fi
>  # @CODE
>
>  _python_set_globals() {
> -	local flags=( "${PYTHON_COMPAT[@]/#/python_targets_}" )
> +	local impls=() PYTHON_DEPS=

PYTHON_DEPS should be a global here. I pointed this out in IRC, just
stating it here for posterity.

> +
> +	local i PYTHON_PKG_DEP
> +	for i in "${PYTHON_COMPAT[@]}"; do
> +		_python_impl_supported "${i}" || continue
> +
> +		python_export "${i}" PYTHON_PKG_DEP
> +		PYTHON_DEPS+="python_targets_${i}? ( ${PYTHON_PKG_DEP} ) "
> +
> +		impls+=( "${i}" )
> +	done
> +
> +	if [[ ${#impls[@]} -eq 0 ]]; then
> +		die "No supported implementation in PYTHON_COMPAT."
> +	fi
> +
> +	local flags=( "${impls[@]/#/python_targets_}" )
>  	local optflags=${flags[@]/%/?}
>
>  	# A nice QA trick here. Since a python-single-r1 package has to have
> @@ -141,7 +157,7 @@ _python_set_globals() {
>  	# it should prevent developers from mistakenly depending on packages
>  	# not supporting multiple Python implementations.
>
> -	local flags_st=( "${PYTHON_COMPAT[@]/#/-python_single_target_}" )
> +	local flags_st=( "${impls[@]/#/-python_single_target_}" )
>  	optflags+=,${flags_st[@]/%/(-)}
>
>  	IUSE=${flags[*]}
> @@ -152,12 +168,7 @@ _python_set_globals() {
>  	# but no point in making this overcomplex, BDEP doesn't hurt anyone
>  	# 2) python-exec should be built with all targets forced anyway
>  	# but if new targets were added, we may need to force a rebuild
> -	PYTHON_DEPS="dev-python/python-exec[${PYTHON_USEDEP}]"
> -	local i PYTHON_PKG_DEP
> -	for i in "${PYTHON_COMPAT[@]}"; do
> -		python_export "${i}" PYTHON_PKG_DEP
> -		PYTHON_DEPS+=" python_targets_${i}? ( ${PYTHON_PKG_DEP} )"
> -	done
> +	PYTHON_DEPS+="dev-python/python-exec[${PYTHON_USEDEP}]"
>  }
>  _python_set_globals
>
> @@ -171,6 +182,8 @@ _python_validate_useflags() {
>  	local i
>
>  	for i in "${PYTHON_COMPAT[@]}"; do
> +		_python_impl_supported "${i}" || continue
> +

Just a style thing: The blank line in these 2 statement for loops seems
odd to me. Maybe your brain works differently.

>  		use "python_targets_${i}" && return 0
>  	done
>
> @@ -210,6 +223,8 @@ python_gen_usedep() {
>  	local matches=()
>
>  	for impl in "${PYTHON_COMPAT[@]}"; do
> +		_python_impl_supported "${impl}" || continue
> +
>  		for pattern; do
>  			if [[ ${impl} == ${pattern} ]]; then
>  				matches+=(
> @@ -249,6 +264,8 @@ python_gen_useflags() {
>  	local matches=()
>
>  	for impl in "${PYTHON_COMPAT[@]}"; do
> +		_python_impl_supported "${impl}" || continue
> +
>  		for pattern; do
>  			if [[ ${impl} == ${pattern} ]]; then
>  				matches+=( "python_targets_${impl}" )
> @@ -292,6 +309,8 @@ python_gen_cond_dep() {
>  	shift
>
>  	for impl in "${PYTHON_COMPAT[@]}"; do
> +		_python_impl_supported "${impl}" || continue
> +
>  		for pattern; do
>  			if [[ ${impl} == ${pattern} ]]; then
>  				matches+=( "python_targets_${impl}? ( ${dep} )" )
> @@ -337,6 +356,8 @@ python_copy_sources() {
>  	einfo "Will copy sources from ${S}"
>  	# the order is irrelevant here
>  	for impl in "${PYTHON_COMPAT[@]}"; do
> +		_python_impl_supported "${impl}" || continue
> +
>  		if use "python_targets_${impl}"
>  		then
>  			local BUILD_DIR=${bdir%%/}-${impl}
> @@ -369,6 +390,8 @@ _python_check_USE_PYTHON() {
>
>  			local impl py2 py3 dis_py2 dis_py3
>  			for impl in "${PYTHON_COMPAT[@]}"; do
> +				_python_impl_supported "${impl}" || continue
> +
>  				if use "python_targets_${impl}"; then
>  					case "${impl}" in
>  						python2_*)
> @@ -480,6 +503,8 @@ _python_check_USE_PYTHON() {
>  		local impl old=${USE_PYTHON} new=() removed=()
>
>  		for impl in "${PYTHON_COMPAT[@]}"; do
> +			_python_impl_supported "${impl}" || continue
> +
>  			local abi
>  			case "${impl}" in
>  				python*)
> @@ -573,7 +598,9 @@ python_foreach_impl() {
>
>  	debug-print "${FUNCNAME}: bdir = ${bdir}"
>  	for impl in "${_PYTHON_ALL_IMPLS[@]}"; do
> -		if has "${impl}" "${PYTHON_COMPAT[@]}" && use "python_targets_${impl}"
> +		if has "${impl}" "${PYTHON_COMPAT[@]}" \
> +			&& _python_impl_supported "${impl}" \
> +			&& use "python_targets_${impl}"
>  		then
>  			local EPYTHON PYTHON
>  			python_export "${impl}" EPYTHON PYTHON
> @@ -601,7 +628,9 @@ python_export_best() {
>
>  	local impl best
>  	for impl in "${_PYTHON_ALL_IMPLS[@]}"; do
> -		if has "${impl}" "${PYTHON_COMPAT[@]}" && use "python_targets_${impl}"
> +		if has "${impl}" "${PYTHON_COMPAT[@]}" \
> +			&& _python_impl_supported "${impl}" \
> +			&& use "python_targets_${impl}"
>  		then
>  			best=${impl}
>  		fi
> diff --git a/gx86/eclass/python-single-r1.eclass b/gx86/eclass/python-single-r1.eclass
> index fe26251..774f43e 100644
> --- a/gx86/eclass/python-single-r1.eclass
> +++ b/gx86/eclass/python-single-r1.eclass
> @@ -134,8 +134,31 @@ fi
>  # @CODE
>
>  _python_single_set_globals() {
> -	local flags_mt=( "${PYTHON_COMPAT[@]/#/python_targets_}" )
> -	local flags=( "${PYTHON_COMPAT[@]/#/python_single_target_}" )
> +	local impls=()
> +
> +	PYTHON_DEPS=
> +	local i PYTHON_PKG_DEP
> +	for i in "${PYTHON_COMPAT[@]}"; do
> +		_python_impl_supported "${i}" || continue
> +
> +		# The chosen targets need to be in PYTHON_TARGETS as well.
> +		# This is in order to enforce correct dependencies on packages
> +		# supporting multiple implementations.
> +		#REQUIRED_USE+=" python_single_target_${i}? ( python_targets_${i} )"
> +
> +		python_export "${i}" PYTHON_PKG_DEP
> +		PYTHON_DEPS+="python_single_target_${i}? ( ${PYTHON_PKG_DEP} ) "
> +
> +		impls+=( "${i}" )
> +	done
> +
> +	if [[ ${#impls[@]} -eq 0 ]]; then
> +		die "No supported implementation in PYTHON_COMPAT."
> +	fi
> +
> +	local flags_mt=( "${impls[@]/#/python_targets_}" )
> +	local flags=( "${impls[@]/#/python_single_target_}" )
> +
>  	local optflags=${flags_mt[@]/%/?}
>  	optflags+=,${flags[@]/%/(+)?}
>
> @@ -147,17 +170,7 @@ _python_single_set_globals() {
>  	# but no point in making this overcomplex, BDEP doesn't hurt anyone
>  	# 2) python-exec should be built with all targets forced anyway
>  	# but if new targets were added, we may need to force a rebuild
> -	PYTHON_DEPS="dev-python/python-exec[${PYTHON_USEDEP}]"
> -	local i PYTHON_PKG_DEP
> -	for i in "${PYTHON_COMPAT[@]}"; do
> -		# The chosen targets need to be in PYTHON_TARGETS as well.
> -		# This is in order to enforce correct dependencies on packages
> -		# supporting multiple implementations.
> -		#REQUIRED_USE+=" python_single_target_${i}? ( python_targets_${i} )"
> -
> -		python_export "${i}" PYTHON_PKG_DEP
> -		PYTHON_DEPS+=" python_single_target_${i}? ( ${PYTHON_PKG_DEP} )"
> -	done
> +	PYTHON_DEPS+="dev-python/python-exec[${PYTHON_USEDEP}]"
>  }
>  _python_single_set_globals
>
> diff --git a/gx86/eclass/python-utils-r1.eclass b/gx86/eclass/python-utils-r1.eclass
> index ba270b5..ab72225 100644
> --- a/gx86/eclass/python-utils-r1.eclass
> +++ b/gx86/eclass/python-utils-r1.eclass
> @@ -47,6 +47,37 @@ _PYTHON_ALL_IMPLS=(
>  	python2_5 python2_6 python2_7
>  )
>
> +# @FUNCTION: _python_impl_supported
> +# @USAGE: <impl>
> +# @INTERNAL
> +# @DESCRIPTION:
> +# Check whether the implementation <impl> (PYTHON_COMPAT-form)
> +# is still supported.
> +#
> +# Returns 0 if the implementation is valid and supported. If it is
> +# unsupported, returns 1 -- and the caller should ignore the entry.
> +# If it is invalid, dies with an appopriate error messages.
> +_python_impl_supported() {
> +	debug-print-function ${FUNCNAME} "${@}"
> +
> +	[[ ${#} -eq 1 ]] || die "${FUNCNAME}: takes exactly 1 argument (impl)."
> +
> +	local impl=${1}
> +
> +	# keep in sync with _PYTHON_ALL_IMPLS!
> +	# (not using that list because inline patterns shall be faster)
> +	case "${impl}" in
> +		python2_[567]|python3_[123]|pypy1_[89]|pypy2_0|jython2_5)
> +			return 0
> +			;;
> +#		pypy1_8)
> +#			return 1
> +#			;;
> +		*)
> +			die "Invalid implementation in PYTHON_COMPAT: ${impl}"
> +	esac
> +}
> +
>  # @ECLASS-VARIABLE: PYTHON
>  # @DESCRIPTION:
>  # The absolute path to the current Python interpreter.

Overall, looks good. It looks like you did a pretty thorough audit of
the eclasses for the necessary change sites, so I won't repeat that
unless you want me to.