diff --git a/Documentation/cmd-stream-events.txt b/Documentation/cmd-stream-events.txt index ba3d6092f8..c754f35019 100644 --- a/Documentation/cmd-stream-events.txt +++ b/Documentation/cmd-stream-events.txt @@ -72,6 +72,8 @@ patchSet:: link:json.html#patchSet[patchSet 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 created. diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java index 07e5c39713..16a25cd7a3 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java @@ -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 com.google.common.collect.Lists; +import com.google.common.collect.Maps; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.GitUtil; import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.acceptance.RestResponse; 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.SubmitInput; 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.Project; 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.project.ListBranches.BranchInfo; import com.google.gerrit.server.project.PutConfig; import com.google.gson.reflect.TypeToken; import com.google.gwtorm.server.OrmException; @@ -66,16 +74,39 @@ import java.io.IOException; import java.sql.Timestamp; import java.util.Collections; import java.util.List; +import java.util.Map; public abstract class AbstractSubmit extends AbstractDaemonTest { + + private Map mergeResults; + @Inject private ChangeNotes.Factory notesFactory; @Inject private ApprovalsUtil approvalsUtil; + @Inject + private IdentifiedUser.GenericFactory factory; + + @Inject + ChangeHooks hooks; + @Before 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"); } @@ -181,10 +212,28 @@ public abstract class AbstractSubmit extends AbstractDaemonTest { newGson().fromJson(r.getReader(), new TypeToken() {}.getType()); assertThat(change.status).isEqualTo(ChangeStatus.MERGED); + + checkMergeResult(change); } 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() {}.getType()); + assertThat(branch.revision).isEqualTo( + mergeResults.get(Integer.toString(change._number))); + } + b.consume(); + } + private void approve(String changeId) throws IOException { RestResponse r = adminSession.post( "/changes/" + changeId + "/revisions/current/review", diff --git a/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java b/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java index 179497e782..61bec009e6 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java +++ b/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java @@ -446,14 +446,16 @@ public class ChangeHookRunner implements ChangeHooks, LifecycleListener { } @Override - public void doChangeMergedHook(final Change change, final Account account, - final PatchSet patchSet, final ReviewDb db) throws OrmException { + public void doChangeMergedHook(final Change change, final Account account, + final PatchSet patchSet, final ReviewDb db, String mergeResultRev) + throws OrmException { final ChangeMergedEvent event = new ChangeMergedEvent(); final AccountState owner = accountCache.get(change.getOwner()); event.change = eventFactory.asChangeAttribute(change); event.submitter = eventFactory.asAccountAttribute(account); event.patchSet = eventFactory.asPatchSetAttribute(patchSet); + event.newRev = mergeResultRev; fireEvent(change, event, db); final List args = new ArrayList<>(); @@ -465,6 +467,7 @@ public class ChangeHookRunner implements ChangeHooks, LifecycleListener { addArg(args, "--topic", event.change.topic); addArg(args, "--submitter", getDisplayName(account)); addArg(args, "--commit", event.patchSet.revision); + addArg(args, "--newrev", mergeResultRev); runHook(change.getProject(), changeMergedHook, args); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHooks.java b/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHooks.java index 550b5f0117..c4a8e00150 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHooks.java +++ b/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHooks.java @@ -79,10 +79,11 @@ public interface ChangeHooks { * @param change The change itself. * @param account The gerrit user who submitted the change. * @param patchSet The patchset that was merged. + * @param mergeResultRev The SHA-1 of the merge result revision. * @throws OrmException */ 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. diff --git a/gerrit-server/src/main/java/com/google/gerrit/common/DisabledChangeHooks.java b/gerrit-server/src/main/java/com/google/gerrit/common/DisabledChangeHooks.java index 27b54bc1a3..e719109e21 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/common/DisabledChangeHooks.java +++ b/gerrit-server/src/main/java/com/google/gerrit/common/DisabledChangeHooks.java @@ -44,7 +44,7 @@ public final class DisabledChangeHooks implements ChangeHooks { @Override public void doChangeMergedHook(Change change, Account account, - PatchSet patchSet, ReviewDb db) { + PatchSet patchSet, ReviewDb db, String mergeResultRev) { } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/ChangeMergedEvent.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/ChangeMergedEvent.java index f1aaa0a880..0bf7330a49 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/events/ChangeMergedEvent.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/events/ChangeMergedEvent.java @@ -27,6 +27,7 @@ public class ChangeMergedEvent extends ChangeEvent { public ChangeAttribute change; public PatchSetAttribute patchSet; public AccountAttribute submitter; + public String newRev; @Override public String getType() { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java index e4c6e0a2a7..106c1e3b8c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java @@ -171,7 +171,7 @@ public class MergeOp { private RevWalk rw; private RevFlag canMergeFlag; private CodeReviewCommit branchTip; - private CodeReviewCommit mergeTip; + private MergeTip mergeTip; private ObjectInserter inserter; private PersonIdent refLogIdent; @@ -277,11 +277,11 @@ public class MergeOp { branchUpdate = openBranch(); } SubmitStrategy strategy = createStrategy(submitType); - preMerge(strategy, toMerge.get(submitType)); + MergeTip mergeTip = preMerge(strategy, toMerge.get(submitType)); RefUpdate update = updateBranch(strategy, branchUpdate); reopen = true; - updateChangeStatus(toSubmit.get(submitType)); + updateChangeStatus(toSubmit.get(submitType), mergeTip); updateSubscriptions(toSubmit.get(submitType)); if (update != null) { fireRefUpdated(update); @@ -315,7 +315,7 @@ public class MergeOp { logDebug("Adding {} changes to merge on next run", toMerge.size()); } - updateChangeStatus(toUpdate); + updateChangeStatus(toUpdate, mergeTip); for (CodeReviewCommit commit : potentiallyStillSubmittableOnNextRun) { Capable capable = isSubmitStillPossible(commit); @@ -403,7 +403,7 @@ public class MergeOp { return true; } - private void preMerge(SubmitStrategy strategy, + private MergeTip preMerge(SubmitStrategy strategy, List toMerge) throws MergeException { logDebug("Running submit strategy {} for {} commits", strategy.getClass().getSimpleName(), toMerge.size()); @@ -411,6 +411,7 @@ public class MergeOp { refLogIdent = strategy.getRefLogIdent(); logDebug("Produced {} new commits", strategy.getNewCommits().size()); commits.putAll(strategy.getNewCommits()); + return mergeTip; } private SubmitStrategy createStrategy(SubmitType submitType) @@ -604,7 +605,7 @@ public class MergeOp { idstr, ps.getId()); commit.setStatusCode(CommitMergeStatus.ALREADY_MERGED); try { - setMerged(chg, null); + setMerged(chg, null, commit.getName()); } catch (OrmException e) { logError("Cannot mark change " + chg.getId() + " merged", e); } @@ -650,11 +651,13 @@ public class MergeOp { private RefUpdate updateBranch(SubmitStrategy strategy, 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", - mergeTip.name()); + currentTip.name()); return null; - } else if (mergeTip == null) { + } else if (currentTip == null) { logDebug("No merge tip, no update to perform"); return null; } @@ -664,17 +667,17 @@ public class MergeOp { try { ProjectConfig cfg = new ProjectConfig(destProject.getProject().getNameKey()); - cfg.load(repo, mergeTip); + cfg.load(repo, currentTip); } catch (Exception e) { throw new MergeException("Submit would store invalid" - + " project configuration " + mergeTip.name() + " for " + + " project configuration " + currentTip.name() + " for " + destProject.getProject().getName(), e); } } branchUpdate.setRefLogIdent(refLogIdent); branchUpdate.setForceUpdate(false); - branchUpdate.setNewObjectId(mergeTip); + branchUpdate.setNewObjectId(currentTip); branchUpdate.setRefLogMessage("merged", true); try { RefUpdate.Result result = branchUpdate.update(rw); @@ -688,7 +691,7 @@ public class MergeOp { tagCache.updateFastForward(destBranch.getParentKey(), branchUpdate.getName(), branchUpdate.getOldObjectId(), - mergeTip); + currentTip); } if (RefNames.REFS_CONFIG.equals(branchUpdate.getName())) { @@ -722,7 +725,8 @@ public class MergeOp { private void fireRefUpdated(RefUpdate branchUpdate) { logDebug("Firing ref updated hooks for {}", branchUpdate.getName()); gitRefUpdated.fire(destBranch.getParentKey(), branchUpdate); - hooks.doRefUpdatedHook(destBranch, branchUpdate, getAccount(mergeTip)); + hooks.doRefUpdatedHook(destBranch, branchUpdate, + getAccount(mergeTip.getCurrentTip())); } private Account getAccount(CodeReviewCommit codeReviewCommit) { @@ -743,7 +747,7 @@ public class MergeOp { return ""; } - private void updateChangeStatus(List submitted) + private void updateChangeStatus(List submitted, MergeTip mergeTip) throws NoSuchChangeException { logDebug("Updating change status for {} changes", submitted.size()); for (Change c : submitted) { @@ -761,21 +765,25 @@ public class MergeOp { String txt = s.getMessage(); logDebug("Status of change {} ({}) on {}: {}", c.getId(), commit.name(), 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 { switch (s) { case CLEAN_MERGE: - setMerged(c, message(c, txt + getByAccountName(commit))); + setMerged(c, message(c, txt + getByAccountName(commit)), + mergeResultRev); break; case CLEAN_REBASE: case CLEAN_PICK: setMerged(c, message(c, txt + " as " + commit.name() - + getByAccountName(commit))); + + getByAccountName(commit)), mergeResultRev); break; case ALREADY_MERGED: - setMerged(c, null); + setMerged(c, null, mergeResultRev); break; case PATH_CONFLICT: @@ -810,13 +818,14 @@ public class MergeOp { } private void updateSubscriptions(List submitted) { - if (mergeTip != null && (branchTip == null || branchTip != mergeTip)) { + if (mergeTip != null + && (branchTip == null || branchTip != mergeTip.getCurrentTip())) { logDebug("Updating submodule subscriptions for {} changes", submitted.size()); SubmoduleOp subOp = - subOpFactory.create(destBranch, mergeTip, rw, repo, + subOpFactory.create(destBranch, mergeTip.getCurrentTip(), rw, repo, destProject.getProject(), submitted, commits, - getAccount(mergeTip)); + getAccount(mergeTip.getCurrentTip())); try { subOp.update(); } catch (SubmoduleException e) { @@ -928,7 +937,7 @@ public class MergeOp { return m; } - private void setMerged(Change c, ChangeMessage msg) + private void setMerged(Change c, ChangeMessage msg, String mergeResultRev) throws OrmException, IOException { logDebug("Setting change {} merged", c.getId()); ChangeUpdate update = null; @@ -954,6 +963,7 @@ public class MergeOp { cmUtil.addChangeMessage(db, update, msg); } db.commit(); + } finally { db.rollback(); } @@ -964,7 +974,7 @@ public class MergeOp { try { hooks.doChangeMergedHook(c, accountCache.get(submitter.getAccountId()).getAccount(), - merged, db); + merged, db, mergeResultRev); } catch (OrmException ex) { logError("Cannot run hook for submitted patch set " + c.getId(), ex); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeTip.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeTip.java new file mode 100644 index 0000000000..1d8a1d5cde --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeTip.java @@ -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 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 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<, > + */ + public Map getMergeResults() { + return this.mergeResults; + } + + /** + * + * @return The current tip of the current merge operation. + */ + public CodeReviewCommit getCurrentTip() { + return branchTip; + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java index 02f8643231..4a38242ae6 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java @@ -2147,7 +2147,7 @@ public class ReceiveCommits { hooks.doPatchsetCreatedHook(change, newPatchSet, db); if (mergedIntoRef != null) { hooks.doChangeMergedHook( - change, currentUser.getAccount(), newPatchSet, db); + change, currentUser.getAccount(), newPatchSet, db, newCommit.getName()); } if (magicBranch != null && magicBranch.submit) { @@ -2434,7 +2434,7 @@ public class ReceiveCommits { result.mergedIntoRef = refName; markChangeMergedByPush(db, result, result.changeCtl); hooks.doChangeMergedHook( - change, currentUser.getAccount(), result.newPatchSet, db); + change, currentUser.getAccount(), result.newPatchSet, db, commit.getName()); sendMergedEmail(result); return change.getKey(); } 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 35e0689428..c9fe1d5962 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 @@ -30,6 +30,7 @@ import com.google.gerrit.server.git.CommitMergeStatus; import com.google.gerrit.server.git.MergeConflictException; import com.google.gerrit.server.git.MergeException; 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.project.NoSuchChangeException; import com.google.gwtorm.server.OrmException; @@ -63,18 +64,20 @@ public class CherryPick extends SubmitStrategy { } @Override - protected CodeReviewCommit _run(CodeReviewCommit mergeTip, + protected MergeTip _run(CodeReviewCommit branchTip, Collection toMerge) throws MergeException { - List sorted = CodeReviewCommit.ORDER.sortedCopy(toMerge); - while (!sorted.isEmpty()) { - CodeReviewCommit n = sorted.remove(0); - + MergeTip mergeTip = + branchTip != null ? new MergeTip(branchTip, toMerge) + : null; + List sorted = CodeReviewCommit.ORDER.sortedCopy(toMerge); + while (!sorted.isEmpty()) { + CodeReviewCommit n = sorted.remove(0); try { if (mergeTip == null) { // The branch is unborn. Take a fast-forward resolution to // create the branch. // - mergeTip = n; + mergeTip = new MergeTip(n, Lists.newArrayList(n)); n.setStatusCode(CommitMergeStatus.CLEAN_MERGE); } else if (n.getParentCount() == 0) { @@ -89,8 +92,10 @@ public class CherryPick extends SubmitStrategy { // that on the current merge tip. // try { - mergeTip = writeCherryPickCommit(mergeTip, n); - newCommits.put(mergeTip.getPatchsetId().getParentKey(), mergeTip); + CodeReviewCommit merge = writeCherryPickCommit(mergeTip.getCurrentTip(), n); + mergeTip.moveTipTo(merge, merge.getName()); + newCommits.put(mergeTip.getCurrentTip().getPatchsetId() + .getParentKey(), mergeTip.getCurrentTip()); } catch (MergeConflictException mce) { n.setStatusCode(CommitMergeStatus.PATH_CONFLICT); mergeTip = null; @@ -106,15 +111,17 @@ public class CherryPick extends SubmitStrategy { // instead behave as though MERGE_IF_NECESSARY was configured. // if (!args.mergeUtil.hasMissingDependencies(args.mergeSorter, n)) { - if (args.rw.isMergedInto(mergeTip, n)) { - mergeTip = n; + if (args.rw.isMergedInto(mergeTip.getCurrentTip(), n)) { + mergeTip.moveTipTo(n, n.getName()); } else { - mergeTip = args.mergeUtil.mergeOneCommit(args.serverIdent.get(), - args.repo, args.rw, args.inserter, args.canMergeFlag, - args.destBranch, mergeTip, n); - } - PatchSetApproval submitApproval = args.mergeUtil.markCleanMerges( - args.rw, args.canMergeFlag, mergeTip, args.alreadyAccepted); + mergeTip.moveTipTo( + args.mergeUtil.mergeOneCommit(args.serverIdent.get(), args.repo, + args.rw, args.inserter, args.canMergeFlag, + args.destBranch, mergeTip.getCurrentTip(), n), n.getName()); + } + final PatchSetApproval submitApproval = + args.mergeUtil.markCleanMerges(args.rw, args.canMergeFlag, + mergeTip.getCurrentTip(), args.alreadyAccepted); setRefLogIdent(submitApproval); } else { // One or more dependencies were not met. The status was diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/FastForwardOnly.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/FastForwardOnly.java index 9e37e1103d..cdf3a9cb53 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/FastForwardOnly.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/FastForwardOnly.java @@ -18,6 +18,7 @@ import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.server.git.CodeReviewCommit; import com.google.gerrit.server.git.CommitMergeStatus; import com.google.gerrit.server.git.MergeException; +import com.google.gerrit.server.git.MergeTip; import java.util.Collection; import java.util.List; @@ -28,23 +29,26 @@ public class FastForwardOnly extends SubmitStrategy { } @Override - protected CodeReviewCommit _run(CodeReviewCommit mergeTip, - Collection toMerge) throws MergeException { + protected MergeTip _run(final CodeReviewCommit branchTip, + final Collection toMerge) throws MergeException { + MergeTip mergeTip = new MergeTip(branchTip, toMerge); List sorted = args.mergeUtil.reduceToMinimalMerge( args.mergeSorter, toMerge); - CodeReviewCommit newMergeTip = - args.mergeUtil.getFirstFastForward(mergeTip, args.rw, sorted); + final CodeReviewCommit newMergeTipCommit = + args.mergeUtil.getFirstFastForward(branchTip, args.rw, sorted); + mergeTip.moveTipTo(newMergeTipCommit, newMergeTipCommit.getName()); while (!sorted.isEmpty()) { - CodeReviewCommit n = sorted.remove(0); + final CodeReviewCommit n = sorted.remove(0); n.setStatusCode(CommitMergeStatus.NOT_FAST_FORWARD); } - PatchSetApproval submitApproval = args.mergeUtil.markCleanMerges(args.rw, - args.canMergeFlag, newMergeTip, args.alreadyAccepted); + PatchSetApproval submitApproval = + args.mergeUtil.markCleanMerges(args.rw, args.canMergeFlag, newMergeTipCommit, + args.alreadyAccepted); setRefLogIdent(submitApproval); - return newMergeTip; + return mergeTip; } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/MergeAlways.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/MergeAlways.java index 5bf69e936f..cdf6799429 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/MergeAlways.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/MergeAlways.java @@ -17,6 +17,7 @@ package com.google.gerrit.server.git.strategy; import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.server.git.CodeReviewCommit; import com.google.gerrit.server.git.MergeException; +import com.google.gerrit.server.git.MergeTip; import java.util.Collection; import java.util.List; @@ -27,24 +28,29 @@ public class MergeAlways extends SubmitStrategy { } @Override - protected CodeReviewCommit _run(CodeReviewCommit mergeTip, + protected MergeTip _run(CodeReviewCommit branchTip, Collection toMerge) throws MergeException { - List sorted = args.mergeUtil.reduceToMinimalMerge( - args.mergeSorter, toMerge); - - if (mergeTip == null) { + List sorted = args.mergeUtil.reduceToMinimalMerge(args.mergeSorter, toMerge); + MergeTip mergeTip; + if (branchTip == null) { // The branch is unborn. Take a fast-forward resolution to // 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()) { - mergeTip = args.mergeUtil.mergeOneCommit(args.serverIdent.get(), - args.repo, args.rw, args.inserter, args.canMergeFlag, args.destBranch, - mergeTip, sorted.remove(0)); + CodeReviewCommit mergedFrom = sorted.remove(0); + CodeReviewCommit newTip = + 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 = - args.mergeUtil.markCleanMerges(args.rw, args.canMergeFlag, mergeTip, + final PatchSetApproval submitApproval = + args.mergeUtil.markCleanMerges(args.rw, args.canMergeFlag, mergeTip.getCurrentTip(), args.alreadyAccepted); setRefLogIdent(submitApproval); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/MergeIfNecessary.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/MergeIfNecessary.java index 1c172d05cc..af5d803ad0 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/MergeIfNecessary.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/MergeIfNecessary.java @@ -17,6 +17,7 @@ package com.google.gerrit.server.git.strategy; import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.server.git.CodeReviewCommit; import com.google.gerrit.server.git.MergeException; +import com.google.gerrit.server.git.MergeTip; import java.util.Collection; import java.util.List; @@ -27,29 +28,36 @@ public class MergeIfNecessary extends SubmitStrategy { } @Override - protected CodeReviewCommit _run(CodeReviewCommit mergeTip, + protected MergeTip _run(CodeReviewCommit branchTip, Collection toMerge) throws MergeException { - List sorted = args.mergeUtil.reduceToMinimalMerge( - args.mergeSorter, toMerge); - - if (mergeTip == null) { + List sorted = + args.mergeUtil.reduceToMinimalMerge(args.mergeSorter, toMerge); + MergeTip mergeTip; + if (branchTip == null) { // The branch is unborn. Take a fast-forward resolution to // 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 = - args.mergeUtil.getFirstFastForward(mergeTip, args.rw, sorted); + mergeTip.moveTipTo(branchTip, branchTip.getName()); // For every other commit do a pair-wise merge. while (!sorted.isEmpty()) { - mergeTip = args.mergeUtil.mergeOneCommit(args.serverIdent.get(), - args.repo, args.rw, args.inserter, args.canMergeFlag, args.destBranch, - mergeTip, sorted.remove(0)); + CodeReviewCommit mergedFrom = sorted.remove(0); + branchTip = + 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( - args.rw, args.canMergeFlag, mergeTip, args.alreadyAccepted); + final PatchSetApproval submitApproval = + args.mergeUtil.markCleanMerges(args.rw, args.canMergeFlag, branchTip, + args.alreadyAccepted); setRefLogIdent(submitApproval); return mergeTip; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java index 211cb2739d..482dae3a3e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java @@ -25,6 +25,7 @@ import com.google.gerrit.server.git.CodeReviewCommit; import com.google.gerrit.server.git.CommitMergeStatus; import com.google.gerrit.server.git.MergeConflictException; 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.patch.PatchSetInfoFactory; import com.google.gerrit.server.project.InvalidChangeOperationException; @@ -55,20 +56,19 @@ public class RebaseIfNecessary extends SubmitStrategy { } @Override - protected CodeReviewCommit _run(CodeReviewCommit mergeTip, - Collection toMerge) throws MergeException { - CodeReviewCommit newMergeTip = mergeTip; + protected MergeTip _run(final CodeReviewCommit branchTip, + final Collection toMerge) throws MergeException { + MergeTip mergeTip = new MergeTip(branchTip, toMerge); List sorted = sort(toMerge); - while (!sorted.isEmpty()) { CodeReviewCommit n = sorted.remove(0); - if (newMergeTip == null) { + if (mergeTip.getCurrentTip() == null) { // The branch is unborn. Take a fast-forward resolution to // create the branch. // - newMergeTip = n; n.setStatusCode(CommitMergeStatus.CLEAN_MERGE); + mergeTip.moveTipTo(n, n.getName()); } else if (n.getParentCount() == 0) { // 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); } else if (n.getParentCount() == 1) { - if (args.mergeUtil.canFastForward( - args.mergeSorter, newMergeTip, args.rw, n)) { - newMergeTip = n; + if (args.mergeUtil.canFastForward(args.mergeSorter, + mergeTip.getCurrentTip(), args.rw, n)) { n.setStatusCode(CommitMergeStatus.CLEAN_MERGE); + mergeTip.moveTipTo(n, n.getName()); } else { try { - IdentifiedUser uploader = args.identifiedUserFactory.create( - args.mergeUtil.getSubmitter(n).getAccountId()); - PatchSet newPatchSet = rebaseChange.rebase(args.repo, args.rw, - args.inserter, n.getPatchsetId(), n.change(), uploader, - newMergeTip, args.mergeUtil, args.serverIdent.get(), false, - false, ValidatePolicy.NONE); + IdentifiedUser uploader = + args.identifiedUserFactory.create(args.mergeUtil + .getSubmitter(n).getAccountId()); + PatchSet newPatchSet = + rebaseChange.rebase(args.repo, args.rw, args.inserter, + n.getPatchsetId(), n.change(), uploader, + mergeTip.getCurrentTip(), args.mergeUtil, + args.serverIdent.get(), false, false, ValidatePolicy.NONE); List approvals = Lists.newArrayList(); - for (PatchSetApproval a : args.approvalsUtil.byPatchSet( - args.db, n.getControl(), n.getPatchsetId())) { + for (PatchSetApproval a : args.approvalsUtil.byPatchSet(args.db, + n.getControl(), n.getPatchsetId())) { approvals.add(new PatchSetApproval(newPatchSet.getId(), a)); } // rebaseChange.rebase() may already have copied some approvals, // use upsert, not insert, to avoid constraint violation on database args.db.patchSetApprovals().upsert(approvals); - newMergeTip = (CodeReviewCommit) args.rw.parseCommit( - ObjectId.fromString(newPatchSet.getRevision().get())); + mergeTip.moveTipTo((CodeReviewCommit) args.rw.parseCommit(ObjectId + .fromString(newPatchSet.getRevision().get())), newPatchSet + .getRevision().get()); n.change().setCurrentPatchSet( - patchSetInfoFactory.get(newMergeTip, newPatchSet.getId())); - newMergeTip.copyFrom(n); - newMergeTip.setControl(args.changeControlFactory.controlFor(n.change(), uploader)); - newMergeTip.setPatchsetId(newPatchSet.getId()); - newMergeTip.setStatusCode(CommitMergeStatus.CLEAN_REBASE); - newCommits.put(newPatchSet.getId().getParentKey(), newMergeTip); + patchSetInfoFactory.get(mergeTip.getCurrentTip(), + newPatchSet.getId())); + mergeTip.getCurrentTip().copyFrom(n); + mergeTip.getCurrentTip().setControl( + args.changeControlFactory.controlFor(n.change(), uploader)); + mergeTip.getCurrentTip().setPatchsetId(newPatchSet.getId()); + mergeTip.getCurrentTip().setStatusCode( + CommitMergeStatus.CLEAN_REBASE); + newCommits.put(newPatchSet.getId().getParentKey(), + mergeTip.getCurrentTip()); setRefLogIdent(args.mergeUtil.getSubmitter(n)); } catch (MergeConflictException e) { n.setStatusCode(CommitMergeStatus.REBASE_MERGE_CONFLICT); @@ -125,25 +132,27 @@ public class RebaseIfNecessary extends SubmitStrategy { // instead behave as though MERGE_IF_NECESSARY was configured. // try { - if (args.rw.isMergedInto(newMergeTip, n)) { - newMergeTip = n; + if (args.rw.isMergedInto(mergeTip.getCurrentTip(), n)) { + mergeTip.moveTipTo(n, n.getName()); } else { - newMergeTip = args.mergeUtil.mergeOneCommit( - args.serverIdent.get(), args.repo, args.rw, args.inserter, - args.canMergeFlag, args.destBranch, newMergeTip, n); + mergeTip.moveTipTo( + args.mergeUtil.mergeOneCommit(args.serverIdent.get(), + args.repo, args.rw, args.inserter, args.canMergeFlag, + args.destBranch, mergeTip.getCurrentTip(), n), n.getName()); } - PatchSetApproval submitApproval = args.mergeUtil.markCleanMerges( - args.rw, args.canMergeFlag, newMergeTip, args.alreadyAccepted); + PatchSetApproval submitApproval = + args.mergeUtil.markCleanMerges(args.rw, args.canMergeFlag, + mergeTip.getCurrentTip(), args.alreadyAccepted); setRefLogIdent(submitApproval); } catch (IOException e) { throw new MergeException("Cannot merge " + n.name(), e); } } - args.alreadyAccepted.add(newMergeTip); + args.alreadyAccepted.add(mergeTip.getCurrentTip()); } - return newMergeTip; + return mergeTip; } private List sort(Collection toSort) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java index e38e749920..cba32c91e9 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java @@ -24,6 +24,7 @@ import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.git.CodeReviewCommit; import com.google.gerrit.server.git.MergeException; 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.index.ChangeIndexer; import com.google.gerrit.server.project.ChangeControl; @@ -104,21 +105,21 @@ public abstract class SubmitStrategy { *

* 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 * this submit strategy. Implementations are responsible for ordering * of commits, and should not modify the input in place. * @return the new merge tip. * @throws MergeException */ - public CodeReviewCommit run(CodeReviewCommit mergeTip, - Collection toMerge) throws MergeException { + public final MergeTip run(final CodeReviewCommit currentTip, + final Collection toMerge) throws MergeException { refLogIdent = null; - return _run(mergeTip, toMerge); + return _run(currentTip, toMerge); } /** @see #run(CodeReviewCommit, Collection) */ - protected abstract CodeReviewCommit _run(CodeReviewCommit mergeTip, + protected abstract MergeTip _run(CodeReviewCommit currentTip, Collection toMerge) throws MergeException; /**