commit: 5a34ca0e2001d70bc2037fc2226ad510e26a3349
Author: Kerin Millar <kfm <AT> plushkava <DOT> net>
AuthorDate: Thu Aug 8 01:27:52 2024 +0000
Commit: Sam James <sam <AT> gentoo <DOT> org>
CommitDate: Sun Aug 11 10:10:58 2024 +0000
URL:
https://gitweb.gentoo.org/proj/gentoo-functions.git/commit/?id=5a34ca0e
Avoid unspecified behaviour around simple commands in general
As mentioned by the previous commit, the Shell Command Language leaves
it unspecified as to whether variable assignments affecting the
execution environment of a simple command charged with executing a
function (that is not the implementation of a standard utility) shall
persist after the completion of the function.
It transpires that modifying gentoo-functions so as to steer clear of
this pitfall isn't particularly difficult so this commit does exactly
that. Most of the changes are in test-functions but functions/rc.sh also
required some minor changes regarding the use of the GENFUN_CALLER
variable.
With this, loksh very nearly passes the test suite. There is one
individual test that continues to fail, although it looks as though that
may be caused by a genuine bug on the part of the shell. That will
require investigating in its own right.
Signed-off-by: Kerin Millar <kfm <AT> plushkava.net>
Signed-off-by: Sam James <sam <AT> gentoo.org>
functions/rc.sh | 16 +++++++++++-----
test-functions | 43 ++++++++++++++++++++++++-------------------
2 files changed, 35 insertions(+), 24 deletions(-)
diff --git a/functions/rc.sh b/functions/rc.sh
index 12444d1..0c14035 100644
--- a/functions/rc.sh
+++ b/functions/rc.sh
@@ -49,7 +49,8 @@ ebegin()
#
eend()
{
- GENFUN_CALLER=${GENFUN_CALLER:-eend} _eend eerror "$@"
+ : "${genfun_caller:=eend}"
+ _eend eerror "$@"
}
#
@@ -161,7 +162,8 @@ ewarnn()
#
ewend()
{
- GENFUN_CALLER=${GENFUN_CALLER:-ewend} _eend ewarn "$@"
+ : "${genfun_caller:=ewend}"
+ _eend ewarn "$@"
}
#
@@ -240,7 +242,8 @@ done
veend()
{
if yesno "${EINFO_VERBOSE}"; then
- GENFUN_CALLER=veend eend "$@"
+ genfun_caller=veend
+ eend "$@"
elif [ "$#" -gt 0 ] && { ! is_int "$1" || [ "$1" -lt 0 ]; }; then
_warn_for_args veend "$1"
false
@@ -252,7 +255,8 @@ veend()
vewend()
{
if yesno "${EINFO_VERBOSE}"; then
- GENFUN_CALLER=vewend ewend "$@"
+ genfun_caller=vewend
+ ewend "$@"
elif [ "$#" -gt 0 ] && { ! is_int "$1" || [ "$1" -lt 0 ]; }; then
_warn_for_args vewend "$1"
false
@@ -311,7 +315,7 @@ _eend()
if [ "$#" -eq 0 ]; then
retval=0
elif ! is_int "$1" || [ "$1" -lt 0 ]; then
- _warn_for_args "${GENFUN_CALLER}" "$1"
+ _warn_for_args "${genfun_caller}" "$1"
retval=1
msg=
else
@@ -320,6 +324,8 @@ _eend()
msg=$*
fi
+ genfun_caller=
+
if [ "${retval}" -ne 0 ]; then
# If a message was given, print it with the specified function.
if _is_visible "${msg}"; then
diff --git a/test-functions b/test-functions
index afc56eb..0b0e127 100755
--- a/test-functions
+++ b/test-functions
@@ -46,19 +46,6 @@ test_local() {
return "${retval}"
}
-test_simple_command() {
- f() { :; }
- LEAKED=
- LEAKED=1 f
- retval=0
- if [ "${LEAKED}" ]; then
- printf 'not '
- retval=1
- fi
- printf "ok %d - /bin/sh refrains from leaking environmental changes for
simple commands\\n" "$((testnum += 1))"
- return "${retval}"
-}
-
test_chdir() {
set -- \
ge 1 '' \
@@ -76,13 +63,17 @@ test_chdir() {
fi
callback() {
+ local CDPATH var
+
shift
test_description="chdir $(quote_args "$@")"
if [ "$BASH" ]; then
# shellcheck disable=3044
shopt -s cdable_vars
fi
- CDPATH=child var=child chdir "$@" \
+ CDPATH=child
+ var=child
+ chdir "$@" \
&& test "$PWD" != "$OLDPWD" \
&& cd - >/dev/null
}
@@ -266,7 +257,7 @@ test_esyslog() {
should_log=$2
shift 2
test_description="esyslog $(quote_args "$@")"
- logged=$(EINFO_LOG=1 esyslog "$@")
+ logged=$(EINFO_LOG=1; esyslog "$@")
case $? in
0)
test "${logged:-0}" -eq "${should_log}"
@@ -680,10 +671,22 @@ test_whenceforth() {
whenceforth "$@" >/dev/null
else
test_description="PATH=${path} whenceforth $(quote_args
"$@")"
- PATH=${path} whenceforth "$@" >/dev/null
+ (
+ # If necessary, conduct the test with a printf
+ # function in effect, duly covering shells that
+ # do not implement it as a builtin. Otherwise,
+ # it could become unavailable on account of the
+ # various values of PATH being tested.
+ case ${printf_cmd} in
+ /*) printf() { "${printf_cmd}" "$@"; }
+ esac
+ PATH=${path}
+ whenceforth "$@" >/dev/null
+ )
fi
}
+ printf_cmd=$(command -v printf)
iterate_tests 5 "$@"
}
@@ -923,6 +926,8 @@ test_contains_any() {
}
test_quote_args() {
+ local POSIXLY_CORRECT
+
testnum=$((testnum + 1))
retval=0
i=0
@@ -930,7 +935,7 @@ test_quote_args() {
fmt=$(printf '\\%o' "$i")
# shellcheck disable=2059
str=$(printf "$fmt.")
- POSIXLY_CORRECT= quote_args "${str%.}" || break
+ quote_args "${str%.}" || break
done | cksum | {
read -r cksum _
if [ "${cksum}" != "380900690" ]; then
@@ -1016,7 +1021,7 @@ test_update_time() {
shift
if [ "$1" ]; then
test_description="LC_ALL=$1 _update_time"
- LC_ALL=$1 genfun_time=$(_update_time && printf %s
"${genfun_time}")
+ genfun_time=$(LC_ALL=$1; _update_time && printf %s
"${genfun_time}")
else
test_description="_update_time"
genfun_time=$(_update_time && printf %s
"${genfun_time}")
@@ -1084,7 +1089,7 @@ if [ "${PORTAGE_BIN_PATH}" ] && [ "${S}" ]; then
genfun_basedir=${S}
fi
-if ! test_local || ! test_simple_command; then
+if ! test_local; then
rc=1
elif ! GENFUN_MODULES="portage rc" . ./functions.sh; then
bailout "Couldn't source ./functions.sh"
