diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java index 374cde3139..262e7bd4bb 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java @@ -113,87 +113,89 @@ public class CherryPickChange { Project.NameKey project = change.getProject(); IdentifiedUser identifiedUser = (IdentifiedUser) currentUser.get(); - final Repository git; + Repository git = null; + ObjectInserter oi = null; + RevWalk revWalk = null; + try { git = gitManager.openRepository(project); + oi = git.newObjectInserter(); + revWalk = new RevWalk(oi.newReader()); + + Ref destRef = git.getRef(destinationBranch); + if (destRef == null) { + throw new InvalidChangeOperationException("Branch " + + destinationBranch + " does not exist."); + } + + final RevCommit mergeTip = revWalk.parseCommit(destRef.getObjectId()); + + RevCommit commitToCherryPick = + revWalk.parseCommit(ObjectId.fromString(patch.getRevision().get())); + + PersonIdent committerIdent = + identifiedUser.newCommitterIdent(TimeUtil.nowTs(), + serverTimeZone); + + final ObjectId computedChangeId = + ChangeIdUtil + .computeChangeId(commitToCherryPick.getTree(), mergeTip, + commitToCherryPick.getAuthorIdent(), committerIdent, message); + String commitMessage = + ChangeIdUtil.insertId(message, computedChangeId).trim() + '\n'; + + RevCommit cherryPickCommit; + ProjectState projectState = refControl.getProjectControl().getProjectState(); + cherryPickCommit = + mergeUtilFactory.create(projectState).createCherryPickFromCommit(git, oi, mergeTip, + commitToCherryPick, committerIdent, commitMessage, revWalk); + oi.flush(); + + Change.Key changeKey; + final List idList = cherryPickCommit.getFooterLines( + FooterConstants.CHANGE_ID); + if (!idList.isEmpty()) { + final String idStr = idList.get(idList.size() - 1).trim(); + changeKey = new Change.Key(idStr); + } else { + changeKey = new Change.Key("I" + computedChangeId.name()); + } + + Branch.NameKey newDest = + new Branch.NameKey(change.getProject(), destRef.getName()); + List destChanges = queryProvider.get() + .setLimit(2) + .byBranchKey(newDest, changeKey); + if (destChanges.size() > 1) { + throw new InvalidChangeOperationException("Several changes with key " + + changeKey + " reside on the same branch. " + + "Cannot create a new patch set."); + } else if (destChanges.size() == 1) { + // The change key exists on the destination branch. The cherry pick + // will be added as a new patch set. + return insertPatchSet(git, revWalk, destChanges.get(0).change(), + cherryPickCommit, refControl, identifiedUser); + } else { + // Change key not found on destination branch. We can create a new + // change. + return createNewChange(git, revWalk, changeKey, project, + patch.getId(), destRef, cherryPickCommit, refControl, + identifiedUser, change.getTopic()); + } + } catch (MergeIdenticalTreeException | MergeConflictException e) { + throw new MergeException("Cherry pick failed: " + e.getMessage()); } catch (RepositoryNotFoundException e) { throw new NoSuchChangeException(change.getId(), e); - } - - try { - RevWalk revWalk = new RevWalk(git); - try { - Ref destRef = git.getRef(destinationBranch); - if (destRef == null) { - throw new InvalidChangeOperationException("Branch " - + destinationBranch + " does not exist."); - } - - final RevCommit mergeTip = revWalk.parseCommit(destRef.getObjectId()); - - RevCommit commitToCherryPick = - revWalk.parseCommit(ObjectId.fromString(patch.getRevision().get())); - - PersonIdent committerIdent = - identifiedUser.newCommitterIdent(TimeUtil.nowTs(), - serverTimeZone); - - final ObjectId computedChangeId = - ChangeIdUtil - .computeChangeId(commitToCherryPick.getTree(), mergeTip, - commitToCherryPick.getAuthorIdent(), committerIdent, message); - String commitMessage = - ChangeIdUtil.insertId(message, computedChangeId).trim() + '\n'; - - RevCommit cherryPickCommit; - ObjectInserter oi = git.newObjectInserter(); - try { - ProjectState projectState = refControl.getProjectControl().getProjectState(); - cherryPickCommit = - mergeUtilFactory.create(projectState).createCherryPickFromCommit(git, oi, mergeTip, - commitToCherryPick, committerIdent, commitMessage, revWalk); - } catch (MergeIdenticalTreeException | MergeConflictException e) { - throw new MergeException("Cherry pick failed: " + e.getMessage()); - } finally { - oi.release(); - } - - Change.Key changeKey; - final List idList = cherryPickCommit.getFooterLines( - FooterConstants.CHANGE_ID); - if (!idList.isEmpty()) { - final String idStr = idList.get(idList.size() - 1).trim(); - changeKey = new Change.Key(idStr); - } else { - changeKey = new Change.Key("I" + computedChangeId.name()); - } - - Branch.NameKey newDest = - new Branch.NameKey(change.getProject(), destRef.getName()); - List destChanges = queryProvider.get() - .setLimit(2) - .byBranchKey(newDest, changeKey); - if (destChanges.size() > 1) { - throw new InvalidChangeOperationException("Several changes with key " - + changeKey + " reside on the same branch. " - + "Cannot create a new patch set."); - } else if (destChanges.size() == 1) { - // The change key exists on the destination branch. The cherry pick - // will be added as a new patch set. - return insertPatchSet(git, revWalk, destChanges.get(0).change(), - cherryPickCommit, refControl, identifiedUser); - } else { - // Change key not found on destination branch. We can create a new - // change. - return createNewChange(git, revWalk, changeKey, project, - patch.getId(), destRef, cherryPickCommit, refControl, - identifiedUser, change.getTopic()); - } - } finally { + } finally { + if (revWalk != null) { revWalk.release(); } - } finally { - git.close(); + if (oi != null) { + oi.release(); + } + if (git != null) { + git.close(); + } } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/CodeReviewCommit.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/CodeReviewCommit.java index fac0f7f895..1ff37b0848 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/CodeReviewCommit.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/CodeReviewCommit.java @@ -23,6 +23,7 @@ import com.google.gerrit.server.project.ChangeControl; import org.eclipse.jgit.lib.AnyObjectId; import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevWalk; @@ -50,12 +51,11 @@ public class CodeReviewCommit extends RevCommit { }).nullsFirst(); public static RevWalk newRevWalk(Repository repo) { - return new RevWalk(repo) { - @Override - protected RevCommit createCommit(AnyObjectId id) { - return new CodeReviewCommit(id); - } - }; + return new CodeReviewRevWalk(repo); + } + + public static RevWalk newRevWalk(ObjectReader reader) { + return new CodeReviewRevWalk(reader); } static CodeReviewCommit revisionGone(ChangeControl ctl) { @@ -85,6 +85,21 @@ public class CodeReviewCommit extends RevCommit { return r; } + private static class CodeReviewRevWalk extends RevWalk { + private CodeReviewRevWalk(Repository repo) { + super(repo); + } + + private CodeReviewRevWalk(ObjectReader reader) { + super(reader); + } + + @Override + protected RevCommit createCommit(AnyObjectId id) { + return new CodeReviewCommit(id); + } + } + /** * Unique key of the PatchSet entity from the code review system. *

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java index 106c1e3b8c..8ae3d2babf 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java @@ -430,13 +430,11 @@ public class MergeOp { String m = "Error opening repository \"" + name.get() + '"'; throw new MergeException(m, err); } - - rw = CodeReviewCommit.newRevWalk(repo); + inserter = repo.newObjectInserter(); + rw = CodeReviewCommit.newRevWalk(inserter.newReader()); rw.sort(RevSort.TOPO); rw.sort(RevSort.COMMIT_TIME_DESC, true); canMergeFlag = rw.newFlag("CAN_MERGE"); - - inserter = repo.newObjectInserter(); } private RefUpdate openBranch() @@ -674,6 +672,11 @@ public class MergeOp { + destProject.getProject().getName(), e); } } + try { + inserter.flush(); + } catch (IOException e) { + throw new MergeException("Cannot flush merge results", e); + } branchUpdate.setRefLogIdent(refLogIdent); branchUpdate.setForceUpdate(false); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java index 295ad5244d..c798f4d876 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java @@ -43,7 +43,6 @@ import org.eclipse.jgit.errors.LargeObjectException; import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.errors.NoMergeBaseException; import org.eclipse.jgit.errors.NoMergeBaseException.MergeBaseFailureReason; -import org.eclipse.jgit.lib.AnyObjectId; import org.eclipse.jgit.lib.CommitBuilder; import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Constants; @@ -67,7 +66,6 @@ import org.slf4j.LoggerFactory; import java.io.IOException; import java.io.InputStream; -import java.io.UnsupportedEncodingException; import java.sql.Timestamp; import java.util.ArrayList; import java.util.Collection; @@ -79,6 +77,12 @@ import java.util.List; import java.util.Set; import java.util.TimeZone; +/** + * Utilities for various kinds of merges and cherry-picks. + *

+ * Note: Unless otherwise noted, the methods in this class do not flush + * the {@link ObjectInserter}s passed in after performing a merge. + */ public class MergeUtil { private static final Logger log = LoggerFactory.getLogger(MergeUtil.class); private static final String R_HEADS_MASTER = @@ -177,7 +181,7 @@ public class MergeUtil { final ThreeWayMerger m = newThreeWayMerger(repo, inserter); m.setBase(originalCommit.getParent(0)); - if (m.merge(mergeTip, originalCommit)) { + if (m.merge(false, mergeTip, originalCommit)) { ObjectId tree = m.getResultTreeId(); if (tree.equals(mergeTip.getTree())) { throw new MergeIdenticalTreeException("identical tree"); @@ -189,7 +193,7 @@ public class MergeUtil { mergeCommit.setAuthor(originalCommit.getAuthorIdent()); mergeCommit.setCommitter(cherryPickCommitterIdent); mergeCommit.setMessage(commitMsg); - return rw.parseCommit(commit(inserter, mergeCommit)); + return rw.parseCommit(inserter.insert(mergeCommit)); } else { throw new MergeConflictException("merge conflict"); } @@ -393,7 +397,7 @@ public class MergeUtil { ThreeWayMerger m = newThreeWayMerger(repo, createDryRunInserter(repo)); try { - return m.merge(new AnyObjectId[] {mergeTip, toMerge}); + return m.merge(false, mergeTip, toMerge); } catch (LargeObjectException e) { log.warn("Cannot merge due to LargeObjectException: " + toMerge.name()); return false; @@ -442,7 +446,7 @@ public class MergeUtil { try { ThreeWayMerger m = newThreeWayMerger(repo, createDryRunInserter(repo)); m.setBase(toMerge.getParent(0)); - return m.merge(mergeTip, toMerge); + return m.merge(false, mergeTip, toMerge); } catch (IOException e) { throw new MergeException("Cannot merge " + toMerge.name(), e); } @@ -494,7 +498,7 @@ public class MergeUtil { throws MergeException { final ThreeWayMerger m = newThreeWayMerger(repo, inserter); try { - if (m.merge(new AnyObjectId[] {mergeTip, n})) { + if (m.merge(false, mergeTip, n)) { return writeMergeCommit(myIdent, rw, inserter, canMergeFlag, destBranch, mergeTip, m.getResultTreeId(), n); } else { @@ -582,7 +586,7 @@ public class MergeUtil { mergeCommit.setMessage(msgbuf.toString()); CodeReviewCommit mergeResult = - (CodeReviewCommit) rw.parseCommit(commit(inserter, mergeCommit)); + (CodeReviewCommit) rw.parseCommit(inserter.insert(mergeCommit)); mergeResult.setControl(n.getControl()); return mergeResult; } @@ -656,31 +660,10 @@ public class MergeUtil { Merger m = strategy.newMerger(repo, true); checkArgument(m instanceof ThreeWayMerger, "merge strategy %s does not support three-way merging", strategyName); - m.setObjectInserter(new ObjectInserter.Filter() { - @Override - protected ObjectInserter delegate() { - return inserter; - } - - @Override - public void flush() { - } - - @Override - public void release() { - } - }); + m.setObjectInserter(inserter); return (ThreeWayMerger) m; } - public ObjectId commit(final ObjectInserter inserter, - final CommitBuilder mergeCommit) throws IOException, - UnsupportedEncodingException { - ObjectId id = inserter.insert(mergeCommit); - inserter.flush(); - return id; - } - public PatchSetApproval markCleanMerges(final RevWalk rw, final RevFlag canMergeFlag, final CodeReviewCommit mergeTip, final Set alreadyAccepted) throws MergeException { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java index 0c8c5ecac1..50a8117911 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java @@ -275,24 +275,11 @@ public class PatchListLoader extends CacheLoader { try { DirCache dc = DirCache.newInCore(); m.setDirCache(dc); - m.setObjectInserter(new ObjectInserter.Filter() { - @Override - protected ObjectInserter delegate() { - return ins; - } - - @Override - public void flush() { - } - - @Override - public void release() { - } - }); + m.setObjectInserter(ins); boolean couldMerge; try { - couldMerge = m.merge(b.getParents()); + couldMerge = m.merge(false, b.getParents()); } catch (IOException e) { // It is not safe to continue further down in this method as throwing // an exception most likely means that the merge tree was not created