diff options
Diffstat (limited to 'gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java')
| -rw-r--r-- | gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java | 1317 |
1 files changed, 415 insertions, 902 deletions
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 2e8f183f0d..dace0ae90b 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 @@ -14,30 +14,35 @@ package com.google.gerrit.server.git; +import static com.google.gerrit.server.git.MergeUtil.getSubmitter; +import static java.util.concurrent.TimeUnit.DAYS; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.TimeUnit.MINUTES; +import static java.util.concurrent.TimeUnit.SECONDS; +import com.google.common.base.Objects; +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.Iterables; +import com.google.common.collect.ListMultimap; +import com.google.common.collect.Sets; import com.google.gerrit.common.ChangeHooks; -import com.google.gerrit.common.data.ApprovalType; -import com.google.gerrit.common.data.ApprovalTypes; import com.google.gerrit.common.data.Capable; +import com.google.gerrit.common.data.LabelTypes; +import com.google.gerrit.common.data.SubmitTypeRecord; import com.google.gerrit.reviewdb.client.Account; -import com.google.gerrit.reviewdb.client.ApprovalCategory; import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.ChangeMessage; import com.google.gerrit.reviewdb.client.PatchSet; -import com.google.gerrit.reviewdb.client.PatchSetAncestor; import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.client.Project; +import com.google.gerrit.reviewdb.client.Project.SubmitType; import com.google.gerrit.reviewdb.client.RevId; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.ChangeUtil; -import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.config.AllProjectsName; -import com.google.gerrit.server.config.CanonicalWebUrl; import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.mail.MergeFailSender; import com.google.gerrit.server.mail.MergedSender; @@ -45,24 +50,20 @@ import com.google.gerrit.server.patch.PatchSetInfoFactory; import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.NoSuchChangeException; +import com.google.gerrit.server.project.NoSuchProjectException; import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.util.RequestScopePropagator; -import com.google.gerrit.server.workflow.CategoryFunction; -import com.google.gerrit.server.workflow.FunctionState; import com.google.gwtorm.server.AtomicUpdate; import com.google.gwtorm.server.OrmConcurrencyException; import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.SchemaFactory; import com.google.inject.Inject; -import com.google.inject.Provider; import com.google.inject.assistedinject.Assisted; import org.eclipse.jgit.errors.IncorrectObjectTypeException; -import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.errors.RepositoryNotFoundException; import org.eclipse.jgit.lib.AnyObjectId; -import org.eclipse.jgit.lib.CommitBuilder; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectInserter; @@ -70,11 +71,6 @@ import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.RefUpdate; import org.eclipse.jgit.lib.Repository; -import org.eclipse.jgit.merge.MergeStrategy; -import org.eclipse.jgit.merge.Merger; -import org.eclipse.jgit.merge.ThreeWayMerger; -import org.eclipse.jgit.revwalk.FooterKey; -import org.eclipse.jgit.revwalk.FooterLine; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevFlag; import org.eclipse.jgit.revwalk.RevSort; @@ -83,21 +79,15 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.io.IOException; -import java.io.UnsupportedEncodingException; -import java.sql.Timestamp; import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; -import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Set; -import java.util.TimeZone; - -import javax.annotation.Nullable; /** * Merges changes in submission order into a single branch. @@ -119,53 +109,47 @@ public class MergeOp { } private static final Logger log = LoggerFactory.getLogger(MergeOp.class); - private static final String R_HEADS_MASTER = - Constants.R_HEADS + Constants.MASTER; - private static final ApprovalCategory.Id CRVW = - new ApprovalCategory.Id("CRVW"); - private static final ApprovalCategory.Id VRIF = - new ApprovalCategory.Id("VRIF"); - private static final FooterKey REVIEWED_ON = new FooterKey("Reviewed-on"); - private static final FooterKey CHANGE_ID = new FooterKey("Change-Id"); /** Amount of time to wait between submit and checking for missing deps. */ private static final long DEPENDENCY_DELAY = MILLISECONDS.convert(15, MINUTES); + private static final long LOCK_FAILURE_RETRY_DELAY = + MILLISECONDS.convert(15, SECONDS); + + private static final long DUPLICATE_MESSAGE_INTERVAL = + MILLISECONDS.convert(1, DAYS); + private final GitRepositoryManager repoManager; private final SchemaFactory<ReviewDb> schemaFactory; private final ProjectCache projectCache; - private final FunctionState.Factory functionState; - private final GitReferenceUpdated replication; + private final LabelNormalizer labelNormalizer; + private final GitReferenceUpdated gitRefUpdated; private final MergedSender.Factory mergedSenderFactory; private final MergeFailSender.Factory mergeFailSenderFactory; - private final Provider<String> urlProvider; - private final ApprovalTypes approvalTypes; private final PatchSetInfoFactory patchSetInfoFactory; private final IdentifiedUser.GenericFactory identifiedUserFactory; private final ChangeControl.GenericFactory changeControlFactory; private final MergeQueue mergeQueue; - private final PersonIdent myIdent; private final Branch.NameKey destBranch; - private Project destProject; - private final List<CodeReviewCommit> toMerge; - private List<Change> submitted; + private ProjectState destProject; + private final ListMultimap<SubmitType, CodeReviewCommit> toMerge; + private final List<CodeReviewCommit> potentiallyStillSubmittable; private final Map<Change.Id, CodeReviewCommit> commits; private ReviewDb db; private Repository repo; private RevWalk rw; - private RevFlag CAN_MERGE; + private RevFlag canMergeFlag; private CodeReviewCommit branchTip; private CodeReviewCommit mergeTip; - private Set<RevCommit> alreadyAccepted; - private RefUpdate branchUpdate; private ObjectInserter inserter; + private PersonIdent refLogIdent; private final ChangeHooks hooks; private final AccountCache accountCache; private final TagCache tagCache; - private final CreateCodeReviewNotes.Factory codeReviewNotesFactory; + private final SubmitStrategyFactory submitStrategyFactory; private final SubmoduleOp.Factory subOpFactory; private final WorkQueue workQueue; private final RequestScopePropagator requestScopePropagator; @@ -173,30 +157,27 @@ public class MergeOp { @Inject MergeOp(final GitRepositoryManager grm, final SchemaFactory<ReviewDb> sf, - final ProjectCache pc, final FunctionState.Factory fs, - final GitReferenceUpdated rq, final MergedSender.Factory msf, + final ProjectCache pc, final LabelNormalizer fs, + final GitReferenceUpdated gru, final MergedSender.Factory msf, final MergeFailSender.Factory mfsf, - @CanonicalWebUrl @Nullable final Provider<String> cwu, - final ApprovalTypes approvalTypes, final PatchSetInfoFactory psif, + final LabelTypes labelTypes, final PatchSetInfoFactory psif, final IdentifiedUser.GenericFactory iuf, final ChangeControl.GenericFactory changeControlFactory, - @GerritPersonIdent final PersonIdent myIdent, final MergeQueue mergeQueue, @Assisted final Branch.NameKey branch, final ChangeHooks hooks, final AccountCache accountCache, - final TagCache tagCache, final CreateCodeReviewNotes.Factory crnf, + final TagCache tagCache, + final SubmitStrategyFactory submitStrategyFactory, final SubmoduleOp.Factory subOpFactory, final WorkQueue workQueue, final RequestScopePropagator requestScopePropagator, final AllProjectsName allProjectsName) { repoManager = grm; schemaFactory = sf; - functionState = fs; + labelNormalizer = fs; projectCache = pc; - replication = rq; + gitRefUpdated = gru; mergedSenderFactory = msf; mergeFailSenderFactory = mfsf; - urlProvider = cwu; - this.approvalTypes = approvalTypes; patchSetInfoFactory = psif; identifiedUserFactory = iuf; this.changeControlFactory = changeControlFactory; @@ -204,24 +185,22 @@ public class MergeOp { this.hooks = hooks; this.accountCache = accountCache; this.tagCache = tagCache; - codeReviewNotesFactory = crnf; + this.submitStrategyFactory = submitStrategyFactory; this.subOpFactory = subOpFactory; this.workQueue = workQueue; this.requestScopePropagator = requestScopePropagator; this.allProjectsName = allProjectsName; - this.myIdent = myIdent; destBranch = branch; - toMerge = new ArrayList<CodeReviewCommit>(); + toMerge = ArrayListMultimap.create(); + potentiallyStillSubmittable = new ArrayList<CodeReviewCommit>(); commits = new HashMap<Change.Id, CodeReviewCommit>(); } - public void verifyMergeability(Change change) { + public void verifyMergeability(Change change) throws NoSuchProjectException { try { setDestProject(); openRepository(); final Ref destBranchRef = repo.getRef(destBranch.get()); - submitted = new ArrayList<Change>(); - submitted.add(change); // Test mergeability of the change if the last merged sha1 // in the branch is different from the last sha1 @@ -231,17 +210,27 @@ public class MergeOp { || (destBranchRef != null && !destBranchRef.getObjectId().getName() .equals(change.getLastSha1MergeTested().get()))) { openSchema(); - preMerge(); - - // update sha1 tested merge. - if (destBranchRef != null) { - change.setLastSha1MergeTested(new RevId(destBranchRef - .getObjectId().getName())); + openBranch(); + validateChangeList(Collections.singletonList(change)); + if (!toMerge.isEmpty()) { + final Entry<SubmitType, CodeReviewCommit> e = + toMerge.entries().iterator().next(); + final boolean isMergeable = + createStrategy(e.getKey()).dryRun(branchTip, e.getValue()); + + // update sha1 tested merge. + if (destBranchRef != null) { + change.setLastSha1MergeTested(new RevId(destBranchRef + .getObjectId().getName())); + } else { + change.setLastSha1MergeTested(new RevId("")); + } + change.setMergeable(isMergeable); + db.changes().update(Collections.singleton(change)); } else { - change.setLastSha1MergeTested(new RevId("")); + log.error("Test merge attempt for change: " + change.getId() + + " failed"); } - change.setMergeable(isMergeable(change)); - db.changes().update(Collections.singleton(change)); } } catch (MergeException e) { log.error("Test merge attempt for change: " + change.getId() @@ -263,11 +252,10 @@ public class MergeOp { } private void setDestProject() throws MergeException { - final ProjectState pe = projectCache.get(destBranch.getParentKey()); - if (pe == null) { + destProject = projectCache.get(destBranch.getParentKey()); + if (destProject == null) { throw new MergeException("No such project: " + destBranch.getParentKey()); } - destProject = pe.getProject(); } private void openSchema() throws OrmException { @@ -276,16 +264,59 @@ public class MergeOp { } } - public void merge() throws MergeException { + public void merge() throws MergeException, NoSuchProjectException { setDestProject(); try { openSchema(); openRepository(); - submitted = db.changes().submitted(destBranch).toList(); - preMerge(); - updateBranch(); - updateChangeStatus(); - updateSubscriptions(); + openBranch(); + final ListMultimap<SubmitType, Change> toSubmit = + validateChangeList(db.changes().submitted(destBranch).toList()); + + final ListMultimap<SubmitType, CodeReviewCommit> toMergeNextTurn = + ArrayListMultimap.create(); + final List<CodeReviewCommit> potentiallyStillSubmittableOnNextRun = + new ArrayList<CodeReviewCommit>(); + while (!toMerge.isEmpty()) { + toMergeNextTurn.clear(); + final Set<SubmitType> submitTypes = + new HashSet<Project.SubmitType>(toMerge.keySet()); + for (final SubmitType submitType : submitTypes) { + final RefUpdate branchUpdate = openBranch(); + final SubmitStrategy strategy = createStrategy(submitType); + preMerge(strategy, toMerge.get(submitType)); + updateBranch(strategy, branchUpdate); + updateChangeStatus(toSubmit.get(submitType)); + updateSubscriptions(toSubmit.get(submitType)); + + for (final Iterator<CodeReviewCommit> it = + potentiallyStillSubmittable.iterator(); it.hasNext();) { + final CodeReviewCommit commit = it.next(); + if (containsMissingCommits(toMerge, commit) + || containsMissingCommits(toMergeNextTurn, commit)) { + // change has missing dependencies, but all commits which are + // missing are still attempted to be merged with another submit + // strategy, retry to merge this commit in the next turn + it.remove(); + commit.statusCode = null; + commit.missing = null; + toMergeNextTurn.put(submitType, commit); + } + } + potentiallyStillSubmittableOnNextRun.addAll(potentiallyStillSubmittable); + potentiallyStillSubmittable.clear(); + } + toMerge.clear(); + toMerge.putAll(toMergeNextTurn); + } + + for (final CodeReviewCommit commit : potentiallyStillSubmittableOnNextRun) { + final Capable capable = isSubmitStillPossible(commit); + if (capable != Capable.OK) { + sendMergeFail(commit.change, + message(commit.change, capable.getMessage()), false); + } + } } catch (OrmException e) { throw new MergeException("Cannot query the database", e); } finally { @@ -304,24 +335,59 @@ public class MergeOp { } } - private void preMerge() throws MergeException, OrmException { - openBranch(); - validateChangeList(); - mergeTip = branchTip; - switch (destProject.getSubmitType()) { - case CHERRY_PICK: - cherryPickChanges(); - break; - - case FAST_FORWARD_ONLY: - case MERGE_ALWAYS: - case MERGE_IF_NECESSARY: - default: - reduceToMinimalMerge(); - mergeTopics(); - markCleanMerges(); - break; + private boolean containsMissingCommits( + final ListMultimap<SubmitType, CodeReviewCommit> map, + final CodeReviewCommit commit) { + if (!isSubmitForMissingCommitsStillPossible(commit)) { + return false; + } + + for (final CodeReviewCommit missingCommit : commit.missing) { + if (!map.containsValue(missingCommit)) { + return false; + } } + return true; + } + + private boolean isSubmitForMissingCommitsStillPossible(final CodeReviewCommit commit) { + if (commit.missing == null || commit.missing.isEmpty()) { + return false; + } + + for (CodeReviewCommit missingCommit : commit.missing) { + loadChangeInfo(missingCommit); + + if (missingCommit.patchsetId == null) { + // The commit doesn't have a patch set, so it cannot be + // submitted to the branch. + // + return false; + } + + if (!missingCommit.change.currentPatchSetId().equals( + missingCommit.patchsetId)) { + // If the missing commit is not the current patch set, + // the change must be rebased to use the proper parent. + // + return false; + } + } + + return true; + } + + private void preMerge(final SubmitStrategy strategy, + final List<CodeReviewCommit> toMerge) throws MergeException { + mergeTip = strategy.run(branchTip, toMerge); + refLogIdent = strategy.getRefLogIdent(); + commits.putAll(strategy.getNewCommits()); + } + + private SubmitStrategy createStrategy(final SubmitType submitType) + throws MergeException, NoSuchProjectException { + return submitStrategyFactory.create(submitType, db, repo, rw, inserter, + canMergeFlag, getAlreadyAccepted(branchTip), destBranch); } private void openRepository() throws MergeException { @@ -344,20 +410,17 @@ public class MergeOp { }; rw.sort(RevSort.TOPO); rw.sort(RevSort.COMMIT_TIME_DESC, true); - CAN_MERGE = rw.newFlag("CAN_MERGE"); + canMergeFlag = rw.newFlag("CAN_MERGE"); inserter = repo.newObjectInserter(); } - private void openBranch() throws MergeException { - alreadyAccepted = new HashSet<RevCommit>(); - + private RefUpdate openBranch() throws MergeException, OrmException { try { - branchUpdate = repo.updateRef(destBranch.get()); + final RefUpdate branchUpdate = repo.updateRef(destBranch.get()); if (branchUpdate.getOldObjectId() != null) { branchTip = (CodeReviewCommit) rw.parseCommit(branchUpdate.getOldObjectId()); - alreadyAccepted.add(branchTip); } else { branchTip = null; } @@ -369,14 +432,31 @@ public class MergeOp { } else if (repo.getFullBranch().equals(destBranch.get())) { branchUpdate.setExpectedOldObjectId(ObjectId.zeroId()); } else { - throw new MergeException("Destination branch \"" - + branchUpdate.getRef().getName() + "\" does not exist"); + for (final Change c : db.changes().submitted(destBranch).toList()) { + setNew(c, message(c, "Your change could not be merged, " + + "because the destination branch does not exist anymore.")); + } } } catch (IOException e) { throw new MergeException( "Failed to check existence of destination branch", e); } + return branchUpdate; + } catch (IOException e) { + throw new MergeException("Cannot open branch", e); + } + } + + private Set<RevCommit> getAlreadyAccepted(final CodeReviewCommit branchTip) + throws MergeException { + final Set<RevCommit> alreadyAccepted = new HashSet<RevCommit>(); + + if (branchTip != null) { + alreadyAccepted.add(branchTip); + } + + try { for (final Ref r : repo.getAllRefs().values()) { if (r.getName().startsWith(Constants.R_HEADS) || r.getName().startsWith(Constants.R_TAGS)) { @@ -388,11 +468,17 @@ public class MergeOp { } } } catch (IOException e) { - throw new MergeException("Cannot open branch", e); + throw new MergeException("Failed to determine already accepted commits.", e); } + + return alreadyAccepted; } - private void validateChangeList() throws MergeException { + private ListMultimap<SubmitType, Change> validateChangeList( + final List<Change> submitted) throws MergeException { + final ListMultimap<SubmitType, Change> toSubmit = + ArrayListMultimap.create(); + final Set<ObjectId> tips = new HashSet<ObjectId>(); for (final Ref r : repo.getAllRefs().values()) { tips.add(r.getObjectId()); @@ -455,10 +541,11 @@ public class MergeOp { continue; } - if (GitRepositoryManager.REF_CONFIG.equals(branchUpdate.getName())) { + if (GitRepositoryManager.REF_CONFIG.equals(destBranch.get())) { final Project.NameKey newParent; try { - ProjectConfig cfg = new ProjectConfig(destProject.getNameKey()); + ProjectConfig cfg = + new ProjectConfig(destProject.getProject().getNameKey()); cfg.load(repo, commit); newParent = cfg.getProject().getParent(allProjectsName); } catch (Exception e) { @@ -466,7 +553,8 @@ public class MergeOp { .error(CommitMergeStatus.INVALID_PROJECT_CONFIGURATION)); continue; } - final Project.NameKey oldParent = destProject.getParent(allProjectsName); + final Project.NameKey oldParent = + destProject.getProject().getParent(allProjectsName); if (oldParent == null) { // update of the 'All-Projects' project if (newParent != null) { @@ -506,7 +594,7 @@ public class MergeOp { if (branchTip != null) { // If this commit is already merged its a bug in the queuing code - // that we got back here. Just mark it complete and move on. Its + // that we got back here. Just mark it complete and move on. It's // merged and that is all that mattered to the requestor. // try { @@ -519,593 +607,58 @@ public class MergeOp { } } - commit.add(CAN_MERGE); - toMerge.add(commit); - } - } - - private void reduceToMinimalMerge() throws MergeException { - final Collection<CodeReviewCommit> heads; - try { - heads = new MergeSorter(rw, alreadyAccepted, CAN_MERGE).sort(toMerge); - } catch (IOException e) { - throw new MergeException("Branch head sorting failed", e); - } - - toMerge.clear(); - toMerge.addAll(heads); - Collections.sort(toMerge, new Comparator<CodeReviewCommit>() { - public int compare(final CodeReviewCommit a, final CodeReviewCommit b) { - return a.originalOrder - b.originalOrder; - } - }); - } - - private void mergeTopics() throws MergeException { - // Take the first fast-forward available, if any is available in the set. - // - if (destProject.getSubmitType() != Project.SubmitType.MERGE_ALWAYS) { - for (final Iterator<CodeReviewCommit> i = toMerge.iterator(); i.hasNext();) { - try { - final CodeReviewCommit n = i.next(); - if (mergeTip == null || rw.isMergedInto(mergeTip, n)) { - mergeTip = n; - i.remove(); - break; - } - } catch (IOException e) { - throw new MergeException("Cannot fast-forward test during merge", e); - } - } - } - - if (destProject.getSubmitType() == Project.SubmitType.FAST_FORWARD_ONLY) { - // If this project only permits fast-forwards, abort everything else. - // - while (!toMerge.isEmpty()) { - final CodeReviewCommit n = toMerge.remove(0); - n.statusCode = CommitMergeStatus.NOT_FAST_FORWARD; + final SubmitType submitType = getSubmitType(chg, ps); + if (submitType == null) { + commits.put(changeId, + CodeReviewCommit.error(CommitMergeStatus.NO_SUBMIT_TYPE)); + continue; } - } else { - // For every other commit do a pair-wise merge. - // - while (!toMerge.isEmpty()) { - mergeOneCommit(toMerge.remove(0)); - } + commit.add(canMergeFlag); + toMerge.put(submitType, commit); + toSubmit.put(submitType, chg); } + return toSubmit; } - private void mergeOneCommit(final CodeReviewCommit n) throws MergeException { - final ThreeWayMerger m = newThreeWayMerger(); + private SubmitType getSubmitType(final Change change, final PatchSet ps) { try { - if (m.merge(new AnyObjectId[] {mergeTip, n})) { - writeMergeCommit(m.getResultTreeId(), n); - - } else { - failed(n, CommitMergeStatus.PATH_CONFLICT); - } - } catch (IOException e) { - if (e.getMessage().startsWith("Multiple merge bases for")) { - try { - failed(n, CommitMergeStatus.CRISS_CROSS_MERGE); - } catch (IOException e2) { - throw new MergeException("Cannot merge " + n.name(), e); - } - } else { - throw new MergeException("Cannot merge " + n.name(), e); - } - } - } - - private ThreeWayMerger newThreeWayMerger() { - ThreeWayMerger m; - if (destProject.isUseContentMerge()) { - // Settings for this project allow us to try and - // automatically resolve conflicts within files if needed. - // Use ResolveMerge and instruct to operate in core. - m = MergeStrategy.RESOLVE.newMerger(repo, true); - } else { - // No auto conflict resolving allowed. If any of the - // affected files was modified, merge will fail. - m = MergeStrategy.SIMPLE_TWO_WAY_IN_CORE.newMerger(repo); - } - m.setObjectInserter(new ObjectInserter.Filter() { - @Override - protected ObjectInserter delegate() { - return inserter; - } - - @Override - public void flush() { - } - - @Override - public void release() { - } - }); - return m; - } - - private CodeReviewCommit failed(final CodeReviewCommit n, - final CommitMergeStatus failure) throws MissingObjectException, - IncorrectObjectTypeException, IOException { - rw.reset(); - rw.markStart(n); - rw.markUninteresting(mergeTip); - CodeReviewCommit failed; - while ((failed = (CodeReviewCommit) rw.next()) != null) { - failed.statusCode = failure; - } - return failed; - } - - private void writeMergeCommit(ObjectId treeId, CodeReviewCommit n) - throws IOException, MissingObjectException, IncorrectObjectTypeException { - final List<CodeReviewCommit> merged = new ArrayList<CodeReviewCommit>(); - rw.reset(); - rw.markStart(n); - rw.markUninteresting(mergeTip); - for (final RevCommit c : rw) { - final CodeReviewCommit crc = (CodeReviewCommit) c; - if (crc.patchsetId != null) { - merged.add(crc); - } - } - - final StringBuilder msgbuf = new StringBuilder(); - if (merged.size() == 1) { - final CodeReviewCommit c = merged.get(0); - rw.parseBody(c); - msgbuf.append("Merge \""); - msgbuf.append(c.getShortMessage()); - msgbuf.append("\""); - - } else { - msgbuf.append("Merge changes "); - for (final Iterator<CodeReviewCommit> i = merged.iterator(); i.hasNext();) { - msgbuf.append(i.next().change.getKey().abbreviate()); - if (i.hasNext()) { - msgbuf.append(','); - } - } - } - - if (!R_HEADS_MASTER.equals(destBranch.get())) { - msgbuf.append(" into "); - msgbuf.append(destBranch.getShortName()); - } - - if (merged.size() > 1) { - msgbuf.append("\n\n* changes:\n"); - for (final CodeReviewCommit c : merged) { - rw.parseBody(c); - msgbuf.append(" "); - msgbuf.append(c.getShortMessage()); - msgbuf.append("\n"); - } - } - - PersonIdent authorIdent = computeAuthor(merged); - - final CommitBuilder mergeCommit = new CommitBuilder(); - mergeCommit.setTreeId(treeId); - mergeCommit.setParentIds(mergeTip, n); - mergeCommit.setAuthor(authorIdent); - mergeCommit.setCommitter(myIdent); - mergeCommit.setMessage(msgbuf.toString()); - - mergeTip = (CodeReviewCommit) rw.parseCommit(commit(mergeCommit)); - } - - private PersonIdent computeAuthor( - final List<CodeReviewCommit> codeReviewCommits) { - PatchSetApproval submitter = null; - for (final CodeReviewCommit c : codeReviewCommits) { - PatchSetApproval s = getSubmitter(db, c.patchsetId); - if (submitter == null - || (s != null && s.getGranted().compareTo(submitter.getGranted()) > 0)) { - submitter = s; - } - } - - // Try to use the submitter's identity for the merge commit author. - // If all of the commits being merged are created by the submitter, - // prefer the identity line they used in the commits rather than the - // preferred identity stored in the user account. This way the Git - // commit records are more consistent internally. - // - PersonIdent authorIdent; - if (submitter != null) { - IdentifiedUser who = - identifiedUserFactory.create(submitter.getAccountId()); - Set<String> emails = new HashSet<String>(); - for (RevCommit c : codeReviewCommits) { - try { - rw.parseBody(c); - } catch (IOException e) { - log.warn("Cannot parse commit " + c.name() + " in " + destBranch, e); - continue; - } - emails.add(c.getAuthorIdent().getEmailAddress()); - } - - final Timestamp dt = submitter.getGranted(); - final TimeZone tz = myIdent.getTimeZone(); - if (emails.size() == 1 - && who.getEmailAddresses().contains(emails.iterator().next())) { - authorIdent = - new PersonIdent(codeReviewCommits.get(0).getAuthorIdent(), dt, tz); - } else { - authorIdent = who.newCommitterIdent(dt, tz); - } - } else { - authorIdent = myIdent; + final SubmitTypeRecord r = + changeControlFactory.controlFor(change, + identifiedUserFactory.create(change.getOwner())) + .getSubmitTypeRecord(db, ps); + if (r.status != SubmitTypeRecord.Status.OK) { + log.error("Failed to get submit type for " + change.getKey()); + return null; + } + return r.type; + } catch (NoSuchChangeException e) { + log.error("Failed to get submit type for " + change.getKey(), e); + return null; } - return authorIdent; } - private void markCleanMerges() throws MergeException { - if (mergeTip == null) { - // If mergeTip is null here, branchTip was null, indicating a new branch - // at the start of the merge process. We also elected to merge nothing, - // probably due to missing dependencies. Nothing was cleanly merged. - // + private void updateBranch(final SubmitStrategy strategy, + final RefUpdate branchUpdate) throws MergeException { + if ((branchTip == null && mergeTip == null) || branchTip == mergeTip) { + // nothing to do return; } - try { - rw.reset(); - rw.sort(RevSort.TOPO); - rw.sort(RevSort.REVERSE, true); - rw.markStart(mergeTip); - for (RevCommit c : alreadyAccepted) { - rw.markUninteresting(c); - } - - CodeReviewCommit c; - while ((c = (CodeReviewCommit) rw.next()) != null) { - if (c.patchsetId != null) { - c.statusCode = CommitMergeStatus.CLEAN_MERGE; - if (branchUpdate.getRefLogIdent() == null) { - setRefLogIdent(getSubmitter(db, c.patchsetId)); - } - } - } - } catch (IOException e) { - throw new MergeException("Cannot mark clean merges", e); - } - } - - private void setRefLogIdent(final PatchSetApproval submitAudit) { - if (submitAudit != null) { - branchUpdate.setRefLogIdent(identifiedUserFactory.create( - submitAudit.getAccountId()).newRefLogIdent()); - } - } - - private void cherryPickChanges() throws MergeException, OrmException { - while (!toMerge.isEmpty()) { - final CodeReviewCommit n = toMerge.remove(0); - final ThreeWayMerger m = newThreeWayMerger(); - try { - if (mergeTip == null) { - // The branch is unborn. Take a fast-forward resolution to - // create the branch. - // - mergeTip = n; - n.statusCode = CommitMergeStatus.CLEAN_MERGE; - - } else if (n.getParentCount() == 0) { - // Refuse to merge a root commit into an existing branch, - // we cannot obtain a delta for the cherry-pick to apply. - // - n.statusCode = CommitMergeStatus.CANNOT_CHERRY_PICK_ROOT; - - } else if (n.getParentCount() == 1) { - // If there is only one parent, a cherry-pick can be done by - // taking the delta relative to that one parent and redoing - // that on the current merge tip. - // - m.setBase(n.getParent(0)); - if (m.merge(mergeTip, n)) { - writeCherryPickCommit(m, n); - - } else { - n.statusCode = CommitMergeStatus.PATH_CONFLICT; - } - - } else { - // There are multiple parents, so this is a merge commit. We - // don't want to cherry-pick the merge as clients can't easily - // rebase their history with that merge present and replaced - // by an equivalent merge with a different first parent. So - // instead behave as though MERGE_IF_NECESSARY was configured. - // - if (hasDependenciesMet(n)) { - if (rw.isMergedInto(mergeTip, n)) { - mergeTip = n; - } else { - mergeOneCommit(n); - } - markCleanMerges(); - - } else { - // One or more dependencies were not met. The status was - // already marked on the commit so we have nothing further - // to perform at this time. - // - } - } - - } catch (IOException e) { - throw new MergeException("Cannot merge " + n.name(), e); - } - } - } - - private boolean hasDependenciesMet(final CodeReviewCommit n) - throws IOException { - // Oddly we can determine this by running the merge sorter and - // look for the one commit to come out as a result. This works - // as the merge sorter checks the dependency chain as part of - // its logic trying to find a minimal merge path. - // - return new MergeSorter(rw, alreadyAccepted, CAN_MERGE).sort( - Collections.singleton(n)).contains(n); - } - - private void writeCherryPickCommit(final Merger m, final CodeReviewCommit n) - throws IOException, OrmException { - rw.parseBody(n); - - final List<FooterLine> footers = n.getFooterLines(); - final StringBuilder msgbuf = new StringBuilder(); - msgbuf.append(n.getFullMessage()); - - if (msgbuf.length() == 0) { - // WTF, an empty commit message? - msgbuf.append("<no commit message provided>"); - } - if (msgbuf.charAt(msgbuf.length() - 1) != '\n') { - // Missing a trailing LF? Correct it (perhaps the editor was broken). - msgbuf.append('\n'); - } - if (footers.isEmpty()) { - // Doesn't end in a "Signed-off-by: ..." style line? Add another line - // break to start a new paragraph for the reviewed-by tag lines. - // - msgbuf.append('\n'); - } - - if (!contains(footers, CHANGE_ID, n.change.getKey().get())) { - msgbuf.append(CHANGE_ID.getName()); - msgbuf.append(": "); - msgbuf.append(n.change.getKey().get()); - msgbuf.append('\n'); - } - - final String siteUrl = urlProvider.get(); - if (siteUrl != null) { - final String url = siteUrl + n.patchsetId.getParentKey().get(); - if (!contains(footers, REVIEWED_ON, url)) { - msgbuf.append(REVIEWED_ON.getName()); - msgbuf.append(": "); - msgbuf.append(url); - msgbuf.append('\n'); - } - } - - PatchSetApproval submitAudit = null; - List<PatchSetApproval> approvalList = null; - try { - approvalList = - db.patchSetApprovals().byPatchSet(n.patchsetId).toList(); - Collections.sort(approvalList, new Comparator<PatchSetApproval>() { - public int compare(final PatchSetApproval a, final PatchSetApproval b) { - return a.getGranted().compareTo(b.getGranted()); - } - }); - - for (final PatchSetApproval a : approvalList) { - if (a.getValue() <= 0) { - // Negative votes aren't counted. - continue; - } - - if (ApprovalCategory.SUBMIT.equals(a.getCategoryId())) { - // Submit is treated specially, below (becomes committer) - // - if (submitAudit == null - || a.getGranted().compareTo(submitAudit.getGranted()) > 0) { - submitAudit = a; - } - continue; - } - - final Account acc = - identifiedUserFactory.create(a.getAccountId()).getAccount(); - final StringBuilder identbuf = new StringBuilder(); - if (acc.getFullName() != null && acc.getFullName().length() > 0) { - if (identbuf.length() > 0) { - identbuf.append(' '); - } - identbuf.append(acc.getFullName()); - } - if (acc.getPreferredEmail() != null - && acc.getPreferredEmail().length() > 0) { - if (isSignedOffBy(footers, acc.getPreferredEmail())) { - continue; - } - if (identbuf.length() > 0) { - identbuf.append(' '); - } - identbuf.append('<'); - identbuf.append(acc.getPreferredEmail()); - identbuf.append('>'); - } - if (identbuf.length() == 0) { - // Nothing reasonable to describe them by? Ignore them. - continue; - } - - final String tag; - if (CRVW.equals(a.getCategoryId())) { - tag = "Reviewed-by"; - } else if (VRIF.equals(a.getCategoryId())) { - tag = "Tested-by"; - } else { - final ApprovalType at = - approvalTypes.byId(a.getCategoryId()); - if (at == null) { - // A deprecated/deleted approval type, ignore it. - continue; - } - tag = at.getCategory().getName().replace(' ', '-'); - } - - if (!contains(footers, new FooterKey(tag), identbuf.toString())) { - msgbuf.append(tag); - msgbuf.append(": "); - msgbuf.append(identbuf); - msgbuf.append('\n'); - } - } - } catch (OrmException e) { - log.error("Can't read approval records for " + n.patchsetId, e); - } - - final CommitBuilder mergeCommit = new CommitBuilder(); - mergeCommit.setTreeId(m.getResultTreeId()); - mergeCommit.setParentId(mergeTip); - mergeCommit.setAuthor(n.getAuthorIdent()); - mergeCommit.setCommitter(toCommitterIdent(submitAudit)); - mergeCommit.setMessage(msgbuf.toString()); - - final ObjectId id = commit(mergeCommit); - final CodeReviewCommit newCommit = (CodeReviewCommit) rw.parseCommit(id); - - if (submitAudit != null) { - final Change oldChange = n.change; - - n.change = - db.changes().atomicUpdate(n.change.getId(), - new AtomicUpdate<Change>() { - @Override - public Change update(Change change) { - change.nextPatchSetId(); - return change; - } - }); - - final PatchSet ps = new PatchSet(n.change.currPatchSetId()); - ps.setCreatedOn(new Timestamp(System.currentTimeMillis())); - ps.setUploader(submitAudit.getAccountId()); - ps.setRevision(new RevId(id.getName())); - insertAncestors(ps.getId(), newCommit); - db.patchSets().insert(Collections.singleton(ps)); - - n.change = - db.changes().atomicUpdate(n.change.getId(), - new AtomicUpdate<Change>() { - @Override - public Change update(Change change) { - change.setCurrentPatchSet(patchSetInfoFactory.get(newCommit, - ps.getId())); - return change; - } - }); - - this.submitted.remove(oldChange); - this.submitted.add(n.change); - - if (approvalList != null) { - for (PatchSetApproval a : approvalList) { - db.patchSetApprovals().insert( - Collections.singleton(new PatchSetApproval(ps.getId(), a))); - } - } - - final RefUpdate ru = repo.updateRef(ps.getRefName()); - ru.setExpectedOldObjectId(ObjectId.zeroId()); - ru.setNewObjectId(newCommit); - ru.disableRefLog(); - if (ru.update(rw) != RefUpdate.Result.NEW) { - throw new IOException(String.format( - "Failed to create ref %s in %s: %s", ps.getRefName(), n.change - .getDest().getParentKey().get(), ru.getResult())); - } - replication.fire(n.change.getProject(), ru.getName()); - } - - newCommit.copyFrom(n); - newCommit.statusCode = CommitMergeStatus.CLEAN_PICK; - commits.put(newCommit.patchsetId.getParentKey(), newCommit); - mergeTip = newCommit; - setRefLogIdent(submitAudit); - } - - private void insertAncestors(PatchSet.Id id, RevCommit src) - throws OrmException { - final int cnt = src.getParentCount(); - List<PatchSetAncestor> toInsert = new ArrayList<PatchSetAncestor>(cnt); - for (int p = 0; p < cnt; p++) { - PatchSetAncestor a; - - a = new PatchSetAncestor(new PatchSetAncestor.Id(id, p + 1)); - a.setAncestorRevision(new RevId(src.getParent(p).getId().name())); - toInsert.add(a); - } - db.patchSetAncestors().insert(toInsert); - } - - private ObjectId commit(CommitBuilder mergeCommit) - throws IOException, UnsupportedEncodingException { - ObjectId id = inserter.insert(mergeCommit); - inserter.flush(); - return id; - } - - private boolean contains(List<FooterLine> footers, FooterKey key, String val) { - for (final FooterLine line : footers) { - if (line.matches(key) && val.equals(line.getValue())) { - return true; - } - } - return false; - } - - private boolean isSignedOffBy(List<FooterLine> footers, String email) { - for (final FooterLine line : footers) { - if (line.matches(FooterKey.SIGNED_OFF_BY) - && email.equals(line.getEmailAddress())) { - return true; - } - } - return false; - } - - private PersonIdent toCommitterIdent(final PatchSetApproval audit) { - if (audit != null) { - return identifiedUserFactory.create(audit.getAccountId()) - .newCommitterIdent(audit.getGranted(), myIdent.getTimeZone()); - } - return myIdent; - } - - private void updateBranch() throws MergeException { if (mergeTip != null && (branchTip == null || branchTip != mergeTip)) { if (GitRepositoryManager.REF_CONFIG.equals(branchUpdate.getName())) { try { - ProjectConfig cfg = new ProjectConfig(destProject.getNameKey()); + ProjectConfig cfg = + new ProjectConfig(destProject.getProject().getNameKey()); cfg.load(repo, mergeTip); } catch (Exception e) { throw new MergeException("Submit would store invalid" + " project configuration " + mergeTip.name() + " for " - + destProject.getName(), e); + + destProject.getProject().getName(), e); } } + branchUpdate.setRefLogIdent(refLogIdent); branchUpdate.setForceUpdate(false); branchUpdate.setNewObjectId(mergeTip); branchUpdate.setRefLogMessage("merged", true); @@ -1121,13 +674,14 @@ public class MergeOp { } if (GitRepositoryManager.REF_CONFIG.equals(branchUpdate.getName())) { - projectCache.evict(destProject); - ProjectState ps = projectCache.get(destProject.getNameKey()); - repoManager.setProjectDescription(destProject.getNameKey(), // - ps.getProject().getDescription()); + projectCache.evict(destProject.getProject()); + destProject = projectCache.get(destProject.getProject().getNameKey()); + repoManager.setProjectDescription( + destProject.getProject().getNameKey(), + destProject.getProject().getDescription()); } - replication.fire(destBranch.getParentKey(), branchUpdate.getName()); + gitRefUpdated.fire(destBranch.getParentKey(), branchUpdate); Account account = null; final PatchSetApproval submitter = getSubmitter(db, mergeTip.patchsetId); @@ -1137,6 +691,16 @@ public class MergeOp { hooks.doRefUpdatedHook(destBranch, branchUpdate, account); break; + case LOCK_FAILURE: + String msg; + if (strategy.retryOnLockFailure()) { + mergeQueue.recheckAfter(destBranch, LOCK_FAILURE_RETRY_DELAY, + MILLISECONDS); + msg = "will retry"; + } else { + msg = "will not retry"; + } + throw new IOException(branchUpdate.getResult().name() + ", " + msg); default: throw new IOException(branchUpdate.getResult().name()); } @@ -1146,23 +710,7 @@ public class MergeOp { } } - private boolean isMergeable(Change c) { - final CodeReviewCommit commit = commits.get(c.getId()); - final CommitMergeStatus s = commit != null ? commit.statusCode : null; - boolean isMergeable = false; - if (s != null - && (s.equals(CommitMergeStatus.CLEAN_MERGE) - || s.equals(CommitMergeStatus.CLEAN_PICK) || s - .equals(CommitMergeStatus.ALREADY_MERGED))) { - isMergeable = true; - } - - return isMergeable; - } - - private void updateChangeStatus() { - List<CodeReviewCommit> merged = new ArrayList<CodeReviewCommit>(); - + private void updateChangeStatus(final List<Change> submitted) { for (final Change c : submitted) { final CodeReviewCommit commit = commits.get(c.getId()); final CommitMergeStatus s = commit != null ? commit.statusCode : null; @@ -1175,66 +723,51 @@ public class MergeOp { final String txt = s.getMessage(); - switch (s) { - case CLEAN_MERGE: { - setMerged(c, message(c, txt)); - merged.add(commit); - break; - } + try { + switch (s) { + case CLEAN_MERGE: + setMerged(c, message(c, txt)); + break; - case CLEAN_PICK: { - setMerged(c, message(c, txt + " as " + commit.name())); - merged.add(commit); - break; - } + case CLEAN_REBASE: + case CLEAN_PICK: + setMerged(c, message(c, txt + " as " + commit.name())); + break; - case ALREADY_MERGED: - setMerged(c, null); - merged.add(commit); - break; - - case PATH_CONFLICT: - case CRISS_CROSS_MERGE: - case CANNOT_CHERRY_PICK_ROOT: - case NOT_FAST_FORWARD: - case INVALID_PROJECT_CONFIGURATION: - case INVALID_PROJECT_CONFIGURATION_PARENT_PROJECT_NOT_FOUND: - case INVALID_PROJECT_CONFIGURATION_ROOT_PROJECT_CANNOT_HAVE_PARENT: - case SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN: { - setNew(c, message(c, txt)); - break; - } + case ALREADY_MERGED: + setMerged(c, null); + break; - case MISSING_DEPENDENCY: { - final Capable capable = isSubmitStillPossible(commit); - if (capable != Capable.OK) { - sendMergeFail(c, message(c, capable.getMessage()), false); - } - break; - } + case PATH_CONFLICT: + case MANUAL_RECURSIVE_MERGE: + case CANNOT_CHERRY_PICK_ROOT: + case NOT_FAST_FORWARD: + case INVALID_PROJECT_CONFIGURATION: + case INVALID_PROJECT_CONFIGURATION_PARENT_PROJECT_NOT_FOUND: + case INVALID_PROJECT_CONFIGURATION_ROOT_PROJECT_CANNOT_HAVE_PARENT: + case SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN: + setNew(c, message(c, txt)); + break; - default: - setNew(c, message(c, "Unspecified merge failure: " + s.name())); - break; - } - } + case MISSING_DEPENDENCY: + potentiallyStillSubmittable.add(commit); + break; - CreateCodeReviewNotes codeReviewNotes = - codeReviewNotesFactory.create(db, repo); - try { - codeReviewNotes.create(merged, computeAuthor(merged)); - } catch (CodeReviewNoteCreationException e) { - log.error(e.getMessage()); + default: + setNew(c, message(c, "Unspecified merge failure: " + s.name())); + break; + } + } catch (OrmException err) { + log.warn("Error updating change status for " + c.getId(), err); + } } - replication.fire(destBranch.getParentKey(), - GitRepositoryManager.REFS_NOTES_REVIEW); } - private void updateSubscriptions() { + private void updateSubscriptions(final List<Change> submitted) { if (mergeTip != null && (branchTip == null || branchTip != mergeTip)) { SubmoduleOp subOp = - subOpFactory.create(destBranch, mergeTip, rw, repo, destProject, - submitted, commits); + subOpFactory.create(destBranch, mergeTip, rw, repo, + destProject.getProject(), submitted, commits); try { subOp.update(); } catch (SubmoduleException e) { @@ -1248,32 +781,7 @@ public class MergeOp { private Capable isSubmitStillPossible(final CodeReviewCommit commit) { final Capable capable; final Change c = commit.change; - if (commit.missing == null) { - commit.missing = new ArrayList<CodeReviewCommit>(); - } - - boolean submitStillPossible = commit.missing.size() > 0; - for (CodeReviewCommit missingCommit : commit.missing) { - loadChangeInfo(missingCommit); - - if (missingCommit.patchsetId == null) { - // The commit doesn't have a patch set, so it cannot be - // submitted to the branch. - // - submitStillPossible = false; - break; - } - - if (!missingCommit.change.currentPatchSetId().equals( - missingCommit.patchsetId)) { - // If the missing commit is not the current patch set, - // the change must be rebased to use the proper parent. - // - submitStillPossible = false; - break; - } - } - + final boolean submitStillPossible = isSubmitForMissingCommitsStillPossible(commit); final long now = System.currentTimeMillis(); final long waitUntil = c.getLastUpdatedOn().getTime() + DEPENDENCY_DELAY; if (submitStillPossible && now < waitUntil) { @@ -1287,25 +795,20 @@ public class MergeOp { // dependencies are also submitted. Perhaps the user just // forgot to submit those. // - String txt = - "Change could not be merged because of a missing dependency."; - if (!isAlreadySent(c, txt)) { - StringBuilder m = new StringBuilder(); - m.append(txt); - m.append("\n"); + StringBuilder m = new StringBuilder(); + m.append("Change could not be merged because of a missing dependency."); + m.append("\n"); - m.append("\n"); + m.append("\n"); - m.append("The following changes must also be submitted:\n"); + m.append("The following changes must also be submitted:\n"); + m.append("\n"); + for (CodeReviewCommit missingCommit : commit.missing) { + m.append("* "); + m.append(missingCommit.change.getKey().get()); m.append("\n"); - for (CodeReviewCommit missingCommit : commit.missing) { - m.append("* "); - m.append(missingCommit.change.getKey().get()); - m.append("\n"); - } - txt = m.toString(); } - capable = new Capable(txt); + capable = new Capable(m.toString()); } else { // It is impossible to submit this change as-is. The author // needs to rebase it in order to work around the missing @@ -1323,8 +826,10 @@ public class MergeOp { m.append(missingCommit.patchsetId.get()); m.append(" of "); m.append(missingCommit.change.getKey().abbreviate()); - m.append(", however the current patch set is "); - m.append(missingCommit.change.currentPatchSetId().get()); + if (missingCommit.patchsetId.get() != missingCommit.change.currentPatchSetId().get()) { + m.append(", however the current patch set is "); + m.append(missingCommit.change.currentPatchSetId().get()); + } m.append(".\n"); } else { @@ -1356,30 +861,6 @@ public class MergeOp { } } - private boolean isAlreadySent(final Change c, final String prefix) { - try { - final List<ChangeMessage> msgList = - db.changeMessages().byChange(c.getId()).toList(); - if (msgList.size() > 0) { - final ChangeMessage last = msgList.get(msgList.size() - 1); - if (last.getAuthor() == null && last.getMessage().startsWith(prefix)) { - // The last message was written by us, and it said this - // same message already. Its unlikely anything has changed - // that would cause us to need to repeat ourselves. - // - return true; - } - } - - // The last message was not sent by us, or doesn't match the text - // we are about to send. - // - return false; - } catch (OrmException e) { - return true; - } - } - private ChangeMessage message(final Change c, final String body) { final String uuid; try { @@ -1394,87 +875,87 @@ public class MergeOp { return m; } - private static PatchSetApproval getSubmitter(ReviewDb reviewDb, - PatchSet.Id c) { - if (c == null) { - return null; - } - PatchSetApproval submitter = null; + private void setMerged(final Change c, final ChangeMessage msg) + throws OrmException { try { - final List<PatchSetApproval> approvals = - reviewDb.patchSetApprovals().byPatchSet(c).toList(); - for (PatchSetApproval a : approvals) { - if (a.getValue() > 0 - && ApprovalCategory.SUBMIT.equals(a.getCategoryId())) { - if (submitter == null - || a.getGranted().compareTo(submitter.getGranted()) > 0) { - submitter = a; - } + db.changes().beginTransaction(c.getId()); + + // We must pull the patchset out of commits, because the patchset ID is + // modified when using the cherry-pick merge strategy. + CodeReviewCommit commit = commits.get(c.getId()); + PatchSet.Id merged = commit.change.currentPatchSetId(); + setMergedPatchSet(c.getId(), merged); + PatchSetApproval submitter = saveApprovals(c, merged); + addMergedMessage(submitter, msg); + + db.commit(); + + sendMergedEmail(c, submitter); + if (submitter != null) { + try { + hooks.doChangeMergedHook(c, + accountCache.get(submitter.getAccountId()).getAccount(), + db.patchSets().get(c.currentPatchSetId()), db); + } catch (OrmException ex) { + log.error("Cannot run hook for submitted patch set " + c.getId(), ex); } } - } catch (OrmException e) { + } finally { + db.rollback(); } - return submitter; } - private void setMerged(final Change c, final ChangeMessage msg) { - final Change.Id changeId = c.getId(); - // We must pull the patchset out of commits, because the patchset ID is - // modified when using the cherry-pick merge strategy. - final CodeReviewCommit commit = commits.get(c.getId()); - final PatchSet.Id merged = commit.change.currentPatchSetId(); - - try { - db.changes().atomicUpdate(changeId, new AtomicUpdate<Change>() { - @Override - public Change update(Change c) { - c.setStatus(Change.Status.MERGED); - // It could be possible that the change being merged - // has never had its mergeability tested. So we insure - // merged changes has mergeable field true. - c.setMergeable(true); - if (!merged.equals(c.currentPatchSetId())) { - // Uncool; the patch set changed after we merged it. - // Go back to the patch set that was actually merged. - // - try { - c.setCurrentPatchSet(patchSetInfoFactory.get(db, merged)); - } catch (PatchSetInfoNotAvailableException e1) { - log.error("Cannot read merged patch set " + merged, e1); - } + private void setMergedPatchSet(Change.Id changeId, final PatchSet.Id merged) + throws OrmException { + db.changes().atomicUpdate(changeId, new AtomicUpdate<Change>() { + @Override + public Change update(Change c) { + c.setStatus(Change.Status.MERGED); + // It could be possible that the change being merged + // has never had its mergeability tested. So we insure + // merged changes has mergeable field true. + c.setMergeable(true); + if (!merged.equals(c.currentPatchSetId())) { + // Uncool; the patch set changed after we merged it. + // Go back to the patch set that was actually merged. + // + try { + c.setCurrentPatchSet(patchSetInfoFactory.get(db, merged)); + } catch (PatchSetInfoNotAvailableException e1) { + log.error("Cannot read merged patch set " + merged, e1); } - ChangeUtil.updated(c); - return c; } - }); - } catch (OrmConcurrencyException err) { - } catch (OrmException err) { - log.warn("Cannot update change status", err); - } + ChangeUtil.updated(c); + return c; + } + }); + } - // Flatten out all existing approvals based upon the current - // permissions. Once the change is closed the approvals are - // not updated at presentation view time, so we need to make. - // sure they are accurate now. This way if permissions get - // modified in the future, historical records stay accurate. - // + private PatchSetApproval saveApprovals(Change c, PatchSet.Id merged) + throws OrmException { + // Flatten out existing approvals for this patch set based upon the current + // permissions. Once the change is closed the approvals are not updated at + // presentation view time, except for zero votes used to indicate a reviewer + // was added. So we need to make sure votes are accurate now. This way if + // permissions get modified in the future, historical records stay accurate. PatchSetApproval submitter = null; try { c.setStatus(Change.Status.MERGED); - final List<PatchSetApproval> approvals = - db.patchSetApprovals().byChange(changeId).toList(); - final FunctionState fs = functionState.create( - changeControlFactory.controlFor( - c, - identifiedUserFactory.create(c.getOwner())), - merged, approvals); - for (ApprovalType at : approvalTypes.getApprovalTypes()) { - CategoryFunction.forCategory(at.getCategory()).run(at, fs); + + List<PatchSetApproval> approvals = + db.patchSetApprovals().byPatchSet(merged).toList(); + Set<PatchSetApproval.Key> toDelete = + Sets.newHashSetWithExpectedSize(approvals.size()); + for (PatchSetApproval a : approvals) { + if (a.getValue() != 0) { + toDelete.add(a.getKey()); + } } + + approvals = labelNormalizer.normalize(c, approvals); for (PatchSetApproval a : approvals) { - if (a.getValue() > 0 - && ApprovalCategory.SUBMIT.equals(a.getCategoryId()) - && a.getPatchSetId().equals(merged)) { + toDelete.remove(a.getKey()); + if (a.getValue() > 0 && a.isSubmit()) { if (submitter == null || a.getGranted().compareTo(submitter.getGranted()) > 0) { submitter = a; @@ -1483,24 +964,24 @@ public class MergeOp { a.cache(c); } db.patchSetApprovals().update(approvals); + db.patchSetApprovals().deleteKeys(toDelete); } catch (NoSuchChangeException err) { - log.warn("Cannot normalize approvals for change " + changeId, err); - } catch (OrmException err) { - log.warn("Cannot normalize approvals for change " + changeId, err); + throw new OrmException(err); } + return submitter; + } + private void addMergedMessage(PatchSetApproval submitter, ChangeMessage msg) + throws OrmException { if (msg != null) { if (submitter != null && msg.getAuthor() == null) { msg.setAuthor(submitter.getAccountId()); } - try { - db.changeMessages().insert(Collections.singleton(msg)); - } catch (OrmException err) { - log.warn("Cannot store message on change", err); - } + db.changeMessages().insert(Collections.singleton(msg)); } + } - final PatchSetApproval from = submitter; + private void sendMergedEmail(final Change c, final PatchSetApproval from) { workQueue.getDefaultQueue() .submit(requestScopePropagator.wrap(new Runnable() { @Override @@ -1519,7 +1000,9 @@ public class MergeOp { } try { - final MergedSender cm = mergedSenderFactory.create(c); + final ChangeControl control = changeControlFactory.controlFor(c, + identifiedUserFactory.create(c.getOwner())); + final MergedSender cm = mergedSenderFactory.create(control); if (from != null) { cm.setFrom(from.getAccountId()); } @@ -1535,23 +1018,37 @@ public class MergeOp { return "send-email merged"; } })); - - - try { - hooks.doChangeMergedHook(c, // - accountCache.get(submitter.getAccountId()).getAccount(), // - db.patchSets().get(c.currentPatchSetId()), db); - } catch (OrmException ex) { - log.error("Cannot run hook for submitted patch set " + c.getId(), ex); - } } private void setNew(Change c, ChangeMessage msg) { sendMergeFail(c, msg, true); } + private boolean isDuplicate(ChangeMessage msg) { + try { + ChangeMessage last = Iterables.getLast(db.changeMessages().byChange( + msg.getPatchSetId().getParentKey()), null); + if (last != null) { + long lastMs = last.getWrittenOn().getTime(); + long msgMs = msg.getWrittenOn().getTime(); + if (Objects.equal(last.getAuthor(), msg.getAuthor()) + && Objects.equal(last.getMessage(), msg.getMessage()) + && msgMs - lastMs < DUPLICATE_MESSAGE_INTERVAL) { + return true; + } + } + } catch (OrmException err) { + log.warn("Cannot check previous merge failure message", err); + } + return false; + } + private void sendMergeFail(final Change c, final ChangeMessage msg, final boolean makeNew) { + if (isDuplicate(msg)) { + return; + } + try { db.changeMessages().insert(Collections.singleton(msg)); } catch (OrmException err) { @@ -1582,17 +1079,23 @@ public class MergeOp { } } + PatchSetApproval submitter = null; + try { + submitter = getSubmitter(db, c.currentPatchSetId()); + } catch (Exception e) { + log.error("Cannot get submitter", e); + } + + final PatchSetApproval from = submitter; workQueue.getDefaultQueue() .submit(requestScopePropagator.wrap(new Runnable() { @Override public void run() { PatchSet patchSet; - PatchSetApproval submitter; try { ReviewDb reviewDb = schemaFactory.open(); try { patchSet = reviewDb.patchSets().get(c.currentPatchSetId()); - submitter = getSubmitter(reviewDb, c.currentPatchSetId()); } finally { reviewDb.close(); } @@ -1603,8 +1106,8 @@ public class MergeOp { try { final MergeFailSender cm = mergeFailSenderFactory.create(c); - if (submitter != null) { - cm.setFrom(submitter.getAccountId()); + if (from != null) { + cm.setFrom(from.getAccountId()); } cm.setPatchSet(patchSet); cm.setChangeMessage(msg); @@ -1619,5 +1122,15 @@ public class MergeOp { return "send-email merge-failed"; } })); + + if (submitter != null) { + try { + hooks.doMergeFailedHook(c, + accountCache.get(submitter.getAccountId()).getAccount(), + db.patchSets().get(c.currentPatchSetId()), msg.getMessage(), db); + } catch (OrmException ex) { + log.error("Cannot run hook for merge failed " + c.getId(), ex); + } + } } } |