Skip to contentSkip to conversation

tr portability fixes

PATCH In review Jeff King 8 participants 59 messages Jun 15, 2016 1 message not in the archive

Subsystems (2)
lore
Share & tools
lore

Jeff King

author
#1 lore raw

PATCH 1/16 tr portability fixes

In reply to a message not in the archive

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

---
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(-)
git-bisect.sh +2 -2
index 2c32d0b..48fb92d 100755
@@ -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") &&
git-filter-branch.sh +2 -2
index 010353a..59cf023 100755
@@ -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
t/t4022-diff-rewrite.sh +4 -1
index 6de4acb..bf996fc 100755
@@ -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
 
 '
 
t/t7003-filter-branch.sh +1 -1
index 6e14bf1..553131f 100755
@@ -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
Signature · 1 line
1.5.4.4.543.g30fdd.dirty
#2 lore raw
Quoted 5 lines · Jeff King

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

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

Quoted 7 lines

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

author
#3 lore raw
Quoted 10 lines · Johannes Sixt
Quoted 4 lines

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

author
#4 lore raw

PATCH 2/16 t0050: perl portability fix

In reply to a message not in the archive

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.

---
 t/t0050-filesystem.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
t/t0050-filesystem.sh +2 -2
index cd088b3..3fbad77 100755
@@ -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 ' '
 
Signature · 1 line
1.5.4.4.543.g30fdd.dirty
Tested-by on v1 2/16
#5 lore raw

Re: [PATCH 02/16] t0050: perl portability fix

Quoted 5 lines · Jeff King

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.

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

-- Hannes

author
#6 lore raw

PATCH 3/16 more tr portability test script fixes

In reply to a message not in the archive

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.

---
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(-)
t/diff-lib.sh +2 -2
index 7dc6d7e..28b941c 100644
@@ -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
 }
 
