git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH 0/4] fix tests so they run without needing bash
@ 2006-05-26  2:06 Eric Wong
  2006-05-26  2:06 ` [PATCH 1/4] t3300-funny-names: shell portability fixes Eric Wong
  2006-05-26  3:01 ` [PATCH 0/4] fix tests so they run without needing bash Junio C Hamano
  0 siblings, 2 replies; 17+ messages in thread
From: Eric Wong @ 2006-05-26  2:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Herbert Xu

I've tested these patches with bash, pdksh, and dash.  dash seems
to be 7s faster than bash (28s vs 35s) when running `make test',
and just a hair faster than pdksh.

Herbert:
patches #2 and #4 are required for dash, but not for pdksh.
I'm not sure about #2, but I'm pretty certain the problem #4 works
around is a bug in dash.

[PATCH 1/4] t3300-funny-names: shell portability fixes
[PATCH 2/4] tests: Remove heredoc usage inside quotes
[PATCH 3/4] t5500-fetch-pack: remove local (bashism) usage.
[PATCH 4/4] t6000lib: workaround a possible dash bug

-- 
Eric Wong

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/4] t3300-funny-names: shell portability fixes
  2006-05-26  2:06 [PATCH 0/4] fix tests so they run without needing bash Eric Wong
@ 2006-05-26  2:06 ` Eric Wong
  2006-05-26  2:06   ` [PATCH 2/4] tests: Remove heredoc usage inside quotes Eric Wong
  2006-05-26  3:01 ` [PATCH 0/4] fix tests so they run without needing bash Junio C Hamano
  1 sibling, 1 reply; 17+ messages in thread
From: Eric Wong @ 2006-05-26  2:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Herbert Xu, Eric Wong

echo isn't remotely standardized for handling backslashes,
so cat + heredoc seems better

Signed-off-by: Eric Wong <normalperson@yhbt.net>

---

 t/t3300-funny-names.sh |   51 +++++++++++++++++++++++++++++++-----------------
 1 files changed, 33 insertions(+), 18 deletions(-)

6b5ef54aa860b557f88084c1d642a611f7abcf4d
diff --git a/t/t3300-funny-names.sh b/t/t3300-funny-names.sh
index 72a93da..c12270e 100755
--- a/t/t3300-funny-names.sh
+++ b/t/t3300-funny-names.sh
@@ -40,9 +40,11 @@ test_expect_success 'git-ls-files no-fun
 t0=`git-write-tree`
 echo "$t0" >t0
 
-echo 'just space
+cat > expected <<\EOF
+just space
 no-funny
-"tabs\t,\" (dq) and spaces"' >expected
+"tabs\t,\" (dq) and spaces"
+EOF
 test_expect_success 'git-ls-files with-funny' \
 	'git-update-index --add "$p1" &&
 	git-ls-files >current &&
@@ -58,14 +60,18 @@ test_expect_success 'git-ls-files -z wit
 t1=`git-write-tree`
 echo "$t1" >t1
 
-echo 'just space
+cat > expected <<\EOF
+just space
 no-funny
-"tabs\t,\" (dq) and spaces"' >expected
+"tabs\t,\" (dq) and spaces"
+EOF
 test_expect_success 'git-ls-tree with funny' \
 	'git-ls-tree -r $t1 | sed -e "s/^[^	]*	//" >current &&
 	 diff -u expected current'
 
-echo 'A	"tabs\t,\" (dq) and spaces"' >expected
+cat > expected <<\EOF
+A	"tabs\t,\" (dq) and spaces"
+EOF
 test_expect_success 'git-diff-index with-funny' \
 	'git-diff-index --name-status $t0 >current &&
 	diff -u expected current'
@@ -84,53 +90,62 @@ test_expect_success 'git-diff-tree -z wi
 	'git-diff-tree -z --name-status $t0 $t1 | tr \\0 \\012 >current &&
 	diff -u expected current'
 
-echo 'CNUM	no-funny	"tabs\t,\" (dq) and spaces"' >expected
+cat > expected <<\EOF
+CNUM	no-funny	"tabs\t,\" (dq) and spaces"
+EOF
 test_expect_success 'git-diff-tree -C with-funny' \
 	'git-diff-tree -C --find-copies-harder --name-status \
 		$t0 $t1 | sed -e 's/^C[0-9]*/CNUM/' >current &&
 	diff -u expected current'
 
