[gentoo-dev] [PATCH] Eclass changes for cross-compiling Python modules

[gentoo-dev] [PATCH] Eclass changes for cross-compiling Python modules

[James Le Cuirot]

Here's a series of eclass changes I've been working on since August to
allow Python modules to be cross-compiled. We previously believed this
to be practically impossible without significant changes upstream and
in the wider Python ecosystem so getting here feels like quite an
achievement. While the approach is a little unconventional, it doesn't
feel overly hacky and the results are quite consistently good.

These changes should also benefit prefix and I did a full prefix
bootstrap for the first time this week just to make sure I didn't
break it.

A handful of other Python packages that don't quite follow the usual
mantra have needed fixing up. I haven't included those changes here
but you can view them on this GitHub pull request.

https://github.com/gentoo/gentoo/pull/9822

Once this is in place, I can finish my long-awaited revamp of my
cross-boss project that will allow you to cross-compile @system from
scratch with very little effort.

[gentoo-dev] [PATCH 01/12] python-utils-r1.eclass: Fix some double slash paths

[James Le Cuirot]

If Python returns relative site or script directories then something
is very wrong!

Signed-off-by: James Le Cuirot <chewi@gentoo.org>
---
 eclass/python-utils-r1.eclass | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/eclass/python-utils-r1.eclass b/eclass/python-utils-r1.eclass
index e3cf82b4b58f..da76a755fb34 100644
--- a/eclass/python-utils-r1.eclass
+++ b/eclass/python-utils-r1.eclass
@@ -1,4 +1,4 @@
-# Copyright 1999-2018 Gentoo Foundation
+# Copyright 1999-2019 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
 
 # @ECLASS: python-utils-r1.eclass
@@ -797,11 +797,11 @@ python_newexe() {
 
 	# install the wrapper
 	_python_ln_rel "${ED%/}"/usr/lib/python-exec/python-exec2 \
-		"${ED%/}/${wrapd}/${newfn}" || die
+		"${ED%/}${wrapd}/${newfn}" || die
 
 	# don't use this at home, just call python_doscript() instead
 	if [[ ${_PYTHON_REWRITE_SHEBANG} ]]; then
-		python_fix_shebang -q "${ED%/}/${d}/${newfn}"
+		python_fix_shebang -q "${ED%/}${d}/${newfn}"
 	fi
 }
 
@@ -927,7 +927,7 @@ python_domodule() {
 		doins -r "${@}" || return ${?}
 	)
 
-	python_optimize "${ED%/}/${d}"
+	python_optimize "${ED%/}${d}"
 }
 
 # @FUNCTION: python_doheader
-- 
2.19.2

[gentoo-dev] [PATCH 02/12] python-utils-r1.eclass: New python_fix_shebang approach

[James Le Cuirot]

The previous approach would erroneously match foopython. The new
approach requires the match to start the string or be preceeded by a
slash, the only two cases we actually want. It does this with slightly
less code and allows the replacement of whole path strings that would
be problematic when passed to sed. This will be needed when
cross-compiling is addressed.

Signed-off-by: James Le Cuirot <chewi@gentoo.org>
---
 eclass/python-utils-r1.eclass | 78 ++++++++++++++---------------------
 1 file changed, 31 insertions(+), 47 deletions(-)

diff --git a/eclass/python-utils-r1.eclass b/eclass/python-utils-r1.eclass
index da76a755fb34..1eca0764a202 100644
--- a/eclass/python-utils-r1.eclass
+++ b/eclass/python-utils-r1.eclass
@@ -1165,8 +1165,7 @@ python_fix_shebang() {
 		[[ -d ${path} ]] && is_recursive=1
 
 		while IFS= read -r -d '' f; do
-			local shebang i
-			local error= from=
+			local shebang i= error= fix=
 
 			# note: we can't ||die here since read will fail if file
 			# has no newline characters
@@ -1175,65 +1174,56 @@ python_fix_shebang() {
 			# First, check if it's shebang at all...
 			if [[ ${shebang} == '#!'* ]]; then
 				local split_shebang=()
-				read -r -a split_shebang <<<${shebang} || die
+				read -r -a split_shebang <<<${shebang#\#\!} || die
 
 				# Match left-to-right in a loop, to avoid matching random
 				# repetitions like 'python2.7 python2'.
-				for i in "${split_shebang[@]}"; do
-					case "${i}" in
-						*"${EPYTHON}")
+				for i in $(seq 0 $((${#split_shebang[@]} - 1))); do
+					case "/${split_shebang[${i}]}" in
+						*/${EPYTHON})
 							debug-print "${FUNCNAME}: in file ${f#${D%/}}"
 							debug-print "${FUNCNAME}: shebang matches EPYTHON: ${shebang}"
 
 							# Nothing to do, move along.
 							any_correct=1
-							from=${EPYTHON}
+							continue 2
+							;;
+						*/python)
+							fix=1
 							break
 							;;
-						*python|*python[23])
-							debug-print "${FUNCNAME}: in file ${f#${D%/}}"
-							debug-print "${FUNCNAME}: rewriting shebang: ${shebang}"
-
-							if [[ ${i} == *python2 ]]; then
-								from=python2
-								if [[ ! ${force} ]]; then
-									python_is_python3 "${EPYTHON}" && error=1
-								fi
-							elif [[ ${i} == *python3 ]]; then
-								from=python3
-								if [[ ! ${force} ]]; then
-									python_is_python3 "${EPYTHON}" || error=1
-								fi
-							else
-								from=python
+						*/python2)
+							if [[ ! ${force} ]]; then
+								python_is_python3 "${EPYTHON}" && error=1
 							fi
+							fix=1
 							break
 							;;
-						*python[23].[0123456789]|*pypy|*pypy3|*jython[23].[0123456789])
-							# Explicit mismatch.
+						*/python3)
 							if [[ ! ${force} ]]; then
-								error=1
-							else
-								case "${i}" in
-									*python[23].[0123456789])
-										from="python[23].[0123456789]";;
-									*pypy)
-										from="pypy";;
-									*pypy3)
-										from="pypy3";;
-									*jython[23].[0123456789])
-										from="jython[23].[0123456789]";;
-									*)
-										die "${FUNCNAME}: internal error in 2nd pattern match";;
-								esac
+								python_is_python3 "${EPYTHON}" || error=1
 							fi
+							fix=1
+							break
+							;;
+						*/python[2-3].[0-9]|*/pypy|*/pypy3|*/jython[2-3].[0-9])
+							# Explicit mismatch.
+							[[ ! ${force} ]] && error=1
+							fix=1
 							break
 							;;
 					esac
 				done
 			fi
 
-			if [[ ! ${error} && ! ${from} ]]; then
+			if [[ ${fix} ]]; then
+				debug-print "${FUNCNAME}: in file ${f#${D%/}}"
+				debug-print "${FUNCNAME}: rewriting shebang: ${shebang}"
+
+				split_shebang[${i}]=/${split_shebang[${i}]}
+				split_shebang[${i}]=${split_shebang[${i}]%/*}
+				split_shebang[${i}]=${split_shebang[${i}]#/}${split_shebang[${i}]:+/}${EPYTHON}
+			elif [[ ! ${error} ]]; then
 				# Non-Python shebang. Allowed in recursive mode,
 				# disallowed when specifying file explicitly.
 				[[ ${is_recursive} ]] && continue
@@ -1245,13 +1235,7 @@ python_fix_shebang() {
 			fi
 
 			if [[ ! ${error} ]]; then
-				# We either want to match ${from} followed by space
-				# or at end-of-string.
-				if [[ ${shebang} == *${from}" "* ]]; then
-					sed -i -e "1s:${from} :${EPYTHON} :" "${f}" || die
-				else
-					sed -i -e "1s:${from}$:${EPYTHON}:" "${f}" || die
-				fi
+				sed -i -e "1c#\!${split_shebang[*]}" "${f}" || die
 				any_fixed=1
 			else
 				eerror "The file has incompatible shebang:"
-- 
2.19.2

[gentoo-dev] [PATCH 03/12] python-utils-r1.eclass: Have wrapper workdir default to "impl" arg

[James Le Cuirot]

It currently defaults to EPYTHON, which might not even be defined
yet. The "impl" arg defaults to EPYTHON anyway.

Signed-off-by: James Le Cuirot <chewi@gentoo.org>
---
 eclass/python-utils-r1.eclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/eclass/python-utils-r1.eclass b/eclass/python-utils-r1.eclass
index 1eca0764a202..1a549f49f3f2 100644
--- a/eclass/python-utils-r1.eclass
+++ b/eclass/python-utils-r1.eclass
@@ -980,8 +980,8 @@ python_doheader() {
 python_wrapper_setup() {
 	debug-print-function ${FUNCNAME} "${@}"
 
-	local workdir=${1:-${T}/${EPYTHON}}
 	local impl=${2:-${EPYTHON}}
+	local workdir=${1:-${T}/${impl}}
 
 	[[ ${workdir} ]] || die "${FUNCNAME}: no workdir specified."
 	[[ ${impl} ]] || die "${FUNCNAME}: no impl nor EPYTHON specified."
-- 
2.19.2

[gentoo-dev] [PATCH 04/12] python-utils-r1.eclass: Use wrapper scripts to fix cross-compiling

[James Le Cuirot]

Python has little concept of cross-compiling but it turns out that
making it work isn't so hard after all.

Platform-specific details are held within _sysconfigdata.py,
sysconfig.py, and various distutils files. If we simply symlink these
from SYSROOT into an empty directory and add that directory to
PYTHONPATH then we can utilise the build host's Python with the target
host's settings.

The pkg-config files were already being symlinked in a similar manner
but now we source them from within SYSROOT.

In order for PYTHONPATH to be respected, Python must be executed via
the wrapper, which was not the case before. We previously relied
solely on the PATH but now PYTHON must point to the wrapper rather
than the usual location under /usr/bin. However, we only do this when
SYSROOT or EPREFIX are effectively set to avoid unnecessary
complexity. This has required some rejigging in the way that PYTHON is
set but it should remain compatible with existing packages.

The python_wrapper_setup function that handles all this has had its
arguments reordered because no one ever uses the path/workdir
argument, which makes specifying other arguments tedious.

Some packages rely on python-config but luckily this is just a shell
script so it can be executed from within SYSROOT. This is bending the
rules of PMS slightly but Python leaves us with little choice. I also
wrote those rules so I'm allowed to bend them. ;)

PYTHON_INCLUDEDIR, PYTHON_LIBPATH, and their associated functions are
generally used during src_configure or src_compile and, as such, they
now need to have SYSROOT prepended.

python_doheader uses PYTHON_INCLUDEDIR to install new headers and
therefore needs the value without SYSROOT. It was already stripping
EPREFIX before so now it simply strips SYSROOT as well. Similar
instances of this can do likewise or call the functions with SYSROOT
unset to achieve the same effect.

To make all this work, we are assuming that CPython is located at
/usr/$(get_libdir)/${EPYTHON}, which is admittedly a little circular
when we are querying Python for the right paths. I feel the reason
that python_export was rewritten to query these dynamically was less
because someone might install Python to some weird location and more
to avoid special handling for each of the different
implementations. And what of those other implementations?

Being Java-based, Jython is installed under the platform-neutral
/usr/share and presumably has few other platform-specific
aspects. Enabling native extensions appears non-trivial and none of
our module packages have enabled support for it anyway.

I think PyPy could potentially support cross-compiling but it
hardcodes the native extension filename suffix within its own binaries
with no way to override it. Perhaps we could patch this in somehow but
I'm afraid I don't care enough.

Together with the following changes to distutils-r1.eclass, I have now
been able to cross-compile a large number of packages with native
Python extensions, most with no changes at all, and the rest with only
minor fixes.

Closes: https://bugs.gentoo.org/503874
Bug: https://bugs.gentoo.org/648652
Signed-off-by: James Le Cuirot <chewi@gentoo.org>
---
 eclass/python-utils-r1.eclass | 120 ++++++++++++++++++++++++++--------
 1 file changed, 92 insertions(+), 28 deletions(-)

diff --git a/eclass/python-utils-r1.eclass b/eclass/python-utils-r1.eclass
index 1a549f49f3f2..607af1b52f75 100644
--- a/eclass/python-utils-r1.eclass
+++ b/eclass/python-utils-r1.eclass
@@ -211,9 +211,15 @@ _python_impl_matches() {
 #
 # distutils-r1: Set within any of the python sub-phase functions.
 #
-# Example value:
+# If SYSROOT or EPREFIX are effectively set then this will point to an
+# automatically generated wrapper rather than the usual path under
+# /usr/bin in order to accommodate cross-compiling. We could do this all
+# the time but it would add unnecessary complexity.
+#
+# Example values:
 # @CODE
 # /usr/bin/python2.7
+# /var/tmp/portage/dev-python/pyblake2-1.2.3/temp/python2.7/bin/python2.7
 # @CODE
 
 # @ECLASS-VARIABLE: EPYTHON
@@ -256,6 +262,10 @@ _python_impl_matches() {
 # Set and exported on request using python_export().
 # Requires a proper build-time dependency on the Python implementation.
 #
+# This is prepended with SYSROOT in order to accommodate
+# cross-compiling. You may need to strip SYSROOT and EPREFIX if using it
+# to install new headers.
+#
 # Example value:
 # @CODE
 # /usr/include/python2.7
@@ -270,6 +280,9 @@ _python_impl_matches() {
 # Valid only for CPython. Requires a proper build-time dependency
 # on the Python implementation.
 #
+# This is prepended with SYSROOT in order to accommodate
+# cross-compiling.
+#
 # Example value:
 # @CODE
 # /usr/lib64/libpython2.7.so
@@ -314,6 +327,10 @@ _python_impl_matches() {
 # Valid only for CPython. Requires a proper build-time dependency
 # on the Python implementation and on pkg-config.
 #
+# This is prepended with SYSROOT in order to accommodate
+# cross-compiling. You generally should not execute files within SYSROOT
+# but python-config is always a shell script.
+#
 # Example value:
 # @CODE
 # /usr/bin/python2.7-config
@@ -380,6 +397,10 @@ python_export() {
 	esac
 	debug-print "${FUNCNAME}: implementation: ${impl}"
 
+	# Many variables below need a PYTHON variable but we should not
+	# export it unless explicitly requested so use _PYTHON instead.
+	local _PYTHON
+
 	for var; do
 		case "${var}" in
 			EPYTHON)
@@ -387,21 +408,21 @@ python_export() {
 				debug-print "${FUNCNAME}: EPYTHON = ${EPYTHON}"
 				;;
 			PYTHON)
-				export PYTHON=${EPREFIX}/usr/bin/${impl}
+				python_wrapper_setup ${impl} PYTHON
 				debug-print "${FUNCNAME}: PYTHON = ${PYTHON}"
 				;;
 			PYTHON_SITEDIR)
-				[[ -n ${PYTHON} ]] || die "PYTHON needs to be set for ${var} to be exported, or requested before it"
+				python_wrapper_setup ${impl} _PYTHON
 				# sysconfig can't be used because:
 				# 1) pypy doesn't give site-packages but stdlib
 				# 2) jython gives paths with wrong case
-				PYTHON_SITEDIR=$("${PYTHON}" -c 'import distutils.sysconfig; print(distutils.sysconfig.get_python_lib())') || die
+				PYTHON_SITEDIR=$("${_PYTHON}" -c 'import distutils.sysconfig; print(distutils.sysconfig.get_python_lib())') || die
 				export PYTHON_SITEDIR
 				debug-print "${FUNCNAME}: PYTHON_SITEDIR = ${PYTHON_SITEDIR}"
 				;;
 			PYTHON_INCLUDEDIR)
-				[[ -n ${PYTHON} ]] || die "PYTHON needs to be set for ${var} to be exported, or requested before it"
-				PYTHON_INCLUDEDIR=$("${PYTHON}" -c 'import distutils.sysconfig; print(distutils.sysconfig.get_python_inc())') || die
+				python_wrapper_setup ${impl} _PYTHON
+				PYTHON_INCLUDEDIR=${SYSROOT}$("${_PYTHON}" -c 'import distutils.sysconfig; print(distutils.sysconfig.get_python_inc())') || die
 				export PYTHON_INCLUDEDIR
 				debug-print "${FUNCNAME}: PYTHON_INCLUDEDIR = ${PYTHON_INCLUDEDIR}"
 
@@ -411,8 +432,8 @@ python_export() {
 				fi
 				;;
 			PYTHON_LIBPATH)
-				[[ -n ${PYTHON} ]] || die "PYTHON needs to be set for ${var} to be exported, or requested before it"
-				PYTHON_LIBPATH=$("${PYTHON}" -c 'import os.path, sysconfig; print(os.path.join(sysconfig.get_config_var("LIBDIR"), sysconfig.get_config_var("LDLIBRARY")) if sysconfig.get_config_var("LDLIBRARY") else "")') || die
+				python_wrapper_setup ${impl} _PYTHON
+				PYTHON_LIBPATH=${SYSROOT}$("${_PYTHON}" -c 'import os.path, sysconfig; print(os.path.join(sysconfig.get_config_var("LIBDIR"), sysconfig.get_config_var("LDLIBRARY")) if sysconfig.get_config_var("LDLIBRARY") else "")') || die
 				export PYTHON_LIBPATH
 				debug-print "${FUNCNAME}: PYTHON_LIBPATH = ${PYTHON_LIBPATH}"
 
@@ -457,9 +478,9 @@ python_export() {
 
 				case "${impl}" in
 					python*)
-						[[ -n ${PYTHON} ]] || die "PYTHON needs to be set for ${var} to be exported, or requested before it"
-						flags=$("${PYTHON}" -c 'import sysconfig; print(sysconfig.get_config_var("ABIFLAGS") or "")') || die
-						val=${PYTHON}${flags}-config
+						python_wrapper_setup ${impl} _PYTHON
+						flags=$("${_PYTHON}" -c 'import sysconfig; print(sysconfig.get_config_var("ABIFLAGS") or "")') || die
+						val=${ESYSROOT-${SYSROOT}${EPREFIX}}/usr/bin/${PYTHON##*/}${flags}-config
 						;;
 					*)
 						die "${impl}: obtaining ${var} not supported"
