[gentoo-dev] Re: [PATCH] Check for USE_PYTHON <-> PYTHON_TARGETS sync.

[Mike Gilbert]

On Fri, Nov 16, 2012 at 1:44 PM, Michał Górny <mgorny@g.o> wrote:
> This is much more complex than the previous version.
>

The inline comments and debug-print statements are helpful.

Do we have a way to test this with various combinations of
PYTHON_COMPAT, PYTHON_TARGETS, eselect python, and USE_PYTHON?

I have some suggestions on the actual warning text below.

> If user doesn't have USE_PYTHON set, it tries to solve the issue using
> eselect-python only, suggesting user to switch the active
> implementation.
>
> If eselect-python won't help or user has USE_PYTHON set, it nicely
> suggests a new value, highlighting added and listing removed entries.
>
> The basic issue here is that the package can only check PYTHON_TARGETS
> values in IUSE, so only the implementations supported by the package. It
> tries hard to help and not to do any damage, so user may need to modify
> his USE_PYTHON more than once.
>
> For example, consider the following:
>
> - installed: 2.6, 2.7, 3.1, 3.2, pypy1.8
> - active: 2.6, 3.2
> - USE_PYTHON not set
> - PYTHON_TARGETS: 2.7, 3.1, 3.2, pypy1.8
>
> Package 1 supports Python 2 only. The eclass can't access Python 3
> PYTHON_TARGETS, thus it can only suggest user to switch active Python
> interpreter to Python 2.7. This solves the Python2 packages only.
>
> Package 2 supports both Python 2 & 3. The eclass notices two Python 3
> versions and suggests user to add USE_PYTHON='2.7 3.1 3.2'.
>
> Package 3 supports PyPy additionally. The eclass notices that, and
> suggests user to add '2.7-pypy-1.8' to USE_PYTHON.
>
> Of course, that assumes building one-by-one. If more packages get pulled
> into the dep tree, the results of checks may look even contradictory.
> Hopefully, after a few changes and rebuilds user will finally get
> the correct value.
> ---
>  gx86/eclass/python-r1.eclass | 198 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 198 insertions(+)
>
> diff --git a/gx86/eclass/python-r1.eclass b/gx86/eclass/python-r1.eclass
> index 0d6ef4c..61edf94 100644
> --- a/gx86/eclass/python-r1.eclass
> +++ b/gx86/eclass/python-r1.eclass
> @@ -363,6 +363,202 @@ python_copy_sources() {
>         done
>  }
>
> +# @FUNCTION: _python_check_USE_PYTHON
> +# @INTERNAL
> +# @DESCRIPTION:
> +# Check whether USE_PYTHON and PYTHON_TARGETS are in sync. Output
> +# warnings if they are not.
> +_python_check_USE_PYTHON() {
> +       debug-print-function ${FUNCNAME} "${@}"
> +
> +       if [[ ! ${_PYTHON_USE_PYTHON_CHECKED} ]]; then
> +               _PYTHON_USE_PYTHON_CHECKED=1
> +
> +               # python-exec has profile-forced flags.
> +               if [[ ${CATEGORY}/${PN} == dev-python/python-exec ]]; then
> +                       return
> +               fi
> +
> +               _try_eselect() {
> +                       # The eselect solution will work only with one py2 & py3.
> +
> +                       local impl py2 py3 dis_py2 dis_py3
> +                       for impl in "${PYTHON_COMPAT[@]}"; do
> +                               if use "python_targets_${impl}"; then
> +                                       case "${impl}" in
> +                                               python2_*)
> +                                                       if [[ ${py2+1} ]]; then
> +                                                               debug-print "${FUNCNAME}: -> more than one py2: ${py2} ${impl}"
> +                                                               return 1
> +                                                       fi
> +                                                       py2=${impl/_/.}
> +                                                       ;;
> +                                               python3_*)
> +                                                       if [[ ${py3+1} ]]; then
> +                                                               debug-print "${FUNCNAME}: -> more than one py3: ${py3} ${impl}"
> +                                                               return 1
> +                                                       fi
> +                                                       py3=${impl/_/.}
> +                                                       ;;
> +                                               *)
> +                                                       return 1
> +                                                       ;;
> +                                       esac
> +                               else
> +                                       case "${impl}" in
> +                                               python2_*)
> +                                                       dis_py2=1
> +                                                       ;;
> +                                               python3_*)
> +                                                       dis_py3=1
> +                                                       ;;
> +                                       esac
> +                               fi
> +                       done
> +
> +                       # The eselect solution won't work if the disabled Python version
> +                       # is installed.
> +                       if [[ ! ${py2+1} && ${dis_py2} ]]; then
> +                               debug-print "${FUNCNAME}: -> all py2 versions disabled"
> +                               if has_version '=dev-lang/python-2*'; then
> +                                       debug-print "${FUNCNAME}: ---> but =python-2* installed!"
> +                                       return 1
> +                               fi
> +                       fi
> +                       if [[ ! ${py3+1} && ${dis_py3} ]]; then
> +                               debug-print "${FUNCNAME}: -> all py3 versions disabled"
> +                               if has_version '=dev-lang/python-3*'; then
> +                                       debug-print "${FUNCNAME}: ---> but =python-3* installed!"
> +                                       return 1
> +                               fi
> +                       fi
> +
> +                       local warned
> +
> +                       # Now check whether the correct implementations are active.
> +                       if [[ ${py2+1} ]]; then
> +                               local sel_py2=$(eselect python show --python2)
> +
> +                               debug-print "${FUNCNAME}: -> py2 built: ${py2}, active: ${sel_py2}"
> +                               if [[ ${py2} != ${sel_py2} ]]; then
> +                                       ewarn "Building package for ${py2} only while ${sel_py2} is active."
> +                                       ewarn "Please consider switching the active Python 2 interpreter:"
> +                                       ewarn
> +                                       ewarn " eselect python set --python2 ${py2}"
> +                                       warned=1
> +                               fi
> +                       fi
> +
> +                       if [[ ${py3+1} ]]; then
> +                               local sel_py3=$(eselect python show --python3)
> +
> +                               debug-print "${FUNCNAME}: -> py3 built: ${py3}, active: ${sel_py3}"
> +                               if [[ ${py3} != ${sel_py3} ]]; then
> +                                       [[ ${warned} ]] && ewarn
> +                                       ewarn "Building package for ${py3} only while ${sel_py3} is active."
> +                                       ewarn "Please consider switching the active Python 3 interpreter:"
> +                                       ewarn
> +                                       ewarn " eselect python set --python3 ${py3}"
> +                                       warned=1
> +                               fi
> +                       fi
> +
> +                       if [[ ${warned} ]]; then
> +                               ewarn
> +                               ewarn "Please note that after switching the active Python interpreter,"
> +                               ewarn "you may need to run 'python-updater' to ensure the system integrity."

