Option to reject implicit merges when pushing changes for review

An implicit merge is a case where by submitting an open change one also
merges a branch into the target branch. Typically, this happens when a
change is done on top of master and, by mistake, pushed to stable
branch. Merging this change would also implicitly merge master into
stable.

Example 1:

  o < change pushed for stable
  |
  o < master
  |
  o < stable

  Submitting this change will implicitly merge master into stable:

  o < change pushed for stable, stable
  |
  o < master
  |
  o

Example 2:

           o < change pushed for stable
           |
  master > o   o < stable
            \ /
             o

  Submitting this change will implicitly merge master into stable:

               o < stable
              /|
            /  |
           o < change pushed for stable
           |   |
  master > o   o
            \ /
             o

A new project property receive.rejectImplicitMerges controls whether an
implicit merge will be rejected. When an implicit merge is detected
Gerrit will print error(s) to the user:

  remote: ERROR: Implicit Merge of 39adddb Commit message subject
  remote: ERROR: Implicit Merge of ...

and will reject the push.

Bug: issue 1107
Change-Id: I0b14c64bebe28ea5579fc11f6beedacf5982e5aa
This commit is contained in:
Saša Živkov
2015-11-17 17:37:43 +01:00
parent 968e3b0c6d
commit 8af982ae60
17 changed files with 243 additions and 4 deletions

View File

@@ -1608,8 +1608,8 @@ public class ReceiveCommits {
rp.getRevWalk().sort(RevSort.TOPO);
rp.getRevWalk().sort(RevSort.REVERSE, true);
try {
rp.getRevWalk().markStart(
rp.getRevWalk().parseCommit(magicBranch.cmd.getNewId()));
RevCommit start = rp.getRevWalk().parseCommit(magicBranch.cmd.getNewId());
rp.getRevWalk().markStart(start);
if (magicBranch.baseCommit != null) {
logDebug("Marking {} base commits uninteresting",
magicBranch.baseCommit.size());
@@ -1635,6 +1635,15 @@ public class ReceiveCommits {
receiveConfig.getEffectiveMaxBatchChangesLimit(user);
int total = 0;
int alreadyTracked = 0;
boolean rejectImplicitMerges = start.getParentCount() == 1
&& projectCache.get(project.getNameKey()).isRejectImplicitMerges();
Set<RevCommit> mergedParents;
if (rejectImplicitMerges) {
mergedParents = new HashSet<>();
} else {
mergedParents = null;
}
for (;;) {
RevCommit c = rp.getRevWalk().next();
if (c == null) {
@@ -1644,6 +1653,14 @@ public class ReceiveCommits {
String name = c.name();
groupCollector.visit(c);
Collection<Ref> existingRefs = existing.get(c);
if (rejectImplicitMerges) {
for (RevCommit p : c.getParents()) {
mergedParents.add(p);
}
mergedParents.remove(c);
}
if (!existingRefs.isEmpty()) { // Commit is already tracked.
alreadyTracked++;
// Corner cases where an existing commit might need a new group:
@@ -1717,6 +1734,10 @@ public class ReceiveCommits {
+ " lookups", total, alreadyTracked, newChanges.size(),
pending.size());
if (rejectImplicitMerges) {
rejectImplicitMerges(mergedParents);
}
for (Iterator<ChangeLookup> itr = pending.iterator(); itr.hasNext();) {
ChangeLookup p = itr.next();
if (newChangeIds.contains(p.changeKey)) {
@@ -1833,6 +1854,38 @@ public class ReceiveCommits {
}
}
private void rejectImplicitMerges(Set<RevCommit> mergedParents)
throws MissingObjectException, IncorrectObjectTypeException, IOException {
if (!mergedParents.isEmpty()) {
Ref targetRef = allRefs.get(magicBranch.ctl.getRefName());
if (targetRef != null) {
RevWalk rw = rp.getRevWalk();
RevCommit tip = rw.parseCommit(targetRef.getObjectId());
boolean containsImplicitMerges = true;
for (RevCommit p : mergedParents) {
containsImplicitMerges &= !rw.isMergedInto(p, tip);
}
if (containsImplicitMerges) {
rw.reset();
for (RevCommit p : mergedParents) {
rw.markStart(p);
}
rw.markUninteresting(tip);
RevCommit c;
while ((c = rw.next()) != null) {
rw.parseBody(c);
messages.add(new CommitValidationMessage(
"ERROR: Implicit Merge of " + c.abbreviate(7).name()
+ " " + c.getShortMessage(), false));
}
reject(magicBranch.cmd, "implicit merges detected");
}
}
}
}
private void markHeadsAsUninteresting(RevWalk rw, @Nullable String forRef) {
int i = 0;
for (Ref ref : allRefs.values()) {