Re: [PATCH] rebase -p: avoid grep on potentailly non-ASCII data

From: Michael J Gruber <git@drmicha.warpmail.net>
Date: Mar 8, 2016, 13:45 UTC
Message-ID: <56DED770.4050603@drmicha.warpmail.net>
In-Reply-To: <56DEC4B4.2000902@web.de>

Torsten Bögershausen venit, vidit, dixit 08.03.2016 13:25:

Show 53 quoted lines

> On 03/08/2016 08:59 AM, Anders Kaseorg wrote:
>> The included test case, which uses rebase -p with non-ASCII commit
>> messages, was failing as follows:
>>
>>    Warning: the command isn't recognized in the following line:
>>     - Binary file (standard input) matches
>>
>>    You can fix this with 'git rebase --edit-todo'.
>>    Or you can abort the rebase with 'git rebase --abort'.
>>
>> Possibly related to recent GNU grep changes, as with commit
>> 316336379cf7937c2ecf122c7197cfe5da6b2061.  Avoid the issue by using sed
>> instead.
>>
>> Signed-off-by: Anders Kaseorg <andersk@mit.edu>
>> ---
>>   git-rebase--interactive.sh        |  2 +-
>>   t/t3409-rebase-preserve-merges.sh | 21 +++++++++++++++++++++
>>   2 files changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
>> index c0cfe88..0efc65c 100644
>> --- a/git-rebase--interactive.sh
>> +++ b/git-rebase--interactive.sh
>> @@ -1241,7 +1241,7 @@ then
>>   			# be rebasing on top of it
>>   			git rev-list --parents -1 $rev | cut -d' ' -s -f2 > "$dropped"/$rev
>>   			sha1=$(git rev-list -1 $rev)
>> -			sane_grep -v "^[a-z][a-z]* $sha1" <"$todo" > "${todo}2" ; mv "${todo}2" "$todo"
>> +			sed "/^[a-z][a-z]* $sha1/d" <"$todo" > "${todo}2" ; mv "${todo}2" "$todo"
>>   			rm "$rewritten"/$rev
>>   		fi
>>   	done
>> diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh
>> index 8c251c5..1f01b29 100755
>> --- a/t/t3409-rebase-preserve-merges.sh
>> +++ b/t/t3409-rebase-preserve-merges.sh
>> @@ -119,4 +119,25 @@ test_expect_success 'rebase -p ignores merge.log config' '
>>   	)
>>   '
>>   
>> +test_expect_success 'rebase -p works with non-ASCII commit message' '
>> +	(
>> +	mkdir non-ascii &&
> #The cd should be done in a subshell:
> (
>> +	cd non-ascii &&
>> +	git init &&
>> +	echo a > a &&
>> +	git add a &&
>> +	git commit -m a &&
>> +	echo b > b &&
> #Style: No space after ">" (and even above and below)

And also on the sane_grep/sed line.

Show 17 quoted lines

> 
> echo b >b
> 
> 
>> +	git add b &&
>> +	git commit -m b &&
>> +	git branch foo &&
>> +	git reset --hard HEAD^ &&
>> +	git cherry-pick -x foo &&
>> +	echo c > c &&
>> +	git add c &&
>> +	git commit -m "$(printf "I \\342\\231\\245 Unicode")" &&
>> +	git rebase -p foo
>> +	)
>> +
> #end of subshell
> )

The whole test is in a subshell already, although that is easy to miss (missing indentation).

>> '
>> +
>>   test_done
>

It may be worth noting whether other occurrences of "sane_grep" are safe from binary input.

After all, one my question the degree of sanity of our sane_grep, or whether we need asane_grep and bisane_grep in our shell library - "make our grep sane again".

Michael