Change "ensure the system integrity" to something like "rebuild
affected packages". The former statement is awkward and doesn't really
explain anything.

> +                               ewarn
> +                               ewarn "For more information on python.eclass compatibility, please see"
> +                               ewarn "the appropriate python-r1 User's Guide chapter [1]."
> +                               ewarn
> +                               ewarn "[1] http://www.gentoo.org/proj/en/Python/python-r1/user-guide.xml#doc_chap2"
> +                       fi
> +               }
> +
> +               # If user has no USE_PYTHON, try to avoid it.
> +               if [[ ! ${USE_PYTHON} ]]; then
> +                       debug-print "${FUNCNAME}: trying eselect solution ..."
> +                       _try_eselect && return
> +               fi
> +
> +               debug-print "${FUNCNAME}: trying USE_PYTHON solution ..."
> +               debug-print "${FUNCNAME}: -> USE_PYTHON=${USE_PYTHON}"
> +
> +               local impl old=${USE_PYTHON} new=() removed=()
> +
> +               for impl in "${PYTHON_COMPAT[@]}"; do
> +                       local abi
> +                       case "${impl}" in
> +                               python*)
> +                                       abi=${impl#python}
> +                                       ;;
> +                               jython*)
> +                                       abi=${impl#jython}-jython
> +                                       ;;
> +                               pypy*)
> +                                       abi=2.7-pypy-${impl#pypy}
> +                                       ;;
> +                               *)
> +                                       die "Unexpected Python implementation: ${impl}"
> +                                       ;;
> +                       esac
> +                       abi=${abi/_/.}
> +
> +                       has "${abi}" ${USE_PYTHON}
> +                       local has_abi=${?}
> +                       use "python_targets_${impl}"
> +                       local has_impl=${?}
> +
> +                       # 0 = has, 1 = does not have
> +                       if [[ ${has_abi} == 0 && ${has_impl} == 1 ]]; then
> +                               debug-print "${FUNCNAME}: ---> remove ${abi}"
> +                               # remove from USE_PYTHON
> +                               old=${old/${abi}/}
> +                               removed+=( ${abi} )
> +                       elif [[ ${has_abi} == 1 && ${has_impl} == 0 ]]; then
> +                               debug-print "${FUNCNAME}: ---> add ${abi}"
> +                               # add to USE_PYTHON
> +                               new+=( ${abi} )
> +                       fi
> +               done
> +
> +               if [[ ${removed[@]} || ${new[@]} ]]; then
> +                       old=( ${old} )
> +
> +                       debug-print "${FUNCNAME}: -> old: ${old[@]}"
> +                       debug-print "${FUNCNAME}: -> new: ${new[@]}"
> +                       debug-print "${FUNCNAME}: -> removed: ${removed[@]}"
> +
> +                       if [[ ${USE_PYTHON} ]]; then
> +                               ewarn "It seems that your USE_PYTHON setting does list different Python"

