On Mon, May 25, 2020 at 9:34 PM Zac Medico <zmed...@gentoo.org> wrote:

> Since variables like A and AA can contain extremely large values which
> may trigger E2BIG errors during attempts to execute subprocesses, delay
> export until the last moment, and unexport when appropriate.
>

So I think if you want to do this because PMS says:
 AA should not be visible in EAPI > 3.
 A should only be visible in src_*, pkg_nofetch.

That part of the patch makes sense to me. The part that is confusing to me
is the 'delay' part; can you explain that further? When you say "delay
until the last moment" what do you mean by that and what value is it
delivering?

Is it simply that we don't export these variables on the python side, and
we only use them in the shell portion?

-A


> Bug: https://bugs.gentoo.org/720180
> Signed-off-by: Zac Medico <zmed...@gentoo.org>
> ---
>  bin/eapi.sh                                   |  9 +++++
>  bin/isolated-functions.sh                     | 34 ++++++++++++++++
>  bin/phase-functions.sh                        | 13 +++++++
>  .../ebuild/_config/special_env_vars.py        |  7 +++-
>  lib/portage/package/ebuild/config.py          | 39 ++++++++++++++-----
>  5 files changed, 91 insertions(+), 11 deletions(-)
>
> diff --git a/bin/eapi.sh b/bin/eapi.sh
> index 29dfb008c..f56468e4a 100644
> --- a/bin/eapi.sh
> +++ b/bin/eapi.sh
> @@ -26,6 +26,15 @@ ___eapi_has_S_WORKDIR_fallback() {
>
>  # VARIABLES
>
> +___eapi_exports_A() {
> +       # https://bugs.gentoo.org/721088
> +       true
> +}
> +
> +___eapi_exports_AA() {
> +       [[ ${1-${EAPI-0}} =~ ^(0|1|2|3)$ ]]
> +}
> +
>  ___eapi_has_prefix_variables() {
>         [[ ! ${1-${EAPI-0}} =~ ^(0|1|2)$ || " ${FEATURES} " == *"
> force-prefix "* ]]
>  }
> diff --git a/bin/isolated-functions.sh b/bin/isolated-functions.sh
> index fde684013..973450d86 100644
> --- a/bin/isolated-functions.sh
> +++ b/bin/isolated-functions.sh
> @@ -107,6 +107,39 @@ __bashpid() {
>         sh -c 'echo ${PPID}'
>  }
>
> +# @FUNCTION: ___eapi_vars_export
> +# @DESCRIPTION:
> +# Export variables for the current EAPI. Calls to this function should
> +# be delayed until the last moment, since exporting these variables
> +# may trigger E2BIG errors suring attempts to execute subprocesses.
> +___eapi_vars_export() {
> +       source "${T}/environment.unexported" || die
> +
> +       if ___eapi_exports_A; then
> +               export A
> +       fi
> +
> +       if ___eapi_exports_AA; then
> +               export AA
> +       fi
> +}
> +
> +# @FUNCTION: ___eapi_vars_unexport
> +# @DESCRIPTION:
> +# Unexport variables that were exported for the current EAPI. This
> +# function should be called after an ebuild phase, in order to unexport
> +# variables that may trigger E2BIG errors during attempts to execute
> +# subprocesses.
> +___eapi_vars_unexport() {
> +       if ___eapi_exports_A; then
> +               export -n A
> +       fi
> +
> +       if ___eapi_exports_AA; then
> +               export -n AA
> +       fi
> +}
> +
>  __helpers_die() {
>         if ___eapi_helpers_can_die && [[ ${PORTAGE_NONFATAL} != 1 ]]; then
>                 die "$@"
> @@ -122,6 +155,7 @@ die() {
>
>         set +x # tracing only produces useless noise here
>         local IFS=$' \t\n'
> +       ___eapi_vars_unexport
>
>         if ___eapi_die_can_respect_nonfatal && [[ $1 == -n ]]; then
>                 shift
> diff --git a/bin/phase-functions.sh b/bin/phase-functions.sh
> index 90e622e75..df2c0d8de 100644
> --- a/bin/phase-functions.sh
> +++ b/bin/phase-functions.sh
> @@ -146,6 +146,7 @@ __filter_readonly_variables() {
>                 fi
>         fi
>
> +       ___eapi_vars_unexport
>         "${PORTAGE_PYTHON:-/usr/bin/python}"
> "${PORTAGE_BIN_PATH}"/filter-bash-environment.py "${filtered_vars}" || die
> "filter-bash-environment.py failed"
>  }
>
> @@ -212,6 +213,7 @@ __ebuild_phase() {
>
>  __ebuild_phase_with_hooks() {
>         local x phase_name=${1}
> +       ___eapi_vars_export
>         for x in {pre_,,post_}${phase_name} ; do
>                 __ebuild_phase ${x}
>         done
> @@ -223,6 +225,7 @@ __dyn_pretend() {
>                 __vecho ">>> Remove '$PORTAGE_BUILDDIR/.pretended' to
> force pretend."
>                 return 0
>         fi
> +       ___eapi_vars_export
>         __ebuild_phase pre_pkg_pretend
>         __ebuild_phase pkg_pretend
>         >> "$PORTAGE_BUILDDIR/.pretended" || \
> @@ -236,6 +239,7 @@ __dyn_setup() {
>                 __vecho ">>> Remove '$PORTAGE_BUILDDIR/.setuped' to force
> setup."
>                 return 0
>         fi
> +       ___eapi_vars_export
>         __ebuild_phase pre_pkg_setup
>         __ebuild_phase pkg_setup
>         >> "$PORTAGE_BUILDDIR/.setuped" || \
> @@ -252,6 +256,7 @@ __dyn_unpack() {
>                 install -m${PORTAGE_WORKDIR_MODE:-0700} -d "${WORKDIR}" ||
> die "Failed to create dir '${WORKDIR}'"
>         fi
>         cd "${WORKDIR}" || die "Directory change failed: \`cd
> '${WORKDIR}'\`"
> +       ___eapi_vars_export
>         __ebuild_phase pre_src_unpack
>         __vecho ">>> Unpacking source..."
>         __ebuild_phase src_unpack
> @@ -386,6 +391,7 @@ __dyn_prepare() {
>
>         trap __abort_prepare SIGINT SIGQUIT
>
> +       ___eapi_vars_export
>         __ebuild_phase pre_src_prepare
>         __vecho ">>> Preparing source in $PWD ..."
>         __ebuild_phase src_prepare
> @@ -423,6 +429,7 @@ __dyn_configure() {
>
>         trap __abort_configure SIGINT SIGQUIT
>
> +       ___eapi_vars_export
>         __ebuild_phase pre_src_configure
>
>         __vecho ">>> Configuring source in $PWD ..."
> @@ -456,6 +463,7 @@ __dyn_compile() {
>
>         trap __abort_compile SIGINT SIGQUIT
>
> +       ___eapi_vars_export
>         __ebuild_phase pre_src_compile
>
>         __vecho ">>> Compiling source in $PWD ..."
> @@ -500,6 +508,7 @@ __dyn_test() {
>         else
>                 local save_sp=${SANDBOX_PREDICT}
>                 addpredict /
> +               ___eapi_vars_export
>                 __ebuild_phase pre_src_test
>
>                 __vecho ">>> Test phase: ${CATEGORY}/${PF}"
> @@ -553,6 +562,7 @@ __dyn_install() {
>         eval "[[ -n \$QA_PRESTRIPPED_${ARCH/-/_} ]] && \
>                 export QA_PRESTRIPPED_${ARCH/-/_}"
>
> +       ___eapi_vars_export
>         __ebuild_phase pre_src_install
>
>         if ___eapi_has_prefix_variables; then
> @@ -695,6 +705,7 @@ __dyn_install() {
>                 --filter-path --filter-sandbox --allow-extra-vars > \
>                 "${PORTAGE_BUILDDIR}"/build-info/environment
>         assert "__save_ebuild_env failed"
> +       ___eapi_vars_unexport
>         cd "${PORTAGE_BUILDDIR}"/build-info || die
>
>         ${PORTAGE_BZIP2_COMMAND} -f9 environment
> @@ -1087,11 +1098,13 @@ __ebuild_main() {
>                 __save_ebuild_env | __filter_readonly_variables \
>                         --filter-features > "$T/environment"
>                 assert "__save_ebuild_env failed"
> +               ___eapi_vars_unexport
>                 chgrp "${PORTAGE_GRPNAME:-portage}" "$T/environment"
>                 chmod g+w "$T/environment"
>         fi
>         [[ -n $PORTAGE_EBUILD_EXIT_FILE ]] && > "$PORTAGE_EBUILD_EXIT_FILE"
>         if [[ -n $PORTAGE_IPC_DAEMON ]] ; then
> +               ___eapi_vars_unexport
>                 [[ ! -s $SANDBOX_LOG ]]
>                 "$PORTAGE_BIN_PATH"/ebuild-ipc exit $?
>         fi
> diff --git a/lib/portage/package/ebuild/_config/special_env_vars.py
> b/lib/portage/package/ebuild/_config/special_env_vars.py
> index 440dd00b2..92824e15f 100644
> --- a/lib/portage/package/ebuild/_config/special_env_vars.py
> +++ b/lib/portage/package/ebuild/_config/special_env_vars.py
> @@ -89,7 +89,7 @@ environ_whitelist += [
>  ]
>
>  environ_whitelist += [
> -       "A", "AA", "CATEGORY", "P", "PF", "PN", "PR", "PV", "PVR"
> +       "CATEGORY", "P", "PF", "PN", "PR", "PV", "PVR"
>  ]
>
>  # misc variables inherited from the calling environment
> @@ -124,6 +124,10 @@ environ_whitelist = frozenset(environ_whitelist)
>
>  environ_whitelist_re = re.compile(r'^(CCACHE_|DISTCC_).*')
>
> +environ_unexported = frozenset([
> +       'A', 'AA',
> +])
> +
>  # Filter selected variables in the config.environ() method so that
>  # they don't needlessly propagate down into the ebuild environment.
>  environ_filter = []
> @@ -131,6 +135,7 @@ environ_filter = []
>  # Exclude anything that could be extremely long here (like SRC_URI)
>  # since that could cause execve() calls to fail with E2BIG errors. For
>  # example, see bug #262647.
> +environ_filter.extend(environ_unexported)
>  environ_filter += [
>         'DEPEND', 'RDEPEND', 'PDEPEND', 'SRC_URI',
>  ]
> diff --git a/lib/portage/package/ebuild/config.py
> b/lib/portage/package/ebuild/config.py
> index 47c180c12..a386dc031 100644
> --- a/lib/portage/package/ebuild/config.py
> +++ b/lib/portage/package/ebuild/config.py
> @@ -10,6 +10,7 @@ __all__ = [
>  import copy
>  from itertools import chain
>  import grp
> +import io
>  import logging
>  import platform
>  import pwd
> @@ -29,7 +30,7 @@ portage.proxy.lazyimport.lazyimport(globals(),
>         'portage.util.locale:check_locale,split_LC_ALL',
>  )
>  from portage import bsd_chflags, \
> -       load_mod, os, selinux, _unicode_decode
> +       load_mod, os, selinux, _encodings, _unicode_decode
>  from portage.const import CACHE_PATH, \
>         DEPCACHE_PATH, INCREMENTALS, MAKE_CONF_FILE, \
>         MODULES_FILE_PATH, PORTAGE_BASE_PATH, \
> @@ -2755,14 +2756,36 @@ class config(object):
>                 eapi_attrs = _get_eapi_attrs(eapi)
>                 phase = self.get('EBUILD_PHASE')
>                 emerge_from = self.get('EMERGE_FROM')
> +               temp_dir = None
>                 filter_calling_env = False
>                 if self.mycpv is not None and \
> -                       not (emerge_from == 'ebuild' and phase == 'setup')
> and \
> +                       'PORTAGE_BUILDDIR_LOCKED' in self and \
>                         phase not in ('clean', 'cleanrm', 'depend',
> 'fetch'):
> -                       temp_dir = self.get('T')
> -                       if temp_dir is not None and \
> -                               os.path.exists(os.path.join(temp_dir,
> 'environment')):
> -                               filter_calling_env = True
> +                       temp_dir = self['T']
> +                       # These variables will exported by ebuild.sh if
> appropriate
> +                       # for the current EAPI, but export is delayed
> since large
> +                       # values may trigger E2BIG errors during attempts
> to spawn
> +                       # subprocesses.
> +                       unexported = []
> +                       for key in special_env_vars.environ_unexported:
> +                               # Don't export AA for EAPIs that forbid it.
> +                               if key == 'AA' and not
> eapi_exports_AA(eapi):
> +                                       continue
> +                               value = self.get(key)
> +                               if value is not None:
> +                                       unexported.append((key, value))
> +
> +                       # Write this file even if it's empty, so that
> ebuild.sh can
> +                       # rely on its existence.
> +                       with io.open(os.path.join(temp_dir,
> 'environment.unexported'),
> +                               mode='wt',
> encoding=_encodings['repo.content']) as f:
> +                               for key, value in unexported:
> +                                       f.write('%s="%s"\n' % (key,
> value.replace('"', '\\"')))
> +
> +               if temp_dir is not None and \
> +                       not (emerge_from == 'ebuild' and phase == 'setup')
> and \
> +                       os.path.exists(os.path.join(temp_dir,
> 'environment')):
> +                       filter_calling_env = True
>
>                 environ_whitelist = self._environ_whitelist
>                 for x, myvalue in self.iteritems():
> @@ -2805,10 +2828,6 @@ class config(object):
>                 # Filtered by IUSE and implicit IUSE.
>                 mydict["USE"] = self.get("PORTAGE_USE", "")
>
> -               # Don't export AA to the ebuild environment in EAPIs that
> forbid it
> -               if not eapi_exports_AA(eapi):
> -                       mydict.pop("AA", None)
> -
>                 if not eapi_exports_merge_type(eapi):
>                         mydict.pop("MERGE_TYPE", None)
>
> --
> 2.25.3
>
>
>

Reply via email to