{"thread":{"id":"21663","subject":"[PATCH] unset GREP_OPTIONS in test-lib.sh","startedAt":"2009-11-18T16:15:19Z","lastAt":"2010-03-07T10:30:58Z","messageCount":12,"participants":["Bert Wesarg","Junio C Hamano","René Scharfe","Carlo Marcelo Arenas Belon","David Aguilar"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"127862","messageId":"1258560919-28054-1-git-send-email-bert.wesarg@googlemail.com","threadId":"21663","inReplyTo":null,"subject":"[PATCH] unset GREP_OPTIONS in test-lib.sh","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2009-11-18T16:15:19Z","receivedAt":"2009-11-18T16:15:19Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"I used to set GREP_OPTIONS to exclude *.orig and *.rej files. But with this\nthe test t4252-am-options.sh fails because it calls grep with a .rej file:\n\n grep \"@@ -1,3 +1,3 @@\" file-2.rej\n\nSigned-off-by: Bert Wesarg \n\n---\n t/test-lib.sh | 2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex f2ca536..6ac8dc6 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -65,6 +65,8 @@ GIT_TEST_CMP=${GIT_TEST_CMP:-diff -u}\n # CDPATH into the environment\n unset CDPATH\n \n+unset GREP_OPTIONS\n+\n case $(echo $GIT_TRACE |tr \"[A-Z]\" \"[a-z]\") in\n \t1|2|true)\n \t\techo \"* warning: Some tests will not work if GIT_TRACE\" \\\n-- \ntg: (785c58e..) bw/unset-GREP_OPTIONS (depends on: master)\n"},{"id":"127875","messageId":"7v1vjvebem.fsf@alter.siamese.dyndns.org","threadId":"21663","inReplyTo":"1258560919-28054-1-git-send-email-bert.wesarg@googlemail.com","subject":"Re: [PATCH] unset GREP_OPTIONS in test-lib.sh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-18T22:05:53Z","receivedAt":"2009-11-18T22:05:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Bert Wesarg writes:\n\n> I used to set GREP_OPTIONS to exclude *.orig and *.rej files. But with this\n> the test t4252-am-options.sh fails because it calls grep with a .rej file:\n\nYuck. Will apply.\n\nThat actually makes me worried about a different issue.\n\nDo we kill that environment variable when we call out to external grep in\ngrep.c? If not, we should. An alternative is to teach our internal one\nto also honor it, but I personally do not find it too attractive to mimic\nthe design mistake of GREP_OPTIONS myself.\n"},{"id":"128105","messageId":"4B095F91.8030305@lsrfire.ath.cx","threadId":"21663","inReplyTo":"7v1vjvebem.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] unset GREP_OPTIONS in test-lib.sh","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2009-11-22T15:58:09Z","receivedAt":"2009-11-22T15:58:09Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Junio C Hamano schrieb:\n> Do we kill that environment variable when we call out to external grep in\n> grep.c? If not, we should. An alternative is to teach our internal one\n> to also honor it, but I personally do not find it too attractive to mimic\n> the design mistake of GREP_OPTIONS myself.\n\nWe don't. Here's a patch with a simple test case that makes git grep\nunset GREP_OPTIONS before it calls the external grep.\n\nWhile we're at it, also unset GREP_COLOR and GREP_COLORS in case\ncolouring is not enabled, to be on the safe side. The presence of\nthese variables alone is not sufficient to trigger coloured output with\nGNU grep, but other implementations may behave differently.\n\nSigned-off-by: Rene Scharfe \n---\n builtin-grep.c | 4 ++++\n t/t7002-grep.sh | 5 +++++\n 2 files changed, 9 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-grep.c b/builtin-grep.c\nindex 01be9bf..9a9e3fc 100644\n--- a/builtin-grep.c\n+++ b/builtin-grep.c\n@@ -433,7 +433,11 @@ static int external_grep(struct grep_opt *opt, const char **paths, int cached)\n \n \t\tif (opt->color_external && strlen(opt->color_external) > 0)\n \t\t\tpush_arg(opt->color_external);\n+\t} else {\n+\t\tunsetenv(\"GREP_COLOR\");\n+\t\tunsetenv(\"GREP_COLORS\");\n \t}\n+\tunsetenv(\"GREP_OPTIONS\");\n \n \thit = 0;\n \targc = nr;\ndiff --git a/t/t7002-grep.sh b/t/t7002-grep.sh\nindex ae5290a..dd0da6c 100755\n--- a/t/t7002-grep.sh\n+++ b/t/t7002-grep.sh\n@@ -213,6 +213,11 @@ test_expect_success 'grep -e A --and --not -e B' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'grep should ignore GREP_OPTIONS' '\n+\tGREP_OPTIONS=-v git grep \" mmap bar\\$\" >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'grep -f, non-existent file' '\n \ttest_must_fail git grep -f patterns\n '\n-- \n1.6.5.3\n"},{"id":"128167","messageId":"20091123112221.GA7175@sajinet.com.pe","threadId":"21663","inReplyTo":"4B095F91.8030305@lsrfire.ath.cx","subject":"Re: [PATCH] unset GREP_OPTIONS in test-lib.sh","fromName":"Carlo Marcelo Arenas Belon","fromEmail":"carenas@sajinet.com.pe","sentAt":"2009-11-23T11:22:21Z","receivedAt":"2009-11-23T11:22:21Z","isPatch":true,"sender":{"key":"carenas@sajinet.com.pe","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Sun, Nov 22, 2009 at 04:58:09PM +0100, Ren? Scharfe wrote:\n> Junio C Hamano schrieb:\n> > Do we kill that environment variable when we call out to external grep in\n> > grep.c? If not, we should. An alternative is to teach our internal one\n> > to also honor it, but I personally do not find it too attractive to mimic\n> > the design mistake of GREP_OPTIONS myself.\n> \n> We don't. Here's a patch with a simple test case that makes git grep\n> unset GREP_OPTIONS before it calls the external grep.\n> \n> While we're at it, also unset GREP_COLOR and GREP_COLORS in case\n> colouring is not enabled, to be on the safe side. The presence of\n> these variables alone is not sufficient to trigger coloured output with\n> GNU grep, but other implementations may behave differently.\n\nwhy not better to apply the proposed patch from Junio in :\n\n http://article.gmane.org/gmane.comp.version-control.git/127980/\n\nit would IMHO correct all reported issues and serve as well as a catch\nall from other tools that could be introduced in the future and that\nwill be similarly affected by this misfeature.\n\nCarlo\n"},{"id":"128189","messageId":"7vtywlyu43.fsf@alter.siamese.dyndns.org","threadId":"21663","inReplyTo":"20091123112221.GA7175@sajinet.com.pe","subject":"Re: [PATCH] unset GREP_OPTIONS in test-lib.sh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-23T18:27:24Z","receivedAt":"2009-11-23T18:27:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlo Marcelo Arenas Belon writes:\n\n> why not better to apply the proposed patch from Junio in :\n>\n> http://article.gmane.org/gmane.comp.version-control.git/127980/\n>\n> it would IMHO correct all reported issues and serve as well as a catch\n> all from other tools that could be introduced in the future and that\n> will be similarly affected by this misfeature.\n\nI think René's patch is more sensible than $gmane/127980 because we have\nno business mucking with these environment variables when we are running\nthings other than external grep. You could be using system's \"grep\" in\nyour pre-commit hook to find some stuff, and your hook either may rely\non your having a particular set of GREP_OPTIONS in your environment, or\nmay be designed to work well with GREP_OPTIONS.\n"},{"id":"128210","messageId":"4B0B185B.4090305@lsrfire.ath.cx","threadId":"21663","inReplyTo":"7vtywlyu43.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] unset GREP_OPTIONS in test-lib.sh","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2009-11-23T23:18:51Z","receivedAt":"2009-11-23T23:18:51Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Junio C Hamano schrieb:\n> Carlo Marcelo Arenas Belon writes:\n> \n>> why not better to apply the proposed patch from Junio in :\n>>\n>> http://article.gmane.org/gmane.comp.version-control.git/127980/\n>>\n>> it would IMHO correct all reported issues and serve as well as a catch\n>> all from other tools that could be introduced in the future and that\n>> will be similarly affected by this misfeature.\n> \n> I think René's patch is more sensible than $gmane/127980 because we have\n> no business mucking with these environment variables when we are running\n> things other than external grep. You could be using system's \"grep\" in\n> your pre-commit hook to find some stuff, and your hook either may rely\n> on your having a particular set of GREP_OPTIONS in your environment, or\n> may be designed to work well with GREP_OPTIONS.\n\nYes, but what about git commands that are implemented as shell scripts\nand use grep? Something like the following patch?\n\nWe'd need to run this from time to time to make sure no new grep calls\ncreep in:\n\n git grep -L \"unset GREP_OPTIONS\" -- $(git grep -l \"grep\" git-*.sh)\n\n-- 8< --\nUnset GREP_OPTIONS at the top of git commands that are implemented as\nshell scripts and call grep, in order to avoid side effects caused by\nunexpected default options of users.\n\nSigned-off-by: Rene Scharfe \n---\n git-am.sh | 3 +++\n git-bisect.sh | 3 +++\n git-filter-branch.sh | 3 +++\n git-instaweb.sh | 3 +++\n git-notes.sh | 3 +++\n git-rebase--interactive.sh | 3 +++\n git-rebase.sh | 3 +++\n git-submodule.sh | 3 +++\n 8 files changed, 24 insertions(+), 0 deletions(-)\n\ndiff --git a/git-am.sh b/git-am.sh\nindex 151512a..1390eec 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -38,6 +38,9 @@ set_reflog_action am\n require_work_tree\n cd_to_toplevel\n \n+# Make sure we're in full control when calling grep in this script.\n+unset GREP_OPTIONS\n+\n git var GIT_COMMITTER_IDENT >/dev/null ||\n \tdie \"You need to set your committer info first\"\n \ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex a5ea843..fcf500f 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -30,6 +30,9 @@ OPTIONS_SPEC=\n . git-sh-setup\n require_work_tree\n \n+# Make sure we're in full control when calling grep in this script.\n+unset GREP_OPTIONS\n+\n _x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'\n _x40=\"$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40\"\n \ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 6b8b6a4..d3a8b3e 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -107,6 +107,9 @@ USAGE=\"[--env-filter ] [--tree-filter ]\n OPTIONS_SPEC=\n . git-sh-setup\n \n+# Make sure we're in full control when calling grep in this script.\n+unset GREP_OPTIONS\n+\n if [ \"$(is_bare_repository)\" = false ]; then\n \tgit diff-files --ignore-submodules --quiet &&\n \tgit diff-index --cached --quiet HEAD -- ||\ndiff --git a/git-instaweb.sh b/git-instaweb.sh\nindex 622a5f0..86916e1 100755\n--- a/git-instaweb.sh\n+++ b/git-instaweb.sh\n@@ -21,6 +21,9 @@ restart restart the web server\n \n . git-sh-setup\n \n+# Make sure we're in full control when calling grep in this script.\n+unset GREP_OPTIONS\n+\n fqgitdir=\"$GIT_DIR\"\n local=\"$(git config --bool --get instaweb.local)\"\n httpd=\"$(git config --get instaweb.httpd)\"\ndiff --git a/git-notes.sh b/git-notes.sh\nindex e642e47..e5f0edf 100755\n--- a/git-notes.sh\n+++ b/git-notes.sh\n@@ -3,6 +3,9 @@\n USAGE=\"(edit [-F | -m ] | show) [commit]\"\n . git-sh-setup\n \n+# Make sure we're in full control when calling grep in this script.\n+unset GREP_OPTIONS\n+\n test -z \"$1\" && usage\n ACTION=\"$1\"; shift\n \ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 27daaa9..d0bb8a3 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -33,6 +33,9 @@ root rebase all reachable commmits up to the root(s)\n . git-sh-setup\n require_work_tree\n \n+# Make sure we're in full control when calling grep in this script.\n+unset GREP_OPTIONS\n+\n DOTEST=\"$GIT_DIR/rebase-merge\"\n TODO=\"$DOTEST\"/git-rebase-todo\n DONE=\"$DOTEST\"/done\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex 6830e16..18c680b 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -34,6 +34,9 @@ set_reflog_action rebase\n require_work_tree\n cd_to_toplevel\n \n+# Make sure we're in full control when calling grep in this script.\n+unset GREP_OPTIONS\n+\n OK_TO_SKIP_PRE_REBASE=\n RESOLVEMSG=\"\n When you have resolved this problem run \\\"git rebase --continue\\\".\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 850d423..e557aca 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -17,6 +17,9 @@ OPTIONS_SPEC=\n . git-parse-remote\n require_work_tree\n \n+# Make sure we're in full control when calling grep in this script.\n+unset GREP_OPTIONS\n+\n command=\n branch=\n reference=\n-- \n1.6.5\n"},{"id":"128211","messageId":"4B0B1ACD.6000008@lsrfire.ath.cx","threadId":"21663","inReplyTo":"4B0B185B.4090305@lsrfire.ath.cx","subject":"[PATCH] mergetool--lib: simplify guess_merge_tool()","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2009-11-23T23:29:17Z","receivedAt":"2009-11-23T23:29:17Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Use a case statement instead of calling grep to find out if the editor's\nname contains the string \"vim\". Remove the check for emacs, as this\nbranch did the same as the default one anyway.\n\nSigned-off-by: Rene Scharfe \n---\nThis removes all grep calls from this script.\n\n git-mergetool--lib.sh | 13 ++++++-------\n 1 files changed, 6 insertions(+), 7 deletions(-)\n\ndiff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\nindex f7c571e..5b62785 100644\n--- a/git-mergetool--lib.sh\n+++ b/git-mergetool--lib.sh\n@@ -338,15 +338,14 @@ guess_merge_tool () {\n \t\tfi\n \t\ttools=\"$tools gvimdiff diffuse ecmerge p4merge araxis\"\n \tfi\n-\tif echo \"${VISUAL:-$EDITOR}\" | grep emacs > /dev/null 2>&1; then\n-\t\t# $EDITOR is emacs so add emerge as a candidate\n-\t\ttools=\"$tools emerge vimdiff\"\n-\telif echo \"${VISUAL:-$EDITOR}\" | grep vim > /dev/null 2>&1; then\n-\t\t# $EDITOR is vim so add vimdiff as a candidate\n+\tcase \"${VISUAL:-$EDITOR}\" in\n+\t*vim*)\n \t\ttools=\"$tools vimdiff emerge\"\n-\telse\n+\t\t;;\n+\t*)\n \t\ttools=\"$tools emerge vimdiff\"\n-\tfi\n+\t\t;;\n+\tesac\n \techo >&2 \"merge tool candidates: $tools\"\n \n \t# Loop over each candidate and stop when a valid merge tool is found.\n-- \n1.6.5\n"},{"id":"128213","messageId":"7v4ooku7cv.fsf@alter.siamese.dyndns.org","threadId":"21663","inReplyTo":"4B0B185B.4090305@lsrfire.ath.cx","subject":"Re: [PATCH] unset GREP_OPTIONS in test-lib.sh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-23T23:52:32Z","receivedAt":"2009-11-23T23:52:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n> Yes, but what about git commands that are implemented as shell scripts\n> and use grep? Something like the following patch?\n>\n> We'd need to run this from time to time to make sure no new grep calls\n> creep in:\n>\n> git grep -L \"unset GREP_OPTIONS\" -- $(git grep -l \"grep\" git-*.sh)\n\nHmm, but \"bisect run\" runs user's script and it may want to see\nGREP_OPTIONS from the environment, no? Same for any of the hooks that am\nand rebase might want to run.\n\n\n\n git-sh-setup.sh | 14 ++++++++++++++\n git-am.sh | 4 ++--\n git-bisect.sh | 4 ++--\n git-filter-branch.sh | 2 +-\n git-instaweb.sh | 8 ++++----\n git-rebase--interactive.sh | 10 +++++-----\n git-rebase.sh | 2 +-\n git-submodule.sh | 6 +++---\n 8 files changed, 32 insertions(+), 18 deletions(-)\n\ndiff --git a/git-sh-setup.sh b/git-sh-setup.sh\nindex c41c2f7..2b2afa6 100755\n--- a/git-sh-setup.sh\n+++ b/git-sh-setup.sh\n@@ -114,6 +114,20 @@ git_editor() {\n \teval \"${GIT_EDITOR:=vi}\" '\"$@\"'\n }\n \n+sane_grep () {\n+\tGREP_OPTIONS= \\\n+\tGREP_COLOR= \\\n+\tGREP_COLORS= \\\n+\tLC_ALL=C grep \"$@\"\n+}\n+\n+sane_egrep () {\n+\tGREP_OPTIONS= \\\n+\tGREP_COLOR= \\\n+\tGREP_COLORS= \\\n+\tLC_ALL=C egrep \"$@\"\n+}\n+\n is_bare_repository () {\n \tgit rev-parse --is-bare-repository\n }\ndiff --git a/git-am.sh b/git-am.sh\nindex c132f50..b49f26a 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -205,7 +205,7 @@ check_patch_format () {\n \t\t\t# and see if it looks like that they all begin with the\n \t\t\t# header field names...\n \t\t\tsed -n -e '/^$/q' -e '/^[ \t]/d' -e p \"$1\" |\n-\t\t\tLC_ALL=C egrep -v '^[!-9;-~]+:' >/dev/null ||\n+\t\t\tsane_egrep -v '^[!-9;-~]+:' >/dev/null ||\n \t\t\tpatch_format=mbox\n \t\tfi\n \t} < \"$1\" || clean_abort\n@@ -554,7 +554,7 @@ do\n \t\t\tstop_here $this\n \n \t\t# skip pine's internal folder data\n-\t\tgrep '^Author: Mail System Internal Data$' \\\n+\t\tsane_grep '^Author: Mail System Internal Data$' \\\n \t\t\t<\"$dotest\"/info >/dev/null &&\n \t\t\tgo_next && continue\n \ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex 6f6f039..0c422d5 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -393,7 +393,7 @@ bisect_run () {\n \n cat \"$GIT_DIR/BISECT_RUN\"\n \n- if grep \"first bad commit could be any of\" \"$GIT_DIR/BISECT_RUN\" \\\n+ if sane_grep \"first bad commit could be any of\" \"$GIT_DIR/BISECT_RUN\" \\\n \t\t> /dev/null; then\n \t echo >&2 \"bisect run cannot continue any more\"\n \t exit $res\n@@ -405,7 +405,7 @@ bisect_run () {\n \t exit $res\n fi\n \n- if grep \"is the first bad commit\" \"$GIT_DIR/BISECT_RUN\" > /dev/null; then\n+ if sane_grep \"is the first bad commit\" \"$GIT_DIR/BISECT_RUN\" > /dev/null; then\n \t echo \"bisect run success\"\n \t exit 0;\n fi\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex a480d6f..8ef1bde 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -457,7 +457,7 @@ if [ \"$filter_tag_name\" ]; then\n \t\t\t\tgit mktag) ||\n \t\t\t\tdie \"Could not create new tag object for $ref\"\n \t\t\tif git cat-file tag \"$ref\" | \\\n-\t\t\t grep '^-----BEGIN PGP SIGNATURE-----' >/dev/null 2>&1\n+\t\t\t sane_grep '^-----BEGIN PGP SIGNATURE-----' >/dev/null 2>&1\n \t\t\tthen\n \t\t\t\twarn \"gpg signature stripped from tag object $sha1t\"\n \t\t\tfi\ndiff --git a/git-instaweb.sh b/git-instaweb.sh\nindex d96eddb..84805c6 100755\n--- a/git-instaweb.sh\n+++ b/git-instaweb.sh\n@@ -41,7 +41,7 @@ resolve_full_httpd () {\n \tcase \"$httpd\" in\n \t*apache2*|*lighttpd*)\n \t\t# ensure that the apache2/lighttpd command ends with \"-f\"\n-\t\tif ! echo \"$httpd\" | grep -- '-f *$' >/dev/null 2>&1\n+\t\tif ! echo \"$httpd\" | sane_grep -- '-f *$' >/dev/null 2>&1\n \t\tthen\n \t\t\thttpd=\"$httpd -f\"\n \t\tfi\n@@ -297,8 +297,8 @@ EOF\n \n \t# check to see if Dennis Stosberg's mod_perl compatibility patch\n \t# (<20060621130708.Gcbc6e5c@leonov.stosberg.net>) has been applied\n-\tif test -f \"$module_path/mod_perl.so\" && grep 'MOD_PERL' \\\n-\t\t\t\t\"$GIT_DIR/gitweb/gitweb.cgi\" >/dev/null\n+\tif test -f \"$module_path/mod_perl.so\" &&\n+\t sane_grep 'MOD_PERL' \"$GIT_DIR/gitweb/gitweb.cgi\" >/dev/null\n \tthen\n \t\t# favor mod_perl if available\n \t\tcat >> \"$conf\" </dev/null 2>&1 || \\\n+\t\t$list_mods | sane_grep 'mod_cgi\\.c' >/dev/null 2>&1 || \\\n \t\techo \"LoadModule cgi_module $module_path/mod_cgi.so\" >> \"$conf\"\n \t\tcat >> \"$conf\" <> \"$DONE\"\n \tsed -e 1d < \"$TODO\" >> \"$TODO\".new\n \tmv -f \"$TODO\".new \"$TODO\"\n-\tcount=$(grep -c '^[^#]' < \"$DONE\")\n-\ttotal=$(($count+$(grep -c '^[^#]' < \"$TODO\")))\n+\tcount=$(sane_grep -c '^[^#]' < \"$DONE\")\n+\ttotal=$(($count+$(sane_grep -c '^[^#]' < \"$TODO\")))\n \tif test \"$last_count\" != \"$count\"\n \tthen\n \t\tlast_count=$count\n@@ -147,7 +147,7 @@ die_abort () {\n }\n \n has_action () {\n-\tgrep '^[^#]' \"$1\" >/dev/null\n+\tsane_grep '^[^#]' \"$1\" >/dev/null\n }\n \n pick_one () {\n@@ -731,7 +731,7 @@ first and then run 'git rebase --continue' again.\"\n \t\t\tgit rev-list $REVISIONS |\n \t\t\twhile read rev\n \t\t\tdo\n-\t\t\t\tif test -f \"$REWRITTEN\"/$rev -a \"$(grep \"$rev\" \"$DOTEST\"/not-cherry-picks)\" = \"\"\n+\t\t\t\tif test -f \"$REWRITTEN\"/$rev -a \"$(sane_grep \"$rev\" \"$DOTEST\"/not-cherry-picks)\" = \"\"\n \t\t\t\tthen\n \t\t\t\t\t# Use -f2 because if rev-list is telling us this commit is\n \t\t\t\t\t# not worthwhile, we don't want to track its multiple heads,\n@@ -739,7 +739,7 @@ first and then run 'git rebase --continue' again.\"\n \t\t\t\t\t# be rebasing on top of it\n \t\t\t\t\tgit rev-list --parents -1 $rev | cut -d' ' -s -f2 > \"$DROPPED\"/$rev\n \t\t\t\t\tshort=$(git rev-list -1 --abbrev-commit --abbrev=7 $rev)\n-\t\t\t\t\tgrep -v \"^[a-z][a-z]* $short\" <\"$TODO\" > \"${TODO}2\" ; mv \"${TODO}2\" \"$TODO\"\n+\t\t\t\t\tsane_grep -v \"^[a-z][a-z]* $short\" <\"$TODO\" > \"${TODO}2\" ; mv \"${TODO}2\" \"$TODO\"\n \t\t\t\t\trm \"$REWRITTEN\"/$rev\n \t\t\t\tfi\n \t\t\tdone\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex 6ec155c..0ec4355 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -467,7 +467,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\" | sane_grep \" .* \") > /dev/null\n then\n \tif test -z \"$force_rebase\"\n \tthen\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 0462e52..b7ccd12 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -57,7 +57,7 @@ resolve_relative_url ()\n #\n module_list()\n {\n-\tgit ls-files --error-unmatch --stage -- \"$@\" | grep '^160000 '\n+\tgit ls-files --error-unmatch --stage -- \"$@\" | sane_grep '^160000 '\n }\n \n #\n@@ -567,7 +567,7 @@ cmd_summary() {\n \tcd_to_toplevel\n \t# Get modified modules cared by user\n \tmodules=$(git $diff_cmd $cached --raw $head -- \"$@\" |\n-\t\tegrep '^:([0-7]* )?160000' |\n+\t\tsane_egrep '^:([0-7]* )?160000' |\n \t\twhile read mod_src mod_dst sha1_src sha1_dst status name\n \t\tdo\n \t\t\t# Always show modules deleted or type-changed (blob<->module)\n@@ -581,7 +581,7 @@ cmd_summary() {\n \ttest -z \"$modules\" && return\n \n \tgit $diff_cmd $cached --raw $head -- $modules |\n-\tegrep '^:([0-7]* )?160000' |\n+\tsane_egrep '^:([0-7]* )?160000' |\n \tcut -c2- |\n \twhile read mod_src mod_dst sha1_src sha1_dst status name\n \tdo\n"},{"id":"128214","messageId":"4B0B21CF.5040504@lsrfire.ath.cx","threadId":"21663","inReplyTo":"7v4ooku7cv.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] unset GREP_OPTIONS in test-lib.sh","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2009-11-23T23:59:11Z","receivedAt":"2009-11-23T23:59:11Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Junio C Hamano schrieb:\n> René Scharfe writes:\n> \n>> Yes, but what about git commands that are implemented as shell scripts\n>> and use grep? Something like the following patch?\n>>\n>> We'd need to run this from time to time to make sure no new grep calls\n>> creep in:\n>>\n>> git grep -L \"unset GREP_OPTIONS\" -- $(git grep -l \"grep\" git-*.sh)\n> \n> Hmm, but \"bisect run\" runs user's script and it may want to see\n> GREP_OPTIONS from the environment, no? Same for any of the hooks that am\n> and rebase might want to run.\n> \n> \n> \n> git-sh-setup.sh | 14 ++++++++++++++\n> git-am.sh | 4 ++--\n> git-bisect.sh | 4 ++--\n> git-filter-branch.sh | 2 +-\n> git-instaweb.sh | 8 ++++----\n> git-rebase--interactive.sh | 10 +++++-----\n> git-rebase.sh | 2 +-\n> git-submodule.sh | 6 +++---\n> 8 files changed, 32 insertions(+), 18 deletions(-)\n> \n> diff --git a/git-sh-setup.sh b/git-sh-setup.sh\n> index c41c2f7..2b2afa6 100755\n> --- a/git-sh-setup.sh\n> +++ b/git-sh-setup.sh\n> @@ -114,6 +114,20 @@ git_editor() {\n> \teval \"${GIT_EDITOR:=vi}\" '\"$@\"'\n> }\n> \n> +sane_grep () {\n> +\tGREP_OPTIONS= \\\n> +\tGREP_COLOR= \\\n> +\tGREP_COLORS= \\\n> +\tLC_ALL=C grep \"$@\"\n> +}\n> +\n> +sane_egrep () {\n> +\tGREP_OPTIONS= \\\n> +\tGREP_COLOR= \\\n> +\tGREP_COLORS= \\\n> +\tLC_ALL=C egrep \"$@\"\n> +}\n> +\n\nAh, yes, much nicer.\n\nRené\n"},{"id":"128215","messageId":"7vtywksqtd.fsf@alter.siamese.dyndns.org","threadId":"21663","inReplyTo":"4B0B21CF.5040504@lsrfire.ath.cx","subject":"Re: [PATCH] unset GREP_OPTIONS in test-lib.sh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-24T00:35:10Z","receivedAt":"2009-11-24T00:35:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n>> diff --git a/git-sh-setup.sh b/git-sh-setup.sh\n>> index c41c2f7..2b2afa6 100755\n>> --- a/git-sh-setup.sh\n>> +++ b/git-sh-setup.sh\n>> @@ -114,6 +114,20 @@ git_editor() {\n>> \teval \"${GIT_EDITOR:=vi}\" '\"$@\"'\n>> }\n>> \n>> +sane_grep () {\n>> +\tGREP_OPTIONS= \\\n>> +\tGREP_COLOR= \\\n>> +\tGREP_COLORS= \\\n>> +\tLC_ALL=C grep \"$@\"\n>> +}\n>> +\n>> +sane_egrep () {\n>> +\tGREP_OPTIONS= \\\n>> +\tGREP_COLOR= \\\n>> +\tGREP_COLORS= \\\n>> +\tLC_ALL=C egrep \"$@\"\n>> +}\n>> +\n>\n> Ah, yes, much nicer.\n\nActually I am having a second thought after spending some time trying to\ncome up with a commit log message. This leaves the door open for user\nscripts to honor the environment variables, but it also means that\neverybody needs to be aware of the insanity. It is really tempting to\ntreat these exactly like CDPATH and unconditionally unset it.\n\nOh, and unsetting GREP_COLORS/GREP_COLOR was a mistake, I think. As long\nas we do not pass --color (and unset GREP_OPTIONS to make sure it is not\ngiven), their settings should not matter to us.\n\n-- >8 --\nSubject: [PATCH] Protect scripted Porcelains from GREP_OPTIONS insanity\n\nIf the user has exported the GREP_OPTIONS environment variable, the output\nfrom \"grep\" and \"egrep\" in scripted Porcelains may be different from what\nthey expect. For example, we may want to count number of matching lines,\nby \"grep\" piped to \"wc -l\", and GREP_OPTIONS=-C3 will break such use.\n\nThe approach taken by this change to address this issue is to protect only\nour own use of grep/egrep. Because we do not unset it at the beginning of\nour scripts, hook scripts run from the scripted Porcelains are exposed to\nthe same insanity this environment variable causes when grep/egrep is used\nto implement logic (e.g. \"grep | wc -l\"), and it is entirely up to the\nhook scripts to protect themselves.\n\nOn the other hand, applypatch-msg hook may want to show offending words in\nthe proposed commit log message using grep to the end user, and the user\nmight want to set GREP_OPTIONS=--color to paint the match more visibly.\nThe approach to protect only our own use without unsetting the environment\nvariable globally will allow this use case.\n\nSigned-off-by: Junio C Hamano \n---\n git-sh-setup.sh | 8 ++++++++\n git-am.sh | 4 ++--\n git-bisect.sh | 4 ++--\n git-filter-branch.sh | 2 +-\n git-instaweb.sh | 8 ++++----\n git-rebase--interactive.sh | 10 +++++-----\n git-rebase.sh | 2 +-\n git-submodule.sh | 6 +++---\n 8 files changed, 26 insertions(+), 18 deletions(-)\n\ndiff --git a/git-sh-setup.sh b/git-sh-setup.sh\nindex c41c2f7..aa07cc3 100755\n--- a/git-sh-setup.sh\n+++ b/git-sh-setup.sh\n@@ -114,6 +114,14 @@ git_editor() {\n \teval \"${GIT_EDITOR:=vi}\" '\"$@\"'\n }\n \n+sane_grep () {\n+\tGREP_OPTIONS= LC_ALL=C grep \"$@\"\n+}\n+\n+sane_egrep () {\n+\tGREP_OPTIONS= LC_ALL=C egrep \"$@\"\n+}\n+\n is_bare_repository () {\n \tgit rev-parse --is-bare-repository\n }\ndiff --git a/git-am.sh b/git-am.sh\nindex c132f50..b49f26a 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -205,7 +205,7 @@ check_patch_format () {\n \t\t\t# and see if it looks like that they all begin with the\n \t\t\t# header field names...\n \t\t\tsed -n -e '/^$/q' -e '/^[ \t]/d' -e p \"$1\" |\n-\t\t\tLC_ALL=C egrep -v '^[!-9;-~]+:' >/dev/null ||\n+\t\t\tsane_egrep -v '^[!-9;-~]+:' >/dev/null ||\n \t\t\tpatch_format=mbox\n \t\tfi\n \t} < \"$1\" || clean_abort\n@@ -554,7 +554,7 @@ do\n \t\t\tstop_here $this\n \n \t\t# skip pine's internal folder data\n-\t\tgrep '^Author: Mail System Internal Data$' \\\n+\t\tsane_grep '^Author: Mail System Internal Data$' \\\n \t\t\t<\"$dotest\"/info >/dev/null &&\n \t\t\tgo_next && continue\n \ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex 6f6f039..0c422d5 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -393,7 +393,7 @@ bisect_run () {\n \n cat \"$GIT_DIR/BISECT_RUN\"\n \n- if grep \"first bad commit could be any of\" \"$GIT_DIR/BISECT_RUN\" \\\n+ if sane_grep \"first bad commit could be any of\" \"$GIT_DIR/BISECT_RUN\" \\\n \t\t> /dev/null; then\n \t echo >&2 \"bisect run cannot continue any more\"\n \t exit $res\n@@ -405,7 +405,7 @@ bisect_run () {\n \t exit $res\n fi\n \n- if grep \"is the first bad commit\" \"$GIT_DIR/BISECT_RUN\" > /dev/null; then\n+ if sane_grep \"is the first bad commit\" \"$GIT_DIR/BISECT_RUN\" > /dev/null; then\n \t echo \"bisect run success\"\n \t exit 0;\n fi\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex a480d6f..8ef1bde 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -457,7 +457,7 @@ if [ \"$filter_tag_name\" ]; then\n \t\t\t\tgit mktag) ||\n \t\t\t\tdie \"Could not create new tag object for $ref\"\n \t\t\tif git cat-file tag \"$ref\" | \\\n-\t\t\t grep '^-----BEGIN PGP SIGNATURE-----' >/dev/null 2>&1\n+\t\t\t sane_grep '^-----BEGIN PGP SIGNATURE-----' >/dev/null 2>&1\n \t\t\tthen\n \t\t\t\twarn \"gpg signature stripped from tag object $sha1t\"\n \t\t\tfi\ndiff --git a/git-instaweb.sh b/git-instaweb.sh\nindex d96eddb..84805c6 100755\n--- a/git-instaweb.sh\n+++ b/git-instaweb.sh\n@@ -41,7 +41,7 @@ resolve_full_httpd () {\n \tcase \"$httpd\" in\n \t*apache2*|*lighttpd*)\n \t\t# ensure that the apache2/lighttpd command ends with \"-f\"\n-\t\tif ! echo \"$httpd\" | grep -- '-f *$' >/dev/null 2>&1\n+\t\tif ! echo \"$httpd\" | sane_grep -- '-f *$' >/dev/null 2>&1\n \t\tthen\n \t\t\thttpd=\"$httpd -f\"\n \t\tfi\n@@ -297,8 +297,8 @@ EOF\n \n \t# check to see if Dennis Stosberg's mod_perl compatibility patch\n \t# (<20060621130708.Gcbc6e5c@leonov.stosberg.net>) has been applied\n-\tif test -f \"$module_path/mod_perl.so\" && grep 'MOD_PERL' \\\n-\t\t\t\t\"$GIT_DIR/gitweb/gitweb.cgi\" >/dev/null\n+\tif test -f \"$module_path/mod_perl.so\" &&\n+\t sane_grep 'MOD_PERL' \"$GIT_DIR/gitweb/gitweb.cgi\" >/dev/null\n \tthen\n \t\t# favor mod_perl if available\n \t\tcat >> \"$conf\" </dev/null 2>&1 || \\\n+\t\t$list_mods | sane_grep 'mod_cgi\\.c' >/dev/null 2>&1 || \\\n \t\techo \"LoadModule cgi_module $module_path/mod_cgi.so\" >> \"$conf\"\n \t\tcat >> \"$conf\" <> \"$DONE\"\n \tsed -e 1d < \"$TODO\" >> \"$TODO\".new\n \tmv -f \"$TODO\".new \"$TODO\"\n-\tcount=$(grep -c '^[^#]' < \"$DONE\")\n-\ttotal=$(($count+$(grep -c '^[^#]' < \"$TODO\")))\n+\tcount=$(sane_grep -c '^[^#]' < \"$DONE\")\n+\ttotal=$(($count+$(sane_grep -c '^[^#]' < \"$TODO\")))\n \tif test \"$last_count\" != \"$count\"\n \tthen\n \t\tlast_count=$count\n@@ -147,7 +147,7 @@ die_abort () {\n }\n \n has_action () {\n-\tgrep '^[^#]' \"$1\" >/dev/null\n+\tsane_grep '^[^#]' \"$1\" >/dev/null\n }\n \n pick_one () {\n@@ -731,7 +731,7 @@ first and then run 'git rebase --continue' again.\"\n \t\t\tgit rev-list $REVISIONS |\n \t\t\twhile read rev\n \t\t\tdo\n-\t\t\t\tif test -f \"$REWRITTEN\"/$rev -a \"$(grep \"$rev\" \"$DOTEST\"/not-cherry-picks)\" = \"\"\n+\t\t\t\tif test -f \"$REWRITTEN\"/$rev -a \"$(sane_grep \"$rev\" \"$DOTEST\"/not-cherry-picks)\" = \"\"\n \t\t\t\tthen\n \t\t\t\t\t# Use -f2 because if rev-list is telling us this commit is\n \t\t\t\t\t# not worthwhile, we don't want to track its multiple heads,\n@@ -739,7 +739,7 @@ first and then run 'git rebase --continue' again.\"\n \t\t\t\t\t# be rebasing on top of it\n \t\t\t\t\tgit rev-list --parents -1 $rev | cut -d' ' -s -f2 > \"$DROPPED\"/$rev\n \t\t\t\t\tshort=$(git rev-list -1 --abbrev-commit --abbrev=7 $rev)\n-\t\t\t\t\tgrep -v \"^[a-z][a-z]* $short\" <\"$TODO\" > \"${TODO}2\" ; mv \"${TODO}2\" \"$TODO\"\n+\t\t\t\t\tsane_grep -v \"^[a-z][a-z]* $short\" <\"$TODO\" > \"${TODO}2\" ; mv \"${TODO}2\" \"$TODO\"\n \t\t\t\t\trm \"$REWRITTEN\"/$rev\n \t\t\t\tfi\n \t\t\tdone\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex 6ec155c..0ec4355 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -467,7 +467,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\" | sane_grep \" .* \") > /dev/null\n then\n \tif test -z \"$force_rebase\"\n \tthen\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 0462e52..b7ccd12 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -57,7 +57,7 @@ resolve_relative_url ()\n #\n module_list()\n {\n-\tgit ls-files --error-unmatch --stage -- \"$@\" | grep '^160000 '\n+\tgit ls-files --error-unmatch --stage -- \"$@\" | sane_grep '^160000 '\n }\n \n #\n@@ -567,7 +567,7 @@ cmd_summary() {\n \tcd_to_toplevel\n \t# Get modified modules cared by user\n \tmodules=$(git $diff_cmd $cached --raw $head -- \"$@\" |\n-\t\tegrep '^:([0-7]* )?160000' |\n+\t\tsane_egrep '^:([0-7]* )?160000' |\n \t\twhile read mod_src mod_dst sha1_src sha1_dst status name\n \t\tdo\n \t\t\t# Always show modules deleted or type-changed (blob<->module)\n@@ -581,7 +581,7 @@ cmd_summary() {\n \ttest -z \"$modules\" && return\n \n \tgit $diff_cmd $cached --raw $head -- $modules |\n-\tegrep '^:([0-7]* )?160000' |\n+\tsane_egrep '^:([0-7]* )?160000' |\n \tcut -c2- |\n \twhile read mod_src mod_dst sha1_src sha1_dst status name\n \tdo\n-- \n1.6.6.rc0.15.g4fa80.dirty\n"},{"id":"128524","messageId":"20091127032234.GA2497@gmail.com","threadId":"21663","inReplyTo":"4B0B1ACD.6000008@lsrfire.ath.cx","subject":"Re: [PATCH] mergetool--lib: simplify guess_merge_tool()","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2009-11-27T03:22:35Z","receivedAt":"2009-11-27T03:22:35Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Tue, Nov 24, 2009 at 12:29:17AM +0100, René Scharfe wrote:\n> Use a case statement instead of calling grep to find out if the editor's\n> name contains the string \"vim\". Remove the check for emacs, as this\n> branch did the same as the default one anyway.\n> \n> Signed-off-by: Rene Scharfe \n> ---\n> This removes all grep calls from this script.\n\n\nVery nice.\nThanks,\n\n-- \n\t\tDavid\n"},{"id":"136253","messageId":"36ca99e91003070230r1755277aq7acd916d214e3ba7@mail.gmail.com","threadId":"21663","inReplyTo":"7v1vjvebem.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] unset GREP_OPTIONS in test-lib.sh","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2010-03-07T10:30:58Z","receivedAt":"2010-03-07T10:30:58Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Wed, Nov 18, 2009 at 23:05, Junio C Hamano wrote:\n> Bert Wesarg writes:\n>\n>> I used to set GREP_OPTIONS to exclude *.orig and *.rej files. But with this\n>> the test t4252-am-options.sh fails because it calls grep with a .rej file:\n>\n> Yuck.  Will apply.\n\nPing.\n\nRegards,\nBert\n"}]}