-echo 'RNUM	no-funny	"tabs\t,\" (dq) and spaces"' >expected
+cat > expected <<\EOF
+RNUM	no-funny	"tabs\t,\" (dq) and spaces"
+EOF
 test_expect_success 'git-diff-tree delete with-funny' \
 	'git-update-index --force-remove "$p0" &&
 	git-diff-index -M --name-status \
 		$t0 | sed -e 's/^R[0-9]*/RNUM/' >current &&
 	diff -u expected current'
 
-echo 'diff --git a/no-funny "b/tabs\t,\" (dq) and spaces"
+cat > expected <<\EOF
+diff --git a/no-funny "b/tabs\t,\" (dq) and spaces"
 similarity index NUM%
 rename from no-funny
-rename to "tabs\t,\" (dq) and spaces"' >expected
-
+rename to "tabs\t,\" (dq) and spaces"
+EOF
 test_expect_success 'git-diff-tree delete with-funny' \
 	'git-diff-index -M -p $t0 |
 	 sed -e "s/index [0-9]*%/index NUM%/" >current &&
 	 diff -u expected current'
 
 chmod +x "$p1"
-echo 'diff --git a/no-funny "b/tabs\t,\" (dq) and spaces"
+cat > expected <<\EOF
+diff --git a/no-funny "b/tabs\t,\" (dq) and spaces"
 old mode 100644
 new mode 100755
 similarity index NUM%
 rename from no-funny
-rename to "tabs\t,\" (dq) and spaces"' >expected
-
+rename to "tabs\t,\" (dq) and spaces"
+EOF
 test_expect_success 'git-diff-tree delete with-funny' \
 	'git-diff-index -M -p $t0 |
 	 sed -e "s/index [0-9]*%/index NUM%/" >current &&
 	 diff -u expected current'
 
-echo >expected ' "tabs\t,\" (dq) and spaces"
- 1 files changed, 0 insertions(+), 0 deletions(-)'
+cat >expected <<\EOF
+ "tabs\t,\" (dq) and spaces"
+ 1 files changed, 0 insertions(+), 0 deletions(-)
+EOF
 test_expect_success 'git-diff-tree rename with-funny applied' \
 	'git-diff-index -M -p $t0 |
 	 git-apply --stat | sed -e "s/|.*//" -e "s/ *\$//" >current &&
 	 diff -u expected current'
 
-echo >expected ' no-funny
+cat > expected <<\EOF
+ no-funny
  "tabs\t,\" (dq) and spaces"
- 2 files changed, 3 insertions(+), 3 deletions(-)'
-
+ 2 files changed, 3 insertions(+), 3 deletions(-)
+EOF
 test_expect_success 'git-diff-tree delete with-funny applied' \
 	'git-diff-index -p $t0 |
 	 git-apply --stat | sed -e "s/|.*//" -e "s/ *\$//" >current &&
-- 
1.3.2.g7d11

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/4] tests: Remove heredoc usage inside quotes
  2006-05-26  2:06 ` [PATCH 1/4] t3300-funny-names: shell portability fixes Eric Wong
@ 2006-05-26  2:06   ` Eric Wong
  2006-05-26  2:06     ` [PATCH 3/4] t5500-fetch-pack: remove local (bashism) usage Eric Wong
  2006-05-26 12:22     ` [PATCH 2/4] tests: Remove heredoc usage inside quotes Herbert Xu
  0 siblings, 2 replies; 17+ messages in thread
From: Eric Wong @ 2006-05-26  2:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Herbert Xu, Eric Wong

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
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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/4] t5500-fetch-pack: remove local (bashism) usage.
  2006-05-26  2:06   ` [PATCH 2/4] tests: Remove heredoc usage inside quotes Eric Wong
@ 2006-05-26  2:06     ` Eric Wong
  2006-05-26  2:06       ` [PATCH 4/4] t6000lib: workaround a possible dash bug Eric Wong
  2006-05-26 12:23       ` [PATCH 3/4] t5500-fetch-pack: remove local (bashism) usage Herbert Xu
  2006-05-26 12:22     ` [PATCH 2/4] tests: Remove heredoc usage inside quotes Herbert Xu
  1 sibling, 2 replies; 17+ messages in thread
From: Eric Wong @ 2006-05-26  2:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Herbert Xu, Eric Wong

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
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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 4/4] t6000lib: workaround a possible dash bug
  2006-05-26  2:06     ` [PATCH 3/4] t5500-fetch-pack: remove local (bashism) usage Eric Wong
