opendev/gerrit
Code Issues Proposed changes

Add rejectEmptyCommit project config

Browse Source 
If a change is identified as the root cause of a problem, different
users sometimes create reverts independently and try to submit them. The
first revert merges cleanly and reverts the problematic code. The second
revert rebases cleanly, but results in an empty commit that is then
merged.

Some users don't want empty commits in their project. This commit adds a
project config to prevent empty commits as a result of merging changes
in Gerrit.

The UI will be adapted in a later commit to allow easy modifications of
the new config option.

Change-Id: Ied0c501a6cb8963328440074529834cb43e96439
This commit is contained in:
Patrick Hiesel
2018-01-08 17:20:15 +01:00
parent 93f9809f84
commit dc285c7b46
19 changed files with 149 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
java/com/google/gerrit/server/git/strategy/CherryPick.java
View File

@@ -15,10 +15,12 @@
package com.google.gerrit.server.git.strategy;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.gerrit.server.git.strategy.CommitMergeStatus.EMPTY_COMMIT;
import static com.google.gerrit.server.git.strategy.CommitMergeStatus.SKIPPED_IDENTICAL_TREE;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.extensions.restapi.MergeConflictException;
import com.google.gerrit.reviewdb.client.BooleanProjectConfig;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetInfo;
import com.google.gerrit.server.ChangeUtil;
@@ -123,6 +125,10 @@ public class CherryPick extends SubmitStrategy {
toMerge.setStatusCode(CommitMergeStatus.PATH_CONFLICT);
return;
} catch (MergeIdenticalTreeException mie) {
if (args.project.is(BooleanProjectConfig.REJECT_EMPTY_COMMIT)) {
toMerge.setStatusCode(EMPTY_COMMIT);
return;
}
toMerge.setStatusCode(SKIPPED_IDENTICAL_TREE);
return;
}

7
java/com/google/gerrit/server/git/strategy/CommitMergeStatus.java
View File

@@ -60,7 +60,12 @@ public enum CommitMergeStatus {
NOT_FAST_FORWARD(
"Project policy requires all submissions to be a fast-forward.\n"
+ "\n"
+ "Please rebase the change locally and upload again for review.");
+ "Please rebase the change locally and upload again for review."),
EMPTY_COMMIT(
"Change could not be merged because the commit is empty.\n"
+ "\n"
+ "Project policy requires all commits to contain modifications to at least one file.");
private final String message;

9
java/com/google/gerrit/server/git/strategy/FastForwardOp.java
View File

@@ -14,6 +14,9 @@
package com.google.gerrit.server.git.strategy;
import static com.google.gerrit.server.git.strategy.CommitMergeStatus.EMPTY_COMMIT;
import com.google.gerrit.reviewdb.client.BooleanProjectConfig;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.IntegrationException;
import com.google.gerrit.server.update.RepoContext;
@@ -25,6 +28,12 @@ class FastForwardOp extends SubmitStrategyOp {
@Override
protected void updateRepoImpl(RepoContext ctx) throws IntegrationException {
if (args.project.is(BooleanProjectConfig.REJECT_EMPTY_COMMIT)
&& toMerge.getTree().equals(toMerge.getParent(0).getTree())) {
toMerge.setStatusCode(EMPTY_COMMIT);
return;
}
args.mergeTip.moveTipTo(toMerge, toMerge);
}
}

8
java/com/google/gerrit/server/git/strategy/MergeOneOp.java
View File

@@ -14,6 +14,9 @@
package com.google.gerrit.server.git.strategy;
import static com.google.gerrit.server.git.strategy.CommitMergeStatus.EMPTY_COMMIT;
import com.google.gerrit.reviewdb.client.BooleanProjectConfig;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.IntegrationException;
import com.google.gerrit.server.update.RepoContext;
@@ -47,6 +50,11 @@ class MergeOneOp extends SubmitStrategyOp {
args.destBranch,
args.mergeTip.getCurrentTip(),
toMerge);
if (args.project.is(BooleanProjectConfig.REJECT_EMPTY_COMMIT)
&& merged.getTree().equals(merged.getParent(0).getTree())) {
toMerge.setStatusCode(EMPTY_COMMIT);
return;
}
args.mergeTip.moveTipTo(amendGitlink(merged), toMerge);
}
}

12
java/com/google/gerrit/server/git/strategy/RebaseSubmitStrategy.java
View File

@@ -15,6 +15,7 @@
package com.google.gerrit.server.git.strategy;
import static com.google.common.base.Preconditions.checkState;
import static com.google.gerrit.server.git.strategy.CommitMergeStatus.EMPTY_COMMIT;
import static com.google.gerrit.server.git.strategy.CommitMergeStatus.SKIPPED_IDENTICAL_TREE;
import com.google.common.collect.ImmutableList;
@@ -124,6 +125,12 @@ public class RebaseSubmitStrategy extends SubmitStrategy {
if (args.mergeUtil.canFastForward(
args.mergeSorter, args.mergeTip.getCurrentTip(), args.rw, toMerge)) {
if (!rebaseAlways) {
if (args.project.is(BooleanProjectConfig.REJECT_EMPTY_COMMIT)
&& toMerge.getTree().equals(toMerge.getParent(0).getTree())) {
toMerge.setStatusCode(EMPTY_COMMIT);
return;
}
args.mergeTip.moveTipTo(amendGitlink(toMerge), toMerge);
toMerge.setStatusCode(CommitMergeStatus.CLEAN_MERGE);
acceptMergeTip(args.mergeTip);
@@ -192,6 +199,11 @@ public class RebaseSubmitStrategy extends SubmitStrategy {
newCommit = args.rw.parseCommit(rebaseOp.getRebasedCommit());
newPatchSetId = rebaseOp.getPatchSetId();
}
if (args.project.is(BooleanProjectConfig.REJECT_EMPTY_COMMIT)
&& newCommit.getTree().equals(newCommit.getParent(0).getTree())) {
toMerge.setStatusCode(EMPTY_COMMIT);
return;
}
newCommit = amendGitlink(newCommit);
newCommit.copyFrom(toMerge);
newCommit.setPatchsetId(newPatchSetId);

1
java/com/google/gerrit/server/git/strategy/SubmitStrategyListener.java
View File

@@ -128,6 +128,7 @@ public class SubmitStrategyListener implements BatchUpdateListener {
case CANNOT_CHERRY_PICK_ROOT:
case CANNOT_REBASE_ROOT:
case NOT_FAST_FORWARD:
case EMPTY_COMMIT:
// TODO(dborowitz): Reformat these messages to be more appropriate for
// short problem descriptions.
commitStatus.problem(id, CharMatcher.is('\n').collapseFrom(s.getMessage(), ' '));