{"thread":{"id":"31845","subject":"What can cause empty GIT_AUTHOR_NAME for 'git filter-branch --tree-filter' on Solaris?","startedAt":"2012-10-17T06:47:29Z","lastAt":"2012-11-12T20:32:26Z","messageCount":24,"participants":["Ilya Basin","Jeff King","Johannes Sixt","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"201405","messageId":"1109432467.20121017104729@gmail.com","threadId":"31845","inReplyTo":null,"subject":"What can cause empty GIT_AUTHOR_NAME for 'git filter-branch --tree-filter' on Solaris?","fromName":"Ilya Basin","fromEmail":"basinilya@gmail.com","sentAt":"2012-10-17T06:47:29Z","receivedAt":"2012-10-17T06:47:29Z","isPatch":false,"sender":{"key":"basinilya@gmail.com","avatar":null},"body":"The filter-branch command, the contents of ~/.gitconfig and the tree\nare the same.\nThe command succeeds on cygwin, but fails on Solaris due to\nunset GIT_AUTHOR_NAME and GIT_COMMITTER_NAME :\n\n$ git filter-branch --tree-filter \"env | grep GIT_ ; $CMD\" b416b9bfc5e71531f2f05af4c396bb0ba7560741..HEAD\nRewrite 214efc6eec82b015aefe23b2280979f05b351396 (1/16)GIT_DIR=/home/tester/.ilya/builds/makepkg.rap_0.1-1_sparc.XXXXXX/src/rap/.git\nGIT_INDEX_FILE=/home/tester/.ilya/builds/makepkg.rap_0.1-1_sparc.XXXXXX/src/rap/.git-rewrite/t/../index\nGIT_WORK_TREE=.\nGIT_AUTHOR_NAME=\nGIT_COMMITTER_NAME=\nGIT_COMMIT=214efc6eec82b015aefe23b2280979f05b351396\nfatal: empty ident not allowed\ncould not write rewritten commit\n\nIf I explicitly set these 2 variables, filter-branch succeeds, but\nother commit attributes like commit date aren't preserved.\n\nI use git 1.7.6, from sunfreeware.\n\nI hope there is some simple thing that needs to be configured.\n"},{"id":"201408","messageId":"20121017071817.GA15419@sigill.intra.peff.net","threadId":"31845","inReplyTo":"1109432467.20121017104729@gmail.com","subject":"Re: What can cause empty GIT_AUTHOR_NAME for 'git filter-branch --tree-filter' on Solaris?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-17T07:18:17Z","receivedAt":"2012-10-17T07:18:17Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 17, 2012 at 10:47:29AM +0400, Ilya Basin wrote:\n\n> The filter-branch command, the contents of ~/.gitconfig and the tree\n> are the same.\n> The command succeeds on cygwin, but fails on Solaris due to\n> unset GIT_AUTHOR_NAME and GIT_COMMITTER_NAME :\n\nThat shouldn't happen. The likely culprit is that the sed magic in the\nset_ident function of git-filter-branch is not portable to your version\nof sed.\n\nWhat happens if you run this:\n\n\techo 'author Your Name 1350408529 -0400' >commit\n\tset -- author\n\tlid=\"$(echo \"$1\" | tr \"[A-Z]\" \"[a-z]\")\"\n\tuid=\"$(echo \"$1\" | tr \"[a-z]\" \"[A-Z]\")\"\n pick_id_script='\n /^'$lid' /{\n s/'\\''/'\\''\\\\'\\'\\''/g\n h\n s/^'$lid' \\([^<]*\\) <[^>]*> .*$/\\1/\n s/'\\''/'\\''\\'\\'\\''/g\n s/.*/GIT_'$uid'_NAME='\\''&'\\''; export GIT_'$uid'_NAME/p\n\n g\n s/^'$lid' [^<]* <\\([^>]*\\)> .*$/\\1/\n s/'\\''/'\\''\\'\\'\\''/g\n s/.*/GIT_'$uid'_EMAIL='\\''&'\\''; export GIT_'$uid'_EMAIL/p\n\n g\n s/^'$lid' [^<]* <[^>]*> \\(.*\\)$/@\\1/\n s/'\\''/'\\''\\'\\'\\''/g\n s/.*/GIT_'$uid'_DATE='\\''&'\\''; export GIT_'$uid'_DATE/p\n\n q\n }\n '\n LANG=C LC_ALL=C sed -ne \"$pick_id_script\" I use git 1.7.6, from sunfreeware.\n\nIt might also be worth testing v1.7.12, but reading the logs, I don't\nthink there has been any meaningful update to filter-branch since then.\n\n-Peff\n"},{"id":"201409","messageId":"507E5CE0.10002@viscovery.net","threadId":"31845","inReplyTo":"1109432467.20121017104729@gmail.com","subject":"Re: What can cause empty GIT_AUTHOR_NAME for 'git filter-branch --tree-filter' on Solaris?","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2012-10-17T07:23:12Z","receivedAt":"2012-10-17T07:23:12Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 10/17/2012 8:47, schrieb Ilya Basin:\n> The filter-branch command, the contents of ~/.gitconfig and the tree\n> are the same.\n> The command succeeds on cygwin, but fails on Solaris due to\n> unset GIT_AUTHOR_NAME and GIT_COMMITTER_NAME :\n> \n> $ git filter-branch --tree-filter \"env | grep GIT_ ; $CMD\" b416b9bfc5e71531f2f05af4c396bb0ba7560741..HEAD\n> Rewrite 214efc6eec82b015aefe23b2280979f05b351396 (1/16)GIT_DIR=/home/tester/.ilya/builds/makepkg.rap_0.1-1_sparc.XXXXXX/src/rap/.git\n> GIT_INDEX_FILE=/home/tester/.ilya/builds/makepkg.rap_0.1-1_sparc.XXXXXX/src/rap/.git-rewrite/t/../index\n> GIT_WORK_TREE=.\n> GIT_AUTHOR_NAME=\n> GIT_COMMITTER_NAME=\n> GIT_COMMIT=214efc6eec82b015aefe23b2280979f05b351396\n> fatal: empty ident not allowed\n> could not write rewritten commit\n\nMost likely, your sed has problems with a sed script in function\nget_author_ident_from_commit. I tested it like this:\n\n$ sh -c '. $(git --exec-path)/git-sh-setup;\n\t\tget_author_ident_from_commit HEAD'\nGIT_AUTHOR_NAME='Johannes Sixt'\nGIT_AUTHOR_EMAIL='j6t@kdbg.org'\nGIT_AUTHOR_DATE='@1350025129 +0200'\n\n-- Hannes\n"},{"id":"201416","messageId":"1013956402.20121017125847@gmail.com","threadId":"31845","inReplyTo":"507E5CE0.10002@viscovery.net","subject":"Re[2]: What can cause empty GIT_AUTHOR_NAME for 'git filter-branch --tree-filter' on Solaris?","fromName":"Ilya Basin","fromEmail":"basinilya@gmail.com","sentAt":"2012-10-17T08:58:47Z","receivedAt":"2012-10-17T08:58:47Z","isPatch":false,"sender":{"key":"basinilya@gmail.com","avatar":null},"body":"JS> Most likely, your sed has problems with a sed script in function\nJS> get_author_ident_from_commit. I tested it like this:\n\nJS> $ sh -c '. $(git --exec-path)/git-sh-setup;\nJS> get_author_ident_from_commit HEAD'\nJS> GIT_AUTHOR_NAME='Johannes Sixt'\nJS> GIT_AUTHOR_EMAIL='j6t@kdbg.org'\nJS> GIT_AUTHOR_DATE='@1350025129 +0200'\n\nJS> -- Hannes\n\nBoth systems have GNU sed 4.2.1 installed. I wrote a wrapper script wor sed.\nIt's output attached.\nThe difference is letter case in sed input data:\nSolaris:\n /^AUTHOR /\nWindows:\n /^author /\n\n-- \n\n$ git filter-branch -f --tree-filter \"env | grep GIT_; true\" HEAD~1..HEAD\n\nSED BEGIN\nSED ARGUMENTS: -e s/-/ /\nSED STDIN BEGIN\ngit-filter-branch\n\nSED STDIN END\n\nSED OUTPUT BEGIN\ngit filter-branch\n\nSED OUTPUT END\nSED EXIT CODE: 0\nSED END\n\nSED BEGIN\nSED ARGUMENTS: -e /^^/d /cygdrive/c/sicap/rap/gitcvs/RAP27/.git-rewrite/raw-heads\nSED INPUT FILE BEGIN: /cygdrive/c/sicap/rap/gitcvs/RAP27/.git-rewrite/raw-heads\nrefs/heads/master\n\nSED INPUT FILE END: /cygdrive/c/sicap/rap/gitcvs/RAP27/.git-rewrite/raw-heads\n\nSED OUTPUT BEGIN\nrefs/heads/master\n\nSED OUTPUT END\nSED EXIT CODE: 0\nSED END\nRewrite acd1d2bb1984c96630d5070497590307151c4682 (1/1)\nSED BEGIN\nSED ARGUMENTS: -ne\n /^author /{\n s/'/'\\\\''/g\n h\n s/^author \\([^<]*\\) <[^>]*> .*$/\\1/\n s/'/'\\''/g\n s/.*/GIT_AUTHOR_NAME='&'; export GIT_AUTHOR_NAME/p\n\n g\n s/^author [^<]* <\\([^>]*\\)> .*$/\\1/\n s/'/'\\''/g\n s/.*/GIT_AUTHOR_EMAIL='&'; export GIT_AUTHOR_EMAIL/p\n\n g\n s/^author [^<]* <[^>]*> \\(.*\\)$/\\1/\n s/'/'\\''/g\n s/.*/GIT_AUTHOR_DATE='&'; export GIT_AUTHOR_DATE/p\n\n q\n }\n\nSED STDIN BEGIN\ntree 969f563d319049bb6dabc12054d67671499a6f55\nparent c4734950e37c09ca7d3e3088f6f31d866dbb5077\nauthor Ilya Basin 1350401059 +0400\ncommitter Ilya Basin 1350405585 +0400\n\ntemp\n\nSED STDIN END\n\nSED OUTPUT BEGIN\nGIT_AUTHOR_NAME='Ilya Basin'; export GIT_AUTHOR_NAME\nGIT_AUTHOR_EMAIL='basinilya@gmail.com'; export GIT_AUTHOR_EMAIL\nGIT_AUTHOR_DATE='1350401059 +0400'; export GIT_AUTHOR_DATE\n\nSED OUTPUT END\nSED EXIT CODE: 0\nSED END\n\nSED BEGIN\nSED ARGUMENTS: -ne\n /^committer /{\n s/'/'\\\\''/g\n h\n s/^committer \\([^<]*\\) <[^>]*> .*$/\\1/\n s/'/'\\''/g\n s/.*/GIT_COMMITTER_NAME='&'; export GIT_COMMITTER_NAME/p\n\n g\n s/^committer [^<]* <\\([^>]*\\)> .*$/\\1/\n s/'/'\\''/g\n s/.*/GIT_COMMITTER_EMAIL='&'; export GIT_COMMITTER_EMAIL/p\n\n g\n s/^committer [^<]* <[^>]*> \\(.*\\)$/\\1/\n s/'/'\\''/g\n s/.*/GIT_COMMITTER_DATE='&'; export GIT_COMMITTER_DATE/p\n\n q\n }\n\nSED STDIN BEGIN\ntree 969f563d319049bb6dabc12054d67671499a6f55\nparent c4734950e37c09ca7d3e3088f6f31d866dbb5077\nauthor Ilya Basin 1350401059 +0400\ncommitter Ilya Basin 1350405585 +0400\n\ntemp\n\nSED STDIN END\n\nSED OUTPUT BEGIN\nGIT_COMMITTER_NAME='Ilya Basin'; export GIT_COMMITTER_NAME\nGIT_COMMITTER_EMAIL='basinilya@gmail.com'; export GIT_COMMITTER_EMAIL\nGIT_COMMITTER_DATE='1350405585 +0400'; export GIT_COMMITTER_DATE\n\nSED OUTPUT END\nSED EXIT CODE: 0\nSED END\nGIT_DIR=/cygdrive/c/sicap/rap/gitcvs/RAP27/.git\nGIT_AUTHOR_DATE=1350401059 +0400\nGIT_INDEX_FILE=/cygdrive/c/sicap/rap/gitcvs/RAP27/.git-rewrite/t/../index\nGIT_WORK_TREE=.\nGIT_AUTHOR_NAME=Ilya Basin\nGIT_COMMITTER_NAME=Ilya Basin\nGIT_COMMIT=acd1d2bb1984c96630d5070497590307151c4682\nGIT_COMMITTER_EMAIL=basinilya@gmail.com\nGIT_COMMITTER_DATE=1350405585 +0400\nGIT_AUTHOR_EMAIL=basinilya@gmail.com\n\nSED BEGIN\nSED ARGUMENTS: -e 1,/^$/d\nSED STDIN BEGIN\ntree 969f563d319049bb6dabc12054d67671499a6f55\nparent c4734950e37c09ca7d3e3088f6f31d866dbb5077\nauthor Ilya Basin 1350401059 +0400\ncommitter Ilya Basin 1350405585 +0400\n\ntemp\n\nSED STDIN END\n\nSED OUTPUT BEGIN\ntemp\n\nSED OUTPUT END\nSED EXIT CODE: 0\nSED END\n\nWARNING: Ref 'refs/heads/master' is unchanged\n\n\n-bash-3.00$ git filter-branch -f --tree-filter \"env | grep GIT_; true\" HEAD~1..HEAD\n\nSED BEGIN\nSED ARGUMENTS: -e s/-/ /\nSED STDIN BEGIN\ngit-filter-branch\n\nSED STDIN END\n\nSED OUTPUT BEGIN\ngit filter-branch\n\nSED OUTPUT END\nSED EXIT CODE: 0\nSED END\n\nSED BEGIN\nSED ARGUMENTS: -e /^^/d /home/tester/.ilya/builds/makepkg.rap_0.1-1_sparc.XXXXXX/src/rap/.git-rewrite/raw-heads\nSED INPUT FILE BEGIN: /home/tester/.ilya/builds/makepkg.rap_0.1-1_sparc.XXXXXX/src/rap/.git-rewrite/raw-heads\nrefs/heads/master\n\nSED INPUT FILE END: /home/tester/.ilya/builds/makepkg.rap_0.1-1_sparc.XXXXXX/src/rap/.git-rewrite/raw-heads\n\nSED OUTPUT BEGIN\nrefs/heads/master\n\nSED OUTPUT END\nSED EXIT CODE: 0\nSED END\nRewrite acd1d2bb1984c96630d5070497590307151c4682 (1/1)\nSED BEGIN\nSED ARGUMENTS: -ne\n /^AUTHOR /{\n s/'/'\\\\''/g\n h\n s/^AUTHOR \\([^<]*\\) <[^>]*> .*$/\\1/\n s/'/'\\''/g\n s/.*/GIT_AUTHOR_NAME='&'; export GIT_AUTHOR_NAME/p\n\n g\n s/^AUTHOR [^<]* <\\([^>]*\\)> .*$/\\1/\n s/'/'\\''/g\n s/.*/GIT_AUTHOR_EMAIL='&'; export GIT_AUTHOR_EMAIL/p\n\n g\n s/^AUTHOR [^<]* <[^>]*> \\(.*\\)$/\\1/\n s/'/'\\''/g\n s/.*/GIT_AUTHOR_DATE='&'; export GIT_AUTHOR_DATE/p\n\n q\n }\n\nSED STDIN BEGIN\ntree 969f563d319049bb6dabc12054d67671499a6f55\nparent c4734950e37c09ca7d3e3088f6f31d866dbb5077\nauthor Ilya Basin 1350401059 +0400\ncommitter Ilya Basin 1350405585 +0400\n\ntemp\n\nSED STDIN END\n\nSED OUTPUT BEGIN\n\nSED OUTPUT END\nSED EXIT CODE: 0\nSED END\n\nSED BEGIN\nSED ARGUMENTS: -ne\n /^COMMITTER /{\n s/'/'\\\\''/g\n h\n s/^COMMITTER \\([^<]*\\) <[^>]*> .*$/\\1/\n s/'/'\\''/g\n s/.*/GIT_COMMITTER_NAME='&'; export GIT_COMMITTER_NAME/p\n\n g\n s/^COMMITTER [^<]* <\\([^>]*\\)> .*$/\\1/\n s/'/'\\''/g\n s/.*/GIT_COMMITTER_EMAIL='&'; export GIT_COMMITTER_EMAIL/p\n\n g\n s/^COMMITTER [^<]* <[^>]*> \\(.*\\)$/\\1/\n s/'/'\\''/g\n s/.*/GIT_COMMITTER_DATE='&'; export GIT_COMMITTER_DATE/p\n\n q\n }\n\nSED STDIN BEGIN\ntree 969f563d319049bb6dabc12054d67671499a6f55\nparent c4734950e37c09ca7d3e3088f6f31d866dbb5077\nauthor Ilya Basin 1350401059 +0400\ncommitter Ilya Basin 1350405585 +0400\n\ntemp\n\nSED STDIN END\n\nSED OUTPUT BEGIN\n\nSED OUTPUT END\nSED EXIT CODE: 0\nSED END\nGIT_DIR=/home/tester/.ilya/builds/makepkg.rap_0.1-1_sparc.XXXXXX/src/rap/.git\nGIT_INDEX_FILE=/home/tester/.ilya/builds/makepkg.rap_0.1-1_sparc.XXXXXX/src/rap/.git-rewrite/t/../index\nGIT_WORK_TREE=.\nGIT_AUTHOR_NAME=\nGIT_COMMITTER_NAME=\nGIT_COMMIT=acd1d2bb1984c96630d5070497590307151c4682\n\nSED BEGIN\nSED ARGUMENTS: -e 1,/^$/d\nSED STDIN BEGIN\ntree 969f563d319049bb6dabc12054d67671499a6f55\nparent c4734950e37c09ca7d3e3088f6f31d866dbb5077\nauthor Ilya Basin 1350401059 +0400\ncommitter Ilya Basin 1350405585 +0400\n\ntemp\n\nSED STDIN END\n\nSED OUTPUT BEGIN\ntemp\n\nSED OUTPUT END\nSED EXIT CODE: 0\nSED END\nfatal: empty ident not allowed\ncould not write rewritten commit\n-bash-3.00$\n"},{"id":"201417","messageId":"1665262317.20121017143623@gmail.com","threadId":"31845","inReplyTo":"1013956402.20121017125847@gmail.com","subject":"Re[3]: What can cause empty GIT_AUTHOR_NAME for 'git filter-branch --tree-filter' on Solaris?","fromName":"Ilya Basin","fromEmail":"basinilya@gmail.com","sentAt":"2012-10-17T10:36:23Z","receivedAt":"2012-10-17T10:36:23Z","isPatch":false,"sender":{"key":"basinilya@gmail.com","avatar":null},"body":"JS>> Most likely, your sed has problems with a sed script in function\nJS>> get_author_ident_from_commit. I tested it like this:\n\nJS>> $ sh -c '. $(git --exec-path)/git-sh-setup;\nJS>> get_author_ident_from_commit HEAD'\nJS>> GIT_AUTHOR_NAME='Johannes Sixt'\nJS>> GIT_AUTHOR_EMAIL='j6t@kdbg.org'\nJS>> GIT_AUTHOR_DATE='@1350025129 +0200'\n\nJS>> -- Hannes\n\nIB> Both systems have GNU sed 4.2.1 installed. I wrote a wrapper script wor sed.\nIB> It's output attached.\nIB> The difference is letter case in sed input data:\nIB> Solaris:\nIB> /^AUTHOR /\nIB> Windows:\nIB> /^author /\n\nThe culprit is bad $PATH :\nWhen git-filter-branch runs, for some reason two new entries precede\n/usr/bin in it:\n /tmp/777/.ilya-sparc/bin\n /home/tester/.ilya/opt/SNiFF-3.2.1/bin\n /export/home/testora/app/testora/product/11.2.0/client_32/bin\n +/usr/xpg6/bin\n +/usr/xpg4/bin\n /usr/bin\n /home/tester/apache-ant-1.7.1/bin\n /usr/jdk/instances/jdk1.5.0//bin\n\nAnd /usr/xpg6/bin/tr fails to make \"AUTHOR\" lowercase.\n"},{"id":"201451","messageId":"20121017220912.GA21742@sigill.intra.peff.net","threadId":"31845","inReplyTo":"1013956402.20121017125847@gmail.com","subject":"Re: What can cause empty GIT_AUTHOR_NAME for 'git filter-branch --tree-filter' on Solaris?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-17T22:09:12Z","receivedAt":"2012-10-17T22:09:12Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 17, 2012 at 12:58:47PM +0400, Ilya Basin wrote:\n\n> JS> Most likely, your sed has problems with a sed script in function\n> JS> get_author_ident_from_commit. I tested it like this:\n> \n> JS> $ sh -c '. $(git --exec-path)/git-sh-setup;\n> JS> get_author_ident_from_commit HEAD'\n> JS> GIT_AUTHOR_NAME='Johannes Sixt'\n> JS> GIT_AUTHOR_EMAIL='j6t@kdbg.org'\n> JS> GIT_AUTHOR_DATE='@1350025129 +0200'\n> \n> JS> -- Hannes\n> \n> Both systems have GNU sed 4.2.1 installed. I wrote a wrapper script wor sed.\n> It's output attached.\n> The difference is letter case in sed input data:\n> Solaris:\n> /^AUTHOR /\n> Windows:\n> /^author /\n\nAh, so it's tr that is the culprit. We've had problems with Solaris tr\nbefore, but usually around NULs or the use of brackets. But according to\n40a7ce6 (tr portability fixes, 2008-03-12), filter-branch is already\ndoing it the portable way.\n\nIf you apply this patch, does your filter-branch work?\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 178e453..58b1d69 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -68,8 +68,8 @@ set_ident () {\n # \"author\" or \"committer\n \n set_ident () {\n-\tlid=\"$(echo \"$1\" | tr \"[A-Z]\" \"[a-z]\")\"\n-\tuid=\"$(echo \"$1\" | tr \"[a-z]\" \"[A-Z]\")\"\n+\tlid=\"$(echo \"$1\" | tr ABCDEFGHIJKLMNOPQRSTUVWXYZ abcdefghijklmnopqrstuvwxyz)\"\n+\tuid=\"$(echo \"$1\" | tr abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ)\"\n \tpick_id_script='\n \t\t/^'$lid' /{\n \t\t\ts/'\\''/'\\''\\\\'\\'\\''/g\n\nThat seems like crazy overkill, but it at least will let us double-check\nthat the tr sequences are the problem.\n\n-Peff\n"},{"id":"201452","messageId":"20121017221322.GB21742@sigill.intra.peff.net","threadId":"31845","inReplyTo":"1665262317.20121017143623@gmail.com","subject":"Re: What can cause empty GIT_AUTHOR_NAME for 'git filter-branch --tree-filter' on Solaris?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-17T22:13:22Z","receivedAt":"2012-10-17T22:13:22Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 17, 2012 at 02:36:23PM +0400, Ilya Basin wrote:\n\n> The culprit is bad $PATH :\n> When git-filter-branch runs, for some reason two new entries precede\n> /usr/bin in it:\n> /tmp/777/.ilya-sparc/bin\n> /home/tester/.ilya/opt/SNiFF-3.2.1/bin\n> /export/home/testora/app/testora/product/11.2.0/client_32/bin\n> +/usr/xpg6/bin\n> +/usr/xpg4/bin\n> /usr/bin\n> /home/tester/apache-ant-1.7.1/bin\n> /usr/jdk/instances/jdk1.5.0//bin\n> \n> And /usr/xpg6/bin/tr fails to make \"AUTHOR\" lowercase.\n\nHmph. Those are controlled by SANE_TOOL_PATH at git's build time, with\nthe intent that the xpg tools are less terrible than the ones in\n/usr/bin on Solaris. But it sounds like that may not be the case. Yuck.\n\nI don't have a Solaris box handy. Is there a way to make sequences like\nA-Z work sanely with /usr/xpg6/bin/tr?\n\nDo you have any LANG or locale settings? Sometimes those can affect\nsequences. What does:\n\n echo AUTHOR | LANG=C LC_ALL=C /usr/xpg6/bin/tr '[A-Z]' '[a-z]'\n\ndo?\n\n-Peff\n"},{"id":"201466","messageId":"507F9437.2070501@viscovery.net","threadId":"31845","inReplyTo":"20121017220912.GA21742@sigill.intra.peff.net","subject":"Re: What can cause empty GIT_AUTHOR_NAME for 'git filter-branch --tree-filter' on Solaris?","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2012-10-18T05:31:35Z","receivedAt":"2012-10-18T05:31:35Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 10/18/2012 0:09, schrieb Jeff King:\n> -\tlid=\"$(echo \"$1\" | tr \"[A-Z]\" \"[a-z]\")\"\n> -\tuid=\"$(echo \"$1\" | tr \"[a-z]\" \"[A-Z]\")\"\n> +\tlid=\"$(echo \"$1\" | tr ABCDEFGHIJKLMNOPQRSTUVWXYZ abcdefghijklmnopqrstuvwxyz)\"\n> +\tuid=\"$(echo \"$1\" | tr abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ)\"\n> \n> That seems like crazy overkill, but it at least will let us double-check\n> that the tr sequences are the problem.\n\nRight. But we should really be doing something like this instead to save a\nfew subprocesses.\n\n-- Hannes\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 178e453..018e56e 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -68,8 +68,8 @@ eval \"$functions\"\n # \"author\" or \"committer\n\n set_ident () {\n-\tlid=\"$(echo \"$1\" | tr \"[A-Z]\" \"[a-z]\")\"\n-\tuid=\"$(echo \"$1\" | tr \"[a-z]\" \"[A-Z]\")\"\n+\tuid=$1\n+\tlid=$2\n \tpick_id_script='\n \t\t/^'$lid' /{\n \t\t\ts/'\\''/'\\''\\\\'\\'\\''/g\n@@ -320,9 +320,9 @@ while read commit parents; do\n \tgit cat-file commit \"$commit\" >../commit ||\n \t\tdie \"Cannot read commit $commit\"\n\n-\teval \"$(set_ident AUTHOR <../commit)\" ||\n+\teval \"$(set_ident AUTHOR author <../commit)\" ||\n \t\tdie \"setting author failed for commit $commit\"\n-\teval \"$(set_ident COMMITTER <../commit)\" ||\n+\teval \"$(set_ident COMMITTER committer <../commit)\" ||\n \t\tdie \"setting committer failed for commit $commit\"\n \teval \"$filter_env\" < /dev/null ||\n \t\tdie \"env filter failed: $filter_env\"\n"},{"id":"201467","messageId":"20121018053656.GA6308@sigill.intra.peff.net","threadId":"31845","inReplyTo":"507F9437.2070501@viscovery.net","subject":"Re: What can cause empty GIT_AUTHOR_NAME for 'git filter-branch --tree-filter' on Solaris?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-18T05:36:56Z","receivedAt":"2012-10-18T05:36:56Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 18, 2012 at 07:31:35AM +0200, Johannes Sixt wrote:\n\n> Right. But we should really be doing something like this instead to save a\n> few subprocesses.\n> [...]\n> -\teval \"$(set_ident AUTHOR <../commit)\" ||\n> +\teval \"$(set_ident AUTHOR author <../commit)\" ||\n\nI cringe a little at losing DRY-ness to avoid processes. But the\nrepetition is pretty straightforward and obvious, and I know that some\nplatforms are really hurt by extra processes (and this is being called\nfor every commit).\n\nSpeaking of repetition, this seems like almost the exact same parsing\nthat happens in git-sh-setup's get_author_ident_from_commit. Maybe it's\nworth merging them. I suspect you could also avoid another process\nby parsing out both author and committer information in the same sed\ninvocation.\n\n-Peff\n"},{"id":"201475","messageId":"7vk3uomi0c.fsf@alter.siamese.dyndns.org","threadId":"31845","inReplyTo":"20121018053656.GA6308@sigill.intra.peff.net","subject":"Re: What can cause empty GIT_AUTHOR_NAME for 'git filter-branch --tree-filter' on Solaris?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-10-18T06:06:11Z","receivedAt":"2012-10-18T06:06:11Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Thu, Oct 18, 2012 at 07:31:35AM +0200, Johannes Sixt wrote:\n>\n>> Right. But we should really be doing something like this instead to save a\n>> few subprocesses.\n>> [...]\n>> -\teval \"$(set_ident AUTHOR <../commit)\" ||\n>> +\teval \"$(set_ident AUTHOR author <../commit)\" ||\n>\n> I cringe a little at losing DRY-ness to avoid processes.\n\nWell, the header field token \"author\" and the middle word of the\nvariable GIT_AUTHOR_NAME _happen_ to be the same modulo case, but\nthey did not have to be, so you could argue the updated set_ident\nimplementation is more generally useful (you could even argue that\nwe should spell the first parameter out as \"GIT_AUTHOR_NAME\" and\n\"GIT_AUTHOR_EMAIL\", two separate parameters).\n\n> Speaking of repetition, this seems like almost the exact same parsing\n> that happens in git-sh-setup's get_author_ident_from_commit. Maybe it's\n> worth merging them. I suspect you could also avoid another process\n> by parsing out both author and committer information in the same sed\n> invocation.\n\nYes, yes and yes.\n"},{"id":"201476","messageId":"20121018060847.GB6308@sigill.intra.peff.net","threadId":"31845","inReplyTo":"7vk3uomi0c.fsf@alter.siamese.dyndns.org","subject":"Re: What can cause empty GIT_AUTHOR_NAME for 'git filter-branch --tree-filter' on Solaris?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-18T06:08:47Z","receivedAt":"2012-10-18T06:08:47Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 17, 2012 at 11:06:11PM -0700, Junio C Hamano wrote:\n\n> >> -\teval \"$(set_ident AUTHOR <../commit)\" ||\n> >> +\teval \"$(set_ident AUTHOR author <../commit)\" ||\n> >\n> > I cringe a little at losing DRY-ness to avoid processes.\n> \n> Well, the header field token \"author\" and the middle word of the\n> variable GIT_AUTHOR_NAME _happen_ to be the same modulo case, but\n> they did not have to be, so you could argue the updated set_ident\n> implementation is more generally useful (you could even argue that\n> we should spell the first parameter out as \"GIT_AUTHOR_NAME\" and\n> \"GIT_AUTHOR_EMAIL\", two separate parameters).\n\nTrue, though that is even more work for the caller (and *_DATE, too). We\ncould make it \"GIT_AUTHOR\", but I don't think there is much point in\nbeing that level of half-way general. The caller can always pick it out\nof the variables if they really want to do something tricky.\n\n> > Speaking of repetition, this seems like almost the exact same parsing\n> > that happens in git-sh-setup's get_author_ident_from_commit. Maybe it's\n> > worth merging them. I suspect you could also avoid another process\n> > by parsing out both author and committer information in the same sed\n> > invocation.\n> \n> Yes, yes and yes.\n\nWorking on it now. git-sh-setup works, but chasing an annoying bug in\nfilter-branch. I'm sure it's something silly and stupid.\n\n-Peff\n"},{"id":"201478","messageId":"20121018072207.GA1605@sigill.intra.peff.net","threadId":"31845","inReplyTo":"20121018060847.GB6308@sigill.intra.peff.net","subject":"[PATCH 0/2] clean up filter-branch ident parsing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-18T07:22:08Z","receivedAt":"2012-10-18T07:22:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 18, 2012 at 02:08:47AM -0400, Jeff King wrote:\n\n> Working on it now. git-sh-setup works, but chasing an annoying bug in\n> filter-branch. I'm sure it's something silly and stupid.\n\nUgh. I was being caught by crazy dash-versus-bash stuff.\n\nTry this:\n\n $ bash -c 'echo \"\\\\\\\\\"'\n \\\\\n $ dash -c 'echo \"\\\\\\\\\"'\n \\\n\nIt's the \"echo will automatically do backslash escaping\" magic we have\nso often enjoyed. I solved it with printf.\n\nPatches to follow.\n\nMy primary motivation was cleanup, but it also has a net reduction of 5\nfork+execs for each commit that filter-branch processes. This dropped\nthe run-time of \"git filter-branch HEAD -1000\" on my linux box from 62s\nto 47s. A real filter-branch would do more work in the filters, of\ncourse, but that translates to 7.5 minutes of time saved if you are\nfiltering all 30,000 commits of git.git.\n\n [1/2]: git-sh-setup: refactor ident-parsing functions\n [2/2]: filter-branch: use git-sh-setup's ident parsing functions\n\n-Peff\n"},{"id":"201479","messageId":"20121018072522.GA9999@sigill.intra.peff.net","threadId":"31845","inReplyTo":"20121018072207.GA1605@sigill.intra.peff.net","subject":"[PATCH 1/2] git-sh-setup: refactor ident-parsing functions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-18T07:25:22Z","receivedAt":"2012-10-18T07:25:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The only ident-parsing function we currently provide is\nget_author_ident_from_commit. This is not very\nflexible for two reasons:\n\n 1. It takes a commit as an argument, and can't read from\n commit headers saved on disk.\n\n 2. It will only parse authors, not committers.\n\nThis patch provides a more flexible interface which will\nparse multiple idents from a commit provide on stdin. We can\neasily use it as a building block for the current function\nto retain compatibility.\n\nSigned-off-by: Jeff King \n---\nSince we are counting processes in this series, I should note that this\nactually adds a subshell invocation for each call, since it went from:\n\n script='...'\n sed $script\n\nto:\n\n sed \"$(make_script)\"\n\nFor filter-branch, which is really the only high-performance caller we\nhave, this is negated by the fact that it will do author and committer\nat the same time, saving us an extra subshell (in addition to an extra\nsed invocation).\n\n git-sh-setup.sh | 62 +++++++++++++++++++++++++++++++++++++++------------------\n 1 file changed, 43 insertions(+), 19 deletions(-)\n\ndiff --git a/git-sh-setup.sh b/git-sh-setup.sh\nindex ee0e0bc..22f0aed 100644\n--- a/git-sh-setup.sh\n+++ b/git-sh-setup.sh\n@@ -191,28 +191,52 @@ get_author_ident_from_commit () {\n \tfi\n }\n \n+# Generate a sed script to parse identities from a commit.\n+#\n+# Reads the commit from stdin, which should be in raw format (e.g., from\n+# cat-file or \"--pretty=raw\").\n+#\n+# The first argument specifies the ident line to parse (e.g., \"author\"), and\n+# the second specifies the environment variable to put it in (e.g., \"AUTHOR\"\n+# for \"GIT_AUTHOR_*\"). Multiple pairs can be given to parse author and\n+# committer.\n+pick_ident_script () {\n+\twhile test $# -gt 0\n+\tdo\n+\t\tlid=$1; shift\n+\t\tuid=$1; shift\n+\t\tprintf '%s' \"\n+\t\t/^$lid /{\n+\t\t\ts/'/'\\\\\\\\''/g\n+\t\t\th\n+\t\t\ts/^$lid \"'\\([^<]*\\) <[^>]*> .*$/\\1/'\"\n+\t\t\ts/.*/GIT_${uid}_NAME='&'/p\n+\n+\t\t\tg\n+\t\t\ts/^$lid \"'[^<]* <\\([^>]*\\)> .*$/\\1/'\"\n+\t\t\ts/.*/GIT_${uid}_EMAIL='&'/p\n+\n+\t\t\tg\n+\t\t\ts/^$lid \"'[^<]* <[^>]*> \\(.*\\)$/@\\1/'\"\n+\t\t\ts/.*/GIT_${uid}_DATE='&'/p\n+\t\t}\n+\t\t\"\n+\tdone\n+\techo '/^$/q'\n+}\n+\n+# Create a pick-script as above and feed it to sed. Stdout is suitable for\n+# feeding to eval.\n+parse_ident_from_commit () {\n+\tLANG=C LC_ALL=C sed -ne \"$(pick_ident_script \"$@\")\"\n+}\n+\n+# Parse the author from a commit given as an argument. Stdout is suitable for\n+# feeding to eval to set the usual GIT_* ident variables.\n get_author_ident_from_commit () {\n-\tpick_author_script='\n-\t/^author /{\n-\t\ts/'\\''/'\\''\\\\'\\'\\''/g\n-\t\th\n-\t\ts/^author \\([^<]*\\) <[^>]*> .*$/\\1/\n-\t\ts/.*/GIT_AUTHOR_NAME='\\''&'\\''/p\n-\n-\t\tg\n-\t\ts/^author [^<]* <\\([^>]*\\)> .*$/\\1/\n-\t\ts/.*/GIT_AUTHOR_EMAIL='\\''&'\\''/p\n-\n-\t\tg\n-\t\ts/^author [^<]* <[^>]*> \\(.*\\)$/@\\1/\n-\t\ts/.*/GIT_AUTHOR_DATE='\\''&'\\''/p\n-\n-\t\tq\n-\t}\n-\t'\n \tencoding=$(git config i18n.commitencoding || echo UTF-8)\n \tgit show -s --pretty=raw --encoding=\"$encoding\" \"$1\" -- |\n-\tLANG=C LC_ALL=C sed -ne \"$pick_author_script\"\n+\tparse_ident_from_commit author AUTHOR\n }\n \n # Clear repo-local GIT_* environment variables. Useful when switching to\n-- \n1.8.0.rc3.3.gba630e1\n"},{"id":"201480","messageId":"20121018072535.GB9999@sigill.intra.peff.net","threadId":"31845","inReplyTo":"20121018072207.GA1605@sigill.intra.peff.net","subject":"[PATCH 2/2] filter-branch: use git-sh-setup's ident parsing functions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-18T07:25:35Z","receivedAt":"2012-10-18T07:25:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This saves us some code, but it also reduces the number of\nprocesses we start for each filtered commit. Since we can\nparse both author and committer in the same sed invocation,\nwe save one process. And since the new interface avoids tr,\nwe save 4 processes.\n\nIt also avoids using \"tr\", which has had some odd\nportability problems reported with from Solaris's xpg6\nversion.\n\nSigned-off-by: Jeff King \n---\n git-filter-branch.sh | 42 +++++++++---------------------------------\n 1 file changed, 9 insertions(+), 33 deletions(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 178e453..69406ae 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -64,37 +64,15 @@ set_ident () {\n \n eval \"$functions\"\n \n-# When piped a commit, output a script to set the ident of either\n-# \"author\" or \"committer\n+# Ensure non-empty id name.\n+fallback_name() {\n+\techo \"case \\\"\\$GIT_$1_NAME\\\" in \\\"\\\") GIT_$1_NAME=\\\"\\${GIT_$1_EMAIL%%@*}\\\" && export GIT_$1_NAME;; esac\"\n+}\n \n set_ident () {\n-\tlid=\"$(echo \"$1\" | tr \"[A-Z]\" \"[a-z]\")\"\n-\tuid=\"$(echo \"$1\" | tr \"[a-z]\" \"[A-Z]\")\"\n-\tpick_id_script='\n-\t\t/^'$lid' /{\n-\t\t\ts/'\\''/'\\''\\\\'\\'\\''/g\n-\t\t\th\n-\t\t\ts/^'$lid' \\([^<]*\\) <[^>]*> .*$/\\1/\n-\t\t\ts/'\\''/'\\''\\'\\'\\''/g\n-\t\t\ts/.*/GIT_'$uid'_NAME='\\''&'\\''; export GIT_'$uid'_NAME/p\n-\n-\t\t\tg\n-\t\t\ts/^'$lid' [^<]* <\\([^>]*\\)> .*$/\\1/\n-\t\t\ts/'\\''/'\\''\\'\\'\\''/g\n-\t\t\ts/.*/GIT_'$uid'_EMAIL='\\''&'\\''; export GIT_'$uid'_EMAIL/p\n-\n-\t\t\tg\n-\t\t\ts/^'$lid' [^<]* <[^>]*> \\(.*\\)$/@\\1/\n-\t\t\ts/'\\''/'\\''\\'\\'\\''/g\n-\t\t\ts/.*/GIT_'$uid'_DATE='\\''&'\\''; export GIT_'$uid'_DATE/p\n-\n-\t\t\tq\n-\t\t}\n-\t'\n-\n-\tLANG=C LC_ALL=C sed -ne \"$pick_id_script\"\n-\t# Ensure non-empty id name.\n-\techo \"case \\\"\\$GIT_${uid}_NAME\\\" in \\\"\\\") GIT_${uid}_NAME=\\\"\\${GIT_${uid}_EMAIL%%@*}\\\" && export GIT_${uid}_NAME;; esac\"\n+\tparse_ident_from_commit author AUTHOR committer COMMITTER\n+\tfallback_name AUTHOR\n+\tfallback_name COMMITTER\n }\n \n USAGE=\"[--env-filter ] [--tree-filter ]\n@@ -320,10 +298,8 @@ while read commit parents; do\n \tgit cat-file commit \"$commit\" >../commit ||\n \t\tdie \"Cannot read commit $commit\"\n \n-\teval \"$(set_ident AUTHOR <../commit)\" ||\n-\t\tdie \"setting author failed for commit $commit\"\n-\teval \"$(set_ident COMMITTER <../commit)\" ||\n-\t\tdie \"setting committer failed for commit $commit\"\n+\teval \"$(set_ident <../commit)\" ||\n+\t\tdie \"setting author/committer failed for commit $commit\"\n \teval \"$filter_env\" < /dev/null ||\n \t\tdie \"env filter failed: $filter_env\"\n \n-- \n1.8.0.rc3.3.gba630e1\n"},{"id":"201482","messageId":"507FB470.2060404@viscovery.net","threadId":"31845","inReplyTo":"20121018072535.GB9999@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] filter-branch: use git-sh-setup's ident parsing functions","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2012-10-18T07:49:04Z","receivedAt":"2012-10-18T07:49:04Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 10/18/2012 9:25, schrieb Jeff King:\n> -# When piped a commit, output a script to set the ident of either\n> -# \"author\" or \"committer\n> +# Ensure non-empty id name.\n> +fallback_name() {\n> +\techo \"case \\\"\\$GIT_$1_NAME\\\" in \\\"\\\") GIT_$1_NAME=\\\"\\${GIT_$1_EMAIL%%@*}\\\" && export GIT_$1_NAME;; esac\"\n> +}\n> \n> set_ident () {\n> -\tlid=\"$(echo \"$1\" | tr \"[A-Z]\" \"[a-z]\")\"\n> -\tuid=\"$(echo \"$1\" | tr \"[a-z]\" \"[A-Z]\")\"\n> -\tpick_id_script='\n> -\t\t/^'$lid' /{\n> -\t\t\ts/'\\''/'\\''\\\\'\\'\\''/g\n> -\t\t\th\n> -\t\t\ts/^'$lid' \\([^<]*\\) <[^>]*> .*$/\\1/\n> -\t\t\ts/'\\''/'\\''\\'\\'\\''/g\n> -\t\t\ts/.*/GIT_'$uid'_NAME='\\''&'\\''; export GIT_'$uid'_NAME/p\n> -\n> -\t\t\tg\n> -\t\t\ts/^'$lid' [^<]* <\\([^>]*\\)> .*$/\\1/\n> -\t\t\ts/'\\''/'\\''\\'\\'\\''/g\n> -\t\t\ts/.*/GIT_'$uid'_EMAIL='\\''&'\\''; export GIT_'$uid'_EMAIL/p\n> -\n> -\t\t\tg\n> -\t\t\ts/^'$lid' [^<]* <[^>]*> \\(.*\\)$/@\\1/\n> -\t\t\ts/'\\''/'\\''\\'\\'\\''/g\n> -\t\t\ts/.*/GIT_'$uid'_DATE='\\''&'\\''; export GIT_'$uid'_DATE/p\n> -\n> -\t\t\tq\n> -\t\t}\n> -\t'\n> -\n> -\tLANG=C LC_ALL=C sed -ne \"$pick_id_script\"\n> -\t# Ensure non-empty id name.\n> -\techo \"case \\\"\\$GIT_${uid}_NAME\\\" in \\\"\\\") GIT_${uid}_NAME=\\\"\\${GIT_${uid}_EMAIL%%@*}\\\" && export GIT_${uid}_NAME;; esac\"\n> +\tparse_ident_from_commit author AUTHOR committer COMMITTER\n> +\tfallback_name AUTHOR\n> +\tfallback_name COMMITTER\n> }\n\nDidn't you lose the export GIT_$uid_{NAME,EMAIL,DATE} parts somewhere on\nthe way?\n\nThanks for working on this!\n\n-- Hannes\n"},{"id":"201484","messageId":"20121018075429.GD9999@sigill.intra.peff.net","threadId":"31845","inReplyTo":"507FB470.2060404@viscovery.net","subject":"Re: [PATCH 2/2] filter-branch: use git-sh-setup's ident parsing functions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-18T07:54:29Z","receivedAt":"2012-10-18T07:54:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 18, 2012 at 09:49:04AM +0200, Johannes Sixt wrote:\n\n> > -\t\t\ts/.*/GIT_'$uid'_NAME='\\''&'\\''; export GIT_'$uid'_NAME/p\n> \n> Didn't you lose the export GIT_$uid_{NAME,EMAIL,DATE} parts somewhere on\n> the way?\n\nYikes, you're right. I didn't even notice, as the test suite still\npasses. I can see how the env filter would still be able to see the\nvariables, but the commit-tree call wouldn't. I guess it happens to work\nbecause we do not test alternate idents in our filter branch tests (IOW,\nwe are silently rewriting each commit during the filter-branch, but it\nhappens to have the same identities).\n\nI'll investigate.\n\n-Peff\n"},{"id":"201499","messageId":"20121018102217.GA17332@sigill.intra.peff.net","threadId":"31845","inReplyTo":"20121018075429.GD9999@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] filter-branch: use git-sh-setup's ident parsing functions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-18T10:22:17Z","receivedAt":"2012-10-18T10:22:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 18, 2012 at 03:54:29AM -0400, Jeff King wrote:\n\n> On Thu, Oct 18, 2012 at 09:49:04AM +0200, Johannes Sixt wrote:\n> \n> > > -\t\t\ts/.*/GIT_'$uid'_NAME='\\''&'\\''; export GIT_'$uid'_NAME/p\n> > \n> > Didn't you lose the export GIT_$uid_{NAME,EMAIL,DATE} parts somewhere on\n> > the way?\n> \n> Yikes, you're right. I didn't even notice, as the test suite still\n> passes. I can see how the env filter would still be able to see the\n> variables, but the commit-tree call wouldn't. I guess it happens to work\n> because we do not test alternate idents in our filter branch tests (IOW,\n> we are silently rewriting each commit during the filter-branch, but it\n> happens to have the same identities).\n\nHrm. We _do_ test this in t7003. Weirder, if I instrument filter-branch\nlike this:\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 69406ae..1b504ce 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -298,8 +298,13 @@ while read commit parents; do\n \tgit cat-file commit \"$commit\" >../commit ||\n \t\tdie \"Cannot read commit $commit\"\n \n+\techo >&2 pre: $GIT_AUTHOR_NAME\n+\tsh -c 'echo >&2 pre, subshell: $GIT_AUTHOR_NAME'\n+\n \teval \"$(set_ident <../commit)\" ||\n \t\tdie \"setting author/committer failed for commit $commit\"\n+\techo >&2 post: $GIT_AUTHOR_NAME\n+\tsh -c 'echo >&2 post, subshell: $GIT_AUTHOR_NAME'\n \teval \"$filter_env\" < /dev/null ||\n \t\tdie \"env filter failed: $filter_env\"\n \ndiff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\nindex 4d13e10..ce57fc5 100755\n--- a/t/t7003-filter-branch.sh\n+++ b/t/t7003-filter-branch.sh\n@@ -174,6 +174,8 @@ test_expect_success 'author information is preserved' '\n \ttest 1 = $(git rev-list --author=\"B V Uips\" preserved-author | wc -l)\n '\n \n+test_done\n+\n test_expect_success \"remove a certain author's commits\" '\n \techo i > i &&\n \ttest_tick &&\n\nand run t7003, it shows that the variable is properly exported to the\nsub-process! But I can't seem to figure out why. Confused...\n\n-Peff\n"},{"id":"201500","messageId":"20121018102627.GA17543@sigill.intra.peff.net","threadId":"31845","inReplyTo":"20121018102217.GA17332@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] filter-branch: use git-sh-setup's ident parsing functions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-18T10:26:27Z","receivedAt":"2012-10-18T10:26:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 18, 2012 at 06:22:17AM -0400, Jeff King wrote:\n\n> Hrm. We _do_ test this in t7003. Weirder, if I instrument filter-branch\n> like this:\n> [...]\n> and run t7003, it shows that the variable is properly exported to the\n> sub-process! But I can't seem to figure out why. Confused...\n\nOh, I see. The variables are already exported by test-lib.sh. You can\nsee the breakage with:\n\ndiff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\nindex 4d13e10..1e7a209 100755\n--- a/t/t7003-filter-branch.sh\n+++ b/t/t7003-filter-branch.sh\n@@ -167,10 +167,11 @@ test_expect_success 'author information is preserved' '\n \ttest_tick &&\n \tGIT_AUTHOR_NAME=\"B V Uips\" git commit -m bvuips &&\n \tgit branch preserved-author &&\n-\tgit filter-branch -f --msg-filter \"cat; \\\n+\t(sane_unset GIT_AUTHOR_NAME &&\n+\t git filter-branch -f --msg-filter \"cat; \\\n \t\t\ttest \\$GIT_COMMIT != $(git rev-parse master) || \\\n \t\t\techo Hallo\" \\\n-\t\tpreserved-author &&\n+\t\tpreserved-author) &&\n \ttest 1 = $(git rev-list --author=\"B V Uips\" preserved-author | wc -l)\n '\n \n\n-Peff\n"},{"id":"201501","messageId":"20121018103301.GA28453@sigill.intra.peff.net","threadId":"31845","inReplyTo":"20121018102627.GA17543@sigill.intra.peff.net","subject":"[PATCHv2 2/2] filter-branch: use git-sh-setup's ident parsing functions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-18T10:33:02Z","receivedAt":"2012-10-18T10:33:02Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This saves us some code, but it also reduces the number of\nprocesses we start for each filtered commit. Since we can\nparse both author and committer in the same sed invocation,\nwe save one process. And since the new interface avoids tr,\nwe save 4 processes.\n\nIt also avoids using \"tr\", which has had some odd\nportability problems reported with from Solaris's xpg6\nversion.\n\nWe also tweak one of the tests in t7003 to double-check that\nwe are properly exporting the variables (because test-lib.sh\nexports GIT_AUTHOR_NAME, it will be automatically exported\nin subprograms. We override this to make sure that\nfilter-branch handles it properly itself).\n\nSigned-off-by: Jeff King \n---\nThis fixes the missing exports from v1. There's no changes needed to\npatch 1.\n\n git-filter-branch.sh | 46 +++++++++++++---------------------------------\n t/t7003-filter-branch.sh | 5 +++--\n 2 files changed, 16 insertions(+), 35 deletions(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 178e453..5314249 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -64,37 +64,19 @@ set_ident () {\n \n eval \"$functions\"\n \n-# When piped a commit, output a script to set the ident of either\n-# \"author\" or \"committer\n+finish_ident() {\n+\t# Ensure non-empty id name.\n+\techo \"case \\\"\\$GIT_$1_NAME\\\" in \\\"\\\") GIT_$1_NAME=\\\"\\${GIT_$1_EMAIL%%@*}\\\" && export GIT_$1_NAME;; esac\"\n+\t# And make sure everything is exported.\n+\techo \"export GIT_$1_NAME\"\n+\techo \"export GIT_$1_EMAIL\"\n+\techo \"export GIT_$1_DATE\"\n+}\n \n set_ident () {\n-\tlid=\"$(echo \"$1\" | tr \"[A-Z]\" \"[a-z]\")\"\n-\tuid=\"$(echo \"$1\" | tr \"[a-z]\" \"[A-Z]\")\"\n-\tpick_id_script='\n-\t\t/^'$lid' /{\n-\t\t\ts/'\\''/'\\''\\\\'\\'\\''/g\n-\t\t\th\n-\t\t\ts/^'$lid' \\([^<]*\\) <[^>]*> .*$/\\1/\n-\t\t\ts/'\\''/'\\''\\'\\'\\''/g\n-\t\t\ts/.*/GIT_'$uid'_NAME='\\''&'\\''; export GIT_'$uid'_NAME/p\n-\n-\t\t\tg\n-\t\t\ts/^'$lid' [^<]* <\\([^>]*\\)> .*$/\\1/\n-\t\t\ts/'\\''/'\\''\\'\\'\\''/g\n-\t\t\ts/.*/GIT_'$uid'_EMAIL='\\''&'\\''; export GIT_'$uid'_EMAIL/p\n-\n-\t\t\tg\n-\t\t\ts/^'$lid' [^<]* <[^>]*> \\(.*\\)$/@\\1/\n-\t\t\ts/'\\''/'\\''\\'\\'\\''/g\n-\t\t\ts/.*/GIT_'$uid'_DATE='\\''&'\\''; export GIT_'$uid'_DATE/p\n-\n-\t\t\tq\n-\t\t}\n-\t'\n-\n-\tLANG=C LC_ALL=C sed -ne \"$pick_id_script\"\n-\t# Ensure non-empty id name.\n-\techo \"case \\\"\\$GIT_${uid}_NAME\\\" in \\\"\\\") GIT_${uid}_NAME=\\\"\\${GIT_${uid}_EMAIL%%@*}\\\" && export GIT_${uid}_NAME;; esac\"\n+\tparse_ident_from_commit author AUTHOR committer COMMITTER\n+\tfinish_ident AUTHOR\n+\tfinish_ident COMMITTER\n }\n \n USAGE=\"[--env-filter ] [--tree-filter ]\n@@ -320,10 +302,8 @@ while read commit parents; do\n \tgit cat-file commit \"$commit\" >../commit ||\n \t\tdie \"Cannot read commit $commit\"\n \n-\teval \"$(set_ident AUTHOR <../commit)\" ||\n-\t\tdie \"setting author failed for commit $commit\"\n-\teval \"$(set_ident COMMITTER <../commit)\" ||\n-\t\tdie \"setting committer failed for commit $commit\"\n+\teval \"$(set_ident <../commit)\" ||\n+\t\tdie \"setting author/committer failed for commit $commit\"\n \teval \"$filter_env\" < /dev/null ||\n \t\tdie \"env filter failed: $filter_env\"\n \ndiff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\nindex 4d13e10..1e7a209 100755\n--- a/t/t7003-filter-branch.sh\n+++ b/t/t7003-filter-branch.sh\n@@ -167,10 +167,11 @@ test_expect_success 'author information is preserved' '\n \ttest_tick &&\n \tGIT_AUTHOR_NAME=\"B V Uips\" git commit -m bvuips &&\n \tgit branch preserved-author &&\n-\tgit filter-branch -f --msg-filter \"cat; \\\n+\t(sane_unset GIT_AUTHOR_NAME &&\n+\t git filter-branch -f --msg-filter \"cat; \\\n \t\t\ttest \\$GIT_COMMIT != $(git rev-parse master) || \\\n \t\t\techo Hallo\" \\\n-\t\tpreserved-author &&\n+\t\tpreserved-author) &&\n \ttest 1 = $(git rev-list --author=\"B V Uips\" preserved-author | wc -l)\n '\n \n-- \n1.8.0.rc3.3.gba630e1\n"},{"id":"202971","messageId":"7vpq3ik97i.fsf@alter.siamese.dyndns.org","threadId":"31845","inReplyTo":"20121018072522.GA9999@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] git-sh-setup: refactor ident-parsing functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-12T17:44:01Z","receivedAt":"2012-11-12T17:44:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> The only ident-parsing function we currently provide is\n> get_author_ident_from_commit. This is not very\n> flexible for two reasons:\n>\n> 1. It takes a commit as an argument, and can't read from\n> commit headers saved on disk.\n>\n> 2. It will only parse authors, not committers.\n>\n> This patch provides a more flexible interface which will\n> parse multiple idents from a commit provide on stdin. We can\n> easily use it as a building block for the current function\n> to retain compatibility.\n>\n> Signed-off-by: Jeff King \n> ---\n> Since we are counting processes in this series, I should note that this\n> actually adds a subshell invocation for each call, since it went from:\n>\n> script='...'\n> sed $script\n>\n> to:\n>\n> sed \"$(make_script)\"\n>\n> For filter-branch, which is really the only high-performance caller we\n> have, this is negated by the fact that it will do author and committer\n> at the same time, saving us an extra subshell (in addition to an extra\n> sed invocation).\n\nGiven that pick-ident-script is a const function, a caller that\nrepeatedly call is could call it once and use it in a variable, no?\n\n>\n> git-sh-setup.sh | 62 +++++++++++++++++++++++++++++++++++++++------------------\n> 1 file changed, 43 insertions(+), 19 deletions(-)\n>\n> diff --git a/git-sh-setup.sh b/git-sh-setup.sh\n> index ee0e0bc..22f0aed 100644\n> --- a/git-sh-setup.sh\n> +++ b/git-sh-setup.sh\n> @@ -191,28 +191,52 @@ get_author_ident_from_commit () {\n> \tfi\n> }\n> \n> +# Generate a sed script to parse identities from a commit.\n> +#\n> +# Reads the commit from stdin, which should be in raw format (e.g., from\n> +# cat-file or \"--pretty=raw\").\n> +#\n> +# The first argument specifies the ident line to parse (e.g., \"author\"), and\n> +# the second specifies the environment variable to put it in (e.g., \"AUTHOR\"\n> +# for \"GIT_AUTHOR_*\"). Multiple pairs can be given to parse author and\n> +# committer.\n> +pick_ident_script () {\n> +\twhile test $# -gt 0\n> +\tdo\n> +\t\tlid=$1; shift\n> +\t\tuid=$1; shift\n> +\t\tprintf '%s' \"\n> +\t\t/^$lid /{\n> +\t\t\ts/'/'\\\\\\\\''/g\n> +\t\t\th\n> +\t\t\ts/^$lid \"'\\([^<]*\\) <[^>]*> .*$/\\1/'\"\n> +\t\t\ts/.*/GIT_${uid}_NAME='&'/p\n> +\n> +\t\t\tg\n> +\t\t\ts/^$lid \"'[^<]* <\\([^>]*\\)> .*$/\\1/'\"\n> +\t\t\ts/.*/GIT_${uid}_EMAIL='&'/p\n> +\n> +\t\t\tg\n> +\t\t\ts/^$lid \"'[^<]* <[^>]*> \\(.*\\)$/@\\1/'\"\n> +\t\t\ts/.*/GIT_${uid}_DATE='&'/p\n> +\t\t}\n> +\t\t\"\n> +\tdone\n> +\techo '/^$/q'\n> +}\n> +\n> +# Create a pick-script as above and feed it to sed. Stdout is suitable for\n> +# feeding to eval.\n> +parse_ident_from_commit () {\n> +\tLANG=C LC_ALL=C sed -ne \"$(pick_ident_script \"$@\")\"\n> +}\n> +\n> +# Parse the author from a commit given as an argument. Stdout is suitable for\n> +# feeding to eval to set the usual GIT_* ident variables.\n> get_author_ident_from_commit () {\n> -\tpick_author_script='\n> -\t/^author /{\n> -\t\ts/'\\''/'\\''\\\\'\\'\\''/g\n> -\t\th\n> -\t\ts/^author \\([^<]*\\) <[^>]*> .*$/\\1/\n> -\t\ts/.*/GIT_AUTHOR_NAME='\\''&'\\''/p\n> -\n> -\t\tg\n> -\t\ts/^author [^<]* <\\([^>]*\\)> .*$/\\1/\n> -\t\ts/.*/GIT_AUTHOR_EMAIL='\\''&'\\''/p\n> -\n> -\t\tg\n> -\t\ts/^author [^<]* <[^>]*> \\(.*\\)$/@\\1/\n> -\t\ts/.*/GIT_AUTHOR_DATE='\\''&'\\''/p\n> -\n> -\t\tq\n> -\t}\n> -\t'\n> \tencoding=$(git config i18n.commitencoding || echo UTF-8)\n> \tgit show -s --pretty=raw --encoding=\"$encoding\" \"$1\" -- |\n> -\tLANG=C LC_ALL=C sed -ne \"$pick_author_script\"\n> +\tparse_ident_from_commit author AUTHOR\n> }\n> \n> # Clear repo-local GIT_* environment variables. Useful when switching to\n"},{"id":"202986","messageId":"20121112194434.GB4623@sigill.intra.peff.net","threadId":"31845","inReplyTo":"7vpq3ik97i.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] git-sh-setup: refactor ident-parsing functions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-11-12T19:44:34Z","receivedAt":"2012-11-12T19:44:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 12, 2012 at 09:44:01AM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > The only ident-parsing function we currently provide is\n> > get_author_ident_from_commit. This is not very\n> > flexible for two reasons:\n> >\n> > 1. It takes a commit as an argument, and can't read from\n> > commit headers saved on disk.\n> >\n> > 2. It will only parse authors, not committers.\n> >\n> > This patch provides a more flexible interface which will\n> > parse multiple idents from a commit provide on stdin. We can\n> > easily use it as a building block for the current function\n> > to retain compatibility.\n> >\n> > Signed-off-by: Jeff King \n> > ---\n> > Since we are counting processes in this series, I should note that this\n> > actually adds a subshell invocation for each call, since it went from:\n> >\n> > script='...'\n> > sed $script\n> >\n> > to:\n> >\n> > sed \"$(make_script)\"\n> >\n> > For filter-branch, which is really the only high-performance caller we\n> > have, this is negated by the fact that it will do author and committer\n> > at the same time, saving us an extra subshell (in addition to an extra\n> > sed invocation).\n> \n> Given that pick-ident-script is a const function, a caller that\n> repeatedly call is could call it once and use it in a variable, no?\n\nThe problem is that it is a helper called from parse_ident_from_commit.\nAnd that function just passes along its arguments, so it does not know\nthat it is being called repeatedly with the same arguments. So you'd\nhave to either change the interface or memoize internally.\n\nI don't think memoization is a good option for two reasons:\n\n 1. Storing the arguments to compare to later is complex. You don't\n want to just store \"$*\" from the last run and see if we got the\n same arguments. You'd have to quote your delimiter (e.g., you would\n not want to confuse (\"foo\", \"bar\") with (\"foo bar\"). Though in this\n instance, we know that our args do not have spaces, so we could get\n away with that.\n\n 2. If you are in a subshell or even a while loop, your memoized\n variable will not be retained.\n\nSo unless somebody has some clever scheme for memoizing shell functions\nwithout any process overhead, it is probably not worth it.\n\nChanging the interface for get_author_ident_from_commit would be a pain,\nbut if we just wanted to help filter-branch, we could do something like\nthis:\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 5314249..7a693ba 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -74,7 +74,7 @@ finish_ident() {\n }\n \n set_ident () {\n-\tparse_ident_from_commit author AUTHOR committer COMMITTER\n+\tparse_ident_from_commit_via_script \"$ident_script\"\n \tfinish_ident AUTHOR\n \tfinish_ident COMMITTER\n }\n@@ -93,6 +93,7 @@ if [ \"$(is_bare_repository)\" = false ]; then\n \trequire_clean_work_tree 'rewrite branches'\n fi\n \n+ident_script=$(pick_ident_script author AUTHOR committer COMMITTER)\n tempdir=.git-rewrite\n filter_env=\n filter_tree=\ndiff --git a/git-sh-setup.sh b/git-sh-setup.sh\nindex 22f0aed..1e20e17 100644\n--- a/git-sh-setup.sh\n+++ b/git-sh-setup.sh\n@@ -225,10 +225,17 @@ pick_ident_script () {\n \techo '/^$/q'\n }\n \n+# Feed a pick_ident_script return value to sed. Use this instead of\n+# parse_ident_from_commit below if you are going to be parsing commits in a\n+# tight loop and want to save a process.\n+parse_ident_from_commit_via_script() {\n+\tLANG=C LC_ALL=C sed -ne \"$1\"\n+}\n+\n # Create a pick-script as above and feed it to sed. Stdout is suitable for\n # feeding to eval.\n parse_ident_from_commit () {\n-\tLANG=C LC_ALL=C sed -ne \"$(pick_ident_script \"$@\")\"\n+\tparse_ident_from_commit_via_script \"$(pick_ident_script \"$@\")\"\n }\n \n # Parse the author from a commit given as an argument. Stdout is suitable for\n"},{"id":"202989","messageId":"7vzk2mh9dm.fsf@alter.siamese.dyndns.org","threadId":"31845","inReplyTo":"20121112194434.GB4623@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] git-sh-setup: refactor ident-parsing functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-12T20:08:37Z","receivedAt":"2012-11-12T20:08:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Changing the interface for get_author_ident_from_commit would be a pain,\n> but if we just wanted to help filter-branch, we could do something like\n> this:\n\nYes, that is the direction I was alluding to.\n\nCallers of get_author_ident_from_commit can also do the same and\navoid rebuilding the same $pick_author_script over and over again,\nor get_author_ident_from_commit can do so for its callers.\n\n> diff --git a/git-filter-branch.sh b/git-filter-branch.sh\n> index 5314249..7a693ba 100755\n> --- a/git-filter-branch.sh\n> +++ b/git-filter-branch.sh\n> @@ -74,7 +74,7 @@ finish_ident() {\n> }\n> \n> set_ident () {\n> -\tparse_ident_from_commit author AUTHOR committer COMMITTER\n> +\tparse_ident_from_commit_via_script \"$ident_script\"\n> \tfinish_ident AUTHOR\n> \tfinish_ident COMMITTER\n> }\n> @@ -93,6 +93,7 @@ if [ \"$(is_bare_repository)\" = false ]; then\n> \trequire_clean_work_tree 'rewrite branches'\n> fi\n> \n> +ident_script=$(pick_ident_script author AUTHOR committer COMMITTER)\n> tempdir=.git-rewrite\n> filter_env=\n> filter_tree=\n> diff --git a/git-sh-setup.sh b/git-sh-setup.sh\n> index 22f0aed..1e20e17 100644\n> --- a/git-sh-setup.sh\n> +++ b/git-sh-setup.sh\n> @@ -225,10 +225,17 @@ pick_ident_script () {\n> \techo '/^$/q'\n> }\n> \n> +# Feed a pick_ident_script return value to sed. Use this instead of\n> +# parse_ident_from_commit below if you are going to be parsing commits in a\n> +# tight loop and want to save a process.\n> +parse_ident_from_commit_via_script() {\n> +\tLANG=C LC_ALL=C sed -ne \"$1\"\n> +}\n> +\n> # Create a pick-script as above and feed it to sed. Stdout is suitable for\n> # feeding to eval.\n> parse_ident_from_commit () {\n> -\tLANG=C LC_ALL=C sed -ne \"$(pick_ident_script \"$@\")\"\n> +\tparse_ident_from_commit_via_script \"$(pick_ident_script \"$@\")\"\n> }\n> \n> # Parse the author from a commit given as an argument. Stdout is suitable for\n"},{"id":"202990","messageId":"20121112201235.GA7210@sigill.intra.peff.net","threadId":"31845","inReplyTo":"7vzk2mh9dm.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] git-sh-setup: refactor ident-parsing functions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-11-12T20:12:35Z","receivedAt":"2012-11-12T20:12:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 12, 2012 at 12:08:37PM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > Changing the interface for get_author_ident_from_commit would be a pain,\n> > but if we just wanted to help filter-branch, we could do something like\n> > this:\n> \n> Yes, that is the direction I was alluding to.\n> \n> Callers of get_author_ident_from_commit can also do the same and\n> avoid rebuilding the same $pick_author_script over and over again,\n> or get_author_ident_from_commit can do so for its callers.\n\nI don't think get_author_ident_from_commit can do so on demand due to\nthe subshell issue I mentioned. So you'd have to generate the pick\nscript on inclusion of git-sh-setup, whether the includer wants to call\nthe function or not.\n\nI wonder if we should simply generate these at build time and store them\nin the script.\n\nI'm also not sure that saving one process invocation is worth spending a\nlot of thought cycles on. Maybe somebody on Windows could run a\nfilter-branch with the patch I just sent and measure whether it even\nmakes a difference.\n\n-Peff\n"},{"id":"202995","messageId":"7vr4nyh89x.fsf@alter.siamese.dyndns.org","threadId":"31845","inReplyTo":"20121112201235.GA7210@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] git-sh-setup: refactor ident-parsing functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-12T20:32:26Z","receivedAt":"2012-11-12T20:32:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Mon, Nov 12, 2012 at 12:08:37PM -0800, Junio C Hamano wrote:\n>\n>> Jeff King writes:\n>> \n>> > Changing the interface for get_author_ident_from_commit would be a pain,\n>> > but if we just wanted to help filter-branch, we could do something like\n>> > this:\n>> \n>> Yes, that is the direction I was alluding to.\n>> \n>> Callers of get_author_ident_from_commit can also do the same and\n>> avoid rebuilding the same $pick_author_script over and over again,\n>> or get_author_ident_from_commit can do so for its callers.\n>\n> I don't think get_author_ident_from_commit can do so on demand due to\n> the subshell issue I mentioned.\n\nOK, that is what I obviously missed.\n\n> I wonder if we should simply generate these at build time and store them\n> in the script.\n\nYeah, my knee-jerk reaction was that it was wasteful to run script\nat runtime to build a constant string, but the engineering cost may\nnot be worth it.\n"}]}