@ 2006-05-26  2:06       ` Eric Wong
  2006-05-26 12:33         ` Herbert Xu
  2007-09-21 13:28         ` Herbert Xu
  2006-05-26 12:23       ` [PATCH 3/4] t5500-fetch-pack: remove local (bashism) usage Herbert Xu
  1 sibling, 2 replies; 17+ messages in thread
From: Eric Wong @ 2006-05-26  2:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Herbert Xu, Eric Wong

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
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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/4] fix tests so they run without needing bash
  2006-05-26  2:06 [PATCH 0/4] fix tests so they run without needing bash Eric Wong
  2006-05-26  2:06 ` [PATCH 1/4] t3300-funny-names: shell portability fixes Eric Wong
@ 2006-05-26  3:01 ` Junio C Hamano
  2006-05-29  5:39   ` Eric Wong
  1 sibling, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2006-05-26  3:01 UTC (permalink / raw)
  To: Eric Wong; +Cc: git

You are my hero.

I've really been wanting to get rid of bashism and trying to run
everything with dash was one of my goals.  Although recent trend
seems to indicate people are more interested in getting rid of
shell scripts altogether, it is nice to see somebody actually
cares.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/4] tests: Remove heredoc usage inside quotes
  2006-05-26  2:06   ` [PATCH 2/4] tests: Remove heredoc usage inside quotes Eric Wong
  2006-05-26  2:06     ` [PATCH 3/4] t5500-fetch-pack: remove local (bashism) usage Eric Wong
@ 2006-05-26 12:22     ` Herbert Xu
  2006-05-29  5:30       ` Eric Wong
  1 sibling, 1 reply; 17+ messages in thread
From: Herbert Xu @ 2006-05-26 12:22 UTC (permalink / raw)
  To: Eric Wong; +Cc: Junio C Hamano, git

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/4] t5500-fetch-pack: remove local (bashism) usage.
  2006-05-26  2:06     ` [PATCH 3/4] t5500-fetch-pack: remove local (bashism) usage Eric Wong
  2006-05-26  2:06       ` [PATCH 4/4] t6000lib: workaround a possible dash bug Eric Wong
@ 2006-05-26 12:23       ` Herbert Xu
  2006-05-29  5:28         ` Eric Wong
  1 sibling, 1 reply; 17+ messages in thread
From: Herbert Xu @ 2006-05-26 12:23 UTC (permalink / raw)
  To: Eric Wong; +Cc: Junio C Hamano, git

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/4] t6000lib: workaround a possible dash bug
  2006-05-26  2:06       ` [PATCH 4/4] t6000lib: workaround a possible dash bug Eric Wong
@ 2006-05-26 12:33         ` Herbert Xu
  2007-09-21 13:28         ` Herbert Xu
  1 sibling, 0 replies; 17+ messages in thread
From: Herbert Xu @ 2006-05-26 12:33 UTC (permalink / raw)
  To: Eric Wong; +Cc: Junio C Hamano, git

On Thu, May 25, 2006 at 07:06:18PM -0700, Eric Wong wrote:
>
>  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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/4] t5500-fetch-pack: remove local (bashism) usage.
  2006-05-26 12:23       ` [PATCH 3/4] t5500-fetch-pack: remove local (bashism) usage Herbert Xu
@ 2006-05-29  5:28         ` Eric Wong
  2006-05-29  5:36           ` Junio C Hamano
  2006-05-29  7:31           ` Herbert Xu
  0 siblings, 2 replies; 17+ messages in thread
From: Eric Wong @ 2006-05-29  5:28 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Junio C Hamano, git

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.

-- 
Eric Wong

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/4] tests: Remove heredoc usage inside quotes
  2006-05-26 12:22     ` [PATCH 2/4] tests: Remove heredoc usage inside quotes Herbert Xu
@ 2006-05-29  5:30       ` Eric Wong
  0 siblings, 0 replies; 17+ messages in thread
From: Eric Wong @ 2006-05-29  5:30 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Junio C Hamano, git

