[gentoo-python] [PATCHES] Cleaning python_foreach_impl() up and introduce a parallel variant

[gentoo-python] [PATCHES] Cleaning python_foreach_impl() up and introduce a parallel variant

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 1592 bytes --]

Hello,

I'm sending a batch of patches (git-kind of replies to this mail).
The patches do the following:


1) clean up failure handling in python_foreach_impl().

Currently, python_foreach_impl supposedly dies if one of the functions
returns failure. This is quite unnecessary since most of the functions
called there are supposed to be fatal themselves, and may be a bit
confusing.

The idea is that:

  python_foreach_impl foo

should be as close to:

  for x in ...; do foo; done

as possible. The implicit '|| die' doesn't really look helpful since it
may hurt people with small unimportant conditionals at the end of their
functions, and may cause a few others to actually rely
on python_foreach_impl handling failures for them.

Therefore, the function is not fatal anymore. Instead, it just returns
0 for complete success or status of first failure. In any case,
the thingie called should 'die' on itself.


2) introduce a parallel variant of python_foreach_impl().

Based on the code from distutils-r1. For that reason and compatibility,
the variable controlling no of jobs is still being called
'DISTUTILS_JOBS'.

The new function is also used in distutils-r1, with the two last
patches doing pre-merge cleanup and actually using it.


3) move split log handling into python*_foreach_impl().

With the parallel build feature no longer being distutils-specific,
the split logs shall be accessible outside of it. Therefore, I have
moved their support completely to python*_foreach_impl() functions.


-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 966 bytes --]

[gentoo-python] [PATCH 1/5] Make python_foreach_impl() non-fatal.

[Michał Górny]

The current behavior is ambiguous and a bit unexpected.

It was my mistake to over-hack that function but it seems that people
didn't really depend on it. Mostly because the common helpers are fatal
themselves.
---
 gx86/eclass/python-r1.eclass | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/gx86/eclass/python-r1.eclass b/gx86/eclass/python-r1.eclass
