Re: [PATCH 1/4] fix bsd shell negation

From: Jeff King <peff@peff.net>
Date: May 14, 2008, 04:01 UTC
Message-ID: <20080514040122.GA9985@sigill.intra.peff.net>
In-Reply-To: <7viqxhr55h.fsf@gitster.siamese.dyndns.org>

On Tue, May 13, 2008 at 07:27:54PM -0700, Junio C Hamano wrote:

Show 5 quoted lines

> After applying this patch, we still seem to have fair number of hits:
> 
>     $ git grep -n -e '![^(]* |' -- '*.sh'
> 
> t/t5302-pack-index.sh:68:	! echo "$msg" | grep "pack too large .* off_t"

This one didn't trigger for me because it is the second half of a short-circuit conditional. But it should be fixed.

> t/t7002-grep.sh:111:		! git grep -c test $H | grep -q /dev/null

Hmm. I think my previous analysis is not quite right. The problem is not exactly "! foo | bar". It must happen as part of a logic chain.

So:

  # all sane
  $ !  true |  true; echo $?; \
    !  true | false; echo $?; \
    ! false |  true; echo $?; \
    ! false | false; echo $?
  1
  0
  1
  0

  # but not this
  $ true && !  true |  true; echo $?; \
    true && !  true | false; echo $?; \
    true && ! false |  true; echo $?; \
    true && ! false | false; echo $?
  0
  1
  0
  1

(whereas with bash, one gets the same "1010" response for the second set).

So it seems like it is parsing as:

  (true && ! true) | true

instead of

  true && ! (true | true)

But that is not quite right, either, since the output of the first command doesn't go to the third. E.g.,:

  $ echo foo && ! echo bar | nl
  foo
      1 bar

So just the exit code gets munged.

However, explicitly grouping with a subshell does seem to fix it. In the t7002-grep.sh case, though, there are no other logic operators, so it works either way.

> t/t7600-merge.sh:378:	if ! grep "^ file |  *2 +-$" diffstat.txt
> t/t7600-merge.sh:392:	if ! grep "^ file |  *2 +-$" diffstat.txt

These are false positives to your grep; the | is part of a string. ;)

> t/t9400-git-cvsserver-server.sh:160:    ! cat request-anonymous |
> t/t9400-git-cvsserver-server.sh:169:    ! cat request-anonymous |
> t/t9400-git-cvsserver-server.sh:187:    ! cat request-anonymous |

Same precedence issue as above.

Below is an updated patch 1/4; it includes the one extra fix mentioned above and fixes the inaccuracies in the commit message.

-- >8 -- fix bsd shell negation

On some shells (notably /bin/sh on FreeBSD 6.1), the construct

  foo && ! bar | baz

is true if

  foo && baz

whereas for most other shells (such as bash) is true if

  foo && ! baz

We can work around this by specifying

  foo && ! (bar | baz)

which works everywhere.

Signed-off-by: Jeff King <peff@peff.net>
---
 git-rebase.sh         |    2 +-
 t/t3400-rebase.sh     |    4 ++--
 t/t3700-add.sh        |    6 +++---
 t/t5302-pack-index.sh |    2 +-
 t/t7501-commit.sh     |    2 +-
 5 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 9b13b83..fbb0f28 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -353,7 +353,7 @@ orig_head=$branch
 mb=$(git merge-base "$onto" "$branch")
 if test "$upstream" = "$onto" && test "$mb" = "$onto" &&
 	# linear history?
-	! git rev-list --parents "$onto".."$branch" | grep " .* " > /dev/null
+	! (git rev-list --parents "$onto".."$branch" | grep " .* ") > /dev/null
 then
 	# Lazily switch to the target branch if needed...
 	test -z "$switch_to" || git checkout "$switch_to"
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 496f4ec..fdad7da 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -44,13 +44,13 @@ test_expect_success 'rebase against master' '
 
 test_expect_success \
     'the rebase operation should not have destroyed author information' \
-    '! git log | grep "Author:" | grep "<>"'
+    '! (git log | grep "Author:" | grep "<>")'
 
 test_expect_success 'rebase after merge master' '
      git reset --hard topic &&
      git merge master &&
      git rebase master &&
-     ! git show | grep "^Merge:"
+     ! (git show | grep "^Merge:")
 '
 
 test_expect_success 'rebase of history with merges is linearized' '
diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index 287e058..68c5dde 100755
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh
@@ -81,17 +81,17 @@ test_expect_success '.gitignore test setup' '
 
 test_expect_success '.gitignore is honored' '
 	git add . &&
-	! git ls-files | grep "\\.ig"
+	! (git ls-files | grep "\\.ig")
 '
 
 test_expect_success 'error out when attempting to add ignored ones without -f' '
 	! git add a.?? &&
-	! git ls-files | grep "\\.ig"
+	! (git ls-files | grep "\\.ig")
 '
 
 test_expect_success 'error out when attempting to add ignored ones without -f' '
 	! git add d.?? &&
-	! git ls-files | grep "\\.ig"
+	! (git ls-files | grep "\\.ig")
 '
 
 test_expect_success 'add ignored ones with -f' '
diff --git a/t/t5302-pack-index.sh b/t/t5302-pack-index.sh
index b88b5bb..09fd917 100755
--- a/t/t5302-pack-index.sh
+++ b/t/t5302-pack-index.sh
@@ -65,7 +65,7 @@ test_expect_success \
 
 have_64bits=
 if msg=$(git verify-pack -v "test-3-${pack3}.pack" 2>&1) ||
-	! echo "$msg" | grep "pack too large .* off_t"
+	! (echo "$msg" | grep "pack too large .* off_t")
 then
 	have_64bits=t
 else
diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index c0288f3..89710af 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -41,7 +41,7 @@ test_expect_success \
 test_expect_success \
 	"using paths with --interactive" \
 	"echo bong-o-bong >file &&
-	! echo 7 | git-commit -m foo --interactive file"
+	! (echo 7 | git-commit -m foo --interactive file)"
 
 test_expect_success \
 	"using invalid commit with -C" \

-- 
1.5.5.1.219.g82ba.dirty