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

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

[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...

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

[David Watson]

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.

On Fri, Aug 31, 2007 at 10:04:24PM +0100, Johannes Schindelin wrote:
> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Dave Watson
Software Engineer
MIMvista Corp

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

[Eric Wong]

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> 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...

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.

-- 
Eric Wong

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

[Johannes Schindelin]

Hi,

On Fri, 31 Aug 2007, Eric Wong wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > 
> > 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

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

[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

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.

On Sat, Sep 01, 2007 at 12:17:33AM +0100, Johannes Schindelin wrote:
> Hi,
> 
> On Fri, 31 Aug 2007, Eric Wong wrote:
> 
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > > 
> > > 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Dave Watson
Software Engineer
MIMvista Corp

[PATCH] Test for git-svn dcommit clobbering changes

[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.

---


On Fri, Aug 31, 2007 at 07:48:55PM -0400, David Watson wrote:
> 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(-)

diff --git a/t/t9106-git-svn-commit-diff-clobber.sh b/t/t9106-git-svn-commit-diff-clobber.sh
index 6f132f2..2706153 100755
--- a/t/t9106-git-svn-commit-diff-clobber.sh
+++ b/t/t9106-git-svn-commit-diff-clobber.sh
@@ -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
-- 
1.5.3.rc4

Dave Watson

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

[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.

--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.
---
  David Watson <dwatson@mimvista.com> wrote:
  > 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(-)

diff --git a/git-svn.perl b/git-svn.perl
index 98218da..d3c8cd0 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -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 {
diff --git a/t/t9106-git-svn-commit-diff-clobber.sh b/t/t9106-git-svn-commit-diff-clobber.sh
index 6f132f2..79b7968 100755
--- a/t/t9106-git-svn-commit-diff-clobber.sh
+++ b/t/t9106-git-svn-commit-diff-clobber.sh
@@ -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
-- 
Eric Wong

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

[Johannes Schindelin]

Hi,

On Fri, 31 Aug 2007, Eric Wong wrote:

> 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

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

[Karl Hasselström]

On 2007-08-31 18:16:12 -0700, Eric Wong wrote:

> 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.

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

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

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

[Eric Wong]

Karl Hasselström <kha@treskal.com> wrote:
> On 2007-08-31 18:16:12 -0700, Eric Wong wrote:
> 
> > 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.
> 
> 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.

-- 
Eric Wong

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

[Karl Hasselström]

On 2007-09-01 02:33:03 -0700, Eric Wong wrote:

> Karl Hasselström <kha@treskal.com> wrote:
>
> > 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 ...

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle