threads / patch / 27647

patch, 3 partsgit-svn: Handle git root commits in mergeinfo ranges

Subject: [PATCH 0/3] git-svn: Handle git root commits in mergeinfo ranges

## tl;dr

5 messages between Jun 18, 2011 and Jun 28, 2011. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Michael Haggerty· Jun 18, 2011, 06:47 UTC · lore

The maint version of git-svn fails if there is a svn:mergeinfo range that includes a commit that has no parent; for example, "/trunk:1-59". Similar problems have been discussed before [1,2].

The problem in this case is that git-svn tries to obtain the list of git revisions brought into the branch by this range by running

    git rev-list "$bottom_commit^..$top_commit"

where $bottom_commit is the SHA1 of the git commit corresponding to the first commit in the mergeinfo range. But $bottom_commit is a root commit and does not have a parent.

As noted in [3], the obvious solution,
    git rev-list "$bottom_commit^!" "$top_commit"

, causes breakage in test 5 of t9151-svn-mergeinfo.sh. The problem is that it is important to exclude only those commits reachable from the *first* parent of $bottom_commit, not *all* parents.

Instead, this patch simply uses "git rev-parse" to determine whether $bottom_commit has any parents at all. If not, then the following command is used instead:

    git rev-list "$top_commit"

I don't believe the extra "git rev-parse" invocation to be a performance problem, but I am not familiar enough with the code to be certain.

Michael

[1] http://thread.gmane.org/gmane.comp.version-control.git/146255 [2] http://thread.gmane.org/gmane.comp.version-control.git/171248 [3] http://article.gmane.org/gmane.comp.version-control.git/150416

Michael Haggerty (3):
  git-svn: Demonstrate a bug with root commits in mergeinfo ranges
  git-svn: Disambiguate rev-list arguments to improve error message
  git-svn: Correctly handle root commits in mergeinfo ranges
 git-svn.perl                           |   11 ++++++---
 t/t9159-git-svn-no-parent-mergeinfo.sh |   33 ++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 4 deletions(-)
 create mode 100755 t/t9159-git-svn-no-parent-mergeinfo.sh
-- 
1.7.5.4
Michael Haggerty· Jun 18, 2011, 06:47 UTC · re: Michael Haggerty · lore

[PATCH 1/3] git-svn: Demonstrate a bug with root commits in mergeinfo ranges

If a svn:mergeinfo range starts at a commit that was converted as a git root commit (e.g., r1 or a branch that was created out of thin air), then there is an error when git-svn tries to run

    git rev-list "$bottom_commit^..$top_commit"

because $bottom_commit (the git commit corresponding to r1) has no parent.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 t/t9159-git-svn-no-parent-mergeinfo.sh |   33 ++++++++++++++++++++++++++++++++
 1 files changed, 33 insertions(+), 0 deletions(-)
 create mode 100755 t/t9159-git-svn-no-parent-mergeinfo.sh
Show changes to t/t9159-git-svn-no-parent-mergeinfo.sh +33 −0
diff --git a/t/t9159-git-svn-no-parent-mergeinfo.sh b/t/t9159-git-svn-no-parent-mergeinfo.sh
new file mode 100755
index 0000000..9472de3
--- /dev/null
+++ b/t/t9159-git-svn-no-parent-mergeinfo.sh
@@ -0,0 +1,33 @@
+#!/bin/sh
+test_description='git svn handling of root commits in merge ranges'
+. ./lib-git-svn.sh
+
+test_expect_failure 'test handling of root commits in merge ranges' '
+	mkdir -p init/trunk init/branches init/tags &&
+	echo "r1" > init/trunk/file.txt &&
+	svn_cmd import -m "initial import" init "$svnrepo" &&
+	svn_cmd co "$svnrepo" tmp &&
+	(
+		cd tmp &&
+		echo "r2" > trunk/file.txt &&
+		svn_cmd commit -m "Modify file.txt on trunk" &&
+		svn_cmd cp trunk@1 branches/a &&
+		svn_cmd commit -m "Create branch a from trunk r1" &&
+		svn_cmd propset svn:mergeinfo /trunk:1-2 branches/a &&
+		svn_cmd commit -m "Fake merge of trunk r2 into branch a" &&
+		mkdir branches/b &&
+		echo "r5" > branches/b/file2.txt &&
+		svn_cmd add branches/b &&
+		svn_cmd commit -m "Create branch b from thin air" &&
+		echo "r6" > branches/b/file2.txt &&
+		svn_cmd commit -m "Modify file2.txt on branch b" &&
+		svn_cmd cp branches/b@5 branches/c &&
+		svn_cmd commit -m "Create branch c from branch b r5" &&
+		svn_cmd propset svn:mergeinfo /branches/b:5-6 branches/c &&
+		svn_cmd commit -m "Fake merge of branch b r6 into branch c"
+	) &&
+	git svn init -s "$svnrepo" &&
+	git svn fetch
+	'
+
+test_done

