From cb17408eeb1c52557e8977b8666b3875e50e6a98 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sat, 1 Feb 2025 16:49:15 +0100 Subject: [PATCH] Ban 'assert' and introduce 'pipestatus' instead in EAPI 9 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Starting with EAPI 9, we decided to phase out 'assert', because, for example, its name is confusing. Instead 'assert' is going to be replaced with 'pipestatus'. This also introduces the portage internal __pipestatus helper that can be used in bash code for all EAPIs. The __pipestatus in eapi9-pipestatus.sh was taken from eapi9-pipestatus.eclass, written by Ulrich Müller. Thanks ulm! Bug: https://bugs.gentoo.org/566342 --- bin/eapi.sh | 4 + bin/eapi9-pipestatus.sh | 30 +++++ bin/isolated-functions.sh | 20 +++- bin/misc-functions.sh | 2 +- bin/phase-functions.sh | 6 +- bin/save-ebuild-env.sh | 3 +- lib/portage/tests/bin/meson.build | 1 + .../tests/bin/test_eapi9_pipestatus.py | 107 ++++++++++++++++++ man/ebuild.5 | 37 +++++- 9 files changed, 196 insertions(+), 14 deletions(-) create mode 100644 bin/eapi9-pipestatus.sh create mode 100644 lib/portage/tests/bin/test_eapi9_pipestatus.py diff --git a/bin/eapi.sh b/bin/eapi.sh index 3ea24cbb06..fae735420b 100644 --- a/bin/eapi.sh +++ b/bin/eapi.sh @@ -140,6 +140,10 @@ ___eapi_has_useq() { [[ ${1-${EAPI-0}} =~ ^(0|1|2|3|4|5|6|7)$ ]] } +___eapi_has_pipestatus() { + [[ ${1-${EAPI-0}} =~ ^(0|1|2|3|4|5|6|7|8)$ ]] +} + # HELPERS BEHAVIOR ___eapi_best_version_and_has_version_support_--host-root() { diff --git a/bin/eapi9-pipestatus.sh b/bin/eapi9-pipestatus.sh new file mode 100644 index 0000000000..109c285d88 --- /dev/null +++ b/bin/eapi9-pipestatus.sh @@ -0,0 +1,30 @@ +#!/usr/bin/env bash +# Copyright 2024-2025 Gentoo Authors +# Distributed under the terms of the GNU General Public License v2 + +__pipestatus() { + local status=( "${PIPESTATUS[@]}" ) + local s ret=0 verbose="" + + [[ ${1} == -v ]] && { verbose=1; shift; } + [[ $# -ne 0 ]] && die "usage: ${FUNCNAME} [-v]" + + for s in "${status[@]}"; do + [[ ${s} -ne 0 ]] && ret=${s} + done + + [[ ${verbose} ]] && echo "${status[@]}" + + return "${ret}" +} + +# @FUNCTION: pipestatus +# @USAGE: [-v] +# @RETURN: last non-zero element of PIPESTATUS, or zero if all are zero +# @DESCRIPTION: +# Check the PIPESTATUS array, i.e. the exit status of the command(s) +# in the most recently executed foreground pipeline. If called with +# option -v, also output the PIPESTATUS array. +pipestatus() { + __pipestatus "$@" +} diff --git a/bin/isolated-functions.sh b/bin/isolated-functions.sh index cbd93fce97..c5fbf75dc2 100644 --- a/bin/isolated-functions.sh +++ b/bin/isolated-functions.sh @@ -12,12 +12,20 @@ fi # It _must_ preceed all the calls to die and assert. shopt -s expand_aliases -assert() { - local x pipestatus=${PIPESTATUS[*]} - for x in ${pipestatus} ; do - [[ ${x} -eq 0 ]] || die "$@" - done -} +source "${PORTAGE_BIN_PATH}/eapi9-pipestatus.sh" || exit 1 +if ___eapi_has_pipestatus; then + assert() { + die "assert is banned since EAPI 9 (EAPI=${EAPI}), use pipestatus instead" + } +else + unset -f pipestatus + assert() { + local x pipestatus=${PIPESTATUS[*]} + for x in ${pipestatus} ; do + [[ ${x} -eq 0 ]] || die "$@" + done + } +fi __assert_sigpipe_ok() { # When extracting a tar file like this: diff --git a/bin/misc-functions.sh b/bin/misc-functions.sh index 386e50cc8d..aa1362ec7e 100755 --- a/bin/misc-functions.sh +++ b/bin/misc-functions.sh @@ -540,7 +540,7 @@ __dyn_package() { tar ${tar_options} -cf - ${PORTAGE_BINPKG_TAR_OPTS} -C "${D}" . | \ ${PORTAGE_COMPRESSION_COMMAND} > "${PORTAGE_BINPKG_TMPFILE}" - assert "failed to pack binary package: '${PORTAGE_BINPKG_TMPFILE}'" + __pipestatus || die "failed to pack binary package: '${PORTAGE_BINPKG_TMPFILE}'" PYTHONPATH=${PORTAGE_PYTHONPATH:-${PORTAGE_PYM_PATH}} \ "${PORTAGE_PYTHON:-/usr/bin/python}" "${PORTAGE_BIN_PATH}"/xpak-helper.py recompose \ diff --git a/bin/phase-functions.sh b/bin/phase-functions.sh index b0654533cc..18e6165699 100644 --- a/bin/phase-functions.sh +++ b/bin/phase-functions.sh @@ -716,7 +716,7 @@ __dyn_install() { __save_ebuild_env --exclude-init-phases | __filter_readonly_variables \ --filter-path --filter-sandbox --allow-extra-vars > \ "${PORTAGE_BUILDDIR}"/build-info/environment - assert "__save_ebuild_env failed" + __pipestatus || die "__save_ebuild_env failed" cd "${PORTAGE_BUILDDIR}"/build-info || die ${PORTAGE_BZIP2_COMMAND} -f9 environment @@ -1022,7 +1022,7 @@ __ebuild_main() { __filter_readonly_variables --filter-path \ --filter-sandbox --allow-extra-vars \ | ${PORTAGE_BZIP2_COMMAND} -c -f9 > "${PORTAGE_UPDATE_ENV}" - assert "__save_ebuild_env failed" + __pipestatus || die "__save_ebuild_env failed" fi ;; unpack|prepare|configure|compile|test|clean|install) @@ -1114,7 +1114,7 @@ __ebuild_main() { cd "${PORTAGE_PYM_PATH}" __save_ebuild_env | __filter_readonly_variables \ --filter-features > "${T}/environment" - assert "__save_ebuild_env failed" + __pipestatus || die "__save_ebuild_env failed" chgrp "${PORTAGE_GRPNAME:-portage}" "${T}/environment" chmod g+w "${T}/environment" diff --git a/bin/save-ebuild-env.sh b/bin/save-ebuild-env.sh index 25f885f961..cbca52090f 100644 --- a/bin/save-ebuild-env.sh +++ b/bin/save-ebuild-env.sh @@ -50,7 +50,7 @@ __save_ebuild_env() { done unset x - unset -f assert __assert_sigpipe_ok \ + unset -f assert __assert_sigpipe_ok __pipestatus \ __dump_trace die \ __quiet_mode __vecho __elog_base eqawarn elog \ einfo einfon ewarn eerror ebegin __eend eend KV_major \ @@ -91,6 +91,7 @@ __save_ebuild_env() { ___eapi_has_eapply && unset -f eapply eapply_user ___eapi_has_in_iuse && unset -f in_iuse ___eapi_has_version_functions && unset -f ver_cut ver_rs ver_test + ___eapi_has_pipestatus && -f pipestatus # Clear out the triple underscore namespace as it is reserved by the PM. while IFS=' ' read -r _ _ REPLY; do diff --git a/lib/portage/tests/bin/meson.build b/lib/portage/tests/bin/meson.build index 519972f0a8..c267234b2b 100644 --- a/lib/portage/tests/bin/meson.build +++ b/lib/portage/tests/bin/meson.build @@ -5,6 +5,7 @@ py.install_sources( 'test_dodir.py', 'test_doins.py', 'test_eapi7_ver_funcs.py', + 'test_eapi9_pipestatus.py', 'test_filter_bash_env.py', '__init__.py', '__test__.py', diff --git a/lib/portage/tests/bin/test_eapi9_pipestatus.py b/lib/portage/tests/bin/test_eapi9_pipestatus.py new file mode 100644 index 0000000000..71dde1e88f --- /dev/null +++ b/lib/portage/tests/bin/test_eapi9_pipestatus.py @@ -0,0 +1,107 @@ +# Copyright 2018 Gentoo Foundation +# Distributed under the terms of the GNU General Public License v2 + +import subprocess +import tempfile +import itertools + +from portage.const import PORTAGE_BIN_PATH +from portage.tests import TestCase + + +class TestEAPI9Pipestatus(TestCase): + test_script_prelude = """\ +tps() { + local cmd=${1} + eval "${cmd}; pipestatus" + echo $? +} + +tpsv() { + local cmd=${1} + local out ret + out=$(eval "${cmd}; pipestatus -v") + ret=$? + echo $ret $out +} + +ret() { + return ${1} +} + +die() { + if [[ $@ ]]; then + 2>&1 echo "$@" + fi + exit 42 +} +""" + + def _test_pipestatus(self, test_cases): + with tempfile.NamedTemporaryFile("w") as test_script: + test_script.write(f'source "{PORTAGE_BIN_PATH}"/eapi9-pipestatus.sh\n') + test_script.write(self.test_script_prelude) + for cmd, _ in test_cases: + test_script.write(f"{cmd}\n") + + test_script.flush() + + s = subprocess.Popen( + ["bash", test_script.name], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + sout, serr = s.communicate() + self.assertEqual(s.returncode, 0) + for test_case, result in zip(test_cases, sout.decode().splitlines()): + cmd, exp = test_case + self.assertEqual( + result, exp, f"{cmd} -> '{result}' but expected: {exp}" + ) + + def test_pipestatus(self): + test_cases = [ + # (command, expected exit status) + ("true", 0), + ("false", 1), + ("true | true", 0), + ("false | true", 1), + ("true | false", 1), + ("ret 2 | true", 2), + ("true | false | true", 1), + ("true |false | ret 5 | true", 5), + ] + test_cases = itertools.starmap(lambda a, b: (f"tps {a}", f"{b}"), test_cases) + self._test_pipestatus(test_cases) + + def test_pipestatus_v(self): + test_cases = [ + # (command, expected exit status, expected output) + ("true | true | true", 0, "0 0 O"), + ("false | true", 1, "1 0"), + ("ret 3 | ret 2 | true", 2, "3 2 0"), + ] + test_cases = itertools.starmap( + lambda a, b, c: (f"tpsv {a}", f"{b} {c}"), test_cases + ) + self._test_pipestatus(test_cases) + + def test_pipestatus_xfail(self): + test_cases = [ + "pipestatus bad_arg", + "pipestatus -v extra_arg", + ] + for cmd in test_cases: + with tempfile.NamedTemporaryFile("w") as test_script: + test_script.write(f'source "{PORTAGE_BIN_PATH}"/eapi9-pipestatus.sh\n') + test_script.write(self.test_script_prelude) + test_script.write(f"{cmd}\n") + + test_script.flush() + s = subprocess.Popen( + ["bash", test_script.name], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + s.wait() + self.assertEqual(s.returncode, 42) diff --git a/man/ebuild.5 b/man/ebuild.5 index b83ea46e17..c82161e10a 100644 --- a/man/ebuild.5 +++ b/man/ebuild.5 @@ -1085,10 +1085,41 @@ default_src_test .SS "General:" .TP +.B pipestatus\fR \fI[-v] +Since \fBEAPI 9\fR: Checks the PIPESTATUS array, i.e., the exit status +of the command(s) in the most recently executed foreground pipeline. +Returns the last non-zero-element of PIPESTATUS, or zero if all are +zero. If called with option -v, also outputs the PIPESTATUS array. +In its simplest form it can be called like this: +.RS +.TP +.I Example: +.nf +foo | bar +pipestatus || die +.fi +.RE +.IP +Using the -v option allows to save the value of the PIPESTATUS array +for later diagnostics: +.RS +.TP +.I Example: +.nf +local status +foo | bar +status=$(pipestatus -v) || die "foo | bar failed, PIPESTATUS: ${status}" +.fi +.RE +.IP +Note that pipestatus must be the next command following the pipeline. +In particular, the "local status" declaration must be before the +pipeline, otherwise it would reset PIPESTATUS. +.TP .B assert\fR \fI[reason] -Checks the value of the shell's PIPESTATUS array variable, and if any -component is non-zero (indicating failure), calls die with \fIreason\fR -as a failure message. +Until \fBEAPI 8\fR. Checks the value of the shell's PIPESTATUS array +variable, and if any component is non-zero (indicating failure), calls +die with \fIreason\fR as a failure message. .TP .B die\fR \fI[reason] Causes the current emerge process to be aborted. The final display will