t/t0020-crlf.sh +1 -1
index 90ea081..2bfeac9 100755
@@ -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 () {
t/t1300-repo-config.sh +2 -2
index 4928a57..b36a901 100755
@@ -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'
t/t3300-funny-names.sh +3 -3
index 98c133d..24a00a9 100755
@@ -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
t/t4020-diff-external.sh +1 -1
index 8882933..bf8f778 100755
@@ -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" &&
t/t4103-apply-binary.sh +2 -2
index 7c25634..1b58233 100755
@@ -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'
t/t4116-apply-reverse.sh +2 -2
index b1d35ab..c3f4579 100755
@@ -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 &&
t/t4200-rerere.sh +1 -1
index eeff3c9..3cbfee7 100755
@@ -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'
 
t/t5300-pack-object.sh +1 -1
index cd3c149..c955fe4 100755
@@ -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 &&
test-sha1.sh +2 -2
index bf526c8..0f0bc5d 100755
@@ -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/ .*//'
 	`
Signature · 1 line
1.5.4.4.543.g30fdd.dirty
#7 lore raw

Re: [PATCH 03/16] more tr portability test script fixes

t/diff-lib.sh compare_diff_raw_z () { · line 25

Quoted 10 lines · Jeff King
t/diff-lib.sh +2 -2
@@ -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,

Signature · 2 lines
Frank Lichtenheld <[email protected]>
www: http://www.djpig.de/
author
#8 lore raw

Re: [PATCH 03/16] more tr portability test script fixes

Quoted 7 lines · Frank Lichtenheld

t/diff-lib.sh compare_diff_raw_z () { · line 25

Quoted 4 lines
t/diff-lib.sh +2 -2
-    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

author
#16 lore raw

PATCH 4/16 grep portability fix: don't use "-e" or "-q"

In reply to a message not in the archive

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.

---
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(-)
git-rebase--interactive.sh +3 -3
index c2bedd6..4c3280a 100755
@@ -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 () {
git-submodule.sh +3 -3
index 7171cb6..ceb2295 100755
@@ -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
t/t0030-stripspace.sh +17 -17
index 818c862..3ecdd66 100755
@@ -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 \
t/t3404-rebase-interactive.sh +1 -2
index 049aa37..f098231 100755
@@ -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' '
t/t3800-mktag.sh +1 -1
index f280320..2780758 100755
@@ -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
 	'
 }
 
t/t5400-send-pack.sh +1 -1
index 2d0c07f..2b6b6e3 100755
@@ -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() {
t/t7502-status.sh +1 -1
index e006074..70b802b 100755
@@ -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
 
 '
 
t/t7600-merge.sh +2 -2
index 5d16628..590505b 100755
@@ -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
t/t9400-git-cvsserver-server.sh +13 -13
index 0a20971..e82b365 100755
@@ -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
Signature · 1 line
1.5.4.4.543.g30fdd.dirty
#17 lore raw

Re: [PATCH 04/16] grep portability fix: don't use "-e" or "-q"

Quoted 2 lines · Jeff King

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?

#18 lore raw

Re: [PATCH 04/16] grep portability fix: don't use "-e" or "-q"

Quoted 9 lines · Junio C Hamano
Quoted 2 lines · Jeff King

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

Quoted 7 lines

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

author
#19 lore raw

PATCH 5/16 remove use of "tail -n 1" and "tail -1"

In reply to a message not in the archive

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'

---
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(-)
git-am.sh +1 -1
index 1f6b5e0..ac5c388 100755
@@ -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" || {
git-rebase--interactive.sh +1 -1
index 4c3280a..8aa7371 100755
@@ -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
t/t3404-rebase-interactive.sh +2 -2
index f098231..9c0acc5 100755
@@ -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
t/t5302-pack-index.sh +2 -2
index 67b9a7b..b88b5bb 100755
@@ -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" &&
t/t6030-bisect-porcelain.sh +3 -3
index 4908e87..f471c15 100755
@@ -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 &&
t/t7600-merge.sh +1 -1
index 590505b..219411f 100755
@@ -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"
t/t9400-git-cvsserver-server.sh +10 -10
index e82b365..b91b151 100755
@@ -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
 
Signature · 1 line
1.5.4.4.543.g30fdd.dirty
author
#20 lore raw

PATCH 6/16 add test_cmp function for test scripts

In reply to a message not in the archive

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

---
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(-)
t/t0003-attributes.sh +1 -1
index 47f08a4..3faf135 100755
@@ -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
 
 }
 
t/t0022-crlf-rename.sh +1 -1
index 430a1d1..7d1ce2d 100755
@@ -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
 
 '
 
t/t1005-read-tree-reset.sh +1 -1
index 8c45564..b0d31f5 100755
@@ -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
t/t2200-add-update.sh +1 -1
index 24f892f..b664341 100755
@@ -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
 
 '
 
t/t3001-ls-files-others-exclude.sh +1 -1
index b4297ba..55f057c 100755
@@ -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)' '
 
t/t3050-subprojects-fetch.sh +2 -2
index 34f26a8..2b21b10 100755
@@ -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
t/t3060-ls-files-with-tree.sh +1 -1
index 68eb266..3ce501b 100755
@@ -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
t/t3201-branch-contains.sh +3 -3
index 9ef593f..b4cf628 100755
@@ -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
 
 '
 
t/t3404-rebase-interactive.sh +2 -2
index 9c0acc5..9cf873f 100755
@@ -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)
 '
t/t3405-rebase-malformed.sh +2 -2
index e4e2e64..e5ad67c 100755
@@ -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
t/t3406-rebase-message.sh +1 -1
index 332b2b2..5391080 100755
@@ -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
 
 '
 
t/t3701-add-interactive.sh +2 -2
index c8dc1ac..77c90f6 100755
@@ -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 &&
t/t3902-quoted.sh +8 -8
index 73da45f..fe4fb51 100755
@@ -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
 
 '
 
t/t3903-stash.sh +1 -1
index aa282e1..2d3ee3b 100755
@@ -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' '
t/t4023-diff-rename-typechange.sh +3 -3
index 255604e..4dbfc6e 100755
@@ -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
 
 '
 
t/t4024-diff-optimize-common.sh +1 -1
index 3c66102..c4d733f 100755
@@ -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
 
 '
 
t/t4025-hunk-header.sh +1 -1
index 9ba06b7..7a3dbc1 100755
@@ -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
 
 '
 
t/t4027-diff-submodule.sh +3 -3
index 3d2d081..1fd3fb7 100755
@@ -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
t/t4105-apply-fuzz.sh +1 -1
index 0e8d25f..3266e39 100755
@@ -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
 	"
 }
 
t/t4125-apply-ws-fuzz.sh +4 -4
index d6f15be..3b471b6 100755
@@ -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
 
 '
 
t/t4150-am-subdir.sh +5 -5
index 929d2cb..52069b4 100755
@@ -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
t/t4201-shortlog.sh +1 -1
index 6d12efb..91ea696 100755
@@ -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
t/t5505-remote.sh +3 -3
index 2822a65..ecfc999 100755
@@ -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
 )
 '
 
t/t5510-fetch.sh +1 -1
index 9b948c1..6946557 100755
@@ -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
 
 '
 
t/t5512-ls-remote.sh +4 -4
index 6ec5f7c..c0dc949 100755
@@ -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
 
 '
 
t/t6004-rev-list-path-optim.sh +1 -1
index 80d7198..5dabf1c 100755
@@ -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
t/t6009-rev-list-parent.sh +1 -1
index be3d238..f248a32 100755
@@ -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
 
 '
 
t/t6027-merge-binary.sh +2 -2
index a7358f7..92ca1f0 100755
@@ -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
 '
 
t/t6029-merge-subtree.sh +1 -1
index 35d66e8..43f5459 100755
@@ -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
 
 '
 
t/t7010-setup.sh +9 -9
index bc8ab6a..02cf7c5 100755
@@ -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' '
t/t7201-co.sh +9 -9
index 63915cd..3111baa 100755
@@ -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" &&
t/t7501-commit.sh +7 -7
index 361886c..c0288f3 100755
@@ -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
 
 '
 
t/t7502-commit.sh +7 -7
index b780fdd..284c941 100755
@@ -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
 
 '
 
t/t7502-status.sh +1 -1
index 70b802b..cd08516 100755
@@ -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
t/t7600-merge.sh +1 -1
index 219411f..56869ac 100755
@@ -108,7 +108,7 @@ create_merge_msgs() {
 }
 
 verify_diff() {
-	if ! diff -u "$1" "$2"
+	if ! test_cmp "$1" "$2"
 	then
 		echo "$3"
 		false
t/t8003-blame.sh +2 -2
index db51b3a..966bb0a 100755
@@ -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
 
 '
 
t/t9001-send-email.sh +1 -1
index cbbfa9c..c0973b4 100755
@@ -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
t/t9116-git-svn-log.sh +12 -12
index 902ed41..e1e8bdf 100755
@@ -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
t/t9200-git-cvsexportcommit.sh +7 -7
index 58c59ed..42b144b 100755
@@ -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"
 
 '
 
t/test-lib.sh +18 -0
index 6aea0ea..268b26c 100644
@@ -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 () {
Signature · 1 line
1.5.4.4.543.g30fdd.dirty
#21 lore raw

Re: [PATCH 06/16] add test_cmp function for test scripts

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

author
#22 lore raw

Re: [PATCH 06/16] add test_cmp function for test scripts

Quoted 1 line · Junio C Hamano

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

#23 lore raw

Re: [PATCH 06/16] add test_cmp function for test scripts

Quoted 7 lines · Jeff King
Quoted 1 line · Junio C Hamano

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.

Quoted 6 lines

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

author
#24 lore raw

PATCH 7/16 t4020: don't use grep -a

In reply to a message not in the archive

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.

---
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
t/t4020-diff-external.sh +2 -1
index bf8f778..637b4e1 100755
@@ -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
t/t4020/diff.NUL new file binary +0 -0
new file mode 100644
index 0000000000000000000000000000000000000000..db2f89090c1c4de05e4f82ea39ea118fccfd48dd
GIT binary patch
Signature · 1 line
1.5.4.4.543.g30fdd.dirty
author
#25 lore raw

PATCH 8/16 t4200: use cut instead of sed

In reply to a message not in the archive

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.

---
 t/t4200-rerere.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
t/t4200-rerere.sh +1 -1
index 3cbfee7..d3dea2a 100755
@@ -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"
 
Signature · 1 line
1.5.4.4.543.g30fdd.dirty
#26 lore raw

Re: [PATCH 08/16] t4200: use cut instead of sed

Quoted 5 lines · Jeff King

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

author
#27 lore raw

Re: [PATCH 08/16] t4200: use cut instead of sed

Quoted 7 lines · Junio C Hamano
Quoted 5 lines

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:

builtin-rerere.c +2 -1
index c607aad..e4a1dc1 100644
@@ -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

#28 lore raw

Re: [PATCH 08/16] t4200: use cut instead of sed

Quoted 9 lines · Jeff King

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:

builtin-rerere.c +0 -0
index c607aad..e4a1dc1 100644

builtin-rerere.c static int write_rr(struct path_list *rr, int out_fd) · line 65

Quoted 9 lines
builtin-rerere.c +2 -1
@@ -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).

Quoted 2 lines

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.

author
#30 lore raw

PATCH 9/16 t6000lib: tr portability fix

In reply to a message not in the archive

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.

---
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(-)
t/t6000lib.sh +4 -1
index 180633e..b69f7c4 100755
@@ -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/-*\$//"
 }
 
 
Signature · 1 line
1.5.4.4.543.g30fdd.dirty
author
#37 lore raw

PATCH 10/16 add NO_EXTERNAL_GREP build option

In reply to a message not in the archive

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

---
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(-)
Makefile +7 -0
index e3eaa6a..8e80225 100644
@@ -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
builtin-grep.c +2 -2
index f4f4ecb..f215b28 100644
@@ -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
Signature · 1 line
1.5.4.4.543.g30fdd.dirty
#38 lore raw

Re: [PATCH 10/16] add NO_EXTERNAL_GREP build option

Quoted 8 lines · Jeff King

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

Quoted 4 lines

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

author
#39 lore raw

Re: [PATCH 10/16] add NO_EXTERNAL_GREP build option

Quoted 7 lines · Johannes Sixt
Quoted 2 lines

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.

---
 builtin-grep.c |   32 ++++----------------------------
 1 files changed, 4 insertions(+), 28 deletions(-)
builtin-grep.c +4 -28
index ef29910..44d9bac 100644
@@ -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)
Signature · 1 line
1.5.4.4.553.g83e84.dirty
#40 lore raw

Re: [PATCH 10/16] add NO_EXTERNAL_GREP build option

Quoted 2 lines · Jeff King

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.

Quoted 13 lines
builtin-grep.c +0 -13
-	while (waitpid(pid, &status, 0) < 0) {
-		if (errno == EINTR)
-			continue;
-		return -1;
-	}
-	if (WIFEXITED(status)) {
-		if (!WEXITSTATUS(status))
-			return 1;
-		return 0;
-	}
-	return -1;
-}
-

...

Quoted 1 line
builtin-grep.c +1 -0
+	status = run_command_v_opt(argv, 0);

...

builtin-grep.c static int flush_grep(struct grep_opt *opt, · line 243

Quoted 1 line
builtin-grep.c +1 -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

author
#41 lore raw

Re: [PATCH 10/16] add NO_EXTERNAL_GREP build option

Quoted 9 lines · Johannes Sixt
Quoted 1 line
builtin-grep.c +1 -0
+	status = run_command_v_opt(argv, 0);

...

builtin-grep.c static int flush_grep(struct grep_opt *opt, · line 243

Quoted 1 line
builtin-grep.c +1 -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

#42 lore raw

Re: [PATCH 10/16] add NO_EXTERNAL_GREP build option

Quoted 11 lines · Jeff King
Quoted 8 lines · Johannes Sixt
Quoted 1 line
builtin-grep.c +1 -0
+	status = run_command_v_opt(argv, 0);

...

builtin-grep.c static int flush_grep(struct grep_opt *opt, · line 243

Quoted 1 line
builtin-grep.c +1 -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'.

Quoted 3 lines

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

#43 lore raw

Re: [PATCH 10/16] add NO_EXTERNAL_GREP build option

In reply to Jeff King
Quoted 18 lines · Jeff King

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

---
This might have fallouts for msysgit (i.e., they need to define NO_EXTERNAL_GREP instead of relying on __unix__ not being defined).
...

builtin-grep.c +0 -0
index f4f4ecb..f215b28 100644

builtin-grep.c static int grep_file(struct grep_opt *opt, const char *filename) · line 156

Quoted 5 lines
builtin-grep.c +1 -1
@@ -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
author
#44 lore raw

Re: [PATCH 10/16] add NO_EXTERNAL_GREP build option

Quoted 10 lines · Junio C Hamano

Perhaps place

    #ifndef NO_EXTERNAL_GREP
    #ifndef __unix__
    #define NO_EXTERNAL_GREP 1
    #else
    #define NO_EXTERNAL_GREP 0
    #endif
    #endif
Quoted 5 lines

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

#45 lore raw

Re: [PATCH 10/16] add NO_EXTERNAL_GREP build option

In reply to Jeff King

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(-)
Makefile +2 -3
index 8e80225..bc46fd4 100644
@@ -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
builtin-grep.c +10 -2
index f215b28..ef29910 100644
@@ -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
author
#46 lore raw

PATCH 11/16 config: add --literal-match option

In reply to a message not in the archive

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.

---
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(-)
Documentation/git-config.txt +7 -0
index fa16171..5dc1af2 100644
@@ -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
 -----
builtin-config.c +37 -13

120 lines not on this page · raw message

builtin-remote.c +1 -1
index 24e6929..4eae74b 100644
@@ -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;
 	}
 
cache.h +1 -1
index 2a1e7ec..30830b0 100644
@@ -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);
config.c +45 -22

120 lines not on this page · raw message

t/t1300-repo-config.sh +13 -0
index b36a901..6c5ccdd 100755
@@ -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
Signature · 1 line
1.5.4.4.543.g30fdd.dirty
#47 lore raw

Re: [PATCH 11/16] config: add --literal-match option

Quoted 6 lines · Jeff King

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.

Quoted 8 lines

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

author
#48 lore raw

Re: [PATCH 11/16] config: add --literal-match option

Quoted 4 lines · Junio C Hamano
Quoted 3 lines

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.

Quoted 8 lines

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

#49 lore raw

Re: [PATCH 11/16] config: add --literal-match option

In reply to Jeff King

Documentation/git-config.txt See also <<FILES>>. · line 161

Quoted 7 lines · Jeff King

+--literal-match::

Documentation/git-config.txt +6 -0
+
+       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?

Signature · 3 lines
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
author
#50 lore raw

Re: [PATCH 11/16] config: add --literal-match option

Quoted 9 lines · Jakub Narebski

Documentation/git-config.txt See also <<FILES>>. · line 161

Quoted 7 lines

+--literal-match::

Documentation/git-config.txt +6 -0
+
+       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

author
#51 lore raw

PATCH 12/16 git-submodule: avoid sed input with no newline

In reply to a message not in the archive

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.

---
And this is safe because of the --literal-match from the last patch.

 git-submodule.sh |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)
git-submodule.sh +6 -7
index ceb2295..fb01d94 100755
@@ -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"
 }
 
 #
Signature · 1 line
1.5.4.4.543.g30fdd.dirty
#52 lore raw

Re: [PATCH 12/16] git-submodule: avoid sed input with no newline

Quoted 3 lines · Jeff King

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

Quoted 7 lines
git-submodule.sh +6 -0
+	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"
 }
author
#53 lore raw

Re: [PATCH 12/16] git-submodule: avoid sed input with no newline

Quoted 7 lines · Junio C Hamano

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

author
#54 lore raw

PATCH 13/16 filter-branch: don't use xargs -0

In reply to a message not in the archive

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

---
 git-filter-branch.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
git-filter-branch.sh +1 -1
index 59cf023..efef732 100755
@@ -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"
 
Signature · 1 line
1.5.4.4.543.g30fdd.dirty
author
#55 lore raw

PATCH 14/16 filter-branch: use $SHELL_PATH instead of 'sh'

In reply to a message not in the archive

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.

---
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(-)
Makefile +1 -0
index 8e80225..bd5a623 100644
@@ -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' \
git-filter-branch.sh +1 -1
index efef732..22b6ed4 100755
@@ -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
 
Signature · 1 line
1.5.4.4.543.g30fdd.dirty
author
#56 lore raw

PATCH 15/16 t9112: add missing #!/bin/sh header

In reply to a message not in the archive

---
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(-)
t/t9112-git-svn-md5less-file.sh +2 -0
index 08313bb..646a5f0 100755
@@ -1,3 +1,5 @@
+#!/bin/sh
+
 test_description='test that git handles an svn repository with missing md5sums'
 
 . ./lib-git-svn.sh
Signature · 1 line
1.5.4.4.543.g30fdd.dirty
author
#57 lore raw

PATCH 16/16 t7505: use SHELL_PATH in hook

In reply to a message not in the archive

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

---
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(-)
t/t7505-prepare-commit-msg-hook.sh +2 -1
index 7ddec99..fd67996 100755
@@ -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")
Signature · 1 line
1.5.4.4.543.g30fdd.dirty
#58 lore raw

Re: [PATCH 16/16] t7505: use SHELL_PATH in hook

Hi!

Quoted 2 lines · Jeff King

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

[...]

t/t7505-prepare-commit-msg-hook.sh export FAKE_EDITOR · line 30

Quoted 10 lines
t/t7505-prepare-commit-msg-hook.sh +2 -1
@@ -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

Signature · 2 lines
.:.  Adam Piatyszek (ediap)  .:.....................................:.
.:.  [email protected]  .:................................:.
author
#59 lore raw

Re: [PATCH 16/16] t7505: use SHELL_PATH in hook

Quoted 6 lines · Adam Piatyszek

t/t7505-prepare-commit-msg-hook.sh export FAKE_EDITOR · line 30

Quoted 4 lines
t/t7505-prepare-commit-msg-hook.sh +2 -1
-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

author
#29 lore raw

Re: [PATCH 08/16] t4200: use cut instead of sed

In reply to Junio C Hamano
Quoted 9 lines · Junio C Hamano
Quoted 9 lines

index c607aad..e4a1dc1 100644

builtin-rerere.c +1 -1
@@ -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 ||
Quoted 8 lines

builtin-rerere.c static int write_rr(struct path_list *rr, int out_fd) · line 65

Quoted 4 lines
builtin-rerere.c +1 -0
+		    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

#31 lore raw

PATCH 1/1 t/t6000lib.sh: tr portability fix fix

In reply to Jeff King

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.

---

Quoted 8 lines · Jeff King

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.

---
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(-)
t/t6000lib.sh +1 -1
index b69f7c4..aac6a31 100755
@@ -98,7 +98,7 @@ check_output()
 name_from_description()
 {
         tr "'" '-' |
-		tr '~`!@#$%^&*()_+={}[]|\;:"<>,/? ' \
+		tr -- '~`!@#$%^&*()_+={}[]|\;:"<>,/? ' \
 		   '------------------------------' |
 		tr -s '-' | tr '[A-Z]' '[a-z]' | sed "s/^-*//;s/-*\$//"
 }
