[PATCH] config.environ: delay export of A and AA (bug 720180)

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] config.environ: delay export of A and AA (bug 720180)

Zac Medico-2
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.

Bug: https://bugs.gentoo.org/720180
Signed-off-by: Zac Medico <[hidden email]>
---
 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 | Threaded
Open this post in threaded view
|

Re: [PATCH] config.environ: delay export of A and AA (bug 720180)

Michał Górny-5
On Mon, 2020-05-25 at 21:31 -0700, Zac Medico 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.
>

Please don't.  This will only hide the problem from developers who will
unknowingly commit broken ebuilds and cause users of alternative package
managers to suffer.

--
Best regards,
Michał Górny


signature.asc (631 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] config.environ: delay export of A and AA (bug 720180)

Alec Warner-2
In reply to this post by Zac Medico-2
On Mon, May 25, 2020 at 9:34 PM Zac Medico <[hidden email]> 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 <[hidden email]>
---
 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 | Threaded
Open this post in threaded view
|

Re: [PATCH] config.environ: delay export of A and AA (bug 720180)

Zac Medico-2
On 5/26/20 1:43 AM, Alec Warner wrote:

> On Mon, May 25, 2020 at 9:34 PM Zac Medico <[hidden email]
> <mailto:[hidden email]>> 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?
If we export an environment variable which contains an extremely large
value, then there's a vulnerability in execve which causes it to fail
with an E2BIG error. Since A and AA values can easily grow large enough
to trigger this vulnerability, portage can protect itself from execve
failures by delaying the export until the moment that it hands control
to the ebuild phase.

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

That's correct. Here's a test case which demonstrates the E2BIG error,
and shows that 'export -n A' can suppress it:

