opendev/gerrit
Code Issues Proposed changes

Add gitlink update for superproject changes in MergeOp

Browse Source 
In current solution, we update the gitlink in superproject in a separate
commit. Ideally, if there are open changes for the superproject in the
same topic of submodule changes, we would like to update the gitlink
together with superproject changes, i.e. update in one commit.

To achieve this, we amend the commit that created during updateRepo()
with gitlink update, then sort all projects by the topological order and
execute them together.

Change-Id: I250ebec44d83fde7a8090d30a17248fcf28841e4
This commit is contained in:
Zhen Chen
2016-06-16 18:10:17 -07:00
parent 9c55ab0ec1
commit fffb2216c8
10 changed files with 110 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -488,29 +488,17 @@ public class MergeOp implements AutoCloseable {
}
// Done checks that don't involve running submit strategies.
commits.maybeFailVerbose();
List<SubmitStrategy> strategies = new ArrayList<>(branches.size());
for (Branch.NameKey branch : branches) {
OpenRepo or = orm.getRepo(branch.getParentKey());
OpenBranch ob = or.getBranch(branch);
BranchBatch submitting = toSubmit.get(branch);
checkNotNull(submitting.submitType(),
"null submit type for %s; expected to previously fail fast",
submitting);
Set<CodeReviewCommit> commitsToSubmit = commits(submitting.changes());
ob.mergeTip = new MergeTip(ob.oldTip, commitsToSubmit);
SubmitStrategy strategy = createStrategy(or, ob.mergeTip, branch,
submitting.submitType(), ob.oldTip);
strategies.add(strategy);
strategy.addOps(or.getUpdate(), commitsToSubmit);
}
SubmoduleOp submoduleOp = subOpFactory.create(branches, orm);
try {
List<SubmitStrategy> strategies = getSubmitStrategies(toSubmit, submoduleOp);
Set<Project.NameKey> allProjects = submoduleOp.getProjectsInOrder();
// in case superproject subscription is disabled, allProjects would be null
if (allProjects == null) {
allProjects = projects;
}
BatchUpdate.execute(
batchUpdates(projects),
orm.batchUpdates(allProjects),
new SubmitStrategyListener(submitInput, strategies, commits));
SubmoduleOp subOp = subOpFactory.create(branches, orm);
subOp.updateSuperProjects();
} catch (UpdateException | SubmoduleException e) {
// BatchUpdate may have inadvertently wrapped an IntegrationException
// thrown by some legacy SubmitStrategyOp code that intended the error
@@ -530,12 +518,37 @@ public class MergeOp implements AutoCloseable {
}
}
private List<BatchUpdate> batchUpdates(Collection<Project.NameKey> projects) {
List<BatchUpdate> updates = new ArrayList<>(projects.size());
for (Project.NameKey project : projects) {
updates.add(orm.getRepo(project).getUpdate());
private List<SubmitStrategy> getSubmitStrategies(
Map<Branch.NameKey, BranchBatch> toSubmit, SubmoduleOp submoduleOp)
throws IntegrationException {
List<SubmitStrategy> strategies = new ArrayList<>();
Set<Branch.NameKey> allBranches = submoduleOp.getBranchesInOrder();
// in case superproject subscription is disabled, allBranches would be null
if (allBranches == null) {
allBranches = toSubmit.keySet();
}
return updates;
for (Branch.NameKey branch : allBranches) {
OpenRepo or = orm.getRepo(branch.getParentKey());
if (toSubmit.containsKey(branch)) {
BranchBatch submitting = toSubmit.get(branch);
OpenBranch ob = or.getBranch(branch);
checkNotNull(submitting.submitType(),
"null submit type for %s; expected to previously fail fast",
submitting);
Set<CodeReviewCommit> commitsToSubmit = commits(submitting.changes());
ob.mergeTip = new MergeTip(ob.oldTip, commitsToSubmit);
SubmitStrategy strategy = createStrategy(or, ob.mergeTip, branch,
submitting.submitType(), ob.oldTip, submoduleOp);
strategies.add(strategy);
strategy.addOps(or.getUpdate(), commitsToSubmit);
} else {
// no open change for this branch
// add submodule triggered op into BatchUpdate
submoduleOp.addOp(or.getUpdate(), branch);
}
}
return strategies;
}
private Set<CodeReviewCommit> commits(List<ChangeData> cds) {
@@ -552,10 +565,10 @@ public class MergeOp implements AutoCloseable {
private SubmitStrategy createStrategy(OpenRepo or,
MergeTip mergeTip, Branch.NameKey destBranch, SubmitType submitType,
CodeReviewCommit branchTip) throws IntegrationException {
CodeReviewCommit branchTip, SubmoduleOp submoduleOp) throws IntegrationException {
return submitStrategyFactory.create(submitType, db, or.repo, or.rw, or.ins,
or.canMergeFlag, getAlreadyAccepted(or, branchTip), destBranch, caller,
mergeTip, commits, submissionId, submitInput.notify);
mergeTip, commits, submissionId, submitInput.notify, submoduleOp);
}
private Set<RevCommit> getAlreadyAccepted(OpenRepo or,

2
gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleException.java
View File

@@ -15,7 +15,7 @@
package com.google.gerrit.server.git;
/** Indicates the gitlink's update cannot be processed at this time. */
class SubmoduleException extends Exception {
public class SubmoduleException extends Exception {
private static final long serialVersionUID = 1L;
SubmoduleException(final String msg) {

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

@@ -308,11 +308,8 @@ public class SubmoduleOp {
superProjects.add(project);
// get a new BatchUpdate for the super project
orm.openRepo(project, false);
//TODO:czhen remove this when MergeOp combine this into BatchUpdate
orm.getRepo(project).resetUpdate();
for (Branch.NameKey branch : dst.get(project)) {
SubmoduleOp.GitlinkOp op = new SubmoduleOp.GitlinkOp(branch);
orm.getRepo(project).getUpdate().addRepoOnlyOp(op);
addOp(orm.getRepo(project).getUpdate(), branch);
}
}
}
@@ -505,10 +502,18 @@ public class SubmoduleOp {
return sortedBranches;
}
public boolean hasSubscription(Branch.NameKey branch) {
return targets.containsKey(branch);
}
public void addBranchTip(Branch.NameKey branch, CodeReviewCommit tip) {
branchTips.put(branch, tip);
}
public void addOp(BatchUpdate bu, Branch.NameKey branch) {
bu.addRepoOnlyOp(new GitlinkOp(branch));
}
private void logDebug(String msg, Object... args) {
if (log.isDebugEnabled()) {
log.debug("[" + orm.getSubmissionId() + "]" + msg, args);

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

@@ -74,11 +74,12 @@ public class CherryPick extends SubmitStrategy {
}
@Override
protected void updateRepoImpl(RepoContext ctx) {
protected void updateRepoImpl(RepoContext ctx) throws IntegrationException {
// The branch is unborn. Take fast-forward resolution to create the
// branch.
args.mergeTip.moveTipTo(toMerge, toMerge);
toMerge.setStatusCode(CommitMergeStatus.CLEAN_MERGE);
CodeReviewCommit newCommit = amendGitlink(toMerge);
args.mergeTip.moveTipTo(newCommit, toMerge);
newCommit.setStatusCode(CommitMergeStatus.CLEAN_MERGE);
}
}
@@ -105,7 +106,8 @@ public class CherryPick extends SubmitStrategy {
}
@Override
protected void updateRepoImpl(RepoContext ctx) throws IOException {
protected void updateRepoImpl(RepoContext ctx)
throws IntegrationException, IOException {
// 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.
@@ -132,6 +134,7 @@ public class CherryPick extends SubmitStrategy {
}
// Initial copy doesn't have new patch set ID since change hasn't been
// updated yet.
newCommit = amendGitlink(newCommit);
newCommit.copyFrom(toMerge);
newCommit.setPatchsetId(psId);
newCommit.setStatusCode(CommitMergeStatus.CLEAN_PICK);
@@ -189,12 +192,13 @@ public class CherryPick extends SubmitStrategy {
// was configured.
MergeTip mergeTip = args.mergeTip;
if (args.rw.isMergedInto(mergeTip.getCurrentTip(), toMerge)) {
mergeTip.moveTipTo(toMerge, toMerge);
mergeTip.moveTipTo(amendGitlink(toMerge), toMerge);
} else {
PersonIdent myIdent = new PersonIdent(args.serverIdent, ctx.getWhen());
CodeReviewCommit result = args.mergeUtil.mergeOneCommit(myIdent,
myIdent, args.repo, args.rw, args.inserter, args.destBranch,
mergeTip.getCurrentTip(), toMerge);
result = amendGitlink(result);
mergeTip.moveTipTo(result, toMerge);
}
args.mergeUtil.markCleanMerges(args.rw, args.canMergeFlag,

7
gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/FastForwardOp.java
View File

@@ -18,16 +18,13 @@ import com.google.gerrit.server.git.BatchUpdate.RepoContext;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.IntegrationException;
import java.io.IOException;
class FastForwardOp extends SubmitStrategyOp {
FastForwardOp(SubmitStrategy.Arguments args, CodeReviewCommit toMerge) {
super(args, toMerge);
}
@Override
public void updateRepoImpl(RepoContext ctx)
throws IntegrationException, IOException {
args.mergeTip.moveTipTo(toMerge, toMerge);
protected void updateRepoImpl(RepoContext ctx) throws IntegrationException {
args.mergeTip.moveTipTo(amendGitlink(toMerge), toMerge);
}
}

2
gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/MergeOneOp.java
View File

@@ -44,6 +44,6 @@ class MergeOneOp extends SubmitStrategyOp {
args.mergeUtil.mergeOneCommit(caller, args.serverIdent,
ctx.getRepository(), args.rw, ctx.getInserter(), args.destBranch,
args.mergeTip.getCurrentTip(), toMerge);
args.mergeTip.moveTipTo(merged, toMerge);
args.mergeTip.moveTipTo(amendGitlink(merged), toMerge);
}
}

13
gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java
View File

@@ -71,11 +71,12 @@ public class RebaseIfNecessary extends SubmitStrategy {
}
@Override
public void updateRepoImpl(RepoContext ctx) {
public void updateRepoImpl(RepoContext ctx) throws IntegrationException {
// The branch is unborn. Take fast-forward resolution to create the
// branch.
toMerge.setStatusCode(CommitMergeStatus.CLEAN_MERGE);
args.mergeTip.moveTipTo(toMerge, toMerge);
CodeReviewCommit newCommit = amendGitlink(toMerge);
args.mergeTip.moveTipTo(newCommit, toMerge);
acceptMergeTip(args.mergeTip);
}
}
@@ -110,8 +111,7 @@ public class RebaseIfNecessary extends SubmitStrategy {
// BatchUpdate how to produce CodeReviewRevWalks.
if (args.mergeUtil.canFastForward(args.mergeSorter,
args.mergeTip.getCurrentTip(), args.rw, toMerge)) {
toMerge.setStatusCode(CommitMergeStatus.CLEAN_MERGE);
args.mergeTip.moveTipTo(toMerge, toMerge);
args.mergeTip.moveTipTo(amendGitlink(toMerge), toMerge);
acceptMergeTip(args.mergeTip);
return;
}
@@ -134,6 +134,7 @@ public class RebaseIfNecessary extends SubmitStrategy {
"Cannot rebase " + toMerge.name() + ": " + e.getMessage(), e);
}
newCommit = args.rw.parseCommit(rebaseOp.getRebasedCommit());
newCommit = amendGitlink(newCommit);
newCommit.copyFrom(toMerge);
newCommit.setStatusCode(CommitMergeStatus.CLEAN_REBASE);
newCommit.setPatchsetId(rebaseOp.getPatchSetId());
@@ -182,13 +183,13 @@ public class RebaseIfNecessary extends SubmitStrategy {
// configured.
MergeTip mergeTip = args.mergeTip;
if (args.rw.isMergedInto(mergeTip.getCurrentTip(), toMerge)) {
mergeTip.moveTipTo(toMerge, toMerge);
mergeTip.moveTipTo(amendGitlink(toMerge), toMerge);
acceptMergeTip(mergeTip);
} else {
CodeReviewCommit newTip = args.mergeUtil.mergeOneCommit(
args.serverIdent, args.serverIdent, args.repo, args.rw,
args.inserter, args.destBranch, mergeTip.getCurrentTip(), toMerge);
mergeTip.moveTipTo(newTip, toMerge);
mergeTip.moveTipTo(amendGitlink(newTip), toMerge);
}
args.mergeUtil.markCleanMerges(args.rw, args.canMergeFlag,
mergeTip.getCurrentTip(), args.alreadyAccepted);

9
gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java
View File

@@ -41,6 +41,7 @@ import com.google.gerrit.server.git.MergeOp.CommitStatus;
import com.google.gerrit.server.git.MergeSorter;
import com.google.gerrit.server.git.MergeTip;
import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.git.SubmoduleOp;
import com.google.gerrit.server.git.TagCache;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.project.ChangeControl;
@@ -93,7 +94,8 @@ public abstract class SubmitStrategy {
ReviewDb db,
Set<RevCommit> alreadyAccepted,
String submissionId,
NotifyHandling notifyHandling);
NotifyHandling notifyHandling,
SubmoduleOp submoduleOp);
}
final AccountCache accountCache;
@@ -125,6 +127,7 @@ public abstract class SubmitStrategy {
final String submissionId;
final SubmitType submitType;
final NotifyHandling notifyHandling;
final SubmoduleOp submoduleOp;
final ProjectState project;
final MergeSorter mergeSorter;
@@ -160,7 +163,8 @@ public abstract class SubmitStrategy {
@Assisted Set<RevCommit> alreadyAccepted,
@Assisted String submissionId,
@Assisted SubmitType submitType,
@Assisted NotifyHandling notifyHandling) {
@Assisted NotifyHandling notifyHandling,
@Assisted SubmoduleOp submoduleOp) {
this.accountCache = accountCache;
this.approvalsUtil = approvalsUtil;
this.batchUpdateFactory = batchUpdateFactory;
@@ -190,6 +194,7 @@ public abstract class SubmitStrategy {
this.submissionId = submissionId;
this.submitType = submitType;
this.notifyHandling = notifyHandling;
this.submoduleOp = submoduleOp;
this.project = checkNotNull(projectCache.get(destBranch.getParentKey()),
"project not found: %s", destBranch.getParentKey());

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

@@ -23,6 +23,7 @@ import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
import com.google.gerrit.server.git.IntegrationException;
import com.google.gerrit.server.git.MergeOp.CommitStatus;
import com.google.gerrit.server.git.MergeTip;
import com.google.gerrit.server.git.SubmoduleOp;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -52,11 +53,12 @@ public class SubmitStrategyFactory {
Repository repo, CodeReviewRevWalk rw, ObjectInserter inserter,
RevFlag canMergeFlag, Set<RevCommit> alreadyAccepted,
Branch.NameKey destBranch, IdentifiedUser caller, MergeTip mergeTip,
CommitStatus commits, String submissionId, NotifyHandling notifyHandling)
CommitStatus commits, String submissionId, NotifyHandling notifyHandling,
SubmoduleOp submoduleOp)
throws IntegrationException {
SubmitStrategy.Arguments args = argsFactory.create(submitType, destBranch,
commits, rw, caller, mergeTip, inserter, repo, canMergeFlag, db,
alreadyAccepted, submissionId, notifyHandling);
alreadyAccepted, submissionId, notifyHandling, submoduleOp);
switch (submitType) {
case CHERRY_PICK:
return new CherryPick(args);

30
gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java
View File

@@ -46,6 +46,7 @@ import com.google.gerrit.server.git.IntegrationException;
import com.google.gerrit.server.git.LabelNormalizer;
import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.git.SubmoduleException;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.project.ProjectState;
import com.google.gwtorm.server.OrmException;
@@ -136,6 +137,7 @@ abstract class SubmitStrategyOp extends BatchUpdate.Op {
tipAfter,
getDest().get());
ctx.addRefUpdate(command);
args.submoduleOp.addBranchTip(getDest(), tipAfter);
}
private void checkProjectConfig(RepoContext ctx, CodeReviewCommit commit)
@@ -556,6 +558,34 @@ abstract class SubmitStrategyOp extends BatchUpdate.Op {
protected void postUpdateImpl(Context ctx) throws Exception {
}
/**
* Amend the commit with gitlink update
* @param commit
*/
protected CodeReviewCommit amendGitlink(CodeReviewCommit commit)
throws IntegrationException {
CodeReviewCommit newCommit = commit;
// Modify the commit with gitlink update
if (args.submoduleOp.hasSubscription(args.destBranch)) {
try {
newCommit =
args.submoduleOp.composeGitlinksCommit(args.destBranch, commit);
newCommit.copyFrom(commit);
if (commit.equals(toMerge)) {
newCommit.setPatchsetId(ChangeUtil.nextPatchSetId(
args.repo, toMerge.change().currentPatchSetId()));
args.commits.put(newCommit);
}
} catch (SubmoduleException | IOException e) {
throw new IntegrationException(
"cannot update gitlink for the commit at branch: "
+ args.destBranch);
}
}
return newCommit;
}
protected final void logDebug(String msg, Object... args) {
if (log.isDebugEnabled()) {
log.debug("[" + this.args.submissionId + "]" + msg, args);