@@ -954,7 +975,7 @@ python_doheader() {
 	local d PYTHON_INCLUDEDIR=${PYTHON_INCLUDEDIR}
 	[[ ${PYTHON_INCLUDEDIR} ]] || python_export PYTHON_INCLUDEDIR
 
-	d=${PYTHON_INCLUDEDIR#${EPREFIX}}
+	d=${PYTHON_INCLUDEDIR#${ESYSROOT-${SYSROOT}${EPREFIX}}}
 
 	(
 		insopts -m 0644
@@ -964,7 +985,7 @@ python_doheader() {
 }
 
 # @FUNCTION: python_wrapper_setup
-# @USAGE: [<path> [<impl>]]
+# @USAGE: [<impl> [<pyvar> [<path>]]]
 # @DESCRIPTION:
 # Create proper 'python' executable and pkg-config wrappers
 # (if available) in the directory named by <path>. Set up PATH
@@ -973,6 +994,9 @@ python_doheader() {
 # The wrappers will be created for implementation named by <impl>,
 # or for one named by ${EPYTHON} if no <impl> passed.
 #
+# The path to the 'python' executable wrapper is exported to the
+# variable named <pyvar> if given.
+#
 # If the named directory contains a python symlink already, it will
 # be assumed to contain proper wrappers already and only environment
 # setup will be done. If wrapper update is requested, the directory
@@ -980,25 +1004,41 @@ python_doheader() {
 python_wrapper_setup() {
 	debug-print-function ${FUNCNAME} "${@}"
 
-	local impl=${2:-${EPYTHON}}
-	local workdir=${1:-${T}/${impl}}
+	local impl=${1:-${EPYTHON}}
+	local pyvar=${2}
+	local lpyvar=_${pyvar:-PYTHON}
+
+	# Use separate directories for SYSROOT in case we need to execute
+	# Python in the context of the build host by unsetting SYSROOT.
+	local workdir=${3:-${T}/${impl}${SYSROOT:+-sysroot}}
 
 	[[ ${workdir} ]] || die "${FUNCNAME}: no workdir specified."
 	[[ ${impl} ]] || die "${FUNCNAME}: no impl nor EPYTHON specified."
 
+	local EPYTHON
+	python_export "${impl}" EPYTHON
+
+	# We could use BROOT here but using the PATH accommodates
+	# cross-prefix where the PATH is sometimes manipulated to prefer
+	# build tools from the target prefix (i.e. EPREFIX).
+	#
+	# Also make sure we don't pick up an existing wrapper by replacing
+	# instances of ${T} with a bogus location. The workdir can be
+	# overridden but hopefully it will be somewhere under ${T}.
+	local ${lpyvar}=$(PATH=${PATH//${T}//dev/null} type -P "${EPYTHON}" || die "${FUNCNAME}: can't find ${EPYTHON} in PATH")
+
+	local pysysroot=${ESYSROOT-${SYSROOT%/}${EPREFIX}}
+
 	if [[ ! -x ${workdir}/bin/python ]]; then
 		_python_check_dead_variables
 
-		mkdir -p "${workdir}"/{bin,pkgconfig} || die
+		mkdir -p "${workdir}"/{bin,lib,pkgconfig} || die
 
 		# Clean up, in case we were supposed to do a cheap update.
 		rm -f "${workdir}"/bin/python{,2,3}{,-config} || die
 		rm -f "${workdir}"/bin/2to3 || die
 		rm -f "${workdir}"/pkgconfig/python{,2,3}.pc || die
 
-		local EPYTHON PYTHON
-		python_export "${impl}" EPYTHON PYTHON
-
 		local pyver pyother
 		if python_is_python3; then
 			pyver=3
@@ -1012,37 +1052,53 @@ python_wrapper_setup() {
 		# note: we don't use symlinks because python likes to do some
 		# symlink reading magic that breaks stuff
 		# https://bugs.gentoo.org/show_bug.cgi?id=555752
-		cat > "${workdir}/bin/python" <<-_EOF_ || die
-			#!/bin/sh
-			exec "${PYTHON}" "\${@}"
-		_EOF_
-		cp "${workdir}/bin/python" "${workdir}/bin/python${pyver}" || die
-		chmod +x "${workdir}/bin/python" "${workdir}/bin/python${pyver}" || die
+		echo '#!/bin/sh' > "${workdir}/bin/python" || die
 
 		local nonsupp=( "python${pyother}" "python${pyother}-config" )
 
 		# CPython-specific
 		if [[ ${EPYTHON} == python* ]]; then
+			local pysysrootlib=${pysysroot}/usr/$(get_libdir)
+
 			cat > "${workdir}/bin/python-config" <<-_EOF_ || die
 				#!/bin/sh
-				exec "${PYTHON}-config" "\${@}"
+				exec "${pysysroot}/usr/bin/${EPYTHON}-config" "\${@}"
 			_EOF_
 			cp "${workdir}/bin/python-config" \
 				"${workdir}/bin/python${pyver}-config" || die
 			chmod +x "${workdir}/bin/python-config" \
 				"${workdir}/bin/python${pyver}-config" || die
 
+			if [[ -n ${pysysroot} ]]; then
+				# Use host-specific data from SYSROOT. Python 2 looks
+				# for _sysconfigdata while Python 3 uses
+				# _PYTHON_SYSCONFIGDATA_NAME.
+				ln -s "${pysysrootlib}/${EPYTHON}"/_sysconfigdata*.py "${workdir}"/lib/_sysconfigdata.py || die
+
+				# Use distutils/sysconfig from SYSROOT as parts of it
+				# have GENTOO_LIBDIR baked in at Python build time.
+				ln -s "${pysysrootlib}/${EPYTHON}"/{distutils,sysconfig.py} "${workdir}"/lib/ || die
+
+				# Add env vars to python wrapper accordingly.
+				echo "PYTHONPATH=\"${workdir}/lib\${PYTHONPATH:+:}\${PYTHONPATH}\" _PYTHON_SYSCONFIGDATA_NAME=_sysconfigdata \\" \
+					 >> "${workdir}/bin/python" || die
+			fi
+
 			# Python 2.6+.
-			ln -s "${PYTHON/python/2to3-}" "${workdir}"/bin/2to3 || die
+			ln -s "${EPYTHON/python/2to3-}" "${workdir}"/bin/2to3 || die
 
 			# Python 2.7+.
-			ln -s "${EPREFIX}"/usr/$(get_libdir)/pkgconfig/${EPYTHON/n/n-}.pc \
+			ln -s "${pysysrootlib}"/pkgconfig/${EPYTHON/n/n-}.pc \
 				"${workdir}"/pkgconfig/python.pc || die
 			ln -s python.pc "${workdir}"/pkgconfig/python${pyver}.pc || die
 		else
 			nonsupp+=( 2to3 python-config "python${pyver}-config" )
 		fi
 
+		echo "exec \"${!lpyvar}\" \"\${@}\"" >> "${workdir}"/bin/python || die
+		tee "${workdir}"/bin/{python${pyver},"${EPYTHON}"} < "${workdir}"/bin/python >/dev/null || die
+		chmod +x "${workdir}"/bin/{python,python${pyver},"${EPYTHON}"} || die
+
 		local x
 		for x in "${nonsupp[@]}"; do
 			cat >"${workdir}"/bin/${x} <<-_EOF_ || die
@@ -1064,6 +1120,14 @@ python_wrapper_setup() {
 		PKG_CONFIG_PATH=${workdir}/pkgconfig${PKG_CONFIG_PATH:+:${PKG_CONFIG_PATH}}
 	fi
 	export PATH PKG_CONFIG_PATH
+
+	if [[ -n ${pyvar} ]]; then
+		if [[ -n ${pysysroot} ]]; then
+			export -- ${pyvar}=${workdir}/bin/${EPYTHON}
+		else
+			export -- ${pyvar}=${!lpyvar}
+		fi
+	fi
 }
 
 # @FUNCTION: python_is_python3
-- 
2.19.2

[gentoo-dev] [PATCH 05/12] python-utils-r1.eclass: Replace temporary paths in python_fix_shebang

[James Le Cuirot]

This will deal with cases where the shebang is defined using ${PYTHON}
or the location of python in the PATH.

Signed-off-by: James Le Cuirot <chewi@gentoo.org>
---
 eclass/python-utils-r1.eclass | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/eclass/python-utils-r1.eclass b/eclass/python-utils-r1.eclass
index 607af1b52f75..19cfaf2798ab 100644
--- a/eclass/python-utils-r1.eclass
+++ b/eclass/python-utils-r1.eclass
@@ -989,7 +989,8 @@ python_doheader() {
 # @DESCRIPTION:
 # Create proper 'python' executable and pkg-config wrappers
 # (if available) in the directory named by <path>. Set up PATH
-# and PKG_CONFIG_PATH appropriately. <path> defaults to ${T}/${EPYTHON}.
+# and PKG_CONFIG_PATH appropriately. <path> defaults to
+# ${T}/python-wrappers/${EPYTHON}${SYSROOT:+-sysroot}.
 #
 # The wrappers will be created for implementation named by <impl>,
 # or for one named by ${EPYTHON} if no <impl> passed.
@@ -1010,7 +1011,7 @@ python_wrapper_setup() {
 
 	# Use separate directories for SYSROOT in case we need to execute
 	# Python in the context of the build host by unsetting SYSROOT.
-	local workdir=${3:-${T}/${impl}${SYSROOT:+-sysroot}}
+	local workdir=${3:-${T}/python-wrappers/${impl}${SYSROOT:+-sysroot}}
 
 	[[ ${workdir} ]] || die "${FUNCNAME}: no workdir specified."
 	[[ ${impl} ]] || die "${FUNCNAME}: no impl nor EPYTHON specified."
@@ -1243,8 +1244,25 @@ python_fix_shebang() {
 				# Match left-to-right in a loop, to avoid matching random
 				# repetitions like 'python2.7 python2'.
 				for i in $(seq 0 $((${#split_shebang[@]} - 1))); do
+					# Fix the leading path if it points to a wrapper
+					# script created by this eclass or points to
+					# ${BROOT} rather than ${EPREFIX}. We generally only
+					# touch the tail end of the string as this is safer
+					# but if a path starts with ${T} or ${BROOT}, where
+					# the latter differs from ${EPREFIX}, then it
+					# definitely needs fixing. We preserve the tail for
+					# further processing in the case block below.
+					if [[ ${split_shebang[${i}]} == ${T}/python-wrappers/*/bin/* || ( ${split_shebang[${i}]} == ${BROOT}/usr/bin/* && ${BROOT} != ${EPREFIX} ) ]]; then
+						split_shebang[${i}]=${EPREFIX}/usr/bin/${split_shebang[${i}]##*/}
+						fix=1
+					fi
+
 					case "/${split_shebang[${i}]}" in
 						*/${EPYTHON})
+							# EPYTHON matched but the leading path may
+							# still need fixing.
+							[[ ${fix} ]] && break
+
 							debug-print "${FUNCNAME}: in file ${f#${D%/}}"
 							debug-print "${FUNCNAME}: shebang matches EPYTHON: ${shebang}"
 
-- 
2.19.2

[gentoo-dev] [PATCH 06/12] python-utils-r1.eclass: Don't die in python_fix_shebang if all fixed

[James Le Cuirot]

Shebangs may need fixing on prefix systems or when cross-building but
not at other times.

Signed-off-by: James Le Cuirot <chewi@gentoo.org>
---
 eclass/python-utils-r1.eclass | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/eclass/python-utils-r1.eclass b/eclass/python-utils-r1.eclass
index 19cfaf2798ab..91e457f3cf14 100644
--- a/eclass/python-utils-r1.eclass
+++ b/eclass/python-utils-r1.eclass
@@ -1328,16 +1328,12 @@ python_fix_shebang() {
 			fi
 		done < <(find -H "${path}" -type f -print0 || die)
 
-		if [[ ! ${any_fixed} ]]; then
+		if [[ ! ${any_fixed} && ! ${any_correct} ]]; then
 			local cmd=eerror
 			[[ ${EAPI:-0} == [012345] ]] && cmd=eqawarn
 
 			"${cmd}" "QA warning: ${FUNCNAME}, ${path#${D%/}} did not match any fixable files."
-			if [[ ${any_correct} ]]; then
-				"${cmd}" "All files have ${EPYTHON} shebang already."
-			else
-				"${cmd}" "There are no Python files in specified directory."
-			fi
+			"${cmd}" "There are no Python files in specified directory."
 
 			[[ ${cmd} == eerror ]] && die "${FUNCNAME} did not match any fixable files (QA warning fatal in EAPI ${EAPI})"
 		fi
-- 
2.19.2

[gentoo-dev] [PATCH 07/12] python-utils-r1.eclass: Add special wrapper handling for Python itself

[James Le Cuirot]

dev-lang/python sometimes used to install the epython module into the
wrong libdir (e.g. lib vs lib64). The earlier eclass changes actually
break the build entirely as sysconfigdata is now sourced from within
SYSROOT, where it may not even be installed yet. This therefore needs
to be handled as a special case where sysconfigdata is sourced from
within ED instead.

Signed-off-by: James Le Cuirot <chewi@gentoo.org>
---
 eclass/python-utils-r1.eclass | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/eclass/python-utils-r1.eclass b/eclass/python-utils-r1.eclass
index 91e457f3cf14..17de9ca9c44f 100644
--- a/eclass/python-utils-r1.eclass
+++ b/eclass/python-utils-r1.eclass
@@ -1028,7 +1028,11 @@ python_wrapper_setup() {
 	# overridden but hopefully it will be somewhere under ${T}.
 	local ${lpyvar}=$(PATH=${PATH//${T}//dev/null} type -P "${EPYTHON}" || die "${FUNCNAME}: can't find ${EPYTHON} in PATH")
 
-	local pysysroot=${ESYSROOT-${SYSROOT%/}${EPREFIX}}
+	if [[ ${CATEGORY}/${PN} == dev-lang/python ]]; then
+		local pysysroot=${ED%/}
+	else
+		local pysysroot=${ESYSROOT-${SYSROOT%/}${EPREFIX}}
+	fi
 
 	if [[ ! -x ${workdir}/bin/python ]]; then
 		_python_check_dead_variables
-- 
2.19.2

[gentoo-dev] [PATCH 08/12] python-utils-r1.eclass: Adjust wrappers for Python 3.7 libdir change

[James Le Cuirot]

We assumed Python to be located at /usr/$(get_libdir) in recent
changes to this eclass but this has changed from Python 3.7. We now
follow upstream, installing most files to /usr/lib, with only certain
files, such as the pkg-config files, going into /usr/$(get_libdir).

Signed-off-by: James Le Cuirot <chewi@gentoo.org>
---
 eclass/python-utils-r1.eclass | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/eclass/python-utils-r1.eclass b/eclass/python-utils-r1.eclass
index 17de9ca9c44f..5e54cc618212 100644
--- a/eclass/python-utils-r1.eclass
+++ b/eclass/python-utils-r1.eclass
@@ -1063,7 +1063,14 @@ python_wrapper_setup() {
 
 		# CPython-specific
 		if [[ ${EPYTHON} == python* ]]; then
-			local pysysrootlib=${pysysroot}/usr/$(get_libdir)
+			local pysysrootlib=${pysysroot}/usr/
+
+			case "${EPYTHON}" in
+				python2.*|python3.[0-6])
+					pysysrootlib+=$(get_libdir) ;;
+				*)
+					pysysrootlib+=lib ;;
+			esac
 
 			cat > "${workdir}/bin/python-config" <<-_EOF_ || die
 				#!/bin/sh
@@ -1093,7 +1100,7 @@ python_wrapper_setup() {
 			ln -s "${EPYTHON/python/2to3-}" "${workdir}"/bin/2to3 || die
 
 			# Python 2.7+.
-			ln -s "${pysysrootlib}"/pkgconfig/${EPYTHON/n/n-}.pc \
+			ln -s "${pysysroot}"/usr/$(get_libdir)/pkgconfig/${EPYTHON/n/n-}.pc \
 				"${workdir}"/pkgconfig/python.pc || die
 			ln -s python.pc "${workdir}"/pkgconfig/python${pyver}.pc || die
 		else
-- 
2.19.2

[gentoo-dev] [PATCH 09/12] python-any-r1.eclass: Export PYTHON after checking whether installed

[James Le Cuirot]

This previously happened before checking but now exporting PYTHON
involves creating the wrappers, which requires the requested Python to
be installed.

We could also drop all the calls to python_wrapper_setup as this is
now already done when exporting PYTHON but it doesn't hurt to be
explicit.

Signed-off-by: James Le Cuirot <chewi@gentoo.org>
---
 eclass/python-any-r1.eclass | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/eclass/python-any-r1.eclass b/eclass/python-any-r1.eclass
index 7a91507a600f..7c5ae87872eb 100644
--- a/eclass/python-any-r1.eclass
+++ b/eclass/python-any-r1.eclass
@@ -1,4 +1,4 @@
-# Copyright 1999-2018 Gentoo Foundation
+# Copyright 1999-2019 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
 
 # @ECLASS: python-any-r1.eclass
@@ -327,8 +327,8 @@ python_setup() {
 	# fallback to best installed impl.
 	# (reverse iteration over _PYTHON_SUPPORTED_IMPLS)
 	for (( i = ${#_PYTHON_SUPPORTED_IMPLS[@]} - 1; i >= 0; i-- )); do
-		python_export "${_PYTHON_SUPPORTED_IMPLS[i]}" EPYTHON PYTHON
-		if _python_EPYTHON_supported "${EPYTHON}"; then
+		if _python_EPYTHON_supported "${_PYTHON_SUPPORTED_IMPLS[i]}"; then
+			python_export "${_PYTHON_SUPPORTED_IMPLS[i]}" EPYTHON PYTHON
 			python_wrapper_setup
 			return
 		fi
-- 
2.19.2

[gentoo-dev] [PATCH 10/12] python-any-r1.eclass: Create wrappers against build system's config

[James Le Cuirot]

python-any-r1 is not to be used in packages requiring Python at
runtime. It may not even be installed.

Signed-off-by: James Le Cuirot <chewi@gentoo.org>
---
 eclass/python-any-r1.eclass | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/eclass/python-any-r1.eclass b/eclass/python-any-r1.eclass
index 7c5ae87872eb..c46ca0e2444b 100644
--- a/eclass/python-any-r1.eclass
+++ b/eclass/python-any-r1.eclass
@@ -283,6 +283,11 @@ _python_EPYTHON_supported() {
 python_setup() {
 	debug-print-function ${FUNCNAME} "${@}"
 
+	# Ensure Python wrappers are created against the build system's
+	# configuration as python-any-r1 is not to be used in packages
+	# requiring Python at runtime. It may not even be installed.
+	local SYSROOT ESYSROOT=${BROOT-${EPREFIX}}
+
 	# support developer override
 	if [[ ${PYTHON_COMPAT_OVERRIDE} ]]; then
 		local impls=( ${PYTHON_COMPAT_OVERRIDE} )
-- 
2.19.2

[gentoo-dev] [PATCH 11/12] distutils-r1.eclass: Fix cross-compiling

[James Le Cuirot]

Following the changes in python-utils-r1.eclass, Python is now needed
in both BDEPEND and DEPEND.

As distutils does not call our eclass helpers, we need to pass the
correct include and library directories in setup.cfg. Unfortunately it
is still hardcoded to add -I/usr/include/pythonX.X and -L/usr/lib but
these appear after the SYSROOT paths so the build succeeds anyway. If
a missing header or library were to cause it to fall back to the build
host paths then it would most likely fail but it does not matter as it
would have failed either way.

Closes: https://bugs.gentoo.org/648652
Signed-off-by: James Le Cuirot <chewi@gentoo.org>
---
 eclass/distutils-r1.eclass | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/eclass/distutils-r1.eclass b/eclass/distutils-r1.eclass
index fa7a3ab5c12b..ff7792f11a87 100644
--- a/eclass/distutils-r1.eclass
+++ b/eclass/distutils-r1.eclass
@@ -1,4 +1,4 @@
-# Copyright 1999-2018 Gentoo Foundation
+# Copyright 1999-2019 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
 
 # @ECLASS: distutils-r1.eclass
@@ -99,11 +99,8 @@ if [[ ! ${_DISTUTILS_R1} ]]; then
 
 if [[ ! ${DISTUTILS_OPTIONAL} ]]; then
 	RDEPEND=${PYTHON_DEPS}
-	if [[ ${EAPI} != [56] ]]; then
-		BDEPEND=${PYTHON_DEPS}
-	else
-		DEPEND=${PYTHON_DEPS}
-	fi
+	DEPEND=${PYTHON_DEPS}
+	[[ ${EAPI} != [56] ]] && BDEPEND=${PYTHON_DEPS}
 	REQUIRED_USE=${PYTHON_REQUIRED_USE}
 fi
 
@@ -407,8 +404,21 @@ _distutils-r1_create_setup_cfg() {
 		# setuptools like to create .egg files for install --home.
 		[bdist_egg]
 		dist-dir = ${BUILD_DIR}/dist
+
+		# this is needed when cross-compiling
+		[build_ext]
 	_EOF_
 
+	if [[ ${EPYTHON} != jython* ]]; then
+		echo "include-dirs = $(python_get_includedir)" \
+			>> "${HOME}"/.pydistutils.cfg || die
+	fi
+
+	if [[ ${EPYTHON} == python* ]]; then
+		echo "library-dirs = $(dirname $(python_get_library_path))" \
+			>> "${HOME}"/.pydistutils.cfg || die
+	fi
+
 	# we can't refer to ${D} before src_install()
 	if [[ ${EBUILD_PHASE} == install ]]; then
 		cat >> "${HOME}"/.pydistutils.cfg <<-_EOF_ || die
-- 
2.19.2

[gentoo-dev] [PATCH 12/12] distutils-r1.eclass: Make distutils-r1_create_setup_cfg external

[James Le Cuirot]

It is useful on its own when the build system calls setup.py for us,
from a Makefile, for example. ${BUILD_DIR} is unlikely to be set in
this instance so it falls back to ${PWD}.

Signed-off-by: James Le Cuirot <chewi@gentoo.org>
---
 eclass/distutils-r1.eclass | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/eclass/distutils-r1.eclass b/eclass/distutils-r1.eclass
index ff7792f11a87..2e969e205b16 100644
--- a/eclass/distutils-r1.eclass
+++ b/eclass/distutils-r1.eclass
@@ -250,7 +250,7 @@ esetup.py() {
 	local die_args=()
 	[[ ${EAPI} != [45] ]] && die_args+=( -n )
 
-	[[ ${BUILD_DIR} ]] && _distutils-r1_create_setup_cfg
+	[[ ${BUILD_DIR} ]] && distutils-r1_create_setup_cfg
 
 	set -- "${EPYTHON:-python}" setup.py "${mydistutilsargs[@]}" "${@}"
 
@@ -288,7 +288,7 @@ distutils_install_for_testing() {
 	#    so we need to set it properly and mkdir them,
 	# 4) it runs a bunch of commands which write random files to cwd,
 	#    in order to avoid that, we add the necessary path overrides
-	#    in _distutils-r1_create_setup_cfg.
+	#    in distutils-r1_create_setup_cfg.
 
 	TEST_DIR=${BUILD_DIR}/test
 	local bindir=${TEST_DIR}/scripts
@@ -377,15 +377,16 @@ distutils-r1_python_configure() {
 	[[ ${EAPI} == [45] ]] || die "${FUNCNAME} is banned in EAPI 6 (it was a no-op)"
 }
 
-# @FUNCTION: _distutils-r1_create_setup_cfg
-# @INTERNAL
+# @FUNCTION: distutils-r1_create_setup_cfg
 # @DESCRIPTION:
 # Create implementation-specific configuration file for distutils,
 # setting proper build-dir (and install-dir) paths.
-_distutils-r1_create_setup_cfg() {
+distutils-r1_create_setup_cfg() {
+	local build_base=${BUILD_DIR:-${PWD}}
+
 	cat > "${HOME}"/.pydistutils.cfg <<-_EOF_ || die
 		[build]
-		build-base = ${BUILD_DIR}
+		build-base = ${build_base}
 
 		# using a single directory for them helps us export
 		# ${PYTHONPATH} and ebuilds find the sources independently
@@ -403,7 +404,7 @@ _distutils-r1_create_setup_cfg() {
 		# this is needed by distutils_install_for_testing since
 		# setuptools like to create .egg files for install --home.
 		[bdist_egg]
-		dist-dir = ${BUILD_DIR}/dist
+		dist-dir = ${build_base}/dist
 
 		# this is needed when cross-compiling
 		[build_ext]
-- 
2.19.2

Re: [gentoo-dev] [PATCH] Eclass changes for cross-compiling Python modules

[Benda Xu]

Hi James,

James Le Cuirot <chewi@gentoo.org> writes:

> Once this is in place, I can finish my long-awaited revamp of my
> cross-boss project that will allow you to cross-compile @system from
> scratch with very little effort.

I haven't gone through the patches yet.  But I want to say thank you!
The cross-boss project has been on my list for ages and you've made it
real.  Your efforts on EAPI-7 and cross python modules are much
appreciated.

Yours,
Benda

Re: [gentoo-dev] [PATCH 02/12] python-utils-r1.eclass: New python_fix_shebang approach

[Michał Górny]

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

On Thu, 2019-01-03 at 21:39 +0000, James Le Cuirot wrote:
> The previous approach would erroneously match foopython. The new
> approach requires the match to start the string or be preceeded by a
> slash, the only two cases we actually want. It does this with slightly
> less code and allows the replacement of whole path strings that would
> be problematic when passed to sed. This will be needed when
> cross-compiling is addressed.
> 
> Signed-off-by: James Le Cuirot <chewi@gentoo.org>
> ---
>  eclass/python-utils-r1.eclass | 78 ++++++++++++++---------------------
>  1 file changed, 31 insertions(+), 47 deletions(-)
> 
> diff --git a/eclass/python-utils-r1.eclass b/eclass/python-utils-r1.eclass
> index da76a755fb34..1eca0764a202 100644
> --- a/eclass/python-utils-r1.eclass
> +++ b/eclass/python-utils-r1.eclass
> @@ -1165,8 +1165,7 @@ python_fix_shebang() {
>  		[[ -d ${path} ]] && is_recursive=1
>  
>  		while IFS= read -r -d '' f; do
> -			local shebang i
> -			local error= from=
> +			local shebang i= error= fix=
>  
>  			# note: we can't ||die here since read will fail if file
>  			# has no newline characters
> @@ -1175,65 +1174,56 @@ python_fix_shebang() {
>  			# First, check if it's shebang at all...
>  			if [[ ${shebang} == '#!'* ]]; then
>  				local split_shebang=()
> -				read -r -a split_shebang <<<${shebang} || die
> +				read -r -a split_shebang <<<${shebang#\#\!} || die

Does '!' really need to be escaped?

>  
>  				# Match left-to-right in a loop, to avoid matching random
>  				# repetitions like 'python2.7 python2'.
> -				for i in "${split_shebang[@]}"; do
> -					case "${i}" in
> -						*"${EPYTHON}")
> +				for i in $(seq 0 $((${#split_shebang[@]} - 1))); do

for i in "${!split_shebang[@]}"; do

> +					case "/${split_shebang[${i}]}" in

case "/${split_shebang[i]}" in

Also below.

> +						*/${EPYTHON})
>  							debug-print "${FUNCNAME}: in file ${f#${D%/}}"
>  							debug-print "${FUNCNAME}: shebang matches EPYTHON: ${shebang}"
>  
>  							# Nothing to do, move along.
>  							any_correct=1
> -							from=${EPYTHON}
> +							continue 2
> +							;;
> +						*/python)
> +							fix=1
>  							break
>  							;;
> -						*python|*python[23])
> -							debug-print "${FUNCNAME}: in file ${f#${D%/}}"
> -							debug-print "${FUNCNAME}: rewriting shebang: ${shebang}"
> -
> -							if [[ ${i} == *python2 ]]; then
> -								from=python2
> -								if [[ ! ${force} ]]; then
> -									python_is_python3 "${EPYTHON}" && error=1
> -								fi
> -							elif [[ ${i} == *python3 ]]; then
> -								from=python3
> -								if [[ ! ${force} ]]; then
> -									python_is_python3 "${EPYTHON}" || error=1
> -								fi
> -							else
> -								from=python
> +						*/python2)
> +							if [[ ! ${force} ]]; then
> +								python_is_python3 "${EPYTHON}" && error=1
>  							fi
> +							fix=1
>  							break
>  							;;
> -						*python[23].[0123456789]|*pypy|*pypy3|*jython[23].[0123456789])
> -							# Explicit mismatch.
> +						*/python3)
>  							if [[ ! ${force} ]]; then
> -								error=1
> -							else
> -								case "${i}" in
> -									*python[23].[0123456789])
> -										from="python[23].[0123456789]";;
> -									*pypy)
> -										from="pypy";;
> -									*pypy3)
> -										from="pypy3";;
> -									*jython[23].[0123456789])
> -										from="jython[23].[0123456789]";;
> -									*)
> -										die "${FUNCNAME}: internal error in 2nd pattern match";;
> -								esac
> +								python_is_python3 "${EPYTHON}" || error=1
>  							fi
> +							fix=1
> +							break
> +							;;
> +						*/python[2-3].[0-9]|*/pypy|*/pypy3|*/jython[2-3].[0-9])
> +							# Explicit mismatch.
> +							[[ ! ${force} ]] && error=1
> +							fix=1
>  							break
>  							;;
>  					esac
>  				done
>  			fi
>  
> -			if [[ ! ${error} && ! ${from} ]]; then
> +			if [[ ${fix} ]]; then

Doesn't this mean errors from above code will be ignored?  Diff makes it
really hard to follow the logic here but I'm pretty sure you set both
error=1 fix=1, then check for fix first.

> +				debug-print "${FUNCNAME}: in file ${f#${D%/}}"
> +				debug-print "${FUNCNAME}: rewriting shebang: ${shebang}"
> +
> +				split_shebang[${i}]=/${split_shebang[${i}]}

A comment above this hack would be helpful.

> +				split_shebang[${i}]=${split_shebang[${i}]%/*}
> +				split_shebang[${i}]=${split_shebang[${i}]#/}${split_shebang[${i}]:+/}${EPYTHON}
> +			elif [[ ! ${error} ]]; then
>  				# Non-Python shebang. Allowed in recursive mode,
>  				# disallowed when specifying file explicitly.
>  				[[ ${is_recursive} ]] && continue
> @@ -1245,13 +1235,7 @@ python_fix_shebang() {
>  			fi
>  
>  			if [[ ! ${error} ]]; then
> -				# We either want to match ${from} followed by space
> -				# or at end-of-string.
> -				if [[ ${shebang} == *${from}" "* ]]; then
> -					sed -i -e "1s:${from} :${EPYTHON} :" "${f}" || die
> -				else
> -					sed -i -e "1s:${from}$:${EPYTHON}:" "${f}" || die
> -				fi
> +				sed -i -e "1c#\!${split_shebang[*]}" "${f}" || die
>  				any_fixed=1
>  			else
>  				eerror "The file has incompatible shebang:"

I'll get to other patches later.

-- 
Best regards,
Michał Górny

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 963 bytes --]

Re: [gentoo-dev] [PATCH 04/12] python-utils-r1.eclass: Use wrapper scripts to fix cross-compiling

[Michał Górny]

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

On Thu, 2019-01-03 at 21:39 +0000, James Le Cuirot wrote:
> Python has little concept of cross-compiling but it turns out that
> making it work isn't so hard after all.
> 
> Platform-specific details are held within _sysconfigdata.py,
> sysconfig.py, and various distutils files. If we simply symlink these
> from SYSROOT into an empty directory and add that directory to
> PYTHONPATH then we can utilise the build host's Python with the target
> host's settings.
> 
> The pkg-config files were already being symlinked in a similar manner
> but now we source them from within SYSROOT.
> 
> In order for PYTHONPATH to be respected, Python must be executed via
> the wrapper, which was not the case before. We previously relied
> solely on the PATH but now PYTHON must point to the wrapper rather
> than the usual location under /usr/bin. However, we only do this when
> SYSROOT or EPREFIX are effectively set to avoid unnecessary
> complexity. This has required some rejigging in the way that PYTHON is
> set but it should remain compatible with existing packages.
> 
> The python_wrapper_setup function that handles all this has had its
> arguments reordered because no one ever uses the path/workdir
> argument, which makes specifying other arguments tedious.

This really belongs in a separate patch.

> 
> Some packages rely on python-config but luckily this is just a shell
> script so it can be executed from within SYSROOT. This is bending the
> rules of PMS slightly but Python leaves us with little choice. I also
> wrote those rules so I'm allowed to bend them. ;)
> 
> PYTHON_INCLUDEDIR, PYTHON_LIBPATH, and their associated functions are
> generally used during src_configure or src_compile and, as such, they
> now need to have SYSROOT prepended.
> 
> python_doheader uses PYTHON_INCLUDEDIR to install new headers and
> therefore needs the value without SYSROOT. It was already stripping
> EPREFIX before so now it simply strips SYSROOT as well. Similar
> instances of this can do likewise or call the functions with SYSROOT
> unset to achieve the same effect.
> 
> To make all this work, we are assuming that CPython is located at
> /usr/$(get_libdir)/${EPYTHON}, which is admittedly a little circular
> when we are querying Python for the right paths. I feel the reason
> that python_export was rewritten to query these dynamically was less
> because someone might install Python to some weird location and more
> to avoid special handling for each of the different
> implementations. And what of those other implementations?

This is a wrong assumption.  CPython 3.7 is in /usr/lib/python3.7.

> 
> Being Java-based, Jython is installed under the platform-neutral
> /usr/share and presumably has few other platform-specific
> aspects. Enabling native extensions appears non-trivial and none of
> our module packages have enabled support for it anyway.
> 
> I think PyPy could potentially support cross-compiling but it
> hardcodes the native extension filename suffix within its own binaries
> with no way to override it. Perhaps we could patch this in somehow but
> I'm afraid I don't care enough.
> 
> Together with the following changes to distutils-r1.eclass, I have now
> been able to cross-compile a large number of packages with native
> Python extensions, most with no changes at all, and the rest with only
> minor fixes.
> 
> Closes: https://bugs.gentoo.org/503874
> Bug: https://bugs.gentoo.org/648652
> Signed-off-by: James Le Cuirot <chewi@gentoo.org>
> ---
>  eclass/python-utils-r1.eclass | 120 ++++++++++++++++++++++++++--------
>  1 file changed, 92 insertions(+), 28 deletions(-)
> 
> diff --git a/eclass/python-utils-r1.eclass b/eclass/python-utils-r1.eclass
> index 1a549f49f3f2..607af1b52f75 100644
> --- a/eclass/python-utils-r1.eclass
> +++ b/eclass/python-utils-r1.eclass
> @@ -211,9 +211,15 @@ _python_impl_matches() {
>  #
>  # distutils-r1: Set within any of the python sub-phase functions.
>  #
> -# Example value:
> +# If SYSROOT or EPREFIX are effectively set then this will point to an
> +# automatically generated wrapper rather than the usual path under
> +# /usr/bin in order to accommodate cross-compiling. We could do this all
> +# the time but it would add unnecessary complexity.
> +#
> +# Example values:
>  # @CODE
>  # /usr/bin/python2.7
> +# /var/tmp/portage/dev-python/pyblake2-1.2.3/temp/python2.7/bin/python2.7
>  # @CODE
>  
>  # @ECLASS-VARIABLE: EPYTHON
> @@ -256,6 +262,10 @@ _python_impl_matches() {
>  # Set and exported on request using python_export().
>  # Requires a proper build-time dependency on the Python implementation.
>  #
> +# This is prepended with SYSROOT in order to accommodate
> +# cross-compiling. You may need to strip SYSROOT and EPREFIX if using it
> +# to install new headers.
> +#
>  # Example value:
>  # @CODE
>  # /usr/include/python2.7
> @@ -270,6 +280,9 @@ _python_impl_matches() {
>  # Valid only for CPython. Requires a proper build-time dependency
>  # on the Python implementation.
>  #
> +# This is prepended with SYSROOT in order to accommodate
> +# cross-compiling.
> +#
>  # Example value:
>  # @CODE
>  # /usr/lib64/libpython2.7.so
> @@ -314,6 +327,10 @@ _python_impl_matches() {
>  # Valid only for CPython. Requires a proper build-time dependency
>  # on the Python implementation and on pkg-config.
>  #
> +# This is prepended with SYSROOT in order to accommodate
> +# cross-compiling. You generally should not execute files within SYSROOT
> +# but python-config is always a shell script.
> +#
>  # Example value:
>  # @CODE
>  # /usr/bin/python2.7-config
> @@ -380,6 +397,10 @@ python_export() {
>  	esac
>  	debug-print "${FUNCNAME}: implementation: ${impl}"
>  
> +	# Many variables below need a PYTHON variable but we should not
> +	# export it unless explicitly requested so use _PYTHON instead.
> +	local _PYTHON
> +
>  	for var; do
>  		case "${var}" in
>  			EPYTHON)
> @@ -387,21 +408,21 @@ python_export() {
>  				debug-print "${FUNCNAME}: EPYTHON = ${EPYTHON}"
>  				;;
>  			PYTHON)
> -				export PYTHON=${EPREFIX}/usr/bin/${impl}
> +				python_wrapper_setup ${impl} PYTHON
>  				debug-print "${FUNCNAME}: PYTHON = ${PYTHON}"
>  				;;
>  			PYTHON_SITEDIR)
> -				[[ -n ${PYTHON} ]] || die "PYTHON needs to be set for ${var} to be exported, or requested before it"
> +				python_wrapper_setup ${impl} _PYTHON
>  				# sysconfig can't be used because:
>  				# 1) pypy doesn't give site-packages but stdlib
>  				# 2) jython gives paths with wrong case
> -				PYTHON_SITEDIR=$("${PYTHON}" -c 'import distutils.sysconfig; print(distutils.sysconfig.get_python_lib())') || die
> +				PYTHON_SITEDIR=$("${_PYTHON}" -c 'import distutils.sysconfig; print(distutils.sysconfig.get_python_lib())') || die
>  				export PYTHON_SITEDIR
>  				debug-print "${FUNCNAME}: PYTHON_SITEDIR = ${PYTHON_SITEDIR}"
>  				;;
>  			PYTHON_INCLUDEDIR)
> -				[[ -n ${PYTHON} ]] || die "PYTHON needs to be set for ${var} to be exported, or requested before it"
> -				PYTHON_INCLUDEDIR=$("${PYTHON}" -c 'import distutils.sysconfig; print(distutils.sysconfig.get_python_inc())') || die
> +				python_wrapper_setup ${impl} _PYTHON
> +				PYTHON_INCLUDEDIR=${SYSROOT}$("${_PYTHON}" -c 'import distutils.sysconfig; print(distutils.sysconfig.get_python_inc())') || die
>  				export PYTHON_INCLUDEDIR
>  				debug-print "${FUNCNAME}: PYTHON_INCLUDEDIR = ${PYTHON_INCLUDEDIR}"
>  
> @@ -411,8 +432,8 @@ python_export() {
>  				fi
>  				;;
>  			PYTHON_LIBPATH)
> -				[[ -n ${PYTHON} ]] || die "PYTHON needs to be set for ${var} to be exported, or requested before it"
> -				PYTHON_LIBPATH=$("${PYTHON}" -c 'import os.path, sysconfig; print(os.path.join(sysconfig.get_config_var("LIBDIR"), sysconfig.get_config_var("LDLIBRARY")) if sysconfig.get_config_var("LDLIBRARY") else "")') || die
> +				python_wrapper_setup ${impl} _PYTHON
> +				PYTHON_LIBPATH=${SYSROOT}$("${_PYTHON}" -c 'import os.path, sysconfig; print(os.path.join(sysconfig.get_config_var("LIBDIR"), sysconfig.get_config_var("LDLIBRARY")) if sysconfig.get_config_var("LDLIBRARY") else "")') || die
>  				export PYTHON_LIBPATH
>  				debug-print "${FUNCNAME}: PYTHON_LIBPATH = ${PYTHON_LIBPATH}"
>  
> @@ -457,9 +478,9 @@ python_export() {
>  
>  				case "${impl}" in
>  					python*)
> -						[[ -n ${PYTHON} ]] || die "PYTHON needs to be set for ${var} to be exported, or requested before it"
> -						flags=$("${PYTHON}" -c 'import sysconfig; print(sysconfig.get_config_var("ABIFLAGS") or "")') || die
> -						val=${PYTHON}${flags}-config
> +						python_wrapper_setup ${impl} _PYTHON
> +						flags=$("${_PYTHON}" -c 'import sysconfig; print(sysconfig.get_config_var("ABIFLAGS") or "")') || die
> +						val=${ESYSROOT-${SYSROOT}${EPREFIX}}/usr/bin/${PYTHON##*/}${flags}-config
>  						;;
>  					*)
>  						die "${impl}: obtaining ${var} not supported"
> @@ -954,7 +975,7 @@ python_doheader() {
>  	local d PYTHON_INCLUDEDIR=${PYTHON_INCLUDEDIR}
>  	[[ ${PYTHON_INCLUDEDIR} ]] || python_export PYTHON_INCLUDEDIR
>  
> -	d=${PYTHON_INCLUDEDIR#${EPREFIX}}
> +	d=${PYTHON_INCLUDEDIR#${ESYSROOT-${SYSROOT}${EPREFIX}}}
>  
>  	(
>  		insopts -m 0644
> @@ -964,7 +985,7 @@ python_doheader() {
>  }
>  
>  # @FUNCTION: python_wrapper_setup
> -# @USAGE: [<path> [<impl>]]
> +# @USAGE: [<impl> [<pyvar> [<path>]]]
>  # @DESCRIPTION:
>  # Create proper 'python' executable and pkg-config wrappers
>  # (if available) in the directory named by <path>. Set up PATH
> @@ -973,6 +994,9 @@ python_doheader() {
>  # The wrappers will be created for implementation named by <impl>,
>  # or for one named by ${EPYTHON} if no <impl> passed.
>  #
> +# The path to the 'python' executable wrapper is exported to the
> +# variable named <pyvar> if given.

Why do you need that in the first place?  The path should be rather
predictable, shouldn't it?

> +#
>  # If the named directory contains a python symlink already, it will
>  # be assumed to contain proper wrappers already and only environment
>  # setup will be done. If wrapper update is requested, the directory
> @@ -980,25 +1004,41 @@ python_doheader() {
>  python_wrapper_setup() {
>  	debug-print-function ${FUNCNAME} "${@}"
>  
> -	local impl=${2:-${EPYTHON}}
> -	local workdir=${1:-${T}/${impl}}
> +	local impl=${1:-${EPYTHON}}
> +	local pyvar=${2}
> +	local lpyvar=_${pyvar:-PYTHON}
> +
> +	# Use separate directories for SYSROOT in case we need to execute
> +	# Python in the context of the build host by unsetting SYSROOT.
> +	local workdir=${3:-${T}/${impl}${SYSROOT:+-sysroot}}
>  
>  	[[ ${workdir} ]] || die "${FUNCNAME}: no workdir specified."
>  	[[ ${impl} ]] || die "${FUNCNAME}: no impl nor EPYTHON specified."
>  
> +	local EPYTHON
> +	python_export "${impl}" EPYTHON

I'm getting dizzy because of the amount of circular dependencies between
the two functions you've introduced.  This is going to make it trivial
to deadlock it when changing code later.

> +
> +	# We could use BROOT here but using the PATH accommodates
> +	# cross-prefix where the PATH is sometimes manipulated to prefer
> +	# build tools from the target prefix (i.e. EPREFIX).
> +	#
> +	# Also make sure we don't pick up an existing wrapper by replacing
> +	# instances of ${T} with a bogus location. The workdir can be
> +	# overridden but hopefully it will be somewhere under ${T}.
> +	local ${lpyvar}=$(PATH=${PATH//${T}//dev/null} type -P "${EPYTHON}" || die "${FUNCNAME}: can't find ${EPYTHON} in PATH")

This PATH substitution hack is horrible.

> +
> +	local pysysroot=${ESYSROOT-${SYSROOT%/}${EPREFIX}}
> +
>  	if [[ ! -x ${workdir}/bin/python ]]; then
>  		_python_check_dead_variables
>  
> -		mkdir -p "${workdir}"/{bin,pkgconfig} || die
> +		mkdir -p "${workdir}"/{bin,lib,pkgconfig} || die
>  
>  		# Clean up, in case we were supposed to do a cheap update.
>  		rm -f "${workdir}"/bin/python{,2,3}{,-config} || die
>  		rm -f "${workdir}"/bin/2to3 || die
>  		rm -f "${workdir}"/pkgconfig/python{,2,3}.pc || die
>  
> -		local EPYTHON PYTHON
> -		python_export "${impl}" EPYTHON PYTHON
> -
>  		local pyver pyother
>  		if python_is_python3; then
>  			pyver=3
> @@ -1012,37 +1052,53 @@ python_wrapper_setup() {
>  		# note: we don't use symlinks because python likes to do some
>  		# symlink reading magic that breaks stuff
>  		# https://bugs.gentoo.org/show_bug.cgi?id=555752
> -		cat > "${workdir}/bin/python" <<-_EOF_ || die
> -			#!/bin/sh
> -			exec "${PYTHON}" "\${@}"
> -		_EOF_
> -		cp "${workdir}/bin/python" "${workdir}/bin/python${pyver}" || die
> -		chmod +x "${workdir}/bin/python" "${workdir}/bin/python${pyver}" || die
> +		echo '#!/bin/sh' > "${workdir}/bin/python" || die 
>  		local nonsupp=( "python${pyother}" "python${pyother}-config" )
>  
>  		# CPython-specific
>  		if [[ ${EPYTHON} == python* ]]; then
> +			local pysysrootlib=${pysysroot}/usr/$(get_libdir)
> +
>  			cat > "${workdir}/bin/python-config" <<-_EOF_ || die
>  				#!/bin/sh
> -				exec "${PYTHON}-config" "\${@}"
> +				exec "${pysysroot}/usr/bin/${EPYTHON}-config" "\${@}"
>  			_EOF_
>  			cp "${workdir}/bin/python-config" \
>  				"${workdir}/bin/python${pyver}-config" || die
>  			chmod +x "${workdir}/bin/python-config" \
>  				"${workdir}/bin/python${pyver}-config" || die
>  
> +			if [[ -n ${pysysroot} ]]; then
> +				# Use host-specific data from SYSROOT. Python 2 looks
> +				# for _sysconfigdata while Python 3 uses
> +				# _PYTHON_SYSCONFIGDATA_NAME.
> +				ln -s "${pysysrootlib}/${EPYTHON}"/_sysconfigdata*.py "${workdir}"/lib/_sysconfigdata.py || die
> +
> +				# Use distutils/sysconfig from SYSROOT as parts of it
> +				# have GENTOO_LIBDIR baked in at Python build time.
> +				ln -s "${pysysrootlib}/${EPYTHON}"/{distutils,sysconfig.py} "${workdir}"/lib/ || die
> +
> +				# Add env vars to python wrapper accordingly.
> +				echo "PYTHONPATH=\"${workdir}/lib\${PYTHONPATH:+:}\${PYTHONPATH}\" _PYTHON_SYSCONFIGDATA_NAME=_sysconfigdata \\" \
> +					 >> "${workdir}/bin/python" || die

Use explicit 'export ...' multi-line thing for readability.

> +			fi
> +
>  			# Python 2.6+.
> -			ln -s "${PYTHON/python/2to3-}" "${workdir}"/bin/2to3 || die
> +			ln -s "${EPYTHON/python/2to3-}" "${workdir}"/bin/2to3 || die
>  
>  			# Python 2.7+.
> -			ln -s "${EPREFIX}"/usr/$(get_libdir)/pkgconfig/${EPYTHON/n/n-}.pc \
> +			ln -s "${pysysrootlib}"/pkgconfig/${EPYTHON/n/n-}.pc \
>  				"${workdir}"/pkgconfig/python.pc || die
>  			ln -s python.pc "${workdir}"/pkgconfig/python${pyver}.pc || die
>  		else
>  			nonsupp+=( 2to3 python-config "python${pyver}-config" )
>  		fi
>  
> +		echo "exec \"${!lpyvar}\" \"\${@}\"" >> "${workdir}"/bin/python || die
> +		tee "${workdir}"/bin/{python${pyver},"${EPYTHON}"} < "${workdir}"/bin/python >/dev/null || die

Whatever this is supposed to do, do it simpler.

> +		chmod +x "${workdir}"/bin/{python,python${pyver},"${EPYTHON}"} || die
> +
>  		local x
>  		for x in "${nonsupp[@]}"; do
>  			cat >"${workdir}"/bin/${x} <<-_EOF_ || die
> @@ -1064,6 +1120,14 @@ python_wrapper_setup() {
>  		PKG_CONFIG_PATH=${workdir}/pkgconfig${PKG_CONFIG_PATH:+:${PKG_CONFIG_PATH}}
>  	fi
>  	export PATH PKG_CONFIG_PATH
> +
> +	if [[ -n ${pyvar} ]]; then
> +		if [[ -n ${pysysroot} ]]; then
> +			export -- ${pyvar}=${workdir}/bin/${EPYTHON}
> +		else
> +			export -- ${pyvar}=${!lpyvar}
> +		fi
> +	fi

This is just plain ugly.  No.

>  }
>  
>  # @FUNCTION: python_is_python3

-- 
Best regards,
Michał Górny

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 963 bytes --]

Re: [gentoo-dev] [PATCH 06/12] python-utils-r1.eclass: Don't die in python_fix_shebang if all fixed

[Michał Górny]

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

On Thu, 2019-01-03 at 21:39 +0000, James Le Cuirot wrote:
> Shebangs may need fixing on prefix systems or when cross-building but
> not at other times.
> 
> Signed-off-by: James Le Cuirot <chewi@gentoo.org>
> ---
>  eclass/python-utils-r1.eclass | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/eclass/python-utils-r1.eclass b/eclass/python-utils-r1.eclass
> index 19cfaf2798ab..91e457f3cf14 100644
> --- a/eclass/python-utils-r1.eclass
> +++ b/eclass/python-utils-r1.eclass
> @@ -1328,16 +1328,12 @@ python_fix_shebang() {
>  			fi
>  		done < <(find -H "${path}" -type f -print0 || die)
>  
> -		if [[ ! ${any_fixed} ]]; then
> +		if [[ ! ${any_fixed} && ! ${any_correct} ]]; then
>  			local cmd=eerror
>  			[[ ${EAPI:-0} == [012345] ]] && cmd=eqawarn
>  
>  			"${cmd}" "QA warning: ${FUNCNAME}, ${path#${D%/}} did not match any fixable files."
> -			if [[ ${any_correct} ]]; then
> -				"${cmd}" "All files have ${EPYTHON} shebang already."
> -			else
> -				"${cmd}" "There are no Python files in specified directory."
> -			fi
> +			"${cmd}" "There are no Python files in specified directory."
>  
>  			[[ ${cmd} == eerror ]] && die "${FUNCNAME} did not match any fixable files (QA warning fatal in EAPI ${EAPI})"
>  		fi

Sounds like you're introducing breakage, then abusing a function to fix
your breakage, then killing a useful diagnostic because you've just
broken it.

-- 
Best regards,
Michał Górny

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 963 bytes --]

Re: [gentoo-dev] [PATCH] Eclass changes for cross-compiling Python modules

[Michał Górny]

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

On Thu, 2019-01-03 at 21:39 +0000, James Le Cuirot wrote:
> Here's a series of eclass changes I've been working on since August to
> allow Python modules to be cross-compiled. We previously believed this
> to be practically impossible without significant changes upstream and
> in the wider Python ecosystem so getting here feels like quite an
> achievement. While the approach is a little unconventional, it doesn't
> feel overly hacky and the results are quite consistently good.
> 
> These changes should also benefit prefix and I did a full prefix
> bootstrap for the first time this week just to make sure I didn't
> break it.
> 
> A handful of other Python packages that don't quite follow the usual
> mantra have needed fixing up. I haven't included those changes here
> but you can view them on this GitHub pull request.
> 
> https://github.com/gentoo/gentoo/pull/9822
> 
> Once this is in place, I can finish my long-awaited revamp of my
> cross-boss project that will allow you to cross-compile @system from
> scratch with very little effort.
> 

I've reviewed a few commits but I think I had enough.  If you really
want to push this forward, please start by merging fixes into your
previous commits because it's really not useful to have commits like
'fix bugs in commit 3 you've already reviewed'.

Then, you should seriously think how to make things simple.  Because
the eclass is very complex right now, and doubling the complexity for
the sake of corner case of cross isn't going work for me.  The last
thing we need is more hacks on top of hacks to work around previous
hacks.

-- 
Best regards,
Michał Górny

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 963 bytes --]

Re: [gentoo-dev] [PATCH 06/12] python-utils-r1.eclass: Don't die in python_fix_shebang if all fixed

[Mike Gilbert]

On Fri, Jan 4, 2019 at 10:55 AM Michał Górny <mgorny@gentoo.org> wrote:
>
> On Thu, 2019-01-03 at 21:39 +0000, James Le Cuirot wrote:
> > Shebangs may need fixing on prefix systems or when cross-building but
> > not at other times.
> >
> > Signed-off-by: James Le Cuirot <chewi@gentoo.org>
> > ---
> >  eclass/python-utils-r1.eclass | 8 ++------
> >  1 file changed, 2 insertions(+), 6 deletions(-)
> >
> > diff --git a/eclass/python-utils-r1.eclass b/eclass/python-utils-r1.eclass
> > index 19cfaf2798ab..91e457f3cf14 100644
> > --- a/eclass/python-utils-r1.eclass
> > +++ b/eclass/python-utils-r1.eclass
> > @@ -1328,16 +1328,12 @@ python_fix_shebang() {
> >                       fi
> >               done < <(find -H "${path}" -type f -print0 || die)
> >
> > -             if [[ ! ${any_fixed} ]]; then
> > +             if [[ ! ${any_fixed} && ! ${any_correct} ]]; then
> >                       local cmd=eerror
> >                       [[ ${EAPI:-0} == [012345] ]] && cmd=eqawarn
> >
> >                       "${cmd}" "QA warning: ${FUNCNAME}, ${path#${D%/}} did not match any fixable files."
> > -                     if [[ ${any_correct} ]]; then
> > -                             "${cmd}" "All files have ${EPYTHON} shebang already."
> > -                     else
> > -                             "${cmd}" "There are no Python files in specified directory."
> > -                     fi
> > +                     "${cmd}" "There are no Python files in specified directory."
> >
> >                       [[ ${cmd} == eerror ]] && die "${FUNCNAME} did not match any fixable files (QA warning fatal in EAPI ${EAPI})"
> >               fi
>
> Sounds like you're introducing breakage, then abusing a function to fix
> your breakage, then killing a useful diagnostic because you've just
> broken it.

I'm unable to make sense of what you are trying to say here. Is there
something wrong with this patch, or are you making a general comment
on the patch series?

Also, please try to be less combative in your review process.

Re: [gentoo-dev] [PATCH 06/12] python-utils-r1.eclass: Don't die in python_fix_shebang if all fixed

[Michał Górny]

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

On Fri, 2019-01-04 at 13:10 -0500, Mike Gilbert wrote:
> On Fri, Jan 4, 2019 at 10:55 AM Michał Górny <mgorny@gentoo.org> wrote:
> > 
> > On Thu, 2019-01-03 at 21:39 +0000, James Le Cuirot wrote:
> > > Shebangs may need fixing on prefix systems or when cross-building but
> > > not at other times.
> > > 
> > > Signed-off-by: James Le Cuirot <chewi@gentoo.org>
> > > ---
> > >  eclass/python-utils-r1.eclass | 8 ++------
> > >  1 file changed, 2 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/eclass/python-utils-r1.eclass b/eclass/python-utils-r1.eclass
> > > index 19cfaf2798ab..91e457f3cf14 100644
> > > --- a/eclass/python-utils-r1.eclass
> > > +++ b/eclass/python-utils-r1.eclass
> > > @@ -1328,16 +1328,12 @@ python_fix_shebang() {
> > >                       fi
> > >               done < <(find -H "${path}" -type f -print0 || die)
> > > 
> > > -             if [[ ! ${any_fixed} ]]; then
> > > +             if [[ ! ${any_fixed} && ! ${any_correct} ]]; then
> > >                       local cmd=eerror
> > >                       [[ ${EAPI:-0} == [012345] ]] && cmd=eqawarn
> > > 
> > >                       "${cmd}" "QA warning: ${FUNCNAME}, ${path#${D%/}} did not match any fixable files."
> > > -                     if [[ ${any_correct} ]]; then
> > > -                             "${cmd}" "All files have ${EPYTHON} shebang already."
> > > -                     else
> > > -                             "${cmd}" "There are no Python files in specified directory."
> > > -                     fi
> > > +                     "${cmd}" "There are no Python files in specified directory."
> > > 
> > >                       [[ ${cmd} == eerror ]] && die "${FUNCNAME} did not match any fixable files (QA warning fatal in EAPI ${EAPI})"
> > >               fi
> > 
> > Sounds like you're introducing breakage, then abusing a function to fix
> > your breakage, then killing a useful diagnostic because you've just
> > broken it.
> 
> I'm unable to make sense of what you are trying to say here. Is there
> something wrong with this patch, or are you making a general comment
> on the patch series?
> 

Original usage: rewrite 'python' to 'pythonX.Y', etc. on static files. 
Throws an error if you try to use for files that have correct shebang
already.

Now:

1. Due to PYTHON override, generated files frequently have wrong
shebangs.

2. python_fix_shebang is modified to fix this -- orthogonal behavior is
introduced.

3. Now diagnostic on files with correct shebang is gone because it
conflicts with the orthogonal behavior added here.

-- 
Best regards,
Michał Górny

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 963 bytes --]

Re: [gentoo-dev] [PATCH] Eclass changes for cross-compiling Python modules

[James Le Cuirot]

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

On Fri, 04 Jan 2019 17:03:42 +0100
Michał Górny <mgorny@gentoo.org> wrote:

> On Thu, 2019-01-03 at 21:39 +0000, James Le Cuirot wrote:
> > Here's a series of eclass changes I've been working on since August to
> > allow Python modules to be cross-compiled. We previously believed this
> > to be practically impossible without significant changes upstream and
> > in the wider Python ecosystem so getting here feels like quite an
> > achievement. While the approach is a little unconventional, it doesn't
> > feel overly hacky and the results are quite consistently good.
> > 
> > These changes should also benefit prefix and I did a full prefix
> > bootstrap for the first time this week just to make sure I didn't
> > break it.
> > 
> > A handful of other Python packages that don't quite follow the usual
> > mantra have needed fixing up. I haven't included those changes here
> > but you can view them on this GitHub pull request.
> > 
> > https://github.com/gentoo/gentoo/pull/9822
> > 
> > Once this is in place, I can finish my long-awaited revamp of my
> > cross-boss project that will allow you to cross-compile @system from
> > scratch with very little effort.
> >   
> 
> I've reviewed a few commits but I think I had enough.  If you really
> want to push this forward, please start by merging fixes into your
> previous commits because it's really not useful to have commits like
> 'fix bugs in commit 3 you've already reviewed'.

Sure, that's what I usually do.

> Then, you should seriously think how to make things simple.  Because
> the eclass is very complex right now, and doubling the complexity for
> the sake of corner case of cross isn't going work for me.  The last
> thing we need is more hacks on top of hacks to work around previous
> hacks.

I knew this was going to be a tough sell and I put it up on GitHub
first to get your initial feedback and keep you in the loop. Of course
I want to keep it simple, not least so it has a greater chance of being
accepted, but you should know better than most that this was never
going to be easy. As I said, we initially thought it was impossible.
I'll respond to your points so far on the list but we can take this
back to GitHub to avoid spamming everyone else.

-- 
James Le Cuirot (chewi)
Gentoo Linux Developer

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

Re: [gentoo-dev] [PATCH 04/12] python-utils-r1.eclass: Use wrapper scripts to fix cross-compiling

[James Le Cuirot]

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

On Fri, 04 Jan 2019 16:53:46 +0100
Michał Górny <mgorny@gentoo.org> wrote:

> On Thu, 2019-01-03 at 21:39 +0000, James Le Cuirot wrote:
> > Python has little concept of cross-compiling but it turns out that
> > making it work isn't so hard after all.
> > 
> > Platform-specific details are held within _sysconfigdata.py,
> > sysconfig.py, and various distutils files. If we simply symlink these
> > from SYSROOT into an empty directory and add that directory to
> > PYTHONPATH then we can utilise the build host's Python with the target
> > host's settings.
> > 
> > The pkg-config files were already being symlinked in a similar manner
> > but now we source them from within SYSROOT.
> > 
> > In order for PYTHONPATH to be respected, Python must be executed via
> > the wrapper, which was not the case before. We previously relied
> > solely on the PATH but now PYTHON must point to the wrapper rather
> > than the usual location under /usr/bin. However, we only do this when
> > SYSROOT or EPREFIX are effectively set to avoid unnecessary
> > complexity. This has required some rejigging in the way that PYTHON is
> > set but it should remain compatible with existing packages.
> > 
> > The python_wrapper_setup function that handles all this has had its
> > arguments reordered because no one ever uses the path/workdir
> > argument, which makes specifying other arguments tedious.  
> 
> This really belongs in a separate patch.

Fair enough.

> > Some packages rely on python-config but luckily this is just a shell
> > script so it can be executed from within SYSROOT. This is bending the
> > rules of PMS slightly but Python leaves us with little choice. I also
> > wrote those rules so I'm allowed to bend them. ;)
> > 
> > PYTHON_INCLUDEDIR, PYTHON_LIBPATH, and their associated functions are
> > generally used during src_configure or src_compile and, as such, they
> > now need to have SYSROOT prepended.
> > 
> > python_doheader uses PYTHON_INCLUDEDIR to install new headers and
> > therefore needs the value without SYSROOT. It was already stripping
> > EPREFIX before so now it simply strips SYSROOT as well. Similar
> > instances of this can do likewise or call the functions with SYSROOT
> > unset to achieve the same effect.
> > 
> > To make all this work, we are assuming that CPython is located at
> > /usr/$(get_libdir)/${EPYTHON}, which is admittedly a little circular
> > when we are querying Python for the right paths. I feel the reason
> > that python_export was rewritten to query these dynamically was less
> > because someone might install Python to some weird location and more
> > to avoid special handling for each of the different
> > implementations. And what of those other implementations?  
> 
> This is a wrong assumption.  CPython 3.7 is in /usr/lib/python3.7.

It was true at the time I wrote it. I addressed 3.7 later in the patch
series. I figured this diff was long enough already so I kept it
separate.

> > Being Java-based, Jython is installed under the platform-neutral
> > /usr/share and presumably has few other platform-specific
> > aspects. Enabling native extensions appears non-trivial and none of
> > our module packages have enabled support for it anyway.
> > 
> > I think PyPy could potentially support cross-compiling but it
> > hardcodes the native extension filename suffix within its own binaries
> > with no way to override it. Perhaps we could patch this in somehow but
> > I'm afraid I don't care enough.
> > 
> > Together with the following changes to distutils-r1.eclass, I have now
> > been able to cross-compile a large number of packages with native
> > Python extensions, most with no changes at all, and the rest with only
> > minor fixes.
> > 
> > Closes: https://bugs.gentoo.org/503874
> > Bug: https://bugs.gentoo.org/648652
> > Signed-off-by: James Le Cuirot <chewi@gentoo.org>
> > ---
> >  eclass/python-utils-r1.eclass | 120 ++++++++++++++++++++++++++--------
> >  1 file changed, 92 insertions(+), 28 deletions(-)
> > 
> > diff --git a/eclass/python-utils-r1.eclass b/eclass/python-utils-r1.eclass
> > index 1a549f49f3f2..607af1b52f75 100644
> > --- a/eclass/python-utils-r1.eclass
> > +++ b/eclass/python-utils-r1.eclass
> > @@ -211,9 +211,15 @@ _python_impl_matches() {
> >  #
> >  # distutils-r1: Set within any of the python sub-phase functions.
> >  #
> > -# Example value:
> > +# If SYSROOT or EPREFIX are effectively set then this will point to an
> > +# automatically generated wrapper rather than the usual path under
> > +# /usr/bin in order to accommodate cross-compiling. We could do this all
> > +# the time but it would add unnecessary complexity.
> > +#
> > +# Example values:
> >  # @CODE
> >  # /usr/bin/python2.7
> > +# /var/tmp/portage/dev-python/pyblake2-1.2.3/temp/python2.7/bin/python2.7
> >  # @CODE
> >  
> >  # @ECLASS-VARIABLE: EPYTHON
> > @@ -256,6 +262,10 @@ _python_impl_matches() {
> >  # Set and exported on request using python_export().
> >  # Requires a proper build-time dependency on the Python implementation.
> >  #
> > +# This is prepended with SYSROOT in order to accommodate
> > +# cross-compiling. You may need to strip SYSROOT and EPREFIX if using it
> > +# to install new headers.
> > +#
> >  # Example value:
> >  # @CODE
> >  # /usr/include/python2.7
> > @@ -270,6 +280,9 @@ _python_impl_matches() {
> >  # Valid only for CPython. Requires a proper build-time dependency
> >  # on the Python implementation.
> >  #
> > +# This is prepended with SYSROOT in order to accommodate
> > +# cross-compiling.
> > +#
> >  # Example value:
> >  # @CODE
> >  # /usr/lib64/libpython2.7.so
> > @@ -314,6 +327,10 @@ _python_impl_matches() {
> >  # Valid only for CPython. Requires a proper build-time dependency
> >  # on the Python implementation and on pkg-config.
> >  #
> > +# This is prepended with SYSROOT in order to accommodate
> > +# cross-compiling. You generally should not execute files within SYSROOT
> > +# but python-config is always a shell script.
> > +#
> >  # Example value:
> >  # @CODE
> >  # /usr/bin/python2.7-config
> > @@ -380,6 +397,10 @@ python_export() {
> >  	esac
> >  	debug-print "${FUNCNAME}: implementation: ${impl}"
> >  
> > +	# Many variables below need a PYTHON variable but we should not
> > +	# export it unless explicitly requested so use _PYTHON instead.
> > +	local _PYTHON
> > +
> >  	for var; do
> >  		case "${var}" in
> >  			EPYTHON)
> > @@ -387,21 +408,21 @@ python_export() {
> >  				debug-print "${FUNCNAME}: EPYTHON = ${EPYTHON}"
> >  				;;
> >  			PYTHON)
> > -				export PYTHON=${EPREFIX}/usr/bin/${impl}
> > +				python_wrapper_setup ${impl} PYTHON
> >  				debug-print "${FUNCNAME}: PYTHON = ${PYTHON}"
> >  				;;
> >  			PYTHON_SITEDIR)
> > -				[[ -n ${PYTHON} ]] || die "PYTHON needs to be set for ${var} to be exported, or requested before it"
> > +				python_wrapper_setup ${impl} _PYTHON
> >  				# sysconfig can't be used because:
> >  				# 1) pypy doesn't give site-packages but stdlib
> >  				# 2) jython gives paths with wrong case
> > -				PYTHON_SITEDIR=$("${PYTHON}" -c 'import distutils.sysconfig; print(distutils.sysconfig.get_python_lib())') || die
> > +				PYTHON_SITEDIR=$("${_PYTHON}" -c 'import distutils.sysconfig; print(distutils.sysconfig.get_python_lib())') || die
> >  				export PYTHON_SITEDIR
> >  				debug-print "${FUNCNAME}: PYTHON_SITEDIR = ${PYTHON_SITEDIR}"
> >  				;;
> >  			PYTHON_INCLUDEDIR)
> > -				[[ -n ${PYTHON} ]] || die "PYTHON needs to be set for ${var} to be exported, or requested before it"
> > -				PYTHON_INCLUDEDIR=$("${PYTHON}" -c 'import distutils.sysconfig; print(distutils.sysconfig.get_python_inc())') || die
> > +				python_wrapper_setup ${impl} _PYTHON
> > +				PYTHON_INCLUDEDIR=${SYSROOT}$("${_PYTHON}" -c 'import distutils.sysconfig; print(distutils.sysconfig.get_python_inc())') || die
> >  				export PYTHON_INCLUDEDIR
> >  				debug-print "${FUNCNAME}: PYTHON_INCLUDEDIR = ${PYTHON_INCLUDEDIR}"
> >  
> > @@ -411,8 +432,8 @@ python_export() {
> >  				fi
> >  				;;
> >  			PYTHON_LIBPATH)
> > -				[[ -n ${PYTHON} ]] || die "PYTHON needs to be set for ${var} to be exported, or requested before it"
> > -				PYTHON_LIBPATH=$("${PYTHON}" -c 'import os.path, sysconfig; print(os.path.join(sysconfig.get_config_var("LIBDIR"), sysconfig.get_config_var("LDLIBRARY")) if sysconfig.get_config_var("LDLIBRARY") else "")') || die
> > +				python_wrapper_setup ${impl} _PYTHON
> > +				PYTHON_LIBPATH=${SYSROOT}$("${_PYTHON}" -c 'import os.path, sysconfig; print(os.path.join(sysconfig.get_config_var("LIBDIR"), sysconfig.get_config_var("LDLIBRARY")) if sysconfig.get_config_var("LDLIBRARY") else "")') || die
> >  				export PYTHON_LIBPATH
> >  				debug-print "${FUNCNAME}: PYTHON_LIBPATH = ${PYTHON_LIBPATH}"
> >  
> > @@ -457,9 +478,9 @@ python_export() {
> >  
> >  				case "${impl}" in
> >  					python*)
> > -						[[ -n ${PYTHON} ]] || die "PYTHON needs to be set for ${var} to be exported, or requested before it"
> > -						flags=$("${PYTHON}" -c 'import sysconfig; print(sysconfig.get_config_var("ABIFLAGS") or "")') || die
> > -						val=${PYTHON}${flags}-config
> > +						python_wrapper_setup ${impl} _PYTHON
> > +						flags=$("${_PYTHON}" -c 'import sysconfig; print(sysconfig.get_config_var("ABIFLAGS") or "")') || die
> > +						val=${ESYSROOT-${SYSROOT}${EPREFIX}}/usr/bin/${PYTHON##*/}${flags}-config
> >  						;;
> >  					*)
> >  						die "${impl}: obtaining ${var} not supported"
> > @@ -954,7 +975,7 @@ python_doheader() {
> >  	local d PYTHON_INCLUDEDIR=${PYTHON_INCLUDEDIR}
> >  	[[ ${PYTHON_INCLUDEDIR} ]] || python_export PYTHON_INCLUDEDIR
> >  
> > -	d=${PYTHON_INCLUDEDIR#${EPREFIX}}
> > +	d=${PYTHON_INCLUDEDIR#${ESYSROOT-${SYSROOT}${EPREFIX}}}
> >  
> >  	(
> >  		insopts -m 0644
> > @@ -964,7 +985,7 @@ python_doheader() {
> >  }
> >  
> >  # @FUNCTION: python_wrapper_setup
> > -# @USAGE: [<path> [<impl>]]
> > +# @USAGE: [<impl> [<pyvar> [<path>]]]
> >  # @DESCRIPTION:
> >  # Create proper 'python' executable and pkg-config wrappers
> >  # (if available) in the directory named by <path>. Set up PATH
> > @@ -973,6 +994,9 @@ python_doheader() {
> >  # The wrappers will be created for implementation named by <impl>,
> >  # or for one named by ${EPYTHON} if no <impl> passed.
> >  #
> > +# The path to the 'python' executable wrapper is exported to the
> > +# variable named <pyvar> if given.  
> 
> Why do you need that in the first place?  The path should be rather
> predictable, shouldn't it?

I thought about keeping the logic of the PYTHON value back in
python_export but it's quite likely someone would export PYTHON without
calling python_wrapper_setup and so it would break. If we do that while
keeping the call to python_wrapper_setup, you'd need to handle infinite
loops for when python_wrapper_setup is called from elsewhere.

I initially had it export PYTHON unconditionally but later realised
this was impolite and actually caused a problem in dev-lang/python
itself. I guess we could export using another variable name but this
seemed tidier.

The only other way to pass the value back from python_wrapper_setup
would be to echo it but that seems ugly and would require amending all
the existing invocations.

> > +#
> >  # If the named directory contains a python symlink already, it will
> >  # be assumed to contain proper wrappers already and only environment
> >  # setup will be done. If wrapper update is requested, the directory
> > @@ -980,25 +1004,41 @@ python_doheader() {
> >  python_wrapper_setup() {
> >  	debug-print-function ${FUNCNAME} "${@}"
> >  
> > -	local impl=${2:-${EPYTHON}}
> > -	local workdir=${1:-${T}/${impl}}
> > +	local impl=${1:-${EPYTHON}}
> > +	local pyvar=${2}
> > +	local lpyvar=_${pyvar:-PYTHON}
> > +
> > +	# Use separate directories for SYSROOT in case we need to execute
> > +	# Python in the context of the build host by unsetting SYSROOT.
> > +	local workdir=${3:-${T}/${impl}${SYSROOT:+-sysroot}}
> >  
> >  	[[ ${workdir} ]] || die "${FUNCNAME}: no workdir specified."
> >  	[[ ${impl} ]] || die "${FUNCNAME}: no impl nor EPYTHON specified."
> >  
> > +	local EPYTHON
> > +	python_export "${impl}" EPYTHON  
> 
> I'm getting dizzy because of the amount of circular dependencies between
> the two functions you've introduced.  This is going to make it trivial
> to deadlock it when changing code later.

Look below, apart from my removing PYTHON, these lines were already
there! Indeed, this made my head hurt too but I tried my best to
respect how the eclass works now. If you want to simplify it somehow,
I'm totally up for that discussion.

> > +
> > +	# We could use BROOT here but using the PATH accommodates
> > +	# cross-prefix where the PATH is sometimes manipulated to prefer
> > +	# build tools from the target prefix (i.e. EPREFIX).
> > +	#
> > +	# Also make sure we don't pick up an existing wrapper by replacing
> > +	# instances of ${T} with a bogus location. The workdir can be
> > +	# overridden but hopefully it will be somewhere under ${T}.
> > +	local ${lpyvar}=$(PATH=${PATH//${T}//dev/null} type -P "${EPYTHON}" || die "${FUNCNAME}: can't find ${EPYTHON} in PATH")  
> 
> This PATH substitution hack is horrible.

It's not my favourite part of the changeset, I'll grant you, but the
PATH value only exists for that one line and I wanted to be sure that
type -P didn't pick up some other location. I could strip out ${T} more
accurately but that would be more code for little benefit. Your call.

> > +
> > +	local pysysroot=${ESYSROOT-${SYSROOT%/}${EPREFIX}}
> > +
> >  	if [[ ! -x ${workdir}/bin/python ]]; then
> >  		_python_check_dead_variables
> >  
> > -		mkdir -p "${workdir}"/{bin,pkgconfig} || die
> > +		mkdir -p "${workdir}"/{bin,lib,pkgconfig} || die
> >  
> >  		# Clean up, in case we were supposed to do a cheap update.
> >  		rm -f "${workdir}"/bin/python{,2,3}{,-config} || die
> >  		rm -f "${workdir}"/bin/2to3 || die
> >  		rm -f "${workdir}"/pkgconfig/python{,2,3}.pc || die
> >  
> > -		local EPYTHON PYTHON
> > -		python_export "${impl}" EPYTHON PYTHON
> > -
> >  		local pyver pyother
> >  		if python_is_python3; then
> >  			pyver=3
> > @@ -1012,37 +1052,53 @@ python_wrapper_setup() {
> >  		# note: we don't use symlinks because python likes to do some
> >  		# symlink reading magic that breaks stuff
> >  		# https://bugs.gentoo.org/show_bug.cgi?id=555752
> > -		cat > "${workdir}/bin/python" <<-_EOF_ || die
> > -			#!/bin/sh
> > -			exec "${PYTHON}" "\${@}"
> > -		_EOF_
> > -		cp "${workdir}/bin/python" "${workdir}/bin/python${pyver}" || die
> > -		chmod +x "${workdir}/bin/python" "${workdir}/bin/python${pyver}" || die
> > +		echo '#!/bin/sh' > "${workdir}/bin/python" || die 
> >  		local nonsupp=( "python${pyother}" "python${pyother}-config" )
> >  
> >  		# CPython-specific
> >  		if [[ ${EPYTHON} == python* ]]; then
> > +			local pysysrootlib=${pysysroot}/usr/$(get_libdir)
> > +
> >  			cat > "${workdir}/bin/python-config" <<-_EOF_ || die
> >  				#!/bin/sh
> > -				exec "${PYTHON}-config" "\${@}"
> > +				exec "${pysysroot}/usr/bin/${EPYTHON}-config" "\${@}"
> >  			_EOF_
> >  			cp "${workdir}/bin/python-config" \
> >  				"${workdir}/bin/python${pyver}-config" || die
> >  			chmod +x "${workdir}/bin/python-config" \
> >  				"${workdir}/bin/python${pyver}-config" || die
> >  
> > +			if [[ -n ${pysysroot} ]]; then
> > +				# Use host-specific data from SYSROOT. Python 2 looks
> > +				# for _sysconfigdata while Python 3 uses
> > +				# _PYTHON_SYSCONFIGDATA_NAME.
> > +				ln -s "${pysysrootlib}/${EPYTHON}"/_sysconfigdata*.py "${workdir}"/lib/_sysconfigdata.py || die
> > +
> > +				# Use distutils/sysconfig from SYSROOT as parts of it
> > +				# have GENTOO_LIBDIR baked in at Python build time.
> > +				ln -s "${pysysrootlib}/${EPYTHON}"/{distutils,sysconfig.py} "${workdir}"/lib/ || die
> > +
> > +				# Add env vars to python wrapper accordingly.
> > +				echo "PYTHONPATH=\"${workdir}/lib\${PYTHONPATH:+:}\${PYTHONPATH}\" _PYTHON_SYSCONFIGDATA_NAME=_sysconfigdata \\" \
> > +					 >> "${workdir}/bin/python" || die  
> 
> Use explicit 'export ...' multi-line thing for readability.

Okay, I guess using a here document wouldn't be so bad.

> > +			fi
> > +
> >  			# Python 2.6+.
> > -			ln -s "${PYTHON/python/2to3-}" "${workdir}"/bin/2to3 || die
> > +			ln -s "${EPYTHON/python/2to3-}" "${workdir}"/bin/2to3 || die
> >  
> >  			# Python 2.7+.
> > -			ln -s "${EPREFIX}"/usr/$(get_libdir)/pkgconfig/${EPYTHON/n/n-}.pc \
> > +			ln -s "${pysysrootlib}"/pkgconfig/${EPYTHON/n/n-}.pc \
> >  				"${workdir}"/pkgconfig/python.pc || die
> >  			ln -s python.pc "${workdir}"/pkgconfig/python${pyver}.pc || die
> >  		else
> >  			nonsupp+=( 2to3 python-config "python${pyver}-config" )
> >  		fi
> >  
> > +		echo "exec \"${!lpyvar}\" \"\${@}\"" >> "${workdir}"/bin/python || die
> > +		tee "${workdir}"/bin/{python${pyver},"${EPYTHON}"} < "${workdir}"/bin/python >/dev/null || die  
> 
> Whatever this is supposed to do, do it simpler.

I suppose the one line it saves isn't worth the extra brain time.

> > +		chmod +x "${workdir}"/bin/{python,python${pyver},"${EPYTHON}"} || die
> > +
> >  		local x
> >  		for x in "${nonsupp[@]}"; do
> >  			cat >"${workdir}"/bin/${x} <<-_EOF_ || die
> > @@ -1064,6 +1120,14 @@ python_wrapper_setup() {
> >  		PKG_CONFIG_PATH=${workdir}/pkgconfig${PKG_CONFIG_PATH:+:${PKG_CONFIG_PATH}}
> >  	fi
> >  	export PATH PKG_CONFIG_PATH
> > +
> > +	if [[ -n ${pyvar} ]]; then
> > +		if [[ -n ${pysysroot} ]]; then
> > +			export -- ${pyvar}=${workdir}/bin/${EPYTHON}
> > +		else
> > +			export -- ${pyvar}=${!lpyvar}
> > +		fi
> > +	fi  
> 
> This is just plain ugly.  No.

Could you be a bit more specific? Whether we make it conditional or
not, we need to export something. I have played it safe by only
pointing to the wrapper when SYSROOT or EPREFIX are effectively set. If
you'd prefer to just use the wrapper all the time then we can do that
but I imagine you'd rather minimise the risk to non-cross/prefix users.
If ${!lpyvar} is the issue, I did this to ensure that our local
variable name doesn't clash with the one being exported but we could
just use some fixed obscure name instead.

> >  }
> >  
> >  # @FUNCTION: python_is_python3  
> 


-- 
James Le Cuirot (chewi)
Gentoo Linux Developer

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

Re: [gentoo-dev] [PATCH 06/12] python-utils-r1.eclass: Don't die in python_fix_shebang if all fixed

[James Le Cuirot]

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

On Fri, 04 Jan 2019 19:26:34 +0100
Michał Górny <mgorny@gentoo.org> wrote:

> On Fri, 2019-01-04 at 13:10 -0500, Mike Gilbert wrote:
> > On Fri, Jan 4, 2019 at 10:55 AM Michał Górny <mgorny@gentoo.org> wrote:  
> > > 
> > > On Thu, 2019-01-03 at 21:39 +0000, James Le Cuirot wrote:  
> > > > Shebangs may need fixing on prefix systems or when cross-building but
> > > > not at other times.
> > > > 
> > > > Signed-off-by: James Le Cuirot <chewi@gentoo.org>
> > > > ---
> > > >  eclass/python-utils-r1.eclass | 8 ++------
> > > >  1 file changed, 2 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/eclass/python-utils-r1.eclass b/eclass/python-utils-r1.eclass
> > > > index 19cfaf2798ab..91e457f3cf14 100644
> > > > --- a/eclass/python-utils-r1.eclass
> > > > +++ b/eclass/python-utils-r1.eclass
> > > > @@ -1328,16 +1328,12 @@ python_fix_shebang() {
> > > >                       fi
> > > >               done < <(find -H "${path}" -type f -print0 || die)
> > > > 
> > > > -             if [[ ! ${any_fixed} ]]; then
> > > > +             if [[ ! ${any_fixed} && ! ${any_correct} ]]; then
> > > >                       local cmd=eerror
> > > >                       [[ ${EAPI:-0} == [012345] ]] && cmd=eqawarn
> > > > 
> > > >                       "${cmd}" "QA warning: ${FUNCNAME}, ${path#${D%/}} did not match any fixable files."
> > > > -                     if [[ ${any_correct} ]]; then
> > > > -                             "${cmd}" "All files have ${EPYTHON} shebang already."
> > > > -                     else
> > > > -                             "${cmd}" "There are no Python files in specified directory."
> > > > -                     fi
> > > > +                     "${cmd}" "There are no Python files in specified directory."
> > > > 
> > > >                       [[ ${cmd} == eerror ]] && die "${FUNCNAME} did not match any fixable files (QA warning fatal in EAPI ${EAPI})"
> > > >               fi  
> > > 
> > > Sounds like you're introducing breakage, then abusing a function to fix
> > > your breakage, then killing a useful diagnostic because you've just
> > > broken it.  
> > 
> > I'm unable to make sense of what you are trying to say here. Is there
> > something wrong with this patch, or are you making a general comment
> > on the patch series?
> >   
> 
> Original usage: rewrite 'python' to 'pythonX.Y', etc. on static files. 
> Throws an error if you try to use for files that have correct shebang
> already.
> 
> Now:
> 
> 1. Due to PYTHON override, generated files frequently have wrong
> shebangs.
> 
> 2. python_fix_shebang is modified to fix this -- orthogonal behavior is
> introduced.
> 
> 3. Now diagnostic on files with correct shebang is gone because it
> conflicts with the orthogonal behavior added here.

The diagnostic is a problem even without my changes.

It's quite likely upstream could hardcode a shebang like
#!/usr/bin/python2.7, which does not need to be modified on normal
systems but does on prefixed systems. 

What about hardcoding #!/usr/bin/python3.6? This would need correcting
against 3.7 but would fall foul of the diagnostic when using 3.6.

The cross-prefix issue where the shebang points to BROOT rather than
EPREFIX could also already happen now.

-- 
James Le Cuirot (chewi)
Gentoo Linux Developer

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

Re: [gentoo-dev] [PATCH 06/12] python-utils-r1.eclass: Don't die in python_fix_shebang if all fixed

[Michał Górny]

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

On Sat, 2019-01-05 at 21:38 +0000, James Le Cuirot wrote:
> On Fri, 04 Jan 2019 19:26:34 +0100
> Michał Górny <mgorny@gentoo.org> wrote:
> 
> > On Fri, 2019-01-04 at 13:10 -0500, Mike Gilbert wrote:
> > > On Fri, Jan 4, 2019 at 10:55 AM Michał Górny <mgorny@gentoo.org> wrote:  
> > > > 
> > > > On Thu, 2019-01-03 at 21:39 +0000, James Le Cuirot wrote:  
> > > > > Shebangs may need fixing on prefix systems or when cross-building but
> > > > > not at other times.
> > > > > 
> > > > > Signed-off-by: James Le Cuirot <chewi@gentoo.org>
> > > > > ---
> > > > >  eclass/python-utils-r1.eclass | 8 ++------
> > > > >  1 file changed, 2 insertions(+), 6 deletions(-)
> > > > > 
> > > > > diff --git a/eclass/python-utils-r1.eclass b/eclass/python-utils-r1.eclass
> > > > > index 19cfaf2798ab..91e457f3cf14 100644
> > > > > --- a/eclass/python-utils-r1.eclass
> > > > > +++ b/eclass/python-utils-r1.eclass
> > > > > @@ -1328,16 +1328,12 @@ python_fix_shebang() {
> > > > >                       fi
> > > > >               done < <(find -H "${path}" -type f -print0 || die)
> > > > > 
> > > > > -             if [[ ! ${any_fixed} ]]; then
> > > > > +             if [[ ! ${any_fixed} && ! ${any_correct} ]]; then
> > > > >                       local cmd=eerror
> > > > >                       [[ ${EAPI:-0} == [012345] ]] && cmd=eqawarn
> > > > > 
> > > > >                       "${cmd}" "QA warning: ${FUNCNAME}, ${path#${D%/}} did not match any fixable files."
> > > > > -                     if [[ ${any_correct} ]]; then
> > > > > -                             "${cmd}" "All files have ${EPYTHON} shebang already."
> > > > > -                     else
> > > > > -                             "${cmd}" "There are no Python files in specified directory."
> > > > > -                     fi
> > > > > +                     "${cmd}" "There are no Python files in specified directory."
> > > > > 
> > > > >                       [[ ${cmd} == eerror ]] && die "${FUNCNAME} did not match any fixable files (QA warning fatal in EAPI ${EAPI})"
> > > > >               fi  
> > > > 
> > > > Sounds like you're introducing breakage, then abusing a function to fix
> > > > your breakage, then killing a useful diagnostic because you've just
> > > > broken it.  
> > > 
> > > I'm unable to make sense of what you are trying to say here. Is there
> > > something wrong with this patch, or are you making a general comment
> > > on the patch series?
> > >   
> > 
> > Original usage: rewrite 'python' to 'pythonX.Y', etc. on static files. 
> > Throws an error if you try to use for files that have correct shebang
> > already.
> > 
> > Now:
> > 
> > 1. Due to PYTHON override, generated files frequently have wrong
> > shebangs.
> > 
> > 2. python_fix_shebang is modified to fix this -- orthogonal behavior is
> > introduced.
> > 
> > 3. Now diagnostic on files with correct shebang is gone because it
> > conflicts with the orthogonal behavior added here.
> 
> The diagnostic is a problem even without my changes.
> 
> It's quite likely upstream could hardcode a shebang like
> #!/usr/bin/python2.7, which does not need to be modified on normal
> systems but does on prefixed systems. 

Fixing prefix was never the goal.

> 
> What about hardcoding #!/usr/bin/python3.6? This would need correcting
> against 3.7 but would fall foul of the diagnostic when using 3.6.

No, it would fail as mismatched shebang.  By design.

python_fix_shebang is meant to fix common cases of generic shebangs
in a safe way, not serve as a generic shebang replacement tool.

-- 
Best regards,
Michał Górny

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 963 bytes --]

Re: [gentoo-dev] [PATCH 06/12] python-utils-r1.eclass: Don't die in python_fix_shebang if all fixed

[James Le Cuirot]

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

On Sat, 05 Jan 2019 22:42:27 +0100
Michał Górny <mgorny@gentoo.org> wrote:

> On Sat, 2019-01-05 at 21:38 +0000, James Le Cuirot wrote:
> > On Fri, 04 Jan 2019 19:26:34 +0100
> > Michał Górny <mgorny@gentoo.org> wrote:
> >   
> > > On Fri, 2019-01-04 at 13:10 -0500, Mike Gilbert wrote:  
> > > > On Fri, Jan 4, 2019 at 10:55 AM Michał Górny <mgorny@gentoo.org> wrote:    
> > > > > 
> > > > > On Thu, 2019-01-03 at 21:39 +0000, James Le Cuirot wrote:    
> > > > > > Shebangs may need fixing on prefix systems or when cross-building but
> > > > > > not at other times.
> > > > > > 
> > > > > > Signed-off-by: James Le Cuirot <chewi@gentoo.org>
> > > > > > ---
> > > > > >  eclass/python-utils-r1.eclass | 8 ++------
> > > > > >  1 file changed, 2 insertions(+), 6 deletions(-)
> > > > > > 
> > > > > > diff --git a/eclass/python-utils-r1.eclass b/eclass/python-utils-r1.eclass
> > > > > > index 19cfaf2798ab..91e457f3cf14 100644
> > > > > > --- a/eclass/python-utils-r1.eclass
> > > > > > +++ b/eclass/python-utils-r1.eclass
> > > > > > @@ -1328,16 +1328,12 @@ python_fix_shebang() {
> > > > > >                       fi
> > > > > >               done < <(find -H "${path}" -type f -print0 || die)
> > > > > > 
> > > > > > -             if [[ ! ${any_fixed} ]]; then
> > > > > > +             if [[ ! ${any_fixed} && ! ${any_correct} ]]; then
> > > > > >                       local cmd=eerror
> > > > > >                       [[ ${EAPI:-0} == [012345] ]] && cmd=eqawarn
> > > > > > 
> > > > > >                       "${cmd}" "QA warning: ${FUNCNAME}, ${path#${D%/}} did not match any fixable files."
> > > > > > -                     if [[ ${any_correct} ]]; then
> > > > > > -                             "${cmd}" "All files have ${EPYTHON} shebang already."
> > > > > > -                     else
> > > > > > -                             "${cmd}" "There are no Python files in specified directory."
> > > > > > -                     fi
> > > > > > +                     "${cmd}" "There are no Python files in specified directory."
> > > > > > 
> > > > > >                       [[ ${cmd} == eerror ]] && die "${FUNCNAME} did not match any fixable files (QA warning fatal in EAPI ${EAPI})"
> > > > > >               fi    
> > > > > 
> > > > > Sounds like you're introducing breakage, then abusing a function to fix
> > > > > your breakage, then killing a useful diagnostic because you've just
> > > > > broken it.    
> > > > 
> > > > I'm unable to make sense of what you are trying to say here. Is there
> > > > something wrong with this patch, or are you making a general comment
> > > > on the patch series?
> > > >     
> > > 
> > > Original usage: rewrite 'python' to 'pythonX.Y', etc. on static files. 
> > > Throws an error if you try to use for files that have correct shebang
> > > already.
> > > 
> > > Now:
> > > 
> > > 1. Due to PYTHON override, generated files frequently have wrong
> > > shebangs.
> > > 
> > > 2. python_fix_shebang is modified to fix this -- orthogonal
> > > behavior is introduced.
> > > 
> > > 3. Now diagnostic on files with correct shebang is gone because it
> > > conflicts with the orthogonal behavior added here.  
> > 
> > The diagnostic is a problem even without my changes.
> > 
> > It's quite likely upstream could hardcode a shebang like
> > #!/usr/bin/python2.7, which does not need to be modified on normal
> > systems but does on prefixed systems.   
> 
> Fixing prefix was never the goal.

And fixing prefix is a bad thing? The prefix team does seem interested
in this work. There is a good deal of overlap here and in making sure I
wasn't making prefix worse, I had to stop and consider how this would
help them too.

> > What about hardcoding #!/usr/bin/python3.6? This would need correcting
> > against 3.7 but would fall foul of the diagnostic when using 3.6.  
> 
> No, it would fail as mismatched shebang.  By design.

Okay, I only thought of this case while writing the mail, having
forgotten the precise logic since working on it. It doesn't take
anything away from the point above though.

> python_fix_shebang is meant to fix common cases of generic shebangs
> in a safe way, not serve as a generic shebang replacement tool.

Allow me to suggest some options for a compromise. We can't just make
the diagnostic conditional on cross/prefix cases as it is the other
cases where it would blow up. This leaves us with:

(a) In cross/prefix cases only, call python_fix_shebang on all files
that look like they have a Python-like shebang in pkg_postinst.
Unfortunately the Python eclasses don't currently export a pkg_postinst
phase function so existing ebuilds using other eclasses may need
fixing. These files could be efficiently found with help from awk using
something along the lines of this:

find "${D}" -type f -exec awk '/^#!.*python/ {print FILENAME} {nextfile}' {} +

(b) Same as (a) but I'll invoke this from cross-boss using bashrc
instead of relying on the eclasses. Prefix would need to do its own thing.

(c) Create a second variant of python_fix_shebang (name TBD) that would
only do something in cross/prefix cases. This would be called against
the files that only fail in these cases. It would have to be gradually
added to ebuilds as problems are found but that was also the case in my
original proposal.

-- 
James Le Cuirot (chewi)
Gentoo Linux Developer

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

Re: [gentoo-dev] [PATCH 02/12] python-utils-r1.eclass: New python_fix_shebang approach

[James Le Cuirot]

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

On Fri, 04 Jan 2019 07:02:40 +0100
Michał Górny <mgorny@gentoo.org> wrote:

> On Thu, 2019-01-03 at 21:39 +0000, James Le Cuirot wrote:
> > The previous approach would erroneously match foopython. The new
> > approach requires the match to start the string or be preceeded by a
> > slash, the only two cases we actually want. It does this with slightly
> > less code and allows the replacement of whole path strings that would
> > be problematic when passed to sed. This will be needed when
> > cross-compiling is addressed.
> > 
> > Signed-off-by: James Le Cuirot <chewi@gentoo.org>
> > ---
> >  eclass/python-utils-r1.eclass | 78 ++++++++++++++---------------------
> >  1 file changed, 31 insertions(+), 47 deletions(-)
> > 
> > diff --git a/eclass/python-utils-r1.eclass b/eclass/python-utils-r1.eclass
> > index da76a755fb34..1eca0764a202 100644
> > --- a/eclass/python-utils-r1.eclass
> > +++ b/eclass/python-utils-r1.eclass
> > @@ -1165,8 +1165,7 @@ python_fix_shebang() {
> >  		[[ -d ${path} ]] && is_recursive=1
> >  
> >  		while IFS= read -r -d '' f; do
> > -			local shebang i
> > -			local error= from=
> > +			local shebang i= error= fix=
> >  
> >  			# note: we can't ||die here since read will fail if file
> >  			# has no newline characters
> > @@ -1175,65 +1174,56 @@ python_fix_shebang() {
> >  			# First, check if it's shebang at all...
> >  			if [[ ${shebang} == '#!'* ]]; then
> >  				local split_shebang=()
> > -				read -r -a split_shebang <<<${shebang} || die
> > +				read -r -a split_shebang <<<${shebang#\#\!} || die  
> 
> Does '!' really need to be escaped?

Seemingly only in an interactive shell.

> >  
> >  				# Match left-to-right in a loop, to avoid matching random
> >  				# repetitions like 'python2.7 python2'.
> > -				for i in "${split_shebang[@]}"; do
> > -					case "${i}" in
> > -						*"${EPYTHON}")
> > +				for i in $(seq 0 $((${#split_shebang[@]} - 1))); do  
> 
> for i in "${!split_shebang[@]}"; do

Interesting, didn't know about that.

> > +					case "/${split_shebang[${i}]}" in  
> 
> case "/${split_shebang[i]}" in

...or that.

> Also below.
> 
> > +						*/${EPYTHON})
> >  							debug-print "${FUNCNAME}: in file ${f#${D%/}}"
> >  							debug-print "${FUNCNAME}: shebang matches EPYTHON: ${shebang}"
> >  
> >  							# Nothing to do, move along.
> >  							any_correct=1
> > -							from=${EPYTHON}
> > +							continue 2
> > +							;;
> > +						*/python)
> > +							fix=1
> >  							break
> >  							;;
> > -						*python|*python[23])
> > -							debug-print "${FUNCNAME}: in file ${f#${D%/}}"
> > -							debug-print "${FUNCNAME}: rewriting shebang: ${shebang}"
> > -
> > -							if [[ ${i} == *python2 ]]; then
> > -								from=python2
> > -								if [[ ! ${force} ]]; then
> > -									python_is_python3 "${EPYTHON}" && error=1
> > -								fi
> > -							elif [[ ${i} == *python3 ]]; then
> > -								from=python3
> > -								if [[ ! ${force} ]]; then
> > -									python_is_python3 "${EPYTHON}" || error=1
> > -								fi
> > -							else
> > -								from=python
> > +						*/python2)
> > +							if [[ ! ${force} ]]; then
> > +								python_is_python3 "${EPYTHON}" && error=1
> >  							fi
> > +							fix=1
> >  							break
> >  							;;
> > -						*python[23].[0123456789]|*pypy|*pypy3|*jython[23].[0123456789])
> > -							# Explicit mismatch.
> > +						*/python3)
> >  							if [[ ! ${force} ]]; then
> > -								error=1
> > -							else
> > -								case "${i}" in
> > -									*python[23].[0123456789])
> > -										from="python[23].[0123456789]";;
> > -									*pypy)
> > -										from="pypy";;
> > -									*pypy3)
> > -										from="pypy3";;
> > -									*jython[23].[0123456789])
> > -										from="jython[23].[0123456789]";;
> > -									*)
> > -										die "${FUNCNAME}: internal error in 2nd pattern match";;
> > -								esac
> > +								python_is_python3 "${EPYTHON}" || error=1
> >  							fi
> > +							fix=1
> > +							break
> > +							;;
> > +						*/python[2-3].[0-9]|*/pypy|*/pypy3|*/jython[2-3].[0-9])
> > +							# Explicit mismatch.
> > +							[[ ! ${force} ]] && error=1
> > +							fix=1
> >  							break
> >  							;;
> >  					esac
> >  				done
> >  			fi
> >  
> > -			if [[ ! ${error} && ! ${from} ]]; then
> > +			if [[ ${fix} ]]; then  
> 
> Doesn't this mean errors from above code will be ignored?  Diff makes it
> really hard to follow the logic here but I'm pretty sure you set both
> error=1 fix=1, then check for fix first.

It will still hit the error clause below regardless. The debug-print
lines were refactored from further up and they were printed even in
error cases. We could skip the split_shebang lines but it would just
make the code longer.

> > +				debug-print "${FUNCNAME}: in file ${f#${D%/}}"
> > +				debug-print "${FUNCNAME}: rewriting shebang: ${shebang}"
> > +
> > +				split_shebang[${i}]=/${split_shebang[${i}]}  
> 
> A comment above this hack would be helpful.

Okay.
 
> > +				split_shebang[${i}]=${split_shebang[${i}]%/*}
> > +				split_shebang[${i}]=${split_shebang[${i}]#/}${split_shebang[${i}]:+/}${EPYTHON}
> > +			elif [[ ! ${error} ]]; then
> >  				# Non-Python shebang. Allowed in recursive mode,
> >  				# disallowed when specifying file explicitly.
> >  				[[ ${is_recursive} ]] && continue
> > @@ -1245,13 +1235,7 @@ python_fix_shebang() {
> >  			fi
> >  
> >  			if [[ ! ${error} ]]; then
> > -				# We either want to match ${from} followed by space
> > -				# or at end-of-string.
> > -				if [[ ${shebang} == *${from}" "* ]]; then
> > -					sed -i -e "1s:${from} :${EPYTHON} :" "${f}" || die
> > -				else
> > -					sed -i -e "1s:${from}$:${EPYTHON}:" "${f}" || die
> > -				fi
> > +				sed -i -e "1c#\!${split_shebang[*]}" "${f}" || die
> >  				any_fixed=1
> >  			else
> >  				eerror "The file has incompatible shebang:"  
> 
> I'll get to other patches later.
> 


-- 
James Le Cuirot (chewi)
Gentoo Linux Developer

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