threads / patch / 4297

patch, 4 partst5500-fetch-pack: remove local (bashism) usage.

Subject: [PATCH 3/4] t5500-fetch-pack: remove local (bashism) usage.

## tl;dr

13 messages between May 26, 2006 and Sep 26, 2007. Diffs are folded; open one to read it.

replies: 12people: 3as markdown or json

Eric Wong· May 26, 2006, 02:06 UTC · lore

[PATCH 2/4] tests: Remove heredoc usage inside quotes

The use of heredoc inside quoted strings doesn't seem to be supported by dash. pdksh seems to handle it fine, however.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 t/t2101-update-index-reupdate.sh |   48 ++++++++++++++++++++------------------
 t/t4012-diff-binary.sh           |   17 +++++--------
 2 files changed, 31 insertions(+), 34 deletions(-)
250810154c837c6999a954fc6bc4318099b67c01
Show changes to 2 files +31 −34

t/t2101-update-index-reupdate.sh, t/t4012-diff-binary.sh

diff --git a/t/t2101-update-index-reupdate.sh b/t/t2101-update-index-reupdate.sh
index 77aed8d..a78ea7f 100755
--- a/t/t2101-update-index-reupdate.sh
+++ b/t/t2101-update-index-reupdate.sh
@@ -8,15 +8,16 @@ test_description='git-update-index --aga
 
 . ./test-lib.sh
 
+cat > expected <<\EOF
+100644 3b18e512dba79e4c8300dd08aeb37f8e728b8dad 0	file1
+100644 9db8893856a8a02eaa73470054b7c1c5a7c82e47 0	file2
+EOF
 test_expect_success 'update-index --add' \
 	'echo hello world >file1 &&
 	 echo goodbye people >file2 &&
 	 git-update-index --add file1 file2 &&
 	 git-ls-files -s >current &&
-	 cmp current - <<\EOF
-100644 3b18e512dba79e4c8300dd08aeb37f8e728b8dad 0	file1
-100644 9db8893856a8a02eaa73470054b7c1c5a7c82e47 0	file2
-EOF'
+	 cmp current expected'
 
 test_expect_success 'update-index --again' \
 	'rm -f file1 &&
@@ -29,20 +30,22 @@ test_expect_success 'update-index --agai
 		echo happy - failed as expected
 	fi &&
 	 git-ls-files -s >current &&
-	 cmp current - <<\EOF
-100644 3b18e512dba79e4c8300dd08aeb37f8e728b8dad 0	file1
-100644 9db8893856a8a02eaa73470054b7c1c5a7c82e47 0	file2
-EOF'
+	 cmp current expected'
 
+cat > expected <<\EOF
+100644 0f1ae1422c2bf43f117d3dbd715c988a9ed2103f 0	file2
+EOF
 test_expect_success 'update-index --remove --again' \
 	'git-update-index --remove --again &&
 	 git-ls-files -s >current &&
-	 cmp current - <<\EOF
-100644 0f1ae1422c2bf43f117d3dbd715c988a9ed2103f 0	file2
-EOF'
+	 cmp current expected'
 
 test_expect_success 'first commit' 'git-commit -m initial'
 
+cat > expected <<\EOF
+100644 53ab446c3f4e42ce9bb728a0ccb283a101be4979 0	dir1/file3
+100644 0f1ae1422c2bf43f117d3dbd715c988a9ed2103f 0	file2
+EOF
 test_expect_success 'update-index again' \
 	'mkdir -p dir1 &&
 	echo hello world >dir1/file3 &&
@@ -52,11 +55,12 @@ test_expect_success 'update-index again'
 	echo happy >dir1/file3 &&
 	git-update-index --again &&
 	git-ls-files -s >current &&
-	cmp current - <<\EOF
-100644 53ab446c3f4e42ce9bb728a0ccb283a101be4979 0	dir1/file3
-100644 0f1ae1422c2bf43f117d3dbd715c988a9ed2103f 0	file2
-EOF'
+	cmp current expected'
 
+cat > expected <<\EOF
+100644 d7fb3f695f06c759dbf3ab00046e7cc2da22d10f 0	dir1/file3
+100644 0f1ae1422c2bf43f117d3dbd715c988a9ed2103f 0	file2
+EOF
 test_expect_success 'update-index --update from subdir' \
 	'echo not so happy >file2 &&
 	cd dir1 &&
