git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 01/16] tr portability fixes
       [not found] <cover.1205356737.git.peff@peff.net>
@ 2008-03-12 21:29 ` Jeff King
  2008-03-13  7:32   ` Johannes Sixt
  2008-03-12 21:30 ` [PATCH 02/16] t0050: perl portability fix Jeff King
                   ` (14 subsequent siblings)
  15 siblings, 1 reply; 59+ messages in thread
From: Jeff King @ 2008-03-12 21:29 UTC (permalink / raw)
  To: Whit Armstrong; +Cc: Junio C Hamano, git

Specifying character ranges in tr differs between System V
and POSIX. In System V, brackets are required (e.g.,
'[A-Z]'), whereas in POSIX they are not.

We can mostly get around this by just using the bracket form
for both sets, as in:

  tr '[A-Z] '[a-z]'

in which case POSIX interpets this as "'[' becomes '['",
which is OK.

However, this doesn't work with multiple sequences, like:

  # rot13
  tr '[A-Z][a-z]' '[N-Z][A-M][n-z][a-m]'

where the POSIX version does not behave the same as the
System V version. In this case, we must simply enumerate the
sequence.

This patch fixes problematic uses of tr in git scripts and
test scripts in one of three ways:

  - if a single sequence, make sure it uses brackets
  - if multiple sequences, enumerate
  - if extra brackets (e.g., tr '[A]' 'a'), eliminate
    brackets

Signed-off-by: Jeff King <peff@peff.net>
---
This was posted earlier, but mid-thread.

 git-bisect.sh            |    4 ++--
 git-filter-branch.sh     |    4 ++--
 t/t4022-diff-rewrite.sh  |    5 ++++-
 t/t7003-filter-branch.sh |    2 +-
 4 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/git-bisect.sh b/git-bisect.sh
