Skip to contentSkip to conversation

git svn dcommit not checking if up-to-date?

PATCH New Johannes Schindelin 4 participants 11 messages Jun 15, 2016

lore
Share & tools
lore
#0 lore raw

Hi Eric,

harningt just asked about known issues of git-svn on IRC, and I remembered that I had an issue: Accidentally, I forgot to "git svn fetch" before "git svn dcommit"ing, and unfortunately, a colleague had just checked in a change, which got undone by my dcommit.

Is this a known issue, has it been fixed, am I a bad pilot?

Ciao,
Dscho

P.S.: harningt promised to get his hands dirty, but I just realised that the issue could be fixed since long ago...

#1 lore raw

I believe I've had this same issue as well. Like you, I assumed it was user error on my part - I had been doing a lot of 'rebase -i' with that particular set of changes, and figured I had just messed up somehow.

In my case, my coworker's commit happened 15 minutes before my dcommit, so it wasn't *that* close in time. I tried to reproduce at that point, but was unable to. I may try to reproduce this weekend if I get a chance.

Quoted 18 lines · Johannes Schindelin

Hi Eric,

harningt just asked about known issues of git-svn on IRC, and I remembered that I had an issue: Accidentally, I forgot to "git svn fetch" before "git svn dcommit"ing, and unfortunately, a colleague had just checked in a change, which got undone by my dcommit.

Is this a known issue, has it been fixed, am I a bad pilot?

Ciao,
Dscho

P.S.: harningt promised to get his hands dirty, but I just realised that the issue could be fixed since long ago...
-
To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 3 lines
Dave Watson
Software Engineer
MIMvista Corp
#2 lore raw
In reply to Johannes Schindelin
Quoted 7 lines · Johannes Schindelin

Hi Eric,

harningt just asked about known issues of git-svn on IRC, and I remembered that I had an issue: Accidentally, I forgot to "git svn fetch" before "git svn dcommit"ing, and unfortunately, a colleague had just checked in a change, which got undone by my dcommit.

Quoted 7 lines

Is this a known issue, has it been fixed, am I a bad pilot?

Ciao,
Dscho

P.S.: harningt promised to get his hands dirty, but I just realised that the issue could be fixed since long ago...

I believe this was fixed a while back in commit
45bf473a7bc2c40c8aea3d34a0eab7a41e77a8ff
(Thu Nov 9 01:19:37 2006 -0800).

I just tested this on a live server, and it seems to be correctly erroring out if the local files are not up-to-date.

The only currently known issues that could break commits is from using merge or pull from another svn branch and trying to dcommit the result (see CAVEATS section in the manpage). I hope to fix this soon (hoping somebody beats me to it, of course)

Otherwise there's still open bugs on svn:// fetch connections dying and permissions issues with some DAV repos, neither of which can cause data corruption on either side.

Signature · 1 line
Eric Wong
#3 lore raw

Hi,

Quoted 10 lines · Eric Wong
Quoted 5 lines · Johannes Schindelin

harningt just asked about known issues of git-svn on IRC, and I remembered that I had an issue: Accidentally, I forgot to "git svn fetch" before "git svn dcommit"ing, and unfortunately, a colleague had just checked in a change, which got undone by my dcommit.

I believe this was fixed a while back in commit
45bf473a7bc2c40c8aea3d34a0eab7a41e77a8ff
(Thu Nov 9 01:19:37 2006 -0800).

That is strange, since I had this issue in July or August (this year). And I am quite certain that I ran with pretty up-to-date git (I usually track "next" quite closely).

Ciao,
Dscho

#4 lore raw

I just reproduced this one in a live repository. Here's what you do:

$ git checkout -b breakme trunk
$ vi file1.txt
$ git-commit -a -m 'first change'
$ vi file2.txt
$ git-commit -a -m 'second change'
..... Full moon, become a werewolf ......
C:\svnrepo> edit file2.txt
C:\svnrepo> svn commit -m 'this will be gone'
..... Become yourself again ....
$ git svn fetch --all # (not sure if this is necessary)
$ git svn dcommit
$ git log -p