@@ -64,19 +68,17 @@ test_expect_success 'update-index --upda
 	git-update-index --again &&
 	cd .. &&
 	git-ls-files -s >current &&
-	cmp current - <<\EOF
-100644 d7fb3f695f06c759dbf3ab00046e7cc2da22d10f 0	dir1/file3
-100644 0f1ae1422c2bf43f117d3dbd715c988a9ed2103f 0	file2
-EOF'
+	cmp current expected'
 
+cat > expected <<\EOF
+100644 594fb5bb1759d90998e2bf2a38261ae8e243c760 0	dir1/file3
+100644 0f1ae1422c2bf43f117d3dbd715c988a9ed2103f 0	file2
+EOF
 test_expect_success 'update-index --update with pathspec' \
 	'echo very happy >file2 &&
 	cat file2 >dir1/file3 &&
 	git-update-index --again dir1/ &&
 	git-ls-files -s >current &&
-	cmp current - <<\EOF
-100644 594fb5bb1759d90998e2bf2a38261ae8e243c760 0	dir1/file3
-100644 0f1ae1422c2bf43f117d3dbd715c988a9ed2103f 0	file2
-EOF'
+	cmp current expected'
 
 test_done
diff --git a/t/t4012-diff-binary.sh b/t/t4012-diff-binary.sh
index bdd95c0..323606c 100755
--- a/t/t4012-diff-binary.sh
+++ b/t/t4012-diff-binary.sh
@@ -16,25 +16,20 @@ test_expect_success 'prepare repository'
 	 echo git >c &&
 	 cat b b >d'
 
-test_expect_success 'diff without --binary' \
-	'git-diff | git-apply --stat --summary >current &&
-	 cmp current - <<\EOF
+cat > expected <<\EOF
  a |    2 +-
  b |  Bin
  c |    2 +-
  d |  Bin
  4 files changed, 2 insertions(+), 2 deletions(-)
-EOF'
+EOF
+test_expect_success 'diff without --binary' \
+	'git-diff | git-apply --stat --summary >current &&
+	 cmp current expected'
 
 test_expect_success 'diff with --binary' \
 	'git-diff --binary | git-apply --stat --summary >current &&
-	 cmp current - <<\EOF
- a |    2 +-
- b |  Bin
- c |    2 +-
- d |  Bin
- 4 files changed, 2 insertions(+), 2 deletions(-)
-EOF'
+	 cmp current expected'
 
 # apply needs to be able to skip the binary material correctly
 # in order to report the line number of a corrupt patch.

-- 
1.3.2.g7d11
Eric Wong· May 26, 2006, 02:06 UTC · re: Eric Wong · lore
None of the variables seem to conflict, so local was unnecessary.
Also replaced ${var:pos:len} with the sed equivalent.
Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 t/t5500-fetch-pack.sh |   30 +++++++++++++++---------------
 1 files changed, 15 insertions(+), 15 deletions(-)
62f64e95d24f90e61af0fa42d88300e41f60c277
Show changes to t/t5500-fetch-pack.sh +15 −15
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 92f12d9..f7625a6 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -12,11 +12,11 @@ # Test fetch-pack/upload-pack pair.
 
 # Some convenience functions
 
