opendev/gerrit
Code Issues Proposed changes

Add new copy votes policy for merge commits

Browse Source 
Adds new 'copyAllScoresOnMergeFirstParentUpdate' vote policy. This
policy allows to carry over votes on "merge commit rebase". This meas
when only first (left parent) changes and rest of parent(s) stays the
same.

Such policy is useful for changes that are merge commit of feature
branches that tracks destination branch. When it is enabled it reduces
the overhead of CI and human reviews on merge commits that only
incorporate newer changes from destination branch in next patch set.

Change-Id: I02062544608546498d9605c628bc1b7a0386e91a
Signed-off-by: Dariusz Luksza <dariusz@luksza.org>
This commit is contained in:
Dariusz Luksza 2016-01-29 12:06:20 +01:00
parent 6f86343193
commit cedbbee78e
7 changed files with 69 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
Documentation/config-labels.txt
View File

@ -246,6 +246,22 @@ forward when a new patch set is uploaded. This can be used to enable
sticky approvals, reducing turn-around for trivial cleanups prior to
submitting a change. Defaults to false.
[[lable_copyAllScoresOnMergCommitFirstParentUpdate]]
=== `label.Label-Name.copyAllScoresOnMergeCommitFirstParentUpdate`
This policy is useful if you don't want to trigger CI or human
verification again if your target branch moved on but the feature
branch being merged into the target branch did not change. It only
applies if the patch set is a merge commit.
If true, all scores for the label are copied forward when a new
patch set is uploaded that is a new merge commit which only
differs from the previous patch set in its first parent. The
first parent would be the parent of the merge commit that is part
of the change's target branch, whereas the other parent(s) refer to
the feature branch(es) to be merged.
Defaults to false.
[[label_copyAllScoresOnTrivialRebase]]
=== `label.Label-Name.copyAllScoresOnTrivialRebase`

14
gerrit-common/src/main/java/com/google/gerrit/common/data/LabelType.java
View File

