diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/CommitMergeStatus.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/CommitMergeStatus.java index 87903511fc..512944f0a2 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/CommitMergeStatus.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/CommitMergeStatus.java @@ -38,6 +38,9 @@ enum CommitMergeStatus { /** */ REVISION_GONE(""), + /** */ + NO_SUBMIT_TYPE(""), + /** */ CRISS_CROSS_MERGE("Your change requires a recursive merge to resolve.\n" + "\n" 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 d60293985f..aae58ba77a 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 @@ -31,6 +31,7 @@ import com.google.gerrit.reviewdb.client.ChangeMessage; 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.reviewdb.client.Project.SubmitType; import com.google.gerrit.reviewdb.client.RevId; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.ChangeUtil; @@ -78,9 +79,11 @@ import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Random; +import java.util.Map.Entry; import java.util.Set; /** @@ -126,7 +129,8 @@ public class MergeOp { private final PersonIdent myIdent; private final Branch.NameKey destBranch; private Project destProject; - private final List toMerge; + private final Map> toMerge; + private final List potentiallyStillSubmittable; private final Map commits; private ReviewDb db; private Repository repo; @@ -184,7 +188,8 @@ public class MergeOp { this.requestScopePropagator = requestScopePropagator; this.myIdent = myIdent; destBranch = branch; - toMerge = new ArrayList(); + toMerge = new HashMap>(); + potentiallyStillSubmittable = new ArrayList(); commits = new HashMap(); } @@ -204,7 +209,9 @@ public class MergeOp { openSchema(); openBranch(); validateChangeList(Collections.singletonList(change)); - preMerge(createStrategy()); + final Entry> entry = + toMerge.entrySet().iterator().next(); + preMerge(createStrategy(entry.getKey()), entry.getValue()); // update sha1 tested merge. if (destBranchRef != null) { @@ -254,14 +261,53 @@ public class MergeOp { try { openSchema(); openRepository(); - final List submitted = db.changes().submitted(destBranch).toList(); - final RefUpdate branchUpdate = openBranch(); - validateChangeList(submitted); - final SubmitStrategy strategy = createStrategy(); - preMerge(strategy); - updateBranch(strategy, branchUpdate); - updateChangeStatus(submitted); - updateSubscriptions(submitted); + openBranch(); + final Map> toSubmit = + validateChangeList(db.changes().submitted(destBranch).toList()); + + final Map> toMergeNextTurn = + new HashMap>(); + final List potentiallyStillSubmittableOnNextRun = + new ArrayList(); + while (!toMerge.isEmpty()) { + toMergeNextTurn.clear(); + for (final Entry> e : toMerge.entrySet()) { + final SubmitType submitType = e.getKey(); + final RefUpdate branchUpdate = openBranch(); + final SubmitStrategy strategy = createStrategy(submitType); + preMerge(strategy, e.getValue()); + updateBranch(strategy, branchUpdate); + updateChangeStatus(toSubmit.get(submitType)); + updateSubscriptions(toSubmit.get(submitType)); + + for (final Iterator 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; + getList(submitType, toMergeNextTurn).add(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 { @@ -280,15 +326,65 @@ public class MergeOp { } } - private void preMerge(final SubmitStrategy strategy) throws MergeException { + private boolean containsMissingCommits( + final Map> map, + final CodeReviewCommit commit) { + if (!isSubmitForMissingCommitsStillPossible(commit)) { + return false; + } + + for (final CodeReviewCommit missingCommit : commit.missing) { + boolean found = false; + for (final List list : map.values()) { + if (list.contains(missingCommit)) { + found = true; + break; + } + } + if (!found) { + 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 toMerge) throws MergeException { mergeTip = strategy.run(branchTip, toMerge); refLogIdent = strategy.getRefLogIdent(); commits.putAll(strategy.getNewCommits()); } - private SubmitStrategy createStrategy() throws MergeException { - return submitStrategyFactory.create(destProject.getSubmitType(), db, repo, - rw, inserter, canMergeFlag, getAlreadyAccepted(branchTip), destBranch, + private SubmitStrategy createStrategy(final SubmitType submitType) throws MergeException { + return submitStrategyFactory.create(submitType, db, repo, rw, inserter, + canMergeFlag, getAlreadyAccepted(branchTip), destBranch, destProject.isUseContentMerge()); } @@ -374,8 +470,11 @@ public class MergeOp { return alreadyAccepted; } - private void validateChangeList(final List submitted) - throws MergeException { + private Map> validateChangeList( + final List submitted) throws MergeException { + final Map> toSubmit = + new HashMap>(); + final Set tips = new HashSet(); for (final Ref r : repo.getAllRefs().values()) { tips.add(r.getObjectId()); @@ -458,13 +557,40 @@ public class MergeOp { } } + final SubmitType submitType = getSubmitType(chg, ps); + if (submitType == null) { + commits.put(changeId, + CodeReviewCommit.error(CommitMergeStatus.NO_SUBMIT_TYPE)); + continue; + } + commit.add(canMergeFlag); - toMerge.add(commit); + getList(submitType, toMerge).add(commit); + getList(submitType, toSubmit).add(chg); } + return toSubmit; + } + + private SubmitType getSubmitType(final Change change, final PatchSet ps) { + return destProject.getSubmitType(); + } + + private static List getList(final K key, final Map> map) { + List list = map.get(key); + if (list == null) { + list = new ArrayList(); + map.put(key, list); + } + return list; } private void updateBranch(final SubmitStrategy strategy, final RefUpdate branchUpdate) throws MergeException { + if ((branchTip == null && mergeTip == null) || branchTip == mergeTip) { + // nothing to do + return; + } + if (mergeTip != null && (branchTip == null || branchTip != mergeTip)) { if (GitRepositoryManager.REF_CONFIG.equals(branchUpdate.getName())) { try { @@ -580,10 +706,7 @@ public class MergeOp { } case MISSING_DEPENDENCY: { - final Capable capable = isSubmitStillPossible(commit); - if (capable != Capable.OK) { - sendMergeFail(c, message(c, capable.getMessage()), false); - } + potentiallyStillSubmittable.add(commit); break; } @@ -625,32 +748,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(); - } - - 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) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSorter.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSorter.java index d76a93555b..3911d96ee1 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSorter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSorter.java @@ -54,7 +54,7 @@ class MergeSorter { RevCommit c; final RevCommitList contents = new RevCommitList(); while ((c = rw.next()) != null) { - if (!c.has(canMergeFlag)) { + if (!c.has(canMergeFlag) || !incoming.contains(c)) { // We cannot merge n as it would bring something we // aren't permitted to merge at this time. Drop n. //