index 2c32d0b..48fb92d 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -293,14 +293,14 @@ bisect_next() {
 	bisect_next_check good
 
 	skip=$(git for-each-ref --format='%(objectname)' \
-		"refs/bisect/skip-*" | tr '[\012]' ' ') || exit
+		"refs/bisect/skip-*" | tr '\012' ' ') || exit
 
 	BISECT_OPT=''
 	test -n "$skip" && BISECT_OPT='--bisect-all'
 
 	bad=$(git rev-parse --verify refs/bisect/bad) &&
 	good=$(git for-each-ref --format='^%(objectname)' \
-		"refs/bisect/good-*" | tr '[\012]' ' ') &&
+		"refs/bisect/good-*" | tr '\012' ' ') &&
 	eval="git rev-list --bisect-vars $BISECT_OPT $good $bad --" &&
 	eval="$eval $(cat "$GIT_DIR/BISECT_NAMES")" &&
 	eval=$(filter_skipped "$eval" "$skip") &&
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 010353a..59cf023 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -58,8 +58,8 @@ eval "$functions"
 # "author" or "committer
 
 set_ident () {
-	lid="$(echo "$1" | tr "A-Z" "a-z")"
-	uid="$(echo "$1" | tr "a-z" "A-Z")"
+	lid="$(echo "$1" | tr "[A-Z]" "[a-z]")"
+	uid="$(echo "$1" | tr "[a-z]" "[A-Z]")"
 	pick_id_script='
 		/^'$lid' /{
 			s/'\''/'\''\\'\'\''/g
diff --git a/t/t4022-diff-rewrite.sh b/t/t4022-diff-rewrite.sh
index 6de4acb..bf996fc 100755
--- a/t/t4022-diff-rewrite.sh
+++ b/t/t4022-diff-rewrite.sh
@@ -8,7 +8,10 @@ test_expect_success setup '
 
 	cat ../../COPYING >test &&
 	git add test &&
-	tr 'a-zA-Z' 'n-za-mN-ZA-M' <../../COPYING >test
+	tr \
+	  "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ" \
+	  "nopqrstuvwxyzabcdefghijklmNOPQRSTUVWXYZABCDEFGHIJKLM" \
+	  <../../COPYING >test
 
 '
 
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 6e14bf1..553131f 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -4,7 +4,7 @@ test_description='git-filter-branch'
 . ./test-lib.sh
 
 make_commit () {
-	lower=$(echo $1 | tr A-Z a-z)
+	lower=$(echo $1 | tr '[A-Z]' '[a-z]')
 	echo $lower > $lower
 	git add $lower
 	test_tick
-- 
1.5.4.4.543.g30fdd.dirty

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

* [PATCH 02/16] t0050: perl portability fix
       [not found] <cover.1205356737.git.peff@peff.net>
  2008-03-12 21:29 ` [PATCH 01/16] tr portability fixes Jeff King
@ 2008-03-12 21:30 ` Jeff King
  2008-03-13  7:38   ` Johannes Sixt
  2008-03-12 21:31 ` [PATCH 03/16] more tr portability test script fixes Jeff King
                   ` (13 subsequent siblings)
  15 siblings, 1 reply; 59+ messages in thread
From: Jeff King @ 2008-03-12 21:30 UTC (permalink / raw)
  To: Whit Armstrong; +Cc: Junio C Hamano, git

Older versions of perl (such as 5.005) don't understand -CO, nor
do they understand the "U" pack specifier. Instead of using perl,
let's just printf the binary bytes we are interested in.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t0050-filesystem.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t0050-filesystem.sh b/t/t0050-filesystem.sh
index cd088b3..3fbad77 100755
--- a/t/t0050-filesystem.sh
+++ b/t/t0050-filesystem.sh
@@ -4,8 +4,8 @@ test_description='Various filesystem issues'
 
 . ./test-lib.sh
 
-auml=`perl -CO -e 'print pack("U",0x00E4)'`
-aumlcdiar=`perl -CO -e 'print pack("U",0x0061).pack("U",0x0308)'`
+auml=`printf '\xc3\xa4'`
+aumlcdiar=`printf '\x61\xcc\x88'`
 
 test_expect_success 'see if we expect ' '
 
-- 
1.5.4.4.543.g30fdd.dirty

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

* [PATCH 03/16] more tr portability test script fixes
       [not found] <cover.1205356737.git.peff@peff.net>
  2008-03-12 21:29 ` [PATCH 01/16] tr portability fixes Jeff King
  2008-03-12 21:30 ` [PATCH 02/16] t0050: perl portability fix Jeff King
@ 2008-03-12 21:31 ` Jeff King
  2008-03-13  8:28   ` Frank Lichtenheld
  2008-03-18 22:23   ` Alex Riesen
  2008-03-12 21:32 ` [PATCH 04/16] grep portability fix: don't use "-e" or "-q" Jeff King
                   ` (12 subsequent siblings)
  15 siblings, 2 replies; 59+ messages in thread
From: Jeff King @ 2008-03-12 21:31 UTC (permalink / raw)
  To: Whit Armstrong; +Cc: Junio C Hamano, git

Dealing with NULs is not always safe with tr. On Solaris,
incoming NULs are silently deleted by both the System V and
UCB versions of tr. When converting to NULs, the System V
version works fine, but the UCB version silently ignores the
request to convert the character.

This patch changes all instances of tr using NULs to use
"perl -pe 'y///'" instead.

Signed-off-by: Jeff King <peff@peff.net>
---
This dramatically increases the amount of perl in the testsuite.
However, it is a portable subset of perl, and it isn't the first perl in
the testsuite.

 t/diff-lib.sh            |    4 ++--
 t/t0020-crlf.sh          |    2 +-
 t/t1300-repo-config.sh   |    4 ++--
 t/t3300-funny-names.sh   |    6 +++---
 t/t4020-diff-external.sh |    2 +-
 t/t4103-apply-binary.sh  |    4 ++--
 t/t4116-apply-reverse.sh |    4 ++--
 t/t4200-rerere.sh        |    2 +-
 t/t5300-pack-object.sh   |    2 +-
 test-sha1.sh             |    4 ++--
 10 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/t/diff-lib.sh b/t/diff-lib.sh
index 7dc6d7e..28b941c 100644
--- a/t/diff-lib.sh
+++ b/t/diff-lib.sh
@@ -21,8 +21,8 @@ compare_diff_raw_z () {
     # Also we do not check SHA1 hash generation in this test, which
     # is a job for t0000-basic.sh
 
-    tr '\000' '\012' <"$1" | sed -e "$sanitize_diff_raw_z" >.tmp-1
-    tr '\000' '\012' <"$2" | sed -e "$sanitize_diff_raw_z" >.tmp-2
+    perl -pe 'y/\000/\012/' <"$1" | sed -e "$sanitize_diff_raw_z" >.tmp-1
+    perl -pe 'y/\000/\012/' <"$2" | sed -e "$sanitize_diff_raw_z" >.tmp-2
     git diff .tmp-1 .tmp-2 && rm -f .tmp-1 .tmp-2
 }
 
diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh
index 90ea081..2bfeac9 100755
--- a/t/t0020-crlf.sh
+++ b/t/t0020-crlf.sh
@@ -5,7 +5,7 @@ test_description='CRLF conversion'
 . ./test-lib.sh
 
 q_to_nul () {
-	tr Q '\000'
+	perl -pe 'y/Q/\000/'
 }
 
 q_to_cr () {
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 4928a57..b36a901 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -657,12 +657,12 @@ Qsection.sub=section.val4
 Qsection.sub=section.val5Q
 EOF
 
-git config --null --list | tr '\000' 'Q' > result
+git config --null --list | perl -pe 'y/\000/Q/' > result
 echo >>result
 
 test_expect_success '--null --list' 'cmp result expect'
 
-git config --null --get-regexp 'val[0-9]' | tr '\000' 'Q' > result
+git config --null --get-regexp 'val[0-9]' | perl -pe 'y/\000/Q/' > result
 echo >>result
 
 test_expect_success '--null --get-regexp' 'cmp result expect'
diff --git a/t/t3300-funny-names.sh b/t/t3300-funny-names.sh
index 98c133d..24a00a9 100755
--- a/t/t3300-funny-names.sh
+++ b/t/t3300-funny-names.sh
@@ -54,7 +54,7 @@ echo 'just space
 no-funny
 tabs	," (dq) and spaces' >expected
 test_expect_success 'git ls-files -z with-funny' \
-	'git ls-files -z | tr \\000 \\012 >current &&
+	'git ls-files -z | perl -pe y/\\000/\\012/ >current &&
 	git diff expected current'
 
 t1=`git write-tree`
@@ -83,11 +83,11 @@ test_expect_success 'git diff-tree with-funny' \
 echo 'A
 tabs	," (dq) and spaces' >expected
 test_expect_success 'git diff-index -z with-funny' \
-	'git diff-index -z --name-status $t0 | tr \\000 \\012 >current &&
+	'git diff-index -z --name-status $t0 | perl -pe y/\\000/\\012/ >current &&
 	git diff expected current'
 
 test_expect_success 'git diff-tree -z with-funny' \
-	'git diff-tree -z --name-status $t0 $t1 | tr \\000 \\012 >current &&
+	'git diff-tree -z --name-status $t0 $t1 | perl -pe y/\\000/\\012/ >current &&
 	git diff expected current'
 
 cat > expected <<\EOF
diff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh
index 8882933..bf8f778 100755
--- a/t/t4020-diff-external.sh
+++ b/t/t4020-diff-external.sh
@@ -99,7 +99,7 @@ test_expect_success 'no diff with -diff' '
 	git diff | grep Binary
 '
 
-echo NULZbetweenZwords | tr Z '\000' > file
+echo NULZbetweenZwords | perl -pe 'y/Z/\000/' > file
 
 test_expect_success 'force diff with "diff"' '
 	echo >.gitattributes "file diff" &&
diff --git a/t/t4103-apply-binary.sh b/t/t4103-apply-binary.sh
index 7c25634..1b58233 100755
--- a/t/t4103-apply-binary.sh
+++ b/t/t4103-apply-binary.sh
@@ -24,10 +24,10 @@ git update-index --add --remove file1 file2 file4
 git-commit -m 'Initial Version' 2>/dev/null
 
 git-checkout -b binary
-tr 'x' '\000' <file1 >file3
+perl -pe 'y/x/\000/' <file1 >file3
 cat file3 >file4
 git add file2
-tr '\000' 'v' <file3 >file1
+perl -pe 'y/\000/v/' <file3 >file1
 rm -f file2
 git update-index --add --remove file1 file2 file3 file4
 git-commit -m 'Second Version'
diff --git a/t/t4116-apply-reverse.sh b/t/t4116-apply-reverse.sh
index b1d35ab..c3f4579 100755
--- a/t/t4116-apply-reverse.sh
+++ b/t/t4116-apply-reverse.sh
@@ -12,14 +12,14 @@ test_description='git apply in reverse
 test_expect_success setup '
 
 	for i in a b c d e f g h i j k l m n; do echo $i; done >file1 &&
-	tr "ijk" '\''\000\001\002'\'' <file1 >file2 &&
+	perl -pe "y/ijk/\\000\\001\\002/" <file1 >file2 &&
 
 	git add file1 file2 &&
 	git commit -m initial &&
 	git tag initial &&
 
 	for i in a b c g h i J K L m o n p q; do echo $i; done >file1 &&
-	tr "mon" '\''\000\001\002'\'' <file1 >file2 &&
+	perl -pe "y/mon/\\000\\001\\002/" <file1 >file2 &&
 
 	git commit -a -m second &&
 	git tag second &&
diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index eeff3c9..3cbfee7 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -129,7 +129,7 @@ test_expect_success 'rerere kicked in' "! grep ======= a1"
 test_expect_success 'rerere prefers first change' 'git diff a1 expect'
 
 rm $rr/postimage
-echo "$sha1	a1" | tr '\012' '\000' > .git/rr-cache/MERGE_RR
+echo "$sha1	a1" | perl -pe 'y/\012/\000/' > .git/rr-cache/MERGE_RR
 
 test_expect_success 'rerere clear' 'git rerere clear'
 
diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index cd3c149..c955fe4 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -15,7 +15,7 @@ test_expect_success \
     'rm -f .git/index*
      for i in a b c
      do
-	     dd if=/dev/zero bs=4k count=1 | tr "\\000" $i >$i &&
+	     dd if=/dev/zero bs=4k count=1 | perl -pe "y/\\000/$i/" >$i &&
 	     git update-index --add $i || return 1
      done &&
      cat c >d && echo foo >>d && git update-index --add d &&
diff --git a/test-sha1.sh b/test-sha1.sh
index bf526c8..0f0bc5d 100755
--- a/test-sha1.sh
+++ b/test-sha1.sh
@@ -10,7 +10,7 @@ do
 		{
 			test -z "$pfx" || echo "$pfx"
 			dd if=/dev/zero bs=1048576 count=$cnt 2>/dev/null |
-			tr '\000' 'g'
+			perl -pe 'y/\000/g/'
 		} | ./test-sha1 $cnt
 	`
 	if test "$expect" = "$actual"
@@ -55,7 +55,7 @@ do
 		{
 			test -z "$pfx" || echo "$pfx"
 			dd if=/dev/zero bs=1048576 count=$cnt 2>/dev/null |
-			tr '\000' 'g'
+			perl -pe 'y/\000/g/'
 		} | sha1sum |
 		sed -e 's/ .*//'
 	`
-- 
1.5.4.4.543.g30fdd.dirty

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

* [PATCH 04/16] grep portability fix: don't use "-e" or "-q"
       [not found] <cover.1205356737.git.peff@peff.net>
                   ` (2 preceding siblings ...)
  2008-03-12 21:31 ` [PATCH 03/16] more tr portability test script fixes Jeff King
@ 2008-03-12 21:32 ` Jeff King
  2008-03-12 22:10   ` Junio C Hamano
  2008-03-12 21:34 ` [PATCH 05/16] remove use of "tail -n 1" and "tail -1" Jeff King
                   ` (11 subsequent siblings)
  15 siblings, 1 reply; 59+ messages in thread
From: Jeff King @ 2008-03-12 21:32 UTC (permalink / raw)
  To: Whit Armstrong; +Cc: Junio C Hamano, git

System V versions of grep (such as Solaris /usr/bin/grep)
don't understand either of these options. git's usage of
"grep -e pattern" fell into one of two categories:

 1. equivalent to "grep pattern". -e is only useful here if
    the pattern begins with a "-", but all of the patterns
    are hardcoded and do not begin with a dash.

 2. stripping comments and blank lines with

      grep -v -e "^$" -e "^#"

    We can fortunately do this in the affirmative as

      grep '^[^#]'

Uses of "-q" can be replaced with redirection to /dev/null.
In many tests, however, "grep -q" is used as "if this string
is in the expected output, we are OK". In this case, it is
fine to just remove the "-q" entirely; it simply makes the
"verbose" mode of the test slightly more verbose.

Signed-off-by: Jeff King <peff@peff.net>
---
One might disagree with my "grep without -q actually shows useful
verbose output in tests" statement. In that case, we can >/dev/null all
of those instances.

 git-rebase--interactive.sh      |    6 +++---
 git-submodule.sh                |    6 +++---
 t/t0030-stripspace.sh           |   34 +++++++++++++++++-----------------
 t/t3404-rebase-interactive.sh   |    3 +--
 t/t3800-mktag.sh                |    2 +-
 t/t5400-send-pack.sh            |    2 +-
 t/t7502-status.sh               |    2 +-
 t/t7600-merge.sh                |    4 ++--
 t/t9400-git-cvsserver-server.sh |   26 +++++++++++++-------------
 9 files changed, 42 insertions(+), 43 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index c2bedd6..4c3280a 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -78,8 +78,8 @@ mark_action_done () {
 	sed -e 1q < "$TODO" >> "$DONE"
 	sed -e 1d < "$TODO" >> "$TODO".new
 	mv -f "$TODO".new "$TODO"
-	count=$(($(grep -ve '^$' -e '^#' < "$DONE" | wc -l)))
-	total=$(($count+$(grep -ve '^$' -e '^#' < "$TODO" | wc -l)))
+	count=$(grep -c '^[^#]' < "$DONE")
+	total=$(($count+$(grep -c '^[^#]' < "$TODO")))
 	if test "$last_count" != "$count"
 	then
 		last_count=$count
@@ -110,7 +110,7 @@ die_abort () {
 }
 
 has_action () {
-	grep -vqe '^$' -e '^#' "$1"
+	grep '^[^#]' "$1" >/dev/null
 }
 
 pick_one () {
diff --git a/git-submodule.sh b/git-submodule.sh
index 7171cb6..ceb2295 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -230,7 +230,7 @@ cmd_init()
 		shift
 	done
 
-	git ls-files --stage -- "$@" | grep -e '^160000 ' |
+	git ls-files --stage -- "$@" | grep '^160000 ' |
 	while read mode sha1 stage path
 	do
 		# Skip already registered paths
@@ -284,7 +284,7 @@ cmd_update()
 		shift
 	done
 
-	git ls-files --stage -- "$@" | grep -e '^160000 ' |
+	git ls-files --stage -- "$@" | grep '^160000 ' |
 	while read mode sha1 stage path
 	do
 		name=$(module_name "$path") || exit
@@ -367,7 +367,7 @@ cmd_status()
 		shift
 	done
 
-	git ls-files --stage -- "$@" | grep -e '^160000 ' |
+	git ls-files --stage -- "$@" | grep '^160000 ' |
 	while read mode sha1 stage path
 	do
 		name=$(module_name "$path") || exit
diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
index 818c862..3ecdd66 100755
--- a/t/t0030-stripspace.sh
+++ b/t/t0030-stripspace.sh
@@ -245,12 +245,12 @@ test_expect_success \
 
 test_expect_success \
     'text plus spaces without newline at end should not show spaces' '
-    ! (printf "$ttt$sss" | git stripspace | grep -q "  ") &&
-    ! (printf "$ttt$ttt$sss" | git stripspace | grep -q "  ") &&
-    ! (printf "$ttt$ttt$ttt$sss" | git stripspace | grep -q "  ") &&
-    ! (printf "$ttt$sss$sss" | git stripspace | grep -q "  ") &&
-    ! (printf "$ttt$ttt$sss$sss" | git stripspace | grep -q "  ") &&
-    ! (printf "$ttt$sss$sss$sss" | git stripspace | grep -q "  ")
+    ! (printf "$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
+    ! (printf "$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
+    ! (printf "$ttt$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
+    ! (printf "$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
+    ! (printf "$ttt$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
+    ! (printf "$ttt$sss$sss$sss" | git stripspace | grep "  " >/dev/null)
 '
 
 test_expect_success \
@@ -282,12 +282,12 @@ test_expect_success \
 
 test_expect_success \
     'text plus spaces at end should not show spaces' '
-    ! (echo "$ttt$sss" | git stripspace | grep -q "  ") &&
-    ! (echo "$ttt$ttt$sss" | git stripspace | grep -q "  ") &&
-    ! (echo "$ttt$ttt$ttt$sss" | git stripspace | grep -q "  ") &&
-    ! (echo "$ttt$sss$sss" | git stripspace | grep -q "  ") &&
-    ! (echo "$ttt$ttt$sss$sss" | git stripspace | grep -q "  ") &&
-    ! (echo "$ttt$sss$sss$sss" | git stripspace | grep -q "  ")
+    ! (echo "$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
+    ! (echo "$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
+    ! (echo "$ttt$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
+    ! (echo "$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
+    ! (echo "$ttt$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
+    ! (echo "$ttt$sss$sss$sss" | git stripspace | grep "  " >/dev/null)
 '
 
 test_expect_success \
@@ -341,11 +341,11 @@ test_expect_success \
 
 test_expect_success \
     'spaces without newline at end should not show spaces' '
-    ! (printf "" | git stripspace | grep -q " ") &&
-    ! (printf "$sss" | git stripspace | grep -q " ") &&
-    ! (printf "$sss$sss" | git stripspace | grep -q " ") &&
-    ! (printf "$sss$sss$sss" | git stripspace | grep -q " ") &&
-    ! (printf "$sss$sss$sss$sss" | git stripspace | grep -q " ")
+    ! (printf "" | git stripspace | grep " " >/dev/null) &&
+    ! (printf "$sss" | git stripspace | grep " " >/dev/null) &&
+    ! (printf "$sss$sss" | git stripspace | grep " " >/dev/null) &&
+    ! (printf "$sss$sss$sss" | git stripspace | grep " " >/dev/null) &&
+    ! (printf "$sss$sss$sss$sss" | git stripspace | grep " " >/dev/null)
 '
 
 test_expect_success \
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 049aa37..f098231 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -149,8 +149,7 @@ test_expect_success 'stop on conflicting pick' '
 	diff -u expect .git/.dotest-merge/patch &&
 	diff -u expect2 file1 &&
 	test 4 = $(grep -v "^#" < .git/.dotest-merge/done | wc -l) &&
-	test 0 = $(grep -ve "^#" -e "^$" < .git/.dotest-merge/git-rebase-todo |
-		wc -l)
+	test 0 = $(grep -c "^[^#]" < .git/.dotest-merge/git-rebase-todo)
 '
 
 test_expect_success 'abort' '
diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh
index f280320..2780758 100755
--- a/t/t3800-mktag.sh
+++ b/t/t3800-mktag.sh
@@ -15,7 +15,7 @@ check_verify_failure () {
 	expect="$2"
 	test_expect_success "$1" '
 		( ! git-mktag <tag.sig 2>message ) &&
-		grep -q "$expect" message
+		grep "$expect" message
 	'
 }
 
diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
index 2d0c07f..2b6b6e3 100755
--- a/t/t5400-send-pack.sh
+++ b/t/t5400-send-pack.sh
@@ -120,7 +120,7 @@ test_expect_success \
 	cd .. &&
 	git-clone parent child && cd child && git-push --all &&
 	cd ../parent &&
-	git-branch -a >branches && ! grep -q origin/master branches
+	git-branch -a >branches && ! grep origin/master branches
 '
 
 rewound_push_setup() {
diff --git a/t/t7502-status.sh b/t/t7502-status.sh
index e006074..70b802b 100755
--- a/t/t7502-status.sh
+++ b/t/t7502-status.sh
@@ -33,7 +33,7 @@ test_expect_success 'setup' '
 
 test_expect_success 'status (1)' '
 
-	grep -e "use \"git rm --cached <file>\.\.\.\" to unstage" output
+	grep "use \"git rm --cached <file>\.\.\.\" to unstage" output
 
 '
 
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 5d16628..590505b 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -371,7 +371,7 @@ test_expect_success 'override config option -n' '
 	git merge --summary c2 >diffstat.txt &&
 	verify_merge file result.1-5 msg.1-5 &&
 	verify_parents $c1 $c2 &&
-	if ! grep -e "^ file |  *2 +-$" diffstat.txt
+	if ! grep "^ file |  *2 +-$" diffstat.txt
 	then
 		echo "[OOPS] diffstat was not generated"
 	fi
@@ -386,7 +386,7 @@ test_expect_success 'override config option --summary' '
 	git merge -n c2 >diffstat.txt &&
 	verify_merge file result.1-5 msg.1-5 &&
 	verify_parents $c1 $c2 &&
-	if grep -e "^ file |  *2 +-$" diffstat.txt
+	if grep "^ file |  *2 +-$" diffstat.txt
 	then
 		echo "[OOPS] diffstat was generated"
 		false
diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index 0a20971..e82b365 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -94,7 +94,7 @@ EOF
 
 test_expect_success 'pserver authentication' \
   'cat request-anonymous | git-cvsserver pserver >log 2>&1 &&
-   tail -n1 log | grep -q "^I LOVE YOU$"'
+   tail -n1 log | grep "^I LOVE YOU$"'
 
 test_expect_success 'pserver authentication failure (non-anonymous user)' \
   'if cat request-git | git-cvsserver pserver >log 2>&1
@@ -103,11 +103,11 @@ test_expect_success 'pserver authentication failure (non-anonymous user)' \
    else
        true
    fi &&
-   tail -n1 log | grep -q "^I HATE YOU$"'
+   tail -n1 log | grep "^I HATE YOU$"'
 
 test_expect_success 'pserver authentication (login)' \
   'cat login-anonymous | git-cvsserver pserver >log 2>&1 &&
-   tail -n1 log | grep -q "^I LOVE YOU$"'
+   tail -n1 log | grep "^I LOVE YOU$"'
 
 test_expect_success 'pserver authentication failure (login/non-anonymous user)' \
   'if cat login-git | git-cvsserver pserver >log 2>&1
@@ -116,7 +116,7 @@ test_expect_success 'pserver authentication failure (login/non-anonymous user)'
    else
        true
    fi &&
-   tail -n1 log | grep -q "^I HATE YOU$"'
+   tail -n1 log | grep "^I HATE YOU$"'
 
 
 # misuse pserver authentication for testing of req_Root
@@ -146,15 +146,15 @@ test_expect_success 'req_Root failure (relative pathname)' \
    else
        true
    fi &&
-   tail log | grep -q "^error 1 Root must be an absolute pathname$"'
+   tail log | grep "^error 1 Root must be an absolute pathname$"'
 
 test_expect_success 'req_Root failure (conflicting roots)' \
   'cat request-conflict | git-cvsserver pserver >log 2>&1 &&
-   tail log | grep -q "^error 1 Conflicting roots specified$"'
+   tail log | grep "^error 1 Conflicting roots specified$"'
 
 test_expect_success 'req_Root (strict paths)' \
   'cat request-anonymous | git-cvsserver --strict-paths pserver $SERVERDIR >log 2>&1 &&
-   tail -n1 log | grep -q "^I LOVE YOU$"'
+   tail -n1 log | grep "^I LOVE YOU$"'
 
 test_expect_success 'req_Root failure (strict-paths)' '
     ! cat request-anonymous |
@@ -163,7 +163,7 @@ test_expect_success 'req_Root failure (strict-paths)' '
 
 test_expect_success 'req_Root (w/o strict-paths)' \
   'cat request-anonymous | git-cvsserver pserver $WORKDIR/ >log 2>&1 &&
-   tail -n1 log | grep -q "^I LOVE YOU$"'
+   tail -n1 log | grep "^I LOVE YOU$"'
 
 test_expect_success 'req_Root failure (w/o strict-paths)' '
     ! cat request-anonymous |
@@ -181,7 +181,7 @@ EOF
 
 test_expect_success 'req_Root (base-path)' \
   'cat request-base | git-cvsserver --strict-paths --base-path $WORKDIR/ pserver $SERVERDIR >log 2>&1 &&
-   tail -n1 log | grep -q "^I LOVE YOU$"'
+   tail -n1 log | grep "^I LOVE YOU$"'
 
 test_expect_success 'req_Root failure (base-path)' '
     ! cat request-anonymous |
@@ -192,14 +192,14 @@ GIT_DIR="$SERVERDIR" git config --bool gitcvs.enabled false || exit 1
 
 test_expect_success 'req_Root (export-all)' \
   'cat request-anonymous | git-cvsserver --export-all pserver $WORKDIR >log 2>&1 &&
-   tail -n1 log | grep -q "^I LOVE YOU$"'
+   tail -n1 log | grep "^I LOVE YOU$"'
 
 test_expect_success 'req_Root failure (export-all w/o whitelist)' \
   '! (cat request-anonymous | git-cvsserver --export-all pserver >log 2>&1 || false)'
 
 test_expect_success 'req_Root (everything together)' \
   'cat request-base | git-cvsserver --export-all --strict-paths --base-path $WORKDIR/ pserver $SERVERDIR >log 2>&1 &&
-   tail -n1 log | grep -q "^I LOVE YOU$"'
+   tail -n1 log | grep "^I LOVE YOU$"'
 
 GIT_DIR="$SERVERDIR" git config --bool gitcvs.enabled true || exit 1
 
@@ -216,7 +216,7 @@ test_expect_success 'gitcvs.enabled = false' \
    else
      true
    fi &&
-   cat cvs.log | grep -q "GITCVS emulation disabled" &&
+   grep "GITCVS emulation disabled" cvs.log &&
    test ! -d cvswork2'
 
 rm -fr cvswork2
@@ -237,7 +237,7 @@ test_expect_success 'gitcvs.ext.enabled = false' \
    else
      true
    fi &&
-   cat cvs.log | grep -q "GITCVS emulation disabled" &&
+   grep "GITCVS emulation disabled" cvs.log &&
    test ! -d cvswork2'
 
 rm -fr cvswork2
-- 
1.5.4.4.543.g30fdd.dirty

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

* [PATCH 05/16] remove use of "tail -n 1" and "tail -1"
       [not found] <cover.1205356737.git.peff@peff.net>
                   ` (3 preceding siblings ...)
  2008-03-12 21:32 ` [PATCH 04/16] grep portability fix: don't use "-e" or "-q" Jeff King
@ 2008-03-12 21:34 ` Jeff King
  2008-03-12 21:36 ` [PATCH 06/16] add test_cmp function for test scripts Jeff King
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 59+ messages in thread
From: Jeff King @ 2008-03-12 21:34 UTC (permalink / raw)
  To: Whit Armstrong; +Cc: Junio C Hamano, git

The "-n" syntax is not supported by System V versions of
tail (which prefer "tail -1"). Unfortunately "tail -1" is
not actually POSIX.  We had some of both forms in our
scripts.

Since neither form works everywhere, this patch replaces
both with the equivalent sed invocation:

  sed -ne '$p'

Signed-off-by: Jeff King <peff@peff.net>
---
Actually, "tail -1" _does_ seem to be accepted everywhere, even though
it isn't POSIX. I remember the GNU utils complaining about it (but maybe
just with POSIXLY_CORRECT?) a while back, but that seems to have been
reverted. But certainly this sed invocation should work everywhere.

 git-am.sh                       |    2 +-
 git-rebase--interactive.sh      |    2 +-
 t/t3404-rebase-interactive.sh   |    4 ++--
 t/t5302-pack-index.sh           |    4 ++--
 t/t6030-bisect-porcelain.sh     |    6 +++---
 t/t7600-merge.sh                |    2 +-
 t/t9400-git-cvsserver-server.sh |   20 ++++++++++----------
 7 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index 1f6b5e0..ac5c388 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -357,7 +357,7 @@ do
 		LAST_SIGNED_OFF_BY=`
 		    sed -ne '/^Signed-off-by: /p' \
 		    "$dotest/msg-clean" |
-		    tail -n 1
+		    sed -ne '$p'
 		`
 		ADD_SIGNOFF=`
 		    test "$LAST_SIGNED_OFF_BY" = "$SIGNOFF" || {
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 4c3280a..8aa7371 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -218,7 +218,7 @@ nth_string () {
 make_squash_message () {
 	if test -f "$SQUASH_MSG"; then
 		COUNT=$(($(sed -n "s/^# This is [^0-9]*\([1-9][0-9]*\).*/\1/p" \
-			< "$SQUASH_MSG" | tail -n 1)+1))
+			< "$SQUASH_MSG" | sed -ne '$p')+1))
 		echo "# This is a combination of $COUNT commits."
 		sed -e 1d -e '2,/^./{
 			/^$/d
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index f098231..9c0acc5 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -122,8 +122,8 @@ test_expect_success 'reflog for the branch shows state before rebase' '
 
 test_expect_success 'exchange two commits' '
 	FAKE_LINES="2 1" git rebase -i HEAD~2 &&
-	test H = $(git cat-file commit HEAD^ | tail -n 1) &&
-	test G = $(git cat-file commit HEAD | tail -n 1)
+	test H = $(git cat-file commit HEAD^ | sed -ne \$p) &&
+	test G = $(git cat-file commit HEAD | sed -ne \$p)
 '
 
 cat > expect << EOF
diff --git a/t/t5302-pack-index.sh b/t/t5302-pack-index.sh
index 67b9a7b..b88b5bb 100755
--- a/t/t5302-pack-index.sh
+++ b/t/t5302-pack-index.sh
@@ -103,7 +103,7 @@ test_expect_success \
 test_expect_success \
     '[index v1] 2) create a stealth corruption in a delta base reference' \
     '# this test assumes a delta smaller than 16 bytes at the end of the pack
-     git show-index <1.idx | sort -n | tail -n 1 | (
+     git show-index <1.idx | sort -n | sed -ne \$p | (
        read delta_offs delta_sha1 &&
        git cat-file blob "$delta_sha1" > blob_1 &&
        chmod +w ".git/objects/pack/pack-${pack1}.pack" &&
@@ -141,7 +141,7 @@ test_expect_success \
 test_expect_success \
     '[index v2] 2) create a stealth corruption in a delta base reference' \
     '# this test assumes a delta smaller than 16 bytes at the end of the pack
-     git show-index <1.idx | sort -n | tail -n 1 | (
+     git show-index <1.idx | sort -n | sed -ne \$p | (
        read delta_offs delta_sha1 delta_crc &&
        git cat-file blob "$delta_sha1" > blob_3 &&
        chmod +w ".git/objects/pack/pack-${pack1}.pack" &&
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 4908e87..f471c15 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -219,7 +219,7 @@ test_expect_success 'bisect run & skip: cannot tell between 2' '
 	add_line_into_file "6: Yet a line." hello &&
 	HASH6=$(git rev-parse --verify HEAD) &&
 	echo "#"\!"/bin/sh" > test_script.sh &&
-	echo "tail -1 hello | grep Ciao > /dev/null && exit 125" >> test_script.sh &&
+	echo "sed -ne \\\$p hello | grep Ciao > /dev/null && exit 125" >> test_script.sh &&
 	echo "grep line hello > /dev/null" >> test_script.sh &&
 	echo "test \$? -ne 0" >> test_script.sh &&
 	chmod +x test_script.sh &&
@@ -244,8 +244,8 @@ test_expect_success 'bisect run & skip: find first bad' '
 	add_line_into_file "7: Should be the last line." hello &&
 	HASH7=$(git rev-parse --verify HEAD) &&
 	echo "#"\!"/bin/sh" > test_script.sh &&
-	echo "tail -1 hello | grep Ciao > /dev/null && exit 125" >> test_script.sh &&
-	echo "tail -1 hello | grep day > /dev/null && exit 125" >> test_script.sh &&
+	echo "sed -ne \\\$p hello | grep Ciao > /dev/null && exit 125" >> test_script.sh &&
+	echo "sed -ne \\\$p hello | grep day > /dev/null && exit 125" >> test_script.sh &&
 	echo "grep Yet hello > /dev/null" >> test_script.sh &&
 	echo "test \$? -ne 0" >> test_script.sh &&
 	chmod +x test_script.sh &&
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 590505b..219411f 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -165,7 +165,7 @@ verify_mergeheads() {
 	fi &&
 	while test $# -gt 0
 	do
-		head=$(head -n $i .git/MERGE_HEAD | tail -n 1)
+		head=$(head -n $i .git/MERGE_HEAD | sed -ne \$p)
 		if test "$1" != "$head"
 		then
 			echo "[OOPS] MERGE_HEAD $i != $1"
diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index e82b365..b91b151 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -54,7 +54,7 @@ test_expect_success 'setup' '
 test_expect_success 'basic checkout' \
   'GIT_CONFIG="$git_config" cvs -Q co -d cvswork master &&
    test "$(echo $(grep -v ^D cvswork/CVS/Entries|cut -d/ -f2,3,5 | head -n 1))" = "empty/1.1/"
-   test "$(echo $(grep -v ^D cvswork/CVS/Entries|cut -d/ -f2,3,5 | tail -n 1))" = "secondrootfile/1.1/"'
+   test "$(echo $(grep -v ^D cvswork/CVS/Entries|cut -d/ -f2,3,5 | sed -ne \$p))" = "secondrootfile/1.1/"'
 
 #------------------------
 # PSERVER AUTHENTICATION
@@ -94,7 +94,7 @@ EOF
 
 test_expect_success 'pserver authentication' \
   'cat request-anonymous | git-cvsserver pserver >log 2>&1 &&
-   tail -n1 log | grep "^I LOVE YOU$"'
+   sed -ne \$p log | grep "^I LOVE YOU$"'
 
 test_expect_success 'pserver authentication failure (non-anonymous user)' \
   'if cat request-git | git-cvsserver pserver >log 2>&1
@@ -103,11 +103,11 @@ test_expect_success 'pserver authentication failure (non-anonymous user)' \
    else
        true
    fi &&
-   tail -n1 log | grep "^I HATE YOU$"'
+   sed -ne \$p log | grep "^I HATE YOU$"'
 
 test_expect_success 'pserver authentication (login)' \
   'cat login-anonymous | git-cvsserver pserver >log 2>&1 &&
-   tail -n1 log | grep "^I LOVE YOU$"'
+   sed -ne \$p log | grep "^I LOVE YOU$"'
 
 test_expect_success 'pserver authentication failure (login/non-anonymous user)' \
   'if cat login-git | git-cvsserver pserver >log 2>&1
@@ -116,7 +116,7 @@ test_expect_success 'pserver authentication failure (login/non-anonymous user)'
    else
        true
    fi &&
-   tail -n1 log | grep "^I HATE YOU$"'
+   sed -ne \$p log | grep "^I HATE YOU$"'
 
 
 # misuse pserver authentication for testing of req_Root
@@ -154,7 +154,7 @@ test_expect_success 'req_Root failure (conflicting roots)' \
 
 test_expect_success 'req_Root (strict paths)' \
   'cat request-anonymous | git-cvsserver --strict-paths pserver $SERVERDIR >log 2>&1 &&
-   tail -n1 log | grep "^I LOVE YOU$"'
+   sed -ne \$p log | grep "^I LOVE YOU$"'
 
 test_expect_success 'req_Root failure (strict-paths)' '
     ! cat request-anonymous |
@@ -163,7 +163,7 @@ test_expect_success 'req_Root failure (strict-paths)' '
 
 test_expect_success 'req_Root (w/o strict-paths)' \
   'cat request-anonymous | git-cvsserver pserver $WORKDIR/ >log 2>&1 &&
-   tail -n1 log | grep "^I LOVE YOU$"'
+   sed -ne \$p log | grep "^I LOVE YOU$"'
 
 test_expect_success 'req_Root failure (w/o strict-paths)' '
     ! cat request-anonymous |
@@ -181,7 +181,7 @@ EOF
 
 test_expect_success 'req_Root (base-path)' \
   'cat request-base | git-cvsserver --strict-paths --base-path $WORKDIR/ pserver $SERVERDIR >log 2>&1 &&
-   tail -n1 log | grep "^I LOVE YOU$"'
+   sed -ne \$p log | grep "^I LOVE YOU$"'
 
 test_expect_success 'req_Root failure (base-path)' '
     ! cat request-anonymous |
@@ -192,14 +192,14 @@ GIT_DIR="$SERVERDIR" git config --bool gitcvs.enabled false || exit 1
 
 test_expect_success 'req_Root (export-all)' \
   'cat request-anonymous | git-cvsserver --export-all pserver $WORKDIR >log 2>&1 &&
-   tail -n1 log | grep "^I LOVE YOU$"'
+   sed -ne \$p log | grep "^I LOVE YOU$"'
 
 test_expect_success 'req_Root failure (export-all w/o whitelist)' \
   '! (cat request-anonymous | git-cvsserver --export-all pserver >log 2>&1 || false)'
 
 test_expect_success 'req_Root (everything together)' \
   'cat request-base | git-cvsserver --export-all --strict-paths --base-path $WORKDIR/ pserver $SERVERDIR >log 2>&1 &&
-   tail -n1 log | grep "^I LOVE YOU$"'
+   sed -ne \$p log | grep "^I LOVE YOU$"'
 
 GIT_DIR="$SERVERDIR" git config --bool gitcvs.enabled true || exit 1
 
-- 
1.5.4.4.543.g30fdd.dirty

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

* [PATCH 06/16] add test_cmp function for test scripts
       [not found] <cover.1205356737.git.peff@peff.net>
                   ` (4 preceding siblings ...)
  2008-03-12 21:34 ` [PATCH 05/16] remove use of "tail -n 1" and "tail -1" Jeff King
@ 2008-03-12 21:36 ` Jeff King
  2008-03-12 22:12   ` Junio C Hamano
  2008-03-12 21:37 ` [PATCH 07/16] t4020: don't use grep -a Jeff King
                   ` (9 subsequent siblings)
  15 siblings, 1 reply; 59+ messages in thread
From: Jeff King @ 2008-03-12 21:36 UTC (permalink / raw)
  To: Whit Armstrong; +Cc: Junio C Hamano, git

Many scripts compare actual and expected output using
"diff -u". This is nicer than "cmp" because the output shows
how the two differ. However, not all versions of diff
understand -u, leading to unnecessary test failure.

This adds a test_cmp function to the test scripts and
switches all "diff -u" invocations to use it. The function
uses the contents of "$GIT_TEST_CMP" to compare its
arguments; the default is "diff -u".

On systems with a less-capable diff, you can do:

  GIT_TEST_CMP=cmp make test

Signed-off-by: Jeff King <peff@peff.net>
---
The replacements were done mechanically with:

  perl -pi -e 's/(?<!git.)diff -u/test_cmp/' t/*.sh

Perhaps this should be a command-line option instead of an environment
variable? Maybe it should be auto-detected or set up in the Makefile?

 t/t0003-attributes.sh              |    2 +-
 t/t0022-crlf-rename.sh             |    2 +-
 t/t1005-read-tree-reset.sh         |    2 +-
 t/t2200-add-update.sh              |    2 +-
 t/t3001-ls-files-others-exclude.sh |    2 +-
 t/t3050-subprojects-fetch.sh       |    4 ++--
 t/t3060-ls-files-with-tree.sh      |    2 +-
 t/t3201-branch-contains.sh         |    6 +++---
 t/t3404-rebase-interactive.sh      |    4 ++--
 t/t3405-rebase-malformed.sh        |    4 ++--
 t/t3406-rebase-message.sh          |    2 +-
 t/t3701-add-interactive.sh         |    4 ++--
 t/t3902-quoted.sh                  |   16 ++++++++--------
 t/t3903-stash.sh                   |    2 +-
 t/t4023-diff-rename-typechange.sh  |    6 +++---
 t/t4024-diff-optimize-common.sh    |    2 +-
 t/t4025-hunk-header.sh             |    2 +-
 t/t4027-diff-submodule.sh          |    6 +++---
 t/t4105-apply-fuzz.sh              |    2 +-
 t/t4125-apply-ws-fuzz.sh           |    8 ++++----
 t/t4150-am-subdir.sh               |   10 +++++-----
 t/t4201-shortlog.sh                |    2 +-
 t/t5505-remote.sh                  |    6 +++---
 t/t5510-fetch.sh                   |    2 +-
 t/t5512-ls-remote.sh               |    8 ++++----
 t/t6004-rev-list-path-optim.sh     |    2 +-
 t/t6009-rev-list-parent.sh         |    2 +-
 t/t6027-merge-binary.sh            |    4 ++--
 t/t6029-merge-subtree.sh           |    2 +-
 t/t7010-setup.sh                   |   18 +++++++++---------
 t/t7201-co.sh                      |   18 +++++++++---------
 t/t7501-commit.sh                  |   14 +++++++-------
 t/t7502-commit.sh                  |   14 +++++++-------
 t/t7502-status.sh                  |    2 +-
 t/t7600-merge.sh                   |    2 +-
 t/t8003-blame.sh                   |    4 ++--
 t/t9001-send-email.sh              |    2 +-
 t/t9116-git-svn-log.sh             |   24 ++++++++++++------------
 t/t9200-git-cvsexportcommit.sh     |   14 +++++++-------
 t/test-lib.sh                      |   18 ++++++++++++++++++
 40 files changed, 133 insertions(+), 115 deletions(-)

diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index 47f08a4..3faf135 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -11,7 +11,7 @@ attr_check () {
 
 	git check-attr test -- "$path" >actual &&
 	echo "$path: test: $2" >expect &&
-	diff -u expect actual
+	test_cmp expect actual
 
 }
 
diff --git a/t/t0022-crlf-rename.sh b/t/t0022-crlf-rename.sh
index 430a1d1..7d1ce2d 100755
--- a/t/t0022-crlf-rename.sh
+++ b/t/t0022-crlf-rename.sh
@@ -26,7 +26,7 @@ test_expect_success 'diff -M' '
 	git diff-tree -M -r --name-status HEAD^ HEAD |
 	sed -e "s/R[0-9]*/RNUM/" >actual &&
 	echo "RNUM	sample	elpmas" >expect &&
-	diff -u expect actual
+	test_cmp expect actual
 
 '
 
diff --git a/t/t1005-read-tree-reset.sh b/t/t1005-read-tree-reset.sh
index 8c45564..b0d31f5 100755
--- a/t/t1005-read-tree-reset.sh
+++ b/t/t1005-read-tree-reset.sh
@@ -24,7 +24,7 @@ test_expect_success 'setup' '
 test_expect_success 'reset should work' '
   git read-tree -u --reset HEAD^ &&
   git ls-files >actual &&
-  diff -u expect actual
+  test_cmp expect actual
 '
 
 test_done
diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh
index 24f892f..b664341 100755
--- a/t/t2200-add-update.sh
+++ b/t/t2200-add-update.sh
@@ -62,7 +62,7 @@ test_expect_success 'cache tree has not been corrupted' '
 	sed -e "s/ 0	/	/" >expect &&
 	git ls-tree -r $(git write-tree) |
 	sed -e "s/ blob / /" >current &&
-	diff -u expect current
+	test_cmp expect current
 
 '
 
diff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh
index b4297ba..55f057c 100755
--- a/t/t3001-ls-files-others-exclude.sh
+++ b/t/t3001-ls-files-others-exclude.sh
@@ -97,7 +97,7 @@ cat > expect << EOF
 EOF
 
 test_expect_success 'git-status honours core.excludesfile' \
-	'diff -u expect output'
+	'test_cmp expect output'
 
 test_expect_success 'trailing slash in exclude allows directory match(1)' '
 
diff --git a/t/t3050-subprojects-fetch.sh b/t/t3050-subprojects-fetch.sh
index 34f26a8..2b21b10 100755
--- a/t/t3050-subprojects-fetch.sh
+++ b/t/t3050-subprojects-fetch.sh
@@ -26,7 +26,7 @@ test_expect_success clone '
 		cd cloned &&
 		(git rev-parse HEAD; git ls-files -s) >../actual
 	) &&
-	diff -u expected actual
+	test_cmp expected actual
 '
 
 test_expect_success advance '
@@ -46,7 +46,7 @@ test_expect_success fetch '
 		git pull &&
 		(git rev-parse HEAD; git ls-files -s) >../actual
 	) &&
-	diff -u expected actual
+	test_cmp expected actual
 '
 
 test_done
diff --git a/t/t3060-ls-files-with-tree.sh b/t/t3060-ls-files-with-tree.sh
index 68eb266..3ce501b 100755
--- a/t/t3060-ls-files-with-tree.sh
+++ b/t/t3060-ls-files-with-tree.sh
@@ -66,6 +66,6 @@ test_expect_success 'git -ls-files --with-tree should succeed from subdir' '
 cd ..
 test_expect_success \
     'git -ls-files --with-tree should add entries from named tree.' \
-    'diff -u expected output'
+    'test_cmp expected output'
 
 test_done
diff --git a/t/t3201-branch-contains.sh b/t/t3201-branch-contains.sh
index 9ef593f..b4cf628 100755
--- a/t/t3201-branch-contains.sh
+++ b/t/t3201-branch-contains.sh
@@ -31,7 +31,7 @@ test_expect_success 'branch --contains=master' '
 	{
 		echo "  master" && echo "* side"
 	} >expect &&
-	diff -u expect actual
+	test_cmp expect actual
 
 '
 
@@ -41,7 +41,7 @@ test_expect_success 'branch --contains master' '
 	{
 		echo "  master" && echo "* side"
 	} >expect &&
-	diff -u expect actual
+	test_cmp expect actual
 
 '
 
@@ -51,7 +51,7 @@ test_expect_success 'branch --contains=side' '
 	{
 		echo "* side"
 	} >expect &&
-	diff -u expect actual
+	test_cmp expect actual
 
 '
 
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 9c0acc5..9cf873f 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -146,8 +146,8 @@ EOF
 test_expect_success 'stop on conflicting pick' '
 	git tag new-branch1 &&
 	! git rebase -i master &&
-	diff -u expect .git/.dotest-merge/patch &&
-	diff -u expect2 file1 &&
+	test_cmp expect .git/.dotest-merge/patch &&
+	test_cmp expect2 file1 &&
 	test 4 = $(grep -v "^#" < .git/.dotest-merge/done | wc -l) &&
 	test 0 = $(grep -c "^[^#]" < .git/.dotest-merge/git-rebase-todo)
 '
diff --git a/t/t3405-rebase-malformed.sh b/t/t3405-rebase-malformed.sh
index e4e2e64..e5ad67c 100755
--- a/t/t3405-rebase-malformed.sh
+++ b/t/t3405-rebase-malformed.sh
@@ -41,8 +41,8 @@ test_expect_success rebase '
 	git rebase master side &&
 	git cat-file commit HEAD | sed -e "1,/^\$/d" >F1 &&
 
-	diff -u F0 F1 &&
-	diff -u F F0
+	test_cmp F0 F1 &&
+	test_cmp F F0
 '
 
 test_done
diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
index 332b2b2..5391080 100755
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh
@@ -37,7 +37,7 @@ test_expect_success 'rebase -m' '
 	git rebase -m master >report &&
 	sed -n -e "/^Already applied: /p" \
 		-e "/^Committed: /p" report >actual &&
-	diff -u expect actual
+	test_cmp expect actual
 
 '
 
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index c8dc1ac..77c90f6 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -24,7 +24,7 @@ EOF
 test_expect_success 'diff works (initial)' '
 	(echo d; echo 1) | git add -i >output &&
 	sed -ne "/new file/,/content/p" <output >diff &&
-	diff -u expected diff
+	test_cmp expected diff
 '
 test_expect_success 'revert works (initial)' '
 	git add file &&
@@ -57,7 +57,7 @@ EOF
 test_expect_success 'diff works (commit)' '
 	(echo d; echo 1) | git add -i >output &&
 	sed -ne "/^index/,/content/p" <output >diff &&
-	diff -u expected diff
+	test_cmp expected diff
 '
 test_expect_success 'revert works (commit)' '
 	git add file &&
diff --git a/t/t3902-quoted.sh b/t/t3902-quoted.sh
index 73da45f..fe4fb51 100755
--- a/t/t3902-quoted.sh
+++ b/t/t3902-quoted.sh
@@ -78,28 +78,28 @@ EOF
 
 test_expect_success 'check fully quoted output from ls-files' '
 
-	git ls-files >current && diff -u expect.quoted current
+	git ls-files >current && test_cmp expect.quoted current
 
 '
 
 test_expect_success 'check fully quoted output from diff-files' '
 
 	git diff --name-only >current &&
-	diff -u expect.quoted current
+	test_cmp expect.quoted current
 
 '
 
 test_expect_success 'check fully quoted output from diff-index' '
 
 	git diff --name-only HEAD >current &&
-	diff -u expect.quoted current
+	test_cmp expect.quoted current
 
 '
 
 test_expect_success 'check fully quoted output from diff-tree' '
 
 	git diff --name-only HEAD^ HEAD >current &&
-	diff -u expect.quoted current
+	test_cmp expect.quoted current
 
 '
 
@@ -111,28 +111,28 @@ test_expect_success 'setting core.quotepath' '
 
 test_expect_success 'check fully quoted output from ls-files' '
 
-	git ls-files >current && diff -u expect.raw current
+	git ls-files >current && test_cmp expect.raw current
 
 '
 
 test_expect_success 'check fully quoted output from diff-files' '
 
 	git diff --name-only >current &&
-	diff -u expect.raw current
+	test_cmp expect.raw current
 
 '
 
 test_expect_success 'check fully quoted output from diff-index' '
 
 	git diff --name-only HEAD >current &&
-	diff -u expect.raw current
+	test_cmp expect.raw current
 
 '
 
 test_expect_success 'check fully quoted output from diff-tree' '
 
 	git diff --name-only HEAD^ HEAD >current &&
-	diff -u expect.raw current
+	test_cmp expect.raw current
 
 '
 
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index aa282e1..2d3ee3b 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -34,7 +34,7 @@ EOF
 test_expect_success 'parents of stash' '
 	test $(git rev-parse stash^) = $(git rev-parse HEAD) &&
 	git diff stash^2..stash > output &&
-	diff -u output expect
+	test_cmp output expect
 '
 
 test_expect_success 'apply needs clean working directory' '
diff --git a/t/t4023-diff-rename-typechange.sh b/t/t4023-diff-rename-typechange.sh
index 255604e..4dbfc6e 100755
--- a/t/t4023-diff-rename-typechange.sh
+++ b/t/t4023-diff-rename-typechange.sh
@@ -57,7 +57,7 @@ test_expect_success 'cross renames to be detected for regular files' '
 		echo "R100	foo	bar"
 		echo "R100	bar	foo"
 	} | sort >expect &&
-	diff -u expect actual
+	test_cmp expect actual
 
 '
 
@@ -68,7 +68,7 @@ test_expect_success 'cross renames to be detected for typechange' '
 		echo "R100	foo	bar"
 		echo "R100	bar	foo"
 	} | sort >expect &&
-	diff -u expect actual
+	test_cmp expect actual
 
 '
 
@@ -79,7 +79,7 @@ test_expect_success 'moves and renames' '
 		echo "R100	foo	bar"
 		echo "T100	foo"
 	} | sort >expect &&
-	diff -u expect actual
+	test_cmp expect actual
 
 '
 
diff --git a/t/t4024-diff-optimize-common.sh b/t/t4024-diff-optimize-common.sh
index 3c66102..c4d733f 100755
--- a/t/t4024-diff-optimize-common.sh
+++ b/t/t4024-diff-optimize-common.sh
@@ -150,7 +150,7 @@ test_expect_success 'diff -U0' '
 	do
 		git diff -U0 file-?$n
 	done | zc >actual &&
-	diff -u expect actual
+	test_cmp expect actual
 
 '
 
diff --git a/t/t4025-hunk-header.sh b/t/t4025-hunk-header.sh
index 9ba06b7..7a3dbc1 100755
--- a/t/t4025-hunk-header.sh
+++ b/t/t4025-hunk-header.sh
@@ -37,7 +37,7 @@ test_expect_success 'hunk header truncation with an overly long line' '
 		echo " A $N$N$N$N$N$N$N$N$N2"
 		echo " L  $N$N$N$N$N$N$N$N$N1"
 	) >expected &&
-	diff -u actual expected
+	test_cmp actual expected
 
 '
 
diff --git a/t/t4027-diff-submodule.sh b/t/t4027-diff-submodule.sh
index 3d2d081..1fd3fb7 100755
--- a/t/t4027-diff-submodule.sh
+++ b/t/t4027-diff-submodule.sh
@@ -37,17 +37,17 @@ test_expect_success setup '
 
 test_expect_success 'git diff --raw HEAD' '
 	git diff --raw --abbrev=40 HEAD >actual &&
-	diff -u expect actual
+	test_cmp expect actual
 '
 
 test_expect_success 'git diff-index --raw HEAD' '
 	git diff-index --raw HEAD >actual.index &&
-	diff -u expect actual.index
+	test_cmp expect actual.index
 '
 
 test_expect_success 'git diff-files --raw' '
 	git diff-files --raw >actual.files &&
-	diff -u expect actual.files
+	test_cmp expect actual.files
 '
 
 test_done
diff --git a/t/t4105-apply-fuzz.sh b/t/t4105-apply-fuzz.sh
index 0e8d25f..3266e39 100755
--- a/t/t4105-apply-fuzz.sh
+++ b/t/t4105-apply-fuzz.sh
@@ -9,7 +9,7 @@ dotest () {
 	test_expect_success "$name" "
 		git checkout-index -f -q -u file &&
 		git apply $* &&
-		diff -u expect file
+		test_cmp expect file
 	"
 }
 
diff --git a/t/t4125-apply-ws-fuzz.sh b/t/t4125-apply-ws-fuzz.sh
index d6f15be..3b471b6 100755
--- a/t/t4125-apply-ws-fuzz.sh
+++ b/t/t4125-apply-ws-fuzz.sh
@@ -56,7 +56,7 @@ test_expect_success nofix '
 	git apply --whitespace=nowarn patch-1 &&
 
 	# The result should obviously match.
-	diff -u file-1 file
+	test_cmp file-1 file
 '
 
 test_expect_success 'withfix (forward)' '
@@ -70,7 +70,7 @@ test_expect_success 'withfix (forward)' '
 	git apply --whitespace=fix patch-0 &&
 	git apply --whitespace=fix patch-1 &&
 
-	diff -u file-fixed file
+	test_cmp file-fixed file
 '
 
 test_expect_success 'withfix (backward)' '
@@ -91,12 +91,12 @@ test_expect_success 'withfix (backward)' '
 
 	sed -e /h/d file-fixed >fixed-head &&
 	sed -e /h/d file >file-head &&
-	diff -u fixed-head file-head &&
+	test_cmp fixed-head file-head &&
 
 	sed -n -e /h/p file-fixed >fixed-tail &&
 	sed -n -e /h/p file >file-tail &&
 
-	! diff -u fixed-tail file-tail
+	! test_cmp fixed-tail file-tail
 
 '
 
diff --git a/t/t4150-am-subdir.sh b/t/t4150-am-subdir.sh
index 929d2cb..52069b4 100755
--- a/t/t4150-am-subdir.sh
+++ b/t/t4150-am-subdir.sh
@@ -22,14 +22,14 @@ test_expect_success 'am regularly from stdin' '
 	git checkout initial &&
 	git am <patchfile &&
 	git diff master >actual &&
-	diff -u expect actual
+	test_cmp expect actual
 '
 
 test_expect_success 'am regularly from file' '
 	git checkout initial &&
 	git am patchfile &&
 	git diff master >actual &&
-	diff -u expect actual
+	test_cmp expect actual
 '
 
 test_expect_success 'am regularly from stdin in subdirectory' '
@@ -41,7 +41,7 @@ test_expect_success 'am regularly from stdin in subdirectory' '
 		git am <../patchfile
 	) &&
 	git diff master>actual &&
-	diff -u expect actual
+	test_cmp expect actual
 '
 
 test_expect_success 'am regularly from file in subdirectory' '
@@ -53,7 +53,7 @@ test_expect_success 'am regularly from file in subdirectory' '
 		git am ../patchfile
 	) &&
 	git diff master >actual &&
-	diff -u expect actual
+	test_cmp expect actual
 '
 
 test_expect_success 'am regularly from file in subdirectory with full path' '
@@ -66,7 +66,7 @@ test_expect_success 'am regularly from file in subdirectory with full path' '
 		git am "$P/patchfile"
 	) &&
 	git diff master >actual &&
-	diff -u expect actual
+	test_cmp expect actual
 '
 
 test_done
diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh
index 6d12efb..91ea696 100755
--- a/t/t4201-shortlog.sh
+++ b/t/t4201-shortlog.sh
@@ -45,6 +45,6 @@ A U Thor (5):
 
 EOF
 
-test_expect_success 'shortlog wrapping' 'diff -u expect out'
+test_expect_success 'shortlog wrapping' 'test_cmp expect out'
 
 test_done
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 2822a65..ecfc999 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -24,7 +24,7 @@ setup_repository () {
 tokens_match () {
 	echo "$1" | tr ' ' '\012' | sort | sed -e '/^$/d' >expect &&
 	echo "$2" | tr ' ' '\012' | sort | sed -e '/^$/d' >actual &&
-	diff -u expect actual
+	test_cmp expect actual
 }
 
 check_remote_track () {
@@ -73,7 +73,7 @@ test_expect_success 'add another remote' '
 	sed -e "/^refs\/remotes\/origin\//d" \
 	    -e "/^refs\/remotes\/second\//d" >actual &&
 	>expect &&
-	diff -u expect actual
+	test_cmp expect actual
 )
 '
 
@@ -93,7 +93,7 @@ test_expect_success 'remove remote' '
 	git for-each-ref "--format=%(refname)" refs/remotes |
 	sed -e "/^refs\/remotes\/origin\//d" >actual &&
 	>expect &&
-	diff -u expect actual
+	test_cmp expect actual
 )
 '
 
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 9b948c1..6946557 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -249,7 +249,7 @@ test_expect_success 'bundle should record HEAD correctly' '
 	do
 		echo "$(git rev-parse --verify $h) $h"
 	done >expect &&
-	diff -u expect actual
+	test_cmp expect actual
 
 '
 
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index 6ec5f7c..c0dc949 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -24,28 +24,28 @@ test_expect_success setup '
 test_expect_success 'ls-remote --tags .git' '
 
 	git ls-remote --tags .git >actual &&
-	diff -u expected.tag actual
+	test_cmp expected.tag actual
 
 '
 
 test_expect_success 'ls-remote .git' '
 
 	git ls-remote .git >actual &&
-	diff -u expected.all actual
+	test_cmp expected.all actual
 
 '
 
 test_expect_success 'ls-remote --tags self' '
 
 	git ls-remote --tags self >actual &&
-	diff -u expected.tag actual
+	test_cmp expected.tag actual
 
 '
 
 test_expect_success 'ls-remote self' '
 
 	git ls-remote self >actual &&
-	diff -u expected.all actual
+	test_cmp expected.all actual
 
 '
 
diff --git a/t/t6004-rev-list-path-optim.sh b/t/t6004-rev-list-path-optim.sh
index 80d7198..5dabf1c 100755
--- a/t/t6004-rev-list-path-optim.sh
+++ b/t/t6004-rev-list-path-optim.sh
@@ -45,7 +45,7 @@ test_expect_success 'further setup' '
 test_expect_success 'path optimization 2' '
 	( echo "$side"; echo "$initial" ) >expected &&
 	git rev-list HEAD -- a >actual &&
-	diff -u expected actual
+	test_cmp expected actual
 '
 
 test_done
diff --git a/t/t6009-rev-list-parent.sh b/t/t6009-rev-list-parent.sh
index be3d238..f248a32 100755
--- a/t/t6009-rev-list-parent.sh
+++ b/t/t6009-rev-list-parent.sh
@@ -31,7 +31,7 @@ test_expect_failure 'one is ancestor of others and should not be shown' '
 
 	git rev-list one --not four >result &&
 	>expect &&
-	diff -u expect result
+	test_cmp expect result
 
 '
 
diff --git a/t/t6027-merge-binary.sh b/t/t6027-merge-binary.sh
index a7358f7..92ca1f0 100755
--- a/t/t6027-merge-binary.sh
+++ b/t/t6027-merge-binary.sh
@@ -45,7 +45,7 @@ test_expect_success resolve '
 		false
 	else
 		git ls-files -s >current
-		diff -u current expect
+		test_cmp current expect
 	fi
 '
 
@@ -60,7 +60,7 @@ test_expect_success recursive '
 		false
 	else
 		git ls-files -s >current
-		diff -u current expect
+		test_cmp current expect
 	fi
 '
 
diff --git a/t/t6029-merge-subtree.sh b/t/t6029-merge-subtree.sh
index 35d66e8..43f5459 100755
--- a/t/t6029-merge-subtree.sh
+++ b/t/t6029-merge-subtree.sh
@@ -25,7 +25,7 @@ test_expect_success 'subtree available and works like recursive' '
 
 	git merge -s subtree side &&
 	for i in mundo $s world; do echo $i; done >expect &&
-	diff -u expect hello
+	test_cmp expect hello
 
 '
 
diff --git a/t/t7010-setup.sh b/t/t7010-setup.sh
index bc8ab6a..02cf7c5 100755
--- a/t/t7010-setup.sh
+++ b/t/t7010-setup.sh
@@ -18,7 +18,7 @@ test_expect_success 'git add (absolute)' '
 	git add "$D/a/b/c/d" &&
 	git ls-files >current &&
 	echo a/b/c/d >expect &&
-	diff -u expect current
+	test_cmp expect current
 
 '
 
@@ -32,7 +32,7 @@ test_expect_success 'git add (funny relative)' '
 	) &&
 	git ls-files >current &&
 	echo a/e/f >expect &&
-	diff -u expect current
+	test_cmp expect current
 
 '
 
@@ -43,7 +43,7 @@ test_expect_success 'git rm (absolute)' '
 	git rm -f --cached "$D/a/b/c/d" &&
 	git ls-files >current &&
 	echo a/e/f >expect &&
-	diff -u expect current
+	test_cmp expect current
 
 '
 
@@ -57,7 +57,7 @@ test_expect_success 'git rm (funny relative)' '
 	) &&
 	git ls-files >current &&
 	echo a/b/c/d >expect &&
-	diff -u expect current
+	test_cmp expect current
 
 '
 
@@ -67,7 +67,7 @@ test_expect_success 'git ls-files (absolute)' '
 	git add a &&
 	git ls-files "$D/a/e/../b" >current &&
 	echo a/b/c/d >expect &&
-	diff -u expect current
+	test_cmp expect current
 
 '
 
@@ -80,7 +80,7 @@ test_expect_success 'git ls-files (relative #1)' '
 		git ls-files "../b/c"
 	)  >current &&
 	echo c/d >expect &&
-	diff -u expect current
+	test_cmp expect current
 
 '
 
@@ -93,7 +93,7 @@ test_expect_success 'git ls-files (relative #2)' '
 		git ls-files --full-name "../e/f"
 	)  >current &&
 	echo a/e/f >expect &&
-	diff -u expect current
+	test_cmp expect current
 
 '
 
@@ -126,13 +126,13 @@ test_expect_success 'log using absolute path names' '
 
 	git log a/b/c/d >f1.txt &&
 	git log "$(pwd)/a/b/c/d" >f2.txt &&
-	diff -u f1.txt f2.txt
+	test_cmp f1.txt f2.txt
 '
 
 test_expect_success 'blame using absolute path names' '
 	git blame a/b/c/d >f1.txt &&
 	git blame "$(pwd)/a/b/c/d" >f2.txt &&
-	diff -u f1.txt f2.txt
+	test_cmp f1.txt f2.txt
 '
 
 test_expect_success 'setup deeper work tree' '
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 63915cd..3111baa 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -83,13 +83,13 @@ test_expect_success "checkout with unrelated dirty tree without -m" '
 	fill 0 1 2 3 4 5 6 7 8 >same &&
 	cp same kept
 	git checkout side >messages &&
-	diff -u same kept
+	test_cmp same kept
 	(cat > messages.expect <<EOF
 M	same
 EOF
 ) &&
 	touch messages.expect &&
-	diff -u messages.expect messages
+	test_cmp messages.expect messages
 '
 
 test_expect_success "checkout -m with dirty tree" '
@@ -106,19 +106,19 @@ test_expect_success "checkout -m with dirty tree" '
 M	one
 EOF
 ) &&
-	diff -u expect.messages messages &&
+	test_cmp expect.messages messages &&
 
 	fill "M	one" "A	three" "D	two" >expect.master &&
 	git diff --name-status master >current.master &&
-	diff -u expect.master current.master &&
+	test_cmp expect.master current.master &&
 
 	fill "M	one" >expect.side &&
 	git diff --name-status side >current.side &&
-	diff -u expect.side current.side &&
+	test_cmp expect.side current.side &&
 
 	: >expect.index &&
 	git diff --cached >current.index &&
-	diff -u expect.index current.index
+	test_cmp expect.index current.index
 '
 
 test_expect_success "checkout -m with dirty tree, renamed" '
@@ -136,7 +136,7 @@ test_expect_success "checkout -m with dirty tree, renamed" '
 
 	git checkout -m renamer &&
 	fill 1 3 4 5 7 8 >expect &&
-	diff -u expect uno &&
+	test_cmp expect uno &&
 	! test -f one &&
 	git diff --cached >current &&
 	! test -s current
@@ -161,7 +161,7 @@ test_expect_success 'checkout -m with merge conflict' '
 	git diff master:one :3:uno |
 	sed -e "1,/^@@/d" -e "/^ /d" -e "s/^-/d/" -e "s/^+/a/" >current &&
 	fill d2 aT d7 aS >expect &&
-	diff -u current expect &&
+	test_cmp current expect &&
 	git diff --cached two >current &&
 	! test -s current
 '
@@ -178,7 +178,7 @@ If you want to create a new branch from this checkout, you may do so
 HEAD is now at 7329388... Initial A one, A two
 EOF
 ) &&
-	diff -u messages.expect messages &&
+	test_cmp messages.expect messages &&
 	H=$(git rev-parse --verify HEAD) &&
 	M=$(git show-ref -s --verify refs/heads/master) &&
 	test "z$H" = "z$M" &&
diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index 361886c..c0288f3 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -203,7 +203,7 @@ test_expect_success 'sign off (1)' '
 		git var GIT_COMMITTER_IDENT |
 		sed -e "s/>.*/>/" -e "s/^/Signed-off-by: /"
 	) >expected &&
-	diff -u expected actual
+	test_cmp expected actual
 
 '
 
@@ -223,7 +223,7 @@ $existing" &&
 		git var GIT_COMMITTER_IDENT |
 		sed -e "s/>.*/>/" -e "s/^/Signed-off-by: /"
 	) >expected &&
-	diff -u expected actual
+	test_cmp expected actual
 
 '
 
@@ -240,7 +240,7 @@ test_expect_success 'multiple -m' '
 		echo
 		echo three
 	) >expected &&
-	diff -u expected actual
+	test_cmp expected actual
 
 '
 
@@ -301,12 +301,12 @@ test_expect_success 'same tree (merge and amend merge)' '
 	git merge -s ours side -m "empty ok" &&
 	git diff HEAD^ HEAD >actual &&
 	: >expected &&
-	diff -u expected actual &&
+	test_cmp expected actual &&
 
 	git commit --amend -m "empty really ok" &&
 	git diff HEAD^ HEAD >actual &&
 	: >expected &&
-	diff -u expected actual
+	test_cmp expected actual
 
 '
 
@@ -323,7 +323,7 @@ test_expect_success 'amend using the message from another commit' '
 	git commit --allow-empty --amend -C "$old" &&
 	git show --pretty="format:%ad %s" "$old" >expected &&
 	git show --pretty="format:%ad %s" HEAD >actual &&
-	diff -u expected actual
+	test_cmp expected actual
 
 '
 
@@ -341,7 +341,7 @@ test_expect_success 'amend using the message from a commit named with tag' '
 	git commit --allow-empty --amend -C tagged-old &&
 	git show --pretty="format:%ad %s" "$old" >expected &&
 	git show --pretty="format:%ad %s" HEAD >actual &&
-	diff -u expected actual
+	test_cmp expected actual
 
 '
 
diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index b780fdd..284c941 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -85,7 +85,7 @@ test_expect_success 'verbose' '
 	git add negative &&
 	git status -v | sed -ne "/^diff --git /p" >actual &&
 	echo "diff --git a/negative b/negative" >expect &&
-	diff -u expect actual
+	test_cmp expect actual
 
 '
 
@@ -95,7 +95,7 @@ test_expect_success 'cleanup commit messages (verbatim,-t)' '
 	{ echo;echo "# text";echo; } >expect &&
 	git commit --cleanup=verbatim -t expect -a &&
 	git cat-file -p HEAD |sed -e "1,/^\$/d" |head -n 3 >actual &&
-	diff -u expect actual
+	test_cmp expect actual
 
 '
 
@@ -104,7 +104,7 @@ test_expect_success 'cleanup commit messages (verbatim,-F)' '
 	echo >>negative &&
 	git commit --cleanup=verbatim -F expect -a &&
 	git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&
-	diff -u expect actual
+	test_cmp expect actual
 
 '
 
@@ -113,7 +113,7 @@ test_expect_success 'cleanup commit messages (verbatim,-m)' '
 	echo >>negative &&
 	git commit --cleanup=verbatim -m "$(cat expect)" -a &&
 	git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&
-	diff -u expect actual
+	test_cmp expect actual
 
 '
 
@@ -124,7 +124,7 @@ test_expect_success 'cleanup commit messages (whitespace,-F)' '
 	echo "# text" >expect &&
 	git commit --cleanup=whitespace -F text -a &&
 	git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&
-	diff -u expect actual
+	test_cmp expect actual
 
 '
 
@@ -135,7 +135,7 @@ test_expect_success 'cleanup commit messages (strip,-F)' '
 	echo sample >expect &&
 	git commit --cleanup=strip -F text -a &&
 	git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&
-	diff -u expect actual
+	test_cmp expect actual
 
 '
 
@@ -150,7 +150,7 @@ test_expect_success 'cleanup commit messages (strip,-F,-e)' '
 	{ echo;echo sample;echo; } >text &&
 	git commit -e -F text -a &&
 	head -n 4 .git/COMMIT_EDITMSG >actual &&
-	diff -u expect actual
+	test_cmp expect actual
 
 '
 
diff --git a/t/t7502-status.sh b/t/t7502-status.sh
index 70b802b..cd08516 100755
--- a/t/t7502-status.sh
+++ b/t/t7502-status.sh
@@ -146,7 +146,7 @@ cat <<EOF >expect
 EOF
 test_expect_success 'status of partial commit excluding new file in index' '
 	git status dir1/modified >output &&
-	diff -u expect output
+	test_cmp expect output
 '
 
 test_done
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 219411f..56869ac 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -108,7 +108,7 @@ create_merge_msgs() {
 }
 
 verify_diff() {
-	if ! diff -u "$1" "$2"
+	if ! test_cmp "$1" "$2"
 	then
 		echo "$3"
 		false
diff --git a/t/t8003-blame.sh b/t/t8003-blame.sh
index db51b3a..966bb0a 100755
--- a/t/t8003-blame.sh
+++ b/t/t8003-blame.sh
@@ -112,7 +112,7 @@ test_expect_success 'blame wholesale copy' '
 		echo mouse-Second
 		echo mouse-Third
 	} >expected &&
-	diff -u expected current
+	test_cmp expected current
 
 '
 
@@ -125,7 +125,7 @@ test_expect_success 'blame wholesale copy and more' '
 		echo cow-Fifth
 		echo mouse-Third
 	} >expected &&
-	diff -u expected current
+	test_cmp expected current
 
 '
 
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index cbbfa9c..c0973b4 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -81,7 +81,7 @@ test_expect_success 'Show all headers' '
 		-e "s/^\(Message-Id:\).*/\1 MESSAGE-ID-STRING/" \
 		-e "s/^\(X-Mailer:\).*/\1 X-MAILER-STRING/" \
 		>actual-show-all-headers &&
-	diff -u expected-show-all-headers actual-show-all-headers
+	test_cmp expected-show-all-headers actual-show-all-headers
 '
 
 z8=zzzzzzzz
diff --git a/t/t9116-git-svn-log.sh b/t/t9116-git-svn-log.sh
index 902ed41..e1e8bdf 100755
--- a/t/t9116-git-svn-log.sh
+++ b/t/t9116-git-svn-log.sh
@@ -55,74 +55,74 @@ printf 'r1 \nr2 \nr4 \n' > expected-range-r1-r2-r4
 
 test_expect_success 'test ascending revision range' "
 	git reset --hard trunk &&
-	git svn log -r 1:4 | grep '^r[0-9]' | cut -d'|' -f1 | diff -u expected-range-r1-r2-r4 -
+	git svn log -r 1:4 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r1-r2-r4 -
 	"
 
 printf 'r4 \nr2 \nr1 \n' > expected-range-r4-r2-r1
 
 test_expect_success 'test descending revision range' "
 	git reset --hard trunk &&
-	git svn log -r 4:1 | grep '^r[0-9]' | cut -d'|' -f1 | diff -u expected-range-r4-r2-r1 -
+	git svn log -r 4:1 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r4-r2-r1 -
 	"
 
 printf 'r1 \nr2 \n' > expected-range-r1-r2
 
 test_expect_success 'test ascending revision range with unreachable revision' "
 	git reset --hard trunk &&
-	git svn log -r 1:3 | grep '^r[0-9]' | cut -d'|' -f1 | diff -u expected-range-r1-r2 -
+	git svn log -r 1:3 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r1-r2 -
 	"
 
 printf 'r2 \nr1 \n' > expected-range-r2-r1
 
 test_expect_success 'test descending revision range with unreachable revision' "
 	git reset --hard trunk &&
-	git svn log -r 3:1 | grep '^r[0-9]' | cut -d'|' -f1 | diff -u expected-range-r2-r1 -
+	git svn log -r 3:1 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r2-r1 -
 	"
 
 printf 'r2 \n' > expected-range-r2
 
 test_expect_success 'test ascending revision range with unreachable upper boundary revision and 1 commit' "
 	git reset --hard trunk &&
-	git svn log -r 2:3 | grep '^r[0-9]' | cut -d'|' -f1 | diff -u expected-range-r2 -
+	git svn log -r 2:3 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r2 -
 	"
 
 test_expect_success 'test descending revision range with unreachable upper boundary revision and 1 commit' "
 	git reset --hard trunk &&
-	git svn log -r 3:2 | grep '^r[0-9]' | cut -d'|' -f1 | diff -u expected-range-r2 -
+	git svn log -r 3:2 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r2 -
 	"
 
 printf 'r4 \n' > expected-range-r4
 
 test_expect_success 'test ascending revision range with unreachable lower boundary revision and 1 commit' "
 	git reset --hard trunk &&
-	git svn log -r 3:4 | grep '^r[0-9]' | cut -d'|' -f1 | diff -u expected-range-r4 -
+	git svn log -r 3:4 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r4 -
 	"
 
 test_expect_success 'test descending revision range with unreachable lower boundary revision and 1 commit' "
 	git reset --hard trunk &&
-	git svn log -r 4:3 | grep '^r[0-9]' | cut -d'|' -f1 | diff -u expected-range-r4 -
+	git svn log -r 4:3 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r4 -
 	"
 
 printf -- '------------------------------------------------------------------------\n' > expected-separator
 
 test_expect_success 'test ascending revision range with unreachable boundary revisions and no commits' "
 	git reset --hard trunk &&
-	git svn log -r 5:6 | diff -u expected-separator -
+	git svn log -r 5:6 | test_cmp expected-separator -
 	"
 
 test_expect_success 'test descending revision range with unreachable boundary revisions and no commits' "
 	git reset --hard trunk &&
-	git svn log -r 6:5 | diff -u expected-separator -
+	git svn log -r 6:5 | test_cmp expected-separator -
 	"
 
 test_expect_success 'test ascending revision range with unreachable boundary revisions and 1 commit' "
 	git reset --hard trunk &&
-	git svn log -r 3:5 | grep '^r[0-9]' | cut -d'|' -f1 | diff -u expected-range-r4 -
+	git svn log -r 3:5 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r4 -
 	"
 
 test_expect_success 'test descending revision range with unreachable boundary revisions and 1 commit' "
 	git reset --hard trunk &&
-	git svn log -r 5:3 | grep '^r[0-9]' | cut -d'|' -f1 | diff -u expected-range-r4 -
+	git svn log -r 5:3 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r4 -
 	"
 
 test_done
diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh
index 58c59ed..42b144b 100755
--- a/t/t9200-git-cvsexportcommit.sh
+++ b/t/t9200-git-cvsexportcommit.sh
@@ -37,7 +37,7 @@ check_entries () {
 	else
 		printf '%s\n' "$2" | tr '|' '\012' >expected
 	fi
-	diff -u expected actual
+	test_cmp expected actual
 }
 
 test_expect_success \
@@ -257,8 +257,8 @@ test_expect_success '-w option should work with relative GIT_DIR' '
       (cd "$GIT_DIR" &&
       GIT_DIR=. git cvsexportcommit -w "$CVSWORK" -c $id &&
       check_entries "$CVSWORK/W" "file1.txt/1.1/|file2.txt/1.1/" &&
-      diff -u "$CVSWORK/W/file1.txt" ../W/file1.txt &&
-      diff -u "$CVSWORK/W/file2.txt" ../W/file2.txt
+      test_cmp "$CVSWORK/W/file1.txt" ../W/file1.txt &&
+      test_cmp "$CVSWORK/W/file2.txt" ../W/file2.txt
       )
 '
 
@@ -279,9 +279,9 @@ test_expect_success 'check files before directories' '
 	git cvsexportcommit -w "$CVSWORK" -c $id &&
 	check_entries "$CVSWORK/E" "DS/1.1/|newfile5.txt/1.1/" &&
 	check_entries "$CVSWORK" "DS/1.1/|release-notes/1.2/" &&
-	diff -u "$CVSWORK/DS" DS &&
-	diff -u "$CVSWORK/E/DS" E/DS &&
-	diff -u "$CVSWORK/release-notes" release-notes
+	test_cmp "$CVSWORK/DS" DS &&
+	test_cmp "$CVSWORK/E/DS" E/DS &&
+	test_cmp "$CVSWORK/release-notes" release-notes
 
 '
 
@@ -293,7 +293,7 @@ test_expect_success 'commit a file with leading spaces in the name' '
 	id=$(git rev-parse HEAD) &&
 	git cvsexportcommit -w "$CVSWORK" -c $id &&
 	check_entries "$CVSWORK" " space/1.1/|DS/1.1/|release-notes/1.2/" &&
-	diff -u "$CVSWORK/ space" " space"
+	test_cmp "$CVSWORK/ space" " space"
 
 '
 
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 6aea0ea..268b26c 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -42,6 +42,7 @@ export GIT_MERGE_VERBOSITY
 export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
 export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
 export EDITOR VISUAL
+GIT_TEST_CMP=${GIT_TEST_CMP:-diff -u}
 
 # Protect ourselves from common misconfiguration to export
 # CDPATH into the environment
@@ -302,6 +303,23 @@ test_must_fail () {
 	test $? -gt 0 -a $? -le 128
 }
 
+# test_cmp is a helper function to compare actual and expected output.
+# You can use it like:
+#
+#	test_expect_success 'foo works' '
+#		echo expected >expected &&
+#		foo >actual &&
+#		test_cmp expected actual
+#	'
+#
+# This could be written as either "cmp" or "diff -u", but:
+# - cmp's output is not nearly as easy to read as diff -u
+# - not all diff versions understand "-u"
+
+test_cmp() {
+	$GIT_TEST_CMP "$@"
+}
+
 # Most tests can use the created repository, but some may need to create more.
 # Usage: test_create_repo <directory>
 test_create_repo () {
-- 
1.5.4.4.543.g30fdd.dirty

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

* [PATCH 07/16] t4020: don't use grep -a
       [not found] <cover.1205356737.git.peff@peff.net>
                   ` (5 preceding siblings ...)
  2008-03-12 21:36 ` [PATCH 06/16] add test_cmp function for test scripts Jeff King
@ 2008-03-12 21:37 ` Jeff King
  2008-03-12 21:37 ` [PATCH 08/16] t4200: use cut instead of sed Jeff King
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 59+ messages in thread
From: Jeff King @ 2008-03-12 21:37 UTC (permalink / raw)
  To: Whit Armstrong; +Cc: Junio C Hamano, git

Solaris /usr/bin/grep doesn't understand "-a". In this case
we can just include the expected output with the test, which
is a better test anyway.

Signed-off-by: Jeff King <peff@peff.net>
---
Ooh, we get to use binary patches.

 t/t4020-diff-external.sh |    3 ++-
 t/t4020/diff.NUL         |  Bin 0 -> 116 bytes
 2 files changed, 2 insertions(+), 1 deletions(-)
 create mode 100644 t/t4020/diff.NUL

diff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh
index bf8f778..637b4e1 100755
--- a/t/t4020-diff-external.sh
+++ b/t/t4020-diff-external.sh
@@ -103,7 +103,8 @@ echo NULZbetweenZwords | perl -pe 'y/Z/\000/' > file
 
 test_expect_success 'force diff with "diff"' '
 	echo >.gitattributes "file diff" &&
-	git diff | grep -a second
+	git diff >actual &&
+	test_cmp ../t4020/diff.NUL actual
 '
 
 test_done
diff --git a/t/t4020/diff.NUL b/t/t4020/diff.NUL
new file mode 100644
index 0000000000000000000000000000000000000000..db2f89090c1c4de05e4f82ea39ea118fccfd48dd
GIT binary patch
literal 116
zcmXxbF$#k~5Jq9^ImPtD=#EH8x;=oT%K;qs->?u?P{ABu2(fz2_fpB3Ro`XjsmtX9
z_Ft&fgfAo5!x7pxTMzd;TL`ydAXWW)5|QhPk=0m?V<g=$FEx=oUt{Cg51=;3vZjqr
Do_`?W

literal 0
HcmV?d00001

-- 
1.5.4.4.543.g30fdd.dirty

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

* [PATCH 08/16] t4200: use cut instead of sed
       [not found] <cover.1205356737.git.peff@peff.net>
                   ` (6 preceding siblings ...)
  2008-03-12 21:37 ` [PATCH 07/16] t4020: don't use grep -a Jeff King
@ 2008-03-12 21:37 ` Jeff King
  2008-03-13  4:52   ` Junio C Hamano
  2008-03-12 21:38 ` [PATCH 09/16] t6000lib: tr portability fix Jeff King
                   ` (7 subsequent siblings)
  15 siblings, 1 reply; 59+ messages in thread
From: Jeff King @ 2008-03-12 21:37 UTC (permalink / raw)
  To: Whit Armstrong; +Cc: Junio C Hamano, git

Some versions of sed (like the one on Solaris) don't like to
match literal tabs, and simply print nothing. Instead, let's
use cut.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t4200-rerere.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index 3cbfee7..d3dea2a 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -53,7 +53,7 @@ test_expect_success 'conflicting merge' '
 	! git merge first
 '
 
-sha1=$(sed -e 's/	.*//' .git/rr-cache/MERGE_RR)
+sha1=$(cut -d'	' -f1 .git/rr-cache/MERGE_RR)
 rr=.git/rr-cache/$sha1
 test_expect_success 'recorded preimage' "grep ======= $rr/preimage"
 
-- 
1.5.4.4.543.g30fdd.dirty

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

* [PATCH 09/16] t6000lib: tr portability fix
       [not found] <cover.1205356737.git.peff@peff.net>
                   ` (7 preceding siblings ...)
  2008-03-12 21:37 ` [PATCH 08/16] t4200: use cut instead of sed Jeff King
@ 2008-03-12 21:38 ` Jeff King
  2008-03-14 20:47   ` [PATCH] t/t6000lib.sh: tr portability fix fix Brandon Casey
  2008-03-12 21:39 ` [PATCH 10/16] add NO_EXTERNAL_GREP build option Jeff King
                   ` (6 subsequent siblings)
  15 siblings, 1 reply; 59+ messages in thread
From: Jeff King @ 2008-03-12 21:38 UTC (permalink / raw)
  To: Whit Armstrong; +Cc: Junio C Hamano, git

Some versions of tr complain if the number of characters in
both sets isn't the same. So here we must manually expand
the dashes in set2.

Signed-off-by: Jeff King <peff@peff.net>
---
This almost makes me want to just use sed instead. But quoting that line
noise would probably make it less readable.

 t/t6000lib.sh |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/t/t6000lib.sh b/t/t6000lib.sh
index 180633e..b69f7c4 100755
--- a/t/t6000lib.sh
+++ b/t/t6000lib.sh
@@ -97,7 +97,10 @@ check_output()
 # from front and back.
 name_from_description()
 {
-        tr "'" '-' | tr '~`!@#$%^&*()_+={}[]|\;:"<>,/? ' '-' | tr -s '-' | tr '[A-Z]' '[a-z]' | sed "s/^-*//;s/-*\$//"
+        tr "'" '-' |
+		tr '~`!@#$%^&*()_+={}[]|\;:"<>,/? ' \
+		   '------------------------------' |
+		tr -s '-' | tr '[A-Z]' '[a-z]' | sed "s/^-*//;s/-*\$//"
 }
 
 
-- 
1.5.4.4.543.g30fdd.dirty

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

* [PATCH 10/16] add NO_EXTERNAL_GREP build option
       [not found] <cover.1205356737.git.peff@peff.net>
                   ` (8 preceding siblings ...)
  2008-03-12 21:38 ` [PATCH 09/16] t6000lib: tr portability fix Jeff King
@ 2008-03-12 21:39 ` Jeff King
  2008-03-12 22:30   ` Junio C Hamano
                     ` (2 more replies)
  2008-03-12 21:40 ` [PATCH 11/16] config: add --literal-match option Jeff King
                   ` (5 subsequent siblings)
  15 siblings, 3 replies; 59+ messages in thread
From: Jeff King @ 2008-03-12 21:39 UTC (permalink / raw)
  To: Whit Armstrong; +Cc: Junio C Hamano, git

Previously, we just chose whether to allow external grep
based on the __unix__ define. However, there are systems
which define this macro but which have an inferior group
(e.g., one that does not support all options used by t7002).
This allows users to accept the potential speed penalty to
get a more consistent grep experience (and to pass the
testsuite).

Signed-off-by: Jeff King <peff@peff.net>
---
This might have fallouts for msysgit (i.e., they need to define
NO_EXTERNAL_GREP instead of relying on __unix__ not being defined).

 Makefile       |    7 +++++++
 builtin-grep.c |    4 ++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index e3eaa6a..8e80225 100644
--- a/Makefile
+++ b/Makefile
@@ -148,6 +148,9 @@ all::
 # is a simplified version of the merge sort used in glibc. This is
 # recommended if Git triggers O(n^2) behavior in your platform's qsort().
 #
+# Define NO_EXTERNAL_GREP if you don't want "git grep" to ever call
+# your external grep (e.g., if your system lacks grep, or if its grep is
+# not very featureful).
 
 GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -467,6 +470,7 @@ ifeq ($(uname_O),Cygwin)
 	NO_STRCASESTR = YesPlease
 	NO_MEMMEM = YesPlease
 	NO_SYMLINK_HEAD = YesPlease
+	NO_EXTERNAL_GREP = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
 	NO_TRUSTABLE_FILEMODE = UnfortunatelyYes
@@ -760,6 +764,9 @@ endif
 ifdef DIR_HAS_BSD_GROUP_SEMANTICS
 	COMPAT_CFLAGS += -DDIR_HAS_BSD_GROUP_SEMANTICS
 endif
+ifdef NO_EXTERNAL_GREP
+	BASIC_CFLAGS += -DNO_EXTERNAL_GREP
+endif
 
 ifeq ($(TCLTK_PATH),)
 NO_TCLTK=NoThanks
diff --git a/builtin-grep.c b/builtin-grep.c
index f4f4ecb..f215b28 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -153,7 +153,7 @@ static int grep_file(struct grep_opt *opt, const char *filename)
 	return i;
 }
 
-#ifdef __unix__
+#ifndef NO_EXTERNAL_GREP
 static int exec_grep(int argc, const char **argv)
 {
 	pid_t pid;
@@ -372,7 +372,7 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached)
 	int nr;
 	read_cache();
 
-#ifdef __unix__
+#ifndef NO_EXTERNAL_GREP
 	/*
 	 * Use the external "grep" command for the case where
 	 * we grep through the checked-out files. It tends to
-- 
1.5.4.4.543.g30fdd.dirty

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

* [PATCH 11/16] config: add --literal-match option
       [not found] <cover.1205356737.git.peff@peff.net>
                   ` (9 preceding siblings ...)
  2008-03-12 21:39 ` [PATCH 10/16] add NO_EXTERNAL_GREP build option Jeff King
@ 2008-03-12 21:40 ` Jeff King
  2008-03-12 21:46   ` Jakub Narebski
  2008-03-12 22:34   ` Junio C Hamano
  2008-03-12 21:40 ` [PATCH 12/16] git-submodule: avoid sed input with no newline Jeff King
                   ` (4 subsequent siblings)
  15 siblings, 2 replies; 59+ messages in thread
From: Jeff King @ 2008-03-12 21:40 UTC (permalink / raw)
  To: Whit Armstrong; +Cc: Junio C Hamano, git

When limiting the values to be set (or returned), the user
previously had the option of specifying a regex. In some
cases, however, they may want to find a literal value. The
option --literal-match converts any matching regex into a
literal string comparison.

Signed-off-by: Jeff King <peff@peff.net>
---
I think this is a nice addition regardless, but it is used by the next
patch.

The patch is about twice as long as it needs to be since getting and
setting in builtin-config follow two almost-the-same parallel codepaths.
I suspect this could be cleaned up, but I didn't look too closely.

 Documentation/git-config.txt |    7 ++++
 builtin-config.c             |   50 +++++++++++++++++++++++--------
 builtin-remote.c             |    2 +-
 cache.h                      |    2 +-
 config.c                     |   67 ++++++++++++++++++++++++++++--------------
 t/t1300-repo-config.sh       |   13 ++++++++
 6 files changed, 104 insertions(+), 37 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index fa16171..5dc1af2 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -152,6 +152,13 @@ See also <<FILES>>.
 	output.  The optional `default` parameter is used instead, if
 	there is no color configured for `name`.
 
+--literal-match::
+
+	Some invocations of git-config will limit their actions based on
+	matching a config value to a regular expression. If this option
+	is used, then any such matches are done as a string comparison
+	rather than as a regular expression match.
+
 [[FILES]]
 FILES
 -----
diff --git a/builtin-config.c b/builtin-config.c
index 2b9a426..ed318dc 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -8,6 +8,7 @@ static const char git_config_set_usage[] =
 static char *key;
 static regex_t *key_regexp;
 static regex_t *regexp;
+const char *value_match;
 static int show_keys;
 static int use_key_regexp;
 static int do_all;
@@ -17,6 +18,7 @@ static char delim = '=';
 static char key_delim = ' ';
 static char term = '\n';
 static enum { T_RAW, T_INT, T_BOOL } type = T_RAW;
+static int literal_match = 0;
 
 static int show_all_config(const char *key_, const char *value_)
 {
@@ -40,6 +42,9 @@ static int show_config(const char* key_, const char* value_)
 	if (regexp != NULL &&
 	    (do_not_match ^ !!regexec(regexp, (value_?value_:""), 0, NULL, 0)))
 		return 0;
+	if (value_match != NULL &&
+		do_not_match ^ !!strcmp(value_match, (value_ ? value_ : "")))
+		return 0;
 
 	if (show_keys) {
 		if (value_)
@@ -66,7 +71,7 @@ static int show_config(const char* key_, const char* value_)
 	return 0;
 }
 
-static int get_value(const char* key_, const char* regex_)
+static int get_value(const char* key_, const char* match_)
 {
 	int ret = -1;
 	char *tl;
@@ -99,18 +104,28 @@ static int get_value(const char* key_, const char* regex_)
 		}
 	}
 
-	if (regex_) {
-		if (regex_[0] == '!') {
+	if (match_ && literal_match) {
+		value_match = match_;
+		do_not_match = 0;
+		regexp = NULL;
+	}
+	else if(match_) {
+		value_match = NULL;
+		if (match_[0] == '!') {
 			do_not_match = 1;
-			regex_++;
+			match_++;
 		}
 
 		regexp = (regex_t*)xmalloc(sizeof(regex_t));
-		if (regcomp(regexp, regex_, REG_EXTENDED)) {
-			fprintf(stderr, "Invalid pattern: %s\n", regex_);
+		if (regcomp(regexp, match_, REG_EXTENDED)) {
+			fprintf(stderr, "Invalid pattern: %s\n", match_);
 			goto free_strings;
 		}
 	}
+	else {
+		value_match = NULL;
+		regexp = NULL;
+	}
 
 	if (do_all && system_wide)
 		git_config_from_file(show_config, system_wide);
@@ -339,6 +354,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 			return get_color(argc-2, argv+2);
 		} else if (!strcmp(argv[1], "--get-colorbool")) {
 			return get_colorbool(argc-2, argv+2);
+		} else if (!strcmp(argv[1], "--literal-match")) {
+			literal_match = 1;
 		} else
 			break;
 		argc--;
@@ -352,7 +369,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		if (!strcmp(argv[1], "--unset"))
 			return git_config_set(argv[2], NULL);
 		else if (!strcmp(argv[1], "--unset-all"))
-			return git_config_set_multivar(argv[2], NULL, NULL, 1);
+			return git_config_set_multivar(argv[2], NULL, NULL,
+					literal_match, 1);
 		else if (!strcmp(argv[1], "--get"))
 			return get_value(argv[2], NULL);
 		else if (!strcmp(argv[1], "--get-all")) {
@@ -369,9 +387,11 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		}
 	case 4:
 		if (!strcmp(argv[1], "--unset"))
-			return git_config_set_multivar(argv[2], NULL, argv[3], 0);
+			return git_config_set_multivar(argv[2], NULL, argv[3],
+					literal_match, 0);
 		else if (!strcmp(argv[1], "--unset-all"))
-			return git_config_set_multivar(argv[2], NULL, argv[3], 1);
+			return git_config_set_multivar(argv[2], NULL, argv[3],
+					literal_match, 1);
 		else if (!strcmp(argv[1], "--get"))
 			return get_value(argv[2], argv[3]);
 		else if (!strcmp(argv[1], "--get-all")) {
@@ -384,18 +404,22 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 			return get_value(argv[2], argv[3]);
 		} else if (!strcmp(argv[1], "--add")) {
 			value = normalize_value(argv[2], argv[3]);
-			return git_config_set_multivar(argv[2], value, "^$", 0);
+			return git_config_set_multivar(argv[2], value, "^$",
+					0, 0);
 		} else if (!strcmp(argv[1], "--replace-all")) {
 			value = normalize_value(argv[2], argv[3]);
-			return git_config_set_multivar(argv[2], value, NULL, 1);
+			return git_config_set_multivar(argv[2], value, NULL,
+					0, 1);
 		} else {
 			value = normalize_value(argv[1], argv[2]);
-			return git_config_set_multivar(argv[1], value, argv[3], 0);
+			return git_config_set_multivar(argv[1], value, argv[3],
+				       literal_match, 0);
 		}
 	case 5:
 		if (!strcmp(argv[1], "--replace-all")) {
 			value = normalize_value(argv[2], argv[3]);
-			return git_config_set_multivar(argv[2], value, argv[4], 1);
+			return git_config_set_multivar(argv[2], value, argv[4],
+					literal_match, 1);
 		}
 	case 1:
 	default:
diff --git a/builtin-remote.c b/builtin-remote.c
index 24e6929..4eae74b 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -107,7 +107,7 @@ static int add(int argc, const char **argv)
 		else
 			strbuf_addf(&buf2, "refs/heads/%s:refs/remotes/%s/%s",
 					item->path, name, item->path);
-		if (git_config_set_multivar(buf.buf, buf2.buf, "^$", 0))
+		if (git_config_set_multivar(buf.buf, buf2.buf, "^$", 0, 0))
 			return 1;
 	}
 
diff --git a/cache.h b/cache.h
index 2a1e7ec..30830b0 100644
--- a/cache.h
+++ b/cache.h
@@ -695,7 +695,7 @@ extern unsigned long git_config_ulong(const char *, const char *);
 extern int git_config_bool(const char *, const char *);
 extern int git_config_string(const char **, const char *, const char *);
 extern int git_config_set(const char *, const char *);
-extern int git_config_set_multivar(const char *, const char *, const char *, int);
+extern int git_config_set_multivar(const char *, const char *, const char *, int, int);
 extern int git_config_rename_section(const char *, const char *);
 extern const char *git_etc_gitconfig(void);
 extern int check_repository_format_version(const char *var, const char *value);
diff --git a/config.c b/config.c
index 0624494..862b0e6 100644
--- a/config.c
+++ b/config.c
@@ -575,7 +575,15 @@ static struct {
 	int baselen;
 	char* key;
 	int do_not_match;
-	regex_t* value_regex;
+	enum {
+		VALUE_NONE,
+		VALUE_REGEX,
+		VALUE_STRING,
+	} value_type;
+	union {
+		regex_t* regex;
+		const char *string;
+	} value;
 	int multi_replace;
 	size_t offset[MAX_MATCHES];
 	enum { START, SECTION_SEEN, SECTION_END_SEEN, KEY_SEEN } state;
@@ -584,10 +592,19 @@ static struct {
 
 static int matches(const char* key, const char* value)
 {
-	return !strcmp(key, store.key) &&
-		(store.value_regex == NULL ||
-		 (store.do_not_match ^
-		  !regexec(store.value_regex, value, 0, NULL, 0)));
+	if (strcmp(key, store.key))
+		return 0;
+	switch(store.value_type) {
+	case VALUE_NONE:
+		return 1;
+	case VALUE_REGEX:
+		return store.do_not_match ^
+			!regexec(store.value.regex, value, 0, NULL, 0);
+	case VALUE_STRING:
+		return store.do_not_match ^
+			!strcmp(value, store.value.string);
+	}
+	die("bug in config.c:matches");
 }
 
 static int store_aux(const char* key, const char* value)
@@ -764,12 +781,12 @@ contline:
 
 int git_config_set(const char* key, const char* value)
 {
-	return git_config_set_multivar(key, value, NULL, 0);
+	return git_config_set_multivar(key, value, NULL, 0, 0);
 }
 
 /*
  * If value==NULL, unset in (remove from) config,
- * if value_regex!=NULL, disregard key/value pairs where value does not match.
+ * if value_match!=NULL, disregard key/value pairs where value does not match.
  * if multi_replace==0, nothing, or only one matching key/value is replaced,
  *     else all matching key/values (regardless how many) are removed,
  *     before the new pair is written.
@@ -791,7 +808,7 @@ int git_config_set(const char* key, const char* value)
  *
  */
 int git_config_set_multivar(const char* key, const char* value,
-	const char* value_regex, int multi_replace)
+	const char* value_match, int literal_match, int multi_replace)
 {
 	int i, dot;
 	int fd = -1, in_fd;
@@ -892,21 +909,27 @@ int git_config_set_multivar(const char* key, const char* value,
 		size_t contents_sz, copy_begin, copy_end;
 		int i, new_line = 0;
 
-		if (value_regex == NULL)
-			store.value_regex = NULL;
+		if (value_match == NULL)
+			store.value_type = VALUE_NONE;
+		else if(literal_match) {
+			store.value_type = VALUE_STRING;
+			store.do_not_match = 0;
+			store.value.string = value_match;
+		}
 		else {
-			if (value_regex[0] == '!') {
+			store.value_type = VALUE_REGEX;
+			if (value_match[0] == '!') {
 				store.do_not_match = 1;
-				value_regex++;
+				value_match++;
 			} else
 				store.do_not_match = 0;
 
-			store.value_regex = (regex_t*)xmalloc(sizeof(regex_t));
-			if (regcomp(store.value_regex, value_regex,
+			store.value.regex = (regex_t*)xmalloc(sizeof(regex_t));
+			if (regcomp(store.value.regex, value_match,
 					REG_EXTENDED)) {
 				fprintf(stderr, "Invalid pattern: %s\n",
-					value_regex);
-				free(store.value_regex);
+					value_match);
+				free(store.value.regex);
 				ret = 6;
 				goto out_free;
 			}
@@ -925,18 +948,18 @@ int git_config_set_multivar(const char* key, const char* value,
 		if (git_config_from_file(store_aux, config_filename)) {
 			fprintf(stderr, "invalid config file\n");
 			free(store.key);
-			if (store.value_regex != NULL) {
-				regfree(store.value_regex);
-				free(store.value_regex);
+			if (store.value_type == VALUE_REGEX) {
+				regfree(store.value.regex);
+				free(store.value.regex);
 			}
 			ret = 3;
 			goto out_free;
 		}
 
 		free(store.key);
-		if (store.value_regex != NULL) {
-			regfree(store.value_regex);
-			free(store.value_regex);
+		if (store.value_type == VALUE_REGEX) {
+			regfree(store.value.regex);
+			free(store.value.regex);
 		}
 
 		/* if nothing to unset, or too many matches, error out */
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index b36a901..6c5ccdd 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -682,4 +682,17 @@ test_expect_success 'symlinked configuration' '
 
 '
 
+test_expect_success 'literal matching works' '
+
+	git config literal.key1 value &&
+	git config literal.key2 va..e &&
+	git config --get-regexp "literal..*" va..e >output &&
+	grep key1 output &&
+	grep key2 output &&
+	git config --literal-match --get-regexp "literal..*" va..e >output &&
+	! grep key1 output &&
+	grep key2 output
+
+'
+
 test_done
-- 
1.5.4.4.543.g30fdd.dirty

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

* [PATCH 12/16] git-submodule: avoid sed input with no newline
       [not found] <cover.1205356737.git.peff@peff.net>
                   ` (10 preceding siblings ...)
  2008-03-12 21:40 ` [PATCH 11/16] config: add --literal-match option Jeff King
@ 2008-03-12 21:40 ` Jeff King
  2008-03-12 22:41   ` Junio C Hamano
  2008-03-12 21:41 ` [PATCH 13/16] filter-branch: don't use xargs -0 Jeff King
                   ` (3 subsequent siblings)
  15 siblings, 1 reply; 59+ messages in thread
From: Jeff King @ 2008-03-12 21:40 UTC (permalink / raw)
  To: Whit Armstrong; +Cc: Junio C Hamano, git

Some versions of sed don't like this, and give no output at
all. Instead, we can use git-config to pare down the matches
for us.

The content of the last three lines of the patch aren't
changed at all; they merely fix a bogus 7-space indentation.

Signed-off-by: Jeff King <peff@peff.net>
---
And this is safe because of the --literal-match from the last patch.

 git-submodule.sh |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index ceb2295..fb01d94 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -71,13 +71,12 @@ resolve_relative_url ()
 module_name()
 {
 	# Do we have "submodule.<something>.path = $1" defined in .gitmodules file?
-	re=$(printf '%s' "$1" | sed -e 's/[].[^$\\*]/\\&/g')
-	name=$( GIT_CONFIG=.gitmodules \
-		git config --get-regexp '^submodule\..*\.path$' |
-		sed -n -e 's|^submodule\.\(.*\)\.path '"$re"'$|\1|p' )
-       test -z "$name" &&
-       die "No submodule mapping found in .gitmodules for path '$path'"
-       echo "$name"
+	name=$(git config --literal-match -f .gitmodules \
+		--get-regexp 'submodule\..*\.path$' "$1" |
+		sed -e 's/submodule\.//' -e 's/\.path.*//')
+	test -z "$name" &&
+	die "No submodule mapping found in .gitmodules for path '$path'"
+	echo "$name"
 }
 
 #
-- 
1.5.4.4.543.g30fdd.dirty

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

* [PATCH 13/16] filter-branch: don't use xargs -0
       [not found] <cover.1205356737.git.peff@peff.net>
                   ` (11 preceding siblings ...)
  2008-03-12 21:40 ` [PATCH 12/16] git-submodule: avoid sed input with no newline Jeff King
@ 2008-03-12 21:41 ` Jeff King
  2008-03-12 21:41 ` [PATCH 14/16] filter-branch: use $SHELL_PATH instead of 'sh' Jeff King
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 59+ messages in thread
From: Jeff King @ 2008-03-12 21:41 UTC (permalink / raw)
  To: Whit Armstrong; +Cc: Junio C Hamano, git

Some versions of xargs don't understand "-0"; fortunately in
this case we can get the same effect by using "git clean".

Signed-off-by: Jeff King <peff@peff.net>
---
 git-filter-branch.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 59cf023..efef732 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -281,7 +281,7 @@ while read commit parents; do
 			die "Could not checkout the index"
 		# files that $commit removed are now still in the working tree;
 		# remove them, else they would be added again
-		git ls-files -z --others | xargs -0 rm -f
+		git clean -q -f -x
 		eval "$filter_tree" < /dev/null ||
 			die "tree filter failed: $filter_tree"
 
-- 
1.5.4.4.543.g30fdd.dirty

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

* [PATCH 14/16] filter-branch: use $SHELL_PATH instead of 'sh'
       [not found] <cover.1205356737.git.peff@peff.net>
                   ` (12 preceding siblings ...)
  2008-03-12 21:41 ` [PATCH 13/16] filter-branch: don't use xargs -0 Jeff King
@ 2008-03-12 21:41 ` Jeff King
  2008-03-12 21:42 ` [PATCH 15/16] t9112: add missing #!/bin/sh header Jeff King
  2008-03-12 21:42 ` [PATCH 16/16] t7505: use SHELL_PATH in hook Jeff King
  15 siblings, 0 replies; 59+ messages in thread
From: Jeff King @ 2008-03-12 21:41 UTC (permalink / raw)
  To: Whit Armstrong; +Cc: Junio C Hamano, git

On some systems, 'sh' isn't very friendly. In particular,
t7003 fails on Solaris because it doesn't understand $().
Instead, use the specified SHELL_PATH to run shell code.

Signed-off-by: Jeff King <peff@peff.net>
---
I think this makes sense regardless of the Solaris shell. We should be
consistent about which SHELL_PATH we use in scripts.

 Makefile             |    1 +
 git-filter-branch.sh |    2 +-
 2 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index 8e80225..bd5a623 100644
--- a/Makefile
+++ b/Makefile
@@ -874,6 +874,7 @@ common-cmds.h: $(wildcard Documentation/git-*.txt)
 $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh
 	$(QUIET_GEN)$(RM) $@ $@+ && \
 	sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
+	    -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \
 	    -e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \
 	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
 	    -e 's/@@NO_CURL@@/$(NO_CURL)/g' \
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index efef732..22b6ed4 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -309,7 +309,7 @@ while read commit parents; do
 	sed -e '1,/^$/d' <../commit | \
 		eval "$filter_msg" > ../message ||
 			die "msg filter failed: $filter_msg"
-	sh -c "$filter_commit" "git commit-tree" \
+	@SHELL_PATH@ -c "$filter_commit" "git commit-tree" \
 		$(git write-tree) $parentstr < ../message > ../map/$commit
 done <../revs
 
-- 
1.5.4.4.543.g30fdd.dirty

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

* [PATCH 15/16] t9112: add missing #!/bin/sh header
       [not found] <cover.1205356737.git.peff@peff.net>
                   ` (13 preceding siblings ...)
  2008-03-12 21:41 ` [PATCH 14/16] filter-branch: use $SHELL_PATH instead of 'sh' Jeff King
@ 2008-03-12 21:42 ` Jeff King
  2008-03-12 21:42 ` [PATCH 16/16] t7505: use SHELL_PATH in hook Jeff King
  15 siblings, 0 replies; 59+ messages in thread
From: Jeff King @ 2008-03-12 21:42 UTC (permalink / raw)
  To: Whit Armstrong; +Cc: Junio C Hamano, git


Signed-off-by: Jeff King <peff@peff.net>
---
This causes the test to barf under Solaris, even though the "correct"
behavior for me was to just start it and skip the tests.

 t/t9112-git-svn-md5less-file.sh |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/t/t9112-git-svn-md5less-file.sh b/t/t9112-git-svn-md5less-file.sh
index 08313bb..646a5f0 100755
--- a/t/t9112-git-svn-md5less-file.sh
+++ b/t/t9112-git-svn-md5less-file.sh
@@ -1,3 +1,5 @@
+#!/bin/sh
+
 test_description='test that git handles an svn repository with missing md5sums'
 
 . ./lib-git-svn.sh
-- 
1.5.4.4.543.g30fdd.dirty

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

* [PATCH 16/16] t7505: use SHELL_PATH in hook
       [not found] <cover.1205356737.git.peff@peff.net>
                   ` (14 preceding siblings ...)
  2008-03-12 21:42 ` [PATCH 15/16] t9112: add missing #!/bin/sh header Jeff King
@ 2008-03-12 21:42 ` Jeff King
  2008-03-13  7:11   ` Adam Piatyszek
  15 siblings, 1 reply; 59+ messages in thread
From: Jeff King @ 2008-03-12 21:42 UTC (permalink / raw)
  To: Whit Armstrong; +Cc: Junio C Hamano, git

The hook doesn't run properly under Solaris /bin/sh. Let's
use the SHELL_PATH the user told us about already instead.

Signed-off-by: Jeff King <peff@peff.net>
---
I discussed this before, but I never followed up with a patch. So here
it is.

 t/t7505-prepare-commit-msg-hook.sh |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/t/t7505-prepare-commit-msg-hook.sh b/t/t7505-prepare-commit-msg-hook.sh
index 7ddec99..fd67996 100755
--- a/t/t7505-prepare-commit-msg-hook.sh
+++ b/t/t7505-prepare-commit-msg-hook.sh
@@ -25,7 +25,8 @@ export FAKE_EDITOR
 HOOKDIR="$(git rev-parse --git-dir)/hooks"
 HOOK="$HOOKDIR/prepare-commit-msg"
 mkdir -p "$HOOKDIR"
-cat > "$HOOK" <<'EOF'
+echo "#!$SHELL_PATH" > "$HOOK"
+cat >> "$HOOK" <<'EOF'
 #!/bin/sh
 if test "$2" = commit; then
   source=$(git-rev-parse "$3")
-- 
1.5.4.4.543.g30fdd.dirty

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

* Re: [PATCH 11/16] config: add --literal-match option
  2008-03-12 21:40 ` [PATCH 11/16] config: add --literal-match option Jeff King
@ 2008-03-12 21:46   ` Jakub Narebski
  2008-03-13 13:24     ` Jeff King
  2008-03-12 22:34   ` Junio C Hamano
  1 sibling, 1 reply; 59+ messages in thread
From: Jakub Narebski @ 2008-03-12 21:46 UTC (permalink / raw)
  To: git

Jeff King wrote:

> +--literal-match::
> +
> +       Some invocations of git-config will limit their actions based on
> +       matching a config value to a regular expression. If this option
> +       is used, then any such matches are done as a string comparison
> +       rather than as a regular expression match.
> +

Why this option is not named --fixed-strings, as everywhere else, then?

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

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

* Re: [PATCH 04/16] grep portability fix: don't use "-e" or "-q"
  2008-03-12 21:32 ` [PATCH 04/16] grep portability fix: don't use "-e" or "-q" Jeff King
@ 2008-03-12 22:10   ` Junio C Hamano
  2008-03-12 22:45     ` Junio C Hamano
  0 siblings, 1 reply; 59+ messages in thread
From: Junio C Hamano @ 2008-03-12 22:10 UTC (permalink / raw)
  To: Jeff King; +Cc: Whit Armstrong, git

Jeff King <peff@peff.net> writes:

> System V versions of grep (such as Solaris /usr/bin/grep)
> don't understand either of these options. git's usage of

It might be fair for other System V people to qualify the above statement
with "Historic System V" (personally I felt that Solaris without xpg4 was
unusable, and I wish you luck tackling it).

More seriously, you would need to disable "when grepping work-tree,
running the native grep is faster and just as capable" optimization in
builtin-grep.c::grep_cache().  Treat these problematic System V platforms
as if they are not __unix__.

I am surprised that you did not have issues with "tail -n$number".
Historic way to spell it was "tail -$number" wasn't it?

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

* Re: [PATCH 06/16] add test_cmp function for test scripts
  2008-03-12 21:36 ` [PATCH 06/16] add test_cmp function for test scripts Jeff King
@ 2008-03-12 22:12   ` Junio C Hamano
  2008-03-13 12:08     ` Jeff King
  0 siblings, 1 reply; 59+ messages in thread
From: Junio C Hamano @ 2008-03-12 22:12 UTC (permalink / raw)
  To: Jeff King; +Cc: Whit Armstrong, git

I think I have an earlier round of this in 'pu'.

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

* Re: [PATCH 10/16] add NO_EXTERNAL_GREP build option
  2008-03-12 21:39 ` [PATCH 10/16] add NO_EXTERNAL_GREP build option Jeff King
@ 2008-03-12 22:30   ` Junio C Hamano
  2008-03-13 12:10     ` Jeff King
  2008-03-13  7:50   ` Johannes Sixt
  2008-03-13  7:56   ` Junio C Hamano
  2 siblings, 1 reply; 59+ messages in thread
From: Junio C Hamano @ 2008-03-12 22:30 UTC (permalink / raw)
  To: Jeff King; +Cc: Whit Armstrong, git

Jeff King <peff@peff.net> writes:

> Previously, we just chose whether to allow external grep
> based on the __unix__ define. However, there are systems
> which define this macro but which have an inferior group
> (e.g., one that does not support all options used by t7002).
> This allows users to accept the potential speed penalty to
> get a more consistent grep experience (and to pass the
> testsuite).
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> This might have fallouts for msysgit (i.e., they need to define
> NO_EXTERNAL_GREP instead of relying on __unix__ not being defined).
> ...
> diff --git a/builtin-grep.c b/builtin-grep.c
> index f4f4ecb..f215b28 100644
> --- a/builtin-grep.c
> +++ b/builtin-grep.c
> @@ -153,7 +153,7 @@ static int grep_file(struct grep_opt *opt, const char *filename)
>  	return i;
>  }
>  
> -#ifdef __unix__
> +#ifndef NO_EXTERNAL_GREP

Perhaps place

    #ifndef NO_EXTERNAL_GREP
    #ifndef __unix__
    #define NO_EXTERNAL_GREP 1
    #else
    #define NO_EXTERNAL_GREP 0
    #endif
    #endif

in git-compat-util.h, and make the in-code reference to

    #if NO_EXTERNAL_GREP
            ... optimization using external grep ...
    #endif

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

* Re: [PATCH 11/16] config: add --literal-match option
  2008-03-12 21:40 ` [PATCH 11/16] config: add --literal-match option Jeff King
  2008-03-12 21:46   ` Jakub Narebski
@ 2008-03-12 22:34   ` Junio C Hamano
  2008-03-13 12:42     ` Jeff King
  1 sibling, 1 reply; 59+ messages in thread
From: Junio C Hamano @ 2008-03-12 22:34 UTC (permalink / raw)
  To: Jeff King; +Cc: Whit Armstrong, git

Jeff King <peff@peff.net> writes:

> When limiting the values to be set (or returned), the user
> previously had the option of specifying a regex. In some
> cases, however, they may want to find a literal value. The
> option --literal-match converts any matching regex into a
> literal string comparison.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> I think this is a nice addition regardless, but it is used by the next
> patch.
>
> The patch is about twice as long as it needs to be since getting and
> setting in builtin-config follow two almost-the-same parallel codepaths.
> I suspect this could be cleaned up, but I didn't look too closely.

I think that is a good new feature to propose.

Historically, the config_set_multivar() function has been one of the most
buggy part of the then-current codebase.  It might be a good idea to
clean-up first and then enhance.

But in either case I am quite reluctant to touch this part of the code
right now before 1.5.5, especially without extra sets of eyeballs.

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

* Re: [PATCH 12/16] git-submodule: avoid sed input with no newline
  2008-03-12 21:40 ` [PATCH 12/16] git-submodule: avoid sed input with no newline Jeff King
@ 2008-03-12 22:41   ` Junio C Hamano
  2008-03-13 12:46     ` Jeff King
  0 siblings, 1 reply; 59+ messages in thread
From: Junio C Hamano @ 2008-03-12 22:41 UTC (permalink / raw)
  To: Jeff King; +Cc: Whit Armstrong, git

Jeff King <peff@peff.net> writes:

> Some versions of sed don't like this, and give no output at
> all. Instead, we can use git-config to pare down the matches
> for us.

Good use of --literal.

But doesn't this make you wonder if "--literal-match --get-regexp" is
quite a strange combination?  "Literal" covers the value part but the key
is still regexp (and we do want it to behave that way).  However, maybe we
would want to also allow "give entries whose key is this literal key and
whose value matches this regexp"?

> +	name=$(git config --literal-match -f .gitmodules \
> +		--get-regexp 'submodule\..*\.path$' "$1" |
> +		sed -e 's/submodule\.//' -e 's/\.path.*//')
> +	test -z "$name" &&
> +	die "No submodule mapping found in .gitmodules for path '$path'"
> +	echo "$name"
>  }

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

* Re: [PATCH 04/16] grep portability fix: don't use "-e" or "-q"
  2008-03-12 22:10   ` Junio C Hamano
@ 2008-03-12 22:45     ` Junio C Hamano
  0 siblings, 0 replies; 59+ messages in thread
From: Junio C Hamano @ 2008-03-12 22:45 UTC (permalink / raw)
  To: Jeff King; +Cc: Whit Armstrong, git

Junio C Hamano <gitster@pobox.com> writes:

> Jeff King <peff@peff.net> writes:
>
>> System V versions of grep (such as Solaris /usr/bin/grep)
>> don't understand either of these options. git's usage of
>
> It might be fair for other System V people to qualify the above statement
> with "Historic System V" (personally I felt that Solaris without xpg4 was
> unusable, and I wish you luck tackling it).
>
> More seriously, you would need to disable "when grepping work-tree,
> running the native grep is faster and just as capable" optimization in
> builtin-grep.c::grep_cache().  Treat these problematic System V platforms
> as if they are not __unix__.
>
> I am surprised that you did not have issues with "tail -n$number".
> Historic way to spell it was "tail -$number" wasn't it?

Heh, I notice you had to deal with these issues in the later patches ;-)

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

* Re: [PATCH 08/16] t4200: use cut instead of sed
  2008-03-12 21:37 ` [PATCH 08/16] t4200: use cut instead of sed Jeff King
@ 2008-03-13  4:52   ` Junio C Hamano
  2008-03-13 12:59     ` Jeff King
  0 siblings, 1 reply; 59+ messages in thread
From: Junio C Hamano @ 2008-03-13  4:52 UTC (permalink / raw)
  To: Jeff King; +Cc: Whit Armstrong, git

Jeff King <peff@peff.net> writes:

> Some versions of sed (like the one on Solaris) don't like to
> match literal tabs, and simply print nothing. Instead, let's
> use cut.

> -sha1=$(sed -e 's/	.*//' .git/rr-cache/MERGE_RR)

This is a bit hard to believe.  On one of my ancient Sun box:

$ uname -a
SunOS sic.twinsun.com 5.8 Generic_117350-45 sun4u sparc SUNW,UltraSPARC-IIi-Engine
$ ls -l /bin/sed
-r-xr-xr-x   1 root     bin        28748 Aug  2  2005 /bin/sed

the above "sed" does not misbehave (/bin/sh does, of course, on $(...),
but that is a different story).

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

* Re: [PATCH 16/16] t7505: use SHELL_PATH in hook
  2008-03-12 21:42 ` [PATCH 16/16] t7505: use SHELL_PATH in hook Jeff King
@ 2008-03-13  7:11   ` Adam Piatyszek
  2008-03-13 13:00     ` Jeff King
  0 siblings, 1 reply; 59+ messages in thread
From: Adam Piatyszek @ 2008-03-13  7:11 UTC (permalink / raw)
  To: Jeff King; +Cc: Whit Armstrong, Junio C Hamano, git

Hi!

* Jeff King [12 III 2008 22:42]:
> The hook doesn't run properly under Solaris /bin/sh. Let's
> use the SHELL_PATH the user told us about already instead.
[...]
> --- a/t/t7505-prepare-commit-msg-hook.sh
> +++ b/t/t7505-prepare-commit-msg-hook.sh
> @@ -25,7 +25,8 @@ export FAKE_EDITOR
>  HOOKDIR="$(git rev-parse --git-dir)/hooks"
>  HOOK="$HOOKDIR/prepare-commit-msg"
>  mkdir -p "$HOOKDIR"
> -cat > "$HOOK" <<'EOF'
> +echo "#!$SHELL_PATH" > "$HOOK"
> +cat >> "$HOOK" <<'EOF'
>  #!/bin/sh
    ^^^^^^^^^
The above line should be removed in my humble opinion.

BR,
/Adam

-- 
.:.  Adam Piatyszek (ediap)  .:.....................................:.
.:.  ediap@users.sourceforge.net  .:................................:.

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

* Re: [PATCH 01/16] tr portability fixes
  2008-03-12 21:29 ` [PATCH 01/16] tr portability fixes Jeff King
@ 2008-03-13  7:32   ` Johannes Sixt
  2008-03-13 13:06     ` Jeff King
  0 siblings, 1 reply; 59+ messages in thread
From: Johannes Sixt @ 2008-03-13  7:32 UTC (permalink / raw)
  To: Jeff King; +Cc: Whit Armstrong, Junio C Hamano, git

Jeff King schrieb:
> We can mostly get around this by just using the bracket form
> for both sets, as in:
> 
>   tr '[A-Z] '[a-z]'
> 
> in which case POSIX interpets this as "'[' becomes '['",
> which is OK.
> 
> However, this doesn't work with multiple sequences, like:
> 
>   # rot13
>   tr '[A-Z][a-z]' '[N-Z][A-M][n-z][a-m]'

Not that it matters a lot, but I wonder whether

   tr '[A-M][N-Z][a-m][n-z]' '[N-Z][A-M][n-z][a-m]'

would have done the trick.

-- Hannes

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

* Re: [PATCH 02/16] t0050: perl portability fix
  2008-03-12 21:30 ` [PATCH 02/16] t0050: perl portability fix Jeff King
@ 2008-03-13  7:38   ` Johannes Sixt
  0 siblings, 0 replies; 59+ messages in thread
From: Johannes Sixt @ 2008-03-13  7:38 UTC (permalink / raw)
  To: Jeff King; +Cc: Whit Armstrong, Junio C Hamano, git

Jeff King schrieb:
> Older versions of perl (such as 5.005) don't understand -CO, nor
> do they understand the "U" pack specifier. Instead of using perl,
> let's just printf the binary bytes we are interested in.
> 
> Signed-off-by: Jeff King <peff@peff.net>

Yay! We need this on Windows with MSYS's perl, too. Works great:

Tested-by: Johannes Sixt <johannes.sixt@telecom.at>

-- Hannes

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

* Re: [PATCH 10/16] add NO_EXTERNAL_GREP build option
  2008-03-12 21:39 ` [PATCH 10/16] add NO_EXTERNAL_GREP build option Jeff King
  2008-03-12 22:30   ` Junio C Hamano
@ 2008-03-13  7:50   ` Johannes Sixt
  2008-03-13 12:41     ` Jeff King
  2008-03-13  7:56   ` Junio C Hamano
  2 siblings, 1 reply; 59+ messages in thread
From: Johannes Sixt @ 2008-03-13  7:50 UTC (permalink / raw)
  To: Jeff King; +Cc: Whit Armstrong, Junio C Hamano, git

Jeff King schrieb:
> Previously, we just chose whether to allow external grep
> based on the __unix__ define. However, there are systems
> which define this macro but which have an inferior group
> (e.g., one that does not support all options used by t7002).
> This allows users to accept the potential speed penalty to
> get a more consistent grep experience (and to pass the
> testsuite).
> 
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> This might have fallouts for msysgit (i.e., they need to define
> NO_EXTERNAL_GREP instead of relying on __unix__ not being defined).

You name it. Would you mind converting exec_grep() to use run_command(),
too? Or better inline it since it won't do a lot more than run_command()?
That way we at least won't get a broken git when I merge git.git that has
this patch.

Thanks,
-- Hannes

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

* Re: [PATCH 10/16] add NO_EXTERNAL_GREP build option
  2008-03-12 21:39 ` [PATCH 10/16] add NO_EXTERNAL_GREP build option Jeff King
  2008-03-12 22:30   ` Junio C Hamano
  2008-03-13  7:50   ` Johannes Sixt
@ 2008-03-13  7:56   ` Junio C Hamano
  2 siblings, 0 replies; 59+ messages in thread
From: Junio C Hamano @ 2008-03-13  7:56 UTC (permalink / raw)
  To: Jeff King; +Cc: Whit Armstrong, git

There is one more reason other than you said in the Makefile patch one
might want to avoid external grep.  Cygwin may have perfectly well working
grep, but the reason they avoid external grep is because forking is too
slow there.

---

 Makefile       |    5 ++---
 builtin-grep.c |   12 ++++++++++--
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index 8e80225..bc46fd4 100644
--- a/Makefile
+++ b/Makefile
@@ -149,8 +149,8 @@ all::
 # recommended if Git triggers O(n^2) behavior in your platform's qsort().
 #
 # Define NO_EXTERNAL_GREP if you don't want "git grep" to ever call
-# your external grep (e.g., if your system lacks grep, or if its grep is
-# not very featureful).
+# your external grep (e.g., if your system lacks grep, if its grep is
+# broken, or spawning external process is slower than built-in grep git has).
 
 GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -470,7 +470,6 @@ ifeq ($(uname_O),Cygwin)
 	NO_STRCASESTR = YesPlease
 	NO_MEMMEM = YesPlease
 	NO_SYMLINK_HEAD = YesPlease
-	NO_EXTERNAL_GREP = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
 	NO_TRUSTABLE_FILEMODE = UnfortunatelyYes
diff --git a/builtin-grep.c b/builtin-grep.c
index f215b28..ef29910 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -12,6 +12,14 @@
 #include "builtin.h"
 #include "grep.h"
 
+#ifndef NO_EXTERNAL_GREP
+#ifdef __unix__
+#define NO_EXTERNAL_GREP 0
+#else
+#define NO_EXTERNAL_GREP 1
+#endif
+#endif
+
 /*
  * git grep pathspecs are somewhat different from diff-tree pathspecs;
  * pathname wildcards are allowed.
@@ -153,7 +161,7 @@ static int grep_file(struct grep_opt *opt, const char *filename)
 	return i;
 }
 
-#ifndef NO_EXTERNAL_GREP
+#if !NO_EXTERNAL_GREP
 static int exec_grep(int argc, const char **argv)
 {
 	pid_t pid;
@@ -372,7 +380,7 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached)
 	int nr;
 	read_cache();
 
-#ifndef NO_EXTERNAL_GREP
+#if !NO_EXTERNAL_GREP
 	/*
 	 * Use the external "grep" command for the case where
 	 * we grep through the checked-out files. It tends to

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

* Re: [PATCH 03/16] more tr portability test script fixes
  2008-03-12 21:31 ` [PATCH 03/16] more tr portability test script fixes Jeff King
@ 2008-03-13  8:28   ` Frank Lichtenheld
  2008-03-13 13:09     ` Jeff King
  2008-03-18 22:23   ` Alex Riesen
  1 sibling, 1 reply; 59+ messages in thread
From: Frank Lichtenheld @ 2008-03-13  8:28 UTC (permalink / raw)
  To: Jeff King; +Cc: Whit Armstrong, Junio C Hamano, git

On Wed, Mar 12, 2008 at 05:31:06PM -0400, Jeff King wrote:
> --- a/t/diff-lib.sh
> +++ b/t/diff-lib.sh
> @@ -21,8 +21,8 @@ compare_diff_raw_z () {
>      # Also we do not check SHA1 hash generation in this test, which
>      # is a job for t0000-basic.sh
>  
> -    tr '\000' '\012' <"$1" | sed -e "$sanitize_diff_raw_z" >.tmp-1
> -    tr '\000' '\012' <"$2" | sed -e "$sanitize_diff_raw_z" >.tmp-2
> +    perl -pe 'y/\000/\012/' <"$1" | sed -e "$sanitize_diff_raw_z" >.tmp-1
> +    perl -pe 'y/\000/\012/' <"$2" | sed -e "$sanitize_diff_raw_z" >.tmp-2

It might make sense performance-wise to integrate the job of the sed call into the perl
call here. Haven't tested it, though.

Gruesse,
-- 
Frank Lichtenheld <frank@lichtenheld.de>
www: http://www.djpig.de/

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

* Re: [PATCH 06/16] add test_cmp function for test scripts
  2008-03-12 22:12   ` Junio C Hamano
@ 2008-03-13 12:08     ` Jeff King
  2008-03-13 20:48       ` Junio C Hamano
  0 siblings, 1 reply; 59+ messages in thread
From: Jeff King @ 2008-03-13 12:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Whit Armstrong, git

On Wed, Mar 12, 2008 at 03:12:45PM -0700, Junio C Hamano wrote:

> I think I have an earlier round of this in 'pu'.

Oops, so you do. I remember discussing it but didn't recall a patch
coming out of it.

I see you have queued mine now in pu over yours. I actually think yours
looks a little nicer (you sanity-check the comparator, and you are more
careful with stdin (something that I considered, but figured we could
add if something actually broke)).

Any reason not to keep your existing one over mine?

-Peff

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

* Re: [PATCH 10/16] add NO_EXTERNAL_GREP build option
  2008-03-12 22:30   ` Junio C Hamano
@ 2008-03-13 12:10     ` Jeff King
  0 siblings, 0 replies; 59+ messages in thread
From: Jeff King @ 2008-03-13 12:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Whit Armstrong, git

On Wed, Mar 12, 2008 at 03:30:05PM -0700, Junio C Hamano wrote:

> Perhaps place
> 
>     #ifndef NO_EXTERNAL_GREP
>     #ifndef __unix__
>     #define NO_EXTERNAL_GREP 1
>     #else
>     #define NO_EXTERNAL_GREP 0
>     #endif
>     #endif
> 
> in git-compat-util.h, and make the in-code reference to
> 
>     #if NO_EXTERNAL_GREP
>             ... optimization using external grep ...
>     #endif

I agree that is much nicer. Looks like you have already marked it up in
pu; what is there looks sane.

-Peff

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

* Re: [PATCH 10/16] add NO_EXTERNAL_GREP build option
  2008-03-13  7:50   ` Johannes Sixt
@ 2008-03-13 12:41     ` Jeff King
  2008-03-13 13:59       ` Johannes Sixt
  0 siblings, 1 reply; 59+ messages in thread
From: Jeff King @ 2008-03-13 12:41 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Whit Armstrong, Junio C Hamano, git

On Thu, Mar 13, 2008 at 08:50:24AM +0100, Johannes Sixt wrote:

> > This might have fallouts for msysgit (i.e., they need to define
> > NO_EXTERNAL_GREP instead of relying on __unix__ not being defined).
> 
> You name it. Would you mind converting exec_grep() to use run_command(),
> too? Or better inline it since it won't do a lot more than run_command()?
> That way we at least won't get a broken git when I merge git.git that has
> this patch.

Junio's fixups should restore the automagic behavior, so you shouldn't
see any problems now, I think. But the run_command cleanup is sensible.

This is on top of what Junio has in pu.

-- >8 --
use run_command for external grep

The behavior should be identical, but there's no good reason
not to use our abstraction library.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin-grep.c |   32 ++++----------------------------
 1 files changed, 4 insertions(+), 28 deletions(-)

diff --git a/builtin-grep.c b/builtin-grep.c
index ef29910..44d9bac 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -11,6 +11,7 @@
 #include "tree-walk.h"
 #include "builtin.h"
 #include "grep.h"
+#include "run-command.h"
 
 #ifndef NO_EXTERNAL_GREP
 #ifdef __unix__
@@ -162,32 +163,6 @@ static int grep_file(struct grep_opt *opt, const char *filename)
 }
 
 #if !NO_EXTERNAL_GREP
-static int exec_grep(int argc, const char **argv)
-{
-	pid_t pid;
-	int status;
-
-	argv[argc] = NULL;
-	pid = fork();
-	if (pid < 0)
-		return pid;
-	if (!pid) {
-		execvp("grep", (char **) argv);
-		exit(255);
-	}
-	while (waitpid(pid, &status, 0) < 0) {
-		if (errno == EINTR)
-			continue;
-		return -1;
-	}
-	if (WIFEXITED(status)) {
-		if (!WEXITSTATUS(status))
-			return 1;
-		return 0;
-	}
-	return -1;
-}
-
 #define MAXARGS 1000
 #define ARGBUF 4096
 #define push_arg(a) do { \
@@ -253,7 +228,8 @@ static int flush_grep(struct grep_opt *opt,
 		argc -= 2;
 	}
 
-	status = exec_grep(argc, argv);
+	argv[argc] = NULL;
+	status = run_command_v_opt(argv, 0);
 
 	if (kept_0) {
 		/*
@@ -264,7 +240,7 @@ static int flush_grep(struct grep_opt *opt,
 		argv[arg0++] = kept_0;
 		argv[arg0] = argv[argc+1];
 	}
-	return status;
+	return status == 0 ? 1 : -1;
 }
 
 static int external_grep(struct grep_opt *opt, const char **paths, int cached)
-- 
1.5.4.4.553.g83e84.dirty

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

* Re: [PATCH 11/16] config: add --literal-match option
  2008-03-12 22:34   ` Junio C Hamano
@ 2008-03-13 12:42     ` Jeff King
  0 siblings, 0 replies; 59+ messages in thread
From: Jeff King @ 2008-03-13 12:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Whit Armstrong, git

On Wed, Mar 12, 2008 at 03:34:51PM -0700, Junio C Hamano wrote:

> > The patch is about twice as long as it needs to be since getting and
> > setting in builtin-config follow two almost-the-same parallel codepaths.
> > I suspect this could be cleaned up, but I didn't look too closely.
> 
> I think that is a good new feature to propose.
> 
> Historically, the config_set_multivar() function has been one of the most
> buggy part of the then-current codebase.  It might be a good idea to
> clean-up first and then enhance.
> 
> But in either case I am quite reluctant to touch this part of the code
> right now before 1.5.5, especially without extra sets of eyeballs.

That sounds sensible. I will throw this on the backburner until
post-1.5.5, then, and try to respin it with some config cleanups.

-Peff

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

* Re: [PATCH 12/16] git-submodule: avoid sed input with no newline
  2008-03-12 22:41   ` Junio C Hamano
@ 2008-03-13 12:46     ` Jeff King
  0 siblings, 0 replies; 59+ messages in thread
From: Jeff King @ 2008-03-13 12:46 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Whit Armstrong, git

On Wed, Mar 12, 2008 at 03:41:03PM -0700, Junio C Hamano wrote:

> Good use of --literal.
> 
> But doesn't this make you wonder if "--literal-match --get-regexp" is
> quite a strange combination?  "Literal" covers the value part but the key
> is still regexp (and we do want it to behave that way).  However, maybe we
> would want to also allow "give entries whose key is this literal key and
> whose value matches this regexp"?

git config --get?

The problem is that we are matching on two elements: key and value. The
matching style of the key element is chosen by --get versus
--get-regexp.  However, the value_regex is selected implicitly by its
presence, so there is no room to modify the flag in the same way.
Perhaps --literal-value-match or just --literal-value would be a better
name.

-Peff

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

* Re: [PATCH 08/16] t4200: use cut instead of sed
  2008-03-13  4:52   ` Junio C Hamano
@ 2008-03-13 12:59     ` Jeff King
  2008-03-13 18:00       ` Junio C Hamano
  0 siblings, 1 reply; 59+ messages in thread
From: Jeff King @ 2008-03-13 12:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Whit Armstrong, git

On Wed, Mar 12, 2008 at 09:52:18PM -0700, Junio C Hamano wrote:

> > Some versions of sed (like the one on Solaris) don't like to
> > match literal tabs, and simply print nothing. Instead, let's
> > use cut.
> 
> > -sha1=$(sed -e 's/	.*//' .git/rr-cache/MERGE_RR)
> 
> This is a bit hard to believe.  On one of my ancient Sun box:

Ah, sorry. I tested this line by hand, found it didn't work, and
stupidly jumped to the assumption that it was the literal tab (that
being the only interesting thing in the input).

But the actual problem is that MERGE_RR lacks a trailing newline. I
don't see any code to add newlines, even though it seems possible that
we will write out several paths. So I think we need a newline here:

diff --git a/builtin-rerere.c b/builtin-rerere.c
index c607aad..e4a1dc1 100644
--- a/builtin-rerere.c
+++ b/builtin-rerere.c
@@ -58,7 +58,8 @@ static int write_rr(struct path_list *rr, int out_fd)
 		int length = strlen(path) + 1;
 		if (write_in_full(out_fd, rr->items[i].util, 40) != 40 ||
 		    write_in_full(out_fd, "\t", 1) != 1 ||
-		    write_in_full(out_fd, path, length) != length)
+		    write_in_full(out_fd, path, length) != length ||
+		    write_in_full(out_fd, "\n", 1) != 1)
 			die("unable to write rerere record");
 	}
 	if (commit_lock_file(&write_lock) != 0)

And unless I am missing something, rerere on multiple paths is very
broken (but that seems weird, since this code is so old).

-Peff

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

* Re: [PATCH 16/16] t7505: use SHELL_PATH in hook
  2008-03-13  7:11   ` Adam Piatyszek
@ 2008-03-13 13:00     ` Jeff King
  0 siblings, 0 replies; 59+ messages in thread
From: Jeff King @ 2008-03-13 13:00 UTC (permalink / raw)
  To: Adam Piatyszek; +Cc: Whit Armstrong, Junio C Hamano, git

On Thu, Mar 13, 2008 at 08:11:24AM +0100, Adam Piatyszek wrote:

>> -cat > "$HOOK" <<'EOF'
>> +echo "#!$SHELL_PATH" > "$HOOK"
>> +cat >> "$HOOK" <<'EOF'
>>  #!/bin/sh
>    ^^^^^^^^^
> The above line should be removed in my humble opinion.

Oops, yes. I did delete it, but then accidentally editor-typo'd it back
into existence. Good eyes.

-Peff

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

* Re: [PATCH 01/16] tr portability fixes
  2008-03-13  7:32   ` Johannes Sixt
@ 2008-03-13 13:06     ` Jeff King
  0 siblings, 0 replies; 59+ messages in thread
From: Jeff King @ 2008-03-13 13:06 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Whit Armstrong, Junio C Hamano, git

On Thu, Mar 13, 2008 at 08:32:37AM +0100, Johannes Sixt wrote:

> > However, this doesn't work with multiple sequences, like:
> > 
> >   # rot13
> >   tr '[A-Z][a-z]' '[N-Z][A-M][n-z][a-m]'
> 
> Not that it matters a lot, but I wonder whether
> 
>    tr '[A-M][N-Z][a-m][n-z]' '[N-Z][A-M][n-z][a-m]'
> 
> would have done the trick.

For the record, it does on Solaris (and I really can't imagine it _not_
working anywhere else, but then I couldn't imagine my example not
working, either. ;) ).

-Peff

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

* Re: [PATCH 03/16] more tr portability test script fixes
  2008-03-13  8:28   ` Frank Lichtenheld
@ 2008-03-13 13:09     ` Jeff King
  0 siblings, 0 replies; 59+ messages in thread
From: Jeff King @ 2008-03-13 13:09 UTC (permalink / raw)
  To: Frank Lichtenheld; +Cc: Whit Armstrong, Junio C Hamano, git

On Thu, Mar 13, 2008 at 09:28:07AM +0100, Frank Lichtenheld wrote:

> > -    tr '\000' '\012' <"$1" | sed -e "$sanitize_diff_raw_z" >.tmp-1
> > -    tr '\000' '\012' <"$2" | sed -e "$sanitize_diff_raw_z" >.tmp-2
> > +    perl -pe 'y/\000/\012/' <"$1" | sed -e "$sanitize_diff_raw_z" >.tmp-1
> > +    perl -pe 'y/\000/\012/' <"$2" | sed -e "$sanitize_diff_raw_z" >.tmp-2
> 
> It might make sense performance-wise to integrate the job of the sed
> call into the perl call here. Haven't tested it, though.

Probably. My changes were pretty mechanical, and I tried to keep them as
small as possible.

I'm not sure we care too much about an extra fork in a test script for
performance, though it would probably end up more readable.

-Peff

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

* Re: [PATCH 11/16] config: add --literal-match option
  2008-03-12 21:46   ` Jakub Narebski
@ 2008-03-13 13:24     ` Jeff King
  0 siblings, 0 replies; 59+ messages in thread
From: Jeff King @ 2008-03-13 13:24 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git

On Wed, Mar 12, 2008 at 10:46:43PM +0100, Jakub Narebski wrote:

> > +--literal-match::
> > +
> > +       Some invocations of git-config will limit their actions based on
> > +       matching a config value to a regular expression. If this option
> > +       is used, then any such matches are done as a string comparison
> > +       rather than as a regular expression match.
> > +
> 
> Why this option is not named --fixed-strings, as everywhere else, then?

Because I never use --fixed-strings anywhere else and didn't think of
it? :)

I think, though, that there is a key difference, and using that name
would lead to confusion. In grep, --fixed-strings means "find this fixed
_substring_ within the input" whereas this option means "find the value
that is byte-for-byte equal to this."

-Peff

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

* Re: [PATCH 10/16] add NO_EXTERNAL_GREP build option
  2008-03-13 12:41     ` Jeff King
@ 2008-03-13 13:59       ` Johannes Sixt
  2008-03-13 14:04         ` Jeff King
  0 siblings, 1 reply; 59+ messages in thread
From: Johannes Sixt @ 2008-03-13 13:59 UTC (permalink / raw)
  To: Jeff King; +Cc: Whit Armstrong, Junio C Hamano, git

Jeff King schrieb:
> Junio's fixups should restore the automagic behavior, so you shouldn't
> see any problems now, I think. But the run_command cleanup is sensible.

Thanks. But it turns out that things are not /that/ trivial. We better
live with Junio's fixup.

> -	while (waitpid(pid, &status, 0) < 0) {
> -		if (errno == EINTR)
> -			continue;
> -		return -1;
> -	}
> -	if (WIFEXITED(status)) {
> -		if (!WEXITSTATUS(status))
> -			return 1;
> -		return 0;
> -	}
> -	return -1;
> -}
> -
...
> +	status = run_command_v_opt(argv, 0);
...
> +	return status == 0 ? 1 : -1;

grep can return 0 (success, something found), 1 (nothing found), and other
values for "real" failures like usage errors or crashes. This conditional
throws the latter two into the same pot, which makes git-grep unable to
distinguish "nothing found" from failure; cf. the call sites of
flush_grep(), where want to set the flag 'hit'.

-- Hannes

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

* Re: [PATCH 10/16] add NO_EXTERNAL_GREP build option
  2008-03-13 13:59       ` Johannes Sixt
@ 2008-03-13 14:04         ` Jeff King
  2008-03-13 14:09           ` Johannes Sixt
  0 siblings, 1 reply; 59+ messages in thread
From: Jeff King @ 2008-03-13 14:04 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Whit Armstrong, Junio C Hamano, git

On Thu, Mar 13, 2008 at 02:59:08PM +0100, Johannes Sixt wrote:

> > +	status = run_command_v_opt(argv, 0);
> ...
> > +	return status == 0 ? 1 : -1;
> 
> grep can return 0 (success, something found), 1 (nothing found), and other
> values for "real" failures like usage errors or crashes. This conditional
> throws the latter two into the same pot, which makes git-grep unable to
> distinguish "nothing found" from failure; cf. the call sites of
> flush_grep(), where want to set the flag 'hit'.

I noticed that, as well, while writing it. But if you look at
external_grep, it just lumps the two cases together anyway. It only ever
compares "0 < status", so I think the behavior should be identical.

Which isn't to say that the code couldn't be _improved_ by noting grep
failures and crashes, but I don't think it is any worse.

-Peff

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

* Re: [PATCH 10/16] add NO_EXTERNAL_GREP build option
  2008-03-13 14:04         ` Jeff King
@ 2008-03-13 14:09           ` Johannes Sixt
  0 siblings, 0 replies; 59+ messages in thread
From: Johannes Sixt @ 2008-03-13 14:09 UTC (permalink / raw)
  To: Jeff King; +Cc: Whit Armstrong, Junio C Hamano, git

Jeff King schrieb:
> On Thu, Mar 13, 2008 at 02:59:08PM +0100, Johannes Sixt wrote:
> 
>>> +	status = run_command_v_opt(argv, 0);
>> ...
>>> +	return status == 0 ? 1 : -1;
>> grep can return 0 (success, something found), 1 (nothing found), and other
>> values for "real" failures like usage errors or crashes. This conditional
>> throws the latter two into the same pot, which makes git-grep unable to
>> distinguish "nothing found" from failure; cf. the call sites of
>> flush_grep(), where want to set the flag 'hit'.
> 
> I noticed that, as well, while writing it. But if you look at
> external_grep, it just lumps the two cases together anyway. It only ever
> compares "0 < status", so I think the behavior should be identical.

Yes, indeed. I missed that status < 0 is not checked for.

-- Hannes

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

* Re: [PATCH 08/16] t4200: use cut instead of sed
  2008-03-13 12:59     ` Jeff King
@ 2008-03-13 18:00       ` Junio C Hamano
  2008-03-14 21:40         ` Jeff King
  0 siblings, 1 reply; 59+ messages in thread
From: Junio C Hamano @ 2008-03-13 18:00 UTC (permalink / raw)
  To: Jeff King; +Cc: Whit Armstrong, git

Jeff King <peff@peff.net> writes:

> But the actual problem is that MERGE_RR lacks a trailing newline. I
> don't see any code to add newlines, even though it seems possible that
> we will write out several paths. So I think we need a newline here:
>
> diff --git a/builtin-rerere.c b/builtin-rerere.c
> index c607aad..e4a1dc1 100644
> --- a/builtin-rerere.c
> +++ b/builtin-rerere.c
> @@ -58,7 +58,8 @@ static int write_rr(struct path_list *rr, int out_fd)
>  		int length = strlen(path) + 1;
>  		if (write_in_full(out_fd, rr->items[i].util, 40) != 40 ||
>  		    write_in_full(out_fd, "\t", 1) != 1 ||
> -		    write_in_full(out_fd, path, length) != length)
> +		    write_in_full(out_fd, path, length) != length ||
> +		    write_in_full(out_fd, "\n", 1) != 1)
>  			die("unable to write rerere record");
>  	}
>  	if (commit_lock_file(&write_lock) != 0)

No, check 3f43d72392b6c0477debd7edbd49bae9b7f41e60^:git-rerere.perl; the
records in this file are supposed to be NUL terminated (the paths are
allowed to have LF in them).

> And unless I am missing something, rerere on multiple paths is very
> broken (but that seems weird, since this code is so old).

I do not recall offhand an example of multi-path rerere working or not
working correctly, but I am not surprised if the C re-implementation is
broken.  Will need to check.

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

* Re: [PATCH 06/16] add test_cmp function for test scripts
  2008-03-13 12:08     ` Jeff King
@ 2008-03-13 20:48       ` Junio C Hamano
  0 siblings, 0 replies; 59+ messages in thread
From: Junio C Hamano @ 2008-03-13 20:48 UTC (permalink / raw)
  To: Jeff King; +Cc: Whit Armstrong, git

Jeff King <peff@peff.net> writes:

> On Wed, Mar 12, 2008 at 03:12:45PM -0700, Junio C Hamano wrote:
>
>> I think I have an earlier round of this in 'pu'.
>
> Oops, so you do. I remember discussing it but didn't recall a patch
> coming out of it.
>
> I see you have queued mine now in pu over yours. I actually think yours
> looks a little nicer (you sanity-check the comparator, and you are more
> careful with stdin (something that I considered, but figured we could
> add if something actually broke)).
>
> Any reason not to keep your existing one over mine?

Yours is much simpler.

And it is tested on the field, so if it ever breaks I have somebody else
to blame ;-)

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

* [PATCH] t/t6000lib.sh: tr portability fix fix
  2008-03-12 21:38 ` [PATCH 09/16] t6000lib: tr portability fix Jeff King
@ 2008-03-14 20:47   ` Brandon Casey
  2008-03-14 20:54     ` Jeff King
  0 siblings, 1 reply; 59+ messages in thread
From: Brandon Casey @ 2008-03-14 20:47 UTC (permalink / raw)
  To: Jeff King; +Cc: armstrong.whit, Junio C Hamano, Git Mailing List

Some versions of tr have a problem with character sets which begin with
multiple dashes and attempt to interpret them as long options. Use the
'--' notation to signal the end of command line options.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---


Jeff King wrote:
> Some versions of tr complain if the number of characters in
> both sets isn't the same. So here we must manually expand
> the dashes in set2.
> 
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> This almost makes me want to just use sed instead. But quoting that line
> noise would probably make it less readable.

I get the following error on t6002-rev-list-bisect.sh:

*   ok 31: bisection diff --bisect u3 ^U <= 0
*   ok 32: bisection diff --bisect u4 ^U <= 0
*   ok 33: bisection diff --bisect u5 ^U <= 0
tr: unrecognized option `------------------------------'
Try `tr --help' for more information.
* FAIL 34: --bisect l5 ^root
        check_output  "git rev-list $_bisect_option l5 ^root"
tr: unrecognized option `------------------------------'
Try `tr --help' for more information.
* FAIL 35: --bisect l5 ^root ^c3
...


This is tr version 5.2.1.

This patch fixes things. If the dashdash notation is not portable, then
backslashing each dash also works. i.e. '\-\-\-\-.. etc. but as you
mentioned something like that is less readable, but possibly not as bad
as a sed version.

-brandon


 t/t6000lib.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/t/t6000lib.sh b/t/t6000lib.sh
index b69f7c4..aac6a31 100755
--- a/t/t6000lib.sh
+++ b/t/t6000lib.sh
@@ -98,7 +98,7 @@ check_output()
 name_from_description()
 {
         tr "'" '-' |
-		tr '~`!@#$%^&*()_+={}[]|\;:"<>,/? ' \
+		tr -- '~`!@#$%^&*()_+={}[]|\;:"<>,/? ' \
 		   '------------------------------' |
 		tr -s '-' | tr '[A-Z]' '[a-z]' | sed "s/^-*//;s/-*\$//"
 }
-- 
1.5.4.4.481.g5075

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

* Re: [PATCH] t/t6000lib.sh: tr portability fix fix
  2008-03-14 20:47   ` [PATCH] t/t6000lib.sh: tr portability fix fix Brandon Casey
@ 2008-03-14 20:54     ` Jeff King
  2008-03-14 21:00       ` Brandon Casey
  2008-03-14 21:26       ` Brandon Casey
  0 siblings, 2 replies; 59+ messages in thread
From: Jeff King @ 2008-03-14 20:54 UTC (permalink / raw)
  To: Brandon Casey; +Cc: armstrong.whit, Junio C Hamano, Git Mailing List

On Fri, Mar 14, 2008 at 03:47:37PM -0500, Brandon Casey wrote:

> This patch fixes things. If the dashdash notation is not portable, then
> backslashing each dash also works. i.e. '\-\-\-\-.. etc. but as you
> mentioned something like that is less readable, but possibly not as bad
> as a sed version.

It seems to work fine on Solaris with all versions of tr. I did just
blindly extend the '-' without thinking, though...I wonder if there are
systems that will get confused about it being a range. It might be
safer to just use sed anyway.

-Peff

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

* Re: [PATCH] t/t6000lib.sh: tr portability fix fix
  2008-03-14 20:54     ` Jeff King
@ 2008-03-14 21:00       ` Brandon Casey
  2008-03-14 21:26       ` Brandon Casey
  1 sibling, 0 replies; 59+ messages in thread
From: Brandon Casey @ 2008-03-14 21:00 UTC (permalink / raw)
  To: Jeff King; +Cc: armstrong.whit, Junio C Hamano, Git Mailing List

Jeff King wrote:
> On Fri, Mar 14, 2008 at 03:47:37PM -0500, Brandon Casey wrote:
>> If the dashdash notation is not portable, then
>> backslashing each dash also works. i.e. '\-\-\-\-..

> I wonder if there are
> systems that will get confused about it being a range.

Oh, now I understand _why_ backslashing the dashes worked. When your
email arrived I was still trying to figure out why

  echo hello | tr aeiou '\-\-\-\-\-'

correctly converted the e and o into dashes. Because tr must have a
way for the user to escape the range notation. I don't use tr very
often.

-brandon

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

* [PATCH] t/t6000lib.sh: tr portability fix fix
  2008-03-14 20:54     ` Jeff King
  2008-03-14 21:00       ` Brandon Casey
@ 2008-03-14 21:26       ` Brandon Casey
  2008-03-14 21:45         ` Jeff King
  1 sibling, 1 reply; 59+ messages in thread
From: Brandon Casey @ 2008-03-14 21:26 UTC (permalink / raw)
  To: Jeff King; +Cc: armstrong.whit, Junio C Hamano, Git Mailing List

Some versions of tr have a problem with character sets which begin with
multiple dashes and attempt to interpret them as long options. Escape
each dash to avoid this confusion and also prevent a possible
interpretation of the dashes as a range.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---

Jeff King wrote:
> On Fri, Mar 14, 2008 at 03:47:37PM -0500, Brandon Casey wrote:
> 
>> This patch fixes things. If the dashdash notation is not portable, then
>> backslashing each dash also works. i.e. '\-\-\-\-.. etc. but as you
>> mentioned something like that is less readable, but possibly not as bad
>> as a sed version.
> 
> It seems to work fine on Solaris with all versions of tr. I did just
> blindly extend the '-' without thinking, though...I wonder if there are
> systems that will get confused about it being a range. It might be
> safer to just use sed anyway.

Here's the version with escaped dashes. If you do the sed version, it's
something to compare to for readability.

-brandon


 t/t6000lib.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/t/t6000lib.sh b/t/t6000lib.sh
index b69f7c4..71f2140 100755
--- a/t/t6000lib.sh
+++ b/t/t6000lib.sh
@@ -99,7 +99,7 @@ name_from_description()
 {
         tr "'" '-' |
 		tr '~`!@#$%^&*()_+={}[]|\;:"<>,/? ' \
-		   '------------------------------' |
+		   '\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-' |
 		tr -s '-' | tr '[A-Z]' '[a-z]' | sed "s/^-*//;s/-*\$//"
 }
 
-- 
1.5.4.4.481.g5075

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

* Re: [PATCH 08/16] t4200: use cut instead of sed
  2008-03-13 18:00       ` Junio C Hamano
@ 2008-03-14 21:40         ` Jeff King
  0 siblings, 0 replies; 59+ messages in thread
From: Jeff King @ 2008-03-14 21:40 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Whit Armstrong, git

On Thu, Mar 13, 2008 at 11:00:23AM -0700, Junio C Hamano wrote:

> > index c607aad..e4a1dc1 100644
> > --- a/builtin-rerere.c
> > +++ b/builtin-rerere.c
> > @@ -58,7 +58,8 @@ static int write_rr(struct path_list *rr, int out_fd)
> >  		int length = strlen(path) + 1;
> >  		if (write_in_full(out_fd, rr->items[i].util, 40) != 40 ||
> >  		    write_in_full(out_fd, "\t", 1) != 1 ||
> > -		    write_in_full(out_fd, path, length) != length)
> > +		    write_in_full(out_fd, path, length) != length ||
> > +		    write_in_full(out_fd, "\n", 1) != 1)
> >  			die("unable to write rerere record");
> >  	}
> >  	if (commit_lock_file(&write_lock) != 0)
> 
> No, check 3f43d72392b6c0477debd7edbd49bae9b7f41e60^:git-rerere.perl; the
> records in this file are supposed to be NUL terminated (the paths are
> allowed to have LF in them).

Ah, OK. And it actually does do that correctly ("length = strlen(path) +
1"). So I think there isn't a bug there in rerere.

But if it's NUL-terminated, then using sed _definitely_ isn't portable
here. cut does work on Solaris in this case, but that might or might not
be portable to other systems with similar NUL problems. I'm not sure
what is the best route. We really just want to grab the sha1 from the
beginning of the first line. dd count=40? :)

-Peff

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

* Re: [PATCH] t/t6000lib.sh: tr portability fix fix
  2008-03-14 21:26       ` Brandon Casey
@ 2008-03-14 21:45         ` Jeff King
  2008-03-14 22:06           ` Brandon Casey
  0 siblings, 1 reply; 59+ messages in thread
From: Jeff King @ 2008-03-14 21:45 UTC (permalink / raw)
  To: Brandon Casey; +Cc: armstrong.whit, Junio C Hamano, Git Mailing List

On Fri, Mar 14, 2008 at 04:26:31PM -0500, Brandon Casey wrote:

> Here's the version with escaped dashes. If you do the sed version, it's
> something to compare to for readability.
>
> [...]
>
> -		   '------------------------------' |
> +		   '\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-' |

Ugh. How about:

  sed 'yA~`!@#$%^&*()_+={}[]|\;:"<>,/? A------------------------------A'

The 'A' delimiter is because in my test, Solaris sed didn't seem to
understand \/ to include the literal '/'. And all of the other
punctuation is already used in the pattern. ;)

-Peff

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

* Re: [PATCH] t/t6000lib.sh: tr portability fix fix
  2008-03-14 21:45         ` Jeff King
@ 2008-03-14 22:06           ` Brandon Casey
  0 siblings, 0 replies; 59+ messages in thread
From: Brandon Casey @ 2008-03-14 22:06 UTC (permalink / raw)
  To: Jeff King; +Cc: armstrong.whit, Junio C Hamano, Git Mailing List

Jeff King wrote:
> On Fri, Mar 14, 2008 at 04:26:31PM -0500, Brandon Casey wrote:
> 
>> Here's the version with escaped dashes. If you do the sed version, it's
>> something to compare to for readability.
>>
>> [...]
>>
>> -		   '------------------------------' |
>> +		   '\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-' |
> 
> Ugh. How about:
> 
>   sed 'yA~`!@#$%^&*()_+={}[]|\;:"<>,/? A------------------------------A'

Not working. I get:

*   ok 33: bisection diff --bisect u5 ^U <= 0
sed: -e expression #1, char 64: unterminated `y' command
* FAIL 34: --bisect l5 ^root
        check_output  "git rev-list $_bisect_option l5 ^root"
sed: -e expression #1, char 64: unterminated `y' command
* FAIL 35: --bisect l5 ^root ^c3
        check_output  "git rev-list $_bisect_option l5 ^root ^c3"
sed: -e expression #1, char 64: unterminated `y' command
* FAIL 36: --bisect l5 ^root ^c3 ^b4


But this does:

sed 'yA~`!@#$%^&*()_+={}\[]|\\;:"<>,/? A------------------------------A'

I have to escape open bracket and backslash on my end (linux). :(

Have to leave now, so if that doesn't work for you (which I'm thinking it
won't), I won't be able to test on my end for a while.

-brandon

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

* Re: [PATCH 03/16] more tr portability test script fixes
  2008-03-12 21:31 ` [PATCH 03/16] more tr portability test script fixes Jeff King
  2008-03-13  8:28   ` Frank Lichtenheld
@ 2008-03-18 22:23   ` Alex Riesen
  2008-03-18 22:24     ` [PATCH] Add test-tr: poor-man tr for the test suite Alex Riesen
  2008-03-18 22:44     ` [PATCH 03/16] more tr portability test script fixes Jeff King
  1 sibling, 2 replies; 59+ messages in thread
From: Alex Riesen @ 2008-03-18 22:23 UTC (permalink / raw)
  To: Jeff King; +Cc: Whit Armstrong, Junio C Hamano, git

Jeff King, Wed, Mar 12, 2008 22:31:06 +0100:
> -    tr '\000' '\012' <"$1" | sed -e "$sanitize_diff_raw_z" >.tmp-1
> -    tr '\000' '\012' <"$2" | sed -e "$sanitize_diff_raw_z" >.tmp-2
> +    perl -pe 'y/\000/\012/' <"$1" | sed -e "$sanitize_diff_raw_z" >.tmp-1
> +    perl -pe 'y/\000/\012/' <"$2" | sed -e "$sanitize_diff_raw_z" >.tmp-2

These break in presence of ActiveState Perl on Windows.

I suggest replacing such simple construction with a simplified,
in-tree, version of tr.

Patches follow

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

* [PATCH] Add test-tr: poor-man tr for the test suite
  2008-03-18 22:23   ` Alex Riesen
@ 2008-03-18 22:24     ` Alex Riesen
  2008-03-18 22:24       ` [PATCH] Use test-tr in " Alex Riesen
  2008-03-18 22:44     ` [PATCH 03/16] more tr portability test script fixes Jeff King
  1 sibling, 1 reply; 59+ messages in thread
From: Alex Riesen @ 2008-03-18 22:24 UTC (permalink / raw)
  To: Jeff King; +Cc: Whit Armstrong, Junio C Hamano, git

It offers a limited set of POSIX tr, in particular: no character class
support and no [n*m] operators. Only 8bit. C-escapes supported, and
character ranges. Deletion and squeezing should work, but -s does not
match what the GNU tr from coreutils (which, in turn, does not match
SuSv2).

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---

 Makefile              |    2 +-
 t/t0000-test-progs.sh |   72 +++++++++++++++
 test-tr.c             |  236 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 309 insertions(+), 1 deletions(-)
 create mode 100755 t/t0000-test-progs.sh
 create mode 100644 test-tr.c

diff --git a/Makefile b/Makefile
index 7c70b00..a458c30 100644
--- a/Makefile
+++ b/Makefile
@@ -1177,7 +1177,7 @@ endif
 
 ### Testing rules
 
-TEST_PROGRAMS = test-chmtime$X test-genrandom$X test-date$X test-delta$X test-sha1$X test-match-trees$X test-absolute-path$X test-parse-options$X
+TEST_PROGRAMS = test-chmtime$X test-genrandom$X test-date$X test-delta$X test-sha1$X test-match-trees$X test-absolute-path$X test-parse-options$X test-tr$X
 
 all:: $(TEST_PROGRAMS)
 
diff --git a/t/t0000-test-progs.sh b/t/t0000-test-progs.sh
new file mode 100755
index 0000000..d8d339a
--- /dev/null
+++ b/t/t0000-test-progs.sh
@@ -0,0 +1,72 @@
+#!/bin/sh
+
+test_description='Test the test support programs for sanity'
+
+. ./test-lib.sh
+
+test_expect_success 'test-tr character range' '
+
+	echo def >expected &&
+	echo abc | test-tr a-c d-f >result &&
+	cmp expected result &&
+
+	printf def >expected &&
+	printf def | test-tr a-c d-f >result &&
+	cmp expected result
+
+	printf def >expected &&
+	printf abc | test-tr \\141-\\143 \\144-\\146 >result &&
+	cmp expected result
+
+	printf \\r\\n >expected &&
+	printf \\r\\n | test-tr rn \\r\\n >result &&
+	cmp expected result
+
+	printf \\000\\n >expected &&
+	printf 0n | test-tr 0n \\000\\n >result &&
+	cmp expected result
+
+	printf 0n >expected &&
+	printf \\000\\n | test-tr \\000\\n 0n >result &&
+	cmp expected result &&
+
+	echo nopqrstuvwxyzbcdefghklm >expected &&
+	echo abcdefghijklmopqrstuxyz | test-tr A-Za-z N-ZA-Mn-za-m >result &&
+	cmp expected result
+'
+
+test_expect_success 'test-tr delete characters' '
+
+	echo ac >expected &&
+	echo abc | test-tr -d b >result &&
+	cmp expected result &&
+
+	echo abcghijk >expected &&
+	echo abcdefghijk | test-tr -d d-f >result &&
+	cmp expected result
+
+'
+
+test_expect_success 'test-tr squeeze repeating characters' '
+
+	echo abc >expected &&
+	echo abbbbc | test-tr -s b >result &&
+	cmp expected result &&
+
+	echo def >expected &&
+	echo abbbbc | test-tr -s a-c d-f >result &&
+	cmp expected result
+
+'
+
+test_expect_success 'test-tr sanity' '
+
+	test_must_fail test-tr -d a-z A-Z &&
+	test_must_fail test-tr \\477 a &&
+	test_must_fail test-tr z-a a &&
+	test_must_fail test-tr a-z "" &&
+	test_must_fail test-tr a-z
+
+'
+
+test_done
diff --git a/test-tr.c b/test-tr.c
new file mode 100644
index 0000000..477ae16
--- /dev/null
+++ b/test-tr.c
@@ -0,0 +1,236 @@
+/*
+ *  Simplified tr
+ *
+ *  Supports:
+ *
+ *  CHAR1-CHAR2
+ *  Escape sequences
+ *  -d (delete)
+ *  -s (squeeze)
+ *
+ *  No unicode
+ *  No characters classes
+ *  No -c (complement) support
+ *  Behavior of "-s -d" not tested (never used in tests)
+ *
+ *  Squeeze of of repeating characters follows the behavior
+ *  described in the Single Unix Specification (as good as I
+ *  understand it), not what tr does (it squeezes every character
+ *  found in set1 OR set2, whereas the SuS says:
+ *
+ *  "When the -s option is specified, after any deletions or
+ *  translations have taken place, repeated sequences of the same
+ *  character will be replaced by one occurrence of the same
+ *  character, if the character is found in the array specified by the
+ *  last operand."
+ *
+ *  So this tr squeezes only characters matched the set1.
+ *
+ */
+#include "cache.h"
+
+static int squeeze, delete;
+
+static unsigned char *unquote(const char *s, unsigned *len)
+{
+	unsigned char *result = malloc(strlen(s)), *r = result;
+
+	while (*s) {
+		switch (*s) {
+		case '\\':
+			++s;
+#define ISOCT(c) (((c) >= '0' && (c) <= '7'))
+			if (ISOCT(*s)) {
+				unsigned int c;
+				char oct[4] = {0, 0, 0, 0};
+				oct[0] = *s++;
+				c = (oct[0] - '0');
+				if (ISOCT(*s)) {
+					oct[1] = *s++;
+					c = (c << 3) |(oct[1] - '0');
+					if (ISOCT(*s)) {
+						oct[2] = *s++;
+						c = (c << 3) |(oct[2] - '0');
+					}
+				}
+				if (c > 255) {
+					fprintf(stderr, "invalid octal character specification: \\%s\n", oct);
+					exit(1);
+				}
+				*r++ = c & 0xff;
+			} else {
+				switch (*s) {
+				case '\0':
+					*r++ = '\\';
+					break;
+				case '\\':
+					*r++ = *s++;
+					break;
+				case 'a':
+					*r++ = '\a';
+					++s;
+					break;
+				case 'b':
+					*r++ = '\b';
+					++s;
+					break;
+				case 'f':
+					*r++ = '\f';
+					++s;
+					break;
+				case 'n':
+					*r++ = '\n';
+					++s;
+					break;
+				case 'r':
+					*r++ = '\r';
+					++s;
+					break;
+				case 't':
+					*r++ = '\t';
+					++s;
+					break;
+				case 'v':
+					*r++ = '\v';
+					++s;
+					break;
+				default:
+					*r++ = '\\';
+					*r++ = *s++;
+					break;
+				}
+			}
+			break;
+		default:
+			*r++ = *s++;
+		}
+	}
+
+	*len = r - result;
+	*r = '\0';
+	return result;
+}
+
+#define MAX_PATTERN 256
+static void put_ch(unsigned char *conv, unsigned char ch, unsigned *len)
+{
+	unsigned i = (*len)++;
+	if (*len > MAX_PATTERN) {
+		fprintf(stderr, "pattern too long\n");
+		exit(1);
+	}
+	conv[i] = ch;
+}
+
+static void parse(const unsigned char *rule, unsigned rule_len,
+		  unsigned char *set, unsigned *set_len)
+{
+	const unsigned char *p = rule;
+	while (p < rule + rule_len) {
+		if ('-' == *p && p > rule && p[1]) {
+			unsigned c;
+			if (p[-1] > p[1]) {
+				fprintf(stderr, "%c%c%c: range is reversed\n",
+					p[-1], *p, p[1]);
+				exit(1);
+			}
+			c = p[-1] + 1u;
+			for (; c <= p[1]; ++c)
+				put_ch(set, c, set_len);
+			++p;
+			++p;
+			continue;
+		}
+		put_ch(set, *p, set_len);
+		++p;
+	}
+}
+
+int main(int argc, char *argv[])
+{
+	unsigned set1_len = 0, set2_len = 0;
+	unsigned char set1[MAX_PATTERN];
+	unsigned char set2[MAX_PATTERN];
+
+	ssize_t n;
+	unsigned char last = 0, have_last = 0;
+	unsigned char buf[BUFSIZ];
+
+	char *rule1 = NULL, *rule2 = NULL;
+	unsigned char *urule1, *urule2;
+	unsigned urule1_len, urule2_len;
+	int opt;
+
+	for (opt = 1; opt < argc; ++opt) {
+		if (!strcmp("-s", argv[opt]))
+			squeeze = 1;
+		else if (!strcmp("-d", argv[opt]))
+			delete = 1;
+		else if (!rule1) {
+			rule1 = argv[opt];
+		} else if (!rule2)
+			rule2 = argv[opt];
+	}
+	if (!rule1) {
+	    fprintf(stderr, "no source set given\n"
+		    "test-tr [-s] [-d] set1 [set2]\n"
+		    "\"set\" supports only \\NNN, \\a-\\v and CHAR1-CHAR2 rules\n");
+	    exit(1);
+	}
+	if (delete && rule2) {
+		fprintf(stderr, "extra operand %s when deleting\n", rule2);
+		exit(1);
+	}
+	urule1 = unquote(rule1, &urule1_len);
+	urule2 = NULL;
+	urule2_len = 0;
+	if ((!rule2 || !*rule2) && !delete && !squeeze) {
+		fprintf(stderr, "set2 must be non-empty\n");
+		exit(1);
+	}
+
+	parse(urule1, urule1_len, set1, &set1_len);
+
+	if (rule2) {
+		unsigned i;
+		urule2 = unquote(rule2, &urule2_len);
+		parse(urule2, urule2_len, set2, &set2_len);
+		i = set2[set2_len - 1];
+		while (set2_len < set1_len)
+			put_ch(set2, i, &set2_len);
+	}
+
+	while ((n = read(STDIN_FILENO, buf, sizeof(buf)))) {
+		if (n < 0) {
+			int err = errno;
+			if (EINTR == err || EAGAIN == err)
+				continue;
+			fprintf(stderr, "%s: %s\n", argv[0], strerror(err));
+			exit(1);
+		}
+		if (set1_len) {
+			unsigned i, o = 0;
+			for (i = 0; i < (unsigned)n; ++i) {
+				unsigned char *p, ch = buf[i];
+				p = memchr(set1, ch, set1_len);
+				if (p) {
+					if (delete)
+						continue;
+					if (set2_len)
+						ch = set2[p - set1];
+					if (!(squeeze &&
+					      have_last &&
+					      ch == last))
+						buf[o++] = ch;
+				} else
+					buf[o++] = ch;
+
+				have_last = 1;
+				last = ch;
+			}
+			n = o;
+		}
+		write(STDOUT_FILENO, buf, n);
+	}
+	return 0;
+}
-- 
1.5.5.rc0.53.g97734

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

* [PATCH] Use test-tr in the test suite
  2008-03-18 22:24     ` [PATCH] Add test-tr: poor-man tr for the test suite Alex Riesen
@ 2008-03-18 22:24       ` Alex Riesen
  0 siblings, 0 replies; 59+ messages in thread
From: Alex Riesen @ 2008-03-18 22:24 UTC (permalink / raw)
  To: Jeff King; +Cc: Whit Armstrong, Junio C Hamano, git

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
 t/annotate-tests.sh            |    4 ++--
 t/diff-lib.sh                  |    4 ++--
 t/t0000-basic.sh               |    2 +-
 t/t0020-crlf.sh                |   10 +++++-----
 t/t0021-conversion.sh          |    2 +-
 t/t1300-repo-config.sh         |    4 ++--
 t/t3300-funny-names.sh         |    6 +++---
 t/t3402-rebase-merge.sh        |    4 ++--
 t/t4004-diff-rename-symlink.sh |    2 +-
 t/t4015-diff-whitespace.sh     |    6 +++---
 t/t4019-diff-wserror.sh        |    2 +-
 t/t4020-diff-external.sh       |    2 +-
 t/t4022-diff-rewrite.sh        |    2 +-
 t/t4101-apply-nonl.sh          |    4 ++--
 t/t4103-apply-binary.sh        |    4 ++--
 t/t4116-apply-reverse.sh       |    4 ++--
 t/t4118-apply-empty-context.sh |    4 ++--
 t/t4200-rerere.sh              |    2 +-
 t/t4201-shortlog.sh            |    4 ++--
 t/t5300-pack-object.sh         |    2 +-
 t/t5500-fetch-pack.sh          |    2 +-
 t/t5505-remote.sh              |    4 ++--
 t/t6003-rev-list-topo-order.sh |    2 +-
 t/t7003-filter-branch.sh       |    2 +-
 t/t9200-git-cvsexportcommit.sh |    2 +-
 t/test-lib.sh                  |   12 +++++++-----
 26 files changed, 50 insertions(+), 48 deletions(-)

diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index cacb273..8e10323 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -104,7 +104,7 @@ test_expect_success \
 
 test_expect_success \
     'an incomplete line added' \
-    'echo "incomplete" | tr -d "\\012" >>file &&
+    'echo "incomplete" | test-tr -d "\\012" >>file &&
     GIT_AUTHOR_NAME="C" git commit -a -m "Incomplete"'
 
 test_expect_success \
@@ -115,7 +115,7 @@ test_expect_success \
     'some edit' \
     'mv file file.orig &&
     sed -e "s/^3A/99/" -e "/^1A/d" -e "/^incomplete/d" < file.orig > file &&
-    echo "incomplete" | tr -d "\\012" >>file &&
+    echo "incomplete" | test-tr -d "\\012" >>file &&
     GIT_AUTHOR_NAME="D" git commit -a -m "edit"'
 
 test_expect_success \
diff --git a/t/diff-lib.sh b/t/diff-lib.sh
index 28b941c..257e123 100644
--- a/t/diff-lib.sh
+++ b/t/diff-lib.sh
@@ -21,8 +21,8 @@ compare_diff_raw_z () {
     # Also we do not check SHA1 hash generation in this test, which
     # is a job for t0000-basic.sh
 
-    perl -pe 'y/\000/\012/' <"$1" | sed -e "$sanitize_diff_raw_z" >.tmp-1
-    perl -pe 'y/\000/\012/' <"$2" | sed -e "$sanitize_diff_raw_z" >.tmp-2
+    test-tr '\000' '\012' <"$1" | sed -e "$sanitize_diff_raw_z" >.tmp-1
+    test-tr '\000' '\012' <"$2" | sed -e "$sanitize_diff_raw_z" >.tmp-2
     git diff .tmp-1 .tmp-2 && rm -f .tmp-1 .tmp-2
 }
 
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 27b54cb..a1f60eb 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -324,7 +324,7 @@ test_expect_success 'very long name in the index handled sanely' '
 	(
 		git ls-files -s path4 |
 		sed -e "s/	.*/	/" |
-		tr -d "\012"
+		test-tr -d "\012"
 		echo "$a"
 	) | git update-index --index-info &&
 	len=$(git ls-files "a*" | wc -c) &&
diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh
index 2bfeac9..7427d59 100755
--- a/t/t0020-crlf.sh
+++ b/t/t0020-crlf.sh
@@ -5,20 +5,20 @@ test_description='CRLF conversion'
 . ./test-lib.sh
 
 q_to_nul () {
-	perl -pe 'y/Q/\000/'
+	test-tr Q '\000'
 }
 
 q_to_cr () {
-	tr Q '\015'
+	test-tr Q '\015'
 }
 
 append_cr () {
-	sed -e 's/$/Q/' | tr Q '\015'
+	sed -e 's/$/Q/' | test-tr Q '\015'
 }
 
 remove_cr () {
-	tr '\015' Q <"$1" | grep Q >/dev/null &&
-	tr '\015' Q <"$1" | sed -ne 's/Q$//p'
+	test-tr '\015' Q <"$1" | grep Q >/dev/null &&
+	test-tr '\015' Q <"$1" | sed -ne 's/Q$//p'
 }
 
 test_expect_success setup '
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index 8fc39d7..d146db4 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -5,7 +5,7 @@ test_description='blob conversion via gitattributes'
 . ./test-lib.sh
 
 cat <<\EOF >rot13.sh
-tr \
+test-tr \
   'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ' \
   'nopqrstuvwxyzabcdefghijklmNOPQRSTUVWXYZABCDEFGHIJKLM'
 EOF
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index b36a901..42c3f15 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -657,12 +657,12 @@ Qsection.sub=section.val4
 Qsection.sub=section.val5Q
 EOF
 
-git config --null --list | perl -pe 'y/\000/Q/' > result
+git config --null --list | test-tr '\000' Q > result
 echo >>result
 
 test_expect_success '--null --list' 'cmp result expect'
 
-git config --null --get-regexp 'val[0-9]' | perl -pe 'y/\000/Q/' > result
+git config --null --get-regexp 'val[0-9]' | test-tr '\000' Q > result
 echo >>result
 
 test_expect_success '--null --get-regexp' 'cmp result expect'
diff --git a/t/t3300-funny-names.sh b/t/t3300-funny-names.sh
index 24a00a9..10c5217 100755
--- a/t/t3300-funny-names.sh
+++ b/t/t3300-funny-names.sh
@@ -54,7 +54,7 @@ echo 'just space
 no-funny
 tabs	," (dq) and spaces' >expected
 test_expect_success 'git ls-files -z with-funny' \
-	'git ls-files -z | perl -pe y/\\000/\\012/ >current &&
+	'git ls-files -z | test-tr "\000" "\012" >current &&
 	git diff expected current'
 
 t1=`git write-tree`
@@ -83,11 +83,11 @@ test_expect_success 'git diff-tree with-funny' \
 echo 'A
 tabs	," (dq) and spaces' >expected
 test_expect_success 'git diff-index -z with-funny' \
-	'git diff-index -z --name-status $t0 | perl -pe y/\\000/\\012/ >current &&
+	'git diff-index -z --name-status $t0 | test-tr "\000" "\012" >current &&
 	git diff expected current'
 
 test_expect_success 'git diff-tree -z with-funny' \
-	'git diff-tree -z --name-status $t0 $t1 | perl -pe y/\\000/\\012/ >current &&
+	'git diff-tree -z --name-status $t0 $t1 | test-tr "\000" "\012" >current &&
 	git diff expected current'
 
 cat > expected <<\EOF
diff --git a/t/t3402-rebase-merge.sh b/t/t3402-rebase-merge.sh
index 7b7d072..704f9a3 100755
--- a/t/t3402-rebase-merge.sh
+++ b/t/t3402-rebase-merge.sh
@@ -30,11 +30,11 @@ test_expect_success setup '
 	git update-index --force-remove original &&
 	git commit -a -m"side renames and edits." &&
 
-	tr "[a-z]" "[A-Z]" <original >newfile &&
+	test-tr a-z A-Z <original >newfile &&
 	git add newfile &&
 	git commit -a -m"side edits further." &&
 
-	tr "[a-m]" "[A-M]" <original >newfile &&
+	test-tr a-m A-M <original >newfile &&
 	rm -f original &&
 	git commit -a -m"side edits once again." &&
 
diff --git a/t/t4004-diff-rename-symlink.sh b/t/t4004-diff-rename-symlink.sh
index 3d25be7..b49efa4 100755
--- a/t/t4004-diff-rename-symlink.sh
+++ b/t/t4004-diff-rename-symlink.sh
@@ -14,7 +14,7 @@ by an edit for them.
 
 test_expect_success \
     'prepare reference tree' \
-    'echo xyzzy | tr -d '\\\\'012 >yomin &&
+    'echo xyzzy | test-tr -d "\012" >yomin &&
      ln -s xyzzy frotz &&
     git update-index --add frotz yomin &&
     tree=$(git write-tree) &&
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index 83c54b7..595c5e2 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -51,7 +51,7 @@ test_expect_success "Ray's example with -w" 'git diff expect out'
 git diff -b > out
 test_expect_success "Ray's example with -b" 'git diff expect out'
 
-tr 'Q' '\015' << EOF > x
+test-tr 'Q' '\015' << EOF > x
 whitespace at beginning
 whitespace change
 whitespace in the middle
@@ -71,7 +71,7 @@ unchanged line
 CR at end
 EOF
 
-tr 'Q' '\015' << EOF > expect
+test-tr 'Q' '\015' << EOF > expect
 diff --git a/x b/x
 index d99af23..8b32fb5 100644
 --- a/x
@@ -99,7 +99,7 @@ EOF
 git diff -w > out
 test_expect_success 'another test, with -w' 'git diff expect out'
 
-tr 'Q' '\015' << EOF > expect
+test-tr 'Q' '\015' << EOF > expect
 diff --git a/x b/x
 index d99af23..8b32fb5 100644
 --- a/x
diff --git a/t/t4019-diff-wserror.sh b/t/t4019-diff-wserror.sh
index 0d9cbb6..4967026 100755
--- a/t/t4019-diff-wserror.sh
+++ b/t/t4019-diff-wserror.sh
@@ -12,7 +12,7 @@ test_expect_success setup '
 	echo "         Eight SP indent" >>F &&
 	echo " 	HT and SP indent" >>F &&
 	echo "With trailing SP " >>F &&
-	echo "Carriage ReturnQ" | tr Q "\015" >>F &&
+	echo "Carriage ReturnQ" | test-tr Q "\015" >>F &&
 	echo "No problem" >>F
 
 '
diff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh
index 637b4e1..3ae3165 100755
--- a/t/t4020-diff-external.sh
+++ b/t/t4020-diff-external.sh
@@ -99,7 +99,7 @@ test_expect_success 'no diff with -diff' '
 	git diff | grep Binary
 '
 
-echo NULZbetweenZwords | perl -pe 'y/Z/\000/' > file
+echo NULZbetweenZwords | test-tr 'Z' '\000' > file
 
 test_expect_success 'force diff with "diff"' '
 	echo >.gitattributes "file diff" &&
diff --git a/t/t4022-diff-rewrite.sh b/t/t4022-diff-rewrite.sh
index bf996fc..01d221b 100755
--- a/t/t4022-diff-rewrite.sh
+++ b/t/t4022-diff-rewrite.sh
@@ -8,7 +8,7 @@ test_expect_success setup '
 
 	cat ../../COPYING >test &&
 	git add test &&
-	tr \
+	test-tr \
 	  "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ" \
 	  "nopqrstuvwxyzabcdefghijklmNOPQRSTUVWXYZABCDEFGHIJKLM" \
 	  <../../COPYING >test
diff --git a/t/t4101-apply-nonl.sh b/t/t4101-apply-nonl.sh
index da8abcf..53277e6 100755
--- a/t/t4101-apply-nonl.sh
+++ b/t/t4101-apply-nonl.sh
@@ -12,8 +12,8 @@ test_description='git apply should handle files with incomplete lines.
 
 (echo a; echo b) >frotz.0
 (echo a; echo b; echo c) >frotz.1
-(echo a; echo b | tr -d '\012') >frotz.2
-(echo a; echo c; echo b | tr -d '\012') >frotz.3
+(echo a; echo b | test-tr -d '\012') >frotz.2
+(echo a; echo c; echo b | test-tr -d '\012') >frotz.3
 
 for i in 0 1 2 3
 do
diff --git a/t/t4103-apply-binary.sh b/t/t4103-apply-binary.sh
index 1b58233..f7542a0 100755
--- a/t/t4103-apply-binary.sh
+++ b/t/t4103-apply-binary.sh
@@ -24,10 +24,10 @@ git update-index --add --remove file1 file2 file4
 git-commit -m 'Initial Version' 2>/dev/null
 
 git-checkout -b binary
-perl -pe 'y/x/\000/' <file1 >file3
+test-tr x '\000' <file1 >file3
 cat file3 >file4
 git add file2
-perl -pe 'y/\000/v/' <file3 >file1
+test-tr '\000' v <file3 >file1
 rm -f file2
 git update-index --add --remove file1 file2 file3 file4
 git-commit -m 'Second Version'
diff --git a/t/t4116-apply-reverse.sh b/t/t4116-apply-reverse.sh
index c3f4579..bf01619 100755
--- a/t/t4116-apply-reverse.sh
+++ b/t/t4116-apply-reverse.sh
@@ -12,14 +12,14 @@ test_description='git apply in reverse
 test_expect_success setup '
 
 	for i in a b c d e f g h i j k l m n; do echo $i; done >file1 &&
-	perl -pe "y/ijk/\\000\\001\\002/" <file1 >file2 &&
+	test-tr "ijk" "\000\001\002" <file1 >file2 &&
 
 	git add file1 file2 &&
 	git commit -m initial &&
 	git tag initial &&
 
 	for i in a b c g h i J K L m o n p q; do echo $i; done >file1 &&
-	perl -pe "y/mon/\\000\\001\\002/" <file1 >file2 &&
+	test-tr "mon" "\000\001\002" <file1 >file2 &&
 
 	git commit -a -m second &&
 	git tag second &&
diff --git a/t/t4118-apply-empty-context.sh b/t/t4118-apply-empty-context.sh
index 1d531ca..5083100 100755
--- a/t/t4118-apply-empty-context.sh
+++ b/t/t4118-apply-empty-context.sh
@@ -18,13 +18,13 @@ test_expect_success setup '
 	cat file1 >file1.orig &&
 	{
 		cat file1 &&
-		echo Q | tr -d "\\012"
+		echo Q | test-tr -d "\012"
 	} >file2 &&
 	cat file2 >file2.orig
 	git add file1 file2 &&
 	sed -e "/^B/d" <file1.orig >file1 &&
 	sed -e "/^[BQ]/d" <file2.orig >file2 &&
-	echo Q | tr -d "\\012" >>file2 &&
+	echo Q | test-tr -d "\012" >>file2 &&
 	cat file1 >file1.mods &&
 	cat file2 >file2.mods &&
 	git diff |
diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index 3cbfee7..58fc740 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -129,7 +129,7 @@ test_expect_success 'rerere kicked in' "! grep ======= a1"
 test_expect_success 'rerere prefers first change' 'git diff a1 expect'
 
 rm $rr/postimage
-echo "$sha1	a1" | perl -pe 'y/\012/\000/' > .git/rr-cache/MERGE_RR
+echo "$sha1	a1" | test-tr '\012' '\000' > .git/rr-cache/MERGE_RR
 
 test_expect_success 'rerere clear' 'git rerere clear'
 
diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh
index 405b971..1a4b88d 100755
--- a/t/t4201-shortlog.sh
+++ b/t/t4201-shortlog.sh
@@ -19,12 +19,12 @@ git commit --quiet -m "This is a very, very long first line for the commit messa
 
 # test if the wrapping is still valid when replacing all i's by treble clefs.
 echo 3 > a1
-git commit --quiet -m "$(echo "This is a very, very long first line for the commit message to see if it is wrapped correctly" | sed "s/i/1234/g" | tr 1234 '\360\235\204\236')" a1
+git commit --quiet -m "$(echo "This is a very, very long first line for the commit message to see if it is wrapped correctly" | sed "s/i/1234/g" | test-tr 1234 '\360\235\204\236')" a1
 
 # now fsck up the utf8
 git config i18n.commitencoding non-utf-8
 echo 4 > a1
-git commit --quiet -m "$(echo "This is a very, very long first line for the commit message to see if it is wrapped correctly" | sed "s/i/1234/g" | tr 1234 '\370\235\204\236')" a1
+git commit --quiet -m "$(echo "This is a very, very long first line for the commit message to see if it is wrapped correctly" | sed "s/i/1234/g" | test-tr 1234 '\370\235\204\236')" a1
 
 echo 5 > a1
 git commit --quiet -m "a								12	34	56	78" a1
diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index c955fe4..ba49742 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -15,7 +15,7 @@ test_expect_success \
     'rm -f .git/index*
      for i in a b c
      do
-	     dd if=/dev/zero bs=4k count=1 | perl -pe "y/\\000/$i/" >$i &&
+	     dd if=/dev/zero bs=4k count=1 | test-tr "\000" "$i" >$i &&
 	     git update-index --add $i || return 1
      done &&
      cat c >d && echo foo >>d && git update-index --add d &&
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 788b4a5..cda09b5 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -37,7 +37,7 @@ add () {
 }
 
 count_objects () {
-	ls .git/objects/??/* 2>>log2.txt | wc -l | tr -d " "
+	ls .git/objects/??/* 2>>log2.txt | wc -l | test-tr -d " "
 }
 
 test_expect_object_count () {
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index ecfc999..89f06b2 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -22,8 +22,8 @@ setup_repository () {
 }
 
 tokens_match () {
-	echo "$1" | tr ' ' '\012' | sort | sed -e '/^$/d' >expect &&
-	echo "$2" | tr ' ' '\012' | sort | sed -e '/^$/d' >actual &&
+	echo "$1" | test-tr ' ' '\012' | sort | sed -e '/^$/d' >expect &&
+	echo "$2" | test-tr ' ' '\012' | sort | sed -e '/^$/d' >actual &&
 	test_cmp expect actual
 }
 
diff --git a/t/t6003-rev-list-topo-order.sh b/t/t6003-rev-list-topo-order.sh
index 5daa0be..5dbc6a7 100755
--- a/t/t6003-rev-list-topo-order.sh
+++ b/t/t6003-rev-list-topo-order.sh
@@ -79,7 +79,7 @@ save_tag g4 unique_commit g6 tree -p g3 -p h2
 
 git update-ref HEAD $(tag l5)
 
-test_output_expect_success 'rev-list has correct number of entries' 'git rev-list HEAD | wc -l | tr -d \" \"' <<EOF
+test_output_expect_success 'rev-list has correct number of entries' 'git rev-list HEAD | wc -l | test-tr -d \" \"' <<EOF
 19
 EOF
 
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 553131f..5c9bd2b 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -4,7 +4,7 @@ test_description='git-filter-branch'
 . ./test-lib.sh
 
 make_commit () {
-	lower=$(echo $1 | tr '[A-Z]' '[a-z]')
+	lower=$(echo $1 | test-tr 'A-Z' 'a-z')
 	echo $lower > $lower
 	git add $lower
 	test_tick
diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh
index 42b144b..6b4358c 100755
--- a/t/t9200-git-cvsexportcommit.sh
+++ b/t/t9200-git-cvsexportcommit.sh
@@ -35,7 +35,7 @@ check_entries () {
 	then
 		>expected
 	else
-		printf '%s\n' "$2" | tr '|' '\012' >expected
+		printf '%s\n' "$2" | test-tr '|' '\012' >expected
 	fi
 	test_cmp expected actual
 }
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 268b26c..4c7bf9a 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -44,11 +44,16 @@ export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
 export EDITOR VISUAL
 GIT_TEST_CMP=${GIT_TEST_CMP:-diff -u}
 
+# Test the binaries we have just built.  The tests are kept in
+# t/ subdirectory and are run in trash subdirectory.
+PATH=$(pwd)/..:$PATH
+export PATH
+
 # Protect ourselves from common misconfiguration to export
 # CDPATH into the environment
 unset CDPATH
 
-case $(echo $GIT_TRACE |tr "[A-Z]" "[a-z]") in
+case $(echo $GIT_TRACE |test-tr A-Z a-z) in
 	1|2|true)
 		echo "* warning: Some tests will not work if GIT_TRACE" \
 			"is set as to trace on STDERR ! *"
@@ -369,16 +374,13 @@ test_done () {
 	esac
 }
 
-# Test the binaries we have just built.  The tests are kept in
-# t/ subdirectory and are run in trash subdirectory.
-PATH=$(pwd)/..:$PATH
 GIT_EXEC_PATH=$(pwd)/..
 GIT_TEMPLATE_DIR=$(pwd)/../templates/blt
 unset GIT_CONFIG
 unset GIT_CONFIG_LOCAL
 GIT_CONFIG_NOSYSTEM=1
 GIT_CONFIG_NOGLOBAL=1
-export PATH GIT_EXEC_PATH GIT_TEMPLATE_DIR GIT_CONFIG_NOSYSTEM GIT_CONFIG_NOGLOBAL
+export GIT_EXEC_PATH GIT_TEMPLATE_DIR GIT_CONFIG_NOSYSTEM GIT_CONFIG_NOGLOBAL
 
 GITPERLLIB=$(pwd)/../perl/blib/lib:$(pwd)/../perl/blib/arch/auto/Git
 export GITPERLLIB
-- 
1.5.5.rc0.53.g97734

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

* Re: [PATCH 03/16] more tr portability test script fixes
  2008-03-18 22:23   ` Alex Riesen
  2008-03-18 22:24     ` [PATCH] Add test-tr: poor-man tr for the test suite Alex Riesen
@ 2008-03-18 22:44     ` Jeff King
  2008-03-18 23:24       ` Alex Riesen
  2008-03-19 21:40       ` Junio C Hamano
  1 sibling, 2 replies; 59+ messages in thread
From: Jeff King @ 2008-03-18 22:44 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Whit Armstrong, Junio C Hamano, git

On Tue, Mar 18, 2008 at 11:23:02PM +0100, Alex Riesen wrote:

> Jeff King, Wed, Mar 12, 2008 22:31:06 +0100:
> > -    tr '\000' '\012' <"$1" | sed -e "$sanitize_diff_raw_z" >.tmp-1
> > -    tr '\000' '\012' <"$2" | sed -e "$sanitize_diff_raw_z" >.tmp-2
> > +    perl -pe 'y/\000/\012/' <"$1" | sed -e "$sanitize_diff_raw_z" >.tmp-1
> > +    perl -pe 'y/\000/\012/' <"$2" | sed -e "$sanitize_diff_raw_z" >.tmp-2
> 
> These break in presence of ActiveState Perl on Windows.
> 
> I suggest replacing such simple construction with a simplified,
> in-tree, version of tr.

<sigh> It's sad that it must come to that, but your test-tr patches seem
like the only sane choice. They seem to work fine on my Solaris box.

Note that there are still a few uses of 'tr' in actual git scripts.
However, they are pretty tame, so I think they should work everywhere.
Otherwise, test-tr must become "git tr". :)

-Peff

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

* Re: [PATCH 03/16] more tr portability test script fixes
  2008-03-18 22:44     ` [PATCH 03/16] more tr portability test script fixes Jeff King
@ 2008-03-18 23:24       ` Alex Riesen
  2008-03-19 21:40       ` Junio C Hamano
  1 sibling, 0 replies; 59+ messages in thread
From: Alex Riesen @ 2008-03-18 23:24 UTC (permalink / raw)
  To: Jeff King; +Cc: Whit Armstrong, Junio C Hamano, git

Jeff King, Tue, Mar 18, 2008 23:44:37 +0100:
> On Tue, Mar 18, 2008 at 11:23:02PM +0100, Alex Riesen wrote:
> 
> > Jeff King, Wed, Mar 12, 2008 22:31:06 +0100:
> > > -    tr '\000' '\012' <"$1" | sed -e "$sanitize_diff_raw_z" >.tmp-1
> > > -    tr '\000' '\012' <"$2" | sed -e "$sanitize_diff_raw_z" >.tmp-2
> > > +    perl -pe 'y/\000/\012/' <"$1" | sed -e "$sanitize_diff_raw_z" >.tmp-1
> > > +    perl -pe 'y/\000/\012/' <"$2" | sed -e "$sanitize_diff_raw_z" >.tmp-2
> > 
> > These break in presence of ActiveState Perl on Windows.
> > 
> > I suggest replacing such simple construction with a simplified,
> > in-tree, version of tr.
> 
> <sigh> It's sad that it must come to that, but your test-tr patches seem
> like the only sane choice. They seem to work fine on my Solaris box.
> 
> Note that there are still a few uses of 'tr' in actual git scripts.
> However, they are pretty tame, so I think they should work everywhere.

Yes, I thought about them too and hope for the same (they'll do).

> Otherwise, test-tr must become "git tr". :)

We already use "git diff" and "git apply" as reliable "diff" and
"patch" :)

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

* Re: [PATCH 03/16] more tr portability test script fixes
  2008-03-18 22:44     ` [PATCH 03/16] more tr portability test script fixes Jeff King
  2008-03-18 23:24       ` Alex Riesen
@ 2008-03-19 21:40       ` Junio C Hamano
  2008-03-19 22:56         ` Alex Riesen
  1 sibling, 1 reply; 59+ messages in thread
From: Junio C Hamano @ 2008-03-19 21:40 UTC (permalink / raw)
  To: Jeff King; +Cc: Alex Riesen, Whit Armstrong, git

Jeff King <peff@peff.net> writes:

> On Tue, Mar 18, 2008 at 11:23:02PM +0100, Alex Riesen wrote:
>
>> Jeff King, Wed, Mar 12, 2008 22:31:06 +0100:
>> > -    tr '\000' '\012' <"$1" | sed -e "$sanitize_diff_raw_z" >.tmp-1
>> > -    tr '\000' '\012' <"$2" | sed -e "$sanitize_diff_raw_z" >.tmp-2
>> > +    perl -pe 'y/\000/\012/' <"$1" | sed -e "$sanitize_diff_raw_z" >.tmp-1
>> > +    perl -pe 'y/\000/\012/' <"$2" | sed -e "$sanitize_diff_raw_z" >.tmp-2
>> 
>> These break in presence of ActiveState Perl on Windows.
>> 
>> I suggest replacing such simple construction with a simplified,
>> in-tree, version of tr.
>
> <sigh> It's sad that it must come to that, but your test-tr patches seem
> like the only sane choice. They seem to work fine on my Solaris box.

I am very tempted to say that it might make more sense to declare
ActiveState unsupported.  How many times have we suffered from it, and
notice it was only from Alex every time, nobody else?  Are there silent
majorities involved here?

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

* Re: [PATCH 03/16] more tr portability test script fixes
  2008-03-19 21:40       ` Junio C Hamano
@ 2008-03-19 22:56         ` Alex Riesen
  0 siblings, 0 replies; 59+ messages in thread
From: Alex Riesen @ 2008-03-19 22:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Whit Armstrong, git

Junio C Hamano, Wed, Mar 19, 2008 22:40:57 +0100:
> Jeff King <peff@peff.net> writes:
> 
> > On Tue, Mar 18, 2008 at 11:23:02PM +0100, Alex Riesen wrote:
> >
> >> Jeff King, Wed, Mar 12, 2008 22:31:06 +0100:
> >> > -    tr '\000' '\012' <"$1" | sed -e "$sanitize_diff_raw_z" >.tmp-1
> >> > -    tr '\000' '\012' <"$2" | sed -e "$sanitize_diff_raw_z" >.tmp-2
> >> > +    perl -pe 'y/\000/\012/' <"$1" | sed -e "$sanitize_diff_raw_z" >.tmp-1
> >> > +    perl -pe 'y/\000/\012/' <"$2" | sed -e "$sanitize_diff_raw_z" >.tmp-2
> >> 
> >> These break in presence of ActiveState Perl on Windows.
> >> 
> >> I suggest replacing such simple construction with a simplified,
> >> in-tree, version of tr.
> >
> > <sigh> It's sad that it must come to that, but your test-tr patches seem
> > like the only sane choice. They seem to work fine on my Solaris box.
> 
> I am very tempted to say that it might make more sense to declare
> ActiveState unsupported.

I am not asking to really support it, it is too offending a thought.
I am just asking to consider the patch, which removes another
dependency on local system. Wasn't there issues with tr already?

> How many times have we suffered from it, and notice it was only from
> Alex every time, nobody else?

Well, I say that every time. I think by know everyone, who seen a
patch from me, noticed how very specially stupid my kind of setup is.

BTW, who suffered from what?

> Are there silent majorities involved here?

Yeah, the windows-people. They are used to silently workaround their
system.

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

end of thread, other threads:[~2008-03-19 22:57 UTC | newest]

Thread overview: 59+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <cover.1205356737.git.peff@peff.net>
2008-03-12 21:29 ` [PATCH 01/16] tr portability fixes Jeff King
2008-03-13  7:32   ` Johannes Sixt
2008-03-13 13:06     ` Jeff King
2008-03-12 21:30 ` [PATCH 02/16] t0050: perl portability fix Jeff King
2008-03-13  7:38   ` Johannes Sixt
2008-03-12 21:31 ` [PATCH 03/16] more tr portability test script fixes Jeff King
2008-03-13  8:28   ` Frank Lichtenheld
2008-03-13 13:09     ` Jeff King
2008-03-18 22:23   ` Alex Riesen
2008-03-18 22:24     ` [PATCH] Add test-tr: poor-man tr for the test suite Alex Riesen
2008-03-18 22:24       ` [PATCH] Use test-tr in " Alex Riesen
2008-03-18 22:44     ` [PATCH 03/16] more tr portability test script fixes Jeff King
2008-03-18 23:24       ` Alex Riesen
2008-03-19 21:40       ` Junio C Hamano
2008-03-19 22:56         ` Alex Riesen
2008-03-12 21:32 ` [PATCH 04/16] grep portability fix: don't use "-e" or "-q" Jeff King
2008-03-12 22:10   ` Junio C Hamano
2008-03-12 22:45     ` Junio C Hamano
2008-03-12 21:34 ` [PATCH 05/16] remove use of "tail -n 1" and "tail -1" Jeff King
2008-03-12 21:36 ` [PATCH 06/16] add test_cmp function for test scripts Jeff King
2008-03-12 22:12   ` Junio C Hamano
2008-03-13 12:08     ` Jeff King
2008-03-13 20:48       ` Junio C Hamano
2008-03-12 21:37 ` [PATCH 07/16] t4020: don't use grep -a Jeff King
2008-03-12 21:37 ` [PATCH 08/16] t4200: use cut instead of sed Jeff King
2008-03-13  4:52   ` Junio C Hamano
2008-03-13 12:59     ` Jeff King
2008-03-13 18:00       ` Junio C Hamano
2008-03-14 21:40         ` Jeff King
2008-03-12 21:38 ` [PATCH 09/16] t6000lib: tr portability fix Jeff King
2008-03-14 20:47   ` [PATCH] t/t6000lib.sh: tr portability fix fix Brandon Casey
2008-03-14 20:54     ` Jeff King
2008-03-14 21:00       ` Brandon Casey
2008-03-14 21:26       ` Brandon Casey
2008-03-14 21:45         ` Jeff King
2008-03-14 22:06           ` Brandon Casey
2008-03-12 21:39 ` [PATCH 10/16] add NO_EXTERNAL_GREP build option Jeff King
2008-03-12 22:30   ` Junio C Hamano
2008-03-13 12:10     ` Jeff King
2008-03-13  7:50   ` Johannes Sixt
2008-03-13 12:41     ` Jeff King
2008-03-13 13:59       ` Johannes Sixt
2008-03-13 14:04         ` Jeff King
2008-03-13 14:09           ` Johannes Sixt
2008-03-13  7:56   ` Junio C Hamano
2008-03-12 21:40 ` [PATCH 11/16] config: add --literal-match option Jeff King
2008-03-12 21:46   ` Jakub Narebski
2008-03-13 13:24     ` Jeff King
2008-03-12 22:34   ` Junio C Hamano
2008-03-13 12:42     ` Jeff King
2008-03-12 21:40 ` [PATCH 12/16] git-submodule: avoid sed input with no newline Jeff King
2008-03-12 22:41   ` Junio C Hamano
2008-03-13 12:46     ` Jeff King
2008-03-12 21:41 ` [PATCH 13/16] filter-branch: don't use xargs -0 Jeff King
2008-03-12 21:41 ` [PATCH 14/16] filter-branch: use $SHELL_PATH instead of 'sh' Jeff King
2008-03-12 21:42 ` [PATCH 15/16] t9112: add missing #!/bin/sh header Jeff King
2008-03-12 21:42 ` [PATCH 16/16] t7505: use SHELL_PATH in hook Jeff King
2008-03-13  7:11   ` Adam Piatyszek
2008-03-13 13:00     ` Jeff King

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