Signature · 1 line
1.5.4.4.481.g5075
author
#32 lore raw

Re: [PATCH] t/t6000lib.sh: tr portability fix fix

Quoted 4 lines · Brandon Casey

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

#33 lore raw

Re: [PATCH] t/t6000lib.sh: tr portability fix fix

Quoted 6 lines · Jeff King
Quoted 2 lines · Brandon Casey

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

#34 lore raw

PATCH 1/1 t/t6000lib.sh: tr portability fix fix

In reply to Jeff King

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.

---

Quoted 11 lines · Jeff King
Quoted 4 lines · Brandon Casey

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(-)
t/t6000lib.sh +1 -1
index b69f7c4..71f2140 100755
@@ -99,7 +99,7 @@ name_from_description()
 {
         tr "'" '-' |
 		tr '~`!@#$%^&*()_+={}[]|\;:"<>,/? ' \
-		   '------------------------------' |
+		   '\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-' |
 		tr -s '-' | tr '[A-Z]' '[a-z]' | sed "s/^-*//;s/-*\$//"
 }
 
Signature · 1 line
1.5.4.4.481.g5075
author
#35 lore raw

Re: [PATCH] t/t6000lib.sh: tr portability fix fix

t/t6000lib.sh name_from_description() · line 102

Quoted 7 lines · Brandon Casey

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