-- 
1.7.5.4
Michael Haggerty· Jun 18, 2011, 06:47 UTC · re: Michael Haggerty · lore

[PATCH 2/3] git-svn: Disambiguate rev-list arguments to improve error message

Add "--" in the "git rev-list" command line so that if there is a bug and the revisions cannot be found, the error message is a bit less cryptic.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 git-svn.perl |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Show changes to git-svn.perl +2 −2
diff --git a/git-svn.perl b/git-svn.perl
index da3fea8..2cc9242 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -3141,9 +3141,9 @@ sub check_cherry_pick {
 	my $parents = shift;
 	my @ranges = @_;
 	my %commits = map { $_ => 1 }
-		_rev_list("--no-merges", $tip, "--not", $base, @$parents);
+		_rev_list("--no-merges", $tip, "--not", $base, @$parents, "--");
 	for my $range ( @ranges ) {
-		delete @commits{_rev_list($range)};
+		delete @commits{_rev_list($range, "--")};
 	}
 	for my $commit (keys %commits) {
 		if (has_no_changes($commit)) {

-- 
1.7.5.4
Michael Haggerty· Jun 18, 2011, 06:48 UTC · re: Michael Haggerty · lore

[PATCH 3/3] git-svn: Correctly handle root commits in mergeinfo ranges

If the bottom of a mergeinfo range is a commit that maps to a git root commit, then it doesn't have a parent. In such a case, use git commit range "$top_commit" rather than "$bottom_commit^..$top_commit".

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 git-svn.perl                           |    7 +++++--
 t/t9159-git-svn-no-parent-mergeinfo.sh |    2 +-
 2 files changed, 6 insertions(+), 3 deletions(-)
Show changes to 2 files +6 −3

git-svn.perl, t/t9159-git-svn-no-parent-mergeinfo.sh

diff --git a/git-svn.perl b/git-svn.perl
index 2cc9242..7e121ae 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -3111,8 +3111,11 @@ sub lookup_svn_merge {
 			next;
 		}
 
-		push @merged_commit_ranges,
-			"$bottom_commit^..$top_commit";
+		if (scalar(command('rev-parse', "$bottom_commit^@"))) {
+			push @merged_commit_ranges, "$bottom_commit^..$top_commit";
+		} else {
+			push @merged_commit_ranges, "$top_commit";
+		}
 
 		if ( !defined $tip or $top > $tip ) {
 			$tip = $top;
diff --git a/t/t9159-git-svn-no-parent-mergeinfo.sh b/t/t9159-git-svn-no-parent-mergeinfo.sh
index 9472de3..85120b7 100755
--- a/t/t9159-git-svn-no-parent-mergeinfo.sh
+++ b/t/t9159-git-svn-no-parent-mergeinfo.sh
@@ -2,7 +2,7 @@
 test_description='git svn handling of root commits in merge ranges'
 . ./lib-git-svn.sh
 
-test_expect_failure 'test handling of root commits in merge ranges' '
+test_expect_success 'test handling of root commits in merge ranges' '
 	mkdir -p init/trunk init/branches init/tags &&
 	echo "r1" > init/trunk/file.txt &&
 	svn_cmd import -m "initial import" init "$svnrepo" &&

-- 
1.7.5.4
Eric Wong· Jun 28, 2011, 03:37 UTC · re: Michael Haggerty · lore

Re: [PATCH 0/3] git-svn: Handle git root commits in mergeinfo ranges

Michael Haggerty <mhagger@alum.mit.edu> wrote:
> I don't believe the extra "git rev-parse" invocation to be a
> performance problem, but I am not familiar enough with the code to be
> certain.

It shouldn't be a problem. Sam Vilain did all the merge tracking code, but this looks entirely reasonable.

> Michael Haggerty (3):
>   git-svn: Demonstrate a bug with root commits in mergeinfo ranges
>   git-svn: Disambiguate rev-list arguments to improve error message
>   git-svn: Correctly handle root commits in mergeinfo ranges
Thanks Michael.

All acked and pushed to "master" for Junio at git://bogomips.org/git-svn These should go to maint, too.

-- 
Eric Wong

← back to recent threads