Elia Pinto <gitter.spiros@xxxxxxxxx> writes: > The following changes since commit 6308767f0bb58116cb405e1f4f77f5dfc1589920: > > > Merge branch 'fc/prompt-zsh-read-from-file' (2014-05-13 11:53:14 -0700) > > > are available in the git repository at: > > > https://github.com/devzero2000/git-core.git ep/shell-command-substitution-v4 There's a mis-replacement of multiple `..` `..` on the same line in t9300-fast-import.sh. I've sent you a pull request with a fixup. I'm not sure about this one: commit e69c77e580d56d587381066f56027c8a596c237a Author: Elia Pinto <gitter.spiros@xxxxxxxxx> Date: Wed May 14 03:28:11 2014 -0700 t9137-git-svn-dcommit-clobber-series.sh: use the $( ... ) construct for command substitution [...] @@ -38,20 +38,20 @@ test_expect_success 'some unrelated changes to git' " " test_expect_success 'change file but in unrelated area' " - test x\"\`sed -n -e 4p < file\`\" = x4 && - test x\"\`sed -n -e 7p < file\`\" = x7 && + test x"$(sed -n -e 4p < file)" = x4 && + test x"$(sed -n -e 7p < file)" = x7 && ^ here We're inside " from the test_expect_success line. We used to have a literal " (because of the backslash), we now have a closing " because you removed the \. So, the sed command used to be protected by double-quotes, and it is now outside them. Compare: $ sh -c "echo \"\`date\`\"" Wed May 14 18:47:54 MEST 2014 $ sh -c "echo "$(date)"" Wed In your case, it doesn't break because the expected output of sed contains no space, but that seems dangerous to me. I do not understand the use of the \ in front of the ` in the original code. The correct code should be test x\"$(sed -n -e 4p < file)\" = x4 && I guess. perl -i.bak -p -e 's/^4\$/4444/' file && perl -i.bak -p -e 's/^7\$/7777/' file && - test x\"\`sed -n -e 4p < file\`\" = x4444 && - test x\"\`sed -n -e 7p < file\`\" = x7777 && + test x"$(sed -n -e 4p < file)" = x4444 && + test x"$(sed -n -e 7p < file)" = x7777 && Likewise. - test x\"\`sed -n -e 4p < file\`\" = x4444 && - test x\"\`sed -n -e 7p < file\`\" = x7777 && - test x\"\`sed -n -e 58p < file\`\" = x5588 && - test x\"\`sed -n -e 61p < file\`\" = x6611 + test x"$(sed -n -e 4p < file)" = x4444 && + test x"$(sed -n -e 7p < file)" = x7777 && + test x"$(sed -n -e 58p < file)" = x5588 && + test x"$(sed -n -e 61p < file)" = x6611 Likewise. More or less the same issue with commit 020568b9c36c023810a3482b7b73bcadd6406a85 Author: Elia Pinto <gitter.spiros@xxxxxxxxx> Date: Mon Apr 28 05:49:50 2014 -0700 t9114-git-svn-dcommit-merge.sh: use the $( ... ) construct for command substitution [...] diff --git a/t/t9114-git-svn-dcommit-merge.sh b/t/t9114-git-svn-dcommit-merge.sh index fb41876..cf2e25f 100755 --- a/t/t9114-git-svn-dcommit-merge.sh +++ b/t/t9114-git-svn-dcommit-merge.sh @@ -68,8 +68,8 @@ test_expect_success 'setup git mirror and merge' ' test_debug 'gitk --all & sleep 1' test_expect_success 'verify pre-merge ancestry' " - test x\`git rev-parse --verify refs/heads/svn^2\` = \ - x\`git rev-parse --verify refs/heads/merge\` && + test x\$(git rev-parse --verify refs/heads/svn^2\) = \ + x\$(git rev-parse --verify refs/heads/merge\) && git cat-file commit refs/heads/svn^ | grep '^friend$' " I'm not sure what's the intent of the \ in front of ` in the original code, but changing it to $(...) changes the meaning: $ sh -c "echo \`date\`" Wed May 14 18:58:19 MEST 2014 $ sh -c "echo \$(date\)" sh: 1: Syntax error: end of file unexpected (expecting ")") I didn't investigate closely, but I'm getting test failures without your patch, and the script stops in the middle with it so it does break something. @@ -80,10 +80,10 @@ test_expect_success 'git svn dcommit merges' " test_debug 'gitk --all & sleep 1' test_expect_success 'verify post-merge ancestry' " - test x\`git rev-parse --verify refs/heads/svn\` = \ - x\`git rev-parse --verify refs/remotes/origin/trunk \` && - test x\`git rev-parse --verify refs/heads/svn^2\` = \ - x\`git rev-parse --verify refs/heads/merge\` && + test x\$(git rev-parse --verify refs/heads/svn\) = \ + x\$(git rev-parse --verify refs/remotes/origin/trunk \) && + test x\$(git rev-parse --verify refs/heads/svn^2\) = \ + x\$(git rev-parse --verify refs/heads/merge\) && Likewise. commit 7e29ac501ce24aa5af3a50f839cd3ad176481a96 Author: Elia Pinto <gitter.spiros@xxxxxxxxx> Date: Wed Mar 26 04:48:40 2014 -0700 t9100-git-svn-basic.sh: use the $( ... ) construct for command substitution -test_expect_success 'able to dcommit to a subdirectory' " +test_expect_success 'able to dcommit to a subdirectory' ' There is an actual change other than sed + review and trivial fix here. That makes the review harder. Such change should IMHO not be part of the same series. - git commit -m '/bar/d should be in the log' && + git commit -m "bar/d should be in the log" && You lost a / here. git svn dcommit -i bar && - test -z \"\`git diff refs/heads/my-bar refs/remotes/bar\`\" && + test -z $(git diff refs/heads/my-bar refs/remotes/bar) && Did you not loos the \"...\" whitespace protection here? - test -z \"\`git diff refs/heads/my-bar refs/remotes/bar\`\" && + test -z "$(git diff refs/heads/my-bar refs/remotes/bar)" && That seems to be the correct way of doing what you tried right above. commit b438455b7b97d90a1b8da4ec4e9de0063c20f63c Author: Elia Pinto <gitter.spiros@xxxxxxxxx> Date: Wed Mar 26 04:48:40 2014 -0700 t9107-git-svn-migrate.sh: use the $( ... ) construct for command substitution [...] @@ -75,7 +75,7 @@ test_expect_success 'multi-fetch works on partial urls + paths' " for i in trunk a b tags/0.1 tags/0.2 tags/0.3; do git rev-parse --verify refs/remotes/origin/\$i^0 >> refs.out || exit 1; done && - test -z \"\`sort < refs.out | uniq -d\`\" && + test -z \"\$(sort < refs.out | uniq -d\)\" && Same problem as above, this \$( is broken. My advice: apply my small fix for multiple `...` `...`, and eject the other patches from the series for now, they are distracting reviewers. That should lead to a trivially-correct series with ~80 patches. Once this one is accepted, the 4 remaining patches can be fixed and reviewed more carefully (Cc Eric Wong <normalperson@xxxxxxxx> since the patches are about git-svn). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html