Skip to content

Commit 751eb39

Browse files
Eric Wonggitster
Eric Wong
authored and
gitster
committed
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. Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent eeebd8d commit 751eb39

File tree

2 files changed

+62
-28
lines changed

2 files changed

+62
-28
lines changed

‎git-svn.perl‎

Lines changed: 32 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,12 @@ sub cmd_dcommit {
384384
}
385385
my $last_rev;
386386
my ($linear_refs, $parents) = linearize_history($gs, \@refs);
387+
if ($_no_rebase && scalar(@$linear_refs) > 1) {
388+
warn "Attempting to commit more than one change while ",
389+
"--no-rebase is enabled.\n",
390+
"If these changes depend on each other, re-running ",
391+
"without --no-rebase will be required."
392+
}
387393
foreach my $d (@$linear_refs) {
388394
unless (defined $last_rev) {
389395
(undef, $last_rev, undef) = cmt_metadata("$d~1");
@@ -395,49 +401,47 @@ sub cmd_dcommit {
395401
if ($_dry_run) {
396402
print "diff-tree $d~1 $d\n";
397403
} else {
404+
my $cmt_rev;
398405
my %ed_opts = ( r => $last_rev,
399406
log => get_commit_entry($d)->{log},
400407
ra => Git::SVN::Ra->new($gs->full_url),
401408
tree_a => "$d~1",
402409
tree_b => $d,
403410
editor_cb => sub {
404411
print "Committed r$_[0]\n";
405-
$last_rev = $_[0]; },
412+
$cmt_rev = $_[0];
413+
},
406414
svn_path => '');
407415
if (!SVN::Git::Editor->new(\%ed_opts)->apply_diff) {
408416
print "No changes\n$d~1 == $d\n";
409417
} elsif ($parents->{$d} && @{$parents->{$d}}) {
410-
$gs->{inject_parents_dcommit}->{$last_rev} =
418+
$gs->{inject_parents_dcommit}->{$cmt_rev} =
411419
$parents->{$d};
412420
}
421+
$_fetch_all ? $gs->fetch_all : $gs->fetch;
422+
next if $_no_rebase;
423+
424+
# we always want to rebase against the current HEAD,
425+
# not any head that was passed to us
426+
my @diff = command('diff-tree', 'HEAD',
427+
$gs->refname, '--');
428+
my @finish;
429+
if (@diff) {
430+
@finish = rebase_cmd();
431+
print STDERR "W: HEAD and ", $gs->refname,
432+
" differ, using @finish:\n",
433+
"@diff";
434+
} else {
435+
print "No changes between current HEAD and ",
436+
$gs->refname,
437+
"\nResetting to the latest ",
438+
$gs->refname, "\n";
439+
@finish = qw/reset --mixed/;
440+
}
441+
command_noisy(@finish, $gs->refname);
442+
$last_rev = $cmt_rev;
413443
}
414444
}
415-
return if $_dry_run;
416-
unless ($gs) {
417-
warn "Could not determine fetch information for $url\n",
418-
"Will not attempt to fetch and rebase commits.\n",
419-
"This probably means you have useSvmProps and should\n",
420-
"now resync your SVN::Mirror repository.\n";
421-
return;
422-
}
423-
$_fetch_all ? $gs->fetch_all : $gs->fetch;
424-
unless ($_no_rebase) {
425-
# we always want to rebase against the current HEAD, not any
426-
# head that was passed to us
427-
my @diff = command('diff-tree', 'HEAD', $gs->refname, '--');
428-
my @finish;
429-
if (@diff) {
430-
@finish = rebase_cmd();
431-
print STDERR "W: HEAD and ", $gs->refname, " differ, ",
432-
"using @finish:\n", "@diff";
433-
} else {
434-
print "No changes between current HEAD and ",
435-
$gs->refname, "\nResetting to the latest ",
436-
$gs->refname, "\n";
437-
@finish = qw/reset --mixed/;
438-
}
439-
command_noisy(@finish, $gs->refname);
440-
}
441445
}
442446

443447
sub cmd_find_rev {

‎t/t9106-git-svn-commit-diff-clobber.sh‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,4 +66,34 @@ test_expect_success 'dcommit does the svn equivalent of an index merge' "
6666
git-svn dcommit
6767
"
6868

69+
test_expect_success 'commit another change from svn side' "
70+
svn co $svnrepo t.svn &&
71+
cd t.svn &&
72+
echo third line from svn >> file &&
73+
poke file &&
74+
svn commit -m 'third line from svn' &&
75+
cd .. &&
76+
rm -rf t.svn
77+
"
78+
79+
test_expect_failure 'multiple dcommit from git-svn will not clobber svn' "
80+
git reset --hard refs/remotes/git-svn &&
81+
echo new file >> new-file &&
82+
git update-index --add new-file &&
83+
git commit -a -m 'new file' &&
84+
echo clobber > file &&
85+
git commit -a -m 'clobber' &&
86+
git svn dcommit
87+
" || true
88+
89+
90+
test_expect_success 'check that rebase really failed' 'test -d .dotest'
91+
92+
test_expect_success 'resolve, continue the rebase and dcommit' "
93+
echo clobber and I really mean it > file &&
94+
git update-index file &&
95+
git rebase --continue &&
96+
git svn dcommit
97+
"
98+
6999
test_done

0 commit comments

Comments
 (0)