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

[Michał Górny]

On Mon, 21 Jan 2013 12:19:40 -0500
Mike Gilbert <floppym@g.o> wrote:

> 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.

Fixed.

> > +
> > +	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.

Well, I think I separated the general loop condition and the actual
loop code ;).

> >  		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.

Thanks.

I didn't add global-scope checks to python-any-r1 since it simply
doesn't use PYTHON_COMPAT directly.

-- 
Best regards,
Michał Górny