The change to file2 by your hairier, fanged self will be gone. The critical thing is that you must dcommit *multiple* commits, and the first one can't be the conflicting file, otherwise it will stop. At the time the first commit of the dcommit has gone through, git-svn now thinks it's all up-to-date.

Quoted 26 lines · Johannes Schindelin

Hi,

Quoted 10 lines · Eric Wong
Quoted 5 lines · Johannes Schindelin

harningt just asked about known issues of git-svn on IRC, and I remembered that I had an issue: Accidentally, I forgot to "git svn fetch" before "git svn dcommit"ing, and unfortunately, a colleague had just checked in a change, which got undone by my dcommit.

I believe this was fixed a while back in commit
45bf473a7bc2c40c8aea3d34a0eab7a41e77a8ff
(Thu Nov 9 01:19:37 2006 -0800).

That is strange, since I had this issue in July or August (this year). And I am quite certain that I ran with pretty up-to-date git (I usually track "next" quite closely).

Ciao,
Dscho

-
To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 3 lines
Dave Watson
Software Engineer
MIMvista Corp
#5 lore raw

PATCH 1/1 Test for git-svn dcommit clobbering changes

If you make multiple commits, and a commit other than the first applies to the same file as a new change in the SVN repository, git-svn will blow away the other commit's changes.

---

Quoted 14 lines · David Watson

I just reproduced this one in a live repository. Here's what you do:

$ git checkout -b breakme trunk
$ vi file1.txt
$ git-commit -a -m 'first change'
$ vi file2.txt
$ git-commit -a -m 'second change'
..... Full moon, become a werewolf ......
C:\svnrepo> edit file2.txt
C:\svnrepo> svn commit -m 'this will be gone'
..... Become yourself again ....
$ git svn fetch --all # (not sure if this is necessary)
$ git svn dcommit
$ git log -p
 t/t9106-git-svn-commit-diff-clobber.sh |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
t/t9106-git-svn-commit-diff-clobber.sh +3 -0
index 6f132f2..2706153 100755
@@ -50,6 +50,9 @@ test_expect_failure 'dcommit fails to commit because of conflict' "
 	svn commit -m 'fourth line from svn' &&
 	cd .. &&
 	rm -rf t.svn &&
+	echo another file from git >> file2 &&
+	git add file2 &&
+	git commit -m 'another file here' &&
 	echo 'fourth line from git' >> file &&
 	git commit -a -m 'fourth line from git' &&
 	git-svn dcommit
Signature · 3 lines
1.5.3.rc4

Dave Watson
#6 lore raw

PATCH 1/1 git-svn: fix dcommit clobbering upstream when committing multiple changes

Although dcommit could detect if the first commit in the series would conflict with the HEAD revision in SVN, it could not detect conflicts in further commits it made.

Now we rebase each uncommitted change after each revision is committed to SVN to ensure that we are up-to-date. git-rebase will bail out on conflict errors if our next change cannot be applied and committed to SVN cleanly, preventing accidental clobbering of changes on the SVN-side.

--no-rebase users will have trouble with this, and are thus warned if they are committing more than one commit. Fixing this for (hopefully uncommon) --no-rebase users would be more complex and will probably happen at a later date.

Thanks to David Watson for finding this and the original test.
---

Quoted 4 lines · David Watson

If you make multiple commits, and a commit other than the first applies

to the same file as a new change in the SVN repository, git-svn will blow away the other commit's changes.

Thanks David. I separated out the test case to not modify existing ones.

 git-svn.perl                           |   60 +++++++++++++++++---------------
 t/t9106-git-svn-commit-diff-clobber.sh |   30 ++++++++++++++++
 2 files changed, 62 insertions(+), 28 deletions(-)
git-svn.perl +32 -28
index 98218da..d3c8cd0 100755
@@ -384,6 +384,12 @@ sub cmd_dcommit {
 	}
 	my $last_rev;
 	my ($linear_refs, $parents) = linearize_history($gs, \@refs);
