{"thread":{"id":"20868","subject":"[PATCH] git-rebase-interactive: avoid breaking when GREP_OPTIONS=\"-H\"","startedAt":"2009-09-07T12:56:00Z","lastAt":"2009-09-08T08:31:07Z","messageCount":7,"participants":["Carlo Marcelo Arenas Belon","Dave Rodgman","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"122623","messageId":"1252328160-4359-1-git-send-email-carenas@sajinet.com.pe","threadId":"20868","inReplyTo":null,"subject":"[PATCH] git-rebase-interactive: avoid breaking when GREP_OPTIONS=\"-H\"","fromName":"Carlo Marcelo Arenas Belon","fromEmail":"carenas@sajinet.com.pe","sentAt":"2009-09-07T12:56:00Z","receivedAt":"2009-09-07T12:56:00Z","isPatch":true,"sender":{"key":"carenas@sajinet.com.pe","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"if GREP_OPTIONS is set and includes -H, using `grep -c` will fail\nto generate a numeric count and result in the following error :\n\n  /usr/libexec/git-core/git-rebase--interactive: line 110: (standard\n  input):1+(standard input):0: missing `)' (error token is\n  \"input):1+(standard input):0\")\n\ninstead of grep counting use `wc -l` to return the line count.\n\nSigned-off-by: Carlo Marcelo Arenas Belon <carenas@sajinet.com.pe>\n---\n git-rebase--interactive.sh |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 23ded48..c12d980 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -106,8 +106,8 @@ mark_action_done () {\n \tsed -e 1q < \"$TODO\" >> \"$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=$(grep '^[^#]' < \"$DONE\" | wc -l)\n+\ttotal=$(($count+$(grep '^[^#]' < \"$TODO\" | wc -l)))\n \tif test \"$last_count\" != \"$count\"\n \tthen\n \t\tlast_count=$count\n-- \n1.6.3.3\n"},{"id":"122624","messageId":"1252329924.15286.1333585269@webmail.messagingengine.com","threadId":"20868","inReplyTo":"1252328160-4359-1-git-send-email-carenas@sajinet.com.pe","subject":"Re: [PATCH] git-rebase-interactive: avoid breaking when GREP_OPTIONS=\"-H\"","fromName":"Dave Rodgman","fromEmail":"dav1dr@eml.cc","sentAt":"2009-09-07T13:25:24Z","receivedAt":"2009-09-07T13:25:24Z","isPatch":true,"sender":{"key":"dav1dr@eml.cc","avatar":null},"body":"\n\nOn Mon, 07 Sep 2009 05:56 -0700, \"Carlo Marcelo Arenas Belon\"\n<carenas@sajinet.com.pe> wrote:\n> if GREP_OPTIONS is set and includes -H, using `grep -c` will fail\n> to generate a numeric count and result in the following error :\n> \n>   /usr/libexec/git-core/git-rebase--interactive: line 110: (standard\n>   input):1+(standard input):0: missing `)' (error token is\n>   \"input):1+(standard input):0\")\n\nI think in my case, grep is being confused by colours being enabled - I\nhave this wrapper script\nfor grep:\n\n#!/bin/bash\necho $@\n`which -a grep|/bin/grep -v $0|head -n 1` --color=auto $@\n\nyour patch fixes it though.\n\nthanks\n\nDave\n\n> \n> instead of grep counting use `wc -l` to return the line count.\n> \n> Signed-off-by: Carlo Marcelo Arenas Belon <carenas@sajinet.com.pe>\n> ---\n>  git-rebase--interactive.sh |    4 ++--\n>  1 files changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n> index 23ded48..c12d980 100755\n> --- a/git-rebase--interactive.sh\n> +++ b/git-rebase--interactive.sh\n> @@ -106,8 +106,8 @@ mark_action_done () {\n>  \tsed -e 1q < \"$TODO\" >> \"$DONE\"\n>  \tsed -e 1d < \"$TODO\" >> \"$TODO\".new\n>  \tmv -f \"$TODO\".new \"$TODO\"\n> -       count=$(grep -c '^[^#]' < \"$DONE\")\n> -       total=$(($count+$(grep -c '^[^#]' < \"$TODO\")))\n> +       count=$(grep '^[^#]' < \"$DONE\" | wc -l)\n> +       total=$(($count+$(grep '^[^#]' < \"$TODO\" | wc -l)))\n>  \tif test \"$last_count\" != \"$count\"\n>  \tthen\n>  \t\tlast_count=$count\n> -- \n> 1.6.3.3\n> \n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"},{"id":"122639","messageId":"7v7hwar1fp.fsf@alter.siamese.dyndns.org","threadId":"20868","inReplyTo":"1252328160-4359-1-git-send-email-carenas@sajinet.com.pe","subject":"Re: [PATCH] git-rebase-interactive: avoid breaking when GREP_OPTIONS=\"-H\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-07T19:37:30Z","receivedAt":"2009-09-07T19:37:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlo Marcelo Arenas Belon <carenas@sajinet.com.pe> writes:\n\n> if GREP_OPTIONS is set and includes -H, using `grep -c` will fail\n> to generate a numeric count and result in the following error :\n>\n>   /usr/libexec/git-core/git-rebase--interactive: line 110: (standard\n>   input):1+(standard input):0: missing `)' (error token is\n>   \"input):1+(standard input):0\")\n>\n> instead of grep counting use `wc -l` to return the line count.\n\nThanks.\n\nHow does your patch help when the user has GREP_OPTIONS=-C3 in the\nenvironment?\n\nI think a saner workaround for this user environment bug (or GNU grep\nmisfeature) is to unset GREP_OPTIONS at the beginning of the script, or\neven in git-sh-setup.\n"},{"id":"122670","messageId":"20090908064756.GA14155@sajinet.com.pe","threadId":"20868","inReplyTo":"7v7hwar1fp.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-rebase-interactive: avoid breaking when GREP_OPTIONS=\"-H\"","fromName":"Carlo Marcelo Arenas Belon","fromEmail":"carenas@sajinet.com.pe","sentAt":"2009-09-08T06:47:56Z","receivedAt":"2009-09-08T06:47:56Z","isPatch":true,"sender":{"key":"carenas@sajinet.com.pe","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Mon, Sep 07, 2009 at 12:37:30PM -0700, Junio C Hamano wrote:\n> \n> How does your patch help when the user has GREP_OPTIONS=-C3 in the\n> environment?\n\nIt wouldn't help but at least wouldn't break aborting with an script\nerror since you will always get a number.\n\n> I think a saner workaround for this user environment bug (or GNU grep\n> misfeature) is to unset GREP_OPTIONS at the beginning of the script, or\n> even in git-sh-setup.\n\nagree, and since grep is used almost everywhere filtering in git-sh-setup\nlike CDPATH is makes sense, with the only user of grep that wouldn't\nbenefit from that being git-mergetool--lib.sh AFAIK.\n\nwill test and submit a fix for that later, but still think the original\npatch at least improves the status quo (will protect also when using\ncustom grep wrappers as reported earlier) and doesn't do any harm as wc\nis already a dependency as well and was part of the original code as well.\n\nCarlo\n"},{"id":"122672","messageId":"7vmy56gc4x.fsf@alter.siamese.dyndns.org","threadId":"20868","inReplyTo":"20090908064756.GA14155@sajinet.com.pe","subject":"Re: [PATCH] git-rebase-interactive: avoid breaking when GREP_OPTIONS=\"-H\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-08T06:54:06Z","receivedAt":"2009-09-08T06:54:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlo Marcelo Arenas Belon <carenas@sajinet.com.pe> writes:\n\n> On Mon, Sep 07, 2009 at 12:37:30PM -0700, Junio C Hamano wrote:\n>> \n>> How does your patch help when the user has GREP_OPTIONS=-C3 in the\n>> environment?\n>\n> It wouldn't help but at least wouldn't break aborting with an script\n> error since you will always get a number.\n\nThat's actually worse, don't you think?\n\nIt is trying to count how many actions are done and how many are\nremaining, and if you miscount it in that shell function, you will get\nincorrect result.  The function happens to be merely for reporting, but\nthe point is that it is better to fail loudly than doing wrong thing.\n\n>> I think a saner workaround for this user environment bug (or GNU grep\n>> misfeature) is to unset GREP_OPTIONS at the beginning of the script, or\n>> even in git-sh-setup.\n>\n> agree, and since grep is used almost everywhere filtering in git-sh-setup\n> like CDPATH is makes sense, with the only user of grep that wouldn't\n> benefit from that being git-mergetool--lib.sh AFAIK.\n\nNot at all.  \"git grep\" itself will be broken.  See my other patch for a\npossible alternative approach.\n"},{"id":"122675","messageId":"7vpra1gbqc.fsf@alter.siamese.dyndns.org","threadId":"20868","inReplyTo":"7v7hwar1fp.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-rebase-interactive: avoid breaking when GREP_OPTIONS=\"-H\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-08T07:02:51Z","receivedAt":"2009-09-08T07:02:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Carlo Marcelo Arenas Belon <carenas@sajinet.com.pe> writes:\n>\n>> if GREP_OPTIONS is set and includes -H, using `grep -c` will fail\n>> to generate a numeric count and result in the following error :\n>>\n>>   /usr/libexec/git-core/git-rebase--interactive: line 110: (standard\n>>   input):1+(standard input):0: missing `)' (error token is\n>>   \"input):1+(standard input):0\")\n>>\n>> instead of grep counting use `wc -l` to return the line count.\n>\n> Thanks.\n>\n> How does your patch help when the user has GREP_OPTIONS=-C3 in the\n> environment?\n>\n> I think a saner workaround for this user environment bug (or GNU grep\n> misfeature) is to unset GREP_OPTIONS at the beginning of the script, or\n> even in git-sh-setup.\n\nOr even this.\n\n git.c |   13 +++++++++++++\n 1 files changed, 13 insertions(+), 0 deletions(-)\n\ndiff --git a/git.c b/git.c\nindex 0b22595..3548154 100644\n--- a/git.c\n+++ b/git.c\n@@ -450,11 +450,24 @@ static int run_argv(int *argcp, const char ***argv)\n \treturn done_alias;\n }\n \n+static void sanitize_env(void) {\n+\tstatic const char *vars[] = {\n+\t\t\"GREP_OPTIONS\",\n+\t\t\"GREP_COLOR\",\n+\t\t\"GREP_COLORS\",\n+\t\tNULL,\n+\t};\n+\tconst char **p;\n+\n+\tfor (p = vars; *p; p++)\n+\t\tunsetenv(*p);\n+}\n \n int main(int argc, const char **argv)\n {\n \tconst char *cmd;\n \n+\tsanitize_env();\n \tcmd = git_extract_argv0_path(argv[0]);\n \tif (!cmd)\n \t\tcmd = \"git-help\";\n"},{"id":"122687","messageId":"20090908083107.GA14710@sajinet.com.pe","threadId":"20868","inReplyTo":"7vpra1gbqc.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-rebase-interactive: avoid breaking when GREP_OPTIONS=\"-H\"","fromName":"Carlo Marcelo Arenas Belon","fromEmail":"carenas@sajinet.com.pe","sentAt":"2009-09-08T08:31:07Z","receivedAt":"2009-09-08T08:31:07Z","isPatch":true,"sender":{"key":"carenas@sajinet.com.pe","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Tue, Sep 08, 2009 at 12:02:51AM -0700, Junio C Hamano wrote:\n> \n> Or even this.\n\ndefinitely better.\n\nCarlo\n"}]}