threads / patch / 48767

patchfilter-branch: skip commits present on --state-branch

Subject: [PATCH] filter-branch: skip commits present on --state-branch

## tl;dr

5 messages between Jun 23, 2018 and Jun 26, 2018. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Michael Barabanov· Jun 23, 2018, 04:36 UTC · lore

The commits in state:filter.map have already been processed, so don't filter them again. This makes incremental git filter-branch much faster.

Also add tests for --state-branch option.

Signed-off-by: Michael Barabanov <michael.barabanov@gmail.com>
---
 git-filter-branch.sh     |  3 +++
 t/t7003-filter-branch.sh | 15 +++++++++++++++
 2 files changed, 18 insertions(+)

Show changes to 2 files +18 −0

git-filter-branch.sh, t/t7003-filter-branch.sh

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index ccceaf19a..2df7ed107 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -372,6 +372,9 @@ while read commit parents; do
 	git_filter_branch__commit_count=$(($git_filter_branch__commit_count+1))
 
 	report_progress
+	if test -r "$workdir/../map/$commit"; then
+		continue
+	fi
 
 	case "$filter_subdir" in
 	"")
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index ec4b160dd..e23de7d0b 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -107,6 +107,21 @@ test_expect_success 'test that the directory was renamed' '
 	test dir/D = "$(cat diroh/D.t)"
 '
 
+V=$(git rev-parse HEAD)
+
+test_expect_success 'populate --state-branch' '
+	git filter-branch --state-branch state -f --tree-filter "touch file || :" HEAD
+'
+
+W=$(git rev-parse HEAD)
+
+test_expect_success 'using --state-branch to skip already rewritten commits' '
+	test_when_finished git reset --hard $V &&
+	git reset --hard $V &&
+	git filter-branch --state-branch state -f --tree-filter "touch file || :" HEAD &&
+	test_cmp_rev $W HEAD
+'
+
 git tag oldD HEAD~4
 test_expect_success 'rewrite one branch, keeping a side branch' '
 	git branch modD oldD &&

-- 
2.17.1

Junio C Hamano· Jun 25, 2018, 22:14 UTC · re: Michael Barabanov · lore

Re: [PATCH] filter-branch: skip commits present on --state-branch

Michael Barabanov <michael.barabanov@gmail.com> writes:

Show 22 quoted lines

> The commits in state:filter.map have already been processed, so don't
> filter them again. This makes incremental git filter-branch much faster.
>
> Also add tests for --state-branch option.
>
> Signed-off-by: Michael Barabanov <michael.barabanov@gmail.com>
> ---
>  git-filter-branch.sh     |  3 +++
>  t/t7003-filter-branch.sh | 15 +++++++++++++++
>  2 files changed, 18 insertions(+)
>
> diff --git a/git-filter-branch.sh b/git-filter-branch.sh
> index ccceaf19a..2df7ed107 100755
> --- a/git-filter-branch.sh
> +++ b/git-filter-branch.sh
> @@ -372,6 +372,9 @@ while read commit parents; do
>  	git_filter_branch__commit_count=$(($git_filter_branch__commit_count+1))
>  
>  	report_progress
> +	if test -r "$workdir/../map/$commit"; then
> +		continue
> +	fi

The original script is so much of a mess that I needed quite some time to find enough evidence to convince myself that this change is in line with what is already happening in the program. We have

	test -f "$workdir"/../map/$sha1 && continue

in the codepath for remap-to-ancestor prostprocessing to do pretty-much the same skipping.

I think the new code should follow suit, i.e.

	if test -f "$workdir/../map/$commit"
	then
		continue
	fi

to check just the existence for consistency.

It would have been reviewer friendly if the proposed commit log message said how this change does *not* break the progress output and count. A possible alternative optimization could be not to add these already mapped commits in ../revs file in the first place (so they are not even counted as part of $commits), and such a change would give different meaning to the progress output (which may or may not be a good change). Instead, the posted patch counts the commits to be filtered the same way as before, and merely pretends that it filtered those commits to their mapped counterparts without spending any cycle (simply because we already _know_ what they are mapped to), so the meaning of the numbers in the progress display does not change at all---just they appear to progress much faster, which is a welcome change ;-)

Michael Barabanov· Jun 26, 2018, 04:07 UTC · re: Michael Barabanov · lore

[PATCH v2] filter-branch: skip commits present on --state-branch

The commits in state:filter.map have already been processed, so don't filter them again. This makes incremental git filter-branch much faster.

Also add tests for --state-branch option.

Signed-off-by: Michael Barabanov <michael.barabanov@gmail.com>
---
 git-filter-branch.sh     |  1 +
 t/t7003-filter-branch.sh | 15 +++++++++++++++
 2 files changed, 16 insertions(+)

Show changes to 2 files +16 −0

git-filter-branch.sh, t/t7003-filter-branch.sh

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index ccceaf19a..5c5afa2b9 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -372,6 +372,7 @@ while read commit parents; do
 	git_filter_branch__commit_count=$(($git_filter_branch__commit_count+1))
 
 	report_progress
+	test -f "$workdir"/../map/$commit && continue
 
 	case "$filter_subdir" in
 	"")
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index ec4b160dd..e23de7d0b 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -107,6 +107,21 @@ test_expect_success 'test that the directory was renamed' '
 	test dir/D = "$(cat diroh/D.t)"
 '
 
