threads / patch / 21598

patch, 2 partsgit-svn: handle SVN merges from revisions past the tip of the branch

Subject: [PATCH 2/2] git-svn: handle SVN merges from revisions past the tip of the branch

## tl;dr

3 messages between Nov 12, 2009 and Nov 13, 2009. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Toby Allsopp· Nov 12, 2009, 20:18 UTC · lore

When recording the revisions that it has merged, SVN sets the top revision to be the latest revision in the repository, which is not necessarily a revision on the branch that is being merged from. When it is not on the branch, git-svn fails to add the extra parent to represent the merge because it relies on finding the commit on the branch that corresponds to the top of the SVN merge range.

In order to correctly handle this case, we look for the maximum revision less than or equal to the top of the SVN merge range that is actually on the branch being merged from.

Signed-off-by: Toby Allsopp <toby.allsopp@navman.co.nz>
---
 git-svn.perl             |    7 +++++--
 t/t9151-svn-mergeinfo.sh |    2 +-
 2 files changed, 6 insertions(+), 3 deletions(-)

Show changes to 2 files +6 −3

git-svn.perl, t/t9151-svn-mergeinfo.sh

diff --git a/git-svn.perl b/git-svn.perl
index 6a3b501..27fbe30 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -2950,8 +2950,11 @@ sub find_extra_svn_parents {
 			my $bottom_commit =
 				$gs->rev_map_get($bottom, $self->ra_uuid) ||
 				$gs->rev_map_get($bottom+1, $self->ra_uuid);
-			my $top_commit =
-				$gs->rev_map_get($top, $self->ra_uuid);
+			my $top_commit;
+			for (; !$top_commit && $top >= $bottom; --$top) {
+				$top_commit =
+					$gs->rev_map_get($top, $self->ra_uuid);
+			}
 
 			unless ($top_commit and $bottom_commit) {
 				warn "W:unknown path/rev in svn:mergeinfo "
diff --git a/t/t9151-svn-mergeinfo.sh b/t/t9151-svn-mergeinfo.sh
index 0d42c84..f57daf4 100755
--- a/t/t9151-svn-mergeinfo.sh
+++ b/t/t9151-svn-mergeinfo.sh
@@ -19,7 +19,7 @@ test_expect_success 'represent svn merges without intervening commits' "
 	[ `git cat-file commit HEAD^1 | grep parent | wc -l` -eq 2 ]
 	"
 
-test_expect_failure 'represent svn merges with intervening commits' "
+test_expect_success 'represent svn merges with intervening commits' "
 	[ `git cat-file commit HEAD | grep parent | wc -l` -eq 2 ]
 	"

-- 
1.6.5.2.155.gbb47.dirty

Sam Vilain· Nov 13, 2009, 00:47 UTC · re: Toby Allsopp · lore

Re: [PATCH 2/2] git-svn: handle SVN merges from revisions past the tip of the branch

Toby Allsopp wrote:

Show 6 quoted lines

> When recording the revisions that it has merged, SVN sets the top
> revision to be the latest revision in the repository, which is not
> necessarily a revision on the branch that is being merged from.  When
> it is not on the branch, git-svn fails to add the extra parent to
> represent the merge because it relies on finding the commit on the
> branch that corresponds to the top of the SVN merge range.

I thought, "that sounds like he's repeating himself, wait a sec..."

> -test_expect_failure 'represent svn merges with intervening commits' "
> +test_expect_success 'represent svn merges with intervening commits' "
>  	[ `git cat-file commit HEAD | grep parent | wc -l` -eq 2 ]
>  	"

So you made a failing test and then added the implementation for it? Interesting strategy :). I'd probably not repeat the same sentence twice though.

Thanks for contributing this. There might be other bugs too, especially when upstream has a more complicated merge hierarchy ... apparently svn tends to get it wrong, so checking for all commits might not work in that case.

It would be nice if "dcommit" could make these commits, too...

Sam

Toby Allsopp· Nov 13, 2009, 01:03 UTC · re: Sam Vilain · lore

Re: [PATCH 2/2] git-svn: handle SVN merges from revisions past the tip of the branch

On Fri, Nov 13 2009, Sam Vilain wrote:

Show 9 quoted lines

> Toby Allsopp wrote:
> > When recording the revisions that it has merged, SVN sets the top
> > revision to be the latest revision in the repository, which is not
> > necessarily a revision on the branch that is being merged from.  When
> > it is not on the branch, git-svn fails to add the extra parent to
> > represent the merge because it relies on finding the commit on the
> > branch that corresponds to the top of the SVN merge range.
>
> I thought, "that sounds like he's repeating himself, wait a sec..."

Hmm, it makes perfect sense to me :-) Does the explanation in 1/2 make more sense?

The first sentence describes what Subversion does, the second what git-svn does in response.

> Thanks for contributing this.  There might be other bugs too, especially
> when upstream has a more complicated merge hierarchy ... apparently svn
> tends to get it wrong, so checking for all commits might not work in
> that case.

Oh yes, SVN gets the merges wrong in an alarming number of cases, it's really shocking. I only stay sane at work because I tell myself that SVN is making the case for git for me :-)

> It would be nice if "dcommit" could make these commits, too...

Yes.

Toby.