{"thread":{"id":"42312","subject":"[PATCH 0/3] minor fixes to some svn tests","startedAt":"2016-05-13T19:47:16Z","lastAt":"2016-05-13T20:49:00Z","messageCount":5,"participants":["Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"286484","messageId":"20160513194716.GA9806@sigill.intra.peff.net","threadId":"42312","inReplyTo":null,"subject":"[PATCH 0/3] minor fixes to some svn tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-05-13T19:47:16Z","receivedAt":"2016-05-13T19:47:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 13, 2016 at 02:23:25PM -0400, Jeff King wrote:\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\nI looked at those false positives, and they're really gratuitous uses\nof double-quotes that cause the extra layer of quoting.\n\nSo I set off to fix that, but along the way found an actual bug (in the\nfinal patch here), along with a few minor readability fixups.\n\n  [1/3]: t/lib-git-svn: drop $remote_git_svn and $git_svn_id\n  [2/3]: t9100: enclose all test code in single-quotes\n  [3/3]: t9107: use \"return 1\" instead of \"exit 1\"\n\n-Peff\n"},{"id":"286485","messageId":"20160513194804.GA9890@sigill.intra.peff.net","threadId":"42312","inReplyTo":"20160513194716.GA9806@sigill.intra.peff.net","subject":"[PATCH 1/3] t/lib-git-svn: drop $remote_git_svn and $git_svn_id","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-05-13T19:48:04Z","receivedAt":"2016-05-13T19:48:04Z","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":"286486","messageId":"20160513194836.GB9890@sigill.intra.peff.net","threadId":"42312","inReplyTo":"20160513194716.GA9806@sigill.intra.peff.net","subject":"[PATCH 2/3] t9100: enclose all test code in single-quotes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-05-13T19:48:36Z","receivedAt":"2016-05-13T19:48:36Z","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/t9100-git-svn-basic.sh | 28 ++++++++++++++--------------\n 1 file changed, 14 insertions(+), 14 deletions(-)\n\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":"286487","messageId":"20160513195021.GC9890@sigill.intra.peff.net","threadId":"42312","inReplyTo":"20160513194716.GA9806@sigill.intra.peff.net","subject":"[PATCH 3/3] t9107: use \"return 1\" instead of \"exit 1\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-05-13T19:50:21Z","receivedAt":"2016-05-13T19:50: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---\nThis covers all of the \"|| exit 1\" cases I could find. If you grep,\nthere are a number of them, but they are all inside subshells (where\nit's the correct thing to use).\n\nThis is a bugfix, but it's not _that_ big a deal, in the sense that\nusing \"exit 1\" would definitely signal to the caller that the test\nfailed. :) But it disrupts things like the TAP output.\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":"286506","messageId":"20160513204900.GA15433@sigill.intra.peff.net","threadId":"42312","inReplyTo":"20160513194716.GA9806@sigill.intra.peff.net","subject":"Re: [PATCH 0/3] minor fixes to some svn tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-05-13T20:49:00Z","receivedAt":"2016-05-13T20:49:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 13, 2016 at 03:47:16PM -0400, Jeff King wrote:\n\n> On Fri, May 13, 2016 at 02:23:25PM -0400, Jeff King wrote:\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> I looked at those false positives, and they're really gratuitous uses\n> of double-quotes that cause the extra layer of quoting.\n> \n> So I set off to fix that, but along the way found an actual bug (in the\n> final patch here), along with a few minor readability fixups.\n> \n>   [1/3]: t/lib-git-svn: drop $remote_git_svn and $git_svn_id\n>   [2/3]: t9100: enclose all test code in single-quotes\n>   [3/3]: t9107: use \"return 1\" instead of \"exit 1\"\n\nNote that I ended up rolling these into the series posted at:\n\n  http://thread.gmane.org/gmane.comp.version-control.git/294539/focus=294577\n\nOr <20160513204654.GA10684@sigill.intra.peff.net>, since I know Eric has\nhis own list archive. :)\n\n-Peff\n"}]}