[...]

t/t6000lib.sh +1 -1
-		   '------------------------------' |
+		   '\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-' |

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

#36 lore raw

Re: [PATCH] t/t6000lib.sh: tr portability fix fix

Quoted 5 lines · Jeff King
Quoted 3 lines · Brandon Casey

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

Quoted 8 lines

t/t6000lib.sh name_from_description() · line 102

Quoted 4 lines

[...]

t/t6000lib.sh +1 -1
-		   '------------------------------' |
+		   '\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-' |

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

#9 lore raw

Re: [PATCH 03/16] more tr portability test script fixes

In reply to Jeff King

Jeff King, Wed, Mar 12, 2008 22:31:06 +0100:

t/diff-lib.sh compare_diff_raw_z () { · line 25

Quoted 4 lines
t/diff-lib.sh +2 -2
-    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

#10 lore raw

PATCH 1/1 Add test-tr: poor-man tr for the test suite

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

---

 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
Makefile +1 -1
index 7c70b00..a458c30 100644
@@ -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)
 
t/t0000-test-progs.sh new file +72 -0

72 lines not on this page · raw message

test-tr.c new file +236 -0

236 lines not on this page · raw message

Signature · 1 line
1.5.5.rc0.53.g97734
#11 lore raw

PATCH 1/1 Use test-tr in the test suite

