opendev/gerrit
Code Issues Proposed changes

Support changes with different submit types in MergeOp

Browse Source 
In future the submit type for a change should be controlled from
Prolog.

Currently the MergeOp algorithm assumes that the submit type is the
same for all changes to be merged. If the submit type is controlled
from Prolog changes going to the same destination branch can have
different submit types. To support this MergeOp now scans all changes
and determines their submit type, then clusters the changes with the
same submit type and loops through the submit strategies to execute
those clustered groups in turn.

Change-Id: Iadd633491aa07654f93b9ae105da93cd20927f3e
Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>
This commit is contained in:
Edwin Kempin
2012-08-29 12:12:36 +02:00
parent 0ee523efdf
commit edb573adbb
3 changed files with 150 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
gerrit-server/src/main/java/com/google/gerrit/server/git/CommitMergeStatus.java
View File

@@ -38,6 +38,9 @@ enum CommitMergeStatus {
/** */
REVISION_GONE(""),
/** */
NO_SUBMIT_TYPE(""),
/** */
CRISS_CROSS_MERGE("Your change requires a recursive merge to resolve.\n"
+ "\n"

194
gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
View File

@@ -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<CodeReviewCommit> toMerge;
private final Map<SubmitType, List<CodeReviewCommit>> toMerge;
private final List<CodeReviewCommit> potentiallyStillSubmittable;
private final Map<Change.Id, CodeReviewCommit> 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<CodeReviewCommit>();
toMerge = new HashMap<SubmitType, List<CodeReviewCommit>>();
potentiallyStillSubmittable = new ArrayList<CodeReviewCommit>();
commits = new HashMap<Change.Id, CodeReviewCommit>();
}
@@ -204,7 +209,9 @@ public class MergeOp {
openSchema();
openBranch();
validateChangeList(Collections.singletonList(change));
preMerge(createStrategy());
final Entry<SubmitType, List<CodeReviewCommit>> 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<Change> 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<SubmitType, List<Change>> toSubmit =
validateChangeList(db.changes().submitted(destBranch).toList());
final Map<SubmitType, List<CodeReviewCommit>> toMergeNextTurn =
new HashMap<SubmitType, List<CodeReviewCommit>>();
final List<CodeReviewCommit> potentiallyStillSubmittableOnNextRun =
new ArrayList<CodeReviewCommit>();
while (!toMerge.isEmpty()) {
toMergeNextTurn.clear();
for (final Entry<SubmitType, List<CodeReviewCommit>> 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<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;
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<SubmitType, List<CodeReviewCommit>> map,
final CodeReviewCommit commit) {
if (!isSubmitForMissingCommitsStillPossible(commit)) {
return false;
}
for (final CodeReviewCommit missingCommit : commit.missing) {
boolean found = false;
for (final List<CodeReviewCommit> 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<CodeReviewCommit> 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<Change> submitted)
throws MergeException {
private Map<SubmitType, List<Change>> validateChangeList(
final List<Change> submitted) throws MergeException {
final Map<SubmitType, List<Change>> toSubmit =
new HashMap<Project.SubmitType, List<Change>>();
final Set<ObjectId> tips = new HashSet<ObjectId>();
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 <K, T> List<T> getList(final K key, final Map<K, List<T>> map) {
List<T> list = map.get(key);
if (list == null) {
list = new ArrayList<T>();
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<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) {

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

@@ -54,7 +54,7 @@ class MergeSorter {
RevCommit c;
final RevCommitList<RevCommit> contents = new RevCommitList<RevCommit>();
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.
//