Re: [PATCH] git-filter-branch: avoid collisions with variables in eval'ed commands

Hi,

On Wed, Mar 25, 2009 at 3:24 PM, Petr Baudis [off-list ref] wrote:

On Wed, Mar 25, 2009 at 03:13:01PM -0600, newren@gmail.com wrote:

quoted

From: Elijah Newren <redacted>

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

Almost-acked-by: Petr Baudis [off-list ref]

But:

quoted

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

quoted

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:

quoted

$ 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.  :-)

quoted

     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.

quoted

       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.