git-svn: SVK merge commits can have >2 parents

git-svn: SVK merge commits can have >2 parents

[Alex Vandiver]

[-- Attachment #1: Type: text/plain, Size: 1703 bytes --]

  Heya,

While converting a mildly complicated svn repository that was managed
with SVK, I ran across the following oddness.  `svk smerge` can only
merge between _two_ branches at once -- however, the way that svk
merge detection works, you can end up with erroneous extra parents
from long-dead branches.  Case in point:

    export SVKROOT=/tmp/svk-testing-$$
    svk mkdir //trunk -m 'trunk'
    svk mkdir //branches -m 'branches'
    svk co //trunk
    svk cp //trunk/ //branches/feature1 -m 'branch for feature1'
    svk cp //trunk/ //branches/feature2 -m 'branch for feature2'
    svk co //branches/feature1
    cd feature1
    echo "foo" >foo
    svk add foo
    svk ci -m 'feature1 development'
    svk sm //branches/feature1 //trunk -m 'merge feature1 to trunk'
    cd ..
    svk co //branches/feature2
    cd feature2
    echo "bar" >bar
    svk add bar
    svk ci -m 'feature2 development'
    svk sm //trunk //branches/feature2 -m 'merge from trunk'
    cd ..

    git svn clone -s file://$SVKROOT/local

The 'feature2' branch will appear to have three parents: r7, r6, _and_
r5.  The r5 parent is extraneous, and only appears because r5 was
previously merged into trunk, as part of r6.

Given this, I'm a little confused why find_extra_svk_parents is
written the way that it is, in seemingly allowing multiple extra
parents to be found.  Since the most recent (i.e., highest-numbered)
change is by definition the only one that can account for all of the
other svk:merge changes, I _believe_ the attached diff to be correct,
but I'm unsure because of the implicit current assumption that smerges
can produce multiple parents.

 - Alex
-- 
Networking -- only one letter away from not working

[-- Attachment #2: 0001-git-svn-svk-merge-commits-should-only-add-one-exta-p.patch --]
[-- Type: application/octet-stream, Size: 1400 bytes --]

From b86b4bb0cecb70d828558b046aa0b3e7b899bf41 Mon Sep 17 00:00:00 2001
From: Alex Vandiver <alex@chmrr.net>
Date: Sun, 29 Nov 2009 02:20:21 -0500
Subject: [PATCH] git-svn: svk merge commits should only add one exta parent

When merging branches based on svk:merge properties, a single merge
can have updated or added multiple svk:merge lines.  If so. only
include the minimal set of new parents.

Signed-off-by: Alex Vandiver <alex@chmrr.net>
---
 git-svn.perl |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 957d44e..668e50c 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -2940,10 +2940,12 @@ sub find_extra_svk_parents {
 			if ( my $commit = $gs->rev_map_get($rev, $uuid) ) {
 				# wahey!  we found it, but it might be
 				# an old one (!)
-				push @known_parents, $commit;
+				push @known_parents, [ $rev, $commit ];
 			}
 		}
 	}
+	# We only care about the highest-numbered applicable commit
+	@known_parents = map {$_->[1]} sort {$b->[0] <=> $a->[0]} @known_parents;
 	for my $parent ( @known_parents ) {
 		my @cmd = ('rev-list', $parent, map { "^$_" } @$parents );
 		my ($msg_fh, $ctx) = command_output_pipe(@cmd);
@@ -2956,6 +2958,7 @@ sub find_extra_svk_parents {
 			print STDERR
 			    "Found merge parent (svk:merge ticket): $parent\n";
 			push @$parents, $parent;
+			return;
 		}
 	}
 }
-- 
1.6.6.rc0.254.g7352d

Re: git-svn: SVK merge commits can have >2 parents

[Alex Vandiver]