Herbert Xu <herbert@gondor.apana.org.au> wrote:
> 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/4] t5500-fetch-pack: remove local (bashism) usage.
  2006-05-29  5:28         ` Eric Wong
@ 2006-05-29  5:36           ` Junio C Hamano
  2006-05-29  7:31           ` Herbert Xu
  1 sibling, 0 replies; 17+ messages in thread
From: Junio C Hamano @ 2006-05-29  5:36 UTC (permalink / raw)
  To: Eric Wong; +Cc: git

Eric Wong <normalperson@yhbt.net> writes:

> 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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/4] fix tests so they run without needing bash
  2006-05-26  3:01 ` [PATCH 0/4] fix tests so they run without needing bash Junio C Hamano
@ 2006-05-29  5:39   ` Eric Wong
  0 siblings, 0 replies; 17+ messages in thread
From: Eric Wong @ 2006-05-29  5:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Junio C Hamano <junkio@cox.net> wrote:
> You are my hero.

:)

You and Linus have already been my heroes for a while.

I was very pleasantly surprised that all the git-*.sh files work fine
(for me, at least) and only the tests needed modification.

-- 
Eric Wong

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/4] t5500-fetch-pack: remove local (bashism) usage.
  2006-05-29  5:28         ` Eric Wong
  2006-05-29  5:36           ` Junio C Hamano
@ 2006-05-29  7:31           ` Herbert Xu
  1 sibling, 0 replies; 17+ messages in thread
From: Herbert Xu @ 2006-05-29  7:31 UTC (permalink / raw)
  To: Eric Wong; +Cc: Junio C Hamano, git

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/4] t6000lib: workaround a possible dash bug
  2006-05-26  2:06       ` [PATCH 4/4] t6000lib: workaround a possible dash bug Eric Wong
  2006-05-26 12:33         ` Herbert Xu
@ 2007-09-21 13:28         ` Herbert Xu
  2007-09-21 20:45           ` Eric Wong
  1 sibling, 1 reply; 17+ messages in thread
From: Herbert Xu @ 2007-09-21 13:28 UTC (permalink / raw)
  To: Eric Wong; +Cc: Junio C Hamano, git

Hi Eric:

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 "$@"'

> 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/4] t6000lib: workaround a possible dash bug
  2007-09-21 13:28         ` Herbert Xu
@ 2007-09-21 20:45           ` Eric Wong
  2007-09-26  8:41             ` Herbert Xu
  0 siblings, 1 reply; 17+ messages in thread
From: Eric Wong @ 2007-09-21 20:45 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Junio C Hamano, git

Herbert Xu <herbert@gondor.apana.org.au> wrote:
> Hi Eric:

Hi Herbert,

> 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.

> > 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/4] t6000lib: workaround a possible dash bug
  2007-09-21 20:45           ` Eric Wong
@ 2007-09-26  8:41             ` Herbert Xu
  0 siblings, 0 replies; 17+ messages in thread
From: Herbert Xu @ 2007-09-26  8:41 UTC (permalink / raw)
  To: Eric Wong; +Cc: Junio C Hamano, git

On Fri, Sep 21, 2007 at 01:45:11PM -0700, Eric Wong wrote:
>
> 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2007-09-26  8:42 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-05-26  2:06 [PATCH 0/4] fix tests so they run without needing bash Eric Wong
2006-05-26  2:06 ` [PATCH 1/4] t3300-funny-names: shell portability fixes Eric Wong
2006-05-26  2:06   ` [PATCH 2/4] tests: Remove heredoc usage inside quotes Eric Wong
2006-05-26  2:06     ` [PATCH 3/4] t5500-fetch-pack: remove local (bashism) usage Eric Wong
2006-05-26  2:06       ` [PATCH 4/4] t6000lib: workaround a possible dash bug Eric Wong
2006-05-26 12:33         ` Herbert Xu
2007-09-21 13:28         ` Herbert Xu
2007-09-21 20:45           ` Eric Wong
2007-09-26  8:41             ` Herbert Xu
2006-05-26 12:23       ` [PATCH 3/4] t5500-fetch-pack: remove local (bashism) usage Herbert Xu
2006-05-29  5:28         ` Eric Wong
2006-05-29  5:36           ` Junio C Hamano
2006-05-29  7:31           ` Herbert Xu
2006-05-26 12:22     ` [PATCH 2/4] tests: Remove heredoc usage inside quotes Herbert Xu
2006-05-29  5:30       ` Eric Wong
2006-05-26  3:01 ` [PATCH 0/4] fix tests so they run without needing bash Junio C Hamano
2006-05-29  5:39   ` Eric Wong

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).