diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java index 6e5f129999..b6539f1331 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java @@ -36,6 +36,7 @@ import com.google.gerrit.server.cache.h2.DefaultCacheFactory; import com.google.gerrit.server.change.ChangeKindCacheImpl; import com.google.gerrit.server.change.MergeabilityCacheImpl; import com.google.gerrit.server.change.PatchSetInserter; +import com.google.gerrit.server.change.RebaseChangeOp; import com.google.gerrit.server.config.CanonicalWebUrl; import com.google.gerrit.server.config.CanonicalWebUrlProvider; import com.google.gerrit.server.config.DisableReverseDnsLookup; @@ -110,6 +111,7 @@ public class BatchProgramModule extends FactoryModule { factory(BatchUpdate.Factory.class); factory(MergeUtil.Factory.class); factory(PatchSetInserter.Factory.class); + factory(RebaseChangeOp.Factory.class); bind(new TypeLiteral>() {}) .annotatedWith(GitUploadPackGroups.class) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java index b22cb7d35d..29794c00d6 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java @@ -50,7 +50,7 @@ import com.google.gerrit.server.change.Mergeable; import com.google.gerrit.server.change.PostReview; import com.google.gerrit.server.change.PublishDraftPatchSet; import com.google.gerrit.server.change.Rebase; -import com.google.gerrit.server.change.RebaseChange; +import com.google.gerrit.server.change.RebaseUtil; import com.google.gerrit.server.change.Reviewed; import com.google.gerrit.server.change.RevisionResource; import com.google.gerrit.server.change.Submit; @@ -74,7 +74,7 @@ class RevisionApiImpl implements RevisionApi { private final CherryPick cherryPick; private final DeleteDraftPatchSet deleteDraft; private final Rebase rebase; - private final RebaseChange rebaseChange; + private final RebaseUtil rebaseUtil; private final Submit submit; private final PublishDraftPatchSet publish; private final Reviewed.PutReviewed putReviewed; @@ -100,7 +100,7 @@ class RevisionApiImpl implements RevisionApi { CherryPick cherryPick, DeleteDraftPatchSet deleteDraft, Rebase rebase, - RebaseChange rebaseChange, + RebaseUtil rebaseUtil, Submit submit, PublishDraftPatchSet publish, Reviewed.PutReviewed putReviewed, @@ -124,7 +124,7 @@ class RevisionApiImpl implements RevisionApi { this.cherryPick = cherryPick; this.deleteDraft = deleteDraft; this.rebase = rebase; - this.rebaseChange = rebaseChange; + this.rebaseUtil = rebaseUtil; this.review = review; this.submit = submit; this.publish = publish; @@ -205,7 +205,7 @@ class RevisionApiImpl implements RevisionApi { @Override public boolean canRebase() { - return rebaseChange.canRebase(revision); + return rebaseUtil.canRebase(revision); } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java index 74f80b8b5c..9c588ed769 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java @@ -125,6 +125,7 @@ public class Module extends RestApiModule { factory(ChangeInserter.Factory.class); factory(EmailReviewComments.Factory.class); factory(PatchSetInserter.Factory.class); + factory(RebaseChangeOp.Factory.class); factory(ReviewerResource.Factory.class); factory(SetHashtagsOp.Factory.class); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java index b92c361440..68f14973b3 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java @@ -187,6 +187,11 @@ public class PatchSetInserter extends BatchUpdate.Op { return change; } + public PatchSet getPatchSet() { + checkState(patchSet != null, "getPatchSet() only valid after executing update"); + return patchSet; + } + @Override public void updateRepo(RepoContext ctx) throws InvalidChangeOperationException, IOException { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Rebase.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Rebase.java index ca38e35504..60f285f18a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Rebase.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Rebase.java @@ -15,34 +15,34 @@ package com.google.gerrit.server.change; import com.google.common.primitives.Ints; +import com.google.gerrit.common.TimeUtil; import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.extensions.api.changes.RebaseInput; import com.google.gerrit.extensions.client.ListChangesOption; import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.ResourceConflictException; -import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.extensions.webui.UiAction; +import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change.Status; import com.google.gerrit.reviewdb.client.PatchSet; -import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.RevId; import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.git.BatchUpdate; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.UpdateException; +import com.google.gerrit.server.git.validators.CommitValidators; import com.google.gerrit.server.project.ChangeControl; -import com.google.gerrit.server.project.InvalidChangeOperationException; -import com.google.gerrit.server.project.NoSuchChangeException; -import com.google.gerrit.server.project.NoSuchProjectException; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.Singleton; import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.ObjectInserter; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevWalk; @@ -60,18 +60,21 @@ public class Rebase implements RestModifyView, ListChangesOption.CURRENT_REVISION, ListChangesOption.CURRENT_COMMIT); + private final BatchUpdate.Factory updateFactory; private final GitRepositoryManager repoManager; - private final Provider rebaseChange; + private final RebaseChangeOp.Factory rebaseFactory; private final ChangeJson.Factory json; private final Provider dbProvider; @Inject - public Rebase(GitRepositoryManager repoManager, - Provider rebaseChange, + public Rebase(BatchUpdate.Factory updateFactory, + GitRepositoryManager repoManager, + RebaseChangeOp.Factory rebaseFactory, ChangeJson.Factory json, Provider dbProvider) { + this.updateFactory = updateFactory; this.repoManager = repoManager; - this.rebaseChange = rebaseChange; + this.rebaseFactory = rebaseFactory; this.json = json; this.dbProvider = dbProvider; } @@ -83,7 +86,10 @@ public class Rebase implements RestModifyView, ChangeControl control = rsrc.getControl(); Change change = rsrc.getChange(); try (Repository repo = repoManager.openRepository(change.getProject()); - RevWalk rw = new RevWalk(repo)) { + RevWalk rw = new RevWalk(repo); + ObjectInserter oi = repo.newObjectInserter(); + BatchUpdate bu = updateFactory.create(dbProvider.get(), + change.getProject(), rsrc.getUser(), TimeUtil.nowTs())) { if (!control.canRebase()) { throw new AuthException("rebase not permitted"); } else if (!change.getStatus().isOpen()) { @@ -93,13 +99,15 @@ public class Rebase implements RestModifyView, throw new ResourceConflictException( "cannot rebase merge commits or commit with no ancestor"); } - rebaseChange.get().rebase(repo, rw, rsrc, findBaseRev(rw, rsrc, input)); - } catch (InvalidChangeOperationException e) { - throw new ResourceConflictException(e.getMessage()); - } catch (NoSuchChangeException | NoSuchProjectException e) { - throw new ResourceNotFoundException(change.getId().toString()); + bu.setRepository(repo, rw, oi); + bu.addOp(change.getId(), rebaseFactory.create( + control, rsrc.getPatchSet(), + findBaseRev(rw, rsrc, input)) + .setForceContentMerge(true) + .setRunHooks(true) + .setValidatePolicy(CommitValidators.Policy.GERRIT)); + bu.execute(); } - return json.create(OPTIONS).format(change.getId()); } @@ -199,29 +207,30 @@ public class Rebase implements RestModifyView, @Override public UiAction.Description getDescription(RevisionResource resource) { - Project.NameKey project = resource.getChange().getProject(); + PatchSet patchSet = resource.getPatchSet(); + Branch.NameKey dest = resource.getChange().getDest(); boolean visible = resource.getChange().getStatus().isOpen() && resource.isCurrent() && resource.getControl().canRebase(); + boolean enabled = true; + if (visible) { - try (Repository repo = repoManager.openRepository(project); + try (Repository repo = repoManager.openRepository(dest.getParentKey()); RevWalk rw = new RevWalk(repo)) { visible = hasOneParent(rw, resource.getPatchSet()); + enabled = + RebaseUtil.canRebase(patchSet, dest, repo, rw, dbProvider.get()); } catch (IOException e) { - log.error("Failed to get ancestors of patch set " - + resource.getPatchSet().getId(), e); + log.error("Failed to check if patch set can be rebased: " + + resource.getPatchSet(), e); visible = false; } } UiAction.Description descr = new UiAction.Description() .setLabel("Rebase") .setTitle("Rebase onto tip of branch or parent change") - .setVisible(visible); - if (descr.isVisible()) { - // Disable the rebase button in the RebaseDialog if - // the change cannot be rebased. - descr.setEnabled(rebaseChange.get().canRebase(resource)); - } + .setVisible(visible) + .setEnabled(enabled); return descr; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChange.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChange.java deleted file mode 100644 index d7b637474c..0000000000 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChange.java +++ /dev/null @@ -1,373 +0,0 @@ -// 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.change; - -import com.google.gerrit.common.TimeUtil; -import com.google.gerrit.common.errors.EmailException; -import com.google.gerrit.extensions.restapi.ResourceConflictException; -import com.google.gerrit.extensions.restapi.RestApiException; -import com.google.gerrit.reviewdb.client.Branch; -import com.google.gerrit.reviewdb.client.Change; -import com.google.gerrit.reviewdb.client.Change.Status; -import com.google.gerrit.reviewdb.client.PatchSet; -import com.google.gerrit.reviewdb.client.RevId; -import com.google.gerrit.reviewdb.server.ReviewDb; -import com.google.gerrit.server.ChangeUtil; -import com.google.gerrit.server.GerritPersonIdent; -import com.google.gerrit.server.IdentifiedUser; -import com.google.gerrit.server.git.BatchUpdate; -import com.google.gerrit.server.git.GitRepositoryManager; -import com.google.gerrit.server.git.MergeConflictException; -import com.google.gerrit.server.git.MergeUtil; -import com.google.gerrit.server.git.UpdateException; -import com.google.gerrit.server.git.validators.CommitValidators; -import com.google.gerrit.server.project.InvalidChangeOperationException; -import com.google.gerrit.server.project.NoSuchChangeException; -import com.google.gerrit.server.project.NoSuchProjectException; -import com.google.gerrit.server.project.ProjectControl; -import com.google.gerrit.server.project.RefControl; -import com.google.gwtorm.server.OrmException; -import com.google.inject.Inject; -import com.google.inject.Provider; -import com.google.inject.Singleton; - -import org.eclipse.jgit.lib.CommitBuilder; -import org.eclipse.jgit.lib.ObjectId; -import org.eclipse.jgit.lib.ObjectInserter; -import org.eclipse.jgit.lib.PersonIdent; -import org.eclipse.jgit.lib.Ref; -import org.eclipse.jgit.lib.Repository; -import org.eclipse.jgit.merge.ThreeWayMerger; -import org.eclipse.jgit.revwalk.RevCommit; -import org.eclipse.jgit.revwalk.RevWalk; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import java.io.IOException; -import java.util.TimeZone; - -@Singleton -public class RebaseChange { - private static final Logger log = LoggerFactory.getLogger(RebaseChange.class); - - private final ProjectControl.GenericFactory projectControlFactory; - private final Provider db; - private final GitRepositoryManager gitManager; - private final TimeZone serverTimeZone; - private final MergeUtil.Factory mergeUtilFactory; - private final PatchSetInserter.Factory patchSetInserterFactory; - private final BatchUpdate.Factory updateFactory; - - @Inject - RebaseChange(ProjectControl.GenericFactory projectControlFactory, - Provider db, - @GerritPersonIdent PersonIdent myIdent, - GitRepositoryManager gitManager, - MergeUtil.Factory mergeUtilFactory, - PatchSetInserter.Factory patchSetInserterFactory, - BatchUpdate.Factory updateFactory) { - this.projectControlFactory = projectControlFactory; - this.db = db; - this.gitManager = gitManager; - this.serverTimeZone = myIdent.getTimeZone(); - this.mergeUtilFactory = mergeUtilFactory; - this.patchSetInserterFactory = patchSetInserterFactory; - this.updateFactory = updateFactory; - } - - /** - * Rebase the change of the given patch set. - *

- * If the patch set has no dependency to an open change, then the change is - * rebased on the tip of the destination branch. - *

- * If the patch set depends on an open change, it is rebased on the latest - * patch set of this change. - *

- * The rebased commit is added as new patch set to the change. - *

- * E-mail notification and triggering of hooks happens for the creation of the - * new patch set. - * - * @param git the repository. - * @param rw the RevWalk. - * @param rsrc revision to rebase. - * @param newBaseRev the commit that should be the new base. - * @throws NoSuchChangeException if the change to which the patch set belongs - * does not exist or is not visible to the user. - * @throws NoSuchProjectException if the project to which the patch set - * belongs does not exist or is not visible to the user. - * @throws EmailException if sending the e-mail to notify about the new patch - * set fails. - * @throws OrmException if accessing the database fails. - * @throws IOException if accessing the repository fails. - * @throws InvalidChangeOperationException if rebase is not possible or not - * allowed. - * @throws RestApiException if updating the change fails due to an underlying - * API call failing. - * @throws UpdateException if updating the change fails. - */ - public void rebase(Repository git, RevWalk rw, RevisionResource rsrc, - String newBaseRev) throws NoSuchChangeException, NoSuchProjectException, - EmailException, OrmException, IOException, InvalidChangeOperationException, - UpdateException, RestApiException { - Change change = rsrc.getChange(); - PatchSet patchSet = rsrc.getPatchSet(); - IdentifiedUser uploader = rsrc.getControl().getUser().asIdentifiedUser(); - - try (ObjectInserter inserter = git.newObjectInserter()) { - String baseRev = newBaseRev; - if (baseRev == null) { - baseRev = findBaseRevision(patchSet, change.getDest(), git, rw); - } - - ObjectId baseObjectId = git.resolve(baseRev); - if (baseObjectId == null) { - throw new InvalidChangeOperationException( - "Cannot rebase: Failed to resolve baseRev: " + baseRev); - } - - RevCommit baseCommit = rw.parseCommit(baseObjectId); - PersonIdent committerIdent = - uploader.newCommitterIdent(TimeUtil.nowTs(), serverTimeZone); - - rebase(git, rw, inserter, change, patchSet.getId(), - uploader, baseCommit, mergeUtilFactory.create( - rsrc.getControl().getProjectControl().getProjectState(), true), - committerIdent, true, CommitValidators.Policy.GERRIT); - } catch (MergeConflictException e) { - throw new ResourceConflictException(e.getMessage()); - } - } - - /** - * Find the commit onto which a patch set should be rebased. - *

- * This is defined as the latest patch set of the change corresponding to - * this commit's parent, or the destination branch tip in the case where the - * parent's change is merged. - * - * @param patchSet patch set for which the new base commit should be found. - * @param destBranch the destination branch. - * @param git the repository. - * @param rw the RevWalk. - * @return the commit onto which the patch set should be rebased. - * @throws InvalidChangeOperationException if rebase is not possible or not - * allowed. - * @throws IOException if accessing the repository fails. - * @throws OrmException if accessing the database fails. - */ - private String findBaseRevision(PatchSet patchSet, - Branch.NameKey destBranch, Repository git, RevWalk rw) - throws InvalidChangeOperationException, IOException, OrmException { - String baseRev = null; - RevCommit commit = rw.parseCommit( - ObjectId.fromString(patchSet.getRevision().get())); - - if (commit.getParentCount() > 1) { - throw new InvalidChangeOperationException( - "Cannot rebase a change with multiple parents."); - } else if (commit.getParentCount() == 0) { - throw new InvalidChangeOperationException( - "Cannot rebase a change without any parents" - + " (is this the initial commit?)."); - } - - RevId parentRev = new RevId(commit.getParent(0).name()); - - for (PatchSet depPatchSet : db.get().patchSets().byRevision(parentRev)) { - Change.Id depChangeId = depPatchSet.getId().getParentKey(); - Change depChange = db.get().changes().get(depChangeId); - if (!depChange.getDest().equals(destBranch)) { - continue; - } - - if (depChange.getStatus() == Status.ABANDONED) { - throw new InvalidChangeOperationException( - "Cannot rebase a change with an abandoned parent: " - + depChange.getKey()); - } - - if (depChange.getStatus().isOpen()) { - if (depPatchSet.getId().equals(depChange.currentPatchSetId())) { - throw new InvalidChangeOperationException( - "Change is already based on the latest patch set of the" - + " dependent change."); - } - PatchSet latestDepPatchSet = - db.get().patchSets().get(depChange.currentPatchSetId()); - baseRev = latestDepPatchSet.getRevision().get(); - } - break; - } - - if (baseRev == null) { - // We are dependent on a merged PatchSet or have no PatchSet - // dependencies at all. - Ref destRef = git.getRefDatabase().exactRef(destBranch.get()); - if (destRef == null) { - throw new InvalidChangeOperationException( - "The destination branch does not exist: " + destBranch.get()); - } - baseRev = destRef.getObjectId().getName(); - if (baseRev.equals(parentRev.get())) { - throw new InvalidChangeOperationException( - "Change is already up to date."); - } - } - return baseRev; - } - - /** - * Rebase the change of the given patch set on the given base commit. - *

- * The rebased commit is added as new patch set to the change. - *

- * E-mail notification and triggering of hooks is only done for the creation - * of the new patch set if {@code sendEmail} and {@code runHooks} are true, - * respectively. - * - * @param git the repository. - * @param inserter the object inserter. - * @param change the change to rebase. - * @param patchSetId the patch set ID to rebase. - * @param uploader the user that creates the rebased patch set. - * @param baseCommit the commit that should be the new base. - * @param mergeUtil merge utilities for the destination project. - * @param committerIdent the committer's identity. - * @param runHooks if hooks should be run for the new patch set. - * @param validate if commit validation should be run for the new patch set. - * @param rw the RevWalk. - * @return the new patch set, which is based on the given base commit. - * @throws NoSuchProjectException if the project to which the patch set - * belongs does not exist or is not visible to the user. - * @throws OrmException if accessing the database fails. - * @throws IOException if rebase is not possible. - * @throws InvalidChangeOperationException if rebase is not possible or not - * allowed. - * @throws RestApiException if updating the change fails due to an underlying - * API call failing. - * @throws UpdateException if updating the change fails. - */ - public PatchSet rebase(Repository git, RevWalk rw, - ObjectInserter inserter, Change change, PatchSet.Id patchSetId, - IdentifiedUser uploader, RevCommit baseCommit, MergeUtil mergeUtil, - PersonIdent committerIdent, boolean runHooks, - CommitValidators.Policy validate) - throws NoSuchProjectException, OrmException, IOException, - InvalidChangeOperationException, MergeConflictException, UpdateException, - RestApiException { - if (!change.currentPatchSetId().equals(patchSetId)) { - throw new InvalidChangeOperationException("patch set is not current"); - } - PatchSet originalPatchSet = db.get().patchSets().get(patchSetId); - - RevCommit rebasedCommit; - ObjectId oldId = ObjectId.fromString(originalPatchSet.getRevision().get()); - RevCommit original = rw.parseCommit(oldId); - rw.parseBody(original); - ObjectId newId = rebaseCommit( - git, inserter, original, baseCommit, mergeUtil, committerIdent); - - rebasedCommit = rw.parseCommit(newId); - - RefControl ctl = projectControlFactory - .controlFor(change.getProject(), uploader) - .controlForRef(change.getDest()); - - PatchSet.Id psId = - ChangeUtil.nextPatchSetId(git, change.currentPatchSetId()); - PatchSetInserter patchSetInserter = patchSetInserterFactory - .create(ctl, psId, rebasedCommit) - .setValidatePolicy(validate) - .setDraft(originalPatchSet.isDraft()) - .setUploader(uploader.getAccountId()) - .setSendMail(false) - .setRunHooks(runHooks); - - try (BatchUpdate bu = updateFactory.create( - db.get(), change.getProject(), uploader, TimeUtil.nowTs())) { - bu.addOp(change.getId(), patchSetInserter.setMessage( - "Patch Set " + patchSetInserter.getPatchSetId().get() - + ": Patch Set " + patchSetId.get() + " was rebased")); - bu.execute(); - } - - return db.get().patchSets().get(patchSetInserter.getPatchSetId()); - } - - /** - * Rebase a commit. - * - * @param git repository to find commits in. - * @param inserter inserter to handle new trees and blobs. - * @param original the commit to rebase. - * @param base base to rebase against. - * @param mergeUtil merge utilities for the destination project. - * @param committerIdent committer identity. - * @return the id of the rebased commit. - * @throws MergeConflictException the rebase failed due to a merge conflict. - * @throws IOException the merge failed for another reason. - */ - private ObjectId rebaseCommit(Repository git, ObjectInserter inserter, - RevCommit original, RevCommit base, MergeUtil mergeUtil, - PersonIdent committerIdent) throws MergeConflictException, IOException, - InvalidChangeOperationException { - RevCommit parentCommit = original.getParent(0); - - if (base.equals(parentCommit)) { - throw new InvalidChangeOperationException( - "Change is already up to date."); - } - - ThreeWayMerger merger = mergeUtil.newThreeWayMerger(git, inserter); - merger.setBase(parentCommit); - merger.merge(original, base); - - if (merger.getResultTreeId() == null) { - throw new MergeConflictException( - "The change could not be rebased due to a conflict during merge."); - } - - CommitBuilder cb = new CommitBuilder(); - cb.setTreeId(merger.getResultTreeId()); - cb.setParentId(base); - cb.setAuthor(original.getAuthorIdent()); - cb.setMessage(original.getFullMessage()); - cb.setCommitter(committerIdent); - ObjectId objectId = inserter.insert(cb); - inserter.flush(); - return objectId; - } - - public boolean canRebase(RevisionResource r) { - return canRebase(r.getPatchSet(), r.getChange().getDest()); - } - - private boolean canRebase(PatchSet patchSet, Branch.NameKey dest) { - try (Repository git = gitManager.openRepository(dest.getParentKey()); - RevWalk rw = new RevWalk(git)) { - findBaseRevision(patchSet, dest, git, rw); - return true; - } catch (InvalidChangeOperationException e) { - return false; - } catch (OrmException | IOException e) { - log.warn(String.format( - "Error checking if patch set %s on %s can be rebased", - patchSet.getId(), dest), e); - return false; - } - } -} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeOp.java new file mode 100644 index 0000000000..6769a5c87e --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeOp.java @@ -0,0 +1,208 @@ +// Copyright (C) 2015 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.change; + +import static com.google.common.base.Preconditions.checkState; + +import com.google.gerrit.common.Nullable; +import com.google.gerrit.extensions.restapi.ResourceConflictException; +import com.google.gerrit.extensions.restapi.RestApiException; +import com.google.gerrit.reviewdb.client.PatchSet; +import com.google.gerrit.reviewdb.client.RevId; +import com.google.gerrit.server.ChangeUtil; +import com.google.gerrit.server.git.BatchUpdate; +import com.google.gerrit.server.git.BatchUpdate.ChangeContext; +import com.google.gerrit.server.git.BatchUpdate.RepoContext; +import com.google.gerrit.server.git.MergeConflictException; +import com.google.gerrit.server.git.MergeUtil; +import com.google.gerrit.server.git.validators.CommitValidators; +import com.google.gerrit.server.project.ChangeControl; +import com.google.gerrit.server.project.InvalidChangeOperationException; +import com.google.gerrit.server.project.ProjectState; +import com.google.gwtorm.server.OrmException; +import com.google.inject.assistedinject.Assisted; +import com.google.inject.assistedinject.AssistedInject; + +import org.eclipse.jgit.lib.CommitBuilder; +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.PersonIdent; +import org.eclipse.jgit.merge.ThreeWayMerger; +import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; + +import java.io.IOException; + +public class RebaseChangeOp extends BatchUpdate.Op { + public interface Factory { + RebaseChangeOp create(ChangeControl ctl, PatchSet originalPatchSet, + @Nullable String baseCommitish); + } + + private final PatchSetInserter.Factory patchSetInserterFactory; + private final MergeUtil.Factory mergeUtilFactory; + + private final ChangeControl ctl; + private final PatchSet originalPatchSet; + + private String baseCommitish; + private PersonIdent committerIdent; + private boolean runHooks = true; + private CommitValidators.Policy validate; + private boolean forceContentMerge; + + private RevCommit rebasedCommit; + private PatchSet.Id rebasedPatchSetId; + private PatchSetInserter patchSetInserter; + private PatchSet rebasedPatchSet; + + @AssistedInject + RebaseChangeOp( + PatchSetInserter.Factory patchSetInserterFactory, + MergeUtil.Factory mergeUtilFactory, + @Assisted ChangeControl ctl, + @Assisted PatchSet originalPatchSet, + @Assisted @Nullable String baseCommitish) { + this.patchSetInserterFactory = patchSetInserterFactory; + this.mergeUtilFactory = mergeUtilFactory; + this.ctl = ctl; + this.originalPatchSet = originalPatchSet; + this.baseCommitish = baseCommitish; + } + + public RebaseChangeOp setCommitterIdent(PersonIdent committerIdent) { + this.committerIdent = committerIdent; + return this; + } + + public RebaseChangeOp setValidatePolicy(CommitValidators.Policy validate) { + this.validate = validate; + return this; + } + + public RebaseChangeOp setRunHooks(boolean runHooks) { + this.runHooks = runHooks; + return this; + } + + public RebaseChangeOp setForceContentMerge(boolean forceContentMerge) { + this.forceContentMerge = forceContentMerge; + return this; + } + + @Override + public void updateRepo(RepoContext ctx) throws MergeConflictException, + InvalidChangeOperationException, RestApiException, IOException, + OrmException { + // Ok that originalPatchSet was not read in a transaction, since we just + // need its revision. + RevId oldRev = originalPatchSet.getRevision(); + + RevWalk rw = ctx.getRevWalk(); + RevCommit original = rw.parseCommit(ObjectId.fromString(oldRev.get())); + rw.parseBody(original); + RevCommit baseCommit; + if (baseCommitish != null) { + baseCommit = rw.parseCommit(ctx.getRepository().resolve(baseCommitish)); + } else { + baseCommit = rw.parseCommit(RebaseUtil.findBaseRevision( + originalPatchSet, ctl.getChange().getDest(), + ctx.getRepository(), ctx.getRevWalk(), ctx.getDb())); + } + + ObjectId newId = rebaseCommit(ctx, original, baseCommit); + rebasedCommit = rw.parseCommit(newId); + + rebasedPatchSetId = ChangeUtil.nextPatchSetId( + ctx.getRepository(), ctl.getChange().currentPatchSetId()); + patchSetInserter = patchSetInserterFactory + .create(ctl.getRefControl(), rebasedPatchSetId, rebasedCommit) + .setDraft(originalPatchSet.isDraft()) + .setUploader(ctx.getUser().getAccountId()) + .setSendMail(false) + .setRunHooks(runHooks) + .setMessage( + "Patch Set " + rebasedPatchSetId.get() + + ": Patch Set " + originalPatchSet.getId().get() + " was rebased"); + if (validate != null) { + patchSetInserter.setValidatePolicy(validate); + } + patchSetInserter.updateRepo(ctx); + } + + @Override + public void updateChange(ChangeContext ctx) + throws OrmException, InvalidChangeOperationException { + patchSetInserter.updateChange(ctx); + rebasedPatchSet = patchSetInserter.getPatchSet(); + } + + public PatchSet getPatchSet() { + checkState(rebasedPatchSet != null, + "getPatchSet() only valid after executing update"); + return rebasedPatchSet; + } + + private MergeUtil newMergeUtil() { + ProjectState project = ctl.getProjectControl().getProjectState(); + return forceContentMerge + ? mergeUtilFactory.create(project, true) + : mergeUtilFactory.create(project); + } + + /** + * Rebase a commit. + * + * @param ctx repo context. + * @param original the commit to rebase. + * @param base base to rebase against. + * @return the rebased commit. + * @throws MergeConflictException the rebase failed due to a merge conflict. + * @throws IOException the merge failed for another reason. + */ + private RevCommit rebaseCommit(RepoContext ctx, RevCommit original, + ObjectId base) throws ResourceConflictException, MergeConflictException, + IOException { + RevCommit parentCommit = original.getParent(0); + + if (base.equals(parentCommit)) { + throw new ResourceConflictException("Change is already up to date."); + } + + ThreeWayMerger merger = newMergeUtil().newThreeWayMerger( + ctx.getRepository(), ctx.getInserter()); + merger.setBase(parentCommit); + merger.merge(original, base); + + if (merger.getResultTreeId() == null) { + throw new MergeConflictException( + "The change could not be rebased due to a conflict during merge."); + } + + CommitBuilder cb = new CommitBuilder(); + cb.setTreeId(merger.getResultTreeId()); + cb.setParentId(base); + cb.setAuthor(original.getAuthorIdent()); + cb.setMessage(original.getFullMessage()); + if (committerIdent != null) { + cb.setCommitter(committerIdent); + } else { + cb.setCommitter(ctx.getUser().asIdentifiedUser() + .newRefLogIdent(ctx.getWhen(), ctx.getTimeZone())); + } + ObjectId objectId = ctx.getInserter().insert(cb); + ctx.getInserter().flush(); + return ctx.getRevWalk().parseCommit(objectId); + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseUtil.java new file mode 100644 index 0000000000..aea49f149b --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseUtil.java @@ -0,0 +1,160 @@ +// Copyright (C) 2015 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.change; + +import com.google.gerrit.extensions.restapi.ResourceConflictException; +import com.google.gerrit.extensions.restapi.RestApiException; +import com.google.gerrit.extensions.restapi.UnprocessableEntityException; +import com.google.gerrit.reviewdb.client.Branch; +import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.Change.Status; +import com.google.gerrit.reviewdb.client.PatchSet; +import com.google.gerrit.reviewdb.client.RevId; +import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.git.GitRepositoryManager; +import com.google.gwtorm.server.OrmException; +import com.google.inject.Inject; +import com.google.inject.Provider; + +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.Ref; +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.IOException; + +/** Utility methods related to rebasing changes. */ +public class RebaseUtil { + private static final Logger log = LoggerFactory.getLogger(RebaseUtil.class); + + private final Provider db; + private final GitRepositoryManager gitManager; + + @Inject + RebaseUtil(Provider db, + GitRepositoryManager gitManager) { + this.db = db; + this.gitManager = gitManager; + } + + public boolean canRebase(RevisionResource r) { + PatchSet patchSet = r.getPatchSet(); + Branch.NameKey dest = r.getChange().getDest(); + try (Repository git = gitManager.openRepository(dest.getParentKey()); + RevWalk rw = new RevWalk(git)) { + return canRebase( + r.getPatchSet(), dest, git, rw, db.get()); + } catch (IOException e) { + log.warn(String.format( + "Error checking if patch set %s on %s can be rebased", + patchSet.getId(), dest), e); + return false; + } + } + + public static boolean canRebase(PatchSet patchSet, Branch.NameKey dest, + Repository git, RevWalk rw, ReviewDb db) { + try { + findBaseRevision(patchSet, dest, git, rw, db); + return true; + } catch (RestApiException e) { + return false; + } catch (OrmException | IOException e) { + log.warn(String.format( + "Error checking if patch set %s on %s can be rebased", + patchSet.getId(), dest), e); + return false; + } + } + + /** + * Find the commit onto which a patch set should be rebased. + *

+ * This is defined as the latest patch set of the change corresponding to + * this commit's parent, or the destination branch tip in the case where the + * parent's change is merged. + * + * @param patchSet patch set for which the new base commit should be found. + * @param destBranch the destination branch. + * @param git the repository. + * @param rw the RevWalk. + * @return the commit onto which the patch set should be rebased. + * @throws RestApiException if rebase is not possible. + * @throws IOException if accessing the repository fails. + * @throws OrmException if accessing the database fails. + */ + static ObjectId findBaseRevision(PatchSet patchSet, + Branch.NameKey destBranch, Repository git, RevWalk rw, ReviewDb db) + throws RestApiException, IOException, OrmException { + String baseRev = null; + RevCommit commit = rw.parseCommit( + ObjectId.fromString(patchSet.getRevision().get())); + + if (commit.getParentCount() > 1) { + throw new UnprocessableEntityException( + "Cannot rebase a change with multiple parents."); + } else if (commit.getParentCount() == 0) { + throw new UnprocessableEntityException( + "Cannot rebase a change without any parents" + + " (is this the initial commit?)."); + } + + RevId parentRev = new RevId(commit.getParent(0).name()); + + for (PatchSet depPatchSet : db.patchSets().byRevision(parentRev)) { + Change.Id depChangeId = depPatchSet.getId().getParentKey(); + Change depChange = db.changes().get(depChangeId); + if (!depChange.getDest().equals(destBranch)) { + continue; + } + + if (depChange.getStatus() == Status.ABANDONED) { + throw new ResourceConflictException( + "Cannot rebase a change with an abandoned parent: " + + depChange.getKey()); + } + + if (depChange.getStatus().isOpen()) { + if (depPatchSet.getId().equals(depChange.currentPatchSetId())) { + throw new ResourceConflictException( + "Change is already based on the latest patch set of the" + + " dependent change."); + } + PatchSet latestDepPatchSet = + db.patchSets().get(depChange.currentPatchSetId()); + baseRev = latestDepPatchSet.getRevision().get(); + } + break; + } + + if (baseRev == null) { + // We are dependent on a merged PatchSet or have no PatchSet + // dependencies at all. + Ref destRef = git.getRefDatabase().exactRef(destBranch.get()); + if (destRef == null) { + throw new UnprocessableEntityException( + "The destination branch does not exist: " + destBranch.get()); + } + baseRev = destRef.getObjectId().getName(); + if (baseRev.equals(parentRev.get())) { + throw new ResourceConflictException("Change is already up to date."); + } + } + return ObjectId.fromString(baseRev); + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java index 2b46f052cd..c54fe262c2 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java @@ -27,6 +27,7 @@ import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.CurrentUser; +import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.index.ChangeIndexer; import com.google.gerrit.server.notedb.ChangeNotes; @@ -38,6 +39,7 @@ import com.google.inject.assistedinject.AssistedInject; import org.eclipse.jgit.lib.BatchRefUpdate; import org.eclipse.jgit.lib.NullProgressMonitor; import org.eclipse.jgit.lib.ObjectInserter; +import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.transport.ReceiveCommand; @@ -49,6 +51,7 @@ import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.TimeZone; /** * Context for a set of updates that should be applied for a site. @@ -123,6 +126,10 @@ public class BatchUpdate implements AutoCloseable { public void addRefUpdate(ReceiveCommand cmd) throws IOException { getBatchRefUpdate().addCommand(cmd); } + + public TimeZone getTimeZone() { + return tz; + } } public class ChangeContext extends Context { @@ -180,6 +187,7 @@ public class BatchUpdate implements AutoCloseable { private final Project.NameKey project; private final CurrentUser user; private final Timestamp when; + private final TimeZone tz; private final ListMultimap ops = ArrayListMultimap.create(); private final Map newChanges = new HashMap<>(); @@ -198,6 +206,7 @@ public class BatchUpdate implements AutoCloseable { ChangeControl.GenericFactory changeControlFactory, ChangeUpdate.Factory changeUpdateFactory, GitReferenceUpdated gitRefUpdated, + @GerritPersonIdent PersonIdent serverIdent, @Assisted ReviewDb db, @Assisted Project.NameKey project, @Assisted CurrentUser user, @@ -211,6 +220,7 @@ public class BatchUpdate implements AutoCloseable { this.project = project; this.user = user; this.when = when; + tz = serverIdent.getTimeZone(); } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java index 52f1feac73..5ef33a6b01 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java @@ -15,11 +15,13 @@ package com.google.gerrit.server.git.strategy; import com.google.common.collect.Lists; +import com.google.gerrit.common.TimeUtil; import com.google.gerrit.extensions.restapi.RestApiException; 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.change.RebaseChange; +import com.google.gerrit.server.change.RebaseChangeOp; +import com.google.gerrit.server.git.BatchUpdate; import com.google.gerrit.server.git.CodeReviewCommit; import com.google.gerrit.server.git.CommitMergeStatus; import com.google.gerrit.server.git.MergeConflictException; @@ -29,9 +31,7 @@ import com.google.gerrit.server.git.RebaseSorter; import com.google.gerrit.server.git.UpdateException; import com.google.gerrit.server.git.validators.CommitValidators; import com.google.gerrit.server.patch.PatchSetInfoFactory; -import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gerrit.server.project.NoSuchChangeException; -import com.google.gerrit.server.project.NoSuchProjectException; import com.google.gwtorm.server.OrmException; import org.eclipse.jgit.lib.ObjectId; @@ -46,15 +46,15 @@ import java.util.Map; public class RebaseIfNecessary extends SubmitStrategy { private final PatchSetInfoFactory patchSetInfoFactory; - private final RebaseChange rebaseChange; + private final RebaseChangeOp.Factory rebaseFactory; private final Map newCommits; RebaseIfNecessary(SubmitStrategy.Arguments args, PatchSetInfoFactory patchSetInfoFactory, - RebaseChange rebaseChange) { + RebaseChangeOp.Factory rebaseFactory) { super(args); this.patchSetInfoFactory = patchSetInfoFactory; - this.rebaseChange = rebaseChange; + this.rebaseFactory = rebaseFactory; this.newCommits = new HashMap<>(); } @@ -87,12 +87,7 @@ public class RebaseIfNecessary extends SubmitStrategy { } else { try { - PatchSet newPatchSet = - rebaseChange.rebase(args.repo, args.rw, args.inserter, - n.change(), n.getPatchsetId(), args.caller, - mergeTip.getCurrentTip(), args.mergeUtil, - args.serverIdent.get(), false, - CommitValidators.Policy.NONE); + PatchSet newPatchSet = rebase(n, mergeTip); List approvals = Lists.newArrayList(); for (PatchSetApproval a : args.approvalsUtil.byPatchSet(args.db, n.getControl(), n.getPatchsetId())) { @@ -116,11 +111,13 @@ public class RebaseIfNecessary extends SubmitStrategy { newCommits.put(newPatchSet.getId().getParentKey(), mergeTip.getCurrentTip()); setRefLogIdent(); - } catch (MergeConflictException e) { - n.setStatusCode(CommitMergeStatus.REBASE_MERGE_CONFLICT); - } catch (NoSuchChangeException | NoSuchProjectException - | OrmException | IOException | InvalidChangeOperationException - | UpdateException | RestApiException e) { + } catch (UpdateException e) { + if (e.getCause() instanceof MergeConflictException) { + n.setStatusCode(CommitMergeStatus.REBASE_MERGE_CONFLICT); + } + throw new MergeException("Cannot rebase " + n.name(), e); + } catch (NoSuchChangeException | OrmException | IOException + | RestApiException e) { // TODO(dborowitz): Allow Submit to unwrap ResourceConflictException // so it can turn into a 409. throw new MergeException("Cannot rebase " + n.name(), e); @@ -170,6 +167,22 @@ public class RebaseIfNecessary extends SubmitStrategy { } } + private PatchSet rebase(CodeReviewCommit n, MergeTip mergeTip) + throws RestApiException, UpdateException, OrmException { + RebaseChangeOp op = rebaseFactory.create( + n.getControl(), + args.db.patchSets().get(n.getPatchsetId()), + mergeTip.getCurrentTip().name()) + .setCommitterIdent(args.serverIdent.get()) + .setRunHooks(false) + .setValidatePolicy(CommitValidators.Policy.NONE); + try (BatchUpdate bu = args.newBatchUpdate(TimeUtil.nowTs())) { + bu.addOp(n.change().getId(), op); + bu.execute(); + } + return op.getPatchSet(); + } + @Override public Map getNewCommits() { return newCommits; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyFactory.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyFactory.java index 68ba75c98d..05aee25154 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyFactory.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyFactory.java @@ -20,7 +20,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.ApprovalsUtil; import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.IdentifiedUser; -import com.google.gerrit.server.change.RebaseChange; +import com.google.gerrit.server.change.RebaseChangeOp; import com.google.gerrit.server.git.BatchUpdate; import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk; import com.google.gerrit.server.git.MergeException; @@ -56,7 +56,7 @@ public class SubmitStrategyFactory { private final BatchUpdate.Factory batchUpdateFactory; private final ChangeControl.GenericFactory changeControlFactory; private final PatchSetInfoFactory patchSetInfoFactory; - private final RebaseChange rebaseChange; + private final RebaseChangeOp.Factory rebaseFactory; private final ProjectCache projectCache; private final ApprovalsUtil approvalsUtil; private final MergeUtil.Factory mergeUtilFactory; @@ -69,7 +69,7 @@ public class SubmitStrategyFactory { final BatchUpdate.Factory batchUpdateFactory, final ChangeControl.GenericFactory changeControlFactory, final PatchSetInfoFactory patchSetInfoFactory, - final RebaseChange rebaseChange, + final RebaseChangeOp.Factory rebaseFactory, final ProjectCache projectCache, final ApprovalsUtil approvalsUtil, final MergeUtil.Factory mergeUtilFactory, @@ -79,7 +79,7 @@ public class SubmitStrategyFactory { this.batchUpdateFactory = batchUpdateFactory; this.changeControlFactory = changeControlFactory; this.patchSetInfoFactory = patchSetInfoFactory; - this.rebaseChange = rebaseChange; + this.rebaseFactory = rebaseFactory; this.projectCache = projectCache; this.approvalsUtil = approvalsUtil; this.mergeUtilFactory = mergeUtilFactory; @@ -107,7 +107,7 @@ public class SubmitStrategyFactory { case MERGE_IF_NECESSARY: return new MergeIfNecessary(args); case REBASE_IF_NECESSARY: - return new RebaseIfNecessary(args, patchSetInfoFactory, rebaseChange); + return new RebaseIfNecessary(args, patchSetInfoFactory, rebaseFactory); default: final String errorMsg = "No submit strategy for: " + submitType; log.error(errorMsg);