{"thread":{"id":"42926","subject":"[PATCH 1/3] t7900-subtree.sh: fix quoting and broken && chains","startedAt":"2016-07-26T04:14:25Z","lastAt":"2016-07-26T20:56:37Z","messageCount":7,"participants":["David Aguilar","Eric Sunshine","Johannes Sixt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"292172","messageId":"20160726041416.9438-1-davvid@gmail.com","threadId":"42926","inReplyTo":null,"subject":"[PATCH 1/3] t7900-subtree.sh: fix quoting and broken && chains","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2016-07-26T04:14:14Z","receivedAt":"2016-07-26T04:14:25Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Allow whitespace in arguments to subtree_test_create_repo.\nAdd missing && chains.\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\n contrib/subtree/t/t7900-subtree.sh | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh\nindex 3bf96a9..431a2fe 100755\n--- a/contrib/subtree/t/t7900-subtree.sh\n+++ b/contrib/subtree/t/t7900-subtree.sh\n@@ -16,16 +16,16 @@ export TEST_DIRECTORY\n \n subtree_test_create_repo()\n {\n-\ttest_create_repo \"$1\"\n+\ttest_create_repo \"$1\" &&\n \t(\n-\t\tcd $1\n+\t\tcd \"$1\" &&\n \t\tgit config log.date relative\n \t)\n }\n \n create()\n {\n-\techo \"$1\" >\"$1\"\n+\techo \"$1\" >\"$1\" &&\n \tgit add \"$1\"\n }\n \n@@ -73,10 +73,10 @@ join_commits()\n test_create_commit() (\n \trepo=$1\n \tcommit=$2\n-\tcd \"$repo\"\n-\tmkdir -p $(dirname \"$commit\") \\\n+\tcd \"$repo\" &&\n+\tmkdir -p \"$(dirname \"$commit\")\" \\\n \t|| error \"Could not create directory for commit\"\n-\techo \"$commit\" >\"$commit\"\n+\techo \"$commit\" >\"$commit\" &&\n \tgit add \"$commit\" || error \"Could not add commit\"\n \tgit commit -m \"$commit\" || error \"Could not commit\"\n )\n-- \n2.9.2.466.g8c6d1f9.dirty\n\n"},{"id":"292173","messageId":"20160726041416.9438-2-davvid@gmail.com","threadId":"42926","inReplyTo":"20160726041416.9438-1-davvid@gmail.com","subject":"[PATCH 2/3] subtree: fix \"git subtree split --rejoin\"","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2016-07-26T04:14:15Z","receivedAt":"2016-07-26T04:14:29Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"\"git merge\" in v2.9 prevents merging unrelated histories.\n\n\"git subtree split --rejoin\" creates unrelated histories when\ncreating a split repo from a raw sub-directory that did not\noriginate from an invocation of \"git subtree add\".\n\nRestore the original behavior by passing --allow-unrelated-histories\nwhen merging subtrees.  This ensures that the synthetic history\ncreated by \"git subtree split\" can be merged.\n\nAdd a test to ensure that this feature works as advertised.\n\nReported-by: Brett Cundal <brett.cundal@iugome.com>\nHelped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\nThis is a \"re-implementation\" of Brett's original RFC patch.\nI preferred adding a new line (rather than modifying the existing line)\nso I have no problem signing off on this being a distinct patch\nauthored by me.\n\n contrib/subtree/git-subtree.sh     |  1 +\n contrib/subtree/t/t7900-subtree.sh | 16 ++++++++++++++++\n 2 files changed, 17 insertions(+)\n\ndiff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\nindex 7a39b30..b567eae 100755\n--- a/contrib/subtree/git-subtree.sh\n+++ b/contrib/subtree/git-subtree.sh\n@@ -662,6 +662,7 @@ cmd_split()\n \t\tdebug \"Merging split branch into HEAD...\"\n \t\tlatest_old=$(cache_get latest_old)\n \t\tgit merge -s ours \\\n+\t\t\t--allow-unrelated-histories \\\n \t\t\t-m \"$(rejoin_msg \"$dir\" $latest_old $latest_new)\" \\\n \t\t\t$latest_new >&2 || exit $?\n \tfi\ndiff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh\nindex 431a2fe..e179b29 100755\n--- a/contrib/subtree/t/t7900-subtree.sh\n+++ b/contrib/subtree/t/t7900-subtree.sh\n@@ -347,6 +347,22 @@ test_expect_success 'split sub dir/ with --rejoin' '\n  '\n \n next_test\n+test_expect_success 'split sub dir/ with --rejoin from scratch' '\n+\tsubtree_test_create_repo \"$subtree_test_count\" &&\n+\ttest_create_commit \"$subtree_test_count\" main1 &&\n+\t(\n+\t\tcd \"$subtree_test_count\" &&\n+\t\tmkdir \"sub dir\" &&\n+\t\techo file >\"sub dir\"/file &&\n+\t\tgit add \"sub dir/file\" &&\n+\t\tgit commit -m\"sub dir file\" &&\n+\t\tsplit_hash=$(git subtree split --prefix=\"sub dir\" --rejoin) &&\n+\t\tgit subtree split --prefix=\"sub dir\" --rejoin &&\n+\t\tcheck_equal \"$(last_commit_message)\" \"Split '\\''sub dir/'\\'' into commit '\\''$split_hash'\\''\"\n+\t)\n+ '\n+\n+next_test\n test_expect_success 'split sub dir/ with --rejoin and --message' '\n \tsubtree_test_create_repo \"$subtree_test_count\" &&\n \tsubtree_test_create_repo \"$subtree_test_count/sub proj\" &&\n-- \n2.9.2.466.g8c6d1f9.dirty\n\n"},{"id":"292174","messageId":"20160726041416.9438-3-davvid@gmail.com","threadId":"42926","inReplyTo":"20160726041416.9438-1-davvid@gmail.com","subject":"[PATCH 3/3] subtree: adjust style to match CodingGuidelines","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2016-07-26T04:14:16Z","receivedAt":"2016-07-26T04:14:37Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Prefer \"test\" over \"[ ... ]\", use double-quotes around variables, break\nlong lines, and properly indent \"case\" statements.\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\n contrib/subtree/git-subtree.sh | 544 ++++++++++++++++++++++++++---------------\n 1 file changed, 341 insertions(+), 203 deletions(-)\n\ndiff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\nindex b567eae..084c884 100755\n--- a/contrib/subtree/git-subtree.sh\n+++ b/contrib/subtree/git-subtree.sh\n@@ -4,8 +4,9 @@\n #\n # Copyright (C) 2009 Avery Pennarun <apenwarr@gmail.com>\n #\n-if [ $# -eq 0 ]; then\n-    set -- -h\n+if test $# -eq 0\n+then\n+\tset -- -h\n fi\n OPTS_SPEC=\"\\\n git subtree add   --prefix=<prefix> <commit>\n@@ -50,87 +51,145 @@ prefix=\n \n debug()\n {\n-\tif [ -n \"$debug\" ]; then\n-\t\tprintf \"%s\\n\" \"$*\" >&2\n+\tif test -n \"$debug\"\n+\tthen\n+\t\tprintf \"%s\\n\" \"$@\" >&2\n \tfi\n }\n \n say()\n {\n-\tif [ -z \"$quiet\" ]; then\n-\t\tprintf \"%s\\n\" \"$*\" >&2\n+\tif test -z \"$quiet\"\n+\tthen\n+\t\tprintf \"%s\\n\" \"$@\" >&2\n \tfi\n }\n \n progress()\n {\n-\tif [ -z \"$quiet\" ]; then\n-\t\tprintf \"%s\\r\" \"$*\" >&2\n+\tif test -z \"$quiet\"\n+\tthen\n+\t\tprintf \"%s\\r\" \"$@\" >&2\n \tfi\n }\n \n assert()\n {\n-\tif \"$@\"; then\n-\t\t:\n-\telse\n+\tif ! \"$@\"\n+\tthen\n \t\tdie \"assertion failed: \" \"$@\"\n \tfi\n }\n \n \n-#echo \"Options: $*\"\n-\n-while [ $# -gt 0 ]; do\n+while test $# -gt 0\n+do\n \topt=\"$1\"\n \tshift\n+\n \tcase \"$opt\" in\n-\t\t-q) quiet=1 ;;\n-\t\t-d) debug=1 ;;\n-\t\t--annotate) annotate=\"$1\"; shift ;;\n-\t\t--no-annotate) annotate= ;;\n-\t\t-b) branch=\"$1\"; shift ;;\n-\t\t-P) prefix=\"${1%/}\"; shift ;;\n-\t\t-m) message=\"$1\"; shift ;;\n-\t\t--no-prefix) prefix= ;;\n-\t\t--onto) onto=\"$1\"; shift ;;\n-\t\t--no-onto) onto= ;;\n-\t\t--rejoin) rejoin=1 ;;\n-\t\t--no-rejoin) rejoin= ;;\n-\t\t--ignore-joins) ignore_joins=1 ;;\n-\t\t--no-ignore-joins) ignore_joins= ;;\n-\t\t--squash) squash=1 ;;\n-\t\t--no-squash) squash= ;;\n-\t\t--) break ;;\n-\t\t*) die \"Unexpected option: $opt\" ;;\n+\t-q)\n+\t\tquiet=1\n+\t\t;;\n+\t-d)\n+\t\tdebug=1\n+\t\t;;\n+\t--annotate)\n+\t\tannotate=\"$1\"\n+\t\tshift\n+\t\t;;\n+\t--no-annotate)\n+\t\tannotate=\n+\t\t;;\n+\t-b)\n+\t\tbranch=\"$1\"\n+\t\tshift\n+\t\t;;\n+\t-P)\n+\t\tprefix=\"${1%/}\"\n+\t\tshift\n+\t\t;;\n+\t-m)\n+\t\tmessage=\"$1\"\n+\t\tshift\n+\t\t;;\n+\t--no-prefix)\n+\t\tprefix=\n+\t\t;;\n+\t--onto)\n+\t\tonto=\"$1\"\n+\t\tshift\n+\t\t;;\n+\t--no-onto)\n+\t\tonto=\n+\t\t;;\n+\t--rejoin)\n+\t\trejoin=1\n+\t\t;;\n+\t--no-rejoin)\n+\t\trejoin=\n+\t\t;;\n+\t--ignore-joins)\n+\t\tignore_joins=1\n+\t\t;;\n+\t--no-ignore-joins)\n+\t\tignore_joins=\n+\t\t;;\n+\t--squash)\n+\t\tsquash=1\n+\t\t;;\n+\t--no-squash)\n+\t\tsquash=\n+\t\t;;\n+\t--)\n+\t\tbreak\n+\t\t;;\n+\t*)\n+\t\tdie \"Unexpected option: $opt\"\n+\t\t;;\n \tesac\n done\n \n command=\"$1\"\n shift\n+\n case \"$command\" in\n-\tadd|merge|pull) default= ;;\n-\tsplit|push) default=\"--default HEAD\" ;;\n-\t*) die \"Unknown command '$command'\" ;;\n+add|merge|pull)\n+\tdefault=\n+\t;;\n+split|push)\n+\tdefault=\"--default HEAD\"\n+\t;;\n+*)\n+\tdie \"Unknown command '$command'\"\n+\t;;\n esac\n \n-if [ -z \"$prefix\" ]; then\n+if test -z \"$prefix\"\n+then\n \tdie \"You must provide the --prefix option.\"\n fi\n \n case \"$command\" in\n-\tadd) [ -e \"$prefix\" ] && \n-\t\tdie \"prefix '$prefix' already exists.\" ;;\n-\t*)   [ -e \"$prefix\" ] || \n-\t\tdie \"'$prefix' does not exist; use 'git subtree add'\" ;;\n+add)\n+\ttest -e \"$prefix\" && die \"prefix '$prefix' already exists.\"\n+\t;;\n+*)\n+\ttest -e \"$prefix\" ||\n+\tdie \"'$prefix' does not exist; use 'git subtree add'\"\n+\t;;\n esac\n \n dir=\"$(dirname \"$prefix/.\")\"\n \n-if [ \"$command\" != \"pull\" -a \"$command\" != \"add\" -a \"$command\" != \"push\" ]; then\n+if test \"$command\" != \"pull\" &&\n+\t\ttest \"$command\" != \"add\" &&\n+\t\ttest \"$command\" != \"push\"\n+then\n \trevs=$(git rev-parse $default --revs-only \"$@\") || exit $?\n \tdirs=\"$(git rev-parse --no-revs --no-flags \"$@\")\" || exit $?\n-\tif [ -n \"$dirs\" ]; then\n+\tif test -n \"$dirs\"\n+\tthen\n \t\tdie \"Error: Use --prefix instead of bare filenames.\"\n \tfi\n fi\n@@ -139,22 +198,27 @@ debug \"command: {$command}\"\n debug \"quiet: {$quiet}\"\n debug \"revs: {$revs}\"\n debug \"dir: {$dir}\"\n-debug \"opts: {$*}\"\n+debug \"opts: {$@}\"\n debug\n \n cache_setup()\n {\n \tcachedir=\"$GIT_DIR/subtree-cache/$$\"\n-\trm -rf \"$cachedir\" || die \"Can't delete old cachedir: $cachedir\"\n-\tmkdir -p \"$cachedir\" || die \"Can't create new cachedir: $cachedir\"\n-\tmkdir -p \"$cachedir/notree\" || die \"Can't create new cachedir: $cachedir/notree\"\n+\trm -rf \"$cachedir\" ||\n+\t\tdie \"Can't delete old cachedir: $cachedir\"\n+\tmkdir -p \"$cachedir\" ||\n+\t\tdie \"Can't create new cachedir: $cachedir\"\n+\tmkdir -p \"$cachedir/notree\" ||\n+\t\tdie \"Can't create new cachedir: $cachedir/notree\"\n \tdebug \"Using cachedir: $cachedir\" >&2\n }\n \n cache_get()\n {\n-\tfor oldrev in $*; do\n-\t\tif [ -r \"$cachedir/$oldrev\" ]; then\n+\tfor oldrev in \"$@\"\n+\tdo\n+\t\tif test -r \"$cachedir/$oldrev\"\n+\t\tthen\n \t\t\tread newrev <\"$cachedir/$oldrev\"\n \t\t\techo $newrev\n \t\tfi\n@@ -163,8 +227,10 @@ cache_get()\n \n cache_miss()\n {\n-\tfor oldrev in $*; do\n-\t\tif [ ! -r \"$cachedir/$oldrev\" ]; then\n+\tfor oldrev in \"$@\"\n+\tdo\n+\t\tif ! test -r \"$cachedir/$oldrev\"\n+\t\tthen\n \t\t\techo $oldrev\n \t\tfi\n \tdone\n@@ -172,9 +238,11 @@ cache_miss()\n \n check_parents()\n {\n-\tmissed=$(cache_miss $*)\n-\tfor miss in $missed; do\n-\t\tif [ ! -r \"$cachedir/notree/$miss\" ]; then\n+\tmissed=$(cache_miss \"$@\")\n+\tfor miss in $missed\n+\tdo\n+\t\tif ! test -r \"$cachedir/notree/$miss\"\n+\t\tthen\n \t\t\tdebug \"  incorrect order: $miss\"\n \t\tfi\n \tdone\n@@ -189,9 +257,10 @@ cache_set()\n {\n \toldrev=\"$1\"\n \tnewrev=\"$2\"\n-\tif [ \"$oldrev\" != \"latest_old\" \\\n-\t     -a \"$oldrev\" != \"latest_new\" \\\n-\t     -a -e \"$cachedir/$oldrev\" ]; then\n+\tif test \"$oldrev\" != \"latest_old\" &&\n+\t\ttest \"$oldrev\" != \"latest_new\" &&\n+\t\ttest -e \"$cachedir/$oldrev\"\n+\tthen\n \t\tdie \"cache for $oldrev already exists!\"\n \tfi\n \techo \"$newrev\" >\"$cachedir/$oldrev\"\n@@ -199,7 +268,8 @@ cache_set()\n \n rev_exists()\n {\n-\tif git rev-parse \"$1\" >/dev/null 2>&1; then\n+\tif git rev-parse \"$1\" >/dev/null 2>&1\n+\tthen\n \t\treturn 0\n \telse\n \t\treturn 1\n@@ -213,7 +283,8 @@ rev_is_descendant_of_branch()\n \tbranch_hash=$(git rev-parse $branch)\n \tmatch=$(git rev-list -1 $branch_hash ^$newrev)\n \n-\tif [ -z \"$match\" ]; then\n+\tif test -z \"$match\"\n+\tthen\n \t\treturn 0\n \telse\n \t\treturn 1\n@@ -225,7 +296,8 @@ rev_is_descendant_of_branch()\n # be there anyway, so do nothing in that case.\n try_remove_previous()\n {\n-\tif rev_exists \"$1^\"; then\n+\tif rev_exists \"$1^\"\n+\tthen\n \t\techo \"^$1^\"\n \tfi\n }\n@@ -239,31 +311,38 @@ find_latest_squash()\n \tsub=\n \tgit log --grep=\"^git-subtree-dir: $dir/*\\$\" \\\n \t\t--pretty=format:'START %H%n%s%n%n%b%nEND%n' HEAD |\n-\twhile read a b junk; do\n+\twhile read a b junk\n+\tdo\n \t\tdebug \"$a $b $junk\"\n \t\tdebug \"{{$sq/$main/$sub}}\"\n \t\tcase \"$a\" in\n-\t\t\tSTART) sq=\"$b\" ;;\n-\t\t\tgit-subtree-mainline:) main=\"$b\" ;;\n-\t\t\tgit-subtree-split:)\n-\t\t\t\tsub=\"$(git rev-parse \"$b^0\")\" ||\n-\t\t\t\t    die \"could not rev-parse split hash $b from commit $sq\"\n-\t\t\t\t;;\n-\t\t\tEND)\n-\t\t\t\tif [ -n \"$sub\" ]; then\n-\t\t\t\t\tif [ -n \"$main\" ]; then\n-\t\t\t\t\t\t# a rejoin commit?\n-\t\t\t\t\t\t# Pretend its sub was a squash.\n-\t\t\t\t\t\tsq=\"$sub\"\n-\t\t\t\t\tfi\n-\t\t\t\t\tdebug \"Squash found: $sq $sub\"\n-\t\t\t\t\techo \"$sq\" \"$sub\"\n-\t\t\t\t\tbreak\n+\t\tSTART)\n+\t\t\tsq=\"$b\"\n+\t\t\t;;\n+\t\tgit-subtree-mainline:)\n+\t\t\tmain=\"$b\"\n+\t\t\t;;\n+\t\tgit-subtree-split:)\n+\t\t\tsub=\"$(git rev-parse \"$b^0\")\" ||\n+\t\t\tdie \"could not rev-parse split hash $b from commit $sq\"\n+\t\t\t;;\n+\t\tEND)\n+\t\t\tif test -n \"$sub\"\n+\t\t\tthen\n+\t\t\t\tif test -n \"$main\"\n+\t\t\t\tthen\n+\t\t\t\t\t# a rejoin commit?\n+\t\t\t\t\t# Pretend its sub was a squash.\n+\t\t\t\t\tsq=\"$sub\"\n \t\t\t\tfi\n-\t\t\t\tsq=\n-\t\t\t\tmain=\n-\t\t\t\tsub=\n-\t\t\t\t;;\n+\t\t\t\tdebug \"Squash found: $sq $sub\"\n+\t\t\t\techo \"$sq\" \"$sub\"\n+\t\t\t\tbreak\n+\t\t\tfi\n+\t\t\tsq=\n+\t\t\tmain=\n+\t\t\tsub=\n+\t\t\t;;\n \t\tesac\n \tdone\n }\n@@ -277,31 +356,38 @@ find_existing_splits()\n \tsub=\n \tgit log --grep=\"^git-subtree-dir: $dir/*\\$\" \\\n \t\t--pretty=format:'START %H%n%s%n%n%b%nEND%n' $revs |\n-\twhile read a b junk; do\n+\twhile read a b junk\n+\tdo\n \t\tcase \"$a\" in\n-\t\t\tSTART) sq=\"$b\" ;;\n-\t\t\tgit-subtree-mainline:) main=\"$b\" ;;\n-\t\t\tgit-subtree-split:)\n-\t\t\t\tsub=\"$(git rev-parse \"$b^0\")\" ||\n-\t\t\t\t    die \"could not rev-parse split hash $b from commit $sq\"\n-\t\t\t\t;;\n-\t\t\tEND)\n-\t\t\t\tdebug \"  Main is: '$main'\"\n-\t\t\t\tif [ -z \"$main\" -a -n \"$sub\" ]; then\n-\t\t\t\t\t# squash commits refer to a subtree\n-\t\t\t\t\tdebug \"  Squash: $sq from $sub\"\n-\t\t\t\t\tcache_set \"$sq\" \"$sub\"\n-\t\t\t\tfi\n-\t\t\t\tif [ -n \"$main\" -a -n \"$sub\" ]; then\n-\t\t\t\t\tdebug \"  Prior: $main -> $sub\"\n-\t\t\t\t\tcache_set $main $sub\n-\t\t\t\t\tcache_set $sub $sub\n-\t\t\t\t\ttry_remove_previous \"$main\"\n-\t\t\t\t\ttry_remove_previous \"$sub\"\n-\t\t\t\tfi\n-\t\t\t\tmain=\n-\t\t\t\tsub=\n-\t\t\t\t;;\n+\t\tSTART)\n+\t\t\tsq=\"$b\"\n+\t\t\t;;\n+\t\tgit-subtree-mainline:)\n+\t\t\tmain=\"$b\"\n+\t\t\t;;\n+\t\tgit-subtree-split:)\n+\t\t\tsub=\"$(git rev-parse \"$b^0\")\" ||\n+\t\t\tdie \"could not rev-parse split hash $b from commit $sq\"\n+\t\t\t;;\n+\t\tEND)\n+\t\t\tdebug \"  Main is: '$main'\"\n+\t\t\tif test -z \"$main\" -a -n \"$sub\"\n+\t\t\tthen\n+\t\t\t\t# squash commits refer to a subtree\n+\t\t\t\tdebug \"  Squash: $sq from $sub\"\n+\t\t\t\tcache_set \"$sq\" \"$sub\"\n+\t\t\tfi\n+\t\t\tif test -n \"$main\" -a -n \"$sub\"\n+\t\t\tthen\n+\t\t\t\tdebug \"  Prior: $main -> $sub\"\n+\t\t\t\tcache_set $main $sub\n+\t\t\t\tcache_set $sub $sub\n+\t\t\t\ttry_remove_previous \"$main\"\n+\t\t\t\ttry_remove_previous \"$sub\"\n+\t\t\tfi\n+\t\t\tmain=\n+\t\t\tsub=\n+\t\t\t;;\n \t\tesac\n \tdone\n }\n@@ -325,7 +411,10 @@ copy_commit()\n \t\t\tGIT_COMMITTER_NAME \\\n \t\t\tGIT_COMMITTER_EMAIL \\\n \t\t\tGIT_COMMITTER_DATE\n-\t\t(printf \"%s\" \"$annotate\"; cat ) |\n+\t\t(\n+\t\t\tprintf \"%s\" \"$annotate\"\n+\t\t\tcat\n+\t\t) |\n \t\tgit commit-tree \"$2\" $3  # reads the rest of stdin\n \t) || die \"Can't copy commit $1\"\n }\n@@ -335,14 +424,15 @@ add_msg()\n \tdir=\"$1\"\n \tlatest_old=\"$2\"\n \tlatest_new=\"$3\"\n-\tif [ -n \"$message\" ]; then\n+\tif test -n \"$message\"\n+\tthen\n \t\tcommit_message=\"$message\"\n \telse\n \t\tcommit_message=\"Add '$dir/' from commit '$latest_new'\"\n \tfi\n \tcat <<-EOF\n \t\t$commit_message\n-\t\t\n+\n \t\tgit-subtree-dir: $dir\n \t\tgit-subtree-mainline: $latest_old\n \t\tgit-subtree-split: $latest_new\n@@ -351,7 +441,8 @@ add_msg()\n \n add_squashed_msg()\n {\n-\tif [ -n \"$message\" ]; then\n+\tif test -n \"$message\"\n+\tthen\n \t\techo \"$message\"\n \telse\n \t\techo \"Merge commit '$1' as '$2'\"\n@@ -363,14 +454,15 @@ rejoin_msg()\n \tdir=\"$1\"\n \tlatest_old=\"$2\"\n \tlatest_new=\"$3\"\n-\tif [ -n \"$message\" ]; then\n+\tif test -n \"$message\"\n+\tthen\n \t\tcommit_message=\"$message\"\n \telse\n \t\tcommit_message=\"Split '$dir/' into commit '$latest_new'\"\n \tfi\n \tcat <<-EOF\n \t\t$commit_message\n-\t\t\n+\n \t\tgit-subtree-dir: $dir\n \t\tgit-subtree-mainline: $latest_old\n \t\tgit-subtree-split: $latest_new\n@@ -383,8 +475,9 @@ squash_msg()\n \toldsub=\"$2\"\n \tnewsub=\"$3\"\n \tnewsub_short=$(git rev-parse --short \"$newsub\")\n-\t\n-\tif [ -n \"$oldsub\" ]; then\n+\n+\tif test -n \"$oldsub\"\n+\tthen\n \t\toldsub_short=$(git rev-parse --short \"$oldsub\")\n \t\techo \"Squashed '$dir/' changes from $oldsub_short..$newsub_short\"\n \t\techo\n@@ -393,7 +486,7 @@ squash_msg()\n \telse\n \t\techo \"Squashed '$dir/' content from commit $newsub_short\"\n \tfi\n-\t\n+\n \techo\n \techo \"git-subtree-dir: $dir\"\n \techo \"git-subtree-split: $newsub\"\n@@ -410,10 +503,11 @@ subtree_for_commit()\n \tcommit=\"$1\"\n \tdir=\"$2\"\n \tgit ls-tree \"$commit\" -- \"$dir\" |\n-\twhile read mode type tree name; do\n-\t\tassert [ \"$name\" = \"$dir\" ]\n-\t\tassert [ \"$type\" = \"tree\" -o \"$type\" = \"commit\" ]\n-\t\t[ \"$type\" = \"commit\" ] && continue  # ignore submodules\n+\twhile read mode type tree name\n+\tdo\n+\t\tassert test \"$name\" = \"$dir\"\n+\t\tassert test \"$type\" = \"tree\" -o \"$type\" = \"commit\"\n+\t\ttest \"$type\" = \"commit\" && continue  # ignore submodules\n \t\techo $tree\n \t\tbreak\n \tdone\n@@ -423,11 +517,13 @@ tree_changed()\n {\n \ttree=$1\n \tshift\n-\tif [ $# -ne 1 ]; then\n+\tif test $# -ne 1\n+\tthen\n \t\treturn 0   # weird parents, consider it changed\n \telse\n \t\tptree=$(toptree_for_commit $1)\n-\t\tif [ \"$ptree\" != \"$tree\" ]; then\n+\t\tif test \"$ptree\" != \"$tree\"\n+\t\tthen\n \t\t\treturn 0   # changed\n \t\telse\n \t\t\treturn 1   # not changed\n@@ -441,8 +537,9 @@ new_squash_commit()\n \toldsub=\"$2\"\n \tnewsub=\"$3\"\n \ttree=$(toptree_for_commit $newsub) || exit $?\n-\tif [ -n \"$old\" ]; then\n-\t\tsquash_msg \"$dir\" \"$oldsub\" \"$newsub\" | \n+\tif test -n \"$old\"\n+\tthen\n+\t\tsquash_msg \"$dir\" \"$oldsub\" \"$newsub\" |\n \t\t\tgit commit-tree \"$tree\" -p \"$old\" || exit $?\n \telse\n \t\tsquash_msg \"$dir\" \"\" \"$newsub\" |\n@@ -455,46 +552,54 @@ copy_or_skip()\n \trev=\"$1\"\n \ttree=\"$2\"\n \tnewparents=\"$3\"\n-\tassert [ -n \"$tree\" ]\n+\tassert test -n \"$tree\"\n \n \tidentical=\n \tnonidentical=\n \tp=\n \tgotparents=\n-\tfor parent in $newparents; do\n+\tfor parent in $newparents\n+\tdo\n \t\tptree=$(toptree_for_commit $parent) || exit $?\n-\t\t[ -z \"$ptree\" ] && continue\n-\t\tif [ \"$ptree\" = \"$tree\" ]; then\n+\t\ttest -z \"$ptree\" && continue\n+\t\tif test \"$ptree\" = \"$tree\"\n+\t\tthen\n \t\t\t# an identical parent could be used in place of this rev.\n \t\t\tidentical=\"$parent\"\n \t\telse\n \t\t\tnonidentical=\"$parent\"\n \t\tfi\n-\t\t\n+\n \t\t# sometimes both old parents map to the same newparent;\n \t\t# eliminate duplicates\n \t\tis_new=1\n-\t\tfor gp in $gotparents; do\n-\t\t\tif [ \"$gp\" = \"$parent\" ]; then\n+\t\tfor gp in $gotparents\n+\t\tdo\n+\t\t\tif test \"$gp\" = \"$parent\"\n+\t\t\tthen\n \t\t\t\tis_new=\n \t\t\t\tbreak\n \t\t\tfi\n \t\tdone\n-\t\tif [ -n \"$is_new\" ]; then\n+\t\tif test -n \"$is_new\"\n+\t\tthen\n \t\t\tgotparents=\"$gotparents $parent\"\n \t\t\tp=\"$p -p $parent\"\n \t\tfi\n \tdone\n \n \tcopycommit=\n-\tif [ -n \"$identical\" ] && [ -n \"$nonidentical\" ]; then\n+\tif test -n \"$identical\" && test -n \"$nonidentical\"\n+\tthen\n \t\textras=$(git rev-list --count $identical..$nonidentical)\n-\t\tif [ \"$extras\" -ne 0 ]; then\n+\t\tif test \"$extras\" -ne 0\n+\t\tthen\n \t\t\t# we need to preserve history along the other branch\n \t\t\tcopycommit=1\n \t\tfi\n \tfi\n-\tif [ -n \"$identical\" ] && [ -z \"$copycommit\" ]; then\n+\tif test -n \"$identical\" && test -z \"$copycommit\"\n+\tthen\n \t\techo $identical\n \telse\n \t\tcopy_commit $rev $tree \"$p\" || exit $?\n@@ -503,10 +608,12 @@ copy_or_skip()\n \n ensure_clean()\n {\n-\tif ! git diff-index HEAD --exit-code --quiet 2>&1; then\n+\tif ! git diff-index HEAD --exit-code --quiet 2>&1\n+\tthen\n \t\tdie \"Working tree has modifications.  Cannot add.\"\n \tfi\n-\tif ! git diff-index --cached HEAD --exit-code --quiet 2>&1; then\n+\tif ! git diff-index --cached HEAD --exit-code --quiet 2>&1\n+\tthen\n \t\tdie \"Index has modifications.  Cannot add.\"\n \tfi\n }\n@@ -514,34 +621,38 @@ ensure_clean()\n ensure_valid_ref_format()\n {\n \tgit check-ref-format \"refs/heads/$1\" ||\n-\t    die \"'$1' does not look like a ref\"\n+\t\tdie \"'$1' does not look like a ref\"\n }\n \n cmd_add()\n {\n-\tif [ -e \"$dir\" ]; then\n+\tif test -e \"$dir\"\n+\tthen\n \t\tdie \"'$dir' already exists.  Cannot add.\"\n \tfi\n \n \tensure_clean\n-\t\n-\tif [ $# -eq 1 ]; then\n-\t    git rev-parse -q --verify \"$1^{commit}\" >/dev/null ||\n-\t    die \"'$1' does not refer to a commit\"\n-\n-\t    \"cmd_add_commit\" \"$@\"\n-\telif [ $# -eq 2 ]; then\n-\t    # Technically we could accept a refspec here but we're\n-\t    # just going to turn around and add FETCH_HEAD under the\n-\t    # specified directory.  Allowing a refspec might be\n-\t    # misleading because we won't do anything with any other\n-\t    # branches fetched via the refspec.\n-\t    ensure_valid_ref_format \"$2\"\n-\n-\t    \"cmd_add_repository\" \"$@\"\n+\n+\tif test $# -eq 1\n+\tthen\n+\t\tgit rev-parse -q --verify \"$1^{commit}\" >/dev/null ||\n+\t\t\tdie \"'$1' does not refer to a commit\"\n+\n+\t\tcmd_add_commit \"$@\"\n+\n+\telif test $# -eq 2\n+\tthen\n+\t\t# Technically we could accept a refspec here but we're\n+\t\t# just going to turn around and add FETCH_HEAD under the\n+\t\t# specified directory.  Allowing a refspec might be\n+\t\t# misleading because we won't do anything with any other\n+\t\t# branches fetched via the refspec.\n+\t\tensure_valid_ref_format \"$2\"\n+\n+\t\tcmd_add_repository \"$@\"\n \telse\n-\t    say \"error: parameters were '$@'\"\n-\t    die \"Provide either a commit or a repository and commit.\"\n+\t\tsay \"error: parameters were '$@'\"\n+\t\tdie \"Provide either a commit or a repository and commit.\"\n \tfi\n }\n \n@@ -561,30 +672,32 @@ cmd_add_commit()\n \trevs=$(git rev-parse $default --revs-only \"$@\") || exit $?\n \tset -- $revs\n \trev=\"$1\"\n-\t\n+\n \tdebug \"Adding $dir as '$rev'...\"\n \tgit read-tree --prefix=\"$dir\" $rev || exit $?\n \tgit checkout -- \"$dir\" || exit $?\n \ttree=$(git write-tree) || exit $?\n-\t\n+\n \theadrev=$(git rev-parse HEAD) || exit $?\n-\tif [ -n \"$headrev\" -a \"$headrev\" != \"$rev\" ]; then\n+\tif test -n \"$headrev\" && test \"$headrev\" != \"$rev\"\n+\tthen\n \t\theadp=\"-p $headrev\"\n \telse\n \t\theadp=\n \tfi\n-\t\n-\tif [ -n \"$squash\" ]; then\n+\n+\tif test -n \"$squash\"\n+\tthen\n \t\trev=$(new_squash_commit \"\" \"\" \"$rev\") || exit $?\n \t\tcommit=$(add_squashed_msg \"$rev\" \"$dir\" |\n-\t\t\t git commit-tree $tree $headp -p \"$rev\") || exit $?\n+\t\t\tgit commit-tree $tree $headp -p \"$rev\") || exit $?\n \telse\n \t\trevp=$(peel_committish \"$rev\") &&\n \t\tcommit=$(add_msg \"$dir\" \"$headrev\" \"$rev\" |\n-\t\t\t git commit-tree $tree $headp -p \"$revp\") || exit $?\n+\t\t\tgit commit-tree $tree $headp -p \"$revp\") || exit $?\n \tfi\n \tgit reset \"$commit\" || exit $?\n-\t\n+\n \tsay \"Added dir '$dir'\"\n }\n \n@@ -592,24 +705,27 @@ cmd_split()\n {\n \tdebug \"Splitting $dir...\"\n \tcache_setup || exit $?\n-\t\n-\tif [ -n \"$onto\" ]; then\n+\n+\tif test -n \"$onto\"\n+\tthen\n \t\tdebug \"Reading history for --onto=$onto...\"\n \t\tgit rev-list $onto |\n-\t\twhile read rev; do\n+\t\twhile read rev\n+\t\tdo\n \t\t\t# the 'onto' history is already just the subdir, so\n \t\t\t# any parent we find there can be used verbatim\n \t\t\tdebug \"  cache: $rev\"\n \t\t\tcache_set $rev $rev\n \t\tdone\n \tfi\n-\t\n-\tif [ -n \"$ignore_joins\" ]; then\n+\n+\tif test -n \"$ignore_joins\"\n+\tthen\n \t\tunrevs=\n \telse\n \t\tunrevs=\"$(find_existing_splits \"$dir\" \"$revs\")\"\n \tfi\n-\t\n+\n \t# We can't restrict rev-list to only $dir here, because some of our\n \t# parents have the $dir contents the root, and those won't match.\n \t# (and rev-list --follow doesn't seem to solve this)\n@@ -618,12 +734,14 @@ cmd_split()\n \trevcount=0\n \tcreatecount=0\n \teval \"$grl\" |\n-\twhile read rev parents; do\n+\twhile read rev parents\n+\tdo\n \t\trevcount=$(($revcount + 1))\n \t\tprogress \"$revcount/$revmax ($createcount)\"\n \t\tdebug \"Processing commit: $rev\"\n \t\texists=$(cache_get $rev)\n-\t\tif [ -n \"$exists\" ]; then\n+\t\tif test -n \"$exists\"\n+\t\tthen\n \t\t\tdebug \"  prior: $exists\"\n \t\t\tcontinue\n \t\tfi\n@@ -631,17 +749,19 @@ cmd_split()\n \t\tdebug \"  parents: $parents\"\n \t\tnewparents=$(cache_get $parents)\n \t\tdebug \"  newparents: $newparents\"\n-\t\t\n+\n \t\ttree=$(subtree_for_commit $rev \"$dir\")\n \t\tdebug \"  tree is: $tree\"\n \n \t\tcheck_parents $parents\n-\t\t\n+\n \t\t# ugly.  is there no better way to tell if this is a subtree\n \t\t# vs. a mainline commit?  Does it matter?\n-\t\tif [ -z $tree ]; then\n+\t\tif test -z $tree\n+\t\tthen\n \t\t\tset_notree $rev\n-\t\t\tif [ -n \"$newparents\" ]; then\n+\t\t\tif test -n \"$newparents\"\n+\t\t\tthen\n \t\t\t\tcache_set $rev $rev\n \t\t\tfi\n \t\t\tcontinue\n@@ -653,12 +773,15 @@ cmd_split()\n \t\tcache_set latest_new $newrev\n \t\tcache_set latest_old $rev\n \tdone || exit $?\n+\n \tlatest_new=$(cache_get latest_new)\n-\tif [ -z \"$latest_new\" ]; then\n+\tif test -z \"$latest_new\"\n+\tthen\n \t\tdie \"No new revisions were found\"\n \tfi\n-\t\n-\tif [ -n \"$rejoin\" ]; then\n+\n+\tif test -n \"$rejoin\"\n+\tthen\n \t\tdebug \"Merging split branch into HEAD...\"\n \t\tlatest_old=$(cache_get latest_old)\n \t\tgit merge -s ours \\\n@@ -666,16 +789,20 @@ cmd_split()\n \t\t\t-m \"$(rejoin_msg \"$dir\" $latest_old $latest_new)\" \\\n \t\t\t$latest_new >&2 || exit $?\n \tfi\n-\tif [ -n \"$branch\" ]; then\n-\t\tif rev_exists \"refs/heads/$branch\"; then\n-\t\t\tif ! rev_is_descendant_of_branch $latest_new $branch; then\n+\tif test -n \"$branch\"\n+\tthen\n+\t\tif rev_exists \"refs/heads/$branch\"\n+\t\tthen\n+\t\t\tif ! rev_is_descendant_of_branch $latest_new $branch\n+\t\t\tthen\n \t\t\t\tdie \"Branch '$branch' is not an ancestor of commit '$latest_new'.\"\n \t\t\tfi\n \t\t\taction='Updated'\n \t\telse\n \t\t\taction='Created'\n \t\tfi\n-\t\tgit update-ref -m 'subtree split' \"refs/heads/$branch\" $latest_new || exit $?\n+\t\tgit update-ref -m 'subtree split' \\\n+\t\t\t\"refs/heads/$branch\" $latest_new || exit $?\n \t\tsay \"$action branch '$branch'\"\n \tfi\n \techo $latest_new\n@@ -686,22 +813,26 @@ cmd_merge()\n {\n \trevs=$(git rev-parse $default --revs-only \"$@\") || exit $?\n \tensure_clean\n-\t\n+\n \tset -- $revs\n-\tif [ $# -ne 1 ]; then\n+\tif test $# -ne 1\n+\tthen\n \t\tdie \"You must provide exactly one revision.  Got: '$revs'\"\n \tfi\n \trev=\"$1\"\n-\t\n-\tif [ -n \"$squash\" ]; then\n+\n+\tif test -n \"$squash\"\n+\tthen\n \t\tfirst_split=\"$(find_latest_squash \"$dir\")\"\n-\t\tif [ -z \"$first_split\" ]; then\n+\t\tif test -z \"$first_split\"\n+\t\tthen\n \t\t\tdie \"Can't squash-merge: '$dir' was never added.\"\n \t\tfi\n \t\tset $first_split\n \t\told=$1\n \t\tsub=$2\n-\t\tif [ \"$sub\" = \"$rev\" ]; then\n+\t\tif test \"$sub\" = \"$rev\"\n+\t\tthen\n \t\t\tsay \"Subtree is already at commit $rev.\"\n \t\t\texit 0\n \t\tfi\n@@ -711,15 +842,19 @@ cmd_merge()\n \tfi\n \n \tversion=$(git version)\n-\tif [ \"$version\" \\< \"git version 1.7\" ]; then\n-\t\tif [ -n \"$message\" ]; then\n-\t\t\tgit merge -s subtree --message=\"$message\" $rev\n+\tif test \"$version\" \\< \"git version 1.7\"\n+\tthen\n+\t\tif test -n \"$message\"\n+\t\tthen\n+\t\t\tgit merge -s subtree --message=\"$message\" \"$rev\"\n \t\telse\n-\t\t\tgit merge -s subtree $rev\n+\t\t\tgit merge -s subtree \"$rev\"\n \t\tfi\n \telse\n-\t\tif [ -n \"$message\" ]; then\n-\t\t\tgit merge -Xsubtree=\"$prefix\" --message=\"$message\" $rev\n+\t\tif test -n \"$message\"\n+\t\tthen\n+\t\t\tgit merge -Xsubtree=\"$prefix\" \\\n+\t\t\t\t--message=\"$message\" \"$rev\"\n \t\telse\n \t\t\tgit merge -Xsubtree=\"$prefix\" $rev\n \t\tfi\n@@ -728,8 +863,9 @@ cmd_merge()\n \n cmd_pull()\n {\n-\tif [ $# -ne 2 ]; then\n-\t    die \"You must provide <repository> <ref>\"\n+\tif test $# -ne 2\n+\tthen\n+\t\tdie \"You must provide <repository> <ref>\"\n \tfi\n \tensure_clean\n \tensure_valid_ref_format \"$2\"\n@@ -741,18 +877,20 @@ cmd_pull()\n \n cmd_push()\n {\n-\tif [ $# -ne 2 ]; then\n-\t    die \"You must provide <repository> <ref>\"\n+\tif test $# -ne 2\n+\tthen\n+\t\tdie \"You must provide <repository> <ref>\"\n \tfi\n \tensure_valid_ref_format \"$2\"\n-\tif [ -e \"$dir\" ]; then\n-\t    repository=$1\n-\t    refspec=$2\n-\t    echo \"git push using: \" $repository $refspec\n-\t    localrev=$(git subtree split --prefix=\"$prefix\") || die\n-\t    git push \"$repository\" $localrev:refs/heads/$refspec\n+\tif test -e \"$dir\"\n+\tthen\n+\t\trepository=$1\n+\t\trefspec=$2\n+\t\techo \"git push using: \" \"$repository\" \"$refspec\"\n+\t\tlocalrev=$(git subtree split --prefix=\"$prefix\") || die\n+\t\tgit push \"$repository\" \"$localrev\":\"refs/heads/$refspec\"\n \telse\n-\t    die \"'$dir' must already exist. Try 'git subtree add'.\"\n+\t\tdie \"'$dir' must already exist. Try 'git subtree add'.\"\n \tfi\n }\n \n-- \n2.9.2.466.g8c6d1f9.dirty\n\n"},{"id":"292175","messageId":"CAPig+cRGz2eqPK0uaih3hYCV0eLayot3VGyjKaz5Qf8Jy5o=DA@mail.gmail.com","threadId":"42926","inReplyTo":"20160726041416.9438-1-davvid@gmail.com","subject":"Re: [PATCH 1/3] t7900-subtree.sh: fix quoting and broken && chains","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-07-26T06:06:15Z","receivedAt":"2016-07-26T06:06:27Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Jul 26, 2016 at 12:14 AM, David Aguilar <davvid@gmail.com> wrote:\n> Allow whitespace in arguments to subtree_test_create_repo.\n> Add missing && chains.\n>\n> Signed-off-by: David Aguilar <davvid@gmail.com>\n> ---\n> diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh\n> @@ -16,16 +16,16 @@ export TEST_DIRECTORY\n>  subtree_test_create_repo()\n>  {\n> -       test_create_repo \"$1\"\n> +       test_create_repo \"$1\" &&\n>         (\n> -               cd $1\n> +               cd \"$1\" &&\n\nThanks, I noticed this in December 2015 while reviewing a patch on the\nlist and have had a patch to fix it sitting in my queue since then but\nnever found time to formalize it.\n\n>                 git config log.date relative\n>         )\n>  }\n>\n>  create()\n>  {\n> -       echo \"$1\" >\"$1\"\n> +       echo \"$1\" >\"$1\" &&\n>         git add \"$1\"\n>  }\n>\n> @@ -73,10 +73,10 @@ join_commits()\n>  test_create_commit() (\n>         repo=$1\n>         commit=$2\n\nPerhaps &&-chain the above two lines also to future-proof against\nsomeone inserting important code somewhere above the following 'cd'.\n\n> -       cd \"$repo\"\n> -       mkdir -p $(dirname \"$commit\") \\\n> +       cd \"$repo\" &&\n> +       mkdir -p \"$(dirname \"$commit\")\" \\\n>         || error \"Could not create directory for commit\"\n> -       echo \"$commit\" >\"$commit\"\n> +       echo \"$commit\" >\"$commit\" &&\n>         git add \"$commit\" || error \"Could not add commit\"\n>         git commit -m \"$commit\" || error \"Could not commit\"\n>  )\n"},{"id":"292176","messageId":"a12604f4-7dc9-7c35-0853-c3bffd734281@kdbg.org","threadId":"42926","inReplyTo":"20160726041416.9438-3-davvid@gmail.com","subject":"Re: [PATCH 3/3] subtree: adjust style to match CodingGuidelines","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-07-26T06:12:39Z","receivedAt":"2016-07-26T06:13:11Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"These caught my eye browsing through my inbox. I'm not a subtree user.\n\nAm 26.07.2016 um 06:14 schrieb David Aguilar:\n> @@ -50,87 +51,145 @@ prefix=\n>\n>  debug()\n>  {\n> -\tif [ -n \"$debug\" ]; then\n> -\t\tprintf \"%s\\n\" \"$*\" >&2\n> +\tif test -n \"$debug\"\n> +\tthen\n> +\t\tprintf \"%s\\n\" \"$@\" >&2\n\nAre you sure you want this? It prints each argument of the 'debug' \ninvocation on its own line.\n\n>  \tfi\n>  }\n>\n>  say()\n>  {\n> -\tif [ -z \"$quiet\" ]; then\n> -\t\tprintf \"%s\\n\" \"$*\" >&2\n> +\tif test -z \"$quiet\"\n> +\tthen\n> +\t\tprintf \"%s\\n\" \"$@\" >&2\n\nSame here.\n\n>  \tfi\n>  }\n>\n>  progress()\n>  {\n> -\tif [ -z \"$quiet\" ]; then\n> -\t\tprintf \"%s\\r\" \"$*\" >&2\n> +\tif test -z \"$quiet\"\n> +\tthen\n> +\t\tprintf \"%s\\r\" \"$@\" >&2\n\nBut here I'm pretty sure that this is not wanted; the original is \nclearly correct.\n\n>  \tfi\n>  }\n...\n> @@ -139,22 +198,27 @@ debug \"command: {$command}\"\n>  debug \"quiet: {$quiet}\"\n>  debug \"revs: {$revs}\"\n>  debug \"dir: {$dir}\"\n> -debug \"opts: {$*}\"\n> +debug \"opts: {$@}\"\n\nWhen the arguments of a script or function are to be printed for the \nuser's entertainment/education, then it is safer (and, therefore, \nidiomatic) to use \"$*\".\n\n>  debug\n...\n>  cache_get()\n>  {\n> -\tfor oldrev in $*; do\n> -\t\tif [ -r \"$cachedir/$oldrev\" ]; then\n> +\tfor oldrev in \"$@\"\n> +\tdo\n\nIt is idiomatic to write this as\n\n\tfor oldrev\n\tdo\n\n(But your move from bare $* to quoted \"$@\" fits better under the \"fix \nquoting\" topic of this patch.)\n\n> +\t\tif test -r \"$cachedir/$oldrev\"\n> +\t\tthen\n>  \t\t\tread newrev <\"$cachedir/$oldrev\"\n>  \t\t\techo $newrev\n>  \t\tfi\n...\n> @@ -631,17 +749,19 @@ cmd_split()\n>  \t\tdebug \"  parents: $parents\"\n>  \t\tnewparents=$(cache_get $parents)\n>  \t\tdebug \"  newparents: $newparents\"\n> -\t\t\n> +\n>  \t\ttree=$(subtree_for_commit $rev \"$dir\")\n>  \t\tdebug \"  tree is: $tree\"\n>\n>  \t\tcheck_parents $parents\n> -\t\t\n> +\n>  \t\t# ugly.  is there no better way to tell if this is a subtree\n>  \t\t# vs. a mainline commit?  Does it matter?\n> -\t\tif [ -z $tree ]; then\n> +\t\tif test -z $tree\n\nThis works by accident. When $tree is empty, this reduces to 'test -z', \nwhich happens to evaluate to true, just what we want. But it be \nappropriate to put $tree in double-quotes nevertheless.\n\n> +\t\tthen\n>  \t\t\tset_notree $rev\n> -\t\t\tif [ -n \"$newparents\" ]; then\n> +\t\t\tif test -n \"$newparents\"\n> +\t\t\tthen\n>  \t\t\t\tcache_set $rev $rev\n>  \t\t\tfi\n>  \t\t\tcontinue\n\n-- Hannes\n\n"},{"id":"292178","messageId":"xmqqd1m03nio.fsf@gitster.mtv.corp.google.com","threadId":"42926","inReplyTo":"CAPig+cRGz2eqPK0uaih3hYCV0eLayot3VGyjKaz5Qf8Jy5o=DA@mail.gmail.com","subject":"Re: [PATCH 1/3] t7900-subtree.sh: fix quoting and broken && chains","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-26T06:46:07Z","receivedAt":"2016-07-26T06:46:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n>> @@ -73,10 +73,10 @@ join_commits()\n>>  test_create_commit() (\n>>         repo=$1\n>>         commit=$2\n>\n> Perhaps &&-chain the above two lines also to future-proof against\n> someone inserting important code somewhere above the following 'cd'.\n\nYup, also we can have them on a single line, i.e.\n\n\trepo=$1 commit=$2 &&\n\tcd \"$repo\" &&\n        ...\n\nThanks\n\n>\n>> -       cd \"$repo\"\n>> -       mkdir -p $(dirname \"$commit\") \\\n>> +       cd \"$repo\" &&\n>> +       mkdir -p \"$(dirname \"$commit\")\" \\\n>>         || error \"Could not create directory for commit\"\n>> -       echo \"$commit\" >\"$commit\"\n>> +       echo \"$commit\" >\"$commit\" &&\n>>         git add \"$commit\" || error \"Could not add commit\"\n>>         git commit -m \"$commit\" || error \"Could not commit\"\n>>  )\n"},{"id":"292270","messageId":"xmqqfuqwyvac.fsf@gitster.mtv.corp.google.com","threadId":"42926","inReplyTo":"a12604f4-7dc9-7c35-0853-c3bffd734281@kdbg.org","subject":"Re: [PATCH 3/3] subtree: adjust style to match CodingGuidelines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-26T20:54:51Z","receivedAt":"2016-07-26T20:56:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> These caught my eye browsing through my inbox. I'm not a subtree user.\n\nAll good comments.\n\nLet's queue 1/3 and 2/3 and fast-track them down to 'master'.  Style\nfixes can come independently later.\n\nThanks.\n\n> Am 26.07.2016 um 06:14 schrieb David Aguilar:\n>> @@ -50,87 +51,145 @@ prefix=\n>>\n>>  debug()\n>>  {\n>> -\tif [ -n \"$debug\" ]; then\n>> -\t\tprintf \"%s\\n\" \"$*\" >&2\n>> +\tif test -n \"$debug\"\n>> +\tthen\n>> +\t\tprintf \"%s\\n\" \"$@\" >&2\n>\n> Are you sure you want this? It prints each argument of the 'debug'\n> invocation on its own line.\n>\n>>  \tfi\n>>  }\n>>\n>>  say()\n>>  {\n>> -\tif [ -z \"$quiet\" ]; then\n>> -\t\tprintf \"%s\\n\" \"$*\" >&2\n>> +\tif test -z \"$quiet\"\n>> +\tthen\n>> +\t\tprintf \"%s\\n\" \"$@\" >&2\n>\n> Same here.\n>\n>>  \tfi\n>>  }\n>>\n>>  progress()\n>>  {\n>> -\tif [ -z \"$quiet\" ]; then\n>> -\t\tprintf \"%s\\r\" \"$*\" >&2\n>> +\tif test -z \"$quiet\"\n>> +\tthen\n>> +\t\tprintf \"%s\\r\" \"$@\" >&2\n>\n> But here I'm pretty sure that this is not wanted; the original is\n> clearly correct.\n>\n>>  \tfi\n>>  }\n> ...\n>> @@ -139,22 +198,27 @@ debug \"command: {$command}\"\n>>  debug \"quiet: {$quiet}\"\n>>  debug \"revs: {$revs}\"\n>>  debug \"dir: {$dir}\"\n>> -debug \"opts: {$*}\"\n>> +debug \"opts: {$@}\"\n>\n> When the arguments of a script or function are to be printed for the\n> user's entertainment/education, then it is safer (and, therefore,\n> idiomatic) to use \"$*\".\n>\n>>  debug\n> ...\n>>  cache_get()\n>>  {\n>> -\tfor oldrev in $*; do\n>> -\t\tif [ -r \"$cachedir/$oldrev\" ]; then\n>> +\tfor oldrev in \"$@\"\n>> +\tdo\n>\n> It is idiomatic to write this as\n>\n> \tfor oldrev\n> \tdo\n>\n> (But your move from bare $* to quoted \"$@\" fits better under the \"fix\n> quoting\" topic of this patch.)\n>\n>> +\t\tif test -r \"$cachedir/$oldrev\"\n>> +\t\tthen\n>>  \t\t\tread newrev <\"$cachedir/$oldrev\"\n>>  \t\t\techo $newrev\n>>  \t\tfi\n> ...\n>> @@ -631,17 +749,19 @@ cmd_split()\n>>  \t\tdebug \"  parents: $parents\"\n>>  \t\tnewparents=$(cache_get $parents)\n>>  \t\tdebug \"  newparents: $newparents\"\n>> -\t\t\n>> +\n>>  \t\ttree=$(subtree_for_commit $rev \"$dir\")\n>>  \t\tdebug \"  tree is: $tree\"\n>>\n>>  \t\tcheck_parents $parents\n>> -\t\t\n>> +\n>>  \t\t# ugly.  is there no better way to tell if this is a subtree\n>>  \t\t# vs. a mainline commit?  Does it matter?\n>> -\t\tif [ -z $tree ]; then\n>> +\t\tif test -z $tree\n>\n> This works by accident. When $tree is empty, this reduces to 'test\n> -z', which happens to evaluate to true, just what we want. But it be\n> appropriate to put $tree in double-quotes nevertheless.\n>\n>> +\t\tthen\n>>  \t\t\tset_notree $rev\n>> -\t\t\tif [ -n \"$newparents\" ]; then\n>> +\t\t\tif test -n \"$newparents\"\n>> +\t\t\tthen\n>>  \t\t\t\tcache_set $rev $rev\n>>  \t\t\tfi\n>>  \t\t\tcontinue\n>\n> -- Hannes\n"}]}