[-- Attachment #1: Type: text/plain, Size: 1001 bytes --]

At Sun Nov 29 02:28:39 -0500 2009, Alex Vandiver wrote:
> While converting a mildly complicated svn repository that was managed
> with SVK, I ran across the following oddness.  `svk smerge` can only
> merge between _two_ branches at once -- however, the way that svk
> merge detection works, you can end up with erroneous extra parents
> from long-dead branches.

Upon a little more inspection, I now understand that the rev-parse
lines in find_extra_svk_parents are attempting to deal with this exact
circumstance -- but they fail to properly sort the merge tickets
first, which leads to this incorrect behavior.  Armed with this
understanding, I'm more confident in the attached updated patch.  I
assume, however, that the logic allows for more than one extra parent
only because such an occurrance could be constructed by hand-editing
svk:merge, because AFAIK svk's command-line tools should be able to
construct such a circumstance.
 - Alex
-- 
Networking -- only one letter away from not working

[-- Attachment #2: 0001-git-svn-sort-svk-merge-tickets-to-account-for-minima.patch --]
[-- Type: application/octet-stream, Size: 1369 bytes --]

From 4d30e57e5da7c2e880908bc742cf80990d6f9f5d Mon Sep 17 00:00:00 2001
From: Alex Vandiver <alex@chmrr.net>
Date: Sun, 29 Nov 2009 02:20:21 -0500
Subject: [PATCH] git-svn: sort svk merge tickets to account for minimal parents

When merging branches based on svk:merge properties, a single merge
can have updated or added multiple svk:merge lines.  Attempt to
include the minimal set of parents by sorting the merge properties in
order of revision, highest to lowest.

Signed-off-by: Alex Vandiver <alex@chmrr.net>
---
 git-svn.perl |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 957d44e..51f03ad 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -2940,10 +2940,14 @@ sub find_extra_svk_parents {
 			if ( my $commit = $gs->rev_map_get($rev, $uuid) ) {
 				# wahey!  we found it, but it might be
 				# an old one (!)
-				push @known_parents, $commit;
+				push @known_parents, [ $rev, $commit ];
 			}
 		}
 	}
+	# Ordering matters; highest-numbered commit merge tickets
+	# first, as they may account for later merge ticket additions
+	# or changes.
+	@known_parents = map {$_->[1]} sort {$b->[0] <=> $a->[0]} @known_parents;
 	for my $parent ( @known_parents ) {
 		my @cmd = ('rev-list', $parent, map { "^$_" } @$parents );
 		my ($msg_fh, $ctx) = command_output_pipe(@cmd);
-- 
1.6.6.rc0.254.g7352d

Re: git-svn: SVK merge commits can have >2 parents

[Eric Wong]

Alex Vandiver <alex@chmrr.net> wrote:
> At Sun Nov 29 02:28:39 -0500 2009, Alex Vandiver wrote:
> > While converting a mildly complicated svn repository that was managed
> > with SVK, I ran across the following oddness.  `svk smerge` can only
> > merge between _two_ branches at once -- however, the way that svk
> > merge detection works, you can end up with erroneous extra parents
> > from long-dead branches.
> 
> Upon a little more inspection, I now understand that the rev-parse
> lines in find_extra_svk_parents are attempting to deal with this exact
> circumstance -- but they fail to properly sort the merge tickets
> first, which leads to this incorrect behavior.  Armed with this
> understanding, I'm more confident in the attached updated patch.  I

Hi Alex, Sam,

I'll defer to Sam for the Ack, my svk knowledge is limited. Thanks.

-- 
Eric Wong

Re: [spf:guess] Re: git-svn: SVK merge commits can have >2 parents

[Sam Vilain]

On Sun, 2009-11-29 at 08:08 +0000, Eric Wong wrote:
> Alex Vandiver <alex@chmrr.net> wrote:
> > At Sun Nov 29 02:28:39 -0500 2009, Alex Vandiver wrote:
> > > While converting a mildly complicated svn repository that was managed
> > > with SVK, I ran across the following oddness.  `svk smerge` can only
> > > merge between _two_ branches at once -- however, the way that svk
> > > merge detection works, you can end up with erroneous extra parents
> > > from long-dead branches.
> > 
> > Upon a little more inspection, I now understand that the rev-parse
> > lines in find_extra_svk_parents are attempting to deal with this exact
> > circumstance -- but they fail to properly sort the merge tickets
> > first, which leads to this incorrect behavior.  Armed with this
> > understanding, I'm more confident in the attached updated patch.  I
> 
> Hi Alex, Sam,
> 
> I'll defer to Sam for the Ack, my svk knowledge is limited. Thanks.

Yes, the change does make sense to me - nicely done, Alex.

Acked-By: Sam Vilain <sam@vilain.net>

Sam

Re: [spf:guess] Re: git-svn: SVK merge commits can have >2 parents

[Eric Wong]

Sam Vilain <sam@vilain.net> wrote:
> On Sun, 2009-11-29 at 08:08 +0000, Eric Wong wrote:
> > Alex Vandiver <alex@chmrr.net> wrote:
> > > At Sun Nov 29 02:28:39 -0500 2009, Alex Vandiver wrote:
> > > > While converting a mildly complicated svn repository that was managed
> > > > with SVK, I ran across the following oddness.  `svk smerge` can only
> > > > merge between _two_ branches at once -- however, the way that svk
> > > > merge detection works, you can end up with erroneous extra parents
> > > > from long-dead branches.
> > > 
> > > Upon a little more inspection, I now understand that the rev-parse
> > > lines in find_extra_svk_parents are attempting to deal with this exact
> > > circumstance -- but they fail to properly sort the merge tickets
> > > first, which leads to this incorrect behavior.  Armed with this
> > > understanding, I'm more confident in the attached updated patch.  I
> > 
> > Hi Alex, Sam,
> > 
> > I'll defer to Sam for the Ack, my svk knowledge is limited. Thanks.
> 
> Yes, the change does make sense to me - nicely done, Alex.
> 
> Acked-By: Sam Vilain <sam@vilain.net>

Thanks Sam, acked and pushed to git://git.bogomips.org/git-svn

-- 
Eric Wong