git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] git-svn: Handle git root commits in mergeinfo ranges
@ 2011-06-18  6:47 Michael Haggerty
  2011-06-18  6:47 ` [PATCH 1/3] git-svn: Demonstrate a bug with " Michael Haggerty
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Michael Haggerty @ 2011-06-18  6:47 UTC (permalink / raw)
  To: git; +Cc: gitster, bert.wesarg, normalperson, git, Michael Haggerty

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/3] git-svn: Demonstrate a bug with root commits in mergeinfo ranges
  2011-06-18  6:47 [PATCH 0/3] git-svn: Handle git root commits in mergeinfo ranges Michael Haggerty
@ 2011-06-18  6:47 ` Michael Haggerty
  2011-06-18  6:47 ` [PATCH 2/3] git-svn: Disambiguate rev-list arguments to improve error message Michael Haggerty
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Michael Haggerty @ 2011-06-18  6:47 UTC (permalink / raw)
  To: git; +Cc: gitster, bert.wesarg, normalperson, git, Michael Haggerty

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

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/3] git-svn: Disambiguate rev-list arguments to improve error message
  2011-06-18  6:47 [PATCH 0/3] git-svn: Handle git root commits in mergeinfo ranges Michael Haggerty
  2011-06-18  6:47 ` [PATCH 1/3] git-svn: Demonstrate a bug with " Michael Haggerty
@ 2011-06-18  6:47 ` Michael Haggerty
  2011-06-18  6:48 ` [PATCH 3/3] git-svn: Correctly handle root commits in mergeinfo ranges Michael Haggerty
  2011-06-28  3:37 ` [PATCH 0/3] git-svn: Handle git " Eric Wong
  3 siblings, 0 replies; 5+ messages in thread
From: Michael Haggerty @ 2011-06-18  6:47 UTC (permalink / raw)
  To: git; +Cc: gitster, bert.wesarg, normalperson, git, Michael Haggerty

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(-)

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 3/3] git-svn: Correctly handle root commits in mergeinfo ranges
  2011-06-18  6:47 [PATCH 0/3] git-svn: Handle git root commits in mergeinfo ranges Michael Haggerty
  2011-06-18  6:47 ` [PATCH 1/3] git-svn: Demonstrate a bug with " Michael Haggerty
  2011-06-18  6:47 ` [PATCH 2/3] git-svn: Disambiguate rev-list arguments to improve error message Michael Haggerty
@ 2011-06-18  6:48 ` Michael Haggerty
  2011-06-28  3:37 ` [PATCH 0/3] git-svn: Handle git " Eric Wong
  3 siblings, 0 replies; 5+ messages in thread
From: Michael Haggerty @ 2011-06-18  6:48 UTC (permalink / raw)
  To: git; +Cc: gitster, bert.wesarg, normalperson, git, Michael Haggerty

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(-)

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/3] git-svn: Handle git root commits in mergeinfo ranges
  2011-06-18  6:47 [PATCH 0/3] git-svn: Handle git root commits in mergeinfo ranges Michael Haggerty
                   ` (2 preceding siblings ...)
  2011-06-18  6:48 ` [PATCH 3/3] git-svn: Correctly handle root commits in mergeinfo ranges Michael Haggerty
@ 2011-06-28  3:37 ` Eric Wong
  3 siblings, 0 replies; 5+ messages in thread
From: Eric Wong @ 2011-06-28  3:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Michael Haggerty, git, bert.wesarg, git

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2011-06-28  3:39 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-06-18  6:47 [PATCH 0/3] git-svn: Handle git root commits in mergeinfo ranges Michael Haggerty
2011-06-18  6:47 ` [PATCH 1/3] git-svn: Demonstrate a bug with " Michael Haggerty
2011-06-18  6:47 ` [PATCH 2/3] git-svn: Disambiguate rev-list arguments to improve error message Michael Haggerty
2011-06-18  6:48 ` [PATCH 3/3] git-svn: Correctly handle root commits in mergeinfo ranges Michael Haggerty
2011-06-28  3:37 ` [PATCH 0/3] git-svn: Handle git " Eric Wong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).