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

[James Le Cuirot]

On Fri, 04 Jan 2019 16:53:46 +0100
Michał Górny <mgorny@g.o> 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@g.o>
> > ---
> >  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