threads / patch / 20868

patchgit-rebase-interactive: avoid breaking when GREP_OPTIONS="-H"

Subject: [PATCH] git-rebase-interactive: avoid breaking when GREP_OPTIONS="-H"

## tl;dr

7 messages between Sep 7, 2009 and Sep 8, 2009. Diffs are folded; open one to read it.

replies: 6people: 3as markdown or json

Carlo Marcelo Arenas Belon· Sep 7, 2009, 12:56 UTC · lore

if GREP_OPTIONS is set and includes -H, using `grep -c` will fail to generate a numeric count and result in the following error :

  /usr/libexec/git-core/git-rebase--interactive: line 110: (standard
  input):1+(standard input):0: missing `)' (error token is
  "input):1+(standard input):0")
instead of grep counting use `wc -l` to return the line count.
Signed-off-by: Carlo Marcelo Arenas Belon <carenas@sajinet.com.pe>
---
 git-rebase--interactive.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Show changes to git-rebase--interactive.sh +2 −2
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 23ded48..c12d980 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -106,8 +106,8 @@ mark_action_done () {
 	sed -e 1q < "$TODO" >> "$DONE"
 	sed -e 1d < "$TODO" >> "$TODO".new
 	mv -f "$TODO".new "$TODO"
-	count=$(grep -c '^[^#]' < "$DONE")
-	total=$(($count+$(grep -c '^[^#]' < "$TODO")))
+	count=$(grep '^[^#]' < "$DONE" | wc -l)
+	total=$(($count+$(grep '^[^#]' < "$TODO" | wc -l)))
 	if test "$last_count" != "$count"
 	then
 		last_count=$count

-- 
1.6.3.3
Dave Rodgman· Sep 7, 2009, 13:25 UTC · re: Carlo Marcelo Arenas Belon · lore

Re: [PATCH] git-rebase-interactive: avoid breaking when GREP_OPTIONS="-H"

On Mon, 07 Sep 2009 05:56 -0700, "Carlo Marcelo Arenas Belon" <carenas@sajinet.com.pe> wrote:

Show 6 quoted lines
> if GREP_OPTIONS is set and includes -H, using `grep -c` will fail
> to generate a numeric count and result in the following error :
> 
>   /usr/libexec/git-core/git-rebase--interactive: line 110: (standard
>   input):1+(standard input):0: missing `)' (error token is
>   "input):1+(standard input):0")

I think in my case, grep is being confused by colours being enabled - I have this wrapper script for grep:

#!/bin/bash echo $@ `which -a grep|/bin/grep -v $0|head -n 1` --color=auto $@