---
 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(-)
t/annotate-tests.sh +2 -2
index cacb273..8e10323 100644
@@ -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 \
t/diff-lib.sh +2 -2
index 28b941c..257e123 100644
@@ -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
 }
 
t/t0000-basic.sh +1 -1
index 27b54cb..a1f60eb 100755
@@ -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) &&
t/t0020-crlf.sh +5 -5
index 2bfeac9..7427d59 100755
@@ -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 '
t/t0021-conversion.sh +1 -1
index 8fc39d7..d146db4 100755
@@ -5,7 +5,7 @@ test_description='blob conversion via gitattributes'
 . ./test-lib.sh
 
 cat <<\EOF >rot13.sh
-tr \
+test-tr \
   'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ' \
   'nopqrstuvwxyzabcdefghijklmNOPQRSTUVWXYZABCDEFGHIJKLM'
 EOF
t/t1300-repo-config.sh +2 -2
index b36a901..42c3f15 100755
@@ -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'
t/t3300-funny-names.sh +3 -3
index 24a00a9..10c5217 100755
@@ -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
t/t3402-rebase-merge.sh +2 -2
index 7b7d072..704f9a3 100755
@@ -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." &&
 
t/t4004-diff-rename-symlink.sh +1 -1
index 3d25be7..b49efa4 100755
@@ -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) &&
t/t4015-diff-whitespace.sh +3 -3
index 83c54b7..595c5e2 100755
@@ -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
t/t4019-diff-wserror.sh +1 -1
index 0d9cbb6..4967026 100755
@@ -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
 
 '
