Include merge result commit in change-merged stream-event

When CI gets a change merged event what the CI most likely wants
to build is not the last patch set of the change but the result
on the target branch.

If the merge of a change was not fast forward these are two
separate commits.

With this change the CI can retrieve the merge result from the
newRev of a change merged event.

Change-Id: I1d61ab253f4028a26cfbf0427b33873c294ee29e
This commit is contained in:
Sven Selberg 2014-11-17 12:51:06 +01:00 committed by David Pursehouse
parent 959aec4d3e
commit 9686cf224b
15 changed files with 289 additions and 118 deletions

View File

@ -72,6 +72,8 @@ patchSet:: link:json.html#patchSet[patchSet attribute]
submitter:: link:json.html#account[account attribute] submitter:: link:json.html#account[account attribute]
newRev:: The resulting revision of the merge.
eventCreatedOn:: Time in seconds since the UNIX epoch when this event was eventCreatedOn:: Time in seconds since the UNIX epoch when this event was
created. created.

View File

@ -22,11 +22,14 @@ import static com.google.gerrit.extensions.common.ListChangesOption.CURRENT_REVI
import static com.google.gerrit.extensions.common.ListChangesOption.DETAILED_LABELS; import static com.google.gerrit.extensions.common.ListChangesOption.DETAILED_LABELS;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.GitUtil; import com.google.gerrit.acceptance.GitUtil;
import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.RestResponse; import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.SshSession; import com.google.gerrit.acceptance.SshSession;
import com.google.gerrit.common.ChangeHooks;
import com.google.gerrit.common.ChangeListener;
import com.google.gerrit.extensions.api.changes.ReviewInput; import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.SubmitInput; import com.google.gerrit.extensions.api.changes.SubmitInput;
import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.common.ChangeInfo;
@ -41,7 +44,12 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.ApprovalsUtil; import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.events.ChangeEvent;
import com.google.gerrit.server.events.ChangeMergedEvent;
import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.project.ListBranches.BranchInfo;
import com.google.gerrit.server.project.PutConfig; import com.google.gerrit.server.project.PutConfig;
import com.google.gson.reflect.TypeToken; import com.google.gson.reflect.TypeToken;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
@ -66,16 +74,39 @@ import java.io.IOException;
import java.sql.Timestamp; import java.sql.Timestamp;
import java.util.Collections; import java.util.Collections;
import java.util.List; import java.util.List;
import java.util.Map;
public abstract class AbstractSubmit extends AbstractDaemonTest { public abstract class AbstractSubmit extends AbstractDaemonTest {
private Map<String, String> mergeResults;
@Inject @Inject
private ChangeNotes.Factory notesFactory; private ChangeNotes.Factory notesFactory;
@Inject @Inject
private ApprovalsUtil approvalsUtil; private ApprovalsUtil approvalsUtil;
@Inject
private IdentifiedUser.GenericFactory factory;
@Inject
ChangeHooks hooks;
@Before @Before
public void setUp() throws Exception { public void setUp() throws Exception {
mergeResults = Maps.newHashMap();
CurrentUser listenerUser = factory.create(user.id);
hooks.addChangeListener(new ChangeListener() {
@Override
public void onChangeEvent(ChangeEvent event) {
if (event instanceof ChangeMergedEvent) {
ChangeMergedEvent cMEvent = (ChangeMergedEvent) event;
mergeResults.put(cMEvent.change.number, cMEvent.newRev);
}
}
}, listenerUser);
project = new Project.NameKey("p2"); project = new Project.NameKey("p2");
} }
@ -181,10 +212,28 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
newGson().fromJson(r.getReader(), newGson().fromJson(r.getReader(),
new TypeToken<ChangeInfo>() {}.getType()); new TypeToken<ChangeInfo>() {}.getType());
assertThat(change.status).isEqualTo(ChangeStatus.MERGED); assertThat(change.status).isEqualTo(ChangeStatus.MERGED);
checkMergeResult(change);
} }
r.consume(); r.consume();
} }
private void checkMergeResult(ChangeInfo change) throws IOException {
// Get the revision of the branch after the submit to compare with the
// newRev of the ChangeMergedEvent.
RestResponse b =
adminSession.get("/projects/" + project.get() + "/branches/"
+ change.branch);
if (b.getStatusCode() == HttpStatus.SC_OK) {
BranchInfo branch =
newGson().fromJson(b.getReader(),
new TypeToken<BranchInfo>() {}.getType());
assertThat(branch.revision).isEqualTo(
mergeResults.get(Integer.toString(change._number)));
}
b.consume();
}
private void approve(String changeId) throws IOException { private void approve(String changeId) throws IOException {
RestResponse r = adminSession.post( RestResponse r = adminSession.post(
"/changes/" + changeId + "/revisions/current/review", "/changes/" + changeId + "/revisions/current/review",

View File

@ -447,13 +447,15 @@ public class ChangeHookRunner implements ChangeHooks, LifecycleListener {
@Override @Override
public void doChangeMergedHook(final Change change, final Account account, public void doChangeMergedHook(final Change change, final Account account,
final PatchSet patchSet, final ReviewDb db) throws OrmException { final PatchSet patchSet, final ReviewDb db, String mergeResultRev)
throws OrmException {
final ChangeMergedEvent event = new ChangeMergedEvent(); final ChangeMergedEvent event = new ChangeMergedEvent();
final AccountState owner = accountCache.get(change.getOwner()); final AccountState owner = accountCache.get(change.getOwner());
event.change = eventFactory.asChangeAttribute(change); event.change = eventFactory.asChangeAttribute(change);
event.submitter = eventFactory.asAccountAttribute(account); event.submitter = eventFactory.asAccountAttribute(account);
event.patchSet = eventFactory.asPatchSetAttribute(patchSet); event.patchSet = eventFactory.asPatchSetAttribute(patchSet);
event.newRev = mergeResultRev;
fireEvent(change, event, db); fireEvent(change, event, db);
final List<String> args = new ArrayList<>(); final List<String> args = new ArrayList<>();
@ -465,6 +467,7 @@ public class ChangeHookRunner implements ChangeHooks, LifecycleListener {
addArg(args, "--topic", event.change.topic); addArg(args, "--topic", event.change.topic);
addArg(args, "--submitter", getDisplayName(account)); addArg(args, "--submitter", getDisplayName(account));
addArg(args, "--commit", event.patchSet.revision); addArg(args, "--commit", event.patchSet.revision);
addArg(args, "--newrev", mergeResultRev);
runHook(change.getProject(), changeMergedHook, args); runHook(change.getProject(), changeMergedHook, args);
} }

View File

@ -79,10 +79,11 @@ public interface ChangeHooks {
* @param change The change itself. * @param change The change itself.
* @param account The gerrit user who submitted the change. * @param account The gerrit user who submitted the change.
* @param patchSet The patchset that was merged. * @param patchSet The patchset that was merged.
* @param mergeResultRev The SHA-1 of the merge result revision.
* @throws OrmException * @throws OrmException
*/ */
public void doChangeMergedHook(Change change, Account account, public void doChangeMergedHook(Change change, Account account,
PatchSet patchSet, ReviewDb db) throws OrmException; PatchSet patchSet, ReviewDb db, String mergeResultRev) throws OrmException;
/** /**
* Fire the Merge Failed Hook. * Fire the Merge Failed Hook.

View File

@ -44,7 +44,7 @@ public final class DisabledChangeHooks implements ChangeHooks {
@Override @Override
public void doChangeMergedHook(Change change, Account account, public void doChangeMergedHook(Change change, Account account,
PatchSet patchSet, ReviewDb db) { PatchSet patchSet, ReviewDb db, String mergeResultRev) {
} }
@Override @Override

View File

@ -27,6 +27,7 @@ public class ChangeMergedEvent extends ChangeEvent {
public ChangeAttribute change; public ChangeAttribute change;
public PatchSetAttribute patchSet; public PatchSetAttribute patchSet;
public AccountAttribute submitter; public AccountAttribute submitter;
public String newRev;
@Override @Override
public String getType() { public String getType() {

View File

@ -171,7 +171,7 @@ public class MergeOp {
private RevWalk rw; private RevWalk rw;
private RevFlag canMergeFlag; private RevFlag canMergeFlag;
private CodeReviewCommit branchTip; private CodeReviewCommit branchTip;
private CodeReviewCommit mergeTip; private MergeTip mergeTip;
private ObjectInserter inserter; private ObjectInserter inserter;
private PersonIdent refLogIdent; private PersonIdent refLogIdent;
@ -277,11 +277,11 @@ public class MergeOp {
branchUpdate = openBranch(); branchUpdate = openBranch();
} }
SubmitStrategy strategy = createStrategy(submitType); SubmitStrategy strategy = createStrategy(submitType);
preMerge(strategy, toMerge.get(submitType)); MergeTip mergeTip = preMerge(strategy, toMerge.get(submitType));
RefUpdate update = updateBranch(strategy, branchUpdate); RefUpdate update = updateBranch(strategy, branchUpdate);
reopen = true; reopen = true;
updateChangeStatus(toSubmit.get(submitType)); updateChangeStatus(toSubmit.get(submitType), mergeTip);
updateSubscriptions(toSubmit.get(submitType)); updateSubscriptions(toSubmit.get(submitType));
if (update != null) { if (update != null) {
fireRefUpdated(update); fireRefUpdated(update);
@ -315,7 +315,7 @@ public class MergeOp {
logDebug("Adding {} changes to merge on next run", toMerge.size()); logDebug("Adding {} changes to merge on next run", toMerge.size());
} }
updateChangeStatus(toUpdate); updateChangeStatus(toUpdate, mergeTip);
for (CodeReviewCommit commit : potentiallyStillSubmittableOnNextRun) { for (CodeReviewCommit commit : potentiallyStillSubmittableOnNextRun) {
Capable capable = isSubmitStillPossible(commit); Capable capable = isSubmitStillPossible(commit);
@ -403,7 +403,7 @@ public class MergeOp {
return true; return true;
} }
private void preMerge(SubmitStrategy strategy, private MergeTip preMerge(SubmitStrategy strategy,
List<CodeReviewCommit> toMerge) throws MergeException { List<CodeReviewCommit> toMerge) throws MergeException {
logDebug("Running submit strategy {} for {} commits", logDebug("Running submit strategy {} for {} commits",
strategy.getClass().getSimpleName(), toMerge.size()); strategy.getClass().getSimpleName(), toMerge.size());
@ -411,6 +411,7 @@ public class MergeOp {
refLogIdent = strategy.getRefLogIdent(); refLogIdent = strategy.getRefLogIdent();
logDebug("Produced {} new commits", strategy.getNewCommits().size()); logDebug("Produced {} new commits", strategy.getNewCommits().size());
commits.putAll(strategy.getNewCommits()); commits.putAll(strategy.getNewCommits());
return mergeTip;
} }
private SubmitStrategy createStrategy(SubmitType submitType) private SubmitStrategy createStrategy(SubmitType submitType)
@ -604,7 +605,7 @@ public class MergeOp {
idstr, ps.getId()); idstr, ps.getId());
commit.setStatusCode(CommitMergeStatus.ALREADY_MERGED); commit.setStatusCode(CommitMergeStatus.ALREADY_MERGED);
try { try {
setMerged(chg, null); setMerged(chg, null, commit.getName());
} catch (OrmException e) { } catch (OrmException e) {
logError("Cannot mark change " + chg.getId() + " merged", e); logError("Cannot mark change " + chg.getId() + " merged", e);
} }
@ -650,11 +651,13 @@ public class MergeOp {
private RefUpdate updateBranch(SubmitStrategy strategy, private RefUpdate updateBranch(SubmitStrategy strategy,
RefUpdate branchUpdate) throws MergeException { RefUpdate branchUpdate) throws MergeException {
if (branchTip == mergeTip) { CodeReviewCommit currentTip =
mergeTip != null ? mergeTip.getCurrentTip() : null;
if (branchTip == currentTip) {
logDebug("Branch already at merge tip {}, no update to perform", logDebug("Branch already at merge tip {}, no update to perform",
mergeTip.name()); currentTip.name());
return null; return null;
} else if (mergeTip == null) { } else if (currentTip == null) {
logDebug("No merge tip, no update to perform"); logDebug("No merge tip, no update to perform");
return null; return null;
} }
@ -664,17 +667,17 @@ public class MergeOp {
try { try {
ProjectConfig cfg = ProjectConfig cfg =
new ProjectConfig(destProject.getProject().getNameKey()); new ProjectConfig(destProject.getProject().getNameKey());
cfg.load(repo, mergeTip); cfg.load(repo, currentTip);
} catch (Exception e) { } catch (Exception e) {
throw new MergeException("Submit would store invalid" throw new MergeException("Submit would store invalid"
+ " project configuration " + mergeTip.name() + " for " + " project configuration " + currentTip.name() + " for "
+ destProject.getProject().getName(), e); + destProject.getProject().getName(), e);
} }
} }
branchUpdate.setRefLogIdent(refLogIdent); branchUpdate.setRefLogIdent(refLogIdent);
branchUpdate.setForceUpdate(false); branchUpdate.setForceUpdate(false);
branchUpdate.setNewObjectId(mergeTip); branchUpdate.setNewObjectId(currentTip);
branchUpdate.setRefLogMessage("merged", true); branchUpdate.setRefLogMessage("merged", true);
try { try {
RefUpdate.Result result = branchUpdate.update(rw); RefUpdate.Result result = branchUpdate.update(rw);
@ -688,7 +691,7 @@ public class MergeOp {
tagCache.updateFastForward(destBranch.getParentKey(), tagCache.updateFastForward(destBranch.getParentKey(),
branchUpdate.getName(), branchUpdate.getName(),
branchUpdate.getOldObjectId(), branchUpdate.getOldObjectId(),
mergeTip); currentTip);
} }
if (RefNames.REFS_CONFIG.equals(branchUpdate.getName())) { if (RefNames.REFS_CONFIG.equals(branchUpdate.getName())) {
@ -722,7 +725,8 @@ public class MergeOp {
private void fireRefUpdated(RefUpdate branchUpdate) { private void fireRefUpdated(RefUpdate branchUpdate) {
logDebug("Firing ref updated hooks for {}", branchUpdate.getName()); logDebug("Firing ref updated hooks for {}", branchUpdate.getName());
gitRefUpdated.fire(destBranch.getParentKey(), branchUpdate); gitRefUpdated.fire(destBranch.getParentKey(), branchUpdate);
hooks.doRefUpdatedHook(destBranch, branchUpdate, getAccount(mergeTip)); hooks.doRefUpdatedHook(destBranch, branchUpdate,
getAccount(mergeTip.getCurrentTip()));
} }
private Account getAccount(CodeReviewCommit codeReviewCommit) { private Account getAccount(CodeReviewCommit codeReviewCommit) {
@ -743,7 +747,7 @@ public class MergeOp {
return ""; return "";
} }
private void updateChangeStatus(List<Change> submitted) private void updateChangeStatus(List<Change> submitted, MergeTip mergeTip)
throws NoSuchChangeException { throws NoSuchChangeException {
logDebug("Updating change status for {} changes", submitted.size()); logDebug("Updating change status for {} changes", submitted.size());
for (Change c : submitted) { for (Change c : submitted) {
@ -761,21 +765,25 @@ public class MergeOp {
String txt = s.getMessage(); String txt = s.getMessage();
logDebug("Status of change {} ({}) on {}: {}", c.getId(), commit.name(), logDebug("Status of change {} ({}) on {}: {}", c.getId(), commit.name(),
c.getDest(), s); c.getDest(), s);
String commitName = commit.getName();
// If mergeTip is null merge failed and mergeResultRev will not be read.
String mergeResultRev =
mergeTip != null ? mergeTip.getMergeResults().get(commitName) : null;
try { try {
switch (s) { switch (s) {
case CLEAN_MERGE: case CLEAN_MERGE:
setMerged(c, message(c, txt + getByAccountName(commit))); setMerged(c, message(c, txt + getByAccountName(commit)),
mergeResultRev);
break; break;
case CLEAN_REBASE: case CLEAN_REBASE:
case CLEAN_PICK: case CLEAN_PICK:
setMerged(c, message(c, txt + " as " + commit.name() setMerged(c, message(c, txt + " as " + commit.name()
+ getByAccountName(commit))); + getByAccountName(commit)), mergeResultRev);
break; break;
case ALREADY_MERGED: case ALREADY_MERGED:
setMerged(c, null); setMerged(c, null, mergeResultRev);
break; break;
case PATH_CONFLICT: case PATH_CONFLICT:
@ -810,13 +818,14 @@ public class MergeOp {
} }
private void updateSubscriptions(List<Change> submitted) { private void updateSubscriptions(List<Change> submitted) {
if (mergeTip != null && (branchTip == null || branchTip != mergeTip)) { if (mergeTip != null
&& (branchTip == null || branchTip != mergeTip.getCurrentTip())) {
logDebug("Updating submodule subscriptions for {} changes", logDebug("Updating submodule subscriptions for {} changes",
submitted.size()); submitted.size());
SubmoduleOp subOp = SubmoduleOp subOp =
subOpFactory.create(destBranch, mergeTip, rw, repo, subOpFactory.create(destBranch, mergeTip.getCurrentTip(), rw, repo,
destProject.getProject(), submitted, commits, destProject.getProject(), submitted, commits,
getAccount(mergeTip)); getAccount(mergeTip.getCurrentTip()));
try { try {
subOp.update(); subOp.update();
} catch (SubmoduleException e) { } catch (SubmoduleException e) {
@ -928,7 +937,7 @@ public class MergeOp {
return m; return m;
} }
private void setMerged(Change c, ChangeMessage msg) private void setMerged(Change c, ChangeMessage msg, String mergeResultRev)
throws OrmException, IOException { throws OrmException, IOException {
logDebug("Setting change {} merged", c.getId()); logDebug("Setting change {} merged", c.getId());
ChangeUpdate update = null; ChangeUpdate update = null;
@ -954,6 +963,7 @@ public class MergeOp {
cmUtil.addChangeMessage(db, update, msg); cmUtil.addChangeMessage(db, update, msg);
} }
db.commit(); db.commit();
} finally { } finally {
db.rollback(); db.rollback();
} }
@ -964,7 +974,7 @@ public class MergeOp {
try { try {
hooks.doChangeMergedHook(c, hooks.doChangeMergedHook(c,
accountCache.get(submitter.getAccountId()).getAccount(), accountCache.get(submitter.getAccountId()).getAccount(),
merged, db); merged, db, mergeResultRev);
} catch (OrmException ex) { } catch (OrmException ex) {
logError("Cannot run hook for submitted patch set " + c.getId(), ex); logError("Cannot run hook for submitted patch set " + c.getId(), ex);
} }

View File

@ -0,0 +1,70 @@
// Copyright (C) 2014 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.git;
import com.google.common.collect.Maps;
import java.util.Collection;
import java.util.Map;
/**
* Class describing a merge tip during merge operation.
*/
public class MergeTip {
private CodeReviewCommit branchTip;
private Map<String,String> mergeResults;
/**
* @param initial Tip before the merge operation.
* @param toMerge List of CodeReview commits to be merged in merge operation.
*/
public MergeTip(CodeReviewCommit initial, Collection<CodeReviewCommit> toMerge) {
this.mergeResults = Maps.newHashMap();
this.branchTip = initial;
// Assume fast-forward merge until opposite is proven.
for (CodeReviewCommit commit : toMerge) {
mergeResults.put(commit.getName(), commit.getName());
}
}
/**
* Moves this MergeTip to newTip and appends mergeResult.
*
* @param newTip The new tip
* @param mergedFrom The result of the merge of newTip
*/
public void moveTipTo(CodeReviewCommit newTip, String mergedFrom) {
branchTip = newTip;
mergeResults.put(mergedFrom, newTip.getName());
}
/**
* The merge results of all the merges of this merge operation.
*
* @return The merge results of the merge operation Map<<sha1 of the commit to
* be merged>, <sha1 of the merge result>>
*/
public Map<String, String> getMergeResults() {
return this.mergeResults;
}
/**
*
* @return The current tip of the current merge operation.
*/
public CodeReviewCommit getCurrentTip() {
return branchTip;
}
}

View File

@ -2147,7 +2147,7 @@ public class ReceiveCommits {
hooks.doPatchsetCreatedHook(change, newPatchSet, db); hooks.doPatchsetCreatedHook(change, newPatchSet, db);
if (mergedIntoRef != null) { if (mergedIntoRef != null) {
hooks.doChangeMergedHook( hooks.doChangeMergedHook(
change, currentUser.getAccount(), newPatchSet, db); change, currentUser.getAccount(), newPatchSet, db, newCommit.getName());
} }
if (magicBranch != null && magicBranch.submit) { if (magicBranch != null && magicBranch.submit) {
@ -2434,7 +2434,7 @@ public class ReceiveCommits {
result.mergedIntoRef = refName; result.mergedIntoRef = refName;
markChangeMergedByPush(db, result, result.changeCtl); markChangeMergedByPush(db, result, result.changeCtl);
hooks.doChangeMergedHook( hooks.doChangeMergedHook(
change, currentUser.getAccount(), result.newPatchSet, db); change, currentUser.getAccount(), result.newPatchSet, db, commit.getName());
sendMergedEmail(result); sendMergedEmail(result);
return change.getKey(); return change.getKey();
} }

View File

@ -30,6 +30,7 @@ import com.google.gerrit.server.git.CommitMergeStatus;
import com.google.gerrit.server.git.MergeConflictException; import com.google.gerrit.server.git.MergeConflictException;
import com.google.gerrit.server.git.MergeException; import com.google.gerrit.server.git.MergeException;
import com.google.gerrit.server.git.MergeIdenticalTreeException; import com.google.gerrit.server.git.MergeIdenticalTreeException;
import com.google.gerrit.server.git.MergeTip;
import com.google.gerrit.server.patch.PatchSetInfoFactory; import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
@ -63,18 +64,20 @@ public class CherryPick extends SubmitStrategy {
} }
@Override @Override
protected CodeReviewCommit _run(CodeReviewCommit mergeTip, protected MergeTip _run(CodeReviewCommit branchTip,
Collection<CodeReviewCommit> toMerge) throws MergeException { Collection<CodeReviewCommit> toMerge) throws MergeException {
MergeTip mergeTip =
branchTip != null ? new MergeTip(branchTip, toMerge)
: null;
List<CodeReviewCommit> sorted = CodeReviewCommit.ORDER.sortedCopy(toMerge); List<CodeReviewCommit> sorted = CodeReviewCommit.ORDER.sortedCopy(toMerge);
while (!sorted.isEmpty()) { while (!sorted.isEmpty()) {
CodeReviewCommit n = sorted.remove(0); CodeReviewCommit n = sorted.remove(0);
try { try {
if (mergeTip == null) { if (mergeTip == null) {
// The branch is unborn. Take a fast-forward resolution to // The branch is unborn. Take a fast-forward resolution to
// create the branch. // create the branch.
// //
mergeTip = n; mergeTip = new MergeTip(n, Lists.newArrayList(n));
n.setStatusCode(CommitMergeStatus.CLEAN_MERGE); n.setStatusCode(CommitMergeStatus.CLEAN_MERGE);
} else if (n.getParentCount() == 0) { } else if (n.getParentCount() == 0) {
@ -89,8 +92,10 @@ public class CherryPick extends SubmitStrategy {
// that on the current merge tip. // that on the current merge tip.
// //
try { try {
mergeTip = writeCherryPickCommit(mergeTip, n); CodeReviewCommit merge = writeCherryPickCommit(mergeTip.getCurrentTip(), n);
newCommits.put(mergeTip.getPatchsetId().getParentKey(), mergeTip); mergeTip.moveTipTo(merge, merge.getName());
newCommits.put(mergeTip.getCurrentTip().getPatchsetId()
.getParentKey(), mergeTip.getCurrentTip());
} catch (MergeConflictException mce) { } catch (MergeConflictException mce) {
n.setStatusCode(CommitMergeStatus.PATH_CONFLICT); n.setStatusCode(CommitMergeStatus.PATH_CONFLICT);
mergeTip = null; mergeTip = null;
@ -106,15 +111,17 @@ public class CherryPick extends SubmitStrategy {
// instead behave as though MERGE_IF_NECESSARY was configured. // instead behave as though MERGE_IF_NECESSARY was configured.
// //
if (!args.mergeUtil.hasMissingDependencies(args.mergeSorter, n)) { if (!args.mergeUtil.hasMissingDependencies(args.mergeSorter, n)) {
if (args.rw.isMergedInto(mergeTip, n)) { if (args.rw.isMergedInto(mergeTip.getCurrentTip(), n)) {
mergeTip = n; mergeTip.moveTipTo(n, n.getName());
} else { } else {
mergeTip = args.mergeUtil.mergeOneCommit(args.serverIdent.get(), mergeTip.moveTipTo(
args.repo, args.rw, args.inserter, args.canMergeFlag, args.mergeUtil.mergeOneCommit(args.serverIdent.get(), args.repo,
args.destBranch, mergeTip, n); args.rw, args.inserter, args.canMergeFlag,
args.destBranch, mergeTip.getCurrentTip(), n), n.getName());
} }
PatchSetApproval submitApproval = args.mergeUtil.markCleanMerges( final PatchSetApproval submitApproval =
args.rw, args.canMergeFlag, mergeTip, args.alreadyAccepted); args.mergeUtil.markCleanMerges(args.rw, args.canMergeFlag,
mergeTip.getCurrentTip(), args.alreadyAccepted);
setRefLogIdent(submitApproval); setRefLogIdent(submitApproval);
} else { } else {
// One or more dependencies were not met. The status was // One or more dependencies were not met. The status was

View File

@ -18,6 +18,7 @@ import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.server.git.CodeReviewCommit; import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CommitMergeStatus; import com.google.gerrit.server.git.CommitMergeStatus;
import com.google.gerrit.server.git.MergeException; import com.google.gerrit.server.git.MergeException;
import com.google.gerrit.server.git.MergeTip;
import java.util.Collection; import java.util.Collection;
import java.util.List; import java.util.List;
@ -28,23 +29,26 @@ public class FastForwardOnly extends SubmitStrategy {
} }
@Override @Override
protected CodeReviewCommit _run(CodeReviewCommit mergeTip, protected MergeTip _run(final CodeReviewCommit branchTip,
Collection<CodeReviewCommit> toMerge) throws MergeException { final Collection<CodeReviewCommit> toMerge) throws MergeException {
MergeTip mergeTip = new MergeTip(branchTip, toMerge);
List<CodeReviewCommit> sorted = args.mergeUtil.reduceToMinimalMerge( List<CodeReviewCommit> sorted = args.mergeUtil.reduceToMinimalMerge(
args.mergeSorter, toMerge); args.mergeSorter, toMerge);
CodeReviewCommit newMergeTip = final CodeReviewCommit newMergeTipCommit =
args.mergeUtil.getFirstFastForward(mergeTip, args.rw, sorted); args.mergeUtil.getFirstFastForward(branchTip, args.rw, sorted);
mergeTip.moveTipTo(newMergeTipCommit, newMergeTipCommit.getName());
while (!sorted.isEmpty()) { while (!sorted.isEmpty()) {
CodeReviewCommit n = sorted.remove(0); final CodeReviewCommit n = sorted.remove(0);
n.setStatusCode(CommitMergeStatus.NOT_FAST_FORWARD); n.setStatusCode(CommitMergeStatus.NOT_FAST_FORWARD);
} }
PatchSetApproval submitApproval = args.mergeUtil.markCleanMerges(args.rw, PatchSetApproval submitApproval =
args.canMergeFlag, newMergeTip, args.alreadyAccepted); args.mergeUtil.markCleanMerges(args.rw, args.canMergeFlag, newMergeTipCommit,
args.alreadyAccepted);
setRefLogIdent(submitApproval); setRefLogIdent(submitApproval);
return newMergeTip; return mergeTip;
} }
@Override @Override

View File

@ -17,6 +17,7 @@ package com.google.gerrit.server.git.strategy;
import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.server.git.CodeReviewCommit; import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.MergeException; import com.google.gerrit.server.git.MergeException;
import com.google.gerrit.server.git.MergeTip;
import java.util.Collection; import java.util.Collection;
import java.util.List; import java.util.List;
@ -27,24 +28,29 @@ public class MergeAlways extends SubmitStrategy {
} }
@Override @Override
protected CodeReviewCommit _run(CodeReviewCommit mergeTip, protected MergeTip _run(CodeReviewCommit branchTip,
Collection<CodeReviewCommit> toMerge) throws MergeException { Collection<CodeReviewCommit> toMerge) throws MergeException {
List<CodeReviewCommit> sorted = args.mergeUtil.reduceToMinimalMerge( List<CodeReviewCommit> sorted = args.mergeUtil.reduceToMinimalMerge(args.mergeSorter, toMerge);
args.mergeSorter, toMerge); MergeTip mergeTip;
if (branchTip == null) {
if (mergeTip == null) {
// The branch is unborn. Take a fast-forward resolution to // The branch is unborn. Take a fast-forward resolution to
// create the branch. // create the branch.
mergeTip = sorted.remove(0); mergeTip = new MergeTip(sorted.get(0), toMerge);
sorted.remove(0);
} else {
mergeTip = new MergeTip(branchTip, toMerge);
} }
while (!sorted.isEmpty()) { while (!sorted.isEmpty()) {
mergeTip = args.mergeUtil.mergeOneCommit(args.serverIdent.get(), CodeReviewCommit mergedFrom = sorted.remove(0);
args.repo, args.rw, args.inserter, args.canMergeFlag, args.destBranch, CodeReviewCommit newTip =
mergeTip, sorted.remove(0)); args.mergeUtil.mergeOneCommit(args.serverIdent.get(), args.repo, args.rw,
args.inserter, args.canMergeFlag, args.destBranch, mergeTip.getCurrentTip(),
mergedFrom);
mergeTip.moveTipTo(newTip, mergedFrom.getName());
} }
PatchSetApproval submitApproval = final PatchSetApproval submitApproval =
args.mergeUtil.markCleanMerges(args.rw, args.canMergeFlag, mergeTip, args.mergeUtil.markCleanMerges(args.rw, args.canMergeFlag, mergeTip.getCurrentTip(),
args.alreadyAccepted); args.alreadyAccepted);
setRefLogIdent(submitApproval); setRefLogIdent(submitApproval);

View File

@ -17,6 +17,7 @@ package com.google.gerrit.server.git.strategy;
import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.server.git.CodeReviewCommit; import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.MergeException; import com.google.gerrit.server.git.MergeException;
import com.google.gerrit.server.git.MergeTip;
import java.util.Collection; import java.util.Collection;
import java.util.List; import java.util.List;
@ -27,29 +28,36 @@ public class MergeIfNecessary extends SubmitStrategy {
} }
@Override @Override
protected CodeReviewCommit _run(CodeReviewCommit mergeTip, protected MergeTip _run(CodeReviewCommit branchTip,
Collection<CodeReviewCommit> toMerge) throws MergeException { Collection<CodeReviewCommit> toMerge) throws MergeException {
List<CodeReviewCommit> sorted = args.mergeUtil.reduceToMinimalMerge( List<CodeReviewCommit> sorted =
args.mergeSorter, toMerge); args.mergeUtil.reduceToMinimalMerge(args.mergeSorter, toMerge);
MergeTip mergeTip;
if (mergeTip == null) { if (branchTip == null) {
// The branch is unborn. Take a fast-forward resolution to // The branch is unborn. Take a fast-forward resolution to
// create the branch. // create the branch.
mergeTip = sorted.remove(0); mergeTip = new MergeTip(sorted.get(0), toMerge);
branchTip = sorted.remove(0);
} else {
mergeTip = new MergeTip(branchTip, toMerge);
branchTip =
args.mergeUtil.getFirstFastForward(branchTip, args.rw, sorted);
} }
mergeTip.moveTipTo(branchTip, branchTip.getName());
mergeTip =
args.mergeUtil.getFirstFastForward(mergeTip, args.rw, sorted);
// For every other commit do a pair-wise merge. // For every other commit do a pair-wise merge.
while (!sorted.isEmpty()) { while (!sorted.isEmpty()) {
mergeTip = args.mergeUtil.mergeOneCommit(args.serverIdent.get(), CodeReviewCommit mergedFrom = sorted.remove(0);
args.repo, args.rw, args.inserter, args.canMergeFlag, args.destBranch, branchTip =
mergeTip, sorted.remove(0)); args.mergeUtil.mergeOneCommit(args.serverIdent.get(), args.repo,
args.rw, args.inserter, args.canMergeFlag, args.destBranch,
branchTip, mergedFrom);
mergeTip.moveTipTo(branchTip, mergedFrom.getName());
} }
PatchSetApproval submitApproval = args.mergeUtil.markCleanMerges( final PatchSetApproval submitApproval =
args.rw, args.canMergeFlag, mergeTip, args.alreadyAccepted); args.mergeUtil.markCleanMerges(args.rw, args.canMergeFlag, branchTip,
args.alreadyAccepted);
setRefLogIdent(submitApproval); setRefLogIdent(submitApproval);
return mergeTip; return mergeTip;

View File

@ -25,6 +25,7 @@ import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CommitMergeStatus; import com.google.gerrit.server.git.CommitMergeStatus;
import com.google.gerrit.server.git.MergeConflictException; import com.google.gerrit.server.git.MergeConflictException;
import com.google.gerrit.server.git.MergeException; import com.google.gerrit.server.git.MergeException;
import com.google.gerrit.server.git.MergeTip;
import com.google.gerrit.server.git.RebaseSorter; import com.google.gerrit.server.git.RebaseSorter;
import com.google.gerrit.server.patch.PatchSetInfoFactory; import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gerrit.server.project.InvalidChangeOperationException;
@ -55,20 +56,19 @@ public class RebaseIfNecessary extends SubmitStrategy {
} }
@Override @Override
protected CodeReviewCommit _run(CodeReviewCommit mergeTip, protected MergeTip _run(final CodeReviewCommit branchTip,
Collection<CodeReviewCommit> toMerge) throws MergeException { final Collection<CodeReviewCommit> toMerge) throws MergeException {
CodeReviewCommit newMergeTip = mergeTip; MergeTip mergeTip = new MergeTip(branchTip, toMerge);
List<CodeReviewCommit> sorted = sort(toMerge); List<CodeReviewCommit> sorted = sort(toMerge);
while (!sorted.isEmpty()) { while (!sorted.isEmpty()) {
CodeReviewCommit n = sorted.remove(0); CodeReviewCommit n = sorted.remove(0);
if (newMergeTip == null) { if (mergeTip.getCurrentTip() == null) {
// The branch is unborn. Take a fast-forward resolution to // The branch is unborn. Take a fast-forward resolution to
// create the branch. // create the branch.
// //
newMergeTip = n;
n.setStatusCode(CommitMergeStatus.CLEAN_MERGE); n.setStatusCode(CommitMergeStatus.CLEAN_MERGE);
mergeTip.moveTipTo(n, n.getName());
} else if (n.getParentCount() == 0) { } else if (n.getParentCount() == 0) {
// Refuse to merge a root commit into an existing branch, // Refuse to merge a root commit into an existing branch,
@ -77,37 +77,44 @@ public class RebaseIfNecessary extends SubmitStrategy {
n.setStatusCode(CommitMergeStatus.CANNOT_REBASE_ROOT); n.setStatusCode(CommitMergeStatus.CANNOT_REBASE_ROOT);
} else if (n.getParentCount() == 1) { } else if (n.getParentCount() == 1) {
if (args.mergeUtil.canFastForward( if (args.mergeUtil.canFastForward(args.mergeSorter,
args.mergeSorter, newMergeTip, args.rw, n)) { mergeTip.getCurrentTip(), args.rw, n)) {
newMergeTip = n;
n.setStatusCode(CommitMergeStatus.CLEAN_MERGE); n.setStatusCode(CommitMergeStatus.CLEAN_MERGE);
mergeTip.moveTipTo(n, n.getName());
} else { } else {
try { try {
IdentifiedUser uploader = args.identifiedUserFactory.create( IdentifiedUser uploader =
args.mergeUtil.getSubmitter(n).getAccountId()); args.identifiedUserFactory.create(args.mergeUtil
PatchSet newPatchSet = rebaseChange.rebase(args.repo, args.rw, .getSubmitter(n).getAccountId());
args.inserter, n.getPatchsetId(), n.change(), uploader, PatchSet newPatchSet =
newMergeTip, args.mergeUtil, args.serverIdent.get(), false, rebaseChange.rebase(args.repo, args.rw, args.inserter,
false, ValidatePolicy.NONE); n.getPatchsetId(), n.change(), uploader,
mergeTip.getCurrentTip(), args.mergeUtil,
args.serverIdent.get(), false, false, ValidatePolicy.NONE);
List<PatchSetApproval> approvals = Lists.newArrayList(); List<PatchSetApproval> approvals = Lists.newArrayList();
for (PatchSetApproval a : args.approvalsUtil.byPatchSet( for (PatchSetApproval a : args.approvalsUtil.byPatchSet(args.db,
args.db, n.getControl(), n.getPatchsetId())) { n.getControl(), n.getPatchsetId())) {
approvals.add(new PatchSetApproval(newPatchSet.getId(), a)); approvals.add(new PatchSetApproval(newPatchSet.getId(), a));
} }
// rebaseChange.rebase() may already have copied some approvals, // rebaseChange.rebase() may already have copied some approvals,
// use upsert, not insert, to avoid constraint violation on database // use upsert, not insert, to avoid constraint violation on database
args.db.patchSetApprovals().upsert(approvals); args.db.patchSetApprovals().upsert(approvals);
newMergeTip = (CodeReviewCommit) args.rw.parseCommit( mergeTip.moveTipTo((CodeReviewCommit) args.rw.parseCommit(ObjectId
ObjectId.fromString(newPatchSet.getRevision().get())); .fromString(newPatchSet.getRevision().get())), newPatchSet
.getRevision().get());
n.change().setCurrentPatchSet( n.change().setCurrentPatchSet(
patchSetInfoFactory.get(newMergeTip, newPatchSet.getId())); patchSetInfoFactory.get(mergeTip.getCurrentTip(),
newMergeTip.copyFrom(n); newPatchSet.getId()));
newMergeTip.setControl(args.changeControlFactory.controlFor(n.change(), uploader)); mergeTip.getCurrentTip().copyFrom(n);
newMergeTip.setPatchsetId(newPatchSet.getId()); mergeTip.getCurrentTip().setControl(
newMergeTip.setStatusCode(CommitMergeStatus.CLEAN_REBASE); args.changeControlFactory.controlFor(n.change(), uploader));
newCommits.put(newPatchSet.getId().getParentKey(), newMergeTip); mergeTip.getCurrentTip().setPatchsetId(newPatchSet.getId());
mergeTip.getCurrentTip().setStatusCode(
CommitMergeStatus.CLEAN_REBASE);
newCommits.put(newPatchSet.getId().getParentKey(),
mergeTip.getCurrentTip());
setRefLogIdent(args.mergeUtil.getSubmitter(n)); setRefLogIdent(args.mergeUtil.getSubmitter(n));
} catch (MergeConflictException e) { } catch (MergeConflictException e) {
n.setStatusCode(CommitMergeStatus.REBASE_MERGE_CONFLICT); n.setStatusCode(CommitMergeStatus.REBASE_MERGE_CONFLICT);
@ -125,25 +132,27 @@ public class RebaseIfNecessary extends SubmitStrategy {
// instead behave as though MERGE_IF_NECESSARY was configured. // instead behave as though MERGE_IF_NECESSARY was configured.
// //
try { try {
if (args.rw.isMergedInto(newMergeTip, n)) { if (args.rw.isMergedInto(mergeTip.getCurrentTip(), n)) {
newMergeTip = n; mergeTip.moveTipTo(n, n.getName());
} else { } else {
newMergeTip = args.mergeUtil.mergeOneCommit( mergeTip.moveTipTo(
args.serverIdent.get(), args.repo, args.rw, args.inserter, args.mergeUtil.mergeOneCommit(args.serverIdent.get(),
args.canMergeFlag, args.destBranch, newMergeTip, n); args.repo, args.rw, args.inserter, args.canMergeFlag,
args.destBranch, mergeTip.getCurrentTip(), n), n.getName());
} }
PatchSetApproval submitApproval = args.mergeUtil.markCleanMerges( PatchSetApproval submitApproval =
args.rw, args.canMergeFlag, newMergeTip, args.alreadyAccepted); args.mergeUtil.markCleanMerges(args.rw, args.canMergeFlag,
mergeTip.getCurrentTip(), args.alreadyAccepted);
setRefLogIdent(submitApproval); setRefLogIdent(submitApproval);
} catch (IOException e) { } catch (IOException e) {
throw new MergeException("Cannot merge " + n.name(), e); throw new MergeException("Cannot merge " + n.name(), e);
} }
} }
args.alreadyAccepted.add(newMergeTip); args.alreadyAccepted.add(mergeTip.getCurrentTip());
} }
return newMergeTip; return mergeTip;
} }
private List<CodeReviewCommit> sort(Collection<CodeReviewCommit> toSort) private List<CodeReviewCommit> sort(Collection<CodeReviewCommit> toSort)

View File

@ -24,6 +24,7 @@ import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.git.CodeReviewCommit; import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.MergeException; import com.google.gerrit.server.git.MergeException;
import com.google.gerrit.server.git.MergeSorter; import com.google.gerrit.server.git.MergeSorter;
import com.google.gerrit.server.git.MergeTip;
import com.google.gerrit.server.git.MergeUtil; import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.index.ChangeIndexer; import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ChangeControl;
@ -104,21 +105,21 @@ public abstract class SubmitStrategy {
* <p> * <p>
* If possible, the provided commits will be merged with this submit strategy. * If possible, the provided commits will be merged with this submit strategy.
* *
* @param mergeTip the merge tip. * @param currentTip the mergeTip
* @param toMerge the list of submitted commits that should be merged using * @param toMerge the list of submitted commits that should be merged using
* this submit strategy. Implementations are responsible for ordering * this submit strategy. Implementations are responsible for ordering
* of commits, and should not modify the input in place. * of commits, and should not modify the input in place.
* @return the new merge tip. * @return the new merge tip.
* @throws MergeException * @throws MergeException
*/ */
public CodeReviewCommit run(CodeReviewCommit mergeTip, public final MergeTip run(final CodeReviewCommit currentTip,
Collection<CodeReviewCommit> toMerge) throws MergeException { final Collection<CodeReviewCommit> toMerge) throws MergeException {
refLogIdent = null; refLogIdent = null;
return _run(mergeTip, toMerge); return _run(currentTip, toMerge);
} }
/** @see #run(CodeReviewCommit, Collection) */ /** @see #run(CodeReviewCommit, Collection) */
protected abstract CodeReviewCommit _run(CodeReviewCommit mergeTip, protected abstract MergeTip _run(CodeReviewCommit currentTip,
Collection<CodeReviewCommit> toMerge) throws MergeException; Collection<CodeReviewCommit> toMerge) throws MergeException;
/** /**