Re: [PATCH] unset GREP_OPTIONS in test-lib.sh

From: René Scharfe <rene.scharfe@lsrfire.ath.cx>
Date: Nov 23, 2009, 23:18 UTC
Message-ID: <4B0B185B.4090305@lsrfire.ath.cx>
In-Reply-To: <7vtywlyu43.fsf@alter.siamese.dyndns.org>

Junio C Hamano schrieb:

Show 16 quoted lines

> Carlo Marcelo Arenas Belon <carenas@sajinet.com.pe> writes:
> 
>> why not better to apply the proposed patch from Junio in :
>>
>>   http://article.gmane.org/gmane.comp.version-control.git/127980/
>>
>> it would IMHO correct all reported issues and serve as well as a catch
>> all from other tools that could be introduced in the future and that
>> will be similarly affected by this misfeature.
> 
> I think René's patch is more sensible than $gmane/127980 because we have
> no business mucking with these environment variables when we are running
> things other than external grep.  You could be using system's "grep" in
> your pre-commit hook to find some stuff, and your hook either may rely
> on your having a particular set of GREP_OPTIONS in your environment, or
> may be designed to work well with GREP_OPTIONS.

Yes, but what about git commands that are implemented as shell scripts and use grep? Something like the following patch?

We'd need to run this from time to time to make sure no new grep calls creep in:

   git grep -L "unset GREP_OPTIONS" -- $(git grep -l "grep" git-*.sh)

-- 8< -- Unset GREP_OPTIONS at the top of git commands that are implemented as shell scripts and call grep, in order to avoid side effects caused by unexpected default options of users.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 git-am.sh                  |    3 +++
 git-bisect.sh              |    3 +++
 git-filter-branch.sh       |    3 +++
 git-instaweb.sh            |    3 +++
 git-notes.sh               |    3 +++
 git-rebase--interactive.sh |    3 +++
 git-rebase.sh              |    3 +++
 git-submodule.sh           |    3 +++
 8 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index 151512a..1390eec 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -38,6 +38,9 @@ set_reflog_action am
 require_work_tree
 cd_to_toplevel
 
+# Make sure we're in full control when calling grep in this script.
+unset GREP_OPTIONS
+
 git var GIT_COMMITTER_IDENT >/dev/null ||
 	die "You need to set your committer info first"
 
diff --git a/git-bisect.sh b/git-bisect.sh
index a5ea843..fcf500f 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -30,6 +30,9 @@ OPTIONS_SPEC=
 . git-sh-setup
 require_work_tree
 
+# Make sure we're in full control when calling grep in this script.
+unset GREP_OPTIONS
+
 _x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
 _x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
 
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 6b8b6a4..d3a8b3e 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -107,6 +107,9 @@ USAGE="[--env-filter <command>] [--tree-filter <command>]
 OPTIONS_SPEC=
 . git-sh-setup
 
+# Make sure we're in full control when calling grep in this script.
+unset GREP_OPTIONS
+
 if [ "$(is_bare_repository)" = false ]; then
 	git diff-files --ignore-submodules --quiet &&
 	git diff-index --cached --quiet HEAD -- ||
diff --git a/git-instaweb.sh b/git-instaweb.sh
index 622a5f0..86916e1 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -21,6 +21,9 @@ restart        restart the web server
 
 . git-sh-setup
 
+# Make sure we're in full control when calling grep in this script.
+unset GREP_OPTIONS
+
 fqgitdir="$GIT_DIR"
 local="$(git config --bool --get instaweb.local)"
 httpd="$(git config --get instaweb.httpd)"
diff --git a/git-notes.sh b/git-notes.sh
index e642e47..e5f0edf 100755
--- a/git-notes.sh
+++ b/git-notes.sh
@@ -3,6 +3,9 @@
 USAGE="(edit [-F <file> | -m <msg>] | show) [commit]"
 . git-sh-setup
 
+# Make sure we're in full control when calling grep in this script.
+unset GREP_OPTIONS
+
 test -z "$1" && usage
 ACTION="$1"; shift
 
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 27daaa9..d0bb8a3 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -33,6 +33,9 @@ root               rebase all reachable commmits up to the root(s)
 . git-sh-setup
 require_work_tree
 
+# Make sure we're in full control when calling grep in this script.
+unset GREP_OPTIONS
+
 DOTEST="$GIT_DIR/rebase-merge"
 TODO="$DOTEST"/git-rebase-todo
 DONE="$DOTEST"/done
diff --git a/git-rebase.sh b/git-rebase.sh
index 6830e16..18c680b 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -34,6 +34,9 @@ set_reflog_action rebase
 require_work_tree
 cd_to_toplevel
 
+# Make sure we're in full control when calling grep in this script.
+unset GREP_OPTIONS
+
 OK_TO_SKIP_PRE_REBASE=
 RESOLVEMSG="
 When you have resolved this problem run \"git rebase --continue\".
diff --git a/git-submodule.sh b/git-submodule.sh
index 850d423..e557aca 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -17,6 +17,9 @@ OPTIONS_SPEC=
 . git-parse-remote
 require_work_tree
 
+# Make sure we're in full control when calling grep in this script.
+unset GREP_OPTIONS
+
 command=
 branch=
 reference=

-- 
1.6.5