Drop "It seems that"; it softens the statement for no reason. This
also applies to a few statements below.
The phrase "does list" looks odd in English.
Replace with "Your USE_PYTHON setting lists different Python ...".

> +                               ewarn "implementations than your PYTHON_TARGETS variable. Please consider"
> +                               ewarn "using the following value instead:"
> +                               ewarn
> +                               ewarn " USE_PYTHON='\033[35m${old[@]}${new[@]+ \033[1m${new[@]}}\033[0m'"
> +
> +                               if [[ ${removed[@]} ]]; then
> +                                       ewarn
> +                                       ewarn "(removed \033[31m${removed[@]}\033[0m)"
> +                               fi
> +                       else
> +                               ewarn "It seems that you need to set USE_PYTHON to make sure that legacy"

s/It seems that you need/You should/

> +                               ewarn "packages will be built with respect to PYTHON_TARGETS correctly:"
> +                               ewarn
> +                               ewarn " USE_PYTHON='\033[35;1m${new[@]}\033[0m'"
> +                       fi
> +
> +                       ewarn
> +                       ewarn "Please note that after changing the USE_PYTHON variable, you may need"
> +                       ewarn "to run 'python-updater' to ensure the system integrity."

Again, s/ensure the system integrity/rebuild affected packages/.

> +                       ewarn
> +                       ewarn "For more information on python.eclass compatibility, please see"
> +                       ewarn "the appropriate python-r1 User's Guide chapter [1]."
> +                       ewarn
> +                       ewarn "[1] http://www.gentoo.org/proj/en/Python/python-r1/user-guide.xml#doc_chap2"
> +               fi
> +       fi
> +}
> +
>  # @FUNCTION: python_foreach_impl
>  # @USAGE: <command> [<args>...]
>  # @DESCRIPTION:
> @@ -376,6 +572,8 @@ python_copy_sources() {
>  python_foreach_impl() {
>         debug-print-function ${FUNCNAME} "${@}"
>
> +       _python_check_USE_PYTHON
> +
>         local impl
>         local bdir=${BUILD_DIR:-${S}}
>
> --
> 1.8.0
>