threads / patch / 31236

v3, 2 partsgit-svn.perl: consider all ranges for a given merge, instead of only tip-by-tip

Subject: [PATCH v3 1/2] git-svn.perl: consider all ranges for a given merge, instead of only tip-by-tip

## tl;dr

13 messages between Aug 11, 2012 and Aug 21, 2012. Diffs are folded; open one to read it.

replies: 12people: 4as markdown or json

Steven Walter· Aug 11, 2012, 17:14 UTC · lore

Consider the case where you have trunk, branchA of trunk, and branchB of branchA. trunk is merged back into branchB, and then branchB is reintegrated into trunk. The merge of branchB into trunk will have svn:mergeinfo property references to both branchA and branchB. When performing the check_cherry_pick check on branchB, it is necessary to eliminate the merged contents of branchA as well as branchB, or else the merge will be incorrectly ignored as a cherry-pick.

Signed-off-by: Steven Walter <stevenrwalter@gmail.com>
---
 git-svn.perl                                    |    8 ++-
 t/t9163-git-svn-fetch-merge-branch-of-branch.sh |   60 +++++++++++++++++++++++
 2 files changed, 63 insertions(+), 5 deletions(-)
 create mode 100755 t/t9163-git-svn-fetch-merge-branch-of-branch.sh
Show changes to 2 files +63 −5

git-svn.perl, t/t9163-git-svn-fetch-merge-branch-of-branch.sh

