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

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

[Carlo Marcelo Arenas Belon]

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(-)

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

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

[Dave Rodgman]

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

> 
> 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

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

[Junio C Hamano]

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.

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

[Carlo Marcelo Arenas Belon]

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

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

[Junio C Hamano]

Carlo Marcelo Arenas Belon <carenas@sajinet.com.pe> writes:

> 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.

>> 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.

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

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> 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(-)

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";

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

[Carlo Marcelo Arenas Belon]

On Tue, Sep 08, 2009 at 12:02:51AM -0700, Junio C Hamano wrote:
> 
> Or even this.

definitely better.

Carlo