diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt index ae02475a1f..32e93d3118 100644 --- a/Documentation/rest-api-changes.txt +++ b/Documentation/rest-api-changes.txt @@ -4860,11 +4860,13 @@ Which patchset (if any) generated this message. === CherryPickInput The `CherryPickInput` entity contains information for cherry-picking a change to a new branch. -[options="header",cols="1,6"] +[options="header",cols="1,^1,5"] |=========================== -|Field Name |Description -|`message` |Commit message for the cherry-picked change -|`destination` |Destination branch +|Field Name ||Description +|`message` ||Commit message for the cherry-picked change +|`destination` ||Destination branch +|`parent` |optional, defaults to 1| +Number of the parent relative to which the cherry-pick should be considered. |=========================== [[comment-info]] diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java index 7f5c6bc263..92a3382536 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java @@ -26,6 +26,8 @@ import static org.eclipse.jgit.lib.Constants.HEAD; import static org.junit.Assert.fail; import com.google.common.base.Joiner; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.PushOneCommit; @@ -47,9 +49,11 @@ import com.google.gerrit.extensions.common.DiffInfo; import com.google.gerrit.extensions.common.FileInfo; import com.google.gerrit.extensions.common.MergeableInfo; import com.google.gerrit.extensions.common.RevisionInfo; +import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.BinaryResult; import com.google.gerrit.extensions.restapi.ETagView; import com.google.gerrit.extensions.restapi.ResourceConflictException; +import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.server.change.GetRevisionActions; import com.google.gerrit.server.change.RevisionResource; import com.google.inject.Inject; @@ -356,6 +360,111 @@ public class RevisionIT extends AbstractDaemonTest { .isEqualTo("a"); } + @Test + public void cherryPickMergeRelativeToDefaultParent() throws Exception { + String parent1FileName = "a.txt"; + String parent2FileName = "b.txt"; + PushOneCommit.Result mergeChangeResult = + createCherryPickableMerge(parent1FileName, parent2FileName); + + String cherryPickBranchName = "branch_for_cherry_pick"; + createBranch(new Branch.NameKey(project, cherryPickBranchName)); + + CherryPickInput cherryPickInput = new CherryPickInput(); + cherryPickInput.destination = cherryPickBranchName; + cherryPickInput.message = "Cherry-pick a merge commit to another branch"; + + ChangeInfo cherryPickedChangeInfo = gApi.changes() + .id(mergeChangeResult.getChangeId()) + .current() + .cherryPick(cherryPickInput) + .get(); + + Map cherryPickedFilesByName = + cherryPickedChangeInfo.revisions + .get(cherryPickedChangeInfo.currentRevision) + .files; + assertThat(cherryPickedFilesByName).containsKey(parent2FileName); + assertThat(cherryPickedFilesByName).doesNotContainKey(parent1FileName); + } + + @Test + public void cherryPickMergeRelativeToSpecificParent() throws Exception { + String parent1FileName = "a.txt"; + String parent2FileName = "b.txt"; + PushOneCommit.Result mergeChangeResult = + createCherryPickableMerge(parent1FileName, parent2FileName); + + String cherryPickBranchName = "branch_for_cherry_pick"; + createBranch(new Branch.NameKey(project, cherryPickBranchName)); + + CherryPickInput cherryPickInput = new CherryPickInput(); + cherryPickInput.destination = cherryPickBranchName; + cherryPickInput.message = "Cherry-pick a merge commit to another branch"; + cherryPickInput.parent = 2; + + ChangeInfo cherryPickedChangeInfo = gApi.changes() + .id(mergeChangeResult.getChangeId()) + .current() + .cherryPick(cherryPickInput) + .get(); + + Map cherryPickedFilesByName = + cherryPickedChangeInfo.revisions + .get(cherryPickedChangeInfo.currentRevision) + .files; + assertThat(cherryPickedFilesByName).containsKey(parent1FileName); + assertThat(cherryPickedFilesByName).doesNotContainKey(parent2FileName); + } + + @Test + public void cherryPickMergeUsingInvalidParent() throws Exception { + String parent1FileName = "a.txt"; + String parent2FileName = "b.txt"; + PushOneCommit.Result mergeChangeResult = + createCherryPickableMerge(parent1FileName, parent2FileName); + + String cherryPickBranchName = "branch_for_cherry_pick"; + createBranch(new Branch.NameKey(project, cherryPickBranchName)); + + CherryPickInput cherryPickInput = new CherryPickInput(); + cherryPickInput.destination = cherryPickBranchName; + cherryPickInput.message = "Cherry-pick a merge commit to another branch"; + cherryPickInput.parent = 0; + + exception.expect(BadRequestException.class); + exception.expectMessage("Cherry Pick: Parent 0 does not exist. Please" + + " specify a parent in range [1, 2]."); + gApi.changes() + .id(mergeChangeResult.getChangeId()) + .current() + .cherryPick(cherryPickInput); + } + + @Test + public void cherryPickMergeUsingNonExistentParent() throws Exception { + String parent1FileName = "a.txt"; + String parent2FileName = "b.txt"; + PushOneCommit.Result mergeChangeResult = + createCherryPickableMerge(parent1FileName, parent2FileName); + + String cherryPickBranchName = "branch_for_cherry_pick"; + createBranch(new Branch.NameKey(project, cherryPickBranchName)); + + CherryPickInput cherryPickInput = new CherryPickInput(); + cherryPickInput.destination = cherryPickBranchName; + cherryPickInput.message = "Cherry-pick a merge commit to another branch"; + cherryPickInput.parent = 3; + + exception.expect(BadRequestException.class); + exception.expectMessage("Cherry Pick: Parent 3 does not exist. Please" + + " specify a parent in range [1, 2]."); + gApi.changes() + .id(mergeChangeResult.getChangeId()) + .current() + .cherryPick(cherryPickInput); + } + @Test public void canRebase() throws Exception { PushOneCommit push = pushFactory.create(db, admin.getIdent(), testRepo); @@ -806,4 +915,35 @@ public class RevisionIT extends AbstractDaemonTest { assertDiffForNewFile(diff, pushResult.getCommit(), path, expectedContentSideB); } + + private PushOneCommit.Result createCherryPickableMerge(String parent1FileName, + String parent2FileName) throws Exception { + RevCommit initialCommit = getHead(repo()); + + String branchAName = "branchA"; + createBranch(new Branch.NameKey(project, branchAName)); + String branchBName = "branchB"; + createBranch(new Branch.NameKey(project, branchBName)); + + PushOneCommit.Result changeAResult = pushFactory + .create(db, admin.getIdent(), testRepo, "change a", + parent1FileName, "Content of a") + .to("refs/for/" + branchAName); + + testRepo.reset(initialCommit); + PushOneCommit.Result changeBResult = pushFactory + .create(db, admin.getIdent(), testRepo, "change b", + parent2FileName, "Content of b") + .to("refs/for/" + branchBName); + + PushOneCommit pushableMergeCommit = pushFactory.create(db, admin.getIdent(), + testRepo, "merge", ImmutableMap.of(parent1FileName, "Content of a", + parent2FileName, "Content of b")); + pushableMergeCommit.setParents(ImmutableList.of(changeAResult.getCommit(), + changeBResult.getCommit())); + PushOneCommit.Result mergeChangeResult = + pushableMergeCommit.to("refs/for/" + branchAName); + mergeChangeResult.assertOkStatus(); + return mergeChangeResult; + } } diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/CherryPickInput.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/CherryPickInput.java index 7ae7ef1b59..2e1bb131f4 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/CherryPickInput.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/CherryPickInput.java @@ -17,4 +17,5 @@ package com.google.gerrit.extensions.api.changes; public class CherryPickInput { public String message; public String destination; + public Integer parent; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java index 1a063f4dd6..b5eb193ad4 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java @@ -60,10 +60,12 @@ public class CherryPick implements RestModifyView commitToCherryPick.getParentCount()) { + throw new InvalidChangeOperationException(String.format( + "Cherry Pick: Parent %s does not exist. Please specify a parent in" + + " range [1, %s].", + parent, commitToCherryPick.getParentCount())); + } + Timestamp now = TimeUtil.nowTs(); PersonIdent committerIdent = identifiedUser.newCommitterIdent(now, serverTimeZone); @@ -160,10 +167,12 @@ public class CherryPickChange { CodeReviewCommit cherryPickCommit; try { - ProjectState projectState = refControl.getProjectControl().getProjectState(); - cherryPickCommit = - mergeUtilFactory.create(projectState).createCherryPickFromCommit(git, oi, mergeTip, - commitToCherryPick, committerIdent, commitMessage, revWalk); + ProjectState projectState = refControl.getProjectControl() + .getProjectState(); + cherryPickCommit = mergeUtilFactory.create(projectState) + .createCherryPickFromCommit(git, oi, mergeTip, + commitToCherryPick, committerIdent, commitMessage, revWalk, + parent - 1); Change.Key changeKey; final List idList = cherryPickCommit.getFooterLines( diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java index 0667e14d94..90edfb1c71 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java @@ -184,13 +184,13 @@ public class MergeUtil { public CodeReviewCommit createCherryPickFromCommit(Repository repo, ObjectInserter inserter, RevCommit mergeTip, RevCommit originalCommit, PersonIdent cherryPickCommitterIdent, String commitMsg, - CodeReviewRevWalk rw) + CodeReviewRevWalk rw, int parentIndex) throws MissingObjectException, IncorrectObjectTypeException, IOException, MergeIdenticalTreeException, MergeConflictException { final ThreeWayMerger m = newThreeWayMerger(repo, inserter); - m.setBase(originalCommit.getParent(0)); + m.setBase(originalCommit.getParent(parentIndex)); if (m.merge(mergeTip, originalCommit)) { ObjectId tree = m.getResultTreeId(); if (tree.equals(mergeTip.getTree())) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java index 31da05c56f..c0d96c9436 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java @@ -107,7 +107,7 @@ public class CherryPick extends SubmitStrategy { try { newCommit = args.mergeUtil.createCherryPickFromCommit( args.repo, args.inserter, args.mergeTip.getCurrentTip(), toMerge, - committer, cherryPickCmtMsg, args.rw); + committer, cherryPickCmtMsg, args.rw, 0); } catch (MergeConflictException mce) { // Keep going in the case of a single merge failure; the goal is to // cherry-pick as many commits as possible.