{"thread":{"id":"42306","subject":"t3404 static check of bad SHA-1 failure","startedAt":"2016-05-13T16:09:35Z","lastAt":"2016-05-14T19:51:50Z","messageCount":19,"participants":["Armin Kunaschik","Jeff King","Junio C Hamano","Eric Wong","Eric Sunshine"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"286461","messageId":"CALR6jEiH6oxq=KXfz1pqOue9VKnkp=S8zNqC4OFmbuhRFFxoMw@mail.gmail.com","threadId":"42306","inReplyTo":null,"subject":"t3404 static check of bad SHA-1 failure","fromName":"Armin Kunaschik","fromEmail":"megabreit@googlemail.com","sentAt":"2016-05-13T16:09:35Z","receivedAt":"2016-05-13T16:09:35Z","isPatch":false,"sender":{"key":"megabreit@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/10429218?v=4"},"body":"Hello,\n\nin t3404 test 91 - static check of bad SHA-1 fails (with ksh) with a\nsyntax error in git-rebase.\ngit-rebase[6]: test: argument expected\n\nReason is, again, a test -z without quotes in git-rebase--interactive:\n*** ../git-rebase--interactive.orig     Tue May 10 17:36:59 2016\n--- ../git-rebase--interactive  Fri May 13 17:57:27 2016\n***************\n*** 870,876 ****\n                badsha=1\n        else\n                sha1_verif=\"$(git rev-parse --verify --quiet $1^{commit})\"\n!               if test -z $sha1_verif\n                then\n                        badsha=1\n                fi\n--- 870,876 ----\n                badsha=1\n        else\n                sha1_verif=\"$(git rev-parse --verify --quiet $1^{commit})\"\n!               if test -z \"$sha1_verif\"\n                then\n                        badsha=1\n                fi\n\nMaybe it would be a good idea, to quote the test -z $1 in the same\nfunction check_commit_sha too.\nThe test completes successfully without it, but since it's an user\noption, I think it should be quoted.\nThen there would be no more unquoted test -z anymore :-)\n\nArmin\n"},{"id":"286477","messageId":"20160513182325.GB30700@sigill.intra.peff.net","threadId":"42306","inReplyTo":"CALR6jEiH6oxq=KXfz1pqOue9VKnkp=S8zNqC4OFmbuhRFFxoMw@mail.gmail.com","subject":"Re: t3404 static check of bad SHA-1 failure","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-05-13T18:23:26Z","receivedAt":"2016-05-13T18:23:26Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 13, 2016 at 06:09:35PM +0200, Armin Kunaschik wrote:\n\n> in t3404 test 91 - static check of bad SHA-1 fails (with ksh) with a\n> syntax error in git-rebase.\n> git-rebase[6]: test: argument expected\n\nHere's a fix that covers these and another I found:\n\n-- >8 --\nSubject: [PATCH] always quote arguments to \"test -z\" in shell\n\nModern shells are pretty forgiving about us doing:\n\n  test -z $foo\n\nIf $foo is indeed empty, the test command will see only:\n\n  test -z\n\nand treat the missing argument as \"yes, this is empty\". But\nsome older shells, reportedly ksh88, complain about the\nmissing argument. We can be more portable by spelling this\nas:\n\n  test -z \"$foo\"\n\nso that \"test\" sees an empty argument, not a missing one.\n\nThis covers all cases detected by:\n\n  git grep 'test -z [^\"]'\n\n(though note that has a few false positives for tests which\nneed an extra layer of quoting to do '\\\"').\n\nReported-by: Armin Kunaschik <megabreit@googlemail.com>\nSigned-off-by: Jeff King <peff@peff.net>\n---\nActually, this misses the case in t4151 which already has a fix queued\non pu. Arguably these should all just be squashed together (and I am\nhappy, Junio, if you want to do so and leave Armin as the author of the\nnew commit).\n\n git-rebase--interactive.sh | 4 ++--\n git-stash.sh               | 2 +-\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 9ea3075..470413b 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -866,12 +866,12 @@ add_exec_commands () {\n # $3: the input filename\n check_commit_sha () {\n \tbadsha=0\n-\tif test -z $1\n+\tif test -z \"$1\"\n \tthen\n \t\tbadsha=1\n \telse\n \t\tsha1_verif=\"$(git rev-parse --verify --quiet $1^{commit})\"\n-\t\tif test -z $sha1_verif\n+\t\tif test -z \"$sha1_verif\"\n \t\tthen\n \t\t\tbadsha=1\n \t\tfi\ndiff --git a/git-stash.sh b/git-stash.sh\nindex c7c65e2..57f9dc1 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -185,7 +185,7 @@ store_stash () {\n \n \tgit update-ref --create-reflog -m \"$stash_msg\" $ref_stash $w_commit\n \tret=$?\n-\ttest $ret != 0 && test -z $quiet &&\n+\ttest $ret != 0 && test -z \"$quiet\" &&\n \tdie \"$(eval_gettext \"Cannot update \\$ref_stash with \\$w_commit\")\"\n \treturn $ret\n }\n-- \n2.8.2.825.gea31738\n"},{"id":"286488","messageId":"xmqqwpmx91mb.fsf@gitster.mtv.corp.google.com","threadId":"42306","inReplyTo":"20160513182325.GB30700@sigill.intra.peff.net","subject":"Re: t3404 static check of bad SHA-1 failure","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-13T19:52:44Z","receivedAt":"2016-05-13T19:52:44Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Fri, May 13, 2016 at 06:09:35PM +0200, Armin Kunaschik wrote:\n>\n>> in t3404 test 91 - static check of bad SHA-1 fails (with ksh) with a\n>> syntax error in git-rebase.\n>> git-rebase[6]: test: argument expected\n>\n> Here's a fix that covers these and another I found:\n>\n> -- >8 --\n> Subject: [PATCH] always quote arguments to \"test -z\" in shell\n>\n> Modern shells are pretty forgiving about us doing:\n>\n>   test -z $foo\n>\n> If $foo is indeed empty, the test command will see only:\n>\n>   test -z\n>\n> and treat the missing argument as \"yes, this is empty\". But\n> some older shells, reportedly ksh88, complain about the\n> missing argument. We can be more portable by spelling this\n> as:\n>\n>   test -z \"$foo\"\n>\n> so that \"test\" sees an empty argument, not a missing one.\n>\n> This covers all cases detected by:\n>\n>   git grep 'test -z [^\"]'\n>\n> (though note that has a few false positives for tests which\n> need an extra layer of quoting to do '\\\"').\n>\n> Reported-by: Armin Kunaschik <megabreit@googlemail.com>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> Actually, this misses the case in t4151 which already has a fix queued\n> on pu. Arguably these should all just be squashed together (and I am\n> happy, Junio, if you want to do so and leave Armin as the author of the\n> new commit).\n\nI _think_ \"test -z\" should succeed according to POSIX, because\n\n (1) it is not \"test -z string\" because it lacks string,\n\n (2) it is not any of the other \"test -<option> thing\" because -z,\n    and\n\n (3) the only thing it matches in the supported form of \"test\" is\n     \"test <string>\" that tests if the <string> is not the null\n     string, and \"-z\" indeed is not the null string.\n\nFor the same reason, \"test -n\" succeeds.\n\nBut working around older/broken shells is easy and the resulting\nscript it more readable, so let's take this.  It makes the resulting\ncode easier to understand even when we know we run it under POSIX\nshell.\n\nThanks.\n\n>  git-rebase--interactive.sh | 4 ++--\n>  git-stash.sh               | 2 +-\n>  2 files changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n> index 9ea3075..470413b 100644\n> --- a/git-rebase--interactive.sh\n> +++ b/git-rebase--interactive.sh\n> @@ -866,12 +866,12 @@ add_exec_commands () {\n>  # $3: the input filename\n>  check_commit_sha () {\n>  \tbadsha=0\n> -\tif test -z $1\n> +\tif test -z \"$1\"\n>  \tthen\n>  \t\tbadsha=1\n>  \telse\n>  \t\tsha1_verif=\"$(git rev-parse --verify --quiet $1^{commit})\"\n> -\t\tif test -z $sha1_verif\n> +\t\tif test -z \"$sha1_verif\"\n>  \t\tthen\n>  \t\t\tbadsha=1\n>  \t\tfi\n> diff --git a/git-stash.sh b/git-stash.sh\n> index c7c65e2..57f9dc1 100755\n> --- a/git-stash.sh\n> +++ b/git-stash.sh\n> @@ -185,7 +185,7 @@ store_stash () {\n>  \n>  \tgit update-ref --create-reflog -m \"$stash_msg\" $ref_stash $w_commit\n>  \tret=$?\n> -\ttest $ret != 0 && test -z $quiet &&\n> +\ttest $ret != 0 && test -z \"$quiet\" &&\n>  \tdie \"$(eval_gettext \"Cannot update \\$ref_stash with \\$w_commit\")\"\n>  \treturn $ret\n>  }\n"},{"id":"286490","messageId":"20160513195911.GE9890@sigill.intra.peff.net","threadId":"42306","inReplyTo":"xmqqwpmx91mb.fsf@gitster.mtv.corp.google.com","subject":"Re: t3404 static check of bad SHA-1 failure","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-05-13T19:59:11Z","receivedAt":"2016-05-13T19:59:11Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 13, 2016 at 12:52:44PM -0700, Junio C Hamano wrote:\n\n> I _think_ \"test -z\" should succeed according to POSIX, because\n> \n>  (1) it is not \"test -z string\" because it lacks string,\n> \n>  (2) it is not any of the other \"test -<option> thing\" because -z,\n>     and\n> \n>  (3) the only thing it matches in the supported form of \"test\" is\n>      \"test <string>\" that tests if the <string> is not the null\n>      string, and \"-z\" indeed is not the null string.\n> \n> For the same reason, \"test -n\" succeeds.\n\nYeah, I think you're right; POSIX is pretty clear that this falls under\ncase 3. So that means \"test -z\" quietly does what we want. But it means\nthat \"test -n\" does the _opposite_ of what we want.\n\nAnd sadly,\n\n  git grep 'test -n [^\"]'\n\nis not empty.\n\n> But working around older/broken shells is easy and the resulting\n> script it more readable, so let's take this.  It makes the resulting\n> code easier to understand even when we know we run it under POSIX\n> shell.\n\nYep. The POSIX-explanation of what is going on might be worth putting in\nthe commit message for the \"-z\" case (i.e., it should work, but the\n\"why\" is subtle).\n\n-Peff\n"},{"id":"286491","messageId":"20160513200232.GA10371@sigill.intra.peff.net","threadId":"42306","inReplyTo":"20160513195911.GE9890@sigill.intra.peff.net","subject":"Re: t3404 static check of bad SHA-1 failure","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-05-13T20:02:33Z","receivedAt":"2016-05-13T20:02:33Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 13, 2016 at 03:59:11PM -0400, Jeff King wrote:\n\n> On Fri, May 13, 2016 at 12:52:44PM -0700, Junio C Hamano wrote:\n> \n> > I _think_ \"test -z\" should succeed according to POSIX, because\n> > \n> >  (1) it is not \"test -z string\" because it lacks string,\n> > \n> >  (2) it is not any of the other \"test -<option> thing\" because -z,\n> >     and\n> > \n> >  (3) the only thing it matches in the supported form of \"test\" is\n> >      \"test <string>\" that tests if the <string> is not the null\n> >      string, and \"-z\" indeed is not the null string.\n> > \n> > For the same reason, \"test -n\" succeeds.\n> \n> Yeah, I think you're right; POSIX is pretty clear that this falls under\n> case 3. So that means \"test -z\" quietly does what we want. But it means\n> that \"test -n\" does the _opposite_ of what we want.\n> \n> And sadly,\n> \n>   git grep 'test -n [^\"]'\n> \n> is not empty.\n> \n> > But working around older/broken shells is easy and the resulting\n> > script it more readable, so let's take this.  It makes the resulting\n> > code easier to understand even when we know we run it under POSIX\n> > shell.\n> \n> Yep. The POSIX-explanation of what is going on might be worth putting in\n> the commit message for the \"-z\" case (i.e., it should work, but the\n> \"why\" is subtle).\n\nI think we can just lump all of these into a single patch, but there are\na few related cleanups, including the SVN stuff I just sent. So let me\nsend out a unified patch series in a moment.\n\n-Peff\n"},{"id":"286492","messageId":"xmqqshxl9142.fsf@gitster.mtv.corp.google.com","threadId":"42306","inReplyTo":"20160513195911.GE9890@sigill.intra.peff.net","subject":"Re: t3404 static check of bad SHA-1 failure","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-13T20:03:41Z","receivedAt":"2016-05-13T20:03:41Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Fri, May 13, 2016 at 12:52:44PM -0700, Junio C Hamano wrote:\n>\n>> I _think_ \"test -z\" should succeed according to POSIX, because\n>> \n>>  (1) it is not \"test -z string\" because it lacks string,\n>> \n>>  (2) it is not any of the other \"test -<option> thing\" because -z,\n>>     and\n>> \n>>  (3) the only thing it matches in the supported form of \"test\" is\n>>      \"test <string>\" that tests if the <string> is not the null\n>>      string, and \"-z\" indeed is not the null string.\n>> \n>> For the same reason, \"test -n\" succeeds.\n>\n> Yeah, I think you're right; POSIX is pretty clear that this falls under\n> case 3. So that means \"test -z\" quietly does what we want. But it means\n> that \"test -n\" does the _opposite_ of what we want.\n\n;-)\n\n> And sadly,\n>\n>   git grep 'test -n [^\"]'\n>\n> is not empty.\n\nAre you doing an audit?  Otherwise I'm interested in taking a brief\nlook.\n\n>> But working around older/broken shells is easy and the resulting\n>> script it more readable, so let's take this.  It makes the resulting\n>> code easier to understand even when we know we run it under POSIX\n>> shell.\n>\n> Yep. The POSIX-explanation of what is going on might be worth putting in\n> the commit message for the \"-z\" case (i.e., it should work, but the\n> \"why\" is subtle).\n\nPerhaps.\n\nA small consolation for ksh fans around is that ksh93 seems to get\nthis case right ;-)\n"},{"id":"286499","messageId":"20160513204654.GA10684@sigill.intra.peff.net","threadId":"42306","inReplyTo":"xmqqshxl9142.fsf@gitster.mtv.corp.google.com","subject":"[PATCH 0/6] test -z/-n quoting fix + misc cleanups","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-05-13T20:46:54Z","receivedAt":"2016-05-13T20:46:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 13, 2016 at 01:03:41PM -0700, Junio C Hamano wrote:\n\n> > And sadly,\n> >\n> >   git grep 'test -n [^\"]'\n> >\n> > is not empty.\n> \n> Are you doing an audit?  Otherwise I'm interested in taking a brief\n> look.\n\nThere was only one buggy case there (in git-stash). The rest were false\npositives.\n\nI didn't audit for:\n\n  test $foo = bar\n\nwhich also has problems. You can grep for:\n\n  git grep 'test \\$'\n\nand there are a lot of hits. Many of them are probably fine, if they are\nvariables that are known to be non-empty and not contain whitespace\n(e.g., $#). But some of them are questionable, like:\n\n  git-request-pull.sh:if test $(git cat-file -t \"$head\") = tag\n\nI suspect in practice that's fine just because we're likely to see\neither the empty string (in which case test will barf with \"unary\noperator expected\", which matches what we want), or a single-word\nresponse (which doesn't need further quoting).\n\n> >> But working around older/broken shells is easy and the resulting\n> >> script it more readable, so let's take this.  It makes the resulting\n> >> code easier to understand even when we know we run it under POSIX\n> >> shell.\n\nActually, it's not just older shells:\n\n  foo='bar baz'\n  test -z $foo\n\nis \"unspecified\" according to POSIX, though in practice it will complain\nabout \"binary operator expected\". You can get some weirdness, though,\nlike:\n\n  foo='!= bar'\n  test -z $foo\n\nwhich returns 0. Unlikely, but still clearly wrong for us not to be\nquoting.\n\nAnyway. Here's a series that fixes the -n/-z cases, along with a bunch\nof cleanups that remove the false positives (most of which I sent out\njust a few minutes ago as \"minor fixes to some svn tests\").\n\n  [1/6]: t/lib-git-svn: drop $remote_git_svn and $git_svn_id\n  [2/6]: t9100,t3419: enclose all test code in single-quotes\n  [3/6]: t9107: use \"return 1\" instead of \"exit 1\"\n  [4/6]: t9107: switch inverted single/double quotes in test\n  [5/6]: t9103: modernize test style\n  [6/6]: always quote shell arguments to test -z/-n\n\nYou could take just 6/6 as its own series; the rest are just about\nremoving the false positives, and fixing other issues. I put it last,\nthough, because otherwise the \"this grep is now empty\" claim in it is\nnot true. :)\n\n-Peff\n"},{"id":"286502","messageId":"20160513204714.GA15391@sigill.intra.peff.net","threadId":"42306","inReplyTo":"20160513204654.GA10684@sigill.intra.peff.net","subject":"[PATCH 1/6] t/lib-git-svn: drop $remote_git_svn and $git_svn_id","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-05-13T20:47:14Z","receivedAt":"2016-05-13T20:47:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"These variables were added in 16805d3 (t/t91XX-svn: start\nremoving use of \"git-\" from these tests, 2008-09-08) so that\nrunning:\n\n  git grep git-\n\nwould return fewer hits. At the time, we were transitioning\naway from the use of the \"dashed\" git-foo form.\n\nThat transition has been over for years, and grepping for\n\"git-\" in the test suite yields thousands of hits anyway\n(all presumably false positives).\n\nWith their original purpose gone, these variables serve only\nto obfuscate the tests. Let's get rid of them.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/lib-git-svn.sh                              |  3 ---\n t/t9100-git-svn-basic.sh                      | 38 +++++++++++++--------------\n t/t9101-git-svn-props.sh                      | 12 ++++-----\n t/t9102-git-svn-deep-rmdir.sh                 |  2 +-\n t/t9106-git-svn-commit-diff-clobber.sh        |  6 ++---\n t/t9107-git-svn-migrate.sh                    | 14 +++++-----\n t/t9110-git-svn-use-svm-props.sh              | 18 ++++++-------\n t/t9111-git-svn-use-svnsync-props.sh          | 18 ++++++-------\n t/t9120-git-svn-clone-with-percent-escapes.sh |  6 ++---\n t/t9123-git-svn-rebuild-with-rewriteroot.sh   |  2 +-\n t/t9153-git-svn-rewrite-uuid.sh               |  4 +--\n 11 files changed, 60 insertions(+), 63 deletions(-)\n\ndiff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh\nindex 6a50b87..fb88232 100644\n--- a/t/lib-git-svn.sh\n+++ b/t/lib-git-svn.sh\n@@ -1,8 +1,5 @@\n . ./test-lib.sh\n \n-remotes_git_svn=remotes/git\"\"-svn\n-git_svn_id=git\"\"-svn-id\n-\n if test -n \"$NO_SVN_TESTS\"\n then\n \tskip_all='skipping git svn tests, NO_SVN_TESTS defined'\ndiff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh\nindex 22d8367..6ec73ee 100755\n--- a/t/t9100-git-svn-basic.sh\n+++ b/t/t9100-git-svn-basic.sh\n@@ -45,13 +45,13 @@ test_expect_success \"checkout from svn\" 'svn co \"$svnrepo\" \"$SVN_TREE\"'\n \n name='try a deep --rmdir with a commit'\n test_expect_success \"$name\" '\n-\tgit checkout -f -b mybranch ${remotes_git_svn} &&\n+\tgit checkout -f -b mybranch remotes/git-svn &&\n \tmv dir/a/b/c/d/e/file dir/file &&\n \tcp dir/file file &&\n \tgit update-index --add --remove dir/a/b/c/d/e/file dir/file file &&\n \tgit commit -m \"$name\" &&\n \tgit svn set-tree --find-copies-harder --rmdir \\\n-\t\t${remotes_git_svn}..mybranch &&\n+\t\tremotes/git-svn..mybranch &&\n \tsvn_cmd up \"$SVN_TREE\" &&\n \ttest -d \"$SVN_TREE\"/dir && test ! -d \"$SVN_TREE\"/dir/a'\n \n@@ -65,14 +65,14 @@ test_expect_success \"$name\" \"\n \tgit update-index --add dir/file/file &&\n \tgit commit -m '$name' &&\n \ttest_must_fail git svn set-tree --find-copies-harder --rmdir \\\n-\t\t${remotes_git_svn}..mybranch\n+\t\tremotes/git-svn..mybranch\n \"\n \n \n name='detect node change from directory to file #1'\n test_expect_success \"$name\" '\n \trm -rf dir \"$GIT_DIR\"/index &&\n-\tgit checkout -f -b mybranch2 ${remotes_git_svn} &&\n+\tgit checkout -f -b mybranch2 remotes/git-svn &&\n \tmv bar/zzz zzz &&\n \trm -rf bar &&\n \tmv zzz bar &&\n@@ -80,14 +80,14 @@ test_expect_success \"$name\" '\n \tgit update-index --add -- bar &&\n \tgit commit -m \"$name\" &&\n \ttest_must_fail git svn set-tree --find-copies-harder --rmdir \\\n-\t\t${remotes_git_svn}..mybranch2\n+\t\tremotes/git-svn..mybranch2\n '\n \n \n name='detect node change from file to directory #2'\n test_expect_success \"$name\" '\n \trm -f \"$GIT_DIR\"/index &&\n-\tgit checkout -f -b mybranch3 ${remotes_git_svn} &&\n+\tgit checkout -f -b mybranch3 remotes/git-svn &&\n \trm bar/zzz &&\n \tgit update-index --remove bar/zzz &&\n \tmkdir bar/zzz &&\n@@ -95,7 +95,7 @@ test_expect_success \"$name\" '\n \tgit update-index --add bar/zzz/yyy &&\n \tgit commit -m \"$name\" &&\n \tgit svn set-tree --find-copies-harder --rmdir \\\n-\t\t${remotes_git_svn}..mybranch3 &&\n+\t\tremotes/git-svn..mybranch3 &&\n \tsvn_cmd up \"$SVN_TREE\" &&\n \ttest -d \"$SVN_TREE\"/bar/zzz &&\n \ttest -e \"$SVN_TREE\"/bar/zzz/yyy\n@@ -104,7 +104,7 @@ test_expect_success \"$name\" '\n name='detect node change from directory to file #2'\n test_expect_success \"$name\" '\n \trm -f \"$GIT_DIR\"/index &&\n-\tgit checkout -f -b mybranch4 ${remotes_git_svn} &&\n+\tgit checkout -f -b mybranch4 remotes/git-svn &&\n \trm -rf dir &&\n \tgit update-index --remove -- dir/file &&\n \ttouch dir &&\n@@ -112,19 +112,19 @@ test_expect_success \"$name\" '\n \tgit update-index --add -- dir &&\n \tgit commit -m \"$name\" &&\n \ttest_must_fail git svn set-tree --find-copies-harder --rmdir \\\n-\t\t${remotes_git_svn}..mybranch4\n+\t\tremotes/git-svn..mybranch4\n '\n \n \n name='remove executable bit from a file'\n test_expect_success POSIXPERM \"$name\" '\n \trm -f \"$GIT_DIR\"/index &&\n-\tgit checkout -f -b mybranch5 ${remotes_git_svn} &&\n+\tgit checkout -f -b mybranch5 remotes/git-svn &&\n \tchmod -x exec.sh &&\n \tgit update-index exec.sh &&\n \tgit commit -m \"$name\" &&\n \tgit svn set-tree --find-copies-harder --rmdir \\\n-\t\t${remotes_git_svn}..mybranch5 &&\n+\t\tremotes/git-svn..mybranch5 &&\n \tsvn_cmd up \"$SVN_TREE\" &&\n \ttest ! -x \"$SVN_TREE\"/exec.sh'\n \n@@ -135,7 +135,7 @@ test_expect_success POSIXPERM \"$name\" '\n \tgit update-index exec.sh &&\n \tgit commit -m \"$name\" &&\n \tgit svn set-tree --find-copies-harder --rmdir \\\n-\t\t${remotes_git_svn}..mybranch5 &&\n+\t\tremotes/git-svn..mybranch5 &&\n \tsvn_cmd up \"$SVN_TREE\" &&\n \ttest -x \"$SVN_TREE\"/exec.sh'\n \n@@ -147,7 +147,7 @@ test_expect_success SYMLINKS \"$name\" '\n \tgit update-index exec.sh &&\n \tgit commit -m \"$name\" &&\n \tgit svn set-tree --find-copies-harder --rmdir \\\n-\t\t${remotes_git_svn}..mybranch5 &&\n+\t\tremotes/git-svn..mybranch5 &&\n \tsvn_cmd up \"$SVN_TREE\" &&\n \ttest -h \"$SVN_TREE\"/exec.sh'\n \n@@ -159,7 +159,7 @@ test_expect_success POSIXPERM,SYMLINKS \"$name\" '\n \tgit update-index --add file exec-2.sh &&\n \tgit commit -m \"$name\" &&\n \tgit svn set-tree --find-copies-harder --rmdir \\\n-\t\t${remotes_git_svn}..mybranch5 &&\n+\t\tremotes/git-svn..mybranch5 &&\n \tsvn_cmd up \"$SVN_TREE\" &&\n \ttest -x \"$SVN_TREE\"/file &&\n \ttest -h \"$SVN_TREE\"/exec-2.sh'\n@@ -172,7 +172,7 @@ test_expect_success POSIXPERM,SYMLINKS \"$name\" '\n \tgit update-index exec-2.sh &&\n \tgit commit -m \"$name\" &&\n \tgit svn set-tree --find-copies-harder --rmdir \\\n-\t\t${remotes_git_svn}..mybranch5 &&\n+\t\tremotes/git-svn..mybranch5 &&\n \tsvn_cmd up \"$SVN_TREE\" &&\n \ttest -f \"$SVN_TREE\"/exec-2.sh &&\n \ttest ! -h \"$SVN_TREE\"/exec-2.sh &&\n@@ -194,7 +194,7 @@ GIT_SVN_ID=alt\n export GIT_SVN_ID\n test_expect_success \"$name\" \\\n     'git svn init \"$svnrepo\" && git svn fetch &&\n-     git rev-list --pretty=raw ${remotes_git_svn} | grep ^tree | uniq > a &&\n+     git rev-list --pretty=raw remotes/git-svn | grep ^tree | uniq > a &&\n      git rev-list --pretty=raw remotes/alt | grep ^tree | uniq > b &&\n      test_cmp a b'\n \n@@ -219,7 +219,7 @@ test_expect_success POSIXPERM,SYMLINKS \"$name\" \"test_cmp a expected\"\n \n test_expect_success 'exit if remote refs are ambigious' \"\n         git config --add svn-remote.svn.fetch \\\n-                              bar:refs/${remotes_git_svn} &&\n+\t\tbar:refs/remotes/git-svn &&\n \ttest_must_fail git svn migrate\n \"\n \n@@ -227,7 +227,7 @@ test_expect_success 'exit if init-ing a would clobber a URL' '\n         svnadmin create \"${PWD}/svnrepo2\" &&\n         svn mkdir -m \"mkdir bar\" \"${svnrepo}2/bar\" &&\n         git config --unset svn-remote.svn.fetch \\\n-                                \"^bar:refs/${remotes_git_svn}$\" &&\n+\t\t\"^bar:refs/remotes/git-svn$\" &&\n \ttest_must_fail git svn init \"${svnrepo}2/bar\"\n         '\n \n@@ -237,7 +237,7 @@ test_expect_success \\\n         git config --get svn-remote.svn.fetch \\\n                               \"^bar:refs/remotes/bar$\" &&\n         git config --get svn-remote.svn.fetch \\\n-                              \"^:refs/${remotes_git_svn}$\"\n+\t\t\t      \"^:refs/remotes/git-svn$\"\n         '\n \n test_expect_success 'dcommit $rev does not clobber current branch' '\ndiff --git a/t/t9101-git-svn-props.sh b/t/t9101-git-svn-props.sh\nindex e8173d5..07bfb63 100755\n--- a/t/t9101-git-svn-props.sh\n+++ b/t/t9101-git-svn-props.sh\n@@ -73,11 +73,11 @@ test_expect_success 'fetch revisions from svn' 'git svn fetch'\n \n name='test svn:keywords ignoring'\n test_expect_success \"$name\" \\\n-\t'git checkout -b mybranch ${remotes_git_svn} &&\n+\t'git checkout -b mybranch remotes/git-svn &&\n \techo Hi again >> kw.c &&\n \tgit commit -a -m \"test keywords ignoring\" &&\n-\tgit svn set-tree ${remotes_git_svn}..mybranch &&\n-\tgit pull . ${remotes_git_svn}'\n+\tgit svn set-tree remotes/git-svn..mybranch &&\n+\tgit pull . remotes/git-svn'\n \n expect='/* $Id$ */'\n got=\"$(sed -ne 2p kw.c)\"\n@@ -95,7 +95,7 @@ test_expect_success \"propset CR on crlf files\" '\n \n test_expect_success 'fetch and pull latest from svn and checkout a new wc' \\\n \t'git svn fetch &&\n-\t git pull . ${remotes_git_svn} &&\n+\t git pull . remotes/git-svn &&\n \t svn_cmd co \"$svnrepo\" new_wc'\n \n for i in crlf ne_crlf lf ne_lf cr ne_cr empty_cr empty_lf empty empty_crlf\n@@ -117,7 +117,7 @@ cd test_wc\n \t svn_cmd commit -m \"propset CRLF on cr files\"'\n cd ..\n test_expect_success 'fetch and pull latest from svn' \\\n-\t'git svn fetch && git pull . ${remotes_git_svn}'\n+\t'git svn fetch && git pull . remotes/git-svn'\n \n b_cr=\"$(git hash-object cr)\"\n b_ne_cr=\"$(git hash-object ne_cr)\"\n@@ -168,7 +168,7 @@ cat >create-ignore-index.expect <<\\EOF\n EOF\n \n test_expect_success 'test create-ignore' \"\n-\tgit svn fetch && git pull . ${remotes_git_svn} &&\n+\tgit svn fetch && git pull . remotes/git-svn &&\n \tgit svn create-ignore &&\n \tcmp ./.gitignore create-ignore.expect &&\n \tcmp ./deeply/.gitignore create-ignore.expect &&\ndiff --git a/t/t9102-git-svn-deep-rmdir.sh b/t/t9102-git-svn-deep-rmdir.sh\nindex eb70f48..66cd511 100755\n--- a/t/t9102-git-svn-deep-rmdir.sh\n+++ b/t/t9102-git-svn-deep-rmdir.sh\n@@ -17,7 +17,7 @@ test_expect_success 'initialize repo' '\n test_expect_success 'mirror via git svn' '\n \tgit svn init \"$svnrepo\" &&\n \tgit svn fetch &&\n-\tgit checkout -f -b test-rmdir ${remotes_git_svn}\n+\tgit checkout -f -b test-rmdir remotes/git-svn\n \t'\n \n test_expect_success 'Try a commit on rmdir' '\ndiff --git a/t/t9106-git-svn-commit-diff-clobber.sh b/t/t9106-git-svn-commit-diff-clobber.sh\nindex f6d7ac7..dbe8dea 100755\n--- a/t/t9106-git-svn-commit-diff-clobber.sh\n+++ b/t/t9106-git-svn-commit-diff-clobber.sh\n@@ -44,7 +44,7 @@ test_expect_success 'commit complementing change from git' '\n test_expect_success 'dcommit fails to commit because of conflict' '\n \tgit svn init \"$svnrepo\" &&\n \tgit svn fetch &&\n-\tgit reset --hard refs/${remotes_git_svn} &&\n+\tgit reset --hard refs/remotes/git-svn &&\n \tsvn_cmd co \"$svnrepo\" t.svn &&\n \t(\n \t\tcd t.svn &&\n@@ -59,7 +59,7 @@ test_expect_success 'dcommit fails to commit because of conflict' '\n \t'\n \n test_expect_success 'dcommit does the svn equivalent of an index merge' \"\n-\tgit reset --hard refs/${remotes_git_svn} &&\n+\tgit reset --hard refs/remotes/git-svn &&\n \techo 'index merge' > file2 &&\n \tgit update-index --add file2 &&\n \tgit commit -a -m 'index merge' &&\n@@ -81,7 +81,7 @@ test_expect_success 'commit another change from svn side' '\n \t'\n \n test_expect_success 'multiple dcommit from git svn will not clobber svn' \"\n-\tgit reset --hard refs/${remotes_git_svn} &&\n+\tgit reset --hard refs/remotes/git-svn &&\n \techo new file >> new-file &&\n \tgit update-index --add new-file &&\n \tgit commit -a -m 'new file' &&\ndiff --git a/t/t9107-git-svn-migrate.sh b/t/t9107-git-svn-migrate.sh\nindex 9060198..6efc2ab 100755\n--- a/t/t9107-git-svn-migrate.sh\n+++ b/t/t9107-git-svn-migrate.sh\n@@ -19,9 +19,9 @@ test_expect_success 'setup old-looking metadata' '\n \tgit svn init \"$svnrepo\" &&\n \tgit svn fetch &&\n \trm -rf \"$GIT_DIR\"/svn &&\n-\tgit update-ref refs/heads/git-svn-HEAD refs/${remotes_git_svn} &&\n-\tgit update-ref refs/heads/svn-HEAD refs/${remotes_git_svn} &&\n-\tgit update-ref -d refs/${remotes_git_svn} refs/${remotes_git_svn}\n+\tgit update-ref refs/heads/git-svn-HEAD refs/remotes/git-svn &&\n+\tgit update-ref refs/heads/svn-HEAD refs/remotes/git-svn &&\n+\tgit update-ref -d refs/remotes/git-svn refs/remotes/git-svn\n \t'\n \n head=$(git rev-parse --verify refs/heads/git-svn-HEAD^0)\n@@ -35,11 +35,11 @@ test_expect_success 'initialize old-style (v0) git svn layout' '\n \techo \"$svnrepo\" > \"$GIT_DIR\"/svn/info/url &&\n \tgit svn migrate &&\n \t! test -d \"$GIT_DIR\"/git-svn &&\n-\tgit rev-parse --verify refs/${remotes_git_svn}^0 &&\n+\tgit rev-parse --verify refs/remotes/git-svn^0 &&\n \tgit rev-parse --verify refs/remotes/svn^0 &&\n \ttest \"$(git config --get svn-remote.svn.url)\" = \"$svnrepo_escaped\" &&\n \ttest $(git config --get svn-remote.svn.fetch) = \\\n-             \":refs/${remotes_git_svn}\"\n+\t\t\":refs/remotes/git-svn\"\n \t'\n \n test_expect_success 'initialize a multi-repository repo' '\n@@ -66,7 +66,7 @@ test_expect_success 'initialize a multi-repository repo' '\n \tgrep \"^tags/0\\.1:refs/remotes/origin/tags/0\\.1$\" fetch.out &&\n \tgrep \"^tags/0\\.2:refs/remotes/origin/tags/0\\.2$\" fetch.out &&\n \tgrep \"^tags/0\\.3:refs/remotes/origin/tags/0\\.3$\" fetch.out &&\n-\tgrep \"^:refs/${remotes_git_svn}\" fetch.out\n+\tgrep \"^:refs/remotes/git-svn\" fetch.out\n \t'\n \n # refs should all be different, but the trees should all be the same:\n@@ -104,7 +104,7 @@ test_expect_success 'migrate --minimize on old inited layout' '\n \tgrep \"^tags/0\\.1:refs/remotes/origin/tags/0\\.1$\" fetch.out &&\n \tgrep \"^tags/0\\.2:refs/remotes/origin/tags/0\\.2$\" fetch.out &&\n \tgrep \"^tags/0\\.3:refs/remotes/origin/tags/0\\.3$\" fetch.out &&\n-\tgrep \"^:refs/${remotes_git_svn}\" fetch.out\n+\tgrep \"^:refs/remotes/git-svn\" fetch.out\n \t'\n \n test_expect_success  \".rev_db auto-converted to .rev_map.UUID\" '\ndiff --git a/t/t9110-git-svn-use-svm-props.sh b/t/t9110-git-svn-use-svm-props.sh\nindex 29fbdfd..dde0a3c 100755\n--- a/t/t9110-git-svn-use-svm-props.sh\n+++ b/t/t9110-git-svn-use-svm-props.sh\n@@ -22,31 +22,31 @@ uuid=161ce429-a9dd-4828-af4a-52023f968c89\n bar_url=http://mayonaise/svnrepo/bar\n test_expect_success 'verify metadata for /bar' \"\n \tgit cat-file commit refs/remotes/bar | \\\n-\t   grep '^${git_svn_id}: $bar_url@12 $uuid$' &&\n+\t   grep '^git-svn-id: $bar_url@12 $uuid$' &&\n \tgit cat-file commit refs/remotes/bar~1 | \\\n-\t   grep '^${git_svn_id}: $bar_url@11 $uuid$' &&\n+\t   grep '^git-svn-id: $bar_url@11 $uuid$' &&\n \tgit cat-file commit refs/remotes/bar~2 | \\\n-\t   grep '^${git_svn_id}: $bar_url@10 $uuid$' &&\n+\t   grep '^git-svn-id: $bar_url@10 $uuid$' &&\n \tgit cat-file commit refs/remotes/bar~3 | \\\n-\t   grep '^${git_svn_id}: $bar_url@9 $uuid$' &&\n+\t   grep '^git-svn-id: $bar_url@9 $uuid$' &&\n \tgit cat-file commit refs/remotes/bar~4 | \\\n-\t   grep '^${git_svn_id}: $bar_url@6 $uuid$' &&\n+\t   grep '^git-svn-id: $bar_url@6 $uuid$' &&\n \tgit cat-file commit refs/remotes/bar~5 | \\\n-\t   grep '^${git_svn_id}: $bar_url@1 $uuid$'\n+\t   grep '^git-svn-id: $bar_url@1 $uuid$'\n \t\"\n \n e_url=http://mayonaise/svnrepo/dir/a/b/c/d/e\n test_expect_success 'verify metadata for /dir/a/b/c/d/e' \"\n \tgit cat-file commit refs/remotes/e | \\\n-\t   grep '^${git_svn_id}: $e_url@1 $uuid$'\n+\t   grep '^git-svn-id: $e_url@1 $uuid$'\n \t\"\n \n dir_url=http://mayonaise/svnrepo/dir\n test_expect_success 'verify metadata for /dir' \"\n \tgit cat-file commit refs/remotes/dir | \\\n-\t   grep '^${git_svn_id}: $dir_url@2 $uuid$' &&\n+\t   grep '^git-svn-id: $dir_url@2 $uuid$' &&\n \tgit cat-file commit refs/remotes/dir~1 | \\\n-\t   grep '^${git_svn_id}: $dir_url@1 $uuid$'\n+\t   grep '^git-svn-id: $dir_url@1 $uuid$'\n \t\"\n \n test_expect_success 'find commit based on SVN revision number' \"\ndiff --git a/t/t9111-git-svn-use-svnsync-props.sh b/t/t9111-git-svn-use-svnsync-props.sh\nindex bd081c2..22b6e5e 100755\n--- a/t/t9111-git-svn-use-svnsync-props.sh\n+++ b/t/t9111-git-svn-use-svnsync-props.sh\n@@ -21,31 +21,31 @@ uuid=161ce429-a9dd-4828-af4a-52023f968c89\n bar_url=http://mayonaise/svnrepo/bar\n test_expect_success 'verify metadata for /bar' \"\n \tgit cat-file commit refs/remotes/bar | \\\n-\t   grep '^${git_svn_id}: $bar_url@12 $uuid$' &&\n+\t   grep '^git-svn-id: $bar_url@12 $uuid$' &&\n \tgit cat-file commit refs/remotes/bar~1 | \\\n-\t   grep '^${git_svn_id}: $bar_url@11 $uuid$' &&\n+\t   grep '^git-svn-id: $bar_url@11 $uuid$' &&\n \tgit cat-file commit refs/remotes/bar~2 | \\\n-\t   grep '^${git_svn_id}: $bar_url@10 $uuid$' &&\n+\t   grep '^git-svn-id: $bar_url@10 $uuid$' &&\n \tgit cat-file commit refs/remotes/bar~3 | \\\n-\t   grep '^${git_svn_id}: $bar_url@9 $uuid$' &&\n+\t   grep '^git-svn-id: $bar_url@9 $uuid$' &&\n \tgit cat-file commit refs/remotes/bar~4 | \\\n-\t   grep '^${git_svn_id}: $bar_url@6 $uuid$' &&\n+\t   grep '^git-svn-id: $bar_url@6 $uuid$' &&\n \tgit cat-file commit refs/remotes/bar~5 | \\\n-\t   grep '^${git_svn_id}: $bar_url@1 $uuid$'\n+\t   grep '^git-svn-id: $bar_url@1 $uuid$'\n \t\"\n \n e_url=http://mayonaise/svnrepo/dir/a/b/c/d/e\n test_expect_success 'verify metadata for /dir/a/b/c/d/e' \"\n \tgit cat-file commit refs/remotes/e | \\\n-\t   grep '^${git_svn_id}: $e_url@1 $uuid$'\n+\t   grep '^git-svn-id: $e_url@1 $uuid$'\n \t\"\n \n dir_url=http://mayonaise/svnrepo/dir\n test_expect_success 'verify metadata for /dir' \"\n \tgit cat-file commit refs/remotes/dir | \\\n-\t   grep '^${git_svn_id}: $dir_url@2 $uuid$' &&\n+\t   grep '^git-svn-id: $dir_url@2 $uuid$' &&\n \tgit cat-file commit refs/remotes/dir~1 | \\\n-\t   grep '^${git_svn_id}: $dir_url@1 $uuid$'\n+\t   grep '^git-svn-id: $dir_url@1 $uuid$'\n \t\"\n \n test_done\ndiff --git a/t/t9120-git-svn-clone-with-percent-escapes.sh b/t/t9120-git-svn-clone-with-percent-escapes.sh\nindex 1c84ce1..59465b1 100755\n--- a/t/t9120-git-svn-clone-with-percent-escapes.sh\n+++ b/t/t9120-git-svn-clone-with-percent-escapes.sh\n@@ -22,7 +22,7 @@ test_expect_success 'test clone with percent escapes' '\n \tgit svn clone \"$svnrepo/pr%20ject\" clone &&\n \t(\n \t\tcd clone &&\n-\t\tgit rev-parse refs/${remotes_git_svn}\n+\t\tgit rev-parse refs/remotes/git-svn\n \t)\n '\n \n@@ -42,7 +42,7 @@ test_expect_success 'test clone trunk with percent escapes and minimize-url' '\n \tgit svn clone --minimize-url \"$svnrepo/pr%20ject/trunk\" minimize &&\n \t(\n \t\tcd minimize &&\n-\t\tgit rev-parse refs/${remotes_git_svn}\n+\t\tgit rev-parse refs/remotes/git-svn\n \t)\n '\n \n@@ -50,7 +50,7 @@ test_expect_success 'test clone trunk with percent escapes' '\n \tgit svn clone \"$svnrepo/pr%20ject/trunk\" trunk &&\n \t(\n \t\tcd trunk &&\n-\t\tgit rev-parse refs/${remotes_git_svn}\n+\t\tgit rev-parse refs/remotes/git-svn\n \t)\n '\n \ndiff --git a/t/t9123-git-svn-rebuild-with-rewriteroot.sh b/t/t9123-git-svn-rebuild-with-rewriteroot.sh\nindex fd81847..ead4045 100755\n--- a/t/t9123-git-svn-rebuild-with-rewriteroot.sh\n+++ b/t/t9123-git-svn-rebuild-with-rewriteroot.sh\n@@ -17,7 +17,7 @@ rm -rf import\n test_expect_success 'init, fetch and checkout repository' '\n \tgit svn init --rewrite-root=http://invalid.invalid/ \"$svnrepo\" &&\n \tgit svn fetch &&\n-\tgit checkout -b mybranch ${remotes_git_svn}\n+\tgit checkout -b mybranch remotes/git-svn\n \t'\n \n test_expect_success 'remove rev_map' '\ndiff --git a/t/t9153-git-svn-rewrite-uuid.sh b/t/t9153-git-svn-rewrite-uuid.sh\nindex 88a2cfa..372ef15 100755\n--- a/t/t9153-git-svn-rewrite-uuid.sh\n+++ b/t/t9153-git-svn-rewrite-uuid.sh\n@@ -17,9 +17,9 @@ test_expect_success 'load svn repo' \"\n \n test_expect_success 'verify uuid' \"\n \tgit cat-file commit refs/remotes/git-svn~0 | \\\n-\t   grep '^${git_svn_id}: .*@2 $uuid$' &&\n+\t   grep '^git-svn-id: .*@2 $uuid$' &&\n \tgit cat-file commit refs/remotes/git-svn~1 | \\\n-\t   grep '^${git_svn_id}: .*@1 $uuid$'\n+\t   grep '^git-svn-id: .*@1 $uuid$'\n \t\"\n \n test_done\n-- \n2.8.2.825.gea31738\n"},{"id":"286500","messageId":"20160513204717.GB15391@sigill.intra.peff.net","threadId":"42306","inReplyTo":"20160513204654.GA10684@sigill.intra.peff.net","subject":"[PATCH 2/6] t9100,t3419: enclose all test code in single-quotes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-05-13T20:47:18Z","receivedAt":"2016-05-13T20:47:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"A few tests here use double-quotes around the snippets of\nshell code to run the tests. None of these tests wants to do\nany interpolation at all, and it just leads to an extra\nlayer of quoting around all double-quotes and dollar signs\ninside the snippet.  Let's switch to single quotes, like\nmost other test scripts.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t3419-rebase-patch-id.sh | 12 ++++++------\n t/t9100-git-svn-basic.sh   | 28 ++++++++++++++--------------\n 2 files changed, 20 insertions(+), 20 deletions(-)\n\ndiff --git a/t/t3419-rebase-patch-id.sh b/t/t3419-rebase-patch-id.sh\nindex 217dd79..49f548c 100755\n--- a/t/t3419-rebase-patch-id.sh\n+++ b/t/t3419-rebase-patch-id.sh\n@@ -73,17 +73,17 @@ do_tests () {\n \t\trun git format-patch --stdout --ignore-if-in-upstream master\n \t\"\n \n-\ttest_expect_success $pr 'detect upstream patch' \"\n+\ttest_expect_success $pr 'detect upstream patch' '\n \t\tgit checkout -q master &&\n \t\tscramble file &&\n \t\tgit add file &&\n-\t\tgit commit -q -m 'change big file again' &&\n+\t\tgit commit -q -m \"change big file again\" &&\n \t\tgit checkout -q other^{} &&\n \t\tgit rebase master &&\n-\t\ttest_must_fail test -n \\\"\\$(git rev-list master...HEAD~)\\\"\n-\t\"\n+\t\ttest_must_fail test -n \"$(git rev-list master...HEAD~)\"\n+\t'\n \n-\ttest_expect_success $pr 'do not drop patch' \"\n+\ttest_expect_success $pr 'do not drop patch' '\n \t\tgit branch -f squashed master &&\n \t\tgit checkout -q -f squashed &&\n \t\tgit reset -q --soft HEAD~2 &&\n@@ -91,7 +91,7 @@ do_tests () {\n \t\tgit checkout -q other^{} &&\n \t\ttest_must_fail git rebase squashed &&\n \t\trm -rf .git/rebase-apply\n-\t\"\n+\t'\n }\n \n do_tests 500\ndiff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh\nindex 6ec73ee..28082b1 100755\n--- a/t/t9100-git-svn-basic.sh\n+++ b/t/t9100-git-svn-basic.sh\n@@ -217,11 +217,11 @@ EOF\n \n test_expect_success POSIXPERM,SYMLINKS \"$name\" \"test_cmp a expected\"\n \n-test_expect_success 'exit if remote refs are ambigious' \"\n+test_expect_success 'exit if remote refs are ambigious' '\n         git config --add svn-remote.svn.fetch \\\n \t\tbar:refs/remotes/git-svn &&\n \ttest_must_fail git svn migrate\n-\"\n+'\n \n test_expect_success 'exit if init-ing a would clobber a URL' '\n         svnadmin create \"${PWD}/svnrepo2\" &&\n@@ -259,26 +259,26 @@ test_expect_success 'dcommit $rev does not clobber current branch' '\n \tgit branch -D my-bar\n \t'\n \n-test_expect_success 'able to dcommit to a subdirectory' \"\n+test_expect_success 'able to dcommit to a subdirectory' '\n \tgit svn fetch -i bar &&\n \tgit checkout -b my-bar refs/remotes/bar &&\n \techo abc > d &&\n \tgit update-index --add d &&\n-\tgit commit -m '/bar/d should be in the log' &&\n+\tgit commit -m \"/bar/d should be in the log\" &&\n \tgit svn dcommit -i bar &&\n-\ttest -z \\\"\\$(git diff refs/heads/my-bar refs/remotes/bar)\\\" &&\n+\ttest -z \"$(git diff refs/heads/my-bar refs/remotes/bar)\" &&\n \tmkdir newdir &&\n \techo new > newdir/dir &&\n \tgit update-index --add newdir/dir &&\n-\tgit commit -m 'add a new directory' &&\n+\tgit commit -m \"add a new directory\" &&\n \tgit svn dcommit -i bar &&\n-\ttest -z \\\"\\$(git diff refs/heads/my-bar refs/remotes/bar)\\\" &&\n+\ttest -z \"$(git diff refs/heads/my-bar refs/remotes/bar)\" &&\n \techo foo >> newdir/dir &&\n \tgit update-index newdir/dir &&\n-\tgit commit -m 'modify a file in new directory' &&\n+\tgit commit -m \"modify a file in new directory\" &&\n \tgit svn dcommit -i bar &&\n-\ttest -z \\\"\\$(git diff refs/heads/my-bar refs/remotes/bar)\\\"\n-\t\"\n+\ttest -z \"$(git diff refs/heads/my-bar refs/remotes/bar)\"\n+'\n \n test_expect_success 'dcommit should not fail with a touched file' '\n \ttest_commit \"commit-new-file-foo2\" foo2 &&\n@@ -291,13 +291,13 @@ test_expect_success 'rebase should not fail with a touched file' '\n \tgit svn rebase\n '\n \n-test_expect_success 'able to set-tree to a subdirectory' \"\n+test_expect_success 'able to set-tree to a subdirectory' '\n \techo cba > d &&\n \tgit update-index d &&\n-\tgit commit -m 'update /bar/d' &&\n+\tgit commit -m \"update /bar/d\" &&\n \tgit svn set-tree -i bar HEAD &&\n-\ttest -z \\\"\\$(git diff refs/heads/my-bar refs/remotes/bar)\\\"\n-\t\"\n+\ttest -z \"$(git diff refs/heads/my-bar refs/remotes/bar)\"\n+'\n \n test_expect_success 'git-svn works in a bare repository' '\n \tmkdir bare-repo &&\n-- \n2.8.2.825.gea31738\n"},{"id":"286501","messageId":"20160513204721.GC15391@sigill.intra.peff.net","threadId":"42306","inReplyTo":"20160513204654.GA10684@sigill.intra.peff.net","subject":"[PATCH 3/6] t9107: use \"return 1\" instead of \"exit 1\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-05-13T20:47:21Z","receivedAt":"2016-05-13T20:47:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When a test runs a loop, it cannot rely on the usual\n&&-chaining to propagate a failure inside the loop; it needs\nto break out with a failure signal. However, unless you are\nin a subshell, doing so with \"exit 1\" will exit the entire\ntest script, not just the test snippet we are in (and cause\nthe harness to complain that test_done was never reached).\n\nSo the fundamental point of this patch is s/exit/return/.\nBut while we're there, let's fix a number of style and\nreadability issues:\n\n  - snippets in double-quotes need an extra layer of quoting\n    for their meta-characters; let's avoid that by using\n    single quotes\n\n  - accumulating loop output by appending to a file in each\n    iteration is brittle, as it can be affected by content\n    left in the file by earlier tests. Instead, it's better\n    to redirect stdout for the whole loop, so we know the\n    output only comes from that loop.\n\n  - using \"test -z\" to check that diff output is empty is\n    overly verbose; we can just ask diff to use --exit-code.\n\n  - we can factor out long lists of refs to make it more\n    obvious we're using the same ones in each loop\n\n  - subshells are unnecessary when ending an &&-chain with\n    \"|| return 1\"\n\n  - minor style fixups like space-after-redirection, and\n    \"do\" and \"done\" on their own lines\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t9107-git-svn-migrate.sh | 42 ++++++++++++++++++++++++++----------------\n 1 file changed, 26 insertions(+), 16 deletions(-)\n\ndiff --git a/t/t9107-git-svn-migrate.sh b/t/t9107-git-svn-migrate.sh\nindex 6efc2ab..2908aef 100755\n--- a/t/t9107-git-svn-migrate.sh\n+++ b/t/t9107-git-svn-migrate.sh\n@@ -56,9 +56,11 @@ test_expect_success 'initialize a multi-repository repo' '\n \t                        \"^tags/\\*:refs/remotes/origin/tags/\\*$\" &&\n \tgit config --add svn-remote.svn.fetch \"branches/a:refs/remotes/origin/a\" &&\n \tgit config --add svn-remote.svn.fetch \"branches/b:refs/remotes/origin/b\" &&\n-\tfor i in tags/0.1 tags/0.2 tags/0.3; do\n+\tfor i in tags/0.1 tags/0.2 tags/0.3\n+\tdo\n \t\tgit config --add svn-remote.svn.fetch \\\n-\t\t                 $i:refs/remotes/origin/$i || exit 1; done &&\n+\t\t\t$i:refs/remotes/origin/$i || return 1\n+\tdone &&\n \tgit config --get-all svn-remote.svn.fetch > fetch.out &&\n \tgrep \"^trunk:refs/remotes/origin/trunk$\" fetch.out &&\n \tgrep \"^branches/a:refs/remotes/origin/a$\" fetch.out &&\n@@ -70,30 +72,38 @@ test_expect_success 'initialize a multi-repository repo' '\n \t'\n \n # refs should all be different, but the trees should all be the same:\n-test_expect_success 'multi-fetch works on partial urls + paths' \"\n+test_expect_success 'multi-fetch works on partial urls + paths' '\n+\trefs=\"trunk a b tags/0.1 tags/0.2 tags/0.3\" &&\n \tgit svn multi-fetch &&\n-\tfor i in trunk a b tags/0.1 tags/0.2 tags/0.3; do\n-\t\tgit rev-parse --verify refs/remotes/origin/\\$i^0 >> refs.out || exit 1;\n-\t    done &&\n-\ttest -z \\\"\\$(sort < refs.out | uniq -d)\\\" &&\n-\tfor i in trunk a b tags/0.1 tags/0.2 tags/0.3; do\n-\t  for j in trunk a b tags/0.1 tags/0.2 tags/0.3; do\n-\t\tif test \\$j != \\$i; then continue; fi\n-\t    test -z \\\"\\$(git diff refs/remotes/origin/\\$i \\\n-\t\t\t\t refs/remotes/origin/\\$j)\\\" ||exit 1; done; done\n-\t\"\n+\tfor i in $refs\n+\tdo\n+\t\tgit rev-parse --verify refs/remotes/origin/$i^0 || return 1;\n+\tdone >refs.out &&\n+\ttest -z \"$(sort <refs.out | uniq -d)\" &&\n+\t>expect &&\n+\tfor i in $refs\n+\tdo\n+\t\tfor j in $refs\n+\t\tdo\n+\t\t\tgit diff --exit-code refs/remotes/origin/$i refs/remotes/origin/$j ||\n+\t\t\t\treturn 1\n+\t\tdone\n+\tdone\n+'\n \n test_expect_success 'migrate --minimize on old inited layout' '\n \tgit config --unset-all svn-remote.svn.fetch &&\n \tgit config --unset-all svn-remote.svn.url &&\n \trm -rf \"$GIT_DIR\"/svn &&\n-\tfor i in $(cat fetch.out); do\n+\tfor i in $(cat fetch.out)\n+\tdo\n \t\tpath=$(expr $i : \"\\([^:]*\\):.*$\")\n \t\tref=$(expr $i : \"[^:]*:\\(refs/remotes/.*\\)$\")\n \t\tif test -z \"$ref\"; then continue; fi\n \t\tif test -n \"$path\"; then path=\"/$path\"; fi\n-\t\t( mkdir -p \"$GIT_DIR\"/svn/$ref/info/ &&\n-\t\techo \"$svnrepo\"$path > \"$GIT_DIR\"/svn/$ref/info/url ) || exit 1;\n+\t\tmkdir -p \"$GIT_DIR\"/svn/$ref/info/ &&\n+\t\techo \"$svnrepo\"$path >\"$GIT_DIR\"/svn/$ref/info/url ||\n+\t\treturn 1\n \tdone &&\n \tgit svn migrate --minimize &&\n \ttest -z \"$(git config -l | grep \"^svn-remote\\.git-svn\\.\")\" &&\n-- \n2.8.2.825.gea31738\n"},{"id":"286504","messageId":"20160513204724.GD15391@sigill.intra.peff.net","threadId":"42306","inReplyTo":"20160513204654.GA10684@sigill.intra.peff.net","subject":"[PATCH 4/6] t9107: switch inverted single/double quotes in test","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-05-13T20:47:24Z","receivedAt":"2016-05-13T20:47:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"One of the test snippets in t9107 is enclosed in double\nquotes, but then uses single quotes to surround an\ninterpolated variable inside the snippet, like:\n\n  test_expect_success '...' \"\n\ttest -n '$head'\n  \"\n\nThis happens to work because the variable is interpolated\n_before_ the snippet is run, and the result is eval'd. So as\nlong as the variable does not contain any single quotes, the\ntwo are equivalent. And it doesn't, as we know it is a sha1\nfrom rev-parse above.  But this construct is unnecessarily\nconfusing.\n\nBut we can go a step further in cleaning up. The test is\nreally checking that a particular ref has a value. Rather\nthan checking if rev-parse produced output, we can just move\nrev-parse into the test itself, and rely on the exit code\nfrom --verify. Nobody else cares about the $head variable at\nall.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t9107-git-svn-migrate.sh | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t9107-git-svn-migrate.sh b/t/t9107-git-svn-migrate.sh\nindex 2908aef..bb03722 100755\n--- a/t/t9107-git-svn-migrate.sh\n+++ b/t/t9107-git-svn-migrate.sh\n@@ -24,8 +24,9 @@ test_expect_success 'setup old-looking metadata' '\n \tgit update-ref -d refs/remotes/git-svn refs/remotes/git-svn\n \t'\n \n-head=$(git rev-parse --verify refs/heads/git-svn-HEAD^0)\n-test_expect_success 'git-svn-HEAD is a real HEAD' \"test -n '$head'\"\n+test_expect_success 'git-svn-HEAD is a real HEAD' '\n+\tgit rev-parse --verify refs/heads/git-svn-HEAD^0\n+'\n \n svnrepo_escaped=$(echo $svnrepo | sed 's/ /%20/')\n \n-- \n2.8.2.825.gea31738\n"},{"id":"286503","messageId":"20160513204728.GE15391@sigill.intra.peff.net","threadId":"42306","inReplyTo":"20160513204654.GA10684@sigill.intra.peff.net","subject":"[PATCH 5/6] t9103: modernize test style","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-05-13T20:47:28Z","receivedAt":"2016-05-13T20:47:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The main goal here was to avoid double-quotes for\nsurrounding the test snippet, since it makes the code hard\nto read (and to grep for common problems).\n\nBut while we're here, we can fix a few other things:\n\n  - use test_path_* helpers, which are more robust and give\n    better error messages\n\n  - only \"cd\" inside a subshell, which leaves the\n    environment pristine if further tests are added\n\n  - consistently quote shell arguments. These aren't wrong\n    if we assume find-rev output doesn't have any\n    whitespace, but it doesn't hurt to be careful.\n\n  - replace the old-style 'test x$foo = x' with 'test -z\n    \"$foo\"'. Besides the quoting fix, this is the form we\n    generally use in our test suite.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t9103-git-svn-tracked-directory-removed.sh | 22 ++++++++++++----------\n 1 file changed, 12 insertions(+), 10 deletions(-)\n\ndiff --git a/t/t9103-git-svn-tracked-directory-removed.sh b/t/t9103-git-svn-tracked-directory-removed.sh\nindex 3413164..b282713 100755\n--- a/t/t9103-git-svn-tracked-directory-removed.sh\n+++ b/t/t9103-git-svn-tracked-directory-removed.sh\n@@ -23,17 +23,19 @@ test_expect_success 'make history for tracking' '\n \n test_expect_success 'clone repo with git' '\n \tgit svn clone -s \"$svnrepo\" x &&\n-\ttest -f x/FOLLOWME &&\n-\ttest ! -f x/README\n+\ttest_path_is_file x/FOLLOWME &&\n+\ttest_path_is_missing x/README\n '\n \n-test_expect_success 'make sure r2 still has old file' \"\n-\tcd x &&\n-\t\ttest -n \\\"\\$(git svn find-rev r1)\\\" &&\n-\t\tgit reset --hard \\$(git svn find-rev r1) &&\n-\t\ttest -f README &&\n-\t\ttest ! -f FOLLOWME &&\n-\t\ttest x\\$(git svn find-rev r2) = x\n-\"\n+test_expect_success 'make sure r2 still has old file' '\n+\t(\n+\t\tcd x &&\n+\t\ttest -n \"$(git svn find-rev r1)\" &&\n+\t\tgit reset --hard \"$(git svn find-rev r1)\" &&\n+\t\ttest_path_is_file README &&\n+\t\ttest_path_is_missing FOLLOWME &&\n+\t\ttest -z \"$(git svn find-rev r2)\"\n+\t)\n+'\n \n test_done\n-- \n2.8.2.825.gea31738\n"},{"id":"286505","messageId":"20160513204732.GF15391@sigill.intra.peff.net","threadId":"42306","inReplyTo":"20160513204654.GA10684@sigill.intra.peff.net","subject":"[PATCH 6/6] always quote shell arguments to test -z/-n","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-05-13T20:47:33Z","receivedAt":"2016-05-13T20:47:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"In shell code like:\n\n  test -z $foo\n  test -n $foo\n\nthat does not quote its arguments, it's easy to think that\nit is actually looking at the contents of $foo in each case.\nBut if $foo is empty, then \"test\" does not see any argument\nat all! The results are quite subtle.\n\nPOSIX specifies that test's behavior depends on the number\nof arguments it sees, and if $foo is empty, it sees only\none. The behavior in this case is:\n\n  1 argument: Exit true (0) if $1 is not null; otherwise,\n              exit false.\n\nSo in the \"-z $foo\" case, if $foo is empty, then we check\nthat \"-z\" is non-null, and it returns success. Which happens\nto match what we expected.  But for \"-n $foo\", if $foo is\nempty, we'll see that \"-n\" is non-null and still return\nsuccess. That's the opposite of what we intended!\n\nFurthermore, if $foo contains whitespace, we'll end up with\nmore than 2 arguments. The results in this case are\ngenerally unspecified (unless the first part of $foo happens\nto be a valid binary operator, in which case the results are\nspecified but certainly not what we intended).\n\nAnd on top of this, even though \"test -z $foo\" _should_ work\nfor the empty case, some older shells (reportedly ksh88)\ncomplain about the missing argument.\n\nSo let's make sure we consistently quote our variable\narguments to \"test\". After this patch, the results of:\n\n  git grep 'test -[zn] [^\"]'\n\nare empty.\n\nReported-by: Armin Kunaschik <megabreit@googlemail.com>\nHelped-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Jeff King <peff@peff.net>\n---\n git-rebase--interactive.sh | 4 ++--\n git-stash.sh               | 4 ++--\n t/t4151-am-abort.sh        | 2 +-\n 3 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 9ea3075..470413b 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -866,12 +866,12 @@ add_exec_commands () {\n # $3: the input filename\n check_commit_sha () {\n \tbadsha=0\n-\tif test -z $1\n+\tif test -z \"$1\"\n \tthen\n \t\tbadsha=1\n \telse\n \t\tsha1_verif=\"$(git rev-parse --verify --quiet $1^{commit})\"\n-\t\tif test -z $sha1_verif\n+\t\tif test -z \"$sha1_verif\"\n \t\tthen\n \t\t\tbadsha=1\n \t\tfi\ndiff --git a/git-stash.sh b/git-stash.sh\nindex c7c65e2..c7509e8 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -185,7 +185,7 @@ store_stash () {\n \n \tgit update-ref --create-reflog -m \"$stash_msg\" $ref_stash $w_commit\n \tret=$?\n-\ttest $ret != 0 && test -z $quiet &&\n+\ttest $ret != 0 && test -z \"$quiet\" &&\n \tdie \"$(eval_gettext \"Cannot update \\$ref_stash with \\$w_commit\")\"\n \treturn $ret\n }\n@@ -277,7 +277,7 @@ save_stash () {\n \t\t\tgit clean --force --quiet -d $CLEAN_X_OPTION\n \t\tfi\n \n-\t\tif test \"$keep_index\" = \"t\" && test -n $i_tree\n+\t\tif test \"$keep_index\" = \"t\" && test -n \"$i_tree\"\n \t\tthen\n \t\t\tgit read-tree --reset -u $i_tree\n \t\tfi\ndiff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh\nindex ea5ace9..9473c27 100755\n--- a/t/t4151-am-abort.sh\n+++ b/t/t4151-am-abort.sh\n@@ -82,7 +82,7 @@ test_expect_success 'am -3 --abort removes otherfile-4' '\n \ttest 4 = \"$(cat otherfile-4)\" &&\n \tgit am --abort &&\n \ttest_cmp_rev initial HEAD &&\n-\ttest -z $(git ls-files -u) &&\n+\ttest -z \"$(git ls-files -u)\" &&\n \ttest_path_is_missing otherfile-4\n '\n \n-- \n2.8.2.825.gea31738\n"},{"id":"286509","messageId":"xmqqoa898v9s.fsf@gitster.mtv.corp.google.com","threadId":"42306","inReplyTo":"20160513204654.GA10684@sigill.intra.peff.net","subject":"Re: [PATCH 0/6] test -z/-n quoting fix + misc cleanups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-13T22:09:51Z","receivedAt":"2016-05-13T22:09:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Anyway. Here's a series that fixes the -n/-z cases, along with a bunch\n> of cleanups that remove the false positives (most of which I sent out\n> just a few minutes ago as \"minor fixes to some svn tests\").\n>\n>   [1/6]: t/lib-git-svn: drop $remote_git_svn and $git_svn_id\n>   [2/6]: t9100,t3419: enclose all test code in single-quotes\n>   [3/6]: t9107: use \"return 1\" instead of \"exit 1\"\n>   [4/6]: t9107: switch inverted single/double quotes in test\n>   [5/6]: t9103: modernize test style\n>   [6/6]: always quote shell arguments to test -z/-n\n\nAll look sensible; thanks.\n"},{"id":"286512","messageId":"20160513225916.GA17368@dcvr.yhbt.net","threadId":"42306","inReplyTo":"20160513204721.GC15391@sigill.intra.peff.net","subject":"Re: [PATCH 3/6] t9107: use \"return 1\" instead of \"exit 1\"","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-05-13T22:59:16Z","receivedAt":"2016-05-13T22:59:16Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Jeff King <peff@peff.net> wrote:\n> +\t\t\tgit diff --exit-code refs/remotes/origin/$i refs/remotes/origin/$j ||\n> +\t\t\t\treturn 1\n\n80 columns; but I guess Junio picked it up, already.\nOtherwise the rest of the series looks good.  Thanks.\n"},{"id":"286521","messageId":"CAPig+cQKkkP=Go=w65mGcgqZN6ckz21kG2=KCJS8L+0D3mgC+g@mail.gmail.com","threadId":"42306","inReplyTo":"20160513204721.GC15391@sigill.intra.peff.net","subject":"Re: [PATCH 3/6] t9107: use \"return 1\" instead of \"exit 1\"","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-05-13T23:45:42Z","receivedAt":"2016-05-13T23:45:42Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, May 13, 2016 at 4:47 PM, Jeff King <peff@peff.net> wrote:\n> When a test runs a loop, it cannot rely on the usual\n> &&-chaining to propagate a failure inside the loop; it needs\n> to break out with a failure signal. However, unless you are\n> in a subshell, doing so with \"exit 1\" will exit the entire\n> test script, not just the test snippet we are in (and cause\n> the harness to complain that test_done was never reached).\n> [...snip...]\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> diff --git a/t/t9107-git-svn-migrate.sh b/t/t9107-git-svn-migrate.sh\n> @@ -70,30 +72,38 @@ test_expect_success 'initialize a multi-repository repo' '\n>  # refs should all be different, but the trees should all be the same:\n> -test_expect_success 'multi-fetch works on partial urls + paths' \"\n> +test_expect_success 'multi-fetch works on partial urls + paths' '\n> +       refs=\"trunk a b tags/0.1 tags/0.2 tags/0.3\" &&\n>         git svn multi-fetch &&\n> -       for i in trunk a b tags/0.1 tags/0.2 tags/0.3; do\n> -               git rev-parse --verify refs/remotes/origin/\\$i^0 >> refs.out || exit 1;\n> -           done &&\n> -       test -z \\\"\\$(sort < refs.out | uniq -d)\\\" &&\n> -       for i in trunk a b tags/0.1 tags/0.2 tags/0.3; do\n> -         for j in trunk a b tags/0.1 tags/0.2 tags/0.3; do\n> -               if test \\$j != \\$i; then continue; fi\n> -           test -z \\\"\\$(git diff refs/remotes/origin/\\$i \\\n> -                                refs/remotes/origin/\\$j)\\\" ||exit 1; done; done\n> -       \"\n> +       for i in $refs\n> +       do\n> +               git rev-parse --verify refs/remotes/origin/$i^0 || return 1;\n> +       done >refs.out &&\n> +       test -z \"$(sort <refs.out | uniq -d)\" &&\n> +       >expect &&\n\nWhat's this 'expect' file for? Is it leftover gunk from before you\nsettled on 'diff --exit-code'?\n\n> +       for i in $refs\n> +       do\n> +               for j in $refs\n> +               do\n> +                       git diff --exit-code refs/remotes/origin/$i refs/remotes/origin/$j ||\n> +                               return 1\n> +               done\n> +       done\n> +'\n"},{"id":"286522","messageId":"20160513234704.GA20238@sigill.intra.peff.net","threadId":"42306","inReplyTo":"CAPig+cQKkkP=Go=w65mGcgqZN6ckz21kG2=KCJS8L+0D3mgC+g@mail.gmail.com","subject":"Re: [PATCH 3/6] t9107: use \"return 1\" instead of \"exit 1\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-05-13T23:47:04Z","receivedAt":"2016-05-13T23:47:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 13, 2016 at 07:45:42PM -0400, Eric Sunshine wrote:\n\n> > +       >expect &&\n> \n> What's this 'expect' file for? Is it leftover gunk from before you\n> settled on 'diff --exit-code'?\n\nOops, yes, that's exactly it.\n\n-Peff\n"},{"id":"286540","messageId":"xmqqbn488rss.fsf@gitster.mtv.corp.google.com","threadId":"42306","inReplyTo":"20160513234704.GA20238@sigill.intra.peff.net","subject":"Re: [PATCH 3/6] t9107: use \"return 1\" instead of \"exit 1\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-14T17:37:07Z","receivedAt":"2016-05-14T17:37:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Fri, May 13, 2016 at 07:45:42PM -0400, Eric Sunshine wrote:\n>\n>> > +       >expect &&\n>> \n>> What's this 'expect' file for? Is it leftover gunk from before you\n>> settled on 'diff --exit-code'?\n>\n> Oops, yes, that's exactly it.\n>\n> -Peff\n\nThanks for sharp eyes.  Let's squash this in, perhaps?\n\n t/t9107-git-svn-migrate.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t9107-git-svn-migrate.sh b/t/t9107-git-svn-migrate.sh\nindex 2908aef..b6a99b8 100755\n--- a/t/t9107-git-svn-migrate.sh\n+++ b/t/t9107-git-svn-migrate.sh\n@@ -80,12 +80,12 @@ test_expect_success 'multi-fetch works on partial urls + paths' '\n \t\tgit rev-parse --verify refs/remotes/origin/$i^0 || return 1;\n \tdone >refs.out &&\n \ttest -z \"$(sort <refs.out | uniq -d)\" &&\n-\t>expect &&\n \tfor i in $refs\n \tdo\n \t\tfor j in $refs\n \t\tdo\n-\t\t\tgit diff --exit-code refs/remotes/origin/$i refs/remotes/origin/$j ||\n+\t\t\tgit diff --exit-code refs/remotes/origin/$i \\\n+\t\t\t\t\t     refs/remotes/origin/$j ||\n \t\t\t\treturn 1\n \t\tdone\n \tdone\n"},{"id":"286549","messageId":"20160514195150.GA24206@sigill.intra.peff.net","threadId":"42306","inReplyTo":"xmqqbn488rss.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/6] t9107: use \"return 1\" instead of \"exit 1\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-05-14T19:51:50Z","receivedAt":"2016-05-14T19:51:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, May 14, 2016 at 10:37:07AM -0700, Junio C Hamano wrote:\n\n> Thanks for sharp eyes.  Let's squash this in, perhaps?\n> \n>  t/t9107-git-svn-migrate.sh | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/t/t9107-git-svn-migrate.sh b/t/t9107-git-svn-migrate.sh\n> index 2908aef..b6a99b8 100755\n> --- a/t/t9107-git-svn-migrate.sh\n> +++ b/t/t9107-git-svn-migrate.sh\n> @@ -80,12 +80,12 @@ test_expect_success 'multi-fetch works on partial urls + paths' '\n>  \t\tgit rev-parse --verify refs/remotes/origin/$i^0 || return 1;\n>  \tdone >refs.out &&\n>  \ttest -z \"$(sort <refs.out | uniq -d)\" &&\n> -\t>expect &&\n>  \tfor i in $refs\n>  \tdo\n>  \t\tfor j in $refs\n>  \t\tdo\n> -\t\t\tgit diff --exit-code refs/remotes/origin/$i refs/remotes/origin/$j ||\n> +\t\t\tgit diff --exit-code refs/remotes/origin/$i \\\n> +\t\t\t\t\t     refs/remotes/origin/$j ||\n>  \t\t\t\treturn 1\n>  \t\tdone\n>  \tdone\n\nYeah, that looks perfect. Thanks.\n\n-Peff\n"}]}