@ -29,6 +29,7 @@ public class LabelType {
public static final boolean DEF_COPY_ALL_SCORES_IF_NO_CHANGE = true;
public static final boolean DEF_COPY_ALL_SCORES_IF_NO_CODE_CHANGE = false;
public static final boolean DEF_COPY_ALL_SCORES_ON_TRIVIAL_REBASE = false;
public static final boolean DEF_COPY_ALL_SCORES_ON_MERGE_FIRST_PARENT_UPDATE = false;
public static final boolean DEF_COPY_MAX_SCORE = false;
public static final boolean DEF_COPY_MIN_SCORE = false;
@ -99,6 +100,7 @@ public class LabelType {
protected String functionName;
protected boolean copyMinScore;
protected boolean copyMaxScore;
protected boolean copyAllScoresOnMergeFirstParentUpdate;
protected boolean copyAllScoresOnTrivialRebase;
protected boolean copyAllScoresIfNoCodeChange;
protected boolean copyAllScoresIfNoChange;
@ -138,6 +140,8 @@ public class LabelType {
setCopyAllScoresIfNoChange(DEF_COPY_ALL_SCORES_IF_NO_CHANGE);
setCopyAllScoresIfNoCodeChange(DEF_COPY_ALL_SCORES_IF_NO_CODE_CHANGE);
setCopyAllScoresOnTrivialRebase(DEF_COPY_ALL_SCORES_ON_TRIVIAL_REBASE);
setCopyAllScoresOnMergeFirstParentUpdate(
DEF_COPY_ALL_SCORES_ON_MERGE_FIRST_PARENT_UPDATE);
setCopyMaxScore(DEF_COPY_MAX_SCORE);
setCopyMinScore(DEF_COPY_MIN_SCORE);
}
@ -217,6 +221,16 @@ public class LabelType {
this.copyMaxScore = copyMaxScore;
}
public boolean isCopyAllScoresOnMergeFirstParentUpdate() {
return copyAllScoresOnMergeFirstParentUpdate;
}
public void setCopyAllScoresOnMergeFirstParentUpdate(
boolean copyAllScoresOnMergeFirstParentUpdate) {
this.copyAllScoresOnMergeFirstParentUpdate =
copyAllScoresOnMergeFirstParentUpdate;
}
public boolean isCopyAllScoresOnTrivialRebase() {
return copyAllScoresOnTrivialRebase;
}

4
gerrit-server/src/main/java/com/google/gerrit/server/ApprovalCopier.java
View File

@ -16,6 +16,7 @@ package com.google.gerrit.server;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.gerrit.server.change.ChangeKind.MERGE_FIRST_PARENT_UPDATE;
import static com.google.gerrit.server.change.ChangeKind.NO_CHANGE;
import static com.google.gerrit.server.change.ChangeKind.NO_CODE_CHANGE;
import static com.google.gerrit.server.change.ChangeKind.TRIVIAL_REBASE;
@ -172,7 +173,8 @@ public class ApprovalCopier {
// may not be psId.get() - 1).
return true;
}
return (type.isCopyAllScoresOnTrivialRebase() && kind == TRIVIAL_REBASE)
return (type.isCopyAllScoresOnMergeFirstParentUpdate() && kind == MERGE_FIRST_PARENT_UPDATE)
|| (type.isCopyAllScoresOnTrivialRebase() && kind == TRIVIAL_REBASE)
|| (type.isCopyAllScoresIfNoCodeChange() && kind == NO_CODE_CHANGE)
|| (type.isCopyAllScoresIfNoChange() && kind == NO_CHANGE);
}

3
gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeKind.java
View File

@ -22,6 +22,9 @@ public enum ChangeKind {
/** Conflict-free merge between the new parent and the prior patch set. */
TRIVIAL_REBASE,
/** Conflict-free change of first (left) parent of a merge commit. */
MERGE_FIRST_PARENT_UPDATE,
/** Same tree and same parent tree. */
NO_CODE_CHANGE,

30
gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java
View File

@ -21,6 +21,7 @@ import static org.eclipse.jgit.lib.ObjectIdSerialization.writeNotNull;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.cache.Cache;
import com.google.common.cache.Weigher;
import com.google.common.collect.FluentIterable;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.server.ReviewDb;
@ -50,8 +51,10 @@ import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.io.Serializable;
import java.util.Arrays;
import java.util.Collection;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
@ -209,7 +212,8 @@ public class ChangeKindCacheImpl implements ChangeKindCache {
return ChangeKind.NO_CHANGE;
}
if (prior.getParentCount() != 1 || next.getParentCount() != 1) {
if ((prior.getParentCount() != 1 || next.getParentCount() != 1)
&& !onlyFirstParentChanged(prior, next)) {
// Trivial rebases done by machine only work well on 1 parent.
return ChangeKind.REWORK;
}
@ -223,7 +227,11 @@ public class ChangeKindCacheImpl implements ChangeKindCache {
try {
if (merger.merge(next.getParent(0), prior)
&& merger.getResultTreeId().equals(next.getTree())) {
return ChangeKind.TRIVIAL_REBASE;
if (prior.getParentCount() == 1) {
return ChangeKind.TRIVIAL_REBASE;
} else {
return ChangeKind.MERGE_FIRST_PARENT_UPDATE;
}
}
} catch (LargeObjectException e) {
// Some object is too large for the merge attempt to succeed. Assume
@ -233,6 +241,24 @@ public class ChangeKindCacheImpl implements ChangeKindCache {
}
}
public static boolean onlyFirstParentChanged(RevCommit prior, RevCommit next) {
return !sameFirstParents(prior, next) && sameRestOfParents(prior, next);
}
private static boolean sameFirstParents(RevCommit prior, RevCommit next) {
return prior.getParent(0).equals(next.getParent(0));
}
private static boolean sameRestOfParents(RevCommit prior, RevCommit next) {
Set<RevCommit> priorRestParents = allExceptFirstParent(prior.getParents());
Set<RevCommit> nextRestParents = allExceptFirstParent(next.getParents());
return priorRestParents.equals(nextRestParents);
}
private static Set<RevCommit> allExceptFirstParent(RevCommit[] parents) {
return FluentIterable.from(Arrays.asList(parents)).skip(1).toSet();
}
private static boolean isSameDeltaAndTree(RevCommit prior, RevCommit next) {
if (next.getTree() != prior.getTree()) {
return false;

4
gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
View File

@ -134,6 +134,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
private static final String KEY_DEFAULT_VALUE = "defaultValue";
private static final String KEY_COPY_MIN_SCORE = "copyMinScore";
private static final String KEY_COPY_MAX_SCORE = "copyMaxScore";
private static final String KEY_COPY_ALL_SCORES_ON_MERGE_FIRST_PARENT_UPDATE = "copyAllScoresOnMergeFirstParentUpdate";
private static final String KEY_COPY_ALL_SCORES_ON_TRIVIAL_REBASE = "copyAllScoresOnTrivialRebase";
private static final String KEY_COPY_ALL_SCORES_IF_NO_CODE_CHANGE = "copyAllScoresIfNoCodeChange";
private static final String KEY_COPY_ALL_SCORES_IF_NO_CHANGE = "copyAllScoresIfNoChange";
@ -716,6 +717,9 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
label.setCopyMaxScore(
rc.getBoolean(LABEL, name, KEY_COPY_MAX_SCORE,
LabelType.DEF_COPY_MAX_SCORE));
label.setCopyAllScoresOnMergeFirstParentUpdate(
rc.getBoolean(LABEL, name, KEY_COPY_ALL_SCORES_ON_MERGE_FIRST_PARENT_UPDATE,
LabelType.DEF_COPY_ALL_SCORES_ON_MERGE_FIRST_PARENT_UPDATE));
label.setCopyAllScoresOnTrivialRebase(
rc.getBoolean(LABEL, name, KEY_COPY_ALL_SCORES_ON_TRIVIAL_REBASE,
LabelType.DEF_COPY_ALL_SCORES_ON_TRIVIAL_REBASE));

1
gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
View File

@ -2198,6 +2198,7 @@ public class ReceiveCommits {
private String changeKindMessage(ChangeKind changeKind) {
switch (changeKind) {
case MERGE_FIRST_PARENT_UPDATE:
case TRIVIAL_REBASE:
case NO_CHANGE:
return ": Patch Set " + priorPatchSet.get() + " was rebased";