opendev/gerrit
Code Issues Proposed changes

Add submit strategy that rebases the change if necessary

Browse Source 
Add a new submit strategy 'Rebase If Necessary' that automatically
rebases the changes on submit if needed. If a change gets rebased the
rebased commit is added as a new patch set to the change.

Bug: issue 1123
Bug: issue 353
Change-Id: If38747a5ea93bb9baff7695bab25670be135f777
Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>
This commit is contained in:
Edwin Kempin 2012-09-12 13:38:44 +02:00 committed by Gerrit Code Review
parent a54984665d
commit 00aa4f05ee
16 changed files with 364 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
Documentation/project-setup.txt
View File

@ -99,6 +99,13 @@ cherry-pick mode. Submitters must remember to submit changes in
the right order since inter-change dependencies will not be
enforced for them.
* Rebase If Necessary
+
If the change being submitted is a strict superset of the destination
branch, then the branch is fast-forwarded to the change. If not,
then the change is automatically rebased and then the branch is
fast-forwarded to the change.
When Gerrit tries to do a merge, by default the merge will only
succeed if there is no path conflict. By selecting the checkbox
`Automatically resolve conflicts` Gerrit will try do a content merge

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.java
View File

@ -68,6 +68,7 @@ public interface AdminConstants extends Constants {
String projectSubmitType_FAST_FORWARD_ONLY();
String projectSubmitType_MERGE_ALWAYS();
String projectSubmitType_MERGE_IF_NECESSARY();
String projectSubmitType_REBASE_IF_NECESSARY();
String projectSubmitType_CHERRY_PICK();
String projectState_ACTIVE();

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties
View File

@ -46,6 +46,7 @@ headingAgreements = Contributor Agreements
projectSubmitType_FAST_FORWARD_ONLY = Fast Forward Only
projectSubmitType_MERGE_IF_NECESSARY = Merge If Necessary
projectSubmitType_REBASE_IF_NECESSARY = Rebase If Necessary
projectSubmitType_MERGE_ALWAYS = Always Merge
projectSubmitType_CHERRY_PICK = Cherry Pick

2
gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/Util.java
View File

@ -45,6 +45,8 @@ public class Util {
return C.projectSubmitType_FAST_FORWARD_ONLY();
case MERGE_IF_NECESSARY:
return C.projectSubmitType_MERGE_IF_NECESSARY();
case REBASE_IF_NECESSARY:
return C.projectSubmitType_REBASE_IF_NECESSARY();
case MERGE_ALWAYS:
return C.projectSubmitType_MERGE_ALWAYS();
case CHERRY_PICK:

2
gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Project.java
View File

@ -70,6 +70,8 @@ public final class Project {
MERGE_IF_NECESSARY,
REBASE_IF_NECESSARY,
MERGE_ALWAYS,
CHERRY_PICK;

24
gerrit-server/src/main/java/com/google/gerrit/server/changedetail/PathConflictException.java Normal file
View File

@ -0,0 +1,24 @@
// Copyright (C) 2012 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.changedetail;
/** Indicates a path conflict during rebase or merge */
public class PathConflictException extends Exception {
private static final long serialVersionUID = 1L;
public PathConflictException(String msg) {
super(msg);
}
}

10
gerrit-server/src/main/java/com/google/gerrit/server/changedetail/RebaseChange.java
View File

@ -163,6 +163,8 @@ public class RebaseChange {
cm.send();
hooks.doPatchsetCreatedHook(change, newPatchSet, db);
} catch (PathConflictException e) {
throw new IOException(e.getMessage());
} finally {
if (inserter != null) {
inserter.release();
@ -271,7 +273,8 @@ public class RebaseChange {
final ObjectInserter inserter, final PatchSet.Id patchSetId,
final Change chg, final Account.Id uploader, final RevCommit baseCommit,
final boolean useContentMerge) throws NoSuchChangeException,
OrmException, IOException, InvalidChangeOperationException {
OrmException, IOException, InvalidChangeOperationException,
PathConflictException {
Change change = chg;
final ChangeControl changeControl =
changeControlFactory.validateFor(change);
@ -394,7 +397,8 @@ public class RebaseChange {
private static ObjectId rebaseCommit(final Repository git,
final ObjectInserter inserter, final RevCommit original,
final RevCommit base, final boolean useContentMerge,
final PersonIdent committerIdent) throws IOException {
final PersonIdent committerIdent) throws IOException,
PathConflictException {
if (original.getParentCount() == 0) {
throw new IOException(
"Commits with no parents cannot be rebased (is this the initial commit?).");
@ -417,7 +421,7 @@ public class RebaseChange {
merger.merge(original, base);
if (merger.getResultTreeId() == null) {
throw new IOException(
throw new PathConflictException(
"The rebase failed since conflicts occured during the merge.");
}

2
gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java
View File

@ -29,6 +29,7 @@ import com.google.gerrit.server.account.PerformRenameGroup;
import com.google.gerrit.server.account.VisibleGroups;
import com.google.gerrit.server.changedetail.DeleteDraftPatchSet;
import com.google.gerrit.server.changedetail.PublishDraft;
import com.google.gerrit.server.changedetail.RebaseChange;
import com.google.gerrit.server.changedetail.Submit;
import com.google.gerrit.server.git.AsyncReceiveCommits;
import com.google.gerrit.server.git.BanCommit;
@ -95,6 +96,7 @@ public class GerritRequestModule extends FactoryModule {
factory(DeleteDraftPatchSet.Factory.class);
factory(PublishComments.Factory.class);
factory(PublishDraft.Factory.class);
factory(RebaseChange.Factory.class);
factory(ReplacePatchSetSender.Factory.class);
factory(RebasedPatchSetSender.Factory.class);
factory(AbandonedSender.Factory.class);

49
gerrit-server/src/main/java/com/google/gerrit/server/git/CherryPick.java
View File

@ -14,10 +14,8 @@
package com.google.gerrit.server.git;
import static com.google.gerrit.server.git.MergeUtil.canFastForward;
import static com.google.gerrit.server.git.MergeUtil.canMerge;
import static com.google.gerrit.server.git.MergeUtil.canCherryPick;
import static com.google.gerrit.server.git.MergeUtil.commit;
import static com.google.gerrit.server.git.MergeUtil.createDryRunInserter;
import static com.google.gerrit.server.git.MergeUtil.hasMissingDependencies;
import static com.google.gerrit.server.git.MergeUtil.markCleanMerges;
import static com.google.gerrit.server.git.MergeUtil.mergeOneCommit;
@ -169,49 +167,8 @@ public class CherryPick extends SubmitStrategy {
@Override
public boolean dryRun(final CodeReviewCommit mergeTip,
final CodeReviewCommit toMerge) throws MergeException {
return canCherryPick(mergeTip, toMerge);
}
private boolean canCherryPick(final CodeReviewCommit mergeTip,
final CodeReviewCommit toMerge) throws MergeException {
if (mergeTip == null) {
// The branch is unborn. Fast-forward is possible.
//
return true;
}
if (toMerge.getParentCount() == 0) {
// Refuse to merge a root commit into an existing branch,
// we cannot obtain a delta for the cherry-pick to apply.
//
return false;
}
if (toMerge.getParentCount() == 1) {
// If there is only one parent, a cherry-pick can be done by
// taking the delta relative to that one parent and redoing
// that on the current merge tip.
//
try {
final ThreeWayMerger m =
newThreeWayMerger(args.repo, createDryRunInserter(),
args.useContentMerge);
m.setBase(toMerge.getParent(0));
return m.merge(mergeTip, toMerge);
} catch (IOException e) {
throw new MergeException("Cannot merge " + toMerge.name(), e);
}
}
// There are multiple parents, so this is a merge commit. We
// don't want to cherry-pick the merge as clients can't easily
// rebase their history with that merge present and replaced
// by an equivalent merge with a different first parent. So
// instead behave as though MERGE_IF_NECESSARY was configured.
//
return canFastForward(args.mergeSorter, mergeTip, args.rw, toMerge)
|| canMerge(args.mergeSorter, args.repo, args.useContentMerge,
mergeTip, toMerge);
return canCherryPick(args.mergeSorter, args.repo, args.useContentMerge,
mergeTip, args.rw, toMerge);
}
private CodeReviewCommit writeCherryPickCommit(final Merger m,

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

@ -21,6 +21,9 @@ enum CommitMergeStatus {
/** */
CLEAN_PICK("Change has been successfully cherry-picked"),
/** */
CLEAN_REBASE("Change has been successfully rebased"),
/** */
ALREADY_MERGED(""),
@ -51,6 +54,11 @@ enum CommitMergeStatus {
+ "\n"
+ "Please merge the change locally and upload the merge commit for review."),
/** */
CANNOT_REBASE_ROOT("Cannot rebase an initial commit onto an existing branch.\n"
+ "\n"
+ "Please merge the change locally and upload the merge commit for review."),
/** */
NOT_FAST_FORWARD("Project policy requires all submissions to be a fast-forward.\n"
+ "\n"

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

@ -684,6 +684,7 @@ public class MergeOp {
break;
}
case CLEAN_REBASE:
case CLEAN_PICK: {
setMerged(c, message(c, txt + " as " + commit.name()));
merged.add(commit);

42
gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
View File

@ -204,6 +204,48 @@ public class MergeUtil {
}
}
public static boolean canCherryPick(final MergeSorter mergeSorter,
final Repository repo, final boolean useContentMerge,
final CodeReviewCommit mergeTip, final RevWalk rw,
final CodeReviewCommit toMerge) throws MergeException {
if (mergeTip == null) {
// The branch is unborn. Fast-forward is possible.
//
return true;
}
if (toMerge.getParentCount() == 0) {
// Refuse to merge a root commit into an existing branch,
// we cannot obtain a delta for the cherry-pick to apply.
//
return false;
}
if (toMerge.getParentCount() == 1) {
// If there is only one parent, a cherry-pick can be done by
// taking the delta relative to that one parent and redoing
// that on the current merge tip.
//
try {
final ThreeWayMerger m =
newThreeWayMerger(repo, createDryRunInserter(), useContentMerge);
m.setBase(toMerge.getParent(0));
return m.merge(mergeTip, toMerge);
} catch (IOException e) {
throw new MergeException("Cannot merge " + toMerge.name(), e);
}
}
// There are multiple parents, so this is a merge commit. We
// don't want to cherry-pick the merge as clients can't easily
// rebase their history with that merge present and replaced
// by an equivalent merge with a different first parent. So
// instead behave as though MERGE_IF_NECESSARY was configured.
//
return canFastForward(mergeSorter, mergeTip, rw, toMerge)
|| canMerge(mergeSorter, repo, useContentMerge, mergeTip, toMerge);
}
public static boolean hasMissingDependencies(final MergeSorter mergeSorter,
final CodeReviewCommit toMerge) throws MergeException {
try {

164
gerrit-server/src/main/java/com/google/gerrit/server/git/RebaseIfNecessary.java Normal file
View File

@ -0,0 +1,164 @@
// Copyright (C) 2012 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.git;
import static com.google.gerrit.server.git.MergeUtil.canCherryPick;
import static com.google.gerrit.server.git.MergeUtil.canFastForward;
import static com.google.gerrit.server.git.MergeUtil.getSubmitter;
import static com.google.gerrit.server.git.MergeUtil.hasMissingDependencies;
import static com.google.gerrit.server.git.MergeUtil.markCleanMerges;
import static com.google.gerrit.server.git.MergeUtil.mergeOneCommit;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.server.changedetail.PathConflictException;
import com.google.gerrit.server.changedetail.RebaseChange;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gwtorm.server.OrmException;
import org.eclipse.jgit.lib.ObjectId;
import java.io.IOException;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
public class RebaseIfNecessary extends SubmitStrategy {
private final RebaseChange rebaseChange;
private final Map<Change.Id, CodeReviewCommit> newCommits;
RebaseIfNecessary(final SubmitStrategy.Arguments args,
final RebaseChange.Factory rebaseChangeFactory) {
super(args);
this.rebaseChange = rebaseChangeFactory.create();
this.newCommits = new HashMap<Change.Id, CodeReviewCommit>();
}
@Override
protected CodeReviewCommit _run(final CodeReviewCommit mergeTip,
final List<CodeReviewCommit> toMerge) throws MergeException {
CodeReviewCommit newMergeTip = mergeTip;
sort(toMerge);
while (!toMerge.isEmpty()) {
final CodeReviewCommit n = toMerge.remove(0);
if (newMergeTip == null) {
// The branch is unborn. Take a fast-forward resolution to
// create the branch.
//
newMergeTip = n;
n.statusCode = CommitMergeStatus.CLEAN_MERGE;
} else if (n.getParentCount() == 0) {
// Refuse to merge a root commit into an existing branch,
// we cannot obtain a delta for the rebase to apply.
//
n.statusCode = CommitMergeStatus.CANNOT_REBASE_ROOT;
} else if (n.getParentCount() == 1) {
if (canFastForward(args.mergeSorter, newMergeTip, args.rw, n)) {
newMergeTip = n;
n.statusCode = CommitMergeStatus.CLEAN_MERGE;
} else {
try {
final PatchSet newPatchSet =
rebaseChange.rebase(args.repo, args.rw, args.inserter,
n.patchsetId, n.change, getSubmitter(args.db, n.patchsetId)
.getAccountId(), newMergeTip, args.useContentMerge);
newMergeTip =
(CodeReviewCommit) args.rw.parseCommit(ObjectId
.fromString(newPatchSet.getRevision().get()));
newMergeTip.copyFrom(n);
newMergeTip.patchsetId = newPatchSet.getId();
newMergeTip.change =
args.db.changes().get(newPatchSet.getId().getParentKey());
newMergeTip.statusCode = CommitMergeStatus.CLEAN_REBASE;
newCommits.put(newPatchSet.getId().getParentKey(), newMergeTip);
setRefLogIdent(getSubmitter(args.db, n.patchsetId));
} catch (PathConflictException e) {
n.statusCode = CommitMergeStatus.PATH_CONFLICT;
} catch (NoSuchChangeException e) {
throw new MergeException("Cannot rebase " + n.name(), e);
} catch (OrmException e) {
throw new MergeException("Cannot rebase " + n.name(), e);
} catch (IOException e) {
throw new MergeException("Cannot rebase " + n.name(), e);
} catch (InvalidChangeOperationException e) {
throw new MergeException("Cannot rebase " + n.name(), e);
}
}
} else if (n.getParentCount() > 1) {
// There are multiple parents, so this is a merge commit. We
// don't want to rebase the merge as clients can't easily
// rebase their history with that merge present and replaced
// by an equivalent merge with a different first parent. So
// instead behave as though MERGE_IF_NECESSARY was configured.
//
try {
if (args.rw.isMergedInto(newMergeTip, n)) {
newMergeTip = n;
} else {
newMergeTip =
mergeOneCommit(args.db, args.identifiedUserFactory,
args.myIdent, args.repo, args.rw, args.inserter,
args.canMergeFlag, args.useContentMerge, args.destBranch,
newMergeTip, n);
}
final PatchSetApproval submitApproval =
markCleanMerges(args.db, args.rw, args.canMergeFlag, newMergeTip,
args.alreadyAccepted);
setRefLogIdent(submitApproval);
} catch (IOException e) {
throw new MergeException("Cannot merge " + n.name(), e);
}
}
args.alreadyAccepted.add(newMergeTip);
}
return newMergeTip;
}
private void sort(final List<CodeReviewCommit> toSort) throws MergeException {
try {
final List<CodeReviewCommit> sorted =
new RebaseSorter(args.rw, args.alreadyAccepted, args.canMergeFlag)
.sort(toSort);
toSort.clear();
toSort.addAll(sorted);
} catch (IOException e) {
throw new MergeException("Commit sorting failed", e);
}
}
@Override
public Map<Change.Id, CodeReviewCommit> getNewCommits() {
return newCommits;
}
@Override
public boolean dryRun(final CodeReviewCommit mergeTip,
final CodeReviewCommit toMerge) throws MergeException {
return !hasMissingDependencies(args.mergeSorter, toMerge)
&& canCherryPick(args.mergeSorter, args.repo, args.useContentMerge,
mergeTip, args.rw, toMerge);
}
}

91
gerrit-server/src/main/java/com/google/gerrit/server/git/RebaseSorter.java Normal file
View File

@ -0,0 +1,91 @@
// Copyright (C) 2012 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.git;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevFlag;
import org.eclipse.jgit.revwalk.RevWalk;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Set;
class RebaseSorter {
private final RevWalk rw;
private final RevFlag canMergeFlag;
private final Set<RevCommit> accepted;
RebaseSorter(final RevWalk rw, final Set<RevCommit> alreadyAccepted,
final RevFlag canMergeFlag) {
this.rw = rw;
this.canMergeFlag = canMergeFlag;
this.accepted = alreadyAccepted;
}
List<CodeReviewCommit> sort(Collection<CodeReviewCommit> incoming)
throws IOException {
final List<CodeReviewCommit> sorted = new ArrayList<CodeReviewCommit>();
final Set<CodeReviewCommit> sort = new HashSet<CodeReviewCommit>(incoming);
while (!sort.isEmpty()) {
final CodeReviewCommit n = removeOne(sort);
rw.resetRetain(canMergeFlag);
rw.markStart(n);
for (RevCommit c : accepted) {
rw.markUninteresting(c);
}
CodeReviewCommit c;
final List<CodeReviewCommit> contents = new ArrayList<CodeReviewCommit>();
while ((c = (CodeReviewCommit) rw.next()) != null) {
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.
//
if (n.missing == null) {
n.statusCode = CommitMergeStatus.MISSING_DEPENDENCY;
n.missing = new ArrayList<CodeReviewCommit>();
}
n.missing.add((CodeReviewCommit) c);
} else {
contents.add(c);
}
}
if (n.statusCode == CommitMergeStatus.MISSING_DEPENDENCY) {
continue;
}
sort.removeAll(contents);
Collections.reverse(contents);
sorted.removeAll(contents);
sorted.addAll(contents);
}
return sorted;
}
private static <T> T removeOne(final Collection<T> c) {
final Iterator<T> i = c.iterator();
final T r = i.next();
i.remove();
return r;
}
}

8
gerrit-server/src/main/java/com/google/gerrit/server/git/SubmitStrategyFactory.java
View File

@ -20,6 +20,7 @@ import com.google.gerrit.reviewdb.client.Project.SubmitType;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.changedetail.RebaseChange;
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
@ -50,6 +51,7 @@ public class SubmitStrategyFactory {
private final Provider<String> urlProvider;
private final ApprovalTypes approvalTypes;
private final GitReferenceUpdated replication;
private final RebaseChange.Factory rebaseChangeFactory;
@Inject
SubmitStrategyFactory(
@ -57,13 +59,15 @@ public class SubmitStrategyFactory {
@GerritPersonIdent final PersonIdent myIdent,
final PatchSetInfoFactory patchSetInfoFactory,
@CanonicalWebUrl @Nullable final Provider<String> urlProvider,
final ApprovalTypes approvalTypes, final GitReferenceUpdated replication) {
final ApprovalTypes approvalTypes, final GitReferenceUpdated replication,
final RebaseChange.Factory rebaseChangeFactory) {
this.identifiedUserFactory = identifiedUserFactory;
this.myIdent = myIdent;
this.patchSetInfoFactory = patchSetInfoFactory;
this.urlProvider = urlProvider;
this.approvalTypes = approvalTypes;
this.replication = replication;
this.rebaseChangeFactory = rebaseChangeFactory;
}
public SubmitStrategy create(final SubmitType submitType, final ReviewDb db,
@ -85,6 +89,8 @@ public class SubmitStrategyFactory {
return new MergeAlways(args);
case MERGE_IF_NECESSARY:
return new MergeIfNecessary(args);
case REBASE_IF_NECESSARY:
return new RebaseIfNecessary(args, rebaseChangeFactory);
default:
final String errorMsg = "No submit strategy for: " + submitType;
log.error(errorMsg);

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

@ -217,7 +217,8 @@ public class SubmoduleOp {
for (final Change chg : submitted) {
final CodeReviewCommit c = commits.get(chg.getId());
if (c != null
&& (c.statusCode == CommitMergeStatus.CLEAN_MERGE || c.statusCode == CommitMergeStatus.CLEAN_PICK)) {
&& (c.statusCode == CommitMergeStatus.CLEAN_MERGE
|| c.statusCode == CommitMergeStatus.CLEAN_PICK || c.statusCode == CommitMergeStatus.CLEAN_REBASE)) {
msgbuf += "\n";
msgbuf += c.getFullMessage();
}