+V=$(git rev-parse HEAD)
+
+test_expect_success 'populate --state-branch' '
+	git filter-branch --state-branch state -f --tree-filter "touch file || :" HEAD
+'
+
+W=$(git rev-parse HEAD)
+
+test_expect_success 'using --state-branch to skip already rewritten commits' '
+	test_when_finished git reset --hard $V &&
+	git reset --hard $V &&
+	git filter-branch --state-branch state -f --tree-filter "touch file || :" HEAD &&
+	test_cmp_rev $W HEAD
+'
+
 git tag oldD HEAD~4
 test_expect_success 'rewrite one branch, keeping a side branch' '
 	git branch modD oldD &&

-- 
2.18.0

Ian Campbell· Jun 26, 2018, 18:58 UTC · re: Michael Barabanov · lore

Re: [PATCH v2] filter-branch: skip commits present on --state-branch

On Mon, 2018-06-25 at 21:07 -0700, Michael Barabanov wrote:

Show 7 quoted lines

> The commits in state:filter.map have already been processed, so don't
> filter them again. This makes incremental git filter-branch much
> faster.
> 
> Also add tests for --state-branch option.
> 
> Signed-off-by: Michael Barabanov <michael.barabanov@gmail.com>

Acked-by: Ian Campbell <ijc@hellion.org.uk>

Show 48 quoted lines

> ---
>  git-filter-branch.sh     |  1 +
>  t/t7003-filter-branch.sh | 15 +++++++++++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/git-filter-branch.sh b/git-filter-branch.sh
> index ccceaf19a..5c5afa2b9 100755
> --- a/git-filter-branch.sh
> +++ b/git-filter-branch.sh
> @@ -372,6 +372,7 @@ while read commit parents; do
>  	git_filter_branch__commit_count=$(($git_filter_branch__commi
> t_count+1))
>  
>  	report_progress
> +	test -f "$workdir"/../map/$commit && continue
>  
>  	case "$filter_subdir" in
>  	"")
> diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
> index ec4b160dd..e23de7d0b 100755
> --- a/t/t7003-filter-branch.sh
> +++ b/t/t7003-filter-branch.sh
> @@ -107,6 +107,21 @@ test_expect_success 'test that the directory was
> renamed' '
>  	test dir/D = "$(cat diroh/D.t)"
>  '
>  
> +V=$(git rev-parse HEAD)
> +
> +test_expect_success 'populate --state-branch' '
> +	git filter-branch --state-branch state -f --tree-filter
> "touch file || :" HEAD
> +'
> +
> +W=$(git rev-parse HEAD)
> +
> +test_expect_success 'using --state-branch to skip already rewritten
> commits' '
> +	test_when_finished git reset --hard $V &&
> +	git reset --hard $V &&
> +	git filter-branch --state-branch state -f --tree-filter
> "touch file || :" HEAD &&
> +	test_cmp_rev $W HEAD
> +'
> +
>  git tag oldD HEAD~4
>  test_expect_success 'rewrite one branch, keeping a side branch' '
>  	git branch modD oldD &&

Junio C Hamano· Jun 26, 2018, 22:44 UTC · re: Ian Campbell · lore

Re: [PATCH v2] filter-branch: skip commits present on --state-branch

Ian Campbell <ijc@hellion.org.uk> writes:

Show 10 quoted lines

> On Mon, 2018-06-25 at 21:07 -0700, Michael Barabanov wrote:
>> The commits in state:filter.map have already been processed, so don't
>> filter them again. This makes incremental git filter-branch much
>> faster.
>> 
>> Also add tests for --state-branch option.
>> 
>> Signed-off-by: Michael Barabanov <michael.barabanov@gmail.com>
>
> Acked-by: Ian Campbell <ijc@hellion.org.uk>

Thanks.

Show 49 quoted lines

>
>> ---
>>  git-filter-branch.sh     |  1 +
>>  t/t7003-filter-branch.sh | 15 +++++++++++++++
>>  2 files changed, 16 insertions(+)
>> 
>> diff --git a/git-filter-branch.sh b/git-filter-branch.sh
>> index ccceaf19a..5c5afa2b9 100755
>> --- a/git-filter-branch.sh
>> +++ b/git-filter-branch.sh
>> @@ -372,6 +372,7 @@ while read commit parents; do
>>  	git_filter_branch__commit_count=$(($git_filter_branch__commi
>> t_count+1))
>>  
>>  	report_progress
>> +	test -f "$workdir"/../map/$commit && continue
>>  
>>  	case "$filter_subdir" in
>>  	"")
>> diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
>> index ec4b160dd..e23de7d0b 100755
>> --- a/t/t7003-filter-branch.sh
>> +++ b/t/t7003-filter-branch.sh
>> @@ -107,6 +107,21 @@ test_expect_success 'test that the directory was
>> renamed' '
>>  	test dir/D = "$(cat diroh/D.t)"
>>  '
>>  
>> +V=$(git rev-parse HEAD)
>> +
>> +test_expect_success 'populate --state-branch' '
>> +	git filter-branch --state-branch state -f --tree-filter
>> "touch file || :" HEAD
>> +'
>> +
>> +W=$(git rev-parse HEAD)
>> +
>> +test_expect_success 'using --state-branch to skip already rewritten
>> commits' '
>> +	test_when_finished git reset --hard $V &&
>> +	git reset --hard $V &&
>> +	git filter-branch --state-branch state -f --tree-filter
>> "touch file || :" HEAD &&
>> +	test_cmp_rev $W HEAD
>> +'
>> +
>>  git tag oldD HEAD~4
>>  test_expect_success 'rewrite one branch, keeping a side branch' '
>>  	git branch modD oldD &&