t/t4020-diff-external.sh +1 -1
index 637b4e1..3ae3165 100755
@@ -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" &&
t/t4022-diff-rewrite.sh +1 -1
index bf996fc..01d221b 100755
@@ -8,7 +8,7 @@ test_expect_success setup '
 
 	cat ../../COPYING >test &&
 	git add test &&
-	tr \
+	test-tr \
 	  "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ" \
 	  "nopqrstuvwxyzabcdefghijklmNOPQRSTUVWXYZABCDEFGHIJKLM" \
 	  <../../COPYING >test
t/t4101-apply-nonl.sh +2 -2
index da8abcf..53277e6 100755
@@ -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
t/t4103-apply-binary.sh +2 -2
index 1b58233..f7542a0 100755
@@ -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'
t/t4116-apply-reverse.sh +2 -2
index c3f4579..bf01619 100755
@@ -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 &&
t/t4118-apply-empty-context.sh +2 -2
index 1d531ca..5083100 100755
@@ -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 |
t/t4200-rerere.sh +1 -1
index 3cbfee7..58fc740 100755
@@ -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'
 
t/t4201-shortlog.sh +2 -2
index 405b971..1a4b88d 100755
@@ -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
t/t5300-pack-object.sh +1 -1
index c955fe4..ba49742 100755
@@ -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 &&
t/t5500-fetch-pack.sh +1 -1
index 788b4a5..cda09b5 100755
@@ -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 () {
t/t5505-remote.sh +2 -2
index ecfc999..89f06b2 100755
@@ -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
 }
 
