While looking at the blocker in beautifulsoup-4.4.0 dependencies (leading to [1]), I noticed that python_gen_usedep() does not behave according to its documentation[2]. I consulted floppym in irc. Floppym found it odd. Example ebuild: EAPI=5 PYTHON_COMPAT=(python{2_7,3_4}) inherit python-r1 SLOT="0" DEPEND="$(python_gen_cond_dep '!dev-python/chardet[${PYTHON_USEDEP}]' "${PYTHON_COMPAT[@]}")" echo $DEPEND Output: eroen@occam /usr/local/portage/app-misc/python-blocker $ ebuild python-blocker-0.ebuild clean pretend python_targets_python2_7? ( !dev-python/chardet[python_targets_python2_7(-)?,-python_single_target_python2_7(-),python_targets_python3_4(-)?,-python_single_target_python3_4(-)] ) python_targets_python3_4? ( !dev-python/chardet[python_targets_python2_7(-)?,-python_single_target_python2_7(-),python_targets_python3_4(-)?,-python_single_target_python3_4(-)] ) I expect: python_targets_python2_7? ( !dev-python/chardet[python_targets_python2_7(-)?,-python_single_target_python2_7(-)] ) python_targets_python3_4? ( !dev-python/chardet[python_targets_python2_7(-)?,-python_single_target_python2_7(-)] ) This is no big deal for most usage, but when making a blocker it becomes troublesome. 1: https://gitweb.gentoo.org/repo/gentoo.git/commit?id=6cb2905fafdd2f2c0407c2bb4a1bc12b6f3126a9 2: https://gitweb.gentoo.org/repo/gentoo.git/tree/eclass/python-r1.eclass#n344
Suggested change: --- eclass/python-r1.eclass +++ eclass/python-r1.eclass @@ -385,7 +385,7 @@ # (since python_gen_usedep() will not return ${PYTHON_USEDEP} # the code is run at most once) if [[ ${dep} == *'${PYTHON_USEDEP}'* ]]; then - local PYTHON_USEDEP=$(python_gen_usedep "${@}") + local PYTHON_USEDEP=$(python_gen_usedep "$impl") dep=${dep//\$\{PYTHON_USEDEP\}/${PYTHON_USEDEP}} fi
Err, no. My suggestion is wrong and doesn't work. Expected result should really be be: python_targets_python2_7? ( !dev-python/chardet[python_targets_python2_7(-)?,-python_single_target_python2_7(-)] ) python_targets_python3_4? ( !dev-python/chardet[python_targets_python3_4(-)?,-python_single_target_python3_4(-)] )
Suggested change 2: --- eclass/python-r1.eclass +++ eclass/python-r1.eclass @@ -373,19 +373,20 @@ local impl pattern local matches=() - local dep=${1} + local orig_dep=${1} shift for impl in "${PYTHON_COMPAT[@]}"; do _python_impl_supported "${impl}" || continue - for pattern; do + for pattern in "$@"; do if [[ ${impl} == ${pattern} ]]; then + local dep="$orig_dep" # substitute ${PYTHON_USEDEP} if used - # (since python_gen_usedep() will not return ${PYTHON_USEDEP} - # the code is run at most once) + # (python_gen_usedep() will run several times, once for each + # implementation in PYTHON_COMPAT) if [[ ${dep} == *'${PYTHON_USEDEP}'* ]]; then - local PYTHON_USEDEP=$(python_gen_usedep "${@}") + local PYTHON_USEDEP=$(python_gen_usedep "${pattern}") dep=${dep//\$\{PYTHON_USEDEP\}/${PYTHON_USEDEP}} fi It might also be prudent to explicitly mention in documentation that real variable substitution for the "dep" argument doesn't work.
This difference between docs and behaviour has existed since the functionality was added.[3] > It might also be prudent to explicitly mention in documentation that real variable substitution for the "dep" argument doesn't work. Apologies, this is already the case, but it is not rendered correctly in the man pages, which makes it easy to miss. 3: https://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/eclass/python-r1.eclass?r1=1.71&r2=1.72
The idea behind it was rather simple: it's easier and less wasteful to generate one USE-dep and reuse it for all blocks than generate N different strings. Additionally, having them all in the first block means Portage will output better suggestions like 'enable three pythons' instead of 'enable first python', 'enable second python'...
First of all, please don't paste patches inline, bugzie will break them. Either attach them, or (preferable for me) open a pull request with eclass change on github. (In reply to eroen from comment #3) > Suggested change 2: > > --- eclass/python-r1.eclass > +++ eclass/python-r1.eclass > @@ -373,19 +373,20 @@ > local impl pattern > local matches=() > > - local dep=${1} > + local orig_dep=${1} > shift > > for impl in "${PYTHON_COMPAT[@]}"; do > _python_impl_supported "${impl}" || continue > > - for pattern; do > + for pattern in "$@"; do Unrelated and unnecessary. > if [[ ${impl} == ${pattern} ]]; then > + local dep="$orig_dep" > # substitute ${PYTHON_USEDEP} if used > - # (since python_gen_usedep() will not return ${PYTHON_USEDEP} > - # the code is run at most once) > + # (python_gen_usedep() will run several times, once for each > + # implementation in PYTHON_COMPAT) Remove the comment inside () completely, it was only explaining implicit behavior which is irrelevant now. > if [[ ${dep} == *'${PYTHON_USEDEP}'* ]]; then > - local PYTHON_USEDEP=$(python_gen_usedep "${@}") > + local PYTHON_USEDEP=$(python_gen_usedep "${pattern}") > dep=${dep//\$\{PYTHON_USEDEP\}/${PYTHON_USEDEP}} > fi > > > It might also be prudent to explicitly mention in documentation that real > variable substitution for the "dep" argument doesn't work. Also, please check how Portage behaves when it needs to enable more than one flag on dependency, with --autounmask disabled, enabled and enabled with --autounmask-write. In particular, if Portage catches two missing flags immediately or requests two changes in a row.
No reply since Oct 2015, plus I still don't see a major reason to change that.