+	if ($_no_rebase && scalar(@$linear_refs) > 1) {
+		warn "Attempting to commit more than one change while ",
+		     "--no-rebase is enabled.\n",
+		     "If these changes depend on each other, re-running ",
+		     "without --no-rebase will be required."
+	}
 	foreach my $d (@$linear_refs) {
 		unless (defined $last_rev) {
 			(undef, $last_rev, undef) = cmt_metadata("$d~1");
@@ -395,6 +401,7 @@ sub cmd_dcommit {
 		if ($_dry_run) {
 			print "diff-tree $d~1 $d\n";
 		} else {
+			my $cmt_rev;
 			my %ed_opts = ( r => $last_rev,
 			                log => get_commit_entry($d)->{log},
 			                ra => Git::SVN::Ra->new($gs->full_url),
@@ -402,42 +409,39 @@ sub cmd_dcommit {
 			                tree_b => $d,
 			                editor_cb => sub {
 			                       print "Committed r$_[0]\n";
-			                       $last_rev = $_[0]; },
+			                       $cmt_rev = $_[0];
+			                },
 			                svn_path => '');
 			if (!SVN::Git::Editor->new(\%ed_opts)->apply_diff) {
 				print "No changes\n$d~1 == $d\n";
 			} elsif ($parents->{$d} && @{$parents->{$d}}) {
-				$gs->{inject_parents_dcommit}->{$last_rev} =
+				$gs->{inject_parents_dcommit}->{$cmt_rev} =
 				                               $parents->{$d};
 			}
+			$_fetch_all ? $gs->fetch_all : $gs->fetch;
+			next if $_no_rebase;
+
+			# we always want to rebase against the current HEAD,
+			# not any head that was passed to us
+			my @diff = command('diff-tree', 'HEAD',
+			                   $gs->refname, '--');
+			my @finish;
+			if (@diff) {
+				@finish = rebase_cmd();
+				print STDERR "W: HEAD and ", $gs->refname,
+				             " differ, using @finish:\n",
+				             "@diff";
+			} else {
+				print "No changes between current HEAD and ",
+				      $gs->refname,
+				      "\nResetting to the latest ",
+				      $gs->refname, "\n";
+				@finish = qw/reset --mixed/;
+			}
+			command_noisy(@finish, $gs->refname);
+			$last_rev = $cmt_rev;
 		}
 	}
-	return if $_dry_run;
-	unless ($gs) {
-		warn "Could not determine fetch information for $url\n",
-		     "Will not attempt to fetch and rebase commits.\n",
-		     "This probably means you have useSvmProps and should\n",
-		     "now resync your SVN::Mirror repository.\n";
-		return;
-	}
-	$_fetch_all ? $gs->fetch_all : $gs->fetch;
-	unless ($_no_rebase) {
-		# we always want to rebase against the current HEAD, not any
-		# head that was passed to us
-		my @diff = command('diff-tree', 'HEAD', $gs->refname, '--');
-		my @finish;
-		if (@diff) {
-			@finish = rebase_cmd();
-			print STDERR "W: HEAD and ", $gs->refname, " differ, ",
-				     "using @finish:\n", "@diff";
-		} else {
-			print "No changes between current HEAD and ",
-			      $gs->refname, "\nResetting to the latest ",
-			      $gs->refname, "\n";
-			@finish = qw/reset --mixed/;
-		}
-		command_noisy(@finish, $gs->refname);
-	}
 }
 
 sub cmd_find_rev {
t/t9106-git-svn-commit-diff-clobber.sh +30 -0
index 6f132f2..79b7968 100755
@@ -66,4 +66,34 @@ test_expect_success 'dcommit does the svn equivalent of an index merge' "
 	git-svn dcommit
 	"
 
+test_expect_success 'commit another change from svn side' "
+	svn co $svnrepo t.svn &&
+	cd t.svn &&
+		echo third line from svn >> file &&
+		poke file &&
+		svn commit -m 'third line from svn' &&
+	cd .. &&
+	rm -rf t.svn
+	"
+
+test_expect_failure 'multiple dcommit from git-svn will not clobber svn' "
+	git reset --hard refs/remotes/git-svn &&
+	echo new file >> new-file &&
+	git update-index --add new-file &&
+	git commit -a -m 'new file' &&
+	echo clobber > file &&
+	git commit -a -m 'clobber' &&
+	git svn dcommit
+	" || true
+
+
+test_expect_success 'check that rebase really failed' 'test -d .dotest'
+
+test_expect_success 'resolve, continue the rebase and dcommit' "
+	echo clobber and I really mean it > file &&
+	git update-index file &&
+	git rebase --continue &&
+	git svn dcommit
+	"
+
 test_done
Signature · 1 line
Eric Wong
#7 lore raw

Re: [PATCH] git-svn: fix dcommit clobbering upstream when committing multiple changes

Quoted 10 lines · Eric Wong

Although dcommit could detect if the first commit in the series would conflict with the HEAD revision in SVN, it could not detect conflicts in further commits it made.

Now we rebase each uncommitted change after each revision is committed to SVN to ensure that we are up-to-date. git-rebase will bail out on conflict errors if our next change cannot be applied and committed to SVN cleanly, preventing accidental clobbering of changes on the SVN-side.

Quoted 4 lines

--no-rebase users will have trouble with this, and are thus warned if they are committing more than one commit. Fixing this for (hopefully uncommon) --no-rebase users would be more complex and will probably happen at a later date.

Shouldn't it be a simple matter of checking if the total diff over the whole series would conflict with the SVN HEAD?

Signature · 2 lines
Karl Hasselström, [email protected]
      www.treskal.com/kalle
#8 lore raw

Re: [PATCH] git-svn: fix dcommit clobbering upstream when committing multiple changes

Quoted 12 lines · Karl Hasselström
Quoted 10 lines · Eric Wong

Although dcommit could detect if the first commit in the series would conflict with the HEAD revision in SVN, it could not detect conflicts in further commits it made.

Now we rebase each uncommitted change after each revision is committed to SVN to ensure that we are up-to-date. git-rebase will bail out on conflict errors if our next change cannot be applied and committed to SVN cleanly, preventing accidental clobbering of changes on the SVN-side.

Quoted 7 lines
Quoted 4 lines

--no-rebase users will have trouble with this, and are thus warned if they are committing more than one commit. Fixing this for (hopefully uncommon) --no-rebase users would be more complex and will probably happen at a later date.

Shouldn't it be a simple matter of checking if the total diff over the whole series would conflict with the SVN HEAD?

I don't think you can actually check for SVN conflicts until attempting to do commit.

Signature · 1 line
Eric Wong
#9 lore raw

Re: [PATCH] git-svn: fix dcommit clobbering upstream when committing multiple changes

Quoted 7 lines · Eric Wong
Quoted 2 lines · Karl Hasselström

Shouldn't it be a simple matter of checking if the total diff over the whole series would conflict with the SVN HEAD?

I don't think you can actually check for SVN conflicts until attempting to do commit.

Ah, that's true. I was only considering the git side ...

Signature · 2 lines
Karl Hasselström, [email protected]
      www.treskal.com/kalle
#10 lore raw

Re: [PATCH] git-svn: fix dcommit clobbering upstream when committing multiple changes

In reply to Eric Wong

Hi,

Quoted 9 lines · Eric Wong

Although dcommit could detect if the first commit in the series would conflict with the HEAD revision in SVN, it could not detect conflicts in further commits it made.

Now we rebase each uncommitted change after each revision is committed to SVN to ensure that we are up-to-date. git-rebase will bail out on conflict errors if our next change cannot be applied and committed to SVN cleanly, preventing accidental clobbering of changes on the SVN-side.

Thanks for the quick fix; will play with it on Sunday...

Ciao,
Dscho