-function add () {
-	local name=$1
-	local text="$@"
-	local branch=${name:0:1}
-	local parents=""
+add () {
+	name=$1
+	text="$@"
+	branch=`echo $name | sed -e 's/^\(.\).*$/\1/'`
+	parents=""
 
 	shift
 	while test $1; do
@@ -36,13 +36,13 @@ function add () {
 	eval ${branch}TIP=$commit
 }
 
-function count_objects () {
+count_objects () {
 	ls .git/objects/??/* 2>>log2.txt | wc -l | tr -d " "
 }
 
-function test_expect_object_count () {
-	local message=$1
-	local count=$2
+test_expect_object_count () {
+	message=$1
+	count=$2
 
 	output="$(count_objects)"
 	test_expect_success \
@@ -50,18 +50,18 @@ function test_expect_object_count () {
 		"test $count = $output"
 }
 
-function pull_to_client () {
-	local number=$1
-	local heads=$2
-	local count=$3
-	local no_strict_count_check=$4
+pull_to_client () {
+	number=$1
+	heads=$2
+	count=$3
+	no_strict_count_check=$4
 
 	cd client
 	test_expect_success "$number pull" \
 		"git-fetch-pack -k -v .. $heads"
 	case "$heads" in *A*) echo $ATIP > .git/refs/heads/A;; esac
 	case "$heads" in *B*) echo $BTIP > .git/refs/heads/B;; esac
-	git-symbolic-ref HEAD refs/heads/${heads:0:1}
+	git-symbolic-ref HEAD refs/heads/`echo $heads | sed -e 's/^\(.\).*$/\1/'`
 
 	test_expect_success "fsck" 'git-fsck-objects --full > fsck.txt 2>&1'
 

-- 
1.3.2.g7d11
Eric Wong· May 26, 2006, 02:06 UTC · re: Eric Wong · lore

[PATCH 4/4] t6000lib: workaround a possible dash bug

pdksh doesn't need this patch, of course bash works fine since that what most users use.

Normally, 'var=val command' seems to work fine with dash, but perhaps there's something weird going on with "$@". dash is pretty widespread, so it'll be good to support this even though it does seem like a bug in dash.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 t/t6000lib.sh |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
cba907ce0b1c0927fb15cbb5dd91a4129ff9a950
Show changes to t/t6000lib.sh +3 −1
diff --git a/t/t6000lib.sh b/t/t6000lib.sh
index c6752af..d402621 100755
--- a/t/t6000lib.sh
+++ b/t/t6000lib.sh
@@ -69,7 +69,9 @@ on_committer_date()
 {
     _date=$1
     shift 1
-    GIT_COMMITTER_DATE=$_date "$@"
+    export GIT_COMMITTER_DATE="$_date"
+    "$@"
+    unset GIT_COMMITTER_DATE
 }
 
 # Execute a command and suppress any error output.

-- 
1.3.2.g7d11
Herbert Xu· May 26, 2006, 12:33 UTC · re: Eric Wong · lore

Re: [PATCH 4/4] t6000lib: workaround a possible dash bug

On Thu, May 25, 2006 at 07:06:18PM -0700, Eric Wong wrote:
Show 17 quoted lines
>
>  t/t6000lib.sh |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> cba907ce0b1c0927fb15cbb5dd91a4129ff9a950
> diff --git a/t/t6000lib.sh b/t/t6000lib.sh
> index c6752af..d402621 100755
> --- a/t/t6000lib.sh
> +++ b/t/t6000lib.sh
> @@ -69,7 +69,9 @@ on_committer_date()
>  {
>      _date=$1
>      shift 1
> -    GIT_COMMITTER_DATE=$_date "$@"
> +    export GIT_COMMITTER_DATE="$_date"
> +    "$@"
> +    unset GIT_COMMITTER_DATE

The original code looks correct to me. So I think this too should be fixed in dash instead.

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Herbert Xu· Sep 21, 2007, 13:28 UTC · re: Eric Wong · lore

Re: [PATCH 4/4] t6000lib: workaround a possible dash bug

Hi Eric:
On Thu, May 25, 2006 at 07:06:18PM -0700, Eric Wong wrote:
Show 7 quoted lines
> pdksh doesn't need this patch, of course bash works fine since
> that what most users use.
> 
> Normally, 'var=val command' seems to work fine with dash, but
> perhaps there's something weird going on with "$@".  dash is
> pretty widespread, so it'll be good to support this even though
> it does seem like a bug in dash.

Just going through dash issues right now. Do you recall what the bug is in this case? Doing a quick test doesn't seem to show much:

dash -c 'set -- env; a=b "$@"'
Show 17 quoted lines
> diff --git a/t/t6000lib.sh b/t/t6000lib.sh
> index c6752af..d402621 100755
> --- a/t/t6000lib.sh
> +++ b/t/t6000lib.sh
> @@ -69,7 +69,9 @@ on_committer_date()
>  {
>      _date=$1
>      shift 1
> -    GIT_COMMITTER_DATE=$_date "$@"
> +    export GIT_COMMITTER_DATE="$_date"
> +    "$@"
> +    unset GIT_COMMITTER_DATE
>  }
>  
>  # Execute a command and suppress any error output.
> -- 
> 1.3.2.g7d11
Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Eric Wong· Sep 21, 2007, 20:45 UTC · re: Herbert Xu · lore

Re: [PATCH 4/4] t6000lib: workaround a possible dash bug

Herbert Xu <herbert@gondor.apana.org.au> wrote:
> Hi Eric:
Hi Herbert,
Show 14 quoted lines
> On Thu, May 25, 2006 at 07:06:18PM -0700, Eric Wong wrote:
> > pdksh doesn't need this patch, of course bash works fine since
> > that what most users use.
> > 
> > Normally, 'var=val command' seems to work fine with dash, but
> > perhaps there's something weird going on with "$@".  dash is
> > pretty widespread, so it'll be good to support this even though
> > it does seem like a bug in dash.
> 
> Just going through dash issues right now.  Do you recall
> what the bug is in this case? Doing a quick test doesn't
> seem to show much:
> 
> dash -c 'set -- env; a=b "$@"'

I tried to reproduce it on a quick script using shell functions, multiple arguments, spaces in the $a variable.., but haven't been successful. However, reverting the below patch still causes errors in the latest git test suite.

Show 19 quoted lines
> > diff --git a/t/t6000lib.sh b/t/t6000lib.sh
> > index c6752af..d402621 100755
> > --- a/t/t6000lib.sh
> > +++ b/t/t6000lib.sh
> > @@ -69,7 +69,9 @@ on_committer_date()
> >  {
> >      _date=$1
> >      shift 1
> > -    GIT_COMMITTER_DATE=$_date "$@"
> > +    export GIT_COMMITTER_DATE="$_date"
> > +    "$@"
> > +    unset GIT_COMMITTER_DATE
> >  }
> >  
> >  # Execute a command and suppress any error output.
> > -- 
> > 1.3.2.g7d11
> 
> Thanks,

I'm using dash 0.5.3-7 from Debian Etch on x86-32. (git @ 17ed158021ead9cb056f692fc35ff3fcde96a747)

*** t6003-rev-list-topo-order.sh ***
*   ok 1: rev-list has correct number of entries
*   ok 2: simple topo order
*   ok 3: two diamonds topo order (g6)
* FAIL 4: multiple heads
	check_output multiple-heads "git rev-list --topo-order a3 b3 c3"
* FAIL 5: multiple heads, prune at a1
	check_output multiple-heads-prune-at-a1 "git rev-list --topo-order a3 b3 c3 ^a1"
* FAIL 6: multiple heads, prune at l1
	check_output multiple-heads-prune-at-l1 "git rev-list --topo-order a3 b3 c3 ^l1"
*   ok 7: cross-epoch, head at l5, prune at l1
*   ok 8: duplicated head arguments
*   ok 9: prune near topo
*   ok 10: head has no parent
*   ok 11: two nodes - one head, one base
*   ok 12: three nodes one head, one internal, one base
*   ok 13: linear prune l2 ^root
*   ok 14: linear prune l2 ^l0
*   ok 15: linear prune l2 ^l1
*   ok 16: linear prune l5 ^a4
*   ok 17: linear prune l5 ^l3
*   ok 18: linear prune l5 ^l4
*   ok 19: max-count 10 - topo order
* FAIL 20: max-count 10 - non topo order
	check_output max-count-10-non-topo-order "git rev-list --max-count=10 l5"
* FAIL 21: --max-age=c3, no --topo-order
	check_output max-age-c3-no-topo-order "git rev-list --max-age=1190407285 l5"
*   ok 22: one specified head reachable from another a4, c3, --topo-order
*   ok 23: one specified head reachable from another c3, a4, --topo-order
*   ok 24: one specified head reachable from another a4, c3, no --topo-order
*   ok 25: one specified head reachable from another c3, a4, no --topo-order
*   ok 26: graph with c3 and a4 parents of head
*   ok 27: graph with a4 and c3 parents of head
*   ok 28: head ^head --topo-order
*   ok 29: head ^head no --topo-order
*   ok 30: simple topo order (l5r1)
*   ok 31: simple topo order (r1l5)
*   ok 32: don't print things unreachable from one branch
*   ok 33: --topo-order a4 l3
* failed 5 among 33 test(s)
make: *** [t6003-rev-list-topo-order.sh] Error 1
-- 
Eric Wong
Herbert Xu· Sep 26, 2007, 08:41 UTC · re: Eric Wong · lore

Re: [PATCH 4/4] t6000lib: workaround a possible dash bug

On Fri, Sep 21, 2007 at 01:45:11PM -0700, Eric Wong wrote:
Show 5 quoted lines
>
> I tried to reproduce it on a quick script using shell functions,
> multiple arguments, spaces in the $a variable.., but haven't
> been successful.  However, reverting the below patch still
> causes errors in the latest git test suite.

Ah I see, it's a function. Unfortunately POSIX requires shell functions to have the variable assignment properties of special built-ins. So

	X=value func
has the same properties as
	X=value :

In other words, the value assigned to X (and any subsequent values assigned within the function) persists after the call. Also, the variable X is not exported unless it's already been exported.

This is pretty lame but it's how the original Korn shell did things and POSIX has adopted that. Bash's POSIX mode tries to balance things by both making the value persist and exporting X. Unfortunately this is buggy too as it causes X to continue to be exported after the function returns.

So the bottom-line is that your patch is the correct solution after all :)

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Herbert Xu· May 26, 2006, 12:23 UTC · re: Eric Wong · lore

Re: [PATCH 3/4] t5500-fetch-pack: remove local (bashism) usage.

On Thu, May 25, 2006 at 07:06:17PM -0700, Eric Wong wrote:
> None of the variables seem to conflict, so local was unnecessary.

BTW, dash supports (and has always supported) local which is a quite useful feature.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Eric Wong· May 29, 2006, 05:28 UTC · re: Herbert Xu · lore

Re: [PATCH 3/4] t5500-fetch-pack: remove local (bashism) usage.

Herbert Xu <herbert@gondor.apana.org.au> wrote:
Show 5 quoted lines
> On Thu, May 25, 2006 at 07:06:17PM -0700, Eric Wong wrote:
> > None of the variables seem to conflict, so local was unnecessary.
> 
> BTW, dash supports (and has always supported) local which is a quite
> useful feature.

Cool. Hmm... pdksh seems to support it here (Debian sid). I'm pretty sure local is not part of the POSIX spec, though; and I have seen /bin/sh that don't support it.

-- 
Eric Wong
Junio C Hamano· May 29, 2006, 05:36 UTC · re: Eric Wong · lore

Re: [PATCH 3/4] t5500-fetch-pack: remove local (bashism) usage.

Eric Wong <normalperson@yhbt.net> writes:
Show 10 quoted lines
> Herbert Xu <herbert@gondor.apana.org.au> wrote:
>> On Thu, May 25, 2006 at 07:06:17PM -0700, Eric Wong wrote:
>> > None of the variables seem to conflict, so local was unnecessary.
>> 
>> BTW, dash supports (and has always supported) local which is a quite
>> useful feature.
>
> Cool.  Hmm... pdksh seems to support it here (Debian sid).  I'm pretty
> sure local is not part of the POSIX spec, though; and I have seen
> /bin/sh that don't support it.

Concurred. There are things Herbert said are clearly dash bugs, but this one is outside POSIX, so lets leave your changes to the test for it.

Herbert Xu· May 29, 2006, 07:31 UTC · re: Eric Wong · lore

Re: [PATCH 3/4] t5500-fetch-pack: remove local (bashism) usage.

On Sun, May 28, 2006 at 10:28:28PM -0700, Eric Wong wrote:
>
> Cool.  Hmm... pdksh seems to support it here (Debian sid).  I'm pretty
> sure local is not part of the POSIX spec, though; and I have seen
> /bin/sh that don't support it.

It is true that the current POSIX spec does not specify it. However, all useful POSIX-compliant shells on Linux (i.e., excluding those shells that exist only to test POSIX compliance) support it and it is used by a large corpus of existing Linux scripts.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Herbert Xu· May 26, 2006, 12:22 UTC · re: Eric Wong · lore

Re: [PATCH 2/4] tests: Remove heredoc usage inside quotes

On Thu, May 25, 2006 at 07:06:16PM -0700, Eric Wong wrote:
> The use of heredoc inside quoted strings doesn't seem to be
> supported by dash.  pdksh seems to handle it fine, however.

This is a bug in dash and should be fixed there instead. Thanks for drawing my attention to it.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Eric Wong· May 29, 2006, 05:30 UTC · re: Herbert Xu · lore

Re: [PATCH 2/4] tests: Remove heredoc usage inside quotes

Herbert Xu <herbert@gondor.apana.org.au> wrote:
Show 6 quoted lines
> On Thu, May 25, 2006 at 07:06:16PM -0700, Eric Wong wrote:
> > The use of heredoc inside quoted strings doesn't seem to be
> > supported by dash.  pdksh seems to handle it fine, however.
> 
> This is a bug in dash and should be fixed there instead.
> Thanks for drawing my attention to it.

No problem. I think these dash bugs should be fixed in dash, but continue to be worked around in git as old versions of dash will probably continue to exist for a long time.

-- 
Eric Wong

← back to recent threads