From: Elijah Newren <newren@gmail.com>
Date: Wed, 25 Mar 2009 21:33:31 GMT
Subject: Re: [PATCH] git-filter-branch: avoid collisions with variables in eval'ed commands
Message-ID: <51419b2c0903251433s75775206x556fc2d65a347d7d@mail.gmail.com>
In-Reply-To: <20090325212403.GU8940@machine.or.cz>

Hi,

On Wed, Mar 25, 2009 at 3:24 PM, Petr Baudis <pasky@suse.cz> wrote:
> On Wed, Mar 25, 2009 at 03:13:01PM -0600, newren@gmail.com wrote:
>> From: Elijah Newren <newren@gmail.com>
>>
>> Avoid using simple variable names like 'i', since user commands are eval'ed
>> and may clash with and overwrite our values.
>>
>> Signed-off-by: Elijah Newren <newren@gmail.com>
>
> Almost-acked-by: Petr Baudis <pasky@suse.cz>
>
> But:
>
>>-i=0
>>+git_filter_branch_count=0
>
> Why branch_count? It counts commits, not branches, doesn't it?

Oh, I was just changing i->git_filter_branch_i, then thought as long
as it was long I might as well use a word instead of "i".  Didn't
think about the combined meaning.  How about
"git_filter_branch_commit_count"?  Maybe a double underscore between
the "namespace" and the "variable"?

>> I discovered this a few months ago, but apparently never got around to
>> sending it earlier.  Anyway, without this patch in a repository with a
>> file called 'world' I see the following behavior:
>
> Some hints:
>
>> $ git filter-branch --tree-filter '
>>    for i in $(find . -type f); do
>
> This won't work right if your filenames contain $IFS.

Yeah, luckily for me, my repository didn't have any filenames with
whitespace.  :-)

>>      if ( file $i | grep "\btext\b" > /dev/null ); then
>
> if [[ "$(file $i)" == *text* ]] might run noticeably faster (though is
> slightly less precise). Having a filename-keyed cache of file types even
> more so.
>
>>        perl -i -ple "s/\\\$(Id|Date|Source|Header|CVSHeader|Author|Revision):[^\
>> \$]*\\$/\\$\1\\$/" $i;
>
> Using '\'' instead of " could save you quite a few backslashes in net
> count.

Cool, thanks for the tips.