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

9 files changed, 37 insertions, 19 deletions

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index ab39509d59..1d92b2da03 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -188,6 +188,22 @@ For shell scripts specifically (not exhaustive):
    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 dd8107cd7d..b2b28c2ced 100755
--- a/t/check-non-portable-shell.pl
+++ b/t/check-non-portable-shell.pl
@@ -47,6 +47,8 @@ while (<>) {
 	/\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 acaee9cbb6..8324d6c96d 100644
--- a/t/lib-parallel-checkout.sh
+++ b/t/lib-parallel-checkout.sh
@@ -20,7 +20,7 @@ test_checkout_workers () {
 		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 38a535b041..238d4923c4 100755
--- a/t/t0610-reftable-basics.sh
+++ b/t/t0610-reftable-basics.sh
@@ -83,7 +83,7 @@ test_expect_success 'init: reinitializing reftable with files backend fails' '
 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 8132cd37b8..be3206a16f 100755
--- a/t/t1016-compatObjectFormat.sh
+++ b/t/t1016-compatObjectFormat.sh
@@ -79,7 +79,7 @@ commit2_oid () {
 }
 
 del_sigcommit () {
-    local delete=$1
+    local delete="$1"
 
     if test "$delete" = "sha256" ; then
 	local pattern="gpgsig-sha256"
@@ -91,8 +91,8 @@ del_sigcommit () {
 
 
 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 @@ done
 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 @@ compare_oids () {
 
     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 c28c04133c..ba320dc417 100755
--- a/t/t2400-worktree-add.sh
+++ b/t/t2400-worktree-add.sh
@@ -427,7 +427,7 @@ test_expect_success '"add" worktree with orphan branch, lock, and reason' '
 # 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 d7a5f7ae78..bc8ba88719 100755
--- a/t/t4011-diff-symlink.sh
+++ b/t/t4011-diff-symlink.sh
@@ -13,13 +13,13 @@ TEST_PASSES_SANITIZE_LEAK=true
 
 # 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 d2dfcf164e..75216f19ce 100755
--- a/t/t4210-log-i18n.sh
+++ b/t/t4210-log-i18n.sh
@@ -64,7 +64,7 @@ test_expect_success 'log --grep does not find non-reencoded values (latin1)' '
 '
 
 triggers_undefined_behaviour () {
-	local engine=$1
+	local engine="$1"
 
 	case $engine in
 	fixed)
@@ -85,7 +85,7 @@ triggers_undefined_behaviour () {
 }
 
 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 2eccf100c0..862d80c974 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -385,7 +385,7 @@ test_commit () {
 		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 @@ test_oid () {
 # 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 @@ test_parse_ls_tree_oids () {
 # 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 @@ test_subcommand () {
 		shift
 	fi
 
-	local expr=$(printf '"%s",' "$@")
+	local expr="$(printf '"%s",' "$@")"
 	expr="${expr%,}"
 
 	if test -n "$negate"
@@ -1930,7 +1930,7 @@ test_readlink () {
 # 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 @@ test_set_magic_mtime () {
 # 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 &&