$ A=$(dd if=/dev/zero bs=1M count=1 | tr '\0' ' ')
10+0 records in
10+0 records out
10485760 bytes (10 MB, 10 MiB) copied, 0.086557 s, 121 MB/s
$ echo ${#A}
10485760
$ export A
$ ls
bash: /bin/ls: Argument list too long
$ export -n A
$ /bin/echo hello world
hello world

--
Thanks,
Zac


signature.asc (352 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] config.environ: delay export of A and AA (bug 720180)

Zac Medico-2
In reply to this post by Michał Górny-5
On 5/26/20 12:48 AM, Michał Górny wrote:
> On Mon, 2020-05-25 at 21:31 -0700, Zac Medico 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.
>>
>
> Please don't.  This will only hide the problem from developers who will
> unknowingly commit broken ebuilds and cause users of alternative package
> managers to suffer.

We've seen in https://bugs.gentoo.org/719202 that developers can already
do that with existing versions of portage, since the failure can be
dependent on USE configuration.

If we want to proactively prevent this issue, then we need to set a hard
limit on the size of A and abort before we generate the manifest.
--
Thanks,
Zac


signature.asc (352 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] config.environ: delay export of A and AA (bug 720180)

Alec Warner-2
In reply to this post by Zac Medico-2
On Tue, May 26, 2020 at 9:46 AM Zac Medico <[hidden email]> wrote:
On 5/26/20 1:43 AM, Alec Warner wrote:
> On Mon, May 25, 2020 at 9:34 PM Zac Medico <[hidden email]
> <mailto:[hidden email]>> 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?

If we export an environment variable which contains an extremely large
value, then there's a vulnerability in execve which causes it to fail
with an E2BIG error. Since A and AA values can easily grow large enough
to trigger this vulnerability, portage can protect itself from execve
failures by delaying the export until the moment that it hands control
to the ebuild phase.

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

That's correct. Here's a test case which demonstrates the E2BIG error,
and shows that 'export -n A' can suppress it:

I've run similar tests, but I'm less excited by this work around. I think its reasonable to work toward eventually not exporting A and AA (the latter already done in EAPIs > 3). Then when ebuilds encounter problems, we can tell authors "Upgrade to EAPI X" (where X is >3 or >=8; in the potential case of A.) Having a hard limit on A for EAPIs 0-7 and a hard limit on AA for EAPIs 0-3 seems perfectly fine and we should expect authors to refactor (as texlive did) in order to meet the existing API limitations.

Basically unless there are more practical use cases today, the delaying of export seems like a feature no one will use and added complexity. I dunno how large the go-mod use cases are yet; but I myself have not seen any with this many deps.

-A
 

$ A=$(dd if=/dev/zero bs=1M count=1 | tr '\0' ' ')
10+0 records in
10+0 records out
10485760 bytes (10 MB, 10 MiB) copied, 0.086557 s, 121 MB/s
$ echo ${#A}
10485760
$ export A
$ ls
bash: /bin/ls: Argument list too long
$ export -n A
$ /bin/echo hello world
hello world 

--
Thanks,
Zac

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] config.environ: delay export of A and AA (bug 720180)

Ulrich Mueller-2
In reply to this post by Zac Medico-2
>>>>> On Tue, 26 May 2020, Zac Medico wrote:

> On 5/26/20 12:48 AM, Michał Górny wrote:
>> On Mon, 2020-05-25 at 21:31 -0700, Zac Medico 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.
>>
>> Please don't.  This will only hide the problem from developers who will
>> unknowingly commit broken ebuilds and cause users of alternative package
>> managers to suffer.

> We've seen in https://bugs.gentoo.org/719202 that developers can already
> do that with existing versions of portage, since the failure can be
> dependent on USE configuration.

We have seen in bug 719202 that A has exceeded _SC_ARG_MAX which is
128 KiB.

However, your commit message mentions E2BIG which will trigger at a much
larger value, namely 2 MiB. We are far away from reaching that limit in
any ebuild.

These are separate issues (although related), so I think we should make
it very clear about which one we're talking.

Ulrich

signature.asc (517 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] config.environ: delay export of A and AA (bug 720180)

Zac Medico-2
In reply to this post by Alec Warner-2
On 5/26/20 10:16 AM, Alec Warner wrote:

> On Tue, May 26, 2020 at 9:46 AM Zac Medico <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     On 5/26/20 1:43 AM, Alec Warner wrote:
>     > On Mon, May 25, 2020 at 9:34 PM Zac Medico <[hidden email]
>     <mailto:[hidden email]>
>     > <mailto:[hidden email] <mailto:[hidden email]>>> 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?
>
>     If we export an environment variable which contains an extremely large
>     value, then there's a vulnerability in execve which causes it to fail
>     with an E2BIG error. Since A and AA values can easily grow large enough
>     to trigger this vulnerability, portage can protect itself from execve
>     failures by delaying the export until the moment that it hands control
>     to the ebuild phase.
>
>
>     > Is it simply that we don't export these variables on the python side,
>     > and we only use them in the shell portion?
>
>     That's correct. Here's a test case which demonstrates the E2BIG error,
>     and shows that 'export -n A' can suppress it:
>
>
> I've run similar tests, but I'm less excited by this work around. I
> think its reasonable to work toward eventually not exporting A and AA
> (the latter already done in EAPIs > 3). Then when ebuilds encounter
> problems, we can tell authors "Upgrade to EAPI X" (where X is >3 or >=8;
> in the potential case of A.) Having a hard limit on A for EAPIs 0-7 and
> a hard limit on AA for EAPIs 0-3 seems perfectly fine and we should
> expect authors to refactor (as texlive did) in order to meet the
> existing API limitations.
>
> Basically unless there are more practical use cases today, the delaying
> of export seems like a feature no one will use and added complexity. I
> dunno how large the go-mod use cases are yet; but I myself have not seen
> any with this many deps.
>
> -A

I'm in no rush to merge this patch. I wanted to create it as a proof of
concept, so that we'd be prepared for a future EAPI where the size of A
is practically unlimited. We only have to adjust the ___eapi_exports_A
function to have a working EAPI extension.
--
Thanks,
Zac


signature.asc (352 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] config.environ: delay export of A and AA (bug 720180)

Zac Medico-2
In reply to this post by Ulrich Mueller-2
On 5/26/20 10:32 AM, Ulrich Mueller wrote:

>>>>>> On Tue, 26 May 2020, Zac Medico wrote:
>
>> On 5/26/20 12:48 AM, Michał Górny wrote:
>>> On Mon, 2020-05-25 at 21:31 -0700, Zac Medico 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.
>>>
>>> Please don't.  This will only hide the problem from developers who will
>>> unknowingly commit broken ebuilds and cause users of alternative package
>>> managers to suffer.
>
>> We've seen in https://bugs.gentoo.org/719202 that developers can already
>> do that with existing versions of portage, since the failure can be
>> dependent on USE configuration.
>
> We have seen in bug 719202 that A has exceeded _SC_ARG_MAX which is
> 128 KiB.
>
> However, your commit message mentions E2BIG which will trigger at a much
> larger value, namely 2 MiB. We are far away from reaching that limit in
> any ebuild.
>
> These are separate issues (although related), so I think we should make
> it very clear about which one we're talking.
>
> Ulrich
>
If we want to differentiate between these things then that's fine with me,
however, I have not seen an error other than errno 7 which I thought
corresponded to E2BIG. Test case:

$ python -c "import os, subprocess; os.environ['A'] = 131072 * ' '; subprocess.check_call(['echo', 'hello world'])"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/usr/lib64/python3.6/subprocess.py", line 306, in check_call
    retcode = call(*popenargs, **kwargs)
  File "/usr/lib64/python3.6/subprocess.py", line 287, in call
    with Popen(*popenargs, **kwargs) as p:
  File "/usr/lib64/python3.6/subprocess.py", line 729, in __init__
    restore_signals, start_new_session)
  File "/usr/lib64/python3.6/subprocess.py", line 1364, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
OSError: [Errno 7] Argument list too long: 'echo'
--
Thanks,
Zac


signature.asc (352 bytes) Download Attachment