t/t6003-rev-list-topo-order.sh +1 -1
index 5daa0be..5dbc6a7 100755
@@ -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
 
t/t7003-filter-branch.sh +1 -1
index 553131f..5c9bd2b 100755
@@ -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
t/t9200-git-cvsexportcommit.sh +1 -1
index 42b144b..6b4358c 100755
@@ -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
 }
t/test-lib.sh +7 -5
index 268b26c..4c7bf9a 100644
@@ -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
Signature · 1 line
1.5.5.rc0.53.g97734
author
#12 lore raw

Re: [PATCH 03/16] more tr portability test script fixes

In reply to Alex Riesen
Quoted 10 lines · Alex Riesen

Jeff King, Wed, Mar 12, 2008 22:31:06 +0100:

t/diff-lib.sh compare_diff_raw_z () { · line 25

Quoted 4 lines
t/diff-lib.sh +2 -2
-    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

#13 lore raw

Re: [PATCH 03/16] more tr portability test script fixes

Jeff King, Tue, Mar 18, 2008 23:44:37 +0100:

Quoted 10 lines · Alex Riesen
Quoted 9 lines

Jeff King, Wed, Mar 12, 2008 22:31:06 +0100:

t/diff-lib.sh compare_diff_raw_z () { · line 25

Quoted 4 lines
t/diff-lib.sh +2 -2
-    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.

Quoted 8 lines
Quoted 2 lines

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

Quoted 1 line

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

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

#14 lore raw

Re: [PATCH 03/16] more tr portability test script fixes

In reply to Jeff King
Quoted 8 lines · Jeff King
Quoted 6 lines · Alex Riesen

Jeff King, Wed, Mar 12, 2008 22:31:06 +0100:

Quoted 4 lines
t/diff-lib.sh +2 -2
-    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
Quoted 7 lines
Quoted 4 lines

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?

#15 lore raw

Re: [PATCH 03/16] more tr portability test script fixes

Junio C Hamano, Wed, Mar 19, 2008 22:40:57 +0100:

Quoted 12 lines
Quoted 10 lines · Jeff King
Quoted 8 lines · Alex Riesen

Jeff King, Wed, Mar 12, 2008 22:31:06 +0100:

t/diff-lib.sh compare_diff_raw_z () { · line 25

Quoted 4 lines
t/diff-lib.sh +2 -2
-    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.

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

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?

Quoted 2 lines

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?

Quoted 1 line

Are there silent majorities involved here?

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