opendev/gerrit
Code Issues Proposed changes

CherryPick: Keep going in the face of a single merge conflict

Browse Source 
In addition to verifying this behavior in a test, this also fixes an
issue caused by setting mergeTip to null. On the next iteration of the
loop we were incorrectly treating the null merge tip as a cherry-pick
onto an unborn root. Setting the commit as CLEAN_MERGE resulted in
rejecting the later ref update in MergeOp.

Change-Id: I82299ea96725a21430a0871199cde3e2f3af267b
This commit is contained in:
Dave Borowitz
2015-02-18 07:39:47 -08:00
parent c0af7dbeca
commit 8cbf929a20
2 changed files with 43 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

32
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
View File

@@ -268,4 +268,36 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
assertThat(log.get(2).getShortMessage())
.isEqualTo(initialHead.getShortMessage());
}
@Test
public void submitChangeAfterParentFailsDueToConflict() throws Exception {
Git git = createProject();
RevCommit initialHead = getRemoteHead();
checkout(git, initialHead.getId().getName());
PushOneCommit.Result change2 = createChange(git, "Change 2", "b", "b1");
submit(change2.getChangeId());
checkout(git, initialHead.getId().getName());
PushOneCommit.Result change3 = createChange(git, "Change 3", "b", "b2");
assertThat(change3.getCommit().getParent(0)).isEqualTo(initialHead);
PushOneCommit.Result change4 = createChange(git, "Change 3", "c", "c3");
submitStatusOnly(change3.getChangeId());
submitStatusOnly(change4.getChangeId());
// Merge fails; change3 contains the delta "b1" -> "b2", which cannot be
// applied against tip.
submitWithConflict(change3.getChangeId());
// change4 is a clean merge, so should succeed in the same run where change3
// failed.
ChangeInfo info4 = get(change4.getChangeId());
assertThat(info4.status).isEqualTo(ChangeStatus.MERGED);
List<RevCommit> log = getRemoteLog();
assertThat(log.get(0).getShortMessage())
.isEqualTo(change4.getCommit().getShortMessage());
assertThat(log.get(1).getShortMessage())
.isEqualTo(change2.getCommit().getShortMessage());
}
}

23
gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java
View File

@@ -72,11 +72,11 @@ public class CherryPick extends SubmitStrategy {
CodeReviewCommit n = sorted.remove(0);
try {
if (mergeTip.getCurrentTip() == null) {
mergeTip = cherryPickUnbornRoot(n);
cherryPickUnbornRoot(n, mergeTip);
} else if (n.getParentCount() == 0) {
cherryPickRootOntoBranch(n);
} else if (n.getParentCount() == 1) {
mergeTip = cherryPickOne(n, mergeTip);
cherryPickOne(n, mergeTip);
} else {
cherryPickMultipleParents(n, mergeTip);
}
@@ -87,11 +87,10 @@ public class CherryPick extends SubmitStrategy {
return mergeTip;
}
private MergeTip cherryPickUnbornRoot(CodeReviewCommit n) {
private void cherryPickUnbornRoot(CodeReviewCommit n, MergeTip mergeTip) {
// The branch is unborn. Take fast-forward resolution to create the branch.
MergeTip mergeTip = new MergeTip(n, Lists.newArrayList(n));
mergeTip.moveTipTo(n, n);
n.setStatusCode(CommitMergeStatus.CLEAN_MERGE);
return mergeTip;
}
private void cherryPickRootOntoBranch(CodeReviewCommit n) {
@@ -100,24 +99,24 @@ public class CherryPick extends SubmitStrategy {
n.setStatusCode(CommitMergeStatus.CANNOT_CHERRY_PICK_ROOT);
}
private MergeTip cherryPickOne(CodeReviewCommit n, MergeTip mergeTip)
private void cherryPickOne(CodeReviewCommit n, MergeTip mergeTip)
throws NoSuchChangeException, OrmException, IOException {
// If there is only one parent, a cherry-pick can be done by
// taking the delta relative to that one parent and redoing
// that on the current merge tip.
// If there is only one parent, a cherry-pick can be done by taking the
// delta relative to that one parent and redoing that on the current merge
// tip.
//
// Keep going in the case of a single merge failure; the goal is to
// cherry-pick as many commits as possible.
try {
CodeReviewCommit merge =
writeCherryPickCommit(mergeTip.getCurrentTip(), n);
mergeTip.moveTipTo(merge, merge);
newCommits.put(mergeTip.getCurrentTip().getPatchsetId()
.getParentKey(), mergeTip.getCurrentTip());
return mergeTip;
} catch (MergeConflictException mce) {
n.setStatusCode(CommitMergeStatus.PATH_CONFLICT);
return null;
} catch (MergeIdenticalTreeException mie) {
n.setStatusCode(CommitMergeStatus.ALREADY_MERGED);
return null;
}
}