Merge branch 'jc/local-extern-shell-rules'
Document and apply workaround for a buggy version of dash that
mishandles "local var=val" construct.
* jc/local-extern-shell-rules:
t1016: local VAR="VAL" fix
t0610: local VAR="VAL" fix
t: teach lint that RHS of 'local VAR=VAL' needs to be quoted
t: local VAR="VAL" (quote ${magic-reference})
t: local VAR="VAL" (quote command substitution)
t: local VAR="VAL" (quote positional parameters)
CodingGuidelines: quote assigned value in 'local var=$val'
CodingGuidelines: describe "export VAR=VAL" rule
diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index ab39509..1d92b2d 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -188,6 +188,22 @@
hopefully nobody starts using "local" before they are reimplemented
in C ;-)
+ - Some versions of shell do not understand "export variable=value",
+ so we write "variable=value" and then "export variable" on two
+ separate lines.
+
+ - Some versions of dash have broken variable assignment when prefixed
+ with "local", "export", and "readonly", in that the value to be
+ assigned goes through field splitting at $IFS unless quoted.
+
+ (incorrect)
+ local variable=$value
+ local variable=$(command args)
+
+ (correct)
+ local variable="$value"
+ local variable="$(command args)"
+
- Use octal escape sequences (e.g. "\302\242"), not hexadecimal (e.g.
"\xc2\xa2") in printf format strings, since hexadecimal escape
sequences are not portable.
diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl
index dd8107c..b2b28c2 100755
--- a/t/check-non-portable-shell.pl
+++ b/t/check-non-portable-shell.pl
@@ -47,6 +47,8 @@
/\bgrep\b.*--file\b/ and err 'grep --file FILE is not portable (use grep -f FILE)';
/\b[ef]grep\b/ and err 'egrep/fgrep obsolescent (use grep -E/-F)';
/\bexport\s+[A-Za-z0-9_]*=/ and err '"export FOO=bar" is not portable (use FOO=bar && export FOO)';
+ /\blocal\s+[A-Za-z0-9_]*=\$([A-Za-z0-9_{]|[(][^(])/ and
+ err q(quote "$val" in 'local var=$val');
/^\s*([A-Z0-9_]+=(\w*|(["']).*?\3)\s+)+(\w+)/ and exists($func{$4}) and
err '"FOO=bar shell_func" assignment extends beyond "shell_func"';
$line = '';
diff --git a/t/lib-parallel-checkout.sh b/t/lib-parallel-checkout.sh
index acaee9c..8324d6c 100644
--- a/t/lib-parallel-checkout.sh
+++ b/t/lib-parallel-checkout.sh
@@ -20,7 +20,7 @@
BUG "too few arguments to test_checkout_workers"
fi &&
- local expected_workers=$1 &&
+ local expected_workers="$1" &&
shift &&
local trace_file=trace-test-checkout-workers &&
diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
index 38a535b..238d492 100755
--- a/t/t0610-reftable-basics.sh
+++ b/t/t0610-reftable-basics.sh
@@ -83,7 +83,7 @@
test_expect_perms () {
local perms="$1"
local file="$2"
- local actual=$(ls -l "$file") &&
+ local actual="$(ls -l "$file")" &&
case "$actual" in
$perms*)
diff --git a/t/t1016-compatObjectFormat.sh b/t/t1016-compatObjectFormat.sh
index 8132cd3..be3206a 100755
--- a/t/t1016-compatObjectFormat.sh
+++ b/t/t1016-compatObjectFormat.sh
@@ -79,7 +79,7 @@
}
del_sigcommit () {
- local delete=$1
+ local delete="$1"
if test "$delete" = "sha256" ; then
local pattern="gpgsig-sha256"
@@ -91,8 +91,8 @@
del_sigtag () {
- local storage=$1
- local delete=$2
+ local storage="$1"
+ local delete="$2"
if test "$storage" = "$delete" ; then
local pattern="trailer"
@@ -181,7 +181,7 @@
cd "$base"
compare_oids () {
- test "$#" = 5 && { local PREREQ=$1; shift; } || PREREQ=
+ test "$#" = 5 && { local PREREQ="$1"; shift; } || PREREQ=
local type="$1"
local name="$2"
local sha1_oid="$3"
@@ -193,8 +193,8 @@
git --git-dir=repo-sha1/.git rev-parse --output-object-format=sha256 ${sha1_oid} > ${name}_sha1_sha256_found
git --git-dir=repo-sha256/.git rev-parse --output-object-format=sha1 ${sha256_oid} > ${name}_sha256_sha1_found
- local sha1_sha256_oid=$(cat ${name}_sha1_sha256_found)
- local sha256_sha1_oid=$(cat ${name}_sha256_sha1_found)
+ local sha1_sha256_oid="$(cat ${name}_sha1_sha256_found)"
+ local sha256_sha1_oid="$(cat ${name}_sha256_sha1_found)"
test_expect_success $PREREQ "Verify ${type} ${name}'s sha1 oid" '
git --git-dir=repo-sha256/.git rev-parse --output-object-format=sha1 ${sha256_oid} > ${name}_sha1 &&
diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
index c28c041..ba320dc 100755
--- a/t/t2400-worktree-add.sh
+++ b/t/t2400-worktree-add.sh
@@ -427,7 +427,7 @@
# Note: Quoted arguments containing spaces are not supported.
test_wt_add_orphan_hint () {
local context="$1" &&
- local use_branch=$2 &&
+ local use_branch="$2" &&
shift 2 &&
local opts="$*" &&
test_expect_success "'worktree add' show orphan hint in bad/orphan HEAD w/ $context" '
diff --git a/t/t4011-diff-symlink.sh b/t/t4011-diff-symlink.sh
index d7a5f7a..bc8ba88 100755
--- a/t/t4011-diff-symlink.sh
+++ b/t/t4011-diff-symlink.sh
@@ -13,13 +13,13 @@
# Print the short OID of a symlink with the given name.
symlink_oid () {
- local oid=$(printf "%s" "$1" | git hash-object --stdin) &&
+ local oid="$(printf "%s" "$1" | git hash-object --stdin)" &&
git rev-parse --short "$oid"
}
# Print the short OID of the given file.
short_oid () {
- local oid=$(git hash-object "$1") &&
+ local oid="$(git hash-object "$1")" &&
git rev-parse --short "$oid"
}
diff --git a/t/t4210-log-i18n.sh b/t/t4210-log-i18n.sh
index d2dfcf1..75216f1 100755
--- a/t/t4210-log-i18n.sh
+++ b/t/t4210-log-i18n.sh
@@ -64,7 +64,7 @@
'
triggers_undefined_behaviour () {
- local engine=$1
+ local engine="$1"
case $engine in
fixed)
@@ -85,7 +85,7 @@
}
mismatched_git_log () {
- local pattern=$1
+ local pattern="$1"
LC_ALL=$is_IS_locale git log --encoding=ISO-8859-1 --format=%s \
--grep=$pattern
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 2eccf10..862d80c 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -385,7 +385,7 @@
shift
done &&
indir=${indir:+"$indir"/} &&
- local file=${2:-"$1.t"} &&
+ local file="${2:-"$1.t"}" &&
if test -n "$append"
then
$echo "${3-$1}" >>"$indir$file"
@@ -1748,7 +1748,7 @@
# Insert a slash into an object ID so it can be used to reference a location
# under ".git/objects". For example, "deadbeef..." becomes "de/adbeef..".
test_oid_to_path () {
- local basename=${1#??}
+ local basename="${1#??}"
echo "${1%$basename}/$basename"
}
@@ -1765,7 +1765,7 @@
# Choose a port number based on the test script's number and store it in
# the given variable name, unless that variable already contains a number.
test_set_port () {
- local var=$1 port
+ local var="$1" port
if test $# -ne 1 || test -z "$var"
then
@@ -1840,7 +1840,7 @@
shift
fi
- local expr=$(printf '"%s",' "$@")
+ local expr="$(printf '"%s",' "$@")"
expr="${expr%,}"
if test -n "$negate"
@@ -1930,7 +1930,7 @@
# An optional increment to the magic timestamp may be specified as second
# argument.
test_set_magic_mtime () {
- local inc=${2:-0} &&
+ local inc="${2:-0}" &&
local mtime=$((1234567890 + $inc)) &&
test-tool chmtime =$mtime "$1" &&
test_is_magic_mtime "$1" $inc
@@ -1943,7 +1943,7 @@
# argument. Usually, this should be the same increment which was used for
# the associated test_set_magic_mtime.
test_is_magic_mtime () {
- local inc=${2:-0} &&
+ local inc="${2:-0}" &&
local mtime=$((1234567890 + $inc)) &&
echo $mtime >.git/test-mtime-expect &&
test-tool chmtime --get "$1" >.git/test-mtime-actual &&
