{"thread":{"id":"18549","subject":"[PATCH] git-filter-branch: avoid collisions with variables in eval'ed commands","startedAt":"2009-03-25T21:13:01Z","lastAt":"2009-03-25T23:34:53Z","messageCount":6,"participants":["newren@gmail.com","Petr Baudis","Elijah Newren","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"109416","messageId":"1238015581-12801-1-git-send-email-newren@gmail.com","threadId":"18549","inReplyTo":null,"subject":"[PATCH] git-filter-branch: avoid collisions with variables in eval'ed commands","fromName":"","fromEmail":"newren@gmail.com","sentAt":"2009-03-25T21:13:01Z","receivedAt":"2009-03-25T21:13:01Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nAvoid using simple variable names like 'i', since user commands are eval'ed\nand may clash with and overwrite our values.\n\nSigned-off-by: Elijah Newren \n---\n\nI discovered this a few months ago, but apparently never got around to\nsending it earlier. Anyway, without this patch in a repository with a\nfile called 'world' I see the following behavior:\n\n$ git filter-branch --tree-filter '\n for i in $(find . -type f); do\n if ( file $i | grep \"\\btext\\b\" > /dev/null ); then\n perl -i -ple \"s/\\\\\\$(Id|Date|Source|Header|CVSHeader|Author|Revision):[^\\\n\\$]*\\\\$/\\\\$\\1\\\\$/\" $i;\n fi;\n done ' -- --all\n\nRewrite 7a78604e90b9debfaa6a755019b5d3df541abcba (1/6)/usr/bin/git-filter-branch\n: line 249: ./world+1: syntax error: operand expected (error token is \"./world+1\n\")\n\n\nIn case anyone is wondering: This isn't the right way to nuke CVS keywords.\nIt's a good start, but it'll miss a few files (particularly with older\nversions of the 'file' program) due to some types such as 'FORTRAN program\nsource' not containing 'text'. Also, it's incredibly slow to the point\nthat I dropped filter-branch entirely and wrote a 'git_fast_filter' helper\n(serving as a pipe between fast-export and fast-import) that I'll soon be\nmaking available.\n\n git-filter-branch.sh | 6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 20f6f51..5da63b1 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -272,10 +272,10 @@ test $commits -eq 0 && die \"Found nothing to rewrite\"\n \n # Rewrite the commits\n \n-i=0\n+git_filter_branch_count=0\n while read commit parents; do\n-\ti=$(($i+1))\n-\tprintf \"\\rRewrite $commit ($i/$commits)\"\n+\tgit_filter_branch_count=$(($git_filter_branch_count+1))\n+\tprintf \"\\rRewrite $commit ($git_filter_branch_count/$commits)\"\n \n \tcase \"$filter_subdir\" in\n \t\"\")\n-- \n1.6.0.6\n"},{"id":"109419","messageId":"20090325212403.GU8940@machine.or.cz","threadId":"18549","inReplyTo":"1238015581-12801-1-git-send-email-newren@gmail.com","subject":"Re: [PATCH] git-filter-branch: avoid collisions with variables in eval'ed commands","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2009-03-25T21:24:03Z","receivedAt":"2009-03-25T21:24:03Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Wed, Mar 25, 2009 at 03:13:01PM -0600, newren@gmail.com wrote:\n> From: Elijah Newren \n> \n> Avoid using simple variable names like 'i', since user commands are eval'ed\n> and may clash with and overwrite our values.\n> \n> Signed-off-by: Elijah Newren \n\nAlmost-acked-by: Petr Baudis \n\nBut:\n\n>-i=0\n>+git_filter_branch_count=0\n\nWhy branch_count? It counts commits, not branches, doesn't it?\n\n\n> I discovered this a few months ago, but apparently never got around to\n> sending it earlier. Anyway, without this patch in a repository with a\n> file called 'world' I see the following behavior:\n\nSome hints:\n\n> $ git filter-branch --tree-filter '\n> for i in $(find . -type f); do\n\nThis won't work right if your filenames contain $IFS.\n\n> if ( file $i | grep \"\\btext\\b\" > /dev/null ); then\n\nif [[ \"$(file $i)\" == *text* ]] might run noticeably faster (though is\nslightly less precise). Having a filename-keyed cache of file types even\nmore so.\n\n> perl -i -ple \"s/\\\\\\$(Id|Date|Source|Header|CVSHeader|Author|Revision):[^\\\n> \\$]*\\\\$/\\\\$\\1\\\\$/\" $i;\n\nUsing '\\'' instead of \" could save you quite a few backslashes in net\ncount.\n\n> fi;\n> done ' -- --all\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nThe average, healthy, well-adjusted adult gets up at seven-thirty\nin the morning feeling just terrible. -- Jean Kerr\n"},{"id":"109422","messageId":"51419b2c0903251433s75775206x556fc2d65a347d7d@mail.gmail.com","threadId":"18549","inReplyTo":"20090325212403.GU8940@machine.or.cz","subject":"Re: [PATCH] git-filter-branch: avoid collisions with variables in eval'ed commands","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2009-03-25T21:33:31Z","receivedAt":"2009-03-25T21:33:31Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nOn Wed, Mar 25, 2009 at 3:24 PM, Petr Baudis wrote:\n> On Wed, Mar 25, 2009 at 03:13:01PM -0600, newren@gmail.com wrote:\n>> From: Elijah Newren \n>>\n>> Avoid using simple variable names like 'i', since user commands are eval'ed\n>> and may clash with and overwrite our values.\n>>\n>> Signed-off-by: Elijah Newren \n>\n> Almost-acked-by: Petr Baudis \n>\n> But:\n>\n>>-i=0\n>>+git_filter_branch_count=0\n>\n> Why branch_count? It counts commits, not branches, doesn't it?\n\nOh, I was just changing i->git_filter_branch_i, then thought as long\nas it was long I might as well use a word instead of \"i\". Didn't\nthink about the combined meaning. How about\n\"git_filter_branch_commit_count\"? Maybe a double underscore between\nthe \"namespace\" and the \"variable\"?\n\n>> I discovered this a few months ago, but apparently never got around to\n>> sending it earlier.  Anyway, without this patch in a repository with a\n>> file called 'world' I see the following behavior:\n>\n> Some hints:\n>\n>> $ git filter-branch --tree-filter '\n>>    for i in $(find . -type f); do\n>\n> This won't work right if your filenames contain $IFS.\n\nYeah, luckily for me, my repository didn't have any filenames with\nwhitespace. :-)\n\n>>      if ( file $i | grep \"\\btext\\b\" > /dev/null ); then\n>\n> if [[ \"$(file $i)\" == *text* ]] might run noticeably faster (though is\n> slightly less precise). Having a filename-keyed cache of file types even\n> more so.\n>\n>>        perl -i -ple \"s/\\\\\\$(Id|Date|Source|Header|CVSHeader|Author|Revision):[^\\\n>> \\$]*\\\\$/\\\\$\\1\\\\$/\" $i;\n>\n> Using '\\'' instead of \" could save you quite a few backslashes in net\n> count.\n\nCool, thanks for the tips.\n"},{"id":"109432","messageId":"1238017861-15292-1-git-send-email-newren@gmail.com","threadId":"18549","inReplyTo":"51419b2c0903251433s75775206x556fc2d65a347d7d@mail.gmail.com","subject":"Re: [PATCH] git-filter-branch: avoid collisions with variables in eval'ed commands","fromName":"","fromEmail":"newren@gmail.com","sentAt":"2009-03-25T21:51:01Z","receivedAt":"2009-03-25T21:51:01Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nAvoid using simple variable names like 'i', since user commands are eval'ed\nand may clash with and overwrite our values.\n\nSigned-off-by: Elijah Newren \n---\n\nIf you have a different preference for the variable name, I'd be happy to\nchange it (again). All I care about is avoiding conflicts with what people\nmight pass in their scripts.\n\n git-filter-branch.sh | 6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 20f6f51..b90d3df 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -272,10 +272,10 @@ test $commits -eq 0 && die \"Found nothing to rewrite\"\n \n # Rewrite the commits\n \n-i=0\n+git_filter_branch__commit_count=0\n while read commit parents; do\n-\ti=$(($i+1))\n-\tprintf \"\\rRewrite $commit ($i/$commits)\"\n+\tgit_filter_branch__commit_count=$(($git_filter_branch__commit_count+1))\n+\tprintf \"\\rRewrite $commit ($git_filter_branch__commit_count/$commits)\"\n \n \tcase \"$filter_subdir\" in\n \t\"\")\n-- \n1.6.0.6\n"},{"id":"109434","messageId":"20090325215848.GV8940@machine.or.cz","threadId":"18549","inReplyTo":"51419b2c0903251433s75775206x556fc2d65a347d7d@mail.gmail.com","subject":"Re: [PATCH] git-filter-branch: avoid collisions with variables in eval'ed commands","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2009-03-25T21:58:48Z","receivedAt":"2009-03-25T21:58:48Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Wed, Mar 25, 2009 at 03:33:31PM -0600, Elijah Newren wrote:\n> Hi,\n> \n> On Wed, Mar 25, 2009 at 3:24 PM, Petr Baudis wrote:\n> > On Wed, Mar 25, 2009 at 03:13:01PM -0600, newren@gmail.com wrote:\n> >> From: Elijah Newren \n> >>\n> >> Avoid using simple variable names like 'i', since user commands are eval'ed\n> >> and may clash with and overwrite our values.\n> >>\n> >> Signed-off-by: Elijah Newren \n> >\n> > Almost-acked-by: Petr Baudis \n> >\n> > But:\n> >\n> >>-i=0\n> >>+git_filter_branch_count=0\n> >\n> > Why branch_count? It counts commits, not branches, doesn't it?\n> \n> Oh, I was just changing i->git_filter_branch_i, then thought as long\n> as it was long I might as well use a word instead of \"i\". Didn't\n> think about the combined meaning. How about\n> \"git_filter_branch_commit_count\"? Maybe a double underscore between\n> the \"namespace\" and the \"variable\"?\n\nOh, I sort of thought it's \"git-filter branch count\", not\n\"git-filter-branch count\". I'm dumbed down from all day spent in IKEA\nI guess. :/\n\nI'd personally just use $__git_i, $__git_commit or something, but YMMV.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nThe average, healthy, well-adjusted adult gets up at seven-thirty\nin the morning feeling just terrible. -- Jean Kerr\n"},{"id":"109443","messageId":"7vskl1w50i.fsf@gitster.siamese.dyndns.org","threadId":"18549","inReplyTo":"20090325215848.GV8940@machine.or.cz","subject":"Re: [PATCH] git-filter-branch: avoid collisions with variables in eval'ed commands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-25T23:34:53Z","receivedAt":"2009-03-25T23:34:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Petr Baudis writes:\n\n> On Wed, Mar 25, 2009 at 03:33:31PM -0600, Elijah Newren wrote:\n>> Hi,\n>> \n>> On Wed, Mar 25, 2009 at 3:24 PM, Petr Baudis wrote:\n>> > On Wed, Mar 25, 2009 at 03:13:01PM -0600, newren@gmail.com wrote:\n>> >> From: Elijah Newren \n>> >>\n>> >> Avoid using simple variable names like 'i', since user commands are eval'ed\n>> >> and may clash with and overwrite our values.\n>> >>\n>> >> Signed-off-by: Elijah Newren \n>> >\n>> > Almost-acked-by: Petr Baudis \n>> >\n>> > But:\n>> >\n>> >>-i=0\n>> >>+git_filter_branch_count=0\n>> >\n>> > Why branch_count? It counts commits, not branches, doesn't it?\n>> \n>> Oh, I was just changing i->git_filter_branch_i, then thought as long\n>> as it was long I might as well use a word instead of \"i\". Didn't\n>> think about the combined meaning. How about\n>> \"git_filter_branch_commit_count\"? Maybe a double underscore between\n>> the \"namespace\" and the \"variable\"?\n>\n> Oh, I sort of thought it's \"git-filter branch count\", not\n> \"git-filter-branch count\". I'm dumbed down from all day spent in IKEA\n> I guess. :/\n>\n> I'd personally just use $__git_i, $__git_commit or something, but YMMV.\n\nHeh, I'll drop lmost-a from your earlier message and commit the result.\n\nThanks.\n"}]}