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 ab39509d59dd42c9d193776185649e66fa2640a6..1d92b2da03e8ca4f6f562ed55e2a6d9199b592c3 100644 (file)
--- 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 dd8107cd7da24b01541f0bccf4cff332940f31e9..b2b28c2cedc6b2bd5f88876466243581cf294c53 100755 (executable)
--- 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 acaee9cbb6ab9eb52d5b224ff71571b59fe5005b..8324d6c96dd058d5b732fbd222e098e5507329ff 100644 (file)
--- 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 38a535b041961b1921c2d9839b76ac40764316cd..238d4923c433b8e434db6c5eac2e552abc4270aa 100755 (executable)
--- 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 8132cd37b8c8ec519275be94ec4367d4c868d956..be3206a16f6e3018c366bac01c7d3c0b6866e18b 100755 (executable)
--- 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 c28c04133c8a1c953406c5bdea0e83c8cb82870f..ba320dc4171aa7884591067c00853a955d84f4e8 100755 (executable)
--- 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 d7a5f7ae780c0319514fd949dacac98cfe6ce23b..bc8ba887191fac4fd602fe5c2406f3e66a4becfa 100755 (executable)
--- 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 d2dfcf164e25b8771dd852d2aefc4aee4bd3f5ea..75216f19ce3a3e39e20b5d14406d9f1e8c34632b 100755 (executable)
--- 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 2eccf100c024e21363ac799a1d032470ede16ae9..862d80c9748c7f9d6234c6b47d68ed7854f3de03 100644 (file)
--- 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 &&