Thread (11 messages) flat view 11 messages, 3 authors, 2016-06-15

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

From: Sam Vilain <hidden>
Date: 2016-06-15 22:54:30

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.
quoted hunk ↗ jump to hunk
  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 <redacted>
---
  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.
quoted hunk ↗ jump to hunk
  		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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help