index 5735a6d..083157c 100644
--- a/gx86/eclass/python-r1.eclass
+++ b/gx86/eclass/python-r1.eclass
@@ -583,8 +583,11 @@ _python_check_USE_PYTHON() {
 # @DESCRIPTION:
 # Run the given command for each of the enabled Python implementations.
 # If additional parameters are passed, they will be passed through
-# to the command. If the command fails, python_foreach_impl dies.
-# If necessary, use ':' to force a successful return.
+# to the command.
+#
+# The function will return 0 status if all invocations succeed.
+# Otherwise, the return code from first failing invocation will
+# be returned.
 #
 # For each command being run, EPYTHON, PYTHON and BUILD_DIR are set
 # locally, and the former two are exported to the command environment.
@@ -596,6 +599,7 @@ python_foreach_impl() {
 
 	local impl
 	local bdir=${BUILD_DIR:-${S}}
+	local ret=0 lret=0
 
 	debug-print "${FUNCNAME}: bdir = ${bdir}"
 	for impl in "${_PYTHON_ALL_IMPLS[@]}"; do
@@ -609,9 +613,14 @@ python_foreach_impl() {
 			export EPYTHON PYTHON
 
 			einfo "${EPYTHON}: running ${@}"
-			"${@}" || die "${EPYTHON}: ${1} failed"
+			"${@}"
+			lret=${?}
+
+			[[ ${ret} -eq 0 && ${lret} -ne 0 ]] && ret=${lret}
 		fi
 	done
+
+	return ${ret}
 }
 
 # @FUNCTION: python_export_best
-- 
1.8.1.2

[gentoo-python] [PATCH 2/5] Introduce python_parallel_foreach_impl().

[Michał Górny]

A parallel variant of python_foreach_impl. Also, move DISTUTILS_JOBS to
python-r1 then.
---
 gx86/eclass/distutils-r1.eclass |  8 ------
 gx86/eclass/python-r1.eclass    | 54 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 8 deletions(-)

diff --git a/gx86/eclass/distutils-r1.eclass b/gx86/eclass/distutils-r1.eclass
index e662fa2..8457fd2 100644
--- a/gx86/eclass/distutils-r1.eclass
+++ b/gx86/eclass/distutils-r1.eclass
@@ -100,14 +100,6 @@ if [[ ! ${DISTUTILS_OPTIONAL} ]]; then
 	DEPEND=${PYTHON_DEPS}
 fi
 
-# @ECLASS-VARIABLE: DISTUTILS_JOBS
-# @DEFAULT_UNSET
-# @DESCRIPTION:
-# The number of parallel jobs to run for distutils-r1 parallel builds.
-# If unset, the job-count in ${MAKEOPTS} will be used.
-#
-# This variable is intended to be set in make.conf.
-
 # @ECLASS-VARIABLE: PATCHES
 # @DEFAULT_UNSET
 # @DESCRIPTION:
diff --git a/gx86/eclass/python-r1.eclass b/gx86/eclass/python-r1.eclass
index 083157c..09e6417 100644
--- a/gx86/eclass/python-r1.eclass
+++ b/gx86/eclass/python-r1.eclass
@@ -173,6 +173,14 @@ _python_set_globals() {
 }
 _python_set_globals
 
+# @ECLASS-VARIABLE: DISTUTILS_JOBS
+# @DEFAULT_UNSET
+# @DESCRIPTION:
+# The number of parallel jobs to run for distutils-r1 parallel builds.
+# If unset, the job-count in ${MAKEOPTS} will be used.
+#
+# This variable is intended to be set in make.conf.
+
 # @FUNCTION: _python_validate_useflags
 # @INTERNAL
 # @DESCRIPTION:
@@ -623,6 +631,52 @@ python_foreach_impl() {
 	return ${ret}
 }
 
+# @FUNCTION: python_parallel_foreach_impl
+# @USAGE: <command> [<args>...]
+# @DESCRIPTION:
+# Run the given command for each of the enabled Python implementations.
+# If additional parameters are passed, they will be passed through
+# to the command.
+#
+# The function will return 0 status if all invocations succeed.
+# Otherwise, the return code from first failing invocation will
+# be returned.
+#
+# For each command being run, EPYTHON, PYTHON and BUILD_DIR are set
+# locally, and the former two are exported to the command environment.
+#
+# Multiple invocations of the command will be run in parallel, up to
+# DISTUTILS_JOBS (defaulting to '-j' option argument from MAKEOPTS).
+python_parallel_foreach_impl() {
+	debug-print-function ${FUNCNAME} "${@}"
+
+	local ret lret
+
+	_python_parallel() {
+		(
+			multijob_child_init
+			"${@}"
+		) &
+		multijob_post_fork
+	}
+
+	local opts
+	if [[ ${DISTUTILS_JOBS} ]]; then
+		opts=-j${DISTUTILS_JOBS}
+	else
+		opts=${MAKEOPTS}
+	fi
+
+	multijob_init "${opts}"
+	python_foreach_impl _python_parallel "${@}"
+	ret=${?}
+	multijob_finish
+	lret=${?}
+
+	[[ ${ret} -eq 0 ]] && ret=${lret}
+	return ${ret}
+}
+
 # @FUNCTION: python_export_best
 # @USAGE: [<variable>...]
 # @DESCRIPTION:
-- 
1.8.1.2

[gentoo-python] [PATCH 3/5] Init&finish multijob inside d-r1_run_foreach_impl().

[Michał Górny]

The separate init & finish call was needed before
distutils-r1_run_foreach_impl() was introduced. Now it's simpler to call
them both inside that function.
---
 gx86/eclass/distutils-r1.eclass | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/gx86/eclass/distutils-r1.eclass b/gx86/eclass/distutils-r1.eclass
index 8457fd2..adb5d42 100644
--- a/gx86/eclass/distutils-r1.eclass
+++ b/gx86/eclass/distutils-r1.eclass
@@ -610,7 +610,9 @@ _distutils-r1_run_foreach_impl() {
 	set -- distutils-r1_run_phase "${@}"
 
 	if [[ ! ${DISTUTILS_SINGLE_IMPL} ]]; then
+		_distutils-r1_multijob_init
 		python_foreach_impl "${@}"
+		_distutils-r1_multijob_finish
 	else
 		if [[ ! ${EPYTHON} ]]; then
 			die "EPYTHON unset, python-single-r1_pkg_setup not called?!"
@@ -632,19 +634,15 @@ distutils-r1_src_prepare() {
 		distutils-r1_python_prepare_all
 	fi
 
-	_distutils-r1_multijob_init
 	if declare -f python_prepare >/dev/null; then
 		_distutils-r1_run_foreach_impl python_prepare
 	fi
-	_distutils-r1_multijob_finish
 }
 
 distutils-r1_src_configure() {
-	_distutils-r1_multijob_init
 	if declare -f python_configure >/dev/null; then
 		_distutils-r1_run_foreach_impl python_configure
 	fi
-	_distutils-r1_multijob_finish
 
 	if declare -f python_configure_all >/dev/null; then
 		_distutils-r1_run_common_phase python_configure_all
@@ -654,13 +652,11 @@ distutils-r1_src_configure() {
 distutils-r1_src_compile() {
 	debug-print-function ${FUNCNAME} "${@}"
 
-	_distutils-r1_multijob_init
 	if declare -f python_compile >/dev/null; then
 		_distutils-r1_run_foreach_impl python_compile
 	else
 		_distutils-r1_run_foreach_impl distutils-r1_python_compile
 	fi
-	_distutils-r1_multijob_finish
 
 	if declare -f python_compile_all >/dev/null; then
 		_distutils-r1_run_common_phase python_compile_all
@@ -670,11 +666,9 @@ distutils-r1_src_compile() {
 distutils-r1_src_test() {
 	debug-print-function ${FUNCNAME} "${@}"
 
-	_distutils-r1_multijob_init
 	if declare -f python_test >/dev/null; then
 		_distutils-r1_run_foreach_impl python_test
 	fi
-	_distutils-r1_multijob_finish
 
 	if declare -f python_test_all >/dev/null; then
 		_distutils-r1_run_common_phase python_test_all
@@ -684,13 +678,11 @@ distutils-r1_src_test() {
 distutils-r1_src_install() {
 	debug-print-function ${FUNCNAME} "${@}"
 
-	_distutils-r1_multijob_init
 	if declare -f python_install >/dev/null; then
 		_distutils-r1_run_foreach_impl python_install
 	else
 		_distutils-r1_run_foreach_impl distutils-r1_python_install
 	fi
-	_distutils-r1_multijob_finish
 
 	if declare -f python_install_all >/dev/null; then
 		_distutils-r1_run_common_phase python_install_all
-- 
1.8.1.2

[gentoo-python] [PATCH 4/5] Use python_parallel_foreach_impl() in distutils-r1.

[Michał Górny]

---
 gx86/eclass/distutils-r1.eclass | 53 ++++++-----------------------------------
 1 file changed, 7 insertions(+), 46 deletions(-)

diff --git a/gx86/eclass/distutils-r1.eclass b/gx86/eclass/distutils-r1.eclass
index adb5d42..42b48bc 100644
--- a/gx86/eclass/distutils-r1.eclass
+++ b/gx86/eclass/distutils-r1.eclass
@@ -525,16 +525,7 @@ distutils-r1_run_phase() {
 
 	mkdir -p "${TMPDIR}" || die
 
-	if [[ ${DISTUTILS_NO_PARALLEL_BUILD} || ${DISTUTILS_SINGLE_IMPL} ]]
-	then
-		"${@}" 2>&1 | tee -a "${T}/build-${EPYTHON}.log"
-	else
-		(
-			multijob_child_init
-			"${@}" 2>&1 | tee -a "${T}/build-${EPYTHON}.log"
-		) &
-		multijob_post_fork
-	fi
+	"${@}"
 
 	if [[ ${DISTUTILS_IN_SOURCE_BUILD} && ! ${DISTUTILS_SINGLE_IMPL} ]]
 	then
@@ -566,39 +557,6 @@ _distutils-r1_run_common_phase() {
 	"${@}"
 }
 
-# @FUNCTION: _distutils-r1_multijob_init
-# @INTERNAL
-# @DESCRIPTION:
-# Init multijob, taking the job-count from ${DISTUTILS_JOBS}.
-_distutils-r1_multijob_init() {
-	debug-print-function ${FUNCNAME} "${@}"
-
-	if [[ ! ${DISTUTILS_NO_PARALLEL_BUILD} && ! ${DISTUTILS_SINGLE_IMPL} ]]
-	then
-		local opts
-		if [[ ${DISTUTILS_JOBS} ]]; then
-			opts=-j${DISTUTILS_JOBS}
-		else
-			opts=${MAKEOPTS}
-		fi
-
-		multijob_init "${opts}"
-	fi
-}
-
-# @FUNCTION: _distutils-r1_multijob_finish
-# @INTERNAL
-# @DESCRIPTION:
-# Finish multijob if used.
-_distutils-r1_multijob_finish() {
-	debug-print-function ${FUNCNAME} "${@}"
-
-	if [[ ! ${DISTUTILS_NO_PARALLEL_BUILD} && ! ${DISTUTILS_SINGLE_IMPL} ]]
-	then
-		multijob_finish
-	fi
-}
-
 # @FUNCTION: _distutils-r1_run_foreach_impl
 # @INTERNAL
 # @DESCRIPTION:
@@ -610,9 +568,12 @@ _distutils-r1_run_foreach_impl() {
 	set -- distutils-r1_run_phase "${@}"
 
 	if [[ ! ${DISTUTILS_SINGLE_IMPL} ]]; then
-		_distutils-r1_multijob_init
-		python_foreach_impl "${@}"
-		_distutils-r1_multijob_finish
+		if [[ ${DISTUTILS_NO_PARALLEL_BUILD} || ${DISTUTILS_SINGLE_IMPL} ]]
+		then
+			python_foreach_impl "${@}"
+		else
+			python_parallel_foreach_impl "${@}"
+		fi
 	else
 		if [[ ! ${EPYTHON} ]]; then
 			die "EPYTHON unset, python-single-r1_pkg_setup not called?!"
-- 
1.8.1.2

[gentoo-python] [PATCH 5/5] Re-enable split logs, now in python*_foreach_impl().

[Michał Górny]

This provides the split logs accessible to every Python package, not
only ones using distutils.
---
 gx86/eclass/python-r1.eclass | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/gx86/eclass/python-r1.eclass b/gx86/eclass/python-r1.eclass
index 09e6417..1b26513 100644
--- a/gx86/eclass/python-r1.eclass
+++ b/gx86/eclass/python-r1.eclass
@@ -620,8 +620,17 @@ python_foreach_impl() {
 			local BUILD_DIR=${bdir%%/}-${impl}
 			export EPYTHON PYTHON
 
-			einfo "${EPYTHON}: running ${@}"
-			"${@}"
+			einfo "${EPYTHON}: running ${@}" \
+				| tee -a "${T}/build-${EPYTHON}.log"
+
+			# _python_parallel() does redirection internally.
+			# note: this is a hidden API to avoid writing python_foreach_impl
+			# twice. do *not* even think of using it anywhere else.
+			if [[ ${1} == _python_parallel ]]; then
+				"${@}"
+			else
+				"${@}" 2>&1 | tee -a "${T}/build-${EPYTHON}.log"
+			fi
 			lret=${?}
 
 			[[ ${ret} -eq 0 && ${lret} -ne 0 ]] && ret=${lret}
@@ -655,7 +664,8 @@ python_parallel_foreach_impl() {
 	_python_parallel() {
 		(
 			multijob_child_init
-			"${@}"
+			"${@}" 2>&1 | tee -a "${T}/build-${EPYTHON}.log"
+			exit ${PIPESTATUS[0]}
 		) &
 		multijob_post_fork
 	}
-- 
1.8.1.2

[gentoo-python] Re: [PATCH 5/5] Re-enable split logs, now in python*_foreach_impl().

[Mike Gilbert]

[-- Attachment #1: Type: text/plain, Size: 1475 bytes --]

On 02/21/2013 05:10 PM, Michał Górny wrote:
> This provides the split logs accessible to every Python package, not
> only ones using distutils.
> ---
>  gx86/eclass/python-r1.eclass | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/gx86/eclass/python-r1.eclass b/gx86/eclass/python-r1.eclass
> index 09e6417..1b26513 100644
> --- a/gx86/eclass/python-r1.eclass
> +++ b/gx86/eclass/python-r1.eclass
> @@ -620,8 +620,17 @@ python_foreach_impl() {
>  			local BUILD_DIR=${bdir%%/}-${impl}
>  			export EPYTHON PYTHON
>  
> -			einfo "${EPYTHON}: running ${@}"
> -			"${@}"
> +			einfo "${EPYTHON}: running ${@}" \
> +				| tee -a "${T}/build-${EPYTHON}.log"
> +
> +			# _python_parallel() does redirection internally.
> +			# note: this is a hidden API to avoid writing python_foreach_impl
> +			# twice. do *not* even think of using it anywhere else.
> +			if [[ ${1} == _python_parallel ]]; then
> +				"${@}"
> +			else
> +				"${@}" 2>&1 | tee -a "${T}/build-${EPYTHON}.log"
> +			fi
>  			lret=${?}
>  
>  			[[ ${ret} -eq 0 && ${lret} -ne 0 ]] && ret=${lret}
> @@ -655,7 +664,8 @@ python_parallel_foreach_impl() {
>  	_python_parallel() {
>  		(
>  			multijob_child_init
> -			"${@}"
> +			"${@}" 2>&1 | tee -a "${T}/build-${EPYTHON}.log"
> +			exit ${PIPESTATUS[0]}
>  		) &
>  		multijob_post_fork
>  	}
> 

Why do we call "exit ${PIPESTATUS[0]}"? I have never seen that before.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 230 bytes --]

[gentoo-python] Re: [PATCHES] Cleaning python_foreach_impl() up and introduce a parallel variant

[Mike Gilbert]

[-- Attachment #1: Type: text/plain, Size: 1868 bytes --]

On 02/21/2013 05:07 PM, Michał Górny wrote:
> Hello,
> 
> I'm sending a batch of patches (git-kind of replies to this mail).
> The patches do the following:
> 
> 
> 1) clean up failure handling in python_foreach_impl().
> 
> Currently, python_foreach_impl supposedly dies if one of the functions
> returns failure. This is quite unnecessary since most of the functions
> called there are supposed to be fatal themselves, and may be a bit
> confusing.
> 
> The idea is that:
> 
>   python_foreach_impl foo
> 
> should be as close to:
> 
>   for x in ...; do foo; done
> 
> as possible. The implicit '|| die' doesn't really look helpful since it
> may hurt people with small unimportant conditionals at the end of their
> functions, and may cause a few others to actually rely
> on python_foreach_impl handling failures for them.
> 
> Therefore, the function is not fatal anymore. Instead, it just returns
> 0 for complete success or status of first failure. In any case,
> the thingie called should 'die' on itself.
> 
> 
> 2) introduce a parallel variant of python_foreach_impl().
> 
> Based on the code from distutils-r1. For that reason and compatibility,
> the variable controlling no of jobs is still being called
> 'DISTUTILS_JOBS'.
> 
> The new function is also used in distutils-r1, with the two last
> patches doing pre-merge cleanup and actually using it.
> 
> 
> 3) move split log handling into python*_foreach_impl().
> 
> With the parallel build feature no longer being distutils-specific,
> the split logs shall be accessible outside of it. Therefore, I have
> moved their support completely to python*_foreach_impl() functions.
> 
> 

I don't have any problems with this as a whole.

I trust you will do some testing before moving it to the tree; if you
want some help with that, I can pitch in.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 230 bytes --]

Re: [gentoo-python] Re: [PATCH 5/5] Re-enable split logs, now in python*_foreach_impl().

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 752 bytes --]

On Sat, 23 Feb 2013 20:42:10 -0500
Mike Gilbert <floppym@gentoo.org> wrote:

> On 02/21/2013 05:10 PM, Michał Górny wrote:
> >  			[[ ${ret} -eq 0 && ${lret} -ne 0 ]] && ret=${lret}
> > @@ -655,7 +664,8 @@ python_parallel_foreach_impl() {
> >  	_python_parallel() {
> >  		(
> >  			multijob_child_init
> > -			"${@}"
> > +			"${@}" 2>&1 | tee -a "${T}/build-${EPYTHON}.log"
> > +			exit ${PIPESTATUS[0]}
> >  		) &
> >  		multijob_post_fork
> >  	}
> > 
> 
> Why do we call "exit ${PIPESTATUS[0]}"? I have never seen that before.

${PIPESTATUS[n]} is exit status of n-th thing in the pipe. So:

  ${PIPESTATUS[0]} is the status of ${1}

  ${PIPESTATUS[1]} == ${?} is the status of 'tee'

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 966 bytes --]