# [PATCH] git-filter-branch: avoid collisions with variables in eval'ed commands 6 messages from 2009-03-25 to 2009-03-25. Participants: newren@gmail.com, Petr Baudis, Elijah Newren, Junio C Hamano. Thread: https://gitlist.dev/t/18549 ## newren@gmail.com, 2009-03-25 21:13 Subject: [PATCH] git-filter-branch: avoid collisions with variables in eval'ed commands Message-ID: <1238015581-12801-1-git-send-email-newren@gmail.com> URL: https://gitlist.dev/e/1238015581-12801-1-git-send-email-newren%40gmail.com ``` From: Elijah Newren 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 --- 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: $ git filter-branch --tree-filter ' for i in $(find . -type f); do if ( file $i | grep "\btext\b" > /dev/null ); then perl -i -ple "s/\\\$(Id|Date|Source|Header|CVSHeader|Author|Revision):[^\ \$]*\\$/\\$\1\\$/" $i; fi; done ' -- --all Rewrite 7a78604e90b9debfaa6a755019b5d3df541abcba (1/6)/usr/bin/git-filter-branch : line 249: ./world+1: syntax error: operand expected (error token is "./world+1 ") In case anyone is wondering: This isn't the right way to nuke CVS keywords. It's a good start, but it'll miss a few files (particularly with older versions of the 'file' program) due to some types such as 'FORTRAN program source' not containing 'text'. Also, it's incredibly slow to the point that I dropped filter-branch entirely and wrote a 'git_fast_filter' helper (serving as a pipe between fast-export and fast-import) that I'll soon be making available. git-filter-branch.sh | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/git-filter-branch.sh b/git-filter-branch.sh index 20f6f51..5da63b1 100755 --- a/git-filter-branch.sh +++ b/git-filter-branch.sh @@ -272,10 +272,10 @@ test $commits -eq 0 && die "Found nothing to rewrite" # Rewrite the commits -i=0 +git_filter_branch_count=0 while read commit parents; do - i=$(($i+1)) - printf "\rRewrite $commit ($i/$commits)" + git_filter_branch_count=$(($git_filter_branch_count+1)) + printf "\rRewrite $commit ($git_filter_branch_count/$commits)" case "$filter_subdir" in "") -- 1.6.0.6 ``` ## Petr Baudis, 2009-03-25 21:24 Subject: Re: [PATCH] git-filter-branch: avoid collisions with variables in eval'ed commands Message-ID: <20090325212403.GU8940@machine.or.cz> URL: https://gitlist.dev/e/20090325212403.GU8940%40machine.or.cz In-Reply-To: <1238015581-12801-1-git-send-email-newren@gmail.com> ``` On Wed, Mar 25, 2009 at 03:13:01PM -0600, newren@gmail.com wrote: > From: Elijah Newren > > 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 Almost-acked-by: Petr Baudis But: >-i=0 >+git_filter_branch_count=0 Why branch_count? It counts commits, not branches, doesn't it? > 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. > 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. > fi; > done ' -- --all -- Petr "Pasky" Baudis The average, healthy, well-adjusted adult gets up at seven-thirty in the morning feeling just terrible. -- Jean Kerr ``` ## Elijah Newren, 2009-03-25 21:33 Subject: Re: [PATCH] git-filter-branch: avoid collisions with variables in eval'ed commands Message-ID: <51419b2c0903251433s75775206x556fc2d65a347d7d@mail.gmail.com> URL: https://gitlist.dev/e/51419b2c0903251433s75775206x556fc2d65a347d7d%40mail.gmail.com In-Reply-To: <20090325212403.GU8940@machine.or.cz> ``` Hi, On Wed, Mar 25, 2009 at 3:24 PM, Petr Baudis wrote: > On Wed, Mar 25, 2009 at 03:13:01PM -0600, newren@gmail.com wrote: >> From: Elijah Newren >> >> 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 > > Almost-acked-by: Petr Baudis > > 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. ``` ## newren@gmail.com, 2009-03-25 21:51 Subject: Re: [PATCH] git-filter-branch: avoid collisions with variables in eval'ed commands Message-ID: <1238017861-15292-1-git-send-email-newren@gmail.com> URL: https://gitlist.dev/e/1238017861-15292-1-git-send-email-newren%40gmail.com In-Reply-To: <51419b2c0903251433s75775206x556fc2d65a347d7d@mail.gmail.com> ``` From: Elijah Newren 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 --- If you have a different preference for the variable name, I'd be happy to change it (again). All I care about is avoiding conflicts with what people might pass in their scripts. git-filter-branch.sh | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/git-filter-branch.sh b/git-filter-branch.sh index 20f6f51..b90d3df 100755 --- a/git-filter-branch.sh +++ b/git-filter-branch.sh @@ -272,10 +272,10 @@ test $commits -eq 0 && die "Found nothing to rewrite" # Rewrite the commits -i=0 +git_filter_branch__commit_count=0 while read commit parents; do - i=$(($i+1)) - printf "\rRewrite $commit ($i/$commits)" + git_filter_branch__commit_count=$(($git_filter_branch__commit_count+1)) + printf "\rRewrite $commit ($git_filter_branch__commit_count/$commits)" case "$filter_subdir" in "") -- 1.6.0.6 ``` ## Petr Baudis, 2009-03-25 21:58 Subject: Re: [PATCH] git-filter-branch: avoid collisions with variables in eval'ed commands Message-ID: <20090325215848.GV8940@machine.or.cz> URL: https://gitlist.dev/e/20090325215848.GV8940%40machine.or.cz In-Reply-To: <51419b2c0903251433s75775206x556fc2d65a347d7d@mail.gmail.com> ``` On Wed, Mar 25, 2009 at 03:33:31PM -0600, Elijah Newren wrote: > Hi, > > On Wed, Mar 25, 2009 at 3:24 PM, Petr Baudis wrote: > > On Wed, Mar 25, 2009 at 03:13:01PM -0600, newren@gmail.com wrote: > >> From: Elijah Newren > >> > >> 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 > > > > Almost-acked-by: Petr Baudis > > > > 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"? Oh, I sort of thought it's "git-filter branch count", not "git-filter-branch count". I'm dumbed down from all day spent in IKEA I guess. :/ I'd personally just use $__git_i, $__git_commit or something, but YMMV. -- Petr "Pasky" Baudis The average, healthy, well-adjusted adult gets up at seven-thirty in the morning feeling just terrible. -- Jean Kerr ``` ## Junio C Hamano, 2009-03-25 23:34 Subject: Re: [PATCH] git-filter-branch: avoid collisions with variables in eval'ed commands Message-ID: <7vskl1w50i.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vskl1w50i.fsf%40gitster.siamese.dyndns.org In-Reply-To: <20090325215848.GV8940@machine.or.cz> ``` Petr Baudis writes: > On Wed, Mar 25, 2009 at 03:33:31PM -0600, Elijah Newren wrote: >> Hi, >> >> On Wed, Mar 25, 2009 at 3:24 PM, Petr Baudis wrote: >> > On Wed, Mar 25, 2009 at 03:13:01PM -0600, newren@gmail.com wrote: >> >> From: Elijah Newren >> >> >> >> 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 >> > >> > Almost-acked-by: Petr Baudis >> > >> > 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"? > > Oh, I sort of thought it's "git-filter branch count", not > "git-filter-branch count". I'm dumbed down from all day spent in IKEA > I guess. :/ > > I'd personally just use $__git_i, $__git_commit or something, but YMMV. Heh, I'll drop lmost-a from your earlier message and commit the result. Thanks. ```