{"thread":{"id":"48767","subject":"[PATCH] filter-branch: skip commits present on --state-branch","startedAt":"2018-06-23T04:36:57Z","lastAt":"2018-06-26T22:44:42Z","messageCount":5,"participants":["Michael Barabanov","Junio C Hamano","Ian Campbell"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"350776","messageId":"20180623043639.7727-1-michael.barabanov@gmail.com","threadId":"48767","inReplyTo":null,"subject":"[PATCH] filter-branch: skip commits present on --state-branch","fromName":"Michael Barabanov","fromEmail":"michael.barabanov@gmail.com","sentAt":"2018-06-23T04:36:39Z","receivedAt":"2018-06-23T04:36:57Z","isPatch":true,"sender":{"key":"michael.barabanov@gmail.com","avatar":null},"body":"The commits in state:filter.map have already been processed, so don't\nfilter them again. This makes incremental git filter-branch much faster.\n\nAlso add tests for --state-branch option.\n\nSigned-off-by: Michael Barabanov <michael.barabanov@gmail.com>\n---\n git-filter-branch.sh     |  3 +++\n t/t7003-filter-branch.sh | 15 +++++++++++++++\n 2 files changed, 18 insertions(+)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex ccceaf19a..2df7ed107 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -372,6 +372,9 @@ while read commit parents; do\n \tgit_filter_branch__commit_count=$(($git_filter_branch__commit_count+1))\n \n \treport_progress\n+\tif test -r \"$workdir/../map/$commit\"; then\n+\t\tcontinue\n+\tfi\n \n \tcase \"$filter_subdir\" in\n \t\"\")\ndiff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\nindex ec4b160dd..e23de7d0b 100755\n--- a/t/t7003-filter-branch.sh\n+++ b/t/t7003-filter-branch.sh\n@@ -107,6 +107,21 @@ test_expect_success 'test that the directory was renamed' '\n \ttest dir/D = \"$(cat diroh/D.t)\"\n '\n \n+V=$(git rev-parse HEAD)\n+\n+test_expect_success 'populate --state-branch' '\n+\tgit filter-branch --state-branch state -f --tree-filter \"touch file || :\" HEAD\n+'\n+\n+W=$(git rev-parse HEAD)\n+\n+test_expect_success 'using --state-branch to skip already rewritten commits' '\n+\ttest_when_finished git reset --hard $V &&\n+\tgit reset --hard $V &&\n+\tgit filter-branch --state-branch state -f --tree-filter \"touch file || :\" HEAD &&\n+\ttest_cmp_rev $W HEAD\n+'\n+\n git tag oldD HEAD~4\n test_expect_success 'rewrite one branch, keeping a side branch' '\n \tgit branch modD oldD &&\n-- \n2.17.1\n\n"},{"id":"350907","messageId":"xmqqmuvims38.fsf@gitster-ct.c.googlers.com","threadId":"48767","inReplyTo":"20180623043639.7727-1-michael.barabanov@gmail.com","subject":"Re: [PATCH] filter-branch: skip commits present on --state-branch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-06-25T22:14:35Z","receivedAt":"2018-06-25T22:14:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Barabanov <michael.barabanov@gmail.com> writes:\n\n> The commits in state:filter.map have already been processed, so don't\n> filter them again. This makes incremental git filter-branch much faster.\n>\n> Also add tests for --state-branch option.\n>\n> Signed-off-by: Michael Barabanov <michael.barabanov@gmail.com>\n> ---\n>  git-filter-branch.sh     |  3 +++\n>  t/t7003-filter-branch.sh | 15 +++++++++++++++\n>  2 files changed, 18 insertions(+)\n>\n> diff --git a/git-filter-branch.sh b/git-filter-branch.sh\n> index ccceaf19a..2df7ed107 100755\n> --- a/git-filter-branch.sh\n> +++ b/git-filter-branch.sh\n> @@ -372,6 +372,9 @@ while read commit parents; do\n>  \tgit_filter_branch__commit_count=$(($git_filter_branch__commit_count+1))\n>  \n>  \treport_progress\n> +\tif test -r \"$workdir/../map/$commit\"; then\n> +\t\tcontinue\n> +\tfi\n\nThe original script is so much of a mess that I needed quite some\ntime to find enough evidence to convince myself that this change is\nin line with what is already happening in the program.  We have\n\n\ttest -f \"$workdir\"/../map/$sha1 && continue\n\nin the codepath for remap-to-ancestor prostprocessing to do\npretty-much the same skipping.\n\nI think the new code should follow suit, i.e.\n\n\tif test -f \"$workdir/../map/$commit\"\n\tthen\n\t\tcontinue\n\tfi\n\nto check just the existence for consistency.\n\nIt would have been reviewer friendly if the proposed commit log\nmessage said how this change does *not* break the progress output\nand count.  A possible alternative optimization could be not to add\nthese already mapped commits in ../revs file in the first place (so\nthey are not even counted as part of $commits), and such a change\nwould give different meaning to the progress output (which may or\nmay not be a good change).  Instead, the posted patch counts the\ncommits to be filtered the same way as before, and merely pretends\nthat it filtered those commits to their mapped counterparts without\nspending any cycle (simply because we already _know_ what they are\nmapped to), so the meaning of the numbers in the progress display\ndoes not change at all---just they appear to progress much faster,\nwhich is a welcome change ;-)\n\n\n\n"},{"id":"350914","messageId":"20180626040733.30757-1-michael.barabanov@gmail.com","threadId":"48767","inReplyTo":"20180623043639.7727-1-michael.barabanov@gmail.com","subject":"[PATCH v2] filter-branch: skip commits present on --state-branch","fromName":"Michael Barabanov","fromEmail":"michael.barabanov@gmail.com","sentAt":"2018-06-26T04:07:33Z","receivedAt":"2018-06-26T04:07:51Z","isPatch":true,"sender":{"key":"michael.barabanov@gmail.com","avatar":null},"body":"The commits in state:filter.map have already been processed, so don't\nfilter them again. This makes incremental git filter-branch much faster.\n\nAlso add tests for --state-branch option.\n\nSigned-off-by: Michael Barabanov <michael.barabanov@gmail.com>\n---\n git-filter-branch.sh     |  1 +\n t/t7003-filter-branch.sh | 15 +++++++++++++++\n 2 files changed, 16 insertions(+)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex ccceaf19a..5c5afa2b9 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -372,6 +372,7 @@ while read commit parents; do\n \tgit_filter_branch__commit_count=$(($git_filter_branch__commit_count+1))\n \n \treport_progress\n+\ttest -f \"$workdir\"/../map/$commit && continue\n \n \tcase \"$filter_subdir\" in\n \t\"\")\ndiff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\nindex ec4b160dd..e23de7d0b 100755\n--- a/t/t7003-filter-branch.sh\n+++ b/t/t7003-filter-branch.sh\n@@ -107,6 +107,21 @@ test_expect_success 'test that the directory was renamed' '\n \ttest dir/D = \"$(cat diroh/D.t)\"\n '\n \n+V=$(git rev-parse HEAD)\n+\n+test_expect_success 'populate --state-branch' '\n+\tgit filter-branch --state-branch state -f --tree-filter \"touch file || :\" HEAD\n+'\n+\n+W=$(git rev-parse HEAD)\n+\n+test_expect_success 'using --state-branch to skip already rewritten commits' '\n+\ttest_when_finished git reset --hard $V &&\n+\tgit reset --hard $V &&\n+\tgit filter-branch --state-branch state -f --tree-filter \"touch file || :\" HEAD &&\n+\ttest_cmp_rev $W HEAD\n+'\n+\n git tag oldD HEAD~4\n test_expect_success 'rewrite one branch, keeping a side branch' '\n \tgit branch modD oldD &&\n-- \n2.18.0\n\n"},{"id":"350995","messageId":"1530039484.9819.8.camel@hellion.org.uk","threadId":"48767","inReplyTo":"20180626040733.30757-1-michael.barabanov@gmail.com","subject":"Re: [PATCH v2] filter-branch: skip commits present on --state-branch","fromName":"Ian Campbell","fromEmail":"ijc@hellion.org.uk","sentAt":"2018-06-26T18:58:04Z","receivedAt":"2018-06-26T19:17:39Z","isPatch":true,"sender":{"key":"ijc@hellion.org.uk","avatar":"https://avatars.githubusercontent.com/u/12985729?v=4"},"body":"On Mon, 2018-06-25 at 21:07 -0700, Michael Barabanov wrote:\n> The commits in state:filter.map have already been processed, so don't\n> filter them again. This makes incremental git filter-branch much\n> faster.\n> \n> Also add tests for --state-branch option.\n> \n> Signed-off-by: Michael Barabanov <michael.barabanov@gmail.com>\n\nAcked-by: Ian Campbell <ijc@hellion.org.uk>\n\n> ---\n>  git-filter-branch.sh     |  1 +\n>  t/t7003-filter-branch.sh | 15 +++++++++++++++\n>  2 files changed, 16 insertions(+)\n> \n> diff --git a/git-filter-branch.sh b/git-filter-branch.sh\n> index ccceaf19a..5c5afa2b9 100755\n> --- a/git-filter-branch.sh\n> +++ b/git-filter-branch.sh\n> @@ -372,6 +372,7 @@ while read commit parents; do\n>  \tgit_filter_branch__commit_count=$(($git_filter_branch__commi\n> t_count+1))\n>  \n>  \treport_progress\n> +\ttest -f \"$workdir\"/../map/$commit && continue\n>  \n>  \tcase \"$filter_subdir\" in\n>  \t\"\")\n> diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\n> index ec4b160dd..e23de7d0b 100755\n> --- a/t/t7003-filter-branch.sh\n> +++ b/t/t7003-filter-branch.sh\n> @@ -107,6 +107,21 @@ test_expect_success 'test that the directory was\n> renamed' '\n>  \ttest dir/D = \"$(cat diroh/D.t)\"\n>  '\n>  \n> +V=$(git rev-parse HEAD)\n> +\n> +test_expect_success 'populate --state-branch' '\n> +\tgit filter-branch --state-branch state -f --tree-filter\n> \"touch file || :\" HEAD\n> +'\n> +\n> +W=$(git rev-parse HEAD)\n> +\n> +test_expect_success 'using --state-branch to skip already rewritten\n> commits' '\n> +\ttest_when_finished git reset --hard $V &&\n> +\tgit reset --hard $V &&\n> +\tgit filter-branch --state-branch state -f --tree-filter\n> \"touch file || :\" HEAD &&\n> +\ttest_cmp_rev $W HEAD\n> +'\n> +\n>  git tag oldD HEAD~4\n>  test_expect_success 'rewrite one branch, keeping a side branch' '\n>  \tgit branch modD oldD &&\n"},{"id":"351055","messageId":"xmqq1scti2wb.fsf@gitster-ct.c.googlers.com","threadId":"48767","inReplyTo":"1530039484.9819.8.camel@hellion.org.uk","subject":"Re: [PATCH v2] filter-branch: skip commits present on --state-branch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-06-26T22:44:36Z","receivedAt":"2018-06-26T22:44:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ian Campbell <ijc@hellion.org.uk> writes:\n\n> On Mon, 2018-06-25 at 21:07 -0700, Michael Barabanov wrote:\n>> The commits in state:filter.map have already been processed, so don't\n>> filter them again. This makes incremental git filter-branch much\n>> faster.\n>> \n>> Also add tests for --state-branch option.\n>> \n>> Signed-off-by: Michael Barabanov <michael.barabanov@gmail.com>\n>\n> Acked-by: Ian Campbell <ijc@hellion.org.uk>\n\nThanks.\n\n>\n>> ---\n>>  git-filter-branch.sh     |  1 +\n>>  t/t7003-filter-branch.sh | 15 +++++++++++++++\n>>  2 files changed, 16 insertions(+)\n>> \n>> diff --git a/git-filter-branch.sh b/git-filter-branch.sh\n>> index ccceaf19a..5c5afa2b9 100755\n>> --- a/git-filter-branch.sh\n>> +++ b/git-filter-branch.sh\n>> @@ -372,6 +372,7 @@ while read commit parents; do\n>>  \tgit_filter_branch__commit_count=$(($git_filter_branch__commi\n>> t_count+1))\n>>  \n>>  \treport_progress\n>> +\ttest -f \"$workdir\"/../map/$commit && continue\n>>  \n>>  \tcase \"$filter_subdir\" in\n>>  \t\"\")\n>> diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\n>> index ec4b160dd..e23de7d0b 100755\n>> --- a/t/t7003-filter-branch.sh\n>> +++ b/t/t7003-filter-branch.sh\n>> @@ -107,6 +107,21 @@ test_expect_success 'test that the directory was\n>> renamed' '\n>>  \ttest dir/D = \"$(cat diroh/D.t)\"\n>>  '\n>>  \n>> +V=$(git rev-parse HEAD)\n>> +\n>> +test_expect_success 'populate --state-branch' '\n>> +\tgit filter-branch --state-branch state -f --tree-filter\n>> \"touch file || :\" HEAD\n>> +'\n>> +\n>> +W=$(git rev-parse HEAD)\n>> +\n>> +test_expect_success 'using --state-branch to skip already rewritten\n>> commits' '\n>> +\ttest_when_finished git reset --hard $V &&\n>> +\tgit reset --hard $V &&\n>> +\tgit filter-branch --state-branch state -f --tree-filter\n>> \"touch file || :\" HEAD &&\n>> +\ttest_cmp_rev $W HEAD\n>> +'\n>> +\n>>  git tag oldD HEAD~4\n>>  test_expect_success 'rewrite one branch, keeping a side branch' '\n>>  \tgit branch modD oldD &&\n"}]}