opendev/gerrit
Code Issues Proposed changes

Support specifying the parent for a cherry-pick in the REST API

Browse Source 
When cherry-picking a merge, the REST API always selected the first
parent as reference. Sometimes users of the API want to choose a
specific parent just as they could on the command line using 'git
cherry-pick -m'. If the parent parameter isn't specified, it defaults
to 1.

Change-Id: I61dd0d9bee4a3c97354d52a18c66ae3567ab79fb
This commit is contained in:
Alice Kober-Sotzek 2016-10-12 13:13:54 +02:00
parent 861123bde0
commit f271c25fd9
7 changed files with 169 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
Documentation/rest-api-changes.txt
View File

@ -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]]

140
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java
View File

@ -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<String, FileInfo> 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<String, FileInfo> 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;
}
}

1
gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/CherryPickInput.java
View File

@ -17,4 +17,5 @@ package com.google.gerrit.extensions.api.changes;
public class CherryPickInput {
public String message;
public String destination;
public Integer parent;
}

6
gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java
View File

@ -60,10 +60,12 @@ public class CherryPick implements RestModifyView<RevisionResource, CherryPickIn
public ChangeInfo apply(RevisionResource revision, CherryPickInput input)
throws OrmException, IOException, UpdateException, RestApiException {
final ChangeControl control = revision.getControl();
int parent = input.parent == null ? 1 : input.parent;
if (input.message == null || input.message.trim().isEmpty()) {
throw new BadRequestException("message must be non-empty");
} else if (input.destination == null || input.destination.trim().isEmpty()) {
} else if (input.destination == null
|| input.destination.trim().isEmpty()) {
throw new BadRequestException("destination must be non-empty");
}
@ -91,7 +93,7 @@ public class CherryPick implements RestModifyView<RevisionResource, CherryPickIn
Change.Id cherryPickedChangeId =
cherryPickChange.cherryPick(revision.getChange(),
revision.getPatchSet(), input.message, refName,
refControl);
refControl, parent);
return json.create(ChangeJson.NO_OPTIONS).format(revision.getProject(),
cherryPickedChangeId);
} catch (InvalidChangeOperationException e) {

21
gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java
View File

@ -114,8 +114,8 @@ public class CherryPickChange {
}
public Change.Id cherryPick(Change change, PatchSet patch,
final String message, final String ref,
final RefControl refControl) throws NoSuchChangeException,
final String message, final String ref, final RefControl refControl,
int parent) throws NoSuchChangeException,
OrmException, MissingObjectException,
IncorrectObjectTypeException, IOException,
InvalidChangeOperationException, IntegrationException, UpdateException,
@ -147,6 +147,13 @@ public class CherryPickChange {
CodeReviewCommit commitToCherryPick =
revWalk.parseCommit(ObjectId.fromString(patch.getRevision().get()));
if (parent <= 0 || parent > 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<String> idList = cherryPickCommit.getFooterLines(

4
gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
View File

@ -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())) {

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

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