* [PATCH] Avoid unportable nested double- and backquotes in shell scripts.
@ 2011-01-08 9:01 Ralf Wildenhues
2011-01-08 16:14 ` Jonathan Nieder
2011-01-10 18:09 ` Junio C Hamano
0 siblings, 2 replies; 6+ messages in thread
From: Ralf Wildenhues @ 2011-01-08 9:01 UTC (permalink / raw)
To: git
Some shells parse them wrongly, esp. pdksh. On the other hand,
the right-hand side of assignments is not word-split, so they
can be used as workarounds.
Signed-off-by: Ralf Wildenhues <Ralf.Wildenhues@gmx.de>
---
contrib/examples/git-fetch.sh | 2 +-
t/t9107-git-svn-migrate.sh | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/contrib/examples/git-fetch.sh b/contrib/examples/git-fetch.sh
index a314273..06caf6b 100755
--- a/contrib/examples/git-fetch.sh
+++ b/contrib/examples/git-fetch.sh
@@ -67,7 +67,7 @@ do
keep='-k -k'
;;
--depth=*)
- shallow_depth="--depth=`expr "z$1" : 'z-[^=]*=\(.*\)'`"
+ shallow_depth=--depth=`expr "z$1" : 'z-[^=]*=\(.*\)'`
;;
--depth)
shift
diff --git a/t/t9107-git-svn-migrate.sh b/t/t9107-git-svn-migrate.sh
index 289fc31..3d2ae3e 100755
--- a/t/t9107-git-svn-migrate.sh
+++ b/t/t9107-git-svn-migrate.sh
@@ -94,7 +94,7 @@ test_expect_success 'migrate --minimize on old inited layout' '
echo "$svnrepo"$path > "$GIT_DIR"/svn/$ref/info/url ) || exit 1;
done &&
git svn migrate --minimize &&
- test -z "`git config -l | grep "^svn-remote\.git-svn\."`" &&
+ ! git config -l | grep "^svn-remote\.git-svn\." &&
git config --get-all svn-remote.svn.fetch > fetch.out &&
grep "^trunk:refs/remotes/trunk$" fetch.out &&
grep "^branches/a:refs/remotes/a$" fetch.out &&
--
1.7.4.rc1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] Avoid unportable nested double- and backquotes in shell scripts.
2011-01-08 9:01 [PATCH] Avoid unportable nested double- and backquotes in shell scripts Ralf Wildenhues
@ 2011-01-08 16:14 ` Jonathan Nieder
2011-01-08 16:23 ` Ralf Wildenhues
2011-01-10 18:09 ` Junio C Hamano
1 sibling, 1 reply; 6+ messages in thread
From: Jonathan Nieder @ 2011-01-08 16:14 UTC (permalink / raw)
To: Ralf Wildenhues; +Cc: git
Ralf Wildenhues wrote:
> [Subject: Avoid unportable nested double- and backquotes in shell scripts]
>
> Some shells parse them wrongly, esp. pdksh.
How does it treat $( ) command substitutions? (We use those more
heavily and they are easier on the eyes anyway.)
> --- a/t/t9107-git-svn-migrate.sh
> +++ b/t/t9107-git-svn-migrate.sh
> @@ -94,7 +94,7 @@ test_expect_success 'migrate --minimize on old inited layout' '
> echo "$svnrepo"$path > "$GIT_DIR"/svn/$ref/info/url ) || exit 1;
> done &&
> git svn migrate --minimize &&
> - test -z "`git config -l | grep "^svn-remote\.git-svn\."`" &&
> + ! git config -l | grep "^svn-remote\.git-svn\." &&
I thought I remembered portability problems with the
! a | b
construct but it seems I am wrong; t7810-grep.sh uses that
construct without trouble, at least.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Avoid unportable nested double- and backquotes in shell scripts.
2011-01-08 16:14 ` Jonathan Nieder
@ 2011-01-08 16:23 ` Ralf Wildenhues
2011-01-08 16:48 ` Jonathan Nieder
0 siblings, 1 reply; 6+ messages in thread
From: Ralf Wildenhues @ 2011-01-08 16:23 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git
* Jonathan Nieder wrote on Sat, Jan 08, 2011 at 05:14:41PM CET:
> Ralf Wildenhues wrote:
>
> > [Subject: Avoid unportable nested double- and backquotes in shell scripts]
> >
> > Some shells parse them wrongly, esp. pdksh.
>
> How does it treat $( ) command substitutions? (We use those more
> heavily and they are easier on the eyes anyway.)
Better (except for the usual problems when 'case ...)' comes into play).
But git makes heavy use of "no quoting needed on RHS of assignment"
anyway, so it seems like this would be a good move nonetheless. And the
testsuite uses backticks a lot, it seems a move away from that should be
done more uniformly?
Anyway, I'll be happy to respin in whatever form is acceptable.
> > --- a/t/t9107-git-svn-migrate.sh
> > +++ b/t/t9107-git-svn-migrate.sh
> > @@ -94,7 +94,7 @@ test_expect_success 'migrate --minimize on old inited layout' '
> > echo "$svnrepo"$path > "$GIT_DIR"/svn/$ref/info/url ) || exit 1;
> > done &&
> > git svn migrate --minimize &&
> > - test -z "`git config -l | grep "^svn-remote\.git-svn\."`" &&
> > + ! git config -l | grep "^svn-remote\.git-svn\." &&
>
> I thought I remembered portability problems with the
>
> ! a | b
>
> construct but it seems I am wrong; t7810-grep.sh uses that
> construct without trouble, at least.
Some non-Posix-conforming shells have problems with that too, e.g.,
Solaris /bin/sh, but I figured git wouldn't cater to them as I also saw
other such uses in the tree.
Cheers,
Ralf
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Avoid unportable nested double- and backquotes in shell scripts.
2011-01-08 16:23 ` Ralf Wildenhues
@ 2011-01-08 16:48 ` Jonathan Nieder
2011-01-08 18:22 ` Jonathan Nieder
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Nieder @ 2011-01-08 16:48 UTC (permalink / raw)
To: Ralf Wildenhues; +Cc: git
Ralf Wildenhues wrote:
> But git makes heavy use of "no quoting needed on RHS of assignment"
> anyway, so it seems like this would be a good move nonetheless.
No disagreement there.
> And the
> testsuite uses backticks a lot,
>From a quick grep, it seems you are right:
$ git grep -c -F -e '`' -- 't/*.sh' | cut -d: -f2 | sum
65126 1
$ git grep -c -F -e '$(' -- 't/*.sh' | cut -d: -f2 | sum
64807 1
$ git grep -c -F -e '`' -- '*.sh' | cut -d: -f2 | sum
13350 1
$ git grep -c -F -e '$(' -- '*.sh' | cut -d: -f2 | sum
07810 1
Documentation/CodingGuidelines
- We prefer $( ... ) for command substitution; unlike ``, it
properly nests. It should have been the way Bourne spelled
it from day one, but unfortunately isn't.
> it seems a move away from that should be
> done more uniformly?
I don't see why. :) In fact, I personally would not be happy at all
to see such a high-churn patch as that, while using the $( ... )
form in new code and as part of clarifications to other parts of the
same lines would seem to me to be a welcome thing.
Having said all that, I have no strong investment in this. Feel
free to do what works best for you.
Thanks,
Jonathan
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Avoid unportable nested double- and backquotes in shell scripts.
2011-01-08 16:48 ` Jonathan Nieder
@ 2011-01-08 18:22 ` Jonathan Nieder
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Nieder @ 2011-01-08 18:22 UTC (permalink / raw)
To: Ralf Wildenhues; +Cc: git
Jonathan Nieder wrote:
> From a quick grep, it seems you are right:
>
> $ git grep -c -F -e '`' -- 't/*.sh' | cut -d: -f2 | sum
> 65126 1
> $ git grep -c -F -e '$(' -- 't/*.sh' | cut -d: -f2 | sum
> 64807 1
> $ git grep -c -F -e '`' -- '*.sh' | cut -d: -f2 | sum
> 13350 1
> $ git grep -c -F -e '$(' -- '*.sh' | cut -d: -f2 | sum
> 07810 1
sum does something totally different than I expected. With [1]
comes the more reasonable
$ git grep -c -F -e '`' -- 't/*.sh' | cut -d: -f2 | addup
485
$ git grep -c -F -e '$(' -- 't/*.sh' | cut -d: -f2 | addup
2620
$ git grep -c -F -e '`' -- '*.sh' | cut -d: -f2 | addup
594
$ git grep -c -F -e '$(' -- '*.sh' | cut -d: -f2 | addup
3133
So the code bloat and use of backticks are less dire than I feared.
[1]
addup () {
sum=0
while read term
do
: $((sum = $sum + $term))
done
echo $sum
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Avoid unportable nested double- and backquotes in shell scripts.
2011-01-08 9:01 [PATCH] Avoid unportable nested double- and backquotes in shell scripts Ralf Wildenhues
2011-01-08 16:14 ` Jonathan Nieder
@ 2011-01-10 18:09 ` Junio C Hamano
1 sibling, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2011-01-10 18:09 UTC (permalink / raw)
To: Ralf Wildenhues; +Cc: git
Ralf Wildenhues <Ralf.Wildenhues@gmx.de> writes:
> diff --git a/contrib/examples/git-fetch.sh b/contrib/examples/git-fetch.sh
> index a314273..06caf6b 100755
> --- a/contrib/examples/git-fetch.sh
> +++ b/contrib/examples/git-fetch.sh
> @@ -67,7 +67,7 @@ do
> keep='-k -k'
> ;;
> --depth=*)
> - shallow_depth="--depth=`expr "z$1" : 'z-[^=]*=\(.*\)'`"
> + shallow_depth=--depth=`expr "z$1" : 'z-[^=]*=\(.*\)'`
> ;;
> --depth)
> shift
I do not very much like the idea of updating contrib/examples, one of
whose purposes is to document the historical implementation, to ship a
version that has never been battle tested in the field.
There also seem to be a few more uses of `` (the majority used $() even
back then) that are still left behind. To make it more useful to serve as
an example (which is the other purpose of contrib/examples), it would make
more sense to update them all at once.
> diff --git a/t/t9107-git-svn-migrate.sh b/t/t9107-git-svn-migrate.sh
> index 289fc31..3d2ae3e 100755
> --- a/t/t9107-git-svn-migrate.sh
> +++ b/t/t9107-git-svn-migrate.sh
> @@ -94,7 +94,7 @@ test_expect_success 'migrate --minimize on old inited layout' '
> echo "$svnrepo"$path > "$GIT_DIR"/svn/$ref/info/url ) || exit 1;
> done &&
> git svn migrate --minimize &&
> - test -z "`git config -l | grep "^svn-remote\.git-svn\."`" &&
> + ! git config -l | grep "^svn-remote\.git-svn\." &&
> git config --get-all svn-remote.svn.fetch > fetch.out &&
> grep "^trunk:refs/remotes/trunk$" fetch.out &&
> grep "^branches/a:refs/remotes/a$" fetch.out &&
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-01-10 18:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-01-08 9:01 [PATCH] Avoid unportable nested double- and backquotes in shell scripts Ralf Wildenhues
2011-01-08 16:14 ` Jonathan Nieder
2011-01-08 16:23 ` Ralf Wildenhues
2011-01-08 16:48 ` Jonathan Nieder
2011-01-08 18:22 ` Jonathan Nieder
2011-01-10 18:09 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).