{"thread":{"id":"26240","subject":"[PATCH] Avoid unportable nested double- and backquotes in shell scripts.","startedAt":"2011-01-08T09:01:05Z","lastAt":"2011-01-10T18:09:37Z","messageCount":6,"participants":["Ralf Wildenhues","Jonathan Nieder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"159206","messageId":"20110108090105.GB14536@gmx.de","threadId":"26240","inReplyTo":null,"subject":"[PATCH] Avoid unportable nested double- and backquotes in shell scripts.","fromName":"Ralf Wildenhues","fromEmail":"ralf.wildenhues@gmx.de","sentAt":"2011-01-08T09:01:05Z","receivedAt":"2011-01-08T09:01:05Z","isPatch":true,"sender":{"key":"ralf.wildenhues@gmx.de","avatar":null},"body":"Some shells parse them wrongly, esp. pdksh.  On the other hand,\nthe right-hand side of assignments is not word-split, so they\ncan be used as workarounds.\n\nSigned-off-by: Ralf Wildenhues <Ralf.Wildenhues@gmx.de>\n---\n contrib/examples/git-fetch.sh |    2 +-\n t/t9107-git-svn-migrate.sh    |    2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/examples/git-fetch.sh b/contrib/examples/git-fetch.sh\nindex a314273..06caf6b 100755\n--- a/contrib/examples/git-fetch.sh\n+++ b/contrib/examples/git-fetch.sh\n@@ -67,7 +67,7 @@ do\n \t\tkeep='-k -k'\n \t\t;;\n \t--depth=*)\n-\t\tshallow_depth=\"--depth=`expr \"z$1\" : 'z-[^=]*=\\(.*\\)'`\"\n+\t\tshallow_depth=--depth=`expr \"z$1\" : 'z-[^=]*=\\(.*\\)'`\n \t\t;;\n \t--depth)\n \t\tshift\ndiff --git a/t/t9107-git-svn-migrate.sh b/t/t9107-git-svn-migrate.sh\nindex 289fc31..3d2ae3e 100755\n--- a/t/t9107-git-svn-migrate.sh\n+++ b/t/t9107-git-svn-migrate.sh\n@@ -94,7 +94,7 @@ test_expect_success 'migrate --minimize on old inited layout' '\n \t\techo \"$svnrepo\"$path > \"$GIT_DIR\"/svn/$ref/info/url ) || exit 1;\n \tdone &&\n \tgit svn migrate --minimize &&\n-\ttest -z \"`git config -l | grep \"^svn-remote\\.git-svn\\.\"`\" &&\n+\t! git config -l | grep \"^svn-remote\\.git-svn\\.\" &&\n \tgit config --get-all svn-remote.svn.fetch > fetch.out &&\n \tgrep \"^trunk:refs/remotes/trunk$\" fetch.out &&\n \tgrep \"^branches/a:refs/remotes/a$\" fetch.out &&\n-- \n1.7.4.rc1\n"},{"id":"159221","messageId":"20110108161441.GA28898@burratino","threadId":"26240","inReplyTo":"20110108090105.GB14536@gmx.de","subject":"Re: [PATCH] Avoid unportable nested double- and backquotes in shell scripts.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-01-08T16:14:41Z","receivedAt":"2011-01-08T16:14:41Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ralf Wildenhues wrote:\n\n> [Subject: Avoid unportable nested double- and backquotes in shell scripts]\n>\n> Some shells parse them wrongly, esp. pdksh.\n\nHow does it treat $( ) command substitutions?  (We use those more\nheavily and they are easier on the eyes anyway.)\n\n> --- a/t/t9107-git-svn-migrate.sh\n> +++ b/t/t9107-git-svn-migrate.sh\n> @@ -94,7 +94,7 @@ test_expect_success 'migrate --minimize on old inited layout' '\n>  \t\techo \"$svnrepo\"$path > \"$GIT_DIR\"/svn/$ref/info/url ) || exit 1;\n>  \tdone &&\n>  \tgit svn migrate --minimize &&\n> -\ttest -z \"`git config -l | grep \"^svn-remote\\.git-svn\\.\"`\" &&\n> +\t! git config -l | grep \"^svn-remote\\.git-svn\\.\" &&\n\nI thought I remembered portability problems with the\n\n\t! a | b\n\nconstruct but it seems I am wrong; t7810-grep.sh uses that\nconstruct without trouble, at least.\n"},{"id":"159223","messageId":"20110108162353.GB4786@gmx.de","threadId":"26240","inReplyTo":"20110108161441.GA28898@burratino","subject":"Re: [PATCH] Avoid unportable nested double- and backquotes in shell scripts.","fromName":"Ralf Wildenhues","fromEmail":"ralf.wildenhues@gmx.de","sentAt":"2011-01-08T16:23:53Z","receivedAt":"2011-01-08T16:23:53Z","isPatch":true,"sender":{"key":"ralf.wildenhues@gmx.de","avatar":null},"body":"* Jonathan Nieder wrote on Sat, Jan 08, 2011 at 05:14:41PM CET:\n> Ralf Wildenhues wrote:\n> \n> > [Subject: Avoid unportable nested double- and backquotes in shell scripts]\n> >\n> > Some shells parse them wrongly, esp. pdksh.\n> \n> How does it treat $( ) command substitutions?  (We use those more\n> heavily and they are easier on the eyes anyway.)\n\nBetter (except for the usual problems when 'case ...)' comes into play).\nBut git makes heavy use of \"no quoting needed on RHS of assignment\"\nanyway, so it seems like this would be a good move nonetheless.  And the\ntestsuite uses backticks a lot, it seems a move away from that should be\ndone more uniformly?\n\nAnyway, I'll be happy to respin in whatever form is acceptable.\n\n> > --- a/t/t9107-git-svn-migrate.sh\n> > +++ b/t/t9107-git-svn-migrate.sh\n> > @@ -94,7 +94,7 @@ test_expect_success 'migrate --minimize on old inited layout' '\n> >  \t\techo \"$svnrepo\"$path > \"$GIT_DIR\"/svn/$ref/info/url ) || exit 1;\n> >  \tdone &&\n> >  \tgit svn migrate --minimize &&\n> > -\ttest -z \"`git config -l | grep \"^svn-remote\\.git-svn\\.\"`\" &&\n> > +\t! git config -l | grep \"^svn-remote\\.git-svn\\.\" &&\n> \n> I thought I remembered portability problems with the\n> \n> \t! a | b\n> \n> construct but it seems I am wrong; t7810-grep.sh uses that\n> construct without trouble, at least.\n\nSome non-Posix-conforming shells have problems with that too, e.g.,\nSolaris /bin/sh, but I figured git wouldn't cater to them as I also saw\nother such uses in the tree.\n\nCheers,\nRalf\n"},{"id":"159226","messageId":"20110108164825.GC28898@burratino","threadId":"26240","inReplyTo":"20110108162353.GB4786@gmx.de","subject":"Re: [PATCH] Avoid unportable nested double- and backquotes in shell scripts.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-01-08T16:48:25Z","receivedAt":"2011-01-08T16:48:25Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ralf Wildenhues wrote:\n\n> But git makes heavy use of \"no quoting needed on RHS of assignment\"\n> anyway, so it seems like this would be a good move nonetheless.\n\nNo disagreement there.\n\n> And the\n> testsuite uses backticks a lot,\n\n>From a quick grep, it seems you are right:\n\n $ git grep -c -F -e '`' -- 't/*.sh' | cut -d: -f2 | sum\n 65126     1\n $ git grep -c -F -e '$(' -- 't/*.sh' | cut -d: -f2 | sum\n 64807     1\n $ git grep -c -F -e '`' -- '*.sh' | cut -d: -f2 | sum\n 13350     1\n $ git grep -c -F -e '$(' -- '*.sh' | cut -d: -f2 | sum\n 07810     1\n\nDocumentation/CodingGuidelines \n\n - We prefer $( ... ) for command substitution; unlike ``, it\n   properly nests.  It should have been the way Bourne spelled\n   it from day one, but unfortunately isn't.\n\n> it seems a move away from that should be\n> done more uniformly?\n\nI don't see why. :)  In fact, I personally would not be happy at all\nto see such a high-churn patch as that, while using the $( ... )\nform in new code and as part of clarifications to other parts of the\nsame lines would seem to me to be a welcome thing.\n\nHaving said all that, I have no strong investment in this.  Feel\nfree to do what works best for you.\n\nThanks,\nJonathan\n"},{"id":"159229","messageId":"20110108182201.GB29788@burratino","threadId":"26240","inReplyTo":"20110108164825.GC28898@burratino","subject":"Re: [PATCH] Avoid unportable nested double- and backquotes in shell scripts.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-01-08T18:22:01Z","receivedAt":"2011-01-08T18:22:01Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n\n> From a quick grep, it seems you are right:\n> \n>  $ git grep -c -F -e '`' -- 't/*.sh' | cut -d: -f2 | sum\n>  65126     1\n>  $ git grep -c -F -e '$(' -- 't/*.sh' | cut -d: -f2 | sum\n>  64807     1\n>  $ git grep -c -F -e '`' -- '*.sh' | cut -d: -f2 | sum\n>  13350     1\n>  $ git grep -c -F -e '$(' -- '*.sh' | cut -d: -f2 | sum\n>  07810     1\n\nsum does something totally different than I expected.  With [1]\ncomes the more reasonable\n\n $ git grep -c -F -e '`' -- 't/*.sh' | cut -d: -f2 | addup\n 485\n $ git grep -c -F -e '$(' -- 't/*.sh' | cut -d: -f2 | addup\n 2620\n $ git grep -c -F -e '`' -- '*.sh' | cut -d: -f2 | addup\n 594\n $ git grep -c -F -e '$(' -- '*.sh' | cut -d: -f2 | addup\n 3133\n\nSo the code bloat and use of backticks are less dire than I feared.\n\n[1] \n\taddup () {\n\t\tsum=0\n\t\twhile read term\n\t\tdo\n\t\t\t: $((sum = $sum + $term))\n\t\tdone\n\t\techo $sum\n\t}\n"},{"id":"159281","messageId":"7vlj2s636m.fsf@alter.siamese.dyndns.org","threadId":"26240","inReplyTo":"20110108090105.GB14536@gmx.de","subject":"Re: [PATCH] Avoid unportable nested double- and backquotes in shell scripts.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-01-10T18:09:37Z","receivedAt":"2011-01-10T18:09:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ralf Wildenhues <Ralf.Wildenhues@gmx.de> writes:\n\n> diff --git a/contrib/examples/git-fetch.sh b/contrib/examples/git-fetch.sh\n> index a314273..06caf6b 100755\n> --- a/contrib/examples/git-fetch.sh\n> +++ b/contrib/examples/git-fetch.sh\n> @@ -67,7 +67,7 @@ do\n>  \t\tkeep='-k -k'\n>  \t\t;;\n>  \t--depth=*)\n> -\t\tshallow_depth=\"--depth=`expr \"z$1\" : 'z-[^=]*=\\(.*\\)'`\"\n> +\t\tshallow_depth=--depth=`expr \"z$1\" : 'z-[^=]*=\\(.*\\)'`\n>  \t\t;;\n>  \t--depth)\n>  \t\tshift\n\nI do not very much like the idea of updating contrib/examples, one of\nwhose purposes is to document the historical implementation, to ship a\nversion that has never been battle tested in the field.\n\nThere also seem to be a few more uses of `` (the majority used $() even\nback then) that are still left behind.  To make it more useful to serve as\nan example (which is the other purpose of contrib/examples), it would make\nmore sense to update them all at once.\n\n> diff --git a/t/t9107-git-svn-migrate.sh b/t/t9107-git-svn-migrate.sh\n> index 289fc31..3d2ae3e 100755\n> --- a/t/t9107-git-svn-migrate.sh\n> +++ b/t/t9107-git-svn-migrate.sh\n> @@ -94,7 +94,7 @@ test_expect_success 'migrate --minimize on old inited layout' '\n>  \t\techo \"$svnrepo\"$path > \"$GIT_DIR\"/svn/$ref/info/url ) || exit 1;\n>  \tdone &&\n>  \tgit svn migrate --minimize &&\n> -\ttest -z \"`git config -l | grep \"^svn-remote\\.git-svn\\.\"`\" &&\n> +\t! git config -l | grep \"^svn-remote\\.git-svn\\.\" &&\n>  \tgit config --get-all svn-remote.svn.fetch > fetch.out &&\n>  \tgrep \"^trunk:refs/remotes/trunk$\" fetch.out &&\n>  \tgrep \"^branches/a:refs/remotes/a$\" fetch.out &&\n"}]}