{"thread":{"id":"13500","subject":"[PATCH 0/4] freebsd portability fixes","startedAt":"2008-05-13T08:43:39Z","lastAt":"2008-05-14T09:33:50Z","messageCount":13,"participants":["Jeff King","Alex Riesen","Junio C Hamano","Paolo Bonzini"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"76843","messageId":"20080513084338.GA23729@sigill.intra.peff.net","threadId":"13500","inReplyTo":null,"subject":"[PATCH 0/4] freebsd portability fixes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-05-13T08:43:39Z","receivedAt":"2008-05-13T08:43:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"I've been setting up auto-builders for a few platforms to try to catch\nportability problems early. Unfortunately, it was made harder by the\nfact that the _current_ test scripts don't pass on all platforms.\n\nWith these patches, I can successfully run all test scripts from the\ncurrent 'master' on FreeBSD 6.1 (some of them probably affect OS X, too\n-- I recall somebody complaining about the '! foo | bar' construct\nrecently).\n\n-Peff\n"},{"id":"76844","messageId":"20080513084452.GA23799@sigill.intra.peff.net","threadId":"13500","inReplyTo":"20080513084338.GA23729@sigill.intra.peff.net","subject":"[PATCH 1/4] fix bsd shell negation","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-05-13T08:44:52Z","receivedAt":"2008-05-13T08:44:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On some shells (notably /bin/sh on FreeBSD 6.1), the\nconstruct\n\n  ! foo | bar\n\ndoes not negate the exit value of the pipeline, but rather\nof 'foo', producing the opposite of the expected value. We\ncan work around this by specifying it as\n\n  ! (foo | bar)\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n git-rebase.sh     |    2 +-\n t/t3400-rebase.sh |    4 ++--\n t/t3700-add.sh    |    6 +++---\n t/t7501-commit.sh |    2 +-\n 4 files changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex 9b13b83..fbb0f28 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -353,7 +353,7 @@ orig_head=$branch\n mb=$(git merge-base \"$onto\" \"$branch\")\n if test \"$upstream\" = \"$onto\" && test \"$mb\" = \"$onto\" &&\n \t# linear history?\n-\t! git rev-list --parents \"$onto\"..\"$branch\" | grep \" .* \" > /dev/null\n+\t! (git rev-list --parents \"$onto\"..\"$branch\" | grep \" .* \") > /dev/null\n then\n \t# Lazily switch to the target branch if needed...\n \ttest -z \"$switch_to\" || git checkout \"$switch_to\"\ndiff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\nindex 496f4ec..fdad7da 100755\n--- a/t/t3400-rebase.sh\n+++ b/t/t3400-rebase.sh\n@@ -44,13 +44,13 @@ test_expect_success 'rebase against master' '\n \n test_expect_success \\\n     'the rebase operation should not have destroyed author information' \\\n-    '! git log | grep \"Author:\" | grep \"<>\"'\n+    '! (git log | grep \"Author:\" | grep \"<>\")'\n \n test_expect_success 'rebase after merge master' '\n      git reset --hard topic &&\n      git merge master &&\n      git rebase master &&\n-     ! git show | grep \"^Merge:\"\n+     ! (git show | grep \"^Merge:\")\n '\n \n test_expect_success 'rebase of history with merges is linearized' '\ndiff --git a/t/t3700-add.sh b/t/t3700-add.sh\nindex 287e058..68c5dde 100755\n--- a/t/t3700-add.sh\n+++ b/t/t3700-add.sh\n@@ -81,17 +81,17 @@ test_expect_success '.gitignore test setup' '\n \n test_expect_success '.gitignore is honored' '\n \tgit add . &&\n-\t! git ls-files | grep \"\\\\.ig\"\n+\t! (git ls-files | grep \"\\\\.ig\")\n '\n \n test_expect_success 'error out when attempting to add ignored ones without -f' '\n \t! git add a.?? &&\n-\t! git ls-files | grep \"\\\\.ig\"\n+\t! (git ls-files | grep \"\\\\.ig\")\n '\n \n test_expect_success 'error out when attempting to add ignored ones without -f' '\n \t! git add d.?? &&\n-\t! git ls-files | grep \"\\\\.ig\"\n+\t! (git ls-files | grep \"\\\\.ig\")\n '\n \n test_expect_success 'add ignored ones with -f' '\ndiff --git a/t/t7501-commit.sh b/t/t7501-commit.sh\nindex c0288f3..89710af 100755\n--- a/t/t7501-commit.sh\n+++ b/t/t7501-commit.sh\n@@ -41,7 +41,7 @@ test_expect_success \\\n test_expect_success \\\n \t\"using paths with --interactive\" \\\n \t\"echo bong-o-bong >file &&\n-\t! echo 7 | git-commit -m foo --interactive file\"\n+\t! (echo 7 | git-commit -m foo --interactive file)\"\n \n test_expect_success \\\n \t\"using invalid commit with -C\" \\\n-- \n1.5.5.1.296.gf618c\n"},{"id":"76845","messageId":"20080513084532.GB23799@sigill.intra.peff.net","threadId":"13500","inReplyTo":"20080513084338.GA23729@sigill.intra.peff.net","subject":"[PATCH 2/4] t5000: tar portability fix","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-05-13T08:45:32Z","receivedAt":"2008-05-13T08:45:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The output of 'tar tv' varies from system to system. In\nparticular, the t5000 was expecting to parse the date from\nsomething like:\n\n  -rw-rw-r-- root/root         0 2008-05-13 04:27 file\n\nbut FreeBSD's tar produces this:\n\n  -rw-rw-r--  0 root   root        0 May 13 04:27 file\n\nInstead of relying on tar's output, let's just extract the\nfile using tar and stat the result using perl.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t5000-tar-tree.sh |    8 ++++----\n 1 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\nindex fa62b6a..9b0baac 100755\n--- a/t/t5000-tar-tree.sh\n+++ b/t/t5000-tar-tree.sh\n@@ -67,10 +67,10 @@ test_expect_success \\\n \n test_expect_success \\\n     'validate file modification time' \\\n-    'TZ=GMT $TAR tvf b.tar a/a |\n-     awk \\{print\\ \\$4,\\ \\(length\\(\\$5\\)\\<7\\)\\ ?\\ \\$5\\\":00\\\"\\ :\\ \\$5\\} \\\n-     >b.mtime &&\n-     echo \"2005-05-27 22:00:00\" >expected.mtime &&\n+    'mkdir extract &&\n+     $TAR xf b.tar -C extract a/a &&\n+     perl -e '\\''print((stat(\"extract/a/a\"))[9], \"\\n\")'\\'' >b.mtime &&\n+     echo \"1117231200\" >expected.mtime &&\n      diff expected.mtime b.mtime'\n \n test_expect_success \\\n-- \n1.5.5.1.296.gf618c\n"},{"id":"76846","messageId":"20080513084556.GC23799@sigill.intra.peff.net","threadId":"13500","inReplyTo":"20080513084338.GA23729@sigill.intra.peff.net","subject":"[PATCH 3/4] clone: bsd shell portability fix","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-05-13T08:45:56Z","receivedAt":"2008-05-13T08:45:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When using /bin/sh from FreeBSD 6.1, the value of $? is lost\nwhen calling a function inside the 'trap' action. This\nresulted in clone erroneously indicating success when it\nshould have reported failure.\n\nAs a workaround, we save the value of $? before calling any\nfunctions.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n git-clone.sh |    3 +--\n 1 files changed, 1 insertions(+), 2 deletions(-)\n\ndiff --git a/git-clone.sh b/git-clone.sh\nindex 9d88d1c..547228e 100755\n--- a/git-clone.sh\n+++ b/git-clone.sh\n@@ -240,7 +240,6 @@ die \"working tree '$GIT_WORK_TREE' already exists.\"\n D=\n W=\n cleanup() {\n-\terr=$?\n \ttest -z \"$D\" && rm -rf \"$dir\"\n \ttest -z \"$W\" && test -n \"$GIT_WORK_TREE\" && rm -rf \"$GIT_WORK_TREE\"\n \tcd ..\n@@ -248,7 +247,7 @@ cleanup() {\n \ttest -n \"$W\" && rm -rf \"$W\"\n \texit $err\n }\n-trap cleanup 0\n+trap 'err=$?; cleanup' 0\n mkdir -p \"$dir\" && D=$(cd \"$dir\" && pwd) || usage\n test -n \"$GIT_WORK_TREE\" && mkdir -p \"$GIT_WORK_TREE\" &&\n W=$(cd \"$GIT_WORK_TREE\" && pwd) && GIT_WORK_TREE=\"$W\" && export GIT_WORK_TREE\n-- \n1.5.5.1.296.gf618c\n"},{"id":"76847","messageId":"20080513084638.GD23799@sigill.intra.peff.net","threadId":"13500","inReplyTo":"20080513084338.GA23729@sigill.intra.peff.net","subject":"[PATCH 4/4] filter-branch: fix variable export logic","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-05-13T08:46:38Z","receivedAt":"2008-05-13T08:46:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"filter-branch tries to restore \"old\" copies of some\nenvironment variables by using the construct:\n\n  unset var\n  test -z \"$old_var\" || var=\"$old_var\" && export var\n\nHowever, by the short-circuit logic, we will always run\n'export var'. On bash and dash, exporting an unset variable\nhas no effect. However, on some shells (such as FreeBSD's\n/bin/sh), the shell exports the empty value.\n\nThis manifested itself in this case as git-filter-branch\nsetting GIT_INDEX_FILE to the empty string, which in turn\ncaused its call to git-read-tree to fail, leaving the\nworking tree pointing at the original HEAD instead of the\nrewritten one.\n\nTo fix this, we change the short-circuit logic to better\nmatch the intent:\n\n  test -z \"$old_var\" || { var=\"$old_var\" && export var; }\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n git-filter-branch.sh |   10 +++++-----\n 1 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 333f6a8..0304dc5 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -435,11 +435,11 @@ rm -rf \"$tempdir\"\n trap - 0\n \n unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE\n-test -z \"$ORIG_GIT_DIR\" || GIT_DIR=\"$ORIG_GIT_DIR\" && export GIT_DIR\n-test -z \"$ORIG_GIT_WORK_TREE\" || GIT_WORK_TREE=\"$ORIG_GIT_WORK_TREE\" &&\n-\texport GIT_WORK_TREE\n-test -z \"$ORIG_GIT_INDEX_FILE\" || GIT_INDEX_FILE=\"$ORIG_GIT_INDEX_FILE\" &&\n-\texport GIT_INDEX_FILE\n+test -z \"$ORIG_GIT_DIR\" || { GIT_DIR=\"$ORIG_GIT_DIR\" && export GIT_DIR; }\n+test -z \"$ORIG_GIT_WORK_TREE\" || { GIT_WORK_TREE=\"$ORIG_GIT_WORK_TREE\" &&\n+\texport GIT_WORK_TREE; }\n+test -z \"$ORIG_GIT_INDEX_FILE\" || { GIT_INDEX_FILE=\"$ORIG_GIT_INDEX_FILE\" &&\n+\texport GIT_INDEX_FILE; }\n git read-tree -u -m HEAD\n \n exit $ret\n-- \n1.5.5.1.296.gf618c\n"},{"id":"76848","messageId":"20080513090424.GA26181@sigill.intra.peff.net","threadId":"13500","inReplyTo":"20080513084338.GA23729@sigill.intra.peff.net","subject":"Re: [PATCH 0/4] freebsd portability fixes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-05-13T09:04:24Z","receivedAt":"2008-05-13T09:04:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 13, 2008 at 04:43:39AM -0400, Jeff King wrote:\n\n> With these patches, I can successfully run all test scripts from the\n> current 'master' on FreeBSD 6.1 (some of them probably affect OS X, too\n> -- I recall somebody complaining about the '! foo | bar' construct\n> recently).\n\nAh, nevermind about OS X. It was 97ad535b from Alex, and he specifically\nmentioned FreeBSD 4.\n\nAlex, you might want to try re-running the tests with these patches.\n\n-Peff\n"},{"id":"76883","messageId":"20080513203931.GA3981@steel.home","threadId":"13500","inReplyTo":"20080513090424.GA26181@sigill.intra.peff.net","subject":"Re: [PATCH 0/4] freebsd portability fixes","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2008-05-13T20:39:31Z","receivedAt":"2008-05-13T20:39:31Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Jeff King, Tue, May 13, 2008 11:04:24 +0200:\n> On Tue, May 13, 2008 at 04:43:39AM -0400, Jeff King wrote:\n> \n> > With these patches, I can successfully run all test scripts from the\n> > current 'master' on FreeBSD 6.1 (some of them probably affect OS X, too\n> > -- I recall somebody complaining about the '! foo | bar' construct\n> > recently).\n> \n> Ah, nevermind about OS X. It was 97ad535b from Alex, and he specifically\n> mentioned FreeBSD 4.\n> \n> Alex, you might want to try re-running the tests with these patches.\n> \n\nVery good! GnuPG is missing on the system, so t7004 was skipped,\nbut everything besides that ran fine. My config.mak:\n\nNO_STRTOUMAX=Yes\nNO_C99_FORMAT=Yes\nSHELL_PATH=/usr/local/bin/bash\n\nperl 5.8.6\n\nI also have the test-tr patches I sent some time ago in that tree.\n"},{"id":"76885","messageId":"20080513204442.GB3981@steel.home","threadId":"13500","inReplyTo":"20080513203931.GA3981@steel.home","subject":"Re: [PATCH 0/4] freebsd portability fixes","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2008-05-13T20:44:42Z","receivedAt":"2008-05-13T20:44:42Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Alex Riesen, Tue, May 13, 2008 22:39:31 +0200:\n> Jeff King, Tue, May 13, 2008 11:04:24 +0200:\n> > On Tue, May 13, 2008 at 04:43:39AM -0400, Jeff King wrote:\n> > \n> > > With these patches, I can successfully run all test scripts from the\n> > > current 'master' on FreeBSD 6.1 (some of them probably affect OS X, too\n> > > -- I recall somebody complaining about the '! foo | bar' construct\n> > > recently).\n> > \n> > Ah, nevermind about OS X. It was 97ad535b from Alex, and he specifically\n> > mentioned FreeBSD 4.\n> > \n> > Alex, you might want to try re-running the tests with these patches.\n> > \n> \n> Very good! GnuPG is missing on the system, so t7004 was skipped,\n> but everything besides that ran fine. My config.mak:\n> \n> NO_STRTOUMAX=Yes\n> NO_C99_FORMAT=Yes\n> SHELL_PATH=/usr/local/bin/bash\n> \n> perl 5.8.6\n> \n> I also have the test-tr patches I sent some time ago in that tree.\n> \n\nI also have the following in git-compat-util.h:\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 01c4045..161bd50 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -191,6 +191,7 @@ extern size_t gitstrlcpy(char *, const char *, size_t);\n \n #ifdef NO_STRTOUMAX\n #define strtoumax gitstrtoumax\n+typedef unsigned long long uintmax_t;\n extern uintmax_t gitstrtoumax(const char *, char **, int);\n #endif\n \n\nWhich is obviously a hack, but I considered too minor an issue. It\n(the missing uintmax_t) is probably something very specific to this\nparticular system. I am probably wrong about the issue being minor...\n"},{"id":"76908","messageId":"7viqxhr55h.fsf@gitster.siamese.dyndns.org","threadId":"13500","inReplyTo":"20080513084452.GA23799@sigill.intra.peff.net","subject":"Re: [PATCH 1/4] fix bsd shell negation","fromName":"Junio C Hamano","fromEmail":"junio@pobox.com","sentAt":"2008-05-14T02:27:54Z","receivedAt":"2008-05-14T02:27:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On some shells (notably /bin/sh on FreeBSD 6.1), the\n> construct\n>\n>   ! foo | bar\n>\n> does not negate the exit value of the pipeline, but rather\n> of 'foo', producing the opposite of the expected value. We\n> can work around this by specifying it as\n>\n>   ! (foo | bar)\n\nAfter applying this patch, we still seem to have fair number of hits:\n\n    $ git grep -n -e '![^(]* |' -- '*.sh'\n\nt/t5302-pack-index.sh:68:\t! echo \"$msg\" | grep \"pack too large .* off_t\"\nt/t7002-grep.sh:111:\t\t! git grep -c test $H | grep -q /dev/null\nt/t7600-merge.sh:378:\tif ! grep \"^ file |  *2 +-$\" diffstat.txt\nt/t7600-merge.sh:392:\tif ! grep \"^ file |  *2 +-$\" diffstat.txt\nt/t9400-git-cvsserver-server.sh:160:    ! cat request-anonymous |\nt/t9400-git-cvsserver-server.sh:169:    ! cat request-anonymous |\nt/t9400-git-cvsserver-server.sh:187:    ! cat request-anonymous |\n\nDidn't you have trouble with them, or the tests themselves are broken in\nsome other way that these did not triggger?\n\n\n\n    \n    \n"},{"id":"76909","messageId":"20080514040122.GA9985@sigill.intra.peff.net","threadId":"13500","inReplyTo":"7viqxhr55h.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/4] fix bsd shell negation","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-05-14T04:01:22Z","receivedAt":"2008-05-14T04:01:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 13, 2008 at 07:27:54PM -0700, Junio C Hamano wrote:\n\n> After applying this patch, we still seem to have fair number of hits:\n> \n>     $ git grep -n -e '![^(]* |' -- '*.sh'\n> \n> t/t5302-pack-index.sh:68:\t! echo \"$msg\" | grep \"pack too large .* off_t\"\n\nThis one didn't trigger for me because it is the second half of a\nshort-circuit conditional. But it should be fixed.\n\n> t/t7002-grep.sh:111:\t\t! git grep -c test $H | grep -q /dev/null\n\nHmm. I think my previous analysis is not quite right. The problem is not\nexactly \"! foo | bar\". It must happen as part of a logic chain.\n\nSo:\n\n  # all sane\n  $ !  true |  true; echo $?; \\\n    !  true | false; echo $?; \\\n    ! false |  true; echo $?; \\\n    ! false | false; echo $?\n  1\n  0\n  1\n  0\n\n  # but not this\n  $ true && !  true |  true; echo $?; \\\n    true && !  true | false; echo $?; \\\n    true && ! false |  true; echo $?; \\\n    true && ! false | false; echo $?\n  0\n  1\n  0\n  1\n\n(whereas with bash, one gets the same \"1010\" response for the second\nset).\n\nSo it seems like it is parsing as:\n\n  (true && ! true) | true\n\ninstead of\n\n  true && ! (true | true)\n\nBut that is not quite right, either, since the output of the first\ncommand doesn't go to the third. E.g.,:\n\n  $ echo foo && ! echo bar | nl\n  foo\n      1 bar\n\nSo just the exit code gets munged.\n\nHowever, explicitly grouping with a subshell does seem to fix it. In the\nt7002-grep.sh case, though, there are no other logic operators, so it\nworks either way.\n\n> t/t7600-merge.sh:378:\tif ! grep \"^ file |  *2 +-$\" diffstat.txt\n> t/t7600-merge.sh:392:\tif ! grep \"^ file |  *2 +-$\" diffstat.txt\n\nThese are false positives to your grep; the | is part of a string. ;)\n\n> t/t9400-git-cvsserver-server.sh:160:    ! cat request-anonymous |\n> t/t9400-git-cvsserver-server.sh:169:    ! cat request-anonymous |\n> t/t9400-git-cvsserver-server.sh:187:    ! cat request-anonymous |\n\nSame precedence issue as above.\n\nBelow is an updated patch 1/4; it includes the one extra fix mentioned\nabove and fixes the inaccuracies in the commit message.\n\n-- >8 --\nfix bsd shell negation\n\nOn some shells (notably /bin/sh on FreeBSD 6.1), the\nconstruct\n\n  foo && ! bar | baz\n\nis true if\n\n  foo && baz\n\nwhereas for most other shells (such as bash) is true if\n\n  foo && ! baz\n\nWe can work around this by specifying\n\n  foo && ! (bar | baz)\n\nwhich works everywhere.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n git-rebase.sh         |    2 +-\n t/t3400-rebase.sh     |    4 ++--\n t/t3700-add.sh        |    6 +++---\n t/t5302-pack-index.sh |    2 +-\n t/t7501-commit.sh     |    2 +-\n 5 files changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex 9b13b83..fbb0f28 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -353,7 +353,7 @@ orig_head=$branch\n mb=$(git merge-base \"$onto\" \"$branch\")\n if test \"$upstream\" = \"$onto\" && test \"$mb\" = \"$onto\" &&\n \t# linear history?\n-\t! git rev-list --parents \"$onto\"..\"$branch\" | grep \" .* \" > /dev/null\n+\t! (git rev-list --parents \"$onto\"..\"$branch\" | grep \" .* \") > /dev/null\n then\n \t# Lazily switch to the target branch if needed...\n \ttest -z \"$switch_to\" || git checkout \"$switch_to\"\ndiff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\nindex 496f4ec..fdad7da 100755\n--- a/t/t3400-rebase.sh\n+++ b/t/t3400-rebase.sh\n@@ -44,13 +44,13 @@ test_expect_success 'rebase against master' '\n \n test_expect_success \\\n     'the rebase operation should not have destroyed author information' \\\n-    '! git log | grep \"Author:\" | grep \"<>\"'\n+    '! (git log | grep \"Author:\" | grep \"<>\")'\n \n test_expect_success 'rebase after merge master' '\n      git reset --hard topic &&\n      git merge master &&\n      git rebase master &&\n-     ! git show | grep \"^Merge:\"\n+     ! (git show | grep \"^Merge:\")\n '\n \n test_expect_success 'rebase of history with merges is linearized' '\ndiff --git a/t/t3700-add.sh b/t/t3700-add.sh\nindex 287e058..68c5dde 100755\n--- a/t/t3700-add.sh\n+++ b/t/t3700-add.sh\n@@ -81,17 +81,17 @@ test_expect_success '.gitignore test setup' '\n \n test_expect_success '.gitignore is honored' '\n \tgit add . &&\n-\t! git ls-files | grep \"\\\\.ig\"\n+\t! (git ls-files | grep \"\\\\.ig\")\n '\n \n test_expect_success 'error out when attempting to add ignored ones without -f' '\n \t! git add a.?? &&\n-\t! git ls-files | grep \"\\\\.ig\"\n+\t! (git ls-files | grep \"\\\\.ig\")\n '\n \n test_expect_success 'error out when attempting to add ignored ones without -f' '\n \t! git add d.?? &&\n-\t! git ls-files | grep \"\\\\.ig\"\n+\t! (git ls-files | grep \"\\\\.ig\")\n '\n \n test_expect_success 'add ignored ones with -f' '\ndiff --git a/t/t5302-pack-index.sh b/t/t5302-pack-index.sh\nindex b88b5bb..09fd917 100755\n--- a/t/t5302-pack-index.sh\n+++ b/t/t5302-pack-index.sh\n@@ -65,7 +65,7 @@ test_expect_success \\\n \n have_64bits=\n if msg=$(git verify-pack -v \"test-3-${pack3}.pack\" 2>&1) ||\n-\t! echo \"$msg\" | grep \"pack too large .* off_t\"\n+\t! (echo \"$msg\" | grep \"pack too large .* off_t\")\n then\n \thave_64bits=t\n else\ndiff --git a/t/t7501-commit.sh b/t/t7501-commit.sh\nindex c0288f3..89710af 100755\n--- a/t/t7501-commit.sh\n+++ b/t/t7501-commit.sh\n@@ -41,7 +41,7 @@ test_expect_success \\\n test_expect_success \\\n \t\"using paths with --interactive\" \\\n \t\"echo bong-o-bong >file &&\n-\t! echo 7 | git-commit -m foo --interactive file\"\n+\t! (echo 7 | git-commit -m foo --interactive file)\"\n \n test_expect_success \\\n \t\"using invalid commit with -C\" \\\n-- \n1.5.5.1.219.g82ba.dirty\n"},{"id":"76910","messageId":"7v7idxr00i.fsf@gitster.siamese.dyndns.org","threadId":"13500","inReplyTo":"20080513084638.GD23799@sigill.intra.peff.net","subject":"Re: [PATCH 4/4] filter-branch: fix variable export logic","fromName":"Junio C Hamano","fromEmail":"junio@pobox.com","sentAt":"2008-05-14T04:18:53Z","receivedAt":"2008-05-14T04:18:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> filter-branch tries to restore \"old\" copies of some\n> environment variables by using the construct:\n>\n>   unset var\n>   test -z \"$old_var\" || var=\"$old_var\" && export var\n>\n> However, by the short-circuit logic, we will always run\n> 'export var'.\n\nThanks.\n\nI was confused by this description (\"short-circuit logic\"), but I do not\nthink there is no short-circuit going on.  This is a simple ignorance of\nshell syntax.\n\nIn a shell scriptlet:\n\n\ta || b && c\n\nAND list operator (&&) and OR list operator (||) have the same precedence\nand bind to the left.  Because the second part of OR list is always true,\nwe always export.\n\nI have a mild suspecion that this was simply an artifcat of a careless\nconversion from export var=\"$val\" form we did in the past.  I should have\ncaught them back then.\n\nThe patch is fine, but I find this easier to read:\n\n+test -z \"$ORIG_GIT_DIR\" || {\n+\tGIT_DIR=\"$ORIG_GIT_DIR\" && export GIT_DIR\n+}\n+test -z \"$ORIG_GIT_WORK_TREE\" || {\n+\tGIT_WORK_TREE=\"$ORIG_GIT_WORK_TREE\" &&\n+\texport GIT_WORK_TREE\n+}\n+test -z \"$ORIG_GIT_INDEX_FILE\" || {\n+\tGIT_INDEX_FILE=\"$ORIG_GIT_INDEX_FILE\" &&\n+\texport GIT_INDEX_FILE\n+}\n"},{"id":"76911","messageId":"20080514045717.GA16592@sigill.intra.peff.net","threadId":"13500","inReplyTo":"7v7idxr00i.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 4/4] filter-branch: fix variable export logic","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-05-14T04:57:17Z","receivedAt":"2008-05-14T04:57:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 13, 2008 at 09:18:53PM -0700, Junio C Hamano wrote:\n\n> I was confused by this description (\"short-circuit logic\"), but I do not\n> think there is no short-circuit going on.  This is a simple ignorance of\n> shell syntax.\n\nSorry, I meant \"you might think we are not going to run this because of\nshort circuit, but we always will.\" So I think the original author was\nconfused.\n\nBut please feel free to reword in a way that makes more sense.\n\n> I have a mild suspecion that this was simply an artifcat of a careless\n> conversion from export var=\"$val\" form we did in the past.  I should have\n> caught them back then.\n\nNope, it was originally that way (46eb449). Thank goodness for\ngit-blame! :)\n\n> The patch is fine, but I find this easier to read:\n> \n> +test -z \"$ORIG_GIT_DIR\" || {\n> +\tGIT_DIR=\"$ORIG_GIT_DIR\" && export GIT_DIR\n> +}\n> +test -z \"$ORIG_GIT_WORK_TREE\" || {\n> +\tGIT_WORK_TREE=\"$ORIG_GIT_WORK_TREE\" &&\n> +\texport GIT_WORK_TREE\n> +}\n> +test -z \"$ORIG_GIT_INDEX_FILE\" || {\n> +\tGIT_INDEX_FILE=\"$ORIG_GIT_INDEX_FILE\" &&\n> +\texport GIT_INDEX_FILE\n> +}\n\nYes, that is easier to read. Although what also confused me at first was\nthe double negation. It is trying to say \"if the original existed,\nrestore it\", but it is written as \"the original has no content, OR\nrestore it\". So\n\n  if test -n \"$ORIG_GIT_DIR\"\n  then\n    ...\n  fi\n\nwould be even clearer, though I'm not sure if \"-n\" has any portability\nconcerns.\n\n-Peff\n"},{"id":"76932","messageId":"482AB1FE.6030009@gnu.org","threadId":"13500","inReplyTo":"20080514045717.GA16592@sigill.intra.peff.net","subject":"Re: [PATCH 4/4] filter-branch: fix variable export logic","fromName":"Paolo Bonzini","fromEmail":"bonzini@gnu.org","sentAt":"2008-05-14T09:33:50Z","receivedAt":"2008-05-14T09:33:50Z","isPatch":true,"sender":{"key":"bonzini@gnu.org","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"\n>   if test -n \"$ORIG_GIT_DIR\"\n>   then\n>     ...\n>   fi\n> \n> would be even clearer, though I'm not sure if \"-n\" has any portability\n> concerns.\n\nIt has not.  Some autoconf scripts do\n\n   if test \"x$ORIG_GIT_DIR\" != x; then\n     ...\n   fi\n\n(and similarly for -z) but that's being phased out.\n\nPaolo\n"}]}