MergeTip: Clarify what null means

CherryPick was using a null MergeTip instance where the other
strategies were creating a non-null instance whose current tip was set
to null. Fix the documentation (and CherryPick), explicitly mentioning
a null tip as referring to a branch that is unborn at the start of the
merge operation.

Change-Id: Ie3b5e9e2ae3a9fb62827a10f0577d708fb14a8cc
This commit is contained in:
Dave Borowitz
2015-02-17 16:15:39 -08:00
parent 5d9fc0ce21
commit d89a1cccae
2 changed files with 27 additions and 13 deletions

View File

@@ -14,23 +14,36 @@
package com.google.gerrit.server.git;
import static com.google.common.base.Preconditions.checkArgument;
import com.google.common.collect.Maps;
import com.google.gerrit.common.Nullable;
import java.util.Collection;
import java.util.Map;
/**
* Class describing a merge tip during merge operation.
* <p>
* The current tip of a {@link MergeTip} may be null if the merge operation is
* against an unborn branch, and has not yet been attempted. This is distinct
* from a null {@link MergeTip} instance, which may be used to indicate that a
* merge failed or another error state.
*/
public class MergeTip {
private CodeReviewCommit branchTip;
private Map<String,String> mergeResults;
/**
* @param initial Tip before the merge operation.
* @param toMerge List of CodeReview commits to be merged in merge operation.
* @param initial Tip before the merge operation; may be null, indicating an
* unborn branch.
* @param toMerge List of CodeReview commits to be merged in merge operation;
* may not be null or empty.
*/
public MergeTip(CodeReviewCommit initial, Collection<CodeReviewCommit> toMerge) {
public MergeTip(@Nullable CodeReviewCommit initial,
Collection<CodeReviewCommit> toMerge) {
checkArgument(toMerge != null && !toMerge.isEmpty(),
"toMerge may not be null or empty: %s", toMerge);
this.mergeResults = Maps.newHashMap();
this.branchTip = initial;
// Assume fast-forward merge until opposite is proven.
@@ -42,10 +55,11 @@ public class MergeTip {
/**
* Moves this MergeTip to newTip and appends mergeResult.
*
* @param newTip The new tip
* @param mergedFrom The result of the merge of newTip
* @param newTip The new tip; may not be null.
* @param mergedFrom The result of the merge of newTip.
*/
public void moveTipTo(CodeReviewCommit newTip, String mergedFrom) {
checkArgument(newTip != null);
branchTip = newTip;
mergeResults.put(mergedFrom, newTip.getName());
}
@@ -53,17 +67,18 @@ public class MergeTip {
/**
* The merge results of all the merges of this merge operation.
*
* @return The merge results of the merge operation Map<<sha1 of the commit to
* be merged>, <sha1 of the merge result>>
* @return The merge results of the merge operation as a map of SHA-1 to be
* merged to SHA-1 of the merge result.
*/
public Map<String, String> getMergeResults() {
return this.mergeResults;
return mergeResults;
}
/**
*
* @return The current tip of the current merge operation.
* @return The current tip of the current merge operation; may be null,
* indicating an unborn branch.
*/
@Nullable
public CodeReviewCommit getCurrentTip() {
return branchTip;
}

View File

@@ -66,13 +66,12 @@ public class CherryPick extends SubmitStrategy {
@Override
protected MergeTip _run(CodeReviewCommit branchTip,
Collection<CodeReviewCommit> toMerge) throws MergeException {
MergeTip mergeTip = branchTip != null
? new MergeTip(branchTip, toMerge) : null;
MergeTip mergeTip = new MergeTip(branchTip, toMerge);
List<CodeReviewCommit> sorted = CodeReviewCommit.ORDER.sortedCopy(toMerge);
while (!sorted.isEmpty()) {
CodeReviewCommit n = sorted.remove(0);
try {
if (mergeTip == null) {
if (mergeTip.getCurrentTip() == null) {
mergeTip = cherryPickUnbornRoot(n);
} else if (n.getParentCount() == 0) {
cherryPickRootOntoBranch(n);