git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "René Scharfe" <rene.scharfe@lsrfire.ath.cx>
To: Junio C Hamano <gitster@pobox.com>
Cc: Carlo Marcelo Arenas Belon <carenas@sajinet.com.pe>,
	Bert Wesarg <bert.wesarg@googlemail.com>,
	git@vger.kernel.org
Subject: Re: [PATCH] unset GREP_OPTIONS in test-lib.sh
Date: Tue, 24 Nov 2009 00:18:51 +0100	[thread overview]
Message-ID: <4B0B185B.4090305@lsrfire.ath.cx> (raw)
In-Reply-To: <7vtywlyu43.fsf@alter.siamese.dyndns.org>

Junio C Hamano schrieb:
> 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

  reply	other threads:[~2009-11-23 23:19 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-18 16:15 [PATCH] unset GREP_OPTIONS in test-lib.sh Bert Wesarg
2009-11-18 22:05 ` Junio C Hamano
2009-11-22 15:58   ` René Scharfe
2009-11-23 11:22     ` Carlo Marcelo Arenas Belon
2009-11-23 18:27       ` Junio C Hamano
2009-11-23 23:18         ` René Scharfe [this message]
2009-11-23 23:29           ` [PATCH] mergetool--lib: simplify guess_merge_tool() René Scharfe
2009-11-27  3:22             ` David Aguilar
2009-11-23 23:52           ` [PATCH] unset GREP_OPTIONS in test-lib.sh Junio C Hamano
2009-11-23 23:59             ` René Scharfe
2009-11-24  0:35               ` Junio C Hamano
2010-03-07 10:30   ` Bert Wesarg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4B0B185B.4090305@lsrfire.ath.cx \
    --to=rene.scharfe@lsrfire.ath.cx \
    --cc=bert.wesarg@googlemail.com \
    --cc=carenas@sajinet.com.pe \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).