Do not export PMS variables in EAPI 9 (or if FEATURES=-export-pms-vars)

Instead of passing PMS variables as part of the process environment,
we pass them via a file that is later sourced by the ebuild's
bash.

Since, for example, A is usually the greatest contributor to the
process environment, removing it from the process environment
significantly avoids running into MAX_ARG_STRLEN when spawning a new
child process.

This means that A and other PMS variables are no longer exported in
the ebuild and hence unavaiable to child processes. However, A is
mostly used as part of the default_src_unpack function and there A
does not need to be exported.

This started as a change that only unexported A, but was later
extended to most PMS variables as suggested by ulm in
https://bugs.gentoo.org/721088#c23.

Thanks to Zac Medico for helpful input on this change, and to Eli
Schwartz for suggesting that (bash) helpers should simply source the
environment file introduced by this change.

Closes: https://bugs.gentoo.org/721088
Signed-off-by: Florian Schmaus <[email protected]>

0001-Do-not-export-PMS-variables-in-EAPI-9-or-if-FEATURES.patch

@@ -0,0 +1,304 @@
+From b033faf8170057e17d1f237b032a537356c30434 Mon Sep 17 00:00:00 2001
+From: Florian Schmaus <[email protected]>
+Date: Tue, 2 Apr 2024 22:28:30 +0200
+Subject: [PATCH] Do not export PMS variables in EAPI 9 (or if
+ FEATURES=-export-pms-vars)
+
+Instead of passing PMS variables as part of the process environment,
+we pass them via a file that is later sourced by the ebuild's
+bash.
+
+Since, for example, A is usually the greatest contributor to the
+process environment, removing it from the process environment
+significantly avoids running into MAX_ARG_STRLEN when spawning a new
+child process.
+
+This means that A and other PMS variables are no longer exported in
+the ebuild and hence unavaiable to child processes. However, A is
+mostly used as part of the default_src_unpack function and there A
+does not need to be exported.
+
+This started as a change that only unexported A, but was later
+extended to most PMS variables as suggested by ulm in
+https://bugs.gentoo.org/721088#c23.
+
+Thanks to Zac Medico for helpful input on this change, and to Eli
+Schwartz for suggesting that (bash) helpers should simply source the
+environment file introduced by this change.
+
+Closes: https://bugs.gentoo.org/721088
+Signed-off-by: Florian Schmaus <[email protected]>
+---
+ bin/isolated-functions.sh              |   9 ++
+ bin/phase-functions.sh                 |   8 ++
+ cnf/make.globals                       |   2 +-
+ lib/portage/const.py                   |   1 +
+ lib/portage/eapi.py                    |   8 ++
+ lib/portage/package/ebuild/doebuild.py | 110 ++++++++++++++++++++++++-
+ 6 files changed, 136 insertions(+), 2 deletions(-)
+
+diff --git a/bin/isolated-functions.sh b/bin/isolated-functions.sh
+index cbd93fce9763..d4248366d7b6 100644
+--- a/bin/isolated-functions.sh
++++ b/bin/isolated-functions.sh
+@@ -8,6 +8,15 @@ if ___eapi_has_version_functions; then
+ 	source "${PORTAGE_BIN_PATH}/eapi7-ver-funcs.sh" || exit 1
+ fi
+
++if [[ -v PORTAGE_EBUILD_EXTRA_SOURCE ]]; then
++	source "${PORTAGE_EBUILD_EXTRA_SOURCE}" || exit 1
++	# We deliberately do not unset PORTABE_EBUILD_EXTRA_SOURCE, so
++	# that it keeps being exported in the environment of this
++	# process and its child processes. There, for example portage
++	# helper like doins, can pick it up and set the PMS variables
++	# (usually by sourcing isolated-functions.sh).
++fi
++
+ # We need this next line for "die" and "assert". It expands
+ # It _must_ preceed all the calls to die and assert.
+ shopt -s expand_aliases
+diff --git a/bin/phase-functions.sh b/bin/phase-functions.sh
+index d9b524c1a2da..3f62804158c2 100644
+--- a/bin/phase-functions.sh
++++ b/bin/phase-functions.sh
+@@ -20,6 +20,7 @@ PORTAGE_READONLY_VARS="D EBUILD EBUILD_PHASE EBUILD_PHASE_FUNC \
+ 	PORTAGE_BUILD_USER PORTAGE_BUNZIP2_COMMAND \
+ 	PORTAGE_BZIP2_COMMAND PORTAGE_COLORMAP PORTAGE_CONFIGROOT \
+ 	PORTAGE_DEBUG PORTAGE_DEPCACHEDIR PORTAGE_EBUILD_EXIT_FILE \
++	PORTAGE_EBUILD_EXTRA_SOURCE \
+ 	PORTAGE_ECLASS_LOCATIONS PORTAGE_EXPLICIT_INHERIT \
+ 	PORTAGE_GID PORTAGE_GRPNAME PORTAGE_INST_GID PORTAGE_INST_UID \
+ 	PORTAGE_INTERNAL_CALLER PORTAGE_IPC_DAEMON PORTAGE_IUSE PORTAGE_LOG_FILE \
+@@ -962,6 +963,13 @@ __ebuild_main() {
+ 	export EBUILD_MASTER_PID=${BASHPID:-$(__bashpid)}
+ 	trap 'exit 1' SIGTERM
+
++	if [[ -v PORTAGE_EBUILD_EXTRA_SOURCE && ${PORTAGE_EBUILD_EXTRA_SOURCE} != ${T}/* ]]; then
++		# Cleanup PORTAGE_EBUILD_EXTRA_SOURCE after ebuild.sh
++		# (__ebuild_main()) finishes if PORTAGE_EBUILD_EXTRA_SOURCE is
++		# not under T.
++		trap "rm \"${PORTAGE_EBUILD_EXTRA_SOURCE}\"" EXIT
++	fi
++
+ 	# A reasonable default for ${S}
+ 	[[ -z ${S} ]] && export S=${WORKDIR}/${P}
+
+diff --git a/cnf/make.globals b/cnf/make.globals
+index 94eac656843c..dba24c73c7b8 100644
+--- a/cnf/make.globals
++++ b/cnf/make.globals
+@@ -81,7 +81,7 @@ FEATURES="assume-digests binpkg-docompress binpkg-dostrip binpkg-logs
+           network-sandbox news parallel-fetch pkgdir-index-trusted pid-sandbox
+           preserve-libs protect-owned qa-unresolved-soname-deps sandbox strict
+           unknown-features-warn unmerge-logs unmerge-orphans userfetch
+-          userpriv usersandbox usersync"
++          userpriv usersandbox usersync export-pms-vars"
+
+ # Ignore file collisions in /lib/modules since files inside this directory
+ # are never unmerged, and therefore collisions must be ignored in order for
+diff --git a/lib/portage/const.py b/lib/portage/const.py
+index c9a71009a765..2049f5131171 100644
+--- a/lib/portage/const.py
++++ b/lib/portage/const.py
+@@ -183,6 +183,7 @@ SUPPORTED_FEATURES = frozenset(
+         "distlocks",
+         "downgrade-backup",
+         "ebuild-locks",
++        "export-pms-vars",
+         "fail-clean",
+         "fakeroot",
+         "fixlafiles",
+diff --git a/lib/portage/eapi.py b/lib/portage/eapi.py
+index bdf60ef101a9..e0a9fc61a49b 100644
+--- a/lib/portage/eapi.py
++++ b/lib/portage/eapi.py
+@@ -48,6 +48,10 @@ def eapi_supports_prefix(eapi: str) -> bool:
+     return _get_eapi_attrs(eapi).prefix
+
+
++def eapi_exports_pms_vars(eapi: str) -> bool:
++    return _get_eapi_attrs(eapi).exports_pms_vars
++
++
+ def eapi_exports_AA(eapi: str) -> bool:
+     return _get_eapi_attrs(eapi).exports_AA
+
+@@ -157,6 +161,7 @@ _eapi_attrs = collections.namedtuple(
+         "exports_ECLASSDIR",
+         "exports_KV",
+         "exports_merge_type",
++        "exports_pms_vars",
+         "exports_PORTDIR",
+         "exports_replace_vars",
+         "feature_flag_test",
+@@ -197,6 +202,7 @@ class Eapi:
+         "6",
+         "7",
+         "8",
++        "9",
+     )
+
+     _eapi_val: int = -1
+@@ -235,6 +241,7 @@ def _get_eapi_attrs(eapi_str: Optional[str]) -> _eapi_attrs:
+             exports_ECLASSDIR=False,
+             exports_KV=False,
+             exports_merge_type=True,
++            exports_pms_vars=True,
+             exports_PORTDIR=True,
+             exports_replace_vars=True,
+             feature_flag_test=False,
+@@ -274,6 +281,7 @@ def _get_eapi_attrs(eapi_str: Optional[str]) -> _eapi_attrs:
+             exports_ECLASSDIR=eapi <= Eapi("6"),
+             exports_KV=eapi <= Eapi("3"),
+             exports_merge_type=eapi >= Eapi("4"),
++            exports_pms_vars=eapi <= Eapi("8"),
+             exports_PORTDIR=eapi <= Eapi("6"),
+             exports_replace_vars=eapi >= Eapi("4"),
+             feature_flag_test=False,
+diff --git a/lib/portage/package/ebuild/doebuild.py b/lib/portage/package/ebuild/doebuild.py
+index 54831ccdae06..111ac47252c9 100644
+--- a/lib/portage/package/ebuild/doebuild.py
++++ b/lib/portage/package/ebuild/doebuild.py
+@@ -84,6 +84,7 @@ from portage.dep.libc import find_libc_deps
+ from portage.eapi import (
+     eapi_exports_KV,
+     eapi_exports_merge_type,
++    eapi_exports_pms_vars,
+     eapi_exports_replace_vars,
+     eapi_has_required_use,
+     eapi_has_src_prepare_and_src_configure,
+@@ -189,6 +190,57 @@ _vdb_use_conditional_keys = Package._dep_keys + (
+     "RESTRICT",
+ )
+
++# The following is a set of PMS § 11.1 and § 7.4 without
++# - TMPDIR
++# - HOME
++# because these variables are often assumed to be exported and
++# therefore consumed by child processes.
++_unexported_pms_vars = frozenset(
++    # fmt: off
++    [
++        # PMS § 11.1 Defined Variables
++        "P",
++        "PF",
++        "PN",
++        "CATEGORY",
++        "PV",
++        "PR",
++        "PVR",
++        "A",
++        "AA",
++        "FILESDIR",
++        "DISTDIR",
++        "WORKDIR",
++        "S",
++        "PORTDIR",
++        "ECLASSDIR",
++        "ROOT",
++        "EROOT",
++        "SYSROOT",
++        "ESYSROOT",
++        "BROOT",
++        "T",
++#        "TMPDIR",        # EXPORTED: often assumed to be exported and available to child processes
++#        "HOME",          # EXPORTED: often assumed to be exported and available to child processes
++        "EPREFIX",
++        "D",
++        "ED",
++        "DESTTREE",
++        "INSDESTTREE",
++        "EBUILD_PHASE",
++        "EBUILD_PHASE_FUNC",
++        "KV",
++        "MERGE_TYPE",
++        "REPLACING_VERSIONS",
++        "REPLACED_BY_VERSION",
++        # PMS § 7.4 Magic Ebuild-defined Variables
++        "ECLASS",
++        "INHERITED",
++        "DEFINED_PHASES",
++    ]
++    # fmt: on
++)
++
+
+ def _doebuild_spawn(phase, settings, actionmap=None, **kwargs):
+     """
+@@ -1922,6 +1974,8 @@ def _validate_deps(mysettings, myroot, mydo, mydbapi):
+ # XXX This would be to replace getstatusoutput completely.
+ # XXX Issue: cannot block execution. Deadlock condition.
+
++_emerge_tmpdir = None
++
+
+ def spawn(
+     mystring,
+@@ -2133,9 +2187,63 @@ def spawn(
+         logname_backup = mysettings.configdict["env"].get("LOGNAME")
+         mysettings.configdict["env"]["LOGNAME"] = logname
+
++    eapi = mysettings["EAPI"]
++
++    unexported_env_vars = None
++    if "export-pms-vars" not in mysettings.features or not eapi_exports_pms_vars(eapi):
++        unexported_env_vars = _unexported_pms_vars
++
++    if unexported_env_vars:
++        # Starting with EAPI 9 (or if FEATURES="-export-pms-vars"),
++        # PMS variables should not longer be exported.
++
++        phase = mysettings.get("EBUILD_PHASE")
++        is_pms_ebuild_phase = phase in _phase_func_map.keys()
++        # 'None' phase is MiscFunctionsProcess, e.g., where the qa checks run
++        is_ebuild_phase_with_t = phase in [None, "package"]
++        # Copy the environment since we are removing the PMS variables from it.
++        env = mysettings.environ().copy()
++        if is_pms_ebuild_phase or is_ebuild_phase_with_t:
++            t = env["T"]
++            ebuild_extra_source_path = os.path.join(
++                t, f".portage-ebuild-extra-source-{phase}"
++            )
++        else:
++            global _emerge_tmpdir
++            if _emerge_tmpdir is None:
++                _emerge_tmpdir = tempfile.mkdtemp(
++                    prefix=f"portage-tmpdir-{portage.getpid()}-"
++                )
++                os.chmod(_emerge_tmpdir, 0o755)
++                portage.process.atexit_register(shutil.rmtree, _emerge_tmpdir)
++            ebuild_extra_source_fd, ebuild_extra_source_path = tempfile.mkstemp(
++                prefix=f"portage-ebuild-extra-source-{phase}-",
++                dir=_emerge_tmpdir,
++            )
++            try:
++                # Make sure that the file can be writen by us (done below)
++                # and that it is world readable.
++                os.fchmod(ebuild_extra_source_fd, 0o644)
++            finally:
++                os.close(ebuild_extra_source_fd)
++
++        with open(ebuild_extra_source_path, mode="w") as f:
++            for var_name in unexported_env_vars:
++                var_value = mysettings.environ().get(var_name)
++                if var_value is None:
++                    continue
++                quoted_var_value = shlex.quote(var_value)
++                f.write(f"{var_name}={quoted_var_value}\n")
++                del env[var_name]
++
++        env["PORTAGE_EBUILD_EXTRA_SOURCE"] = str(ebuild_extra_source_path)
++    else:
++        # Pre EAPI 9 behavior, all PMS variables are simply exported into the ebuild's environment.
++        env = mysettings.environ()
++
+     try:
+         if keywords.get("returnpid") or keywords.get("returnproc"):
+-            return spawn_func(mystring, env=mysettings.environ(), **keywords)
++            return spawn_func(mystring, env=env, **keywords)
+
+         proc = EbuildSpawnProcess(
+             background=False,
+-- 
+2.45.2
+

bin/isolated-functions.sh

@@ -8,6 +8,15 @@ if ___eapi_has_version_functions; then
 	source "${PORTAGE_BIN_PATH}/eapi7-ver-funcs.sh" || exit 1
 fi
 
+if [[ -v PORTAGE_EBUILD_EXTRA_SOURCE ]]; then
+	source "${PORTAGE_EBUILD_EXTRA_SOURCE}" || exit 1
+	# We deliberately do not unset PORTABE_EBUILD_EXTRA_SOURCE, so
+	# that it keeps being exported in the environment of this
+	# process and its child processes. There, for example portage
+	# helper like doins, can pick it up and set the PMS variables
+	# (usually by sourcing isolated-functions.sh).
+fi
+
 # We need this next line for "die" and "assert". It expands
 # It _must_ preceed all the calls to die and assert.
 shopt -s expand_aliases

bin/phase-functions.sh

@@ -20,6 +20,7 @@ PORTAGE_READONLY_VARS="D EBUILD EBUILD_PHASE EBUILD_PHASE_FUNC \
 	PORTAGE_BUILD_USER PORTAGE_BUNZIP2_COMMAND \
 	PORTAGE_BZIP2_COMMAND PORTAGE_COLORMAP PORTAGE_CONFIGROOT \
 	PORTAGE_DEBUG PORTAGE_DEPCACHEDIR PORTAGE_EBUILD_EXIT_FILE \
+	PORTAGE_EBUILD_EXTRA_SOURCE \
 	PORTAGE_ECLASS_LOCATIONS PORTAGE_EXPLICIT_INHERIT \
 	PORTAGE_GID PORTAGE_GRPNAME PORTAGE_INST_GID PORTAGE_INST_UID \
 	PORTAGE_INTERNAL_CALLER PORTAGE_IPC_DAEMON PORTAGE_IUSE PORTAGE_LOG_FILE \
@@ -962,6 +963,18 @@ __ebuild_main() {
 	export EBUILD_MASTER_PID=${BASHPID:-$(__bashpid)}
 	trap 'exit 1' SIGTERM
 
+	if [[ -v PORTAGE_EBUILD_EXTRA_SOURCE &&
+			  ${PORTAGE_EBUILD_EXTRA_SOURCE} != ${T}/* ]]; then
+		# Cleanup PORTAGE_EBUILD_EXTRA_SOURCE after ebuild.sh
+		# (__ebuild_main()) finishes if PORTAGE_EBUILD_EXTRA_SOURCE is
+		# not under T.
+		__portage_ebuild_exit() {
+			rm "${PORTAGE_EBUILD_EXTRA_SOURCE}" ||
+				die "failed to remove PORTAGE_EBUILD_EXTRA_SOURCE file (${PORTAGE_EBUILD_EXTRA_SOURCE})"
+		}
+		trap __portage_ebuild_exit EXIT
+	fi
+
 	# A reasonable default for ${S}
 	[[ -z ${S} ]] && export S=${WORKDIR}/${P}
 

cnf/make.globals

@@ -81,7 +81,7 @@ FEATURES="assume-digests binpkg-docompress binpkg-dostrip binpkg-logs
           network-sandbox news parallel-fetch pkgdir-index-trusted pid-sandbox
           preserve-libs protect-owned qa-unresolved-soname-deps sandbox strict
           unknown-features-warn unmerge-logs unmerge-orphans userfetch
-          userpriv usersandbox usersync"
+          userpriv usersandbox usersync export-pms-vars"
 
 # Ignore file collisions in /lib/modules since files inside this directory
 # are never unmerged, and therefore collisions must be ignored in order for

lib/_emerge/EbuildMetadataPhase.py

@@ -36,6 +36,7 @@ class EbuildMetadataPhase(SubProcess):
         "repo_path",
         "settings",
         "deallocate_config",
+        "portage_ebuild_extra_source",
         "write_auxdb",
     ) + (
         "_eapi",
@@ -165,6 +166,10 @@ def _async_start_done(self, future):
             self.cancel()
             self._was_cancelled()
 
+        self.portage_ebuild_extra_source = self.settings.get(
+            "PORTAGE_EBUILD_EXTRA_SOURCE"
+        )
+
         if self.deallocate_config is not None and not self.deallocate_config.done():
             self.deallocate_config.set_result(self.settings)
 
@@ -191,6 +196,8 @@ def _unregister(self):
         if self._files is not None:
             self.scheduler.remove_reader(self._files.ebuild)
         SubProcess._unregister(self)
+        if self.portage_ebuild_extra_source:
+            os.unlink(self.portage_ebuild_extra_source)
 
     def _async_waitpid_cb(self, *args, **kwargs):
         """

lib/portage/const.py

@@ -183,6 +183,7 @@
         "distlocks",
         "downgrade-backup",
         "ebuild-locks",
+        "export-pms-vars",
         "fail-clean",
         "fakeroot",
         "fixlafiles",