diff --git a/git-svn.perl b/git-svn.perl
index ca038ec..abcec11 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -3657,14 +3657,14 @@ sub find_extra_svn_parents {
 	my @merge_tips;
 	my $url = $self->{url};
 	my $uuid = $self->ra_uuid;
-	my %ranges;
+	my @all_ranges;
 	for my $merge ( @merges ) {
 		my ($tip_commit, @ranges) =
 			lookup_svn_merge( $uuid, $url, $merge );
 		unless (!$tip_commit or
 				grep { $_ eq $tip_commit } @$parents ) {
 			push @merge_tips, $tip_commit;
-			$ranges{$tip_commit} = \@ranges;
+			push @all_ranges, @ranges;
 		} else {
 			push @merge_tips, undef;
 		}
@@ -3679,8 +3679,6 @@ sub find_extra_svn_parents {
 		my $spec = shift @merges;
 		next unless $merge_tip and $excluded{$merge_tip};
 
-		my $ranges = $ranges{$merge_tip};
-
 		# check out 'new' tips
 		my $merge_base;
 		eval {
@@ -3702,7 +3700,7 @@ sub find_extra_svn_parents {
 		my (@incomplete) = check_cherry_pick(
 			$merge_base, $merge_tip,
 			$parents,
-			@$ranges,
+			@all_ranges,
 		       );
 
 		if ( @incomplete ) {
diff --git a/t/t9163-git-svn-fetch-merge-branch-of-branch.sh b/t/t9163-git-svn-fetch-merge-branch-of-branch.sh
new file mode 100755
index 0000000..13ae7e3
--- /dev/null
+++ b/t/t9163-git-svn-fetch-merge-branch-of-branch.sh
@@ -0,0 +1,60 @@
+#!/bin/sh
+#
+# Copyright (c) 2012 Steven Walter
+#
+
+test_description='git svn merge detection'
+. ./lib-git-svn.sh
+
+svn_ver="$(svn --version --quiet)"
+case $svn_ver in
+0.* | 1.[0-4].*)
+	skip_all="skipping git-svn test - SVN too old ($svn_ver)"
+	test_done
+	;;
+esac
+
+test_expect_success 'initialize source svn repo' '
+	svn_cmd mkdir -m x "$svnrepo"/trunk &&
+	svn_cmd mkdir -m x "$svnrepo"/branches &&
+	svn_cmd co "$svnrepo"/trunk "$SVN_TREE" &&
+	(
+		cd "$SVN_TREE" &&
+		touch foo &&
+		svn_cmd add foo &&
+		svn_cmd commit -m "initial commit" &&
+		svn_cmd cp -m branch "$svnrepo"/trunk "$svnrepo"/branches/branch1 &&
+		svn_cmd switch "$svnrepo"/branches/branch1 &&
+		touch bar &&
+		svn_cmd add bar &&
+		svn_cmd commit -m branch1 &&
+		svn_cmd cp -m branch "$svnrepo"/branches/branch1 "$svnrepo"/branches/branch2 &&
+		svn_cmd switch "$svnrepo"/branches/branch2 &&
+		touch baz &&
+		svn_cmd add baz &&
+		svn_cmd commit -m branch2 &&
+		svn_cmd switch "$svnrepo"/trunk &&
+		touch bar2 &&
+		svn_cmd add bar2 &&
+		svn_cmd commit -m trunk &&
+		svn_cmd switch "$svnrepo"/branches/branch2 &&
+		svn_cmd merge "$svnrepo"/trunk &&
+		svn_cmd commit -m "merge trunk"
+		svn_cmd switch "$svnrepo"/trunk &&
+		svn_cmd merge --reintegrate "$svnrepo"/branches/branch2 &&
+		svn_cmd commit -m "merge branch2"
+	) &&
+	rm -rf "$SVN_TREE"
+'
+
+test_expect_success 'clone svn repo' '
+	git svn init -s "$svnrepo" &&
+	git svn fetch
+'
+
+test_expect_success 'verify merge commit' 'x=$(git rev-parse HEAD^2) &&
+	y=$(git rev-parse branch2) &&
+	test "x$x" = "x$y"
+'
+
+test_done
-- 
1.7.9.5
Steven Walter· Aug 11, 2012, 17:14 UTC · re: Steven Walter · lore

[PATCH v3 2/2] git-svn.perl: keep processing all commits in parents_exclude

This fixes a bug where git finds the incorrect merge parent. Consider a repository with trunk, branch1 of trunk, and branch2 of branch1. Without this change, git interprets a merge of branch2 into trunk as a merge of branch1 into trunk.

Signed-off-by: Steven Walter <stevenrwalter@gmail.com>
---
 git-svn.perl                                     |    1 -
 t/t9164-git-svn-fetch-merge-branch-of-branch2.sh |   53 ++++++++++++++++++++++
 2 files changed, 53 insertions(+), 1 deletion(-)
 create mode 100755 t/t9164-git-svn-fetch-merge-branch-of-branch2.sh
Show changes to 2 files +53 −1

git-svn.perl, t/t9164-git-svn-fetch-merge-branch-of-branch2.sh

diff --git a/git-svn.perl b/git-svn.perl
index abcec11..c4678c1 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -3623,7 +3623,6 @@ sub parents_exclude {
 				if ( $commit eq $excluded ) {
 					push @excluded, $commit;
 					$found++;
-					last;
 				}
 				else {
 					push @new, $commit;
diff --git a/t/t9164-git-svn-fetch-merge-branch-of-branch2.sh b/t/t9164-git-svn-fetch-merge-branch-of-branch2.sh
new file mode 100755
index 0000000..af0ec0e
--- /dev/null
+++ b/t/t9164-git-svn-fetch-merge-branch-of-branch2.sh
@@ -0,0 +1,53 @@
+#!/bin/sh
+#
+# Copyright (c) 2012 Steven Walter
+#
+
+test_description='git svn merge detection'
+. ./lib-git-svn.sh
+
+svn_ver="$(svn --version --quiet)"
+case $svn_ver in
+0.* | 1.[0-4].*)
+	skip_all="skipping git-svn test - SVN too old ($svn_ver)"
+	test_done
+	;;
+esac
+
+test_expect_success 'initialize source svn repo' '
+	svn_cmd mkdir -m x "$svnrepo"/trunk &&
+	svn_cmd mkdir -m x "$svnrepo"/branches &&
+	svn_cmd co "$svnrepo"/trunk "$SVN_TREE" &&
+	(
+		cd "$SVN_TREE" &&
+		touch foo &&
+		svn_cmd add foo &&
+		svn_cmd commit -m "initial commit" &&
+		svn_cmd cp -m branch "$svnrepo"/trunk "$svnrepo"/branches/branch1 &&
+		svn_cmd switch "$svnrepo"/branches/branch1 &&
+		touch bar &&
+		svn_cmd add bar &&
+		svn_cmd commit -m branch1 &&
+		svn_cmd cp -m branch "$svnrepo"/branches/branch1 "$svnrepo"/branches/branch2 &&
+		svn_cmd switch "$svnrepo"/branches/branch2 &&
+		touch baz &&
+		svn_cmd add baz &&
+		svn_cmd commit -m branch2 &&
+		svn_cmd switch "$svnrepo"/trunk &&
+		svn_cmd merge --reintegrate "$svnrepo"/branches/branch2 &&
+		svn_cmd commit -m "merge branch2"
+	) &&
+	rm -rf "$SVN_TREE"
+'
+
+test_expect_success 'clone svn repo' '
+	git svn init -s "$svnrepo" &&
+	git svn fetch
+'
+
+test_expect_success 'verify merge commit' 'x=$(git rev-parse HEAD^2) &&
+	y=$(git rev-parse branch2) &&
+	test "x$x" = "x$y"
+'
+
+test_done
-- 
1.7.9.5
Sam Vilain· Aug 18, 2012, 19:51 UTC · re: Steven Walter · lore

Re: [PATCH v3 2/2] git-svn.perl: keep processing all commits in parents_exclude

On 08/11/2012 10:14 AM, Steven Walter wrote:
Show 22 quoted lines
> This fixes a bug where git finds the incorrect merge parent.  Consider a
> repository with trunk, branch1 of trunk, and branch2 of branch1.
> Without this change, git interprets a merge of branch2 into trunk as a
> merge of branch1 into trunk.
>
> Signed-off-by: Steven Walter <stevenrwalter@gmail.com>
> ---
>   git-svn.perl                                     |    1 -
>   t/t9164-git-svn-fetch-merge-branch-of-branch2.sh |   53 ++++++++++++++++++++++
>   2 files changed, 53 insertions(+), 1 deletion(-)
>   create mode 100755 t/t9164-git-svn-fetch-merge-branch-of-branch2.sh
>
> diff --git a/git-svn.perl b/git-svn.perl
> index abcec11..c4678c1 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -3623,7 +3623,6 @@ sub parents_exclude {
>   				if ( $commit eq $excluded ) {
>   					push @excluded, $commit;
>   					$found++;
> -					last;
>   				}

I could believe that, too. I like this change: one line of code, 53 lines of test and a paragraph of explanation :-).

Cheers, Sam.

Steven Walter· Aug 18, 2012, 20:43 UTC · re: Sam Vilain · lore

Re: [PATCH v3 2/2] git-svn.perl: keep processing all commits in parents_exclude

On Sat, Aug 18, 2012 at 3:51 PM, Sam Vilain <sam@vilain.net> wrote:
Show 29 quoted lines
> On 08/11/2012 10:14 AM, Steven Walter wrote:
>>
>> This fixes a bug where git finds the incorrect merge parent.  Consider a
>> repository with trunk, branch1 of trunk, and branch2 of branch1.
>> Without this change, git interprets a merge of branch2 into trunk as a
>> merge of branch1 into trunk.
>>
>> Signed-off-by: Steven Walter <stevenrwalter@gmail.com>
>> ---
>>   git-svn.perl                                     |    1 -
>>   t/t9164-git-svn-fetch-merge-branch-of-branch2.sh |   53
>> ++++++++++++++++++++++
>>   2 files changed, 53 insertions(+), 1 deletion(-)
>>   create mode 100755 t/t9164-git-svn-fetch-merge-branch-of-branch2.sh
>>
>> diff --git a/git-svn.perl b/git-svn.perl
>> index abcec11..c4678c1 100755
>> --- a/git-svn.perl
>> +++ b/git-svn.perl
>> @@ -3623,7 +3623,6 @@ sub parents_exclude {
>>                                 if ( $commit eq $excluded ) {
>>                                         push @excluded, $commit;
>>                                         $found++;
>> -                                       last;
>>                                 }
>
>
> I could believe that, too.  I like this change: one line of code, 53 lines
> of test and a paragraph of explanation :-).
How about a Signed-Off-By?
-- 
-Steven Walter <stevenrwalter@gmail.com>
Sam Vilain· Aug 18, 2012, 23:37 UTC · re: Steven Walter · lore

Re: [PATCH v3 2/2] git-svn.perl: keep processing all commits in parents_exclude

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
On 08/18/2012 01:43 PM, Steven Walter wrote:
 > How about a Signed-Off-By?
Signed-Off-By: Sam Vilain <sam@vilain.net>
Sam
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)

iQEcBAEBCgAGBQJQMCcnAAoJEBdtaL3wGtIoJ1UIAIJ6Xz5OEMmMk1tq546eggHg I+sJIFjqg+mo53VqT0/bKhqg8sLx8F/Gda15nwOUMcslKJdA+sCc+QhAtgSWJ1WK Idw59jtZHbabfopBHNgneSqVBhXSKpNw3e3EvlRVkK1wobO0+c0X6YkBG0eBCZl2 6RYXIAb6jX04k1hSrnxcPn+REkoyl31aEuFBPNz0wRWHjju+G6bPY/x7D/gO1YOc /uRQXveQngJOLwawDR+dGS+0aWPseX/sbZqsVFo0hVQYqoHt+s4uVuriBfHSRKd+ R1eUoY0ikW4UvEwZX74Zf3SeoVLLFnkCW8B5XsGb10IojbvY3uyYevATXI79j1Y= =Lb7H -----END PGP SIGNATURE-----

Junio C Hamano· Aug 18, 2012, 23:43 UTC · re: Sam Vilain · lore

Re: [PATCH v3 2/2] git-svn.perl: keep processing all commits in parents_exclude

Sam Vilain <sam@vilain.net> writes:
> On 08/18/2012 01:43 PM, Steven Walter wrote:
>  > How about a Signed-Off-By?
>
> Signed-Off-By: Sam Vilain <sam@vilain.net>
I think you meant reviewed-by or acked-by?
Eric Wong· Aug 19, 2012, 19:33 UTC · re: Steven Walter · lore

Re: [PATCH v3 2/2] git-svn.perl: keep processing all commits in parents_exclude

Steven Walter <stevenrwalter@gmail.com> wrote:
Show 6 quoted lines
> On Sat, Aug 18, 2012 at 3:51 PM, Sam Vilain <sam@vilain.net> wrote:
> > On 08/11/2012 10:14 AM, Steven Walter wrote:
> >> ---
> >>   git-svn.perl                                     |    1 -
> >>   t/t9164-git-svn-fetch-merge-branch-of-branch2.sh |   53
> >> ++++++++++++++++++++++
Thanks all.
Steven: This doesn't apply cleanly to master since git-svn.perl is
split into separate files (git-svn.perl -> perl/Git/SVN*)
Can you rebase against Junio's current master?  Thanks again.
Steven Walter· Aug 20, 2012, 01:39 UTC · re: Eric Wong · lore

[PATCH 1/2] git-svn.perl: consider all ranges for a given merge, instead of only tip-by-tip

Consider the case where you have trunk, branch1 of trunk, and branch2 of branch1. trunk is merged back into branch2, and then branch2 is reintegrated into trunk. The merge of branch2 into trunk will have svn:mergeinfo property references to both branch1 and branch2. When git-svn fetches the commit that merges branch2 (check_cherry_pick), it is necessary to eliminate the merged contents of branch1 as well as branch2, or else the merge will be incorrectly ignored as a cherry-pick.

Signed-off-by: Steven Walter <stevenrwalter@gmail.com>
---
 perl/Git/SVN.pm                                 |    8 ++-
 t/t9165-git-svn-fetch-merge-branch-of-branch.sh |   60 +++++++++++++++++++++++
 2 files changed, 63 insertions(+), 5 deletions(-)
 create mode 100755 t/t9165-git-svn-fetch-merge-branch-of-branch.sh
Show changes to 2 files +63 −5

perl/Git/SVN.pm, t/t9165-git-svn-fetch-merge-branch-of-branch.sh

diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index 8478d0c..2707003 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -1695,14 +1695,14 @@ sub find_extra_svn_parents {
 	my @merge_tips;
 	my $url = $self->{url};
 	my $uuid = $self->ra_uuid;
-	my %ranges;
+	my @all_ranges;
 	for my $merge ( @merges ) {
 		my ($tip_commit, @ranges) =
 			lookup_svn_merge( $uuid, $url, $merge );
 		unless (!$tip_commit or
 				grep { $_ eq $tip_commit } @$parents ) {
 			push @merge_tips, $tip_commit;
-			$ranges{$tip_commit} = \@ranges;
+			push @all_ranges, @ranges;
 		} else {
 			push @merge_tips, undef;
 		}
@@ -1717,8 +1717,6 @@ sub find_extra_svn_parents {
 		my $spec = shift @merges;
 		next unless $merge_tip and $excluded{$merge_tip};
 
-		my $ranges = $ranges{$merge_tip};
-
 		# check out 'new' tips
 		my $merge_base;
 		eval {
@@ -1740,7 +1738,7 @@ sub find_extra_svn_parents {
 		my (@incomplete) = check_cherry_pick(
 			$merge_base, $merge_tip,
 			$parents,
-			@$ranges,
+			@all_ranges,
 		       );
 
 		if ( @incomplete ) {
diff --git a/t/t9165-git-svn-fetch-merge-branch-of-branch.sh b/t/t9165-git-svn-fetch-merge-branch-of-branch.sh
new file mode 100755
index 0000000..13ae7e3
--- /dev/null
+++ b/t/t9165-git-svn-fetch-merge-branch-of-branch.sh
@@ -0,0 +1,60 @@
+#!/bin/sh
+#
+# Copyright (c) 2012 Steven Walter
+#
+
+test_description='git svn merge detection'
+. ./lib-git-svn.sh
+
+svn_ver="$(svn --version --quiet)"
+case $svn_ver in
+0.* | 1.[0-4].*)
+	skip_all="skipping git-svn test - SVN too old ($svn_ver)"
+	test_done
+	;;
+esac
+
+test_expect_success 'initialize source svn repo' '
+	svn_cmd mkdir -m x "$svnrepo"/trunk &&
+	svn_cmd mkdir -m x "$svnrepo"/branches &&
+	svn_cmd co "$svnrepo"/trunk "$SVN_TREE" &&
+	(
+		cd "$SVN_TREE" &&
+		touch foo &&
+		svn_cmd add foo &&
+		svn_cmd commit -m "initial commit" &&
+		svn_cmd cp -m branch "$svnrepo"/trunk "$svnrepo"/branches/branch1 &&
+		svn_cmd switch "$svnrepo"/branches/branch1 &&
+		touch bar &&
+		svn_cmd add bar &&
+		svn_cmd commit -m branch1 &&
+		svn_cmd cp -m branch "$svnrepo"/branches/branch1 "$svnrepo"/branches/branch2 &&
+		svn_cmd switch "$svnrepo"/branches/branch2 &&
+		touch baz &&
+		svn_cmd add baz &&
+		svn_cmd commit -m branch2 &&
+		svn_cmd switch "$svnrepo"/trunk &&
+		touch bar2 &&
+		svn_cmd add bar2 &&
+		svn_cmd commit -m trunk &&
+		svn_cmd switch "$svnrepo"/branches/branch2 &&
+		svn_cmd merge "$svnrepo"/trunk &&
+		svn_cmd commit -m "merge trunk"
+		svn_cmd switch "$svnrepo"/trunk &&
+		svn_cmd merge --reintegrate "$svnrepo"/branches/branch2 &&
+		svn_cmd commit -m "merge branch2"
+	) &&
+	rm -rf "$SVN_TREE"
+'
+
+test_expect_success 'clone svn repo' '
+	git svn init -s "$svnrepo" &&
+	git svn fetch
+'
+
+test_expect_success 'verify merge commit' 'x=$(git rev-parse HEAD^2) &&
+	y=$(git rev-parse branch2) &&
+	test "x$x" = "x$y"
+'
+
+test_done
-- 
1.7.9.5
Steven Walter· Aug 20, 2012, 01:39 UTC · re: Steven Walter · lore

[PATCH 2/2] git-svn.perl: keep processing all commits in parents_exclude

This fixes a bug where git finds the incorrect merge parent. Consider a repository with trunk, branch1 of trunk, and branch2 of branch1. Without this change, git interprets a merge of branch2 into trunk as a merge of branch1 into trunk.

Signed-off-by: Steven Walter <stevenrwalter@gmail.com>
---
 perl/Git/SVN.pm                                  |    1 -
 t/t9166-git-svn-fetch-merge-branch-of-branch2.sh |   53 ++++++++++++++++++++++
 2 files changed, 53 insertions(+), 1 deletion(-)
 create mode 100755 t/t9166-git-svn-fetch-merge-branch-of-branch2.sh
Show changes to 2 files +53 −1

perl/Git/SVN.pm, t/t9166-git-svn-fetch-merge-branch-of-branch2.sh

diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index 2707003..e48ba46 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -1661,7 +1661,6 @@ sub parents_exclude {
 				if ( $commit eq $excluded ) {
 					push @excluded, $commit;
 					$found++;
-					last;
 				}
 				else {
 					push @new, $commit;
diff --git a/t/t9166-git-svn-fetch-merge-branch-of-branch2.sh b/t/t9166-git-svn-fetch-merge-branch-of-branch2.sh
new file mode 100755
index 0000000..af0ec0e
--- /dev/null
+++ b/t/t9166-git-svn-fetch-merge-branch-of-branch2.sh
@@ -0,0 +1,53 @@
+#!/bin/sh
+#
+# Copyright (c) 2012 Steven Walter
+#
+
+test_description='git svn merge detection'
+. ./lib-git-svn.sh
+
+svn_ver="$(svn --version --quiet)"
+case $svn_ver in
+0.* | 1.[0-4].*)
+	skip_all="skipping git-svn test - SVN too old ($svn_ver)"
+	test_done
+	;;
+esac
+
+test_expect_success 'initialize source svn repo' '
+	svn_cmd mkdir -m x "$svnrepo"/trunk &&
+	svn_cmd mkdir -m x "$svnrepo"/branches &&
+	svn_cmd co "$svnrepo"/trunk "$SVN_TREE" &&
+	(
+		cd "$SVN_TREE" &&
+		touch foo &&
+		svn_cmd add foo &&
+		svn_cmd commit -m "initial commit" &&
+		svn_cmd cp -m branch "$svnrepo"/trunk "$svnrepo"/branches/branch1 &&
+		svn_cmd switch "$svnrepo"/branches/branch1 &&
+		touch bar &&
+		svn_cmd add bar &&
+		svn_cmd commit -m branch1 &&
+		svn_cmd cp -m branch "$svnrepo"/branches/branch1 "$svnrepo"/branches/branch2 &&
+		svn_cmd switch "$svnrepo"/branches/branch2 &&
+		touch baz &&
+		svn_cmd add baz &&
+		svn_cmd commit -m branch2 &&
+		svn_cmd switch "$svnrepo"/trunk &&
+		svn_cmd merge --reintegrate "$svnrepo"/branches/branch2 &&
+		svn_cmd commit -m "merge branch2"
+	) &&
+	rm -rf "$SVN_TREE"
+'
+
+test_expect_success 'clone svn repo' '
+	git svn init -s "$svnrepo" &&
+	git svn fetch
+'
+
+test_expect_success 'verify merge commit' 'x=$(git rev-parse HEAD^2) &&
+	y=$(git rev-parse branch2) &&
+	test "x$x" = "x$y"
+'
+
+test_done
-- 
1.7.9.5
Eric Wong· Aug 21, 2012, 22:05 UTC · re: Steven Walter · lore

Re: [PATCH 1/2] git-svn.perl: consider all ranges for a given merge, instead of only tip-by-tip

Steven Walter <stevenrwalter@gmail.com> wrote:
Show 9 quoted lines
> Consider the case where you have trunk, branch1 of trunk, and branch2 of
> branch1.  trunk is merged back into branch2, and then branch2 is
> reintegrated into trunk.  The merge of branch2 into trunk will have
> svn:mergeinfo property references to both branch1 and branch2.  When
> git-svn fetches the commit that merges branch2 (check_cherry_pick),
> it is necessary to eliminate the merged contents of branch1 as well as
> branch2, or else the merge will be incorrectly ignored as a cherry-pick.
> 
> Signed-off-by: Steven Walter <stevenrwalter@gmail.com>

Thanks, I've Signed-off (also added Sam's Reviewed-by) and pushed to "for-git-master" on git://bogomips.org/git-svn

(Will wait on a few other patches before asking Junio to pull)
Junio C Hamano· Aug 21, 2012, 22:38 UTC · re: Eric Wong · lore

Re: [PATCH 1/2] git-svn.perl: consider all ranges for a given merge, instead of only tip-by-tip

Eric Wong <normalperson@yhbt.net> writes:
Show 15 quoted lines
> Steven Walter <stevenrwalter@gmail.com> wrote:
>> Consider the case where you have trunk, branch1 of trunk, and branch2 of
>> branch1.  trunk is merged back into branch2, and then branch2 is
>> reintegrated into trunk.  The merge of branch2 into trunk will have
>> svn:mergeinfo property references to both branch1 and branch2.  When
>> git-svn fetches the commit that merges branch2 (check_cherry_pick),
>> it is necessary to eliminate the merged contents of branch1 as well as
>> branch2, or else the merge will be incorrectly ignored as a cherry-pick.
>> 
>> Signed-off-by: Steven Walter <stevenrwalter@gmail.com>
>
> Thanks, I've Signed-off (also added Sam's Reviewed-by) and
> pushed to "for-git-master" on git://bogomips.org/git-svn
>
> (Will wait on a few other patches before asking Junio to pull)

If there are stuff that should go to 1.7.12 maintenance track, please separate them from new features and risky improvements.

Thanks.
Eric Wong· Aug 13, 2012, 07:08 UTC · re: Steven Walter · lore

Re: [PATCH v3 1/2] git-svn.perl: consider all ranges for a given merge, instead of only tip-by-tip

Steven Walter <stevenrwalter@gmail.com> wrote:
Show 10 quoted lines
> Consider the case where you have trunk, branchA of trunk, and branchB of
> branchA.  trunk is merged back into branchB, and then branchB is
> reintegrated into trunk.  The merge of branchB into trunk will have
> svn:mergeinfo property references to both branchA and branchB.  When
> performing the check_cherry_pick check on branchB, it is necessary to
> eliminate the merged contents of branchA as well as branchB, or else the
> merge will be incorrectly ignored as a cherry-pick.
> 
> Signed-off-by: Steven Walter <stevenrwalter@gmail.com>
> ---

I think this series is good, but would feel more comfortable if I got a second opinion from Sam.

This doesn't apply against Junio's master (nor mine on git://bogomips.org/git-svn.git), though it works fine on Junio's maint.

Show 112 quoted lines
>  git-svn.perl                                    |    8 ++-
>  t/t9163-git-svn-fetch-merge-branch-of-branch.sh |   60 +++++++++++++++++++++++
>  2 files changed, 63 insertions(+), 5 deletions(-)
>  create mode 100755 t/t9163-git-svn-fetch-merge-branch-of-branch.sh
> 
> diff --git a/git-svn.perl b/git-svn.perl
> index ca038ec..abcec11 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -3657,14 +3657,14 @@ sub find_extra_svn_parents {
>  	my @merge_tips;
>  	my $url = $self->{url};
>  	my $uuid = $self->ra_uuid;
> -	my %ranges;
> +	my @all_ranges;
>  	for my $merge ( @merges ) {
>  		my ($tip_commit, @ranges) =
>  			lookup_svn_merge( $uuid, $url, $merge );
>  		unless (!$tip_commit or
>  				grep { $_ eq $tip_commit } @$parents ) {
>  			push @merge_tips, $tip_commit;
> -			$ranges{$tip_commit} = \@ranges;
> +			push @all_ranges, @ranges;
>  		} else {
>  			push @merge_tips, undef;
>  		}
> @@ -3679,8 +3679,6 @@ sub find_extra_svn_parents {
>  		my $spec = shift @merges;
>  		next unless $merge_tip and $excluded{$merge_tip};
>  
> -		my $ranges = $ranges{$merge_tip};
> -
>  		# check out 'new' tips
>  		my $merge_base;
>  		eval {
> @@ -3702,7 +3700,7 @@ sub find_extra_svn_parents {
>  		my (@incomplete) = check_cherry_pick(
>  			$merge_base, $merge_tip,
>  			$parents,
> -			@$ranges,
> +			@all_ranges,
>  		       );
>  
>  		if ( @incomplete ) {
> diff --git a/t/t9163-git-svn-fetch-merge-branch-of-branch.sh b/t/t9163-git-svn-fetch-merge-branch-of-branch.sh
> new file mode 100755
> index 0000000..13ae7e3
> --- /dev/null
> +++ b/t/t9163-git-svn-fetch-merge-branch-of-branch.sh
> @@ -0,0 +1,60 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2012 Steven Walter
> +#
> +
> +test_description='git svn merge detection'
> +. ./lib-git-svn.sh
> +
> +svn_ver="$(svn --version --quiet)"
> +case $svn_ver in
> +0.* | 1.[0-4].*)
> +	skip_all="skipping git-svn test - SVN too old ($svn_ver)"
> +	test_done
> +	;;
> +esac
> +
> +test_expect_success 'initialize source svn repo' '
> +	svn_cmd mkdir -m x "$svnrepo"/trunk &&
> +	svn_cmd mkdir -m x "$svnrepo"/branches &&
> +	svn_cmd co "$svnrepo"/trunk "$SVN_TREE" &&
> +	(
> +		cd "$SVN_TREE" &&
> +		touch foo &&
> +		svn_cmd add foo &&
> +		svn_cmd commit -m "initial commit" &&
> +		svn_cmd cp -m branch "$svnrepo"/trunk "$svnrepo"/branches/branch1 &&
> +		svn_cmd switch "$svnrepo"/branches/branch1 &&
> +		touch bar &&
> +		svn_cmd add bar &&
> +		svn_cmd commit -m branch1 &&
> +		svn_cmd cp -m branch "$svnrepo"/branches/branch1 "$svnrepo"/branches/branch2 &&
> +		svn_cmd switch "$svnrepo"/branches/branch2 &&
> +		touch baz &&
> +		svn_cmd add baz &&
> +		svn_cmd commit -m branch2 &&
> +		svn_cmd switch "$svnrepo"/trunk &&
> +		touch bar2 &&
> +		svn_cmd add bar2 &&
> +		svn_cmd commit -m trunk &&
> +		svn_cmd switch "$svnrepo"/branches/branch2 &&
> +		svn_cmd merge "$svnrepo"/trunk &&
> +		svn_cmd commit -m "merge trunk"
> +		svn_cmd switch "$svnrepo"/trunk &&
> +		svn_cmd merge --reintegrate "$svnrepo"/branches/branch2 &&
> +		svn_cmd commit -m "merge branch2"
> +	) &&
> +	rm -rf "$SVN_TREE"
> +'
> +
> +test_expect_success 'clone svn repo' '
> +	git svn init -s "$svnrepo" &&
> +	git svn fetch
> +'
> +
> +test_expect_success 'verify merge commit' 'x=$(git rev-parse HEAD^2) &&
> +	y=$(git rev-parse branch2) &&
> +	test "x$x" = "x$y"
> +'
> +
> +test_done
> -- 
> 1.7.9.5
Sam Vilain· Aug 18, 2012, 19:49 UTC · re: Steven Walter · lore

Re: [PATCH v3 1/2] git-svn.perl: consider all ranges for a given merge, instead of only tip-by-tip

On 08/11/2012 10:14 AM, Steven Walter wrote:
> Consider the case where you have trunk, branchA of trunk, and branchB of
> branchA.  trunk is merged back into branchB, and then branchB is
> reintegrated into trunk.  The merge of branchB into trunk will have
> svn:mergeinfo property references to both branchA and branchB.
Nice start, and then...
>    When
> performing the check_cherry_pick check on branchB,

You switch from a user story to an implementation detail. Perhaps you could state the user action which is affected here and mention the implementation details in parentheses.

Show 48 quoted lines
>   it is necessary to
> eliminate the merged contents of branchA as well as branchB, or else the
> merge will be incorrectly ignored as a cherry-pick.
>
> Signed-off-by: Steven Walter <stevenrwalter@gmail.com>
> ---
>   git-svn.perl                                    |    8 ++-
>   t/t9163-git-svn-fetch-merge-branch-of-branch.sh |   60 +++++++++++++++++++++++
>   2 files changed, 63 insertions(+), 5 deletions(-)
>   create mode 100755 t/t9163-git-svn-fetch-merge-branch-of-branch.sh
>
> diff --git a/git-svn.perl b/git-svn.perl
> index ca038ec..abcec11 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -3657,14 +3657,14 @@ sub find_extra_svn_parents {
>   	my @merge_tips;
>   	my $url = $self->{url};
>   	my $uuid = $self->ra_uuid;
> -	my %ranges;
> +	my @all_ranges;
>   	for my $merge ( @merges ) {
>   		my ($tip_commit, @ranges) =
>   			lookup_svn_merge( $uuid, $url, $merge );
>   		unless (!$tip_commit or
>   				grep { $_ eq $tip_commit } @$parents ) {
>   			push @merge_tips, $tip_commit;
> -			$ranges{$tip_commit} = \@ranges;
> +			push @all_ranges, @ranges;
>   		} else {
>   			push @merge_tips, undef;
>   		}
> @@ -3679,8 +3679,6 @@ sub find_extra_svn_parents {
>   		my $spec = shift @merges;
>   		next unless $merge_tip and $excluded{$merge_tip};
>   
> -		my $ranges = $ranges{$merge_tip};
> -
>   		# check out 'new' tips
>   		my $merge_base;
>   		eval {
> @@ -3702,7 +3700,7 @@ sub find_extra_svn_parents {
>   		my (@incomplete) = check_cherry_pick(
>   			$merge_base, $merge_tip,
>   			$parents,
> -			@$ranges,
> +			@all_ranges,
>   		       );
This all looks very plausibly correct.
Show 34 quoted lines
>   		if ( @incomplete ) {
> diff --git a/t/t9163-git-svn-fetch-merge-branch-of-branch.sh b/t/t9163-git-svn-fetch-merge-branch-of-branch.sh
> new file mode 100755
> index 0000000..13ae7e3
> --- /dev/null
> +++ b/t/t9163-git-svn-fetch-merge-branch-of-branch.sh
> @@ -0,0 +1,60 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2012 Steven Walter
> +#
> +
> +test_description='git svn merge detection'
> +. ./lib-git-svn.sh
> +
> +svn_ver="$(svn --version --quiet)"
> +case $svn_ver in
> +0.* | 1.[0-4].*)
> +	skip_all="skipping git-svn test - SVN too old ($svn_ver)"
> +	test_done
> +	;;
> +esac
> +
> +test_expect_success 'initialize source svn repo' '
> +	svn_cmd mkdir -m x "$svnrepo"/trunk &&
> +	svn_cmd mkdir -m x "$svnrepo"/branches &&
> +	svn_cmd co "$svnrepo"/trunk "$SVN_TREE" &&
> +	(
> +		cd "$SVN_TREE" &&
> +		touch foo &&
> +		svn_cmd add foo &&
> +		svn_cmd commit -m "initial commit" &&
> +		svn_cmd cp -m branch "$svnrepo"/trunk "$svnrepo"/branches/branch1 &&
> +		svn_cmd switch "$svnrepo"/branches/branch1 &&

You refer to these as "branchA" and "branchB" in your commit message. It would be easier to follow if you were consistent.

Otherwise, nice fix. I'm glad that someone is finding the corner cases in this code! :)

Sam

← back to recent threads