your patch fixes it though.
thanks
Dave
Show 30 quoted lines
> 
> instead of grep counting use `wc -l` to return the line count.
> 
> Signed-off-by: Carlo Marcelo Arenas Belon <carenas@sajinet.com.pe>
> ---
>  git-rebase--interactive.sh |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 23ded48..c12d980 100755
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -106,8 +106,8 @@ mark_action_done () {
>  	sed -e 1q < "$TODO" >> "$DONE"
>  	sed -e 1d < "$TODO" >> "$TODO".new
>  	mv -f "$TODO".new "$TODO"
> -       count=$(grep -c '^[^#]' < "$DONE")
> -       total=$(($count+$(grep -c '^[^#]' < "$TODO")))
> +       count=$(grep '^[^#]' < "$DONE" | wc -l)
> +       total=$(($count+$(grep '^[^#]' < "$TODO" | wc -l)))
>  	if test "$last_count" != "$count"
>  	then
>  		last_count=$count
> -- 
> 1.6.3.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Junio C Hamano· Sep 7, 2009, 19:37 UTC · re: Carlo Marcelo Arenas Belon · lore

Re: [PATCH] git-rebase-interactive: avoid breaking when GREP_OPTIONS="-H"

Carlo Marcelo Arenas Belon <carenas@sajinet.com.pe> writes:
Show 8 quoted lines
> if GREP_OPTIONS is set and includes -H, using `grep -c` will fail
> to generate a numeric count and result in the following error :
>
>   /usr/libexec/git-core/git-rebase--interactive: line 110: (standard
>   input):1+(standard input):0: missing `)' (error token is
>   "input):1+(standard input):0")
>
> instead of grep counting use `wc -l` to return the line count.
Thanks.

How does your patch help when the user has GREP_OPTIONS=-C3 in the environment?

I think a saner workaround for this user environment bug (or GNU grep misfeature) is to unset GREP_OPTIONS at the beginning of the script, or even in git-sh-setup.

Carlo Marcelo Arenas Belon· Sep 8, 2009, 06:47 UTC · re: Junio C Hamano · lore

Re: [PATCH] git-rebase-interactive: avoid breaking when GREP_OPTIONS="-H"

On Mon, Sep 07, 2009 at 12:37:30PM -0700, Junio C Hamano wrote:
> 
> How does your patch help when the user has GREP_OPTIONS=-C3 in the
> environment?

It wouldn't help but at least wouldn't break aborting with an script error since you will always get a number.

> I think a saner workaround for this user environment bug (or GNU grep
> misfeature) is to unset GREP_OPTIONS at the beginning of the script, or
> even in git-sh-setup.

agree, and since grep is used almost everywhere filtering in git-sh-setup like CDPATH is makes sense, with the only user of grep that wouldn't benefit from that being git-mergetool--lib.sh AFAIK.

will test and submit a fix for that later, but still think the original patch at least improves the status quo (will protect also when using custom grep wrappers as reported earlier) and doesn't do any harm as wc is already a dependency as well and was part of the original code as well.

Carlo
Junio C Hamano· Sep 8, 2009, 06:54 UTC · re: Carlo Marcelo Arenas Belon · lore

Re: [PATCH] git-rebase-interactive: avoid breaking when GREP_OPTIONS="-H"

Carlo Marcelo Arenas Belon <carenas@sajinet.com.pe> writes:
Show 7 quoted lines
> On Mon, Sep 07, 2009 at 12:37:30PM -0700, Junio C Hamano wrote:
>> 
>> How does your patch help when the user has GREP_OPTIONS=-C3 in the
>> environment?
>
> It wouldn't help but at least wouldn't break aborting with an script
> error since you will always get a number.
That's actually worse, don't you think?

It is trying to count how many actions are done and how many are remaining, and if you miscount it in that shell function, you will get incorrect result. The function happens to be merely for reporting, but the point is that it is better to fail loudly than doing wrong thing.

Show 7 quoted lines
>> I think a saner workaround for this user environment bug (or GNU grep
>> misfeature) is to unset GREP_OPTIONS at the beginning of the script, or
>> even in git-sh-setup.
>
> agree, and since grep is used almost everywhere filtering in git-sh-setup
> like CDPATH is makes sense, with the only user of grep that wouldn't
> benefit from that being git-mergetool--lib.sh AFAIK.

Not at all. "git grep" itself will be broken. See my other patch for a possible alternative approach.

Junio C Hamano· Sep 8, 2009, 07:02 UTC · re: Junio C Hamano · lore

Re: [PATCH] git-rebase-interactive: avoid breaking when GREP_OPTIONS="-H"

Junio C Hamano <gitster@pobox.com> writes:
Show 19 quoted lines
> Carlo Marcelo Arenas Belon <carenas@sajinet.com.pe> writes:
>
>> if GREP_OPTIONS is set and includes -H, using `grep -c` will fail
>> to generate a numeric count and result in the following error :
>>
>>   /usr/libexec/git-core/git-rebase--interactive: line 110: (standard
>>   input):1+(standard input):0: missing `)' (error token is
>>   "input):1+(standard input):0")
>>
>> instead of grep counting use `wc -l` to return the line count.
>
> Thanks.
>
> How does your patch help when the user has GREP_OPTIONS=-C3 in the
> environment?
>
> I think a saner workaround for this user environment bug (or GNU grep
> misfeature) is to unset GREP_OPTIONS at the beginning of the script, or
> even in git-sh-setup.
Or even this.
 git.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)
Show changes to git.c +13 −0
diff --git a/git.c b/git.c
index 0b22595..3548154 100644
--- a/git.c
+++ b/git.c
@@ -450,11 +450,24 @@ static int run_argv(int *argcp, const char ***argv)
 	return done_alias;
 }
 
+static void sanitize_env(void) {
+	static const char *vars[] = {
+		"GREP_OPTIONS",
+		"GREP_COLOR",
+		"GREP_COLORS",
+		NULL,
+	};
+	const char **p;
+
+	for (p = vars; *p; p++)
+		unsetenv(*p);
+}
 
 int main(int argc, const char **argv)
 {
 	const char *cmd;
 
+	sanitize_env();
 	cmd = git_extract_argv0_path(argv[0]);
 	if (!cmd)
 		cmd = "git-help";

← back to recent threads