diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java index db0a4b7070..2131142f7e 100644 --- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java +++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java @@ -648,14 +648,27 @@ public abstract class AbstractDaemonTest { String changeId, String ref, TestAccount testAccount, TestRepository repo) throws Exception { Collections.shuffle(RANDOM); + return amendChange(changeId, ref, testAccount, repo, PushOneCommit.SUBJECT, + PushOneCommit.FILE_NAME, new String(Chars.toArray(RANDOM))); + } + + protected PushOneCommit.Result amendChange(String changeId, String subject, String fileName, + String content) throws Exception { + return amendChange(changeId, "refs/for/master", admin, testRepo, subject, fileName, content); + } + + protected PushOneCommit.Result amendChange( + String changeId, String ref, TestAccount testAccount, TestRepository repo, String subject, + String fileName, String content) + throws Exception { PushOneCommit push = pushFactory.create( db, testAccount.getIdent(), repo, - PushOneCommit.SUBJECT, - PushOneCommit.FILE_NAME, - new String(Chars.toArray(RANDOM)), + subject, + fileName, + content, changeId); return push.to(ref); } diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmitByRebase.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmitByRebase.java index 32c81ad861..77ca14f96a 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmitByRebase.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmitByRebase.java @@ -412,6 +412,27 @@ public abstract class AbstractSubmitByRebase extends AbstractSubmit { submit(change2.getChangeId()); } + @Test + @TestProjectInput(useContentMerge = InheritableBoolean.TRUE) + public void submitChainFailsOnRework() throws Exception { + PushOneCommit.Result change1 = createChange("subject 1", "fileName 1", "content 1"); + RevCommit headAfterChange1 = change1.getCommit(); + PushOneCommit.Result change2 = createChange("subject 2", "fileName 2", "content 2"); + testRepo.reset(headAfterChange1); + change1 = amendChange(change1.getChangeId(), "subject 1 amend", "fileName 2", + "rework content 2"); + submit(change1.getChangeId()); + headAfterChange1 = getRemoteHead(); + + submitWithConflict( + change2.getChangeId(), + "Cannot rebase " + + change2.getCommit().getName() + + ": " + + "The change could not be rebased due to a conflict during merge."); + assertThat(getRemoteHead()).isEqualTo(headAfterChange1); + } + @Test @TestProjectInput(useContentMerge = InheritableBoolean.TRUE) public void submitChainOneByOneManualRebase() throws Exception { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/RebaseSorter.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/RebaseSorter.java index b8c5ff365c..4df1f24d4d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/RebaseSorter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/RebaseSorter.java @@ -14,11 +14,8 @@ package com.google.gerrit.server.git; -import com.google.gerrit.extensions.client.ChangeKind; import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Change.Status; -import com.google.gerrit.reviewdb.client.Project; -import com.google.gerrit.server.change.ChangeKindCache; import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk; import com.google.gerrit.server.git.strategy.CommitMergeStatus; import com.google.gerrit.server.query.change.ChangeData; @@ -32,8 +29,6 @@ import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Set; -import org.eclipse.jgit.lib.ObjectId; -import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevFlag; import org.slf4j.Logger; @@ -47,24 +42,18 @@ public class RebaseSorter { private final RevCommit initialTip; private final Set alreadyAccepted; private final InternalChangeQuery internalChangeQuery; - private final ChangeKindCache changeKindCache; - private final Repository repo; public RebaseSorter( CodeReviewRevWalk rw, RevCommit initialTip, Set alreadyAccepted, RevFlag canMergeFlag, - InternalChangeQuery internalChangeQuery, - ChangeKindCache changeKindCache, - Repository repo) { + InternalChangeQuery internalChangeQuery) { this.rw = rw; this.canMergeFlag = canMergeFlag; this.initialTip = initialTip; this.alreadyAccepted = alreadyAccepted; this.internalChangeQuery = internalChangeQuery; - this.changeKindCache = changeKindCache; - this.repo = repo; } public List sort(Collection incoming) throws IOException { @@ -127,8 +116,7 @@ public class RebaseSorter { List changes = internalChangeQuery.byCommit(commit); for (ChangeData change : changes) { if (change.change().getStatus() == Status.MERGED - && change.change().getDest().equals(dest) - && !isRework(dest.getParentKey(), commit, change)) { + && change.change().getDest().equals(dest)) { log.debug( "Dependency {} associated with merged change {}.", commit.getName(), change.getId()); return true; @@ -140,14 +128,6 @@ public class RebaseSorter { } } - private boolean isRework(Project.NameKey project, RevCommit oldCommit, ChangeData change) - throws OrmException, IOException { - RevCommit currentCommit = - rw.parseCommit(ObjectId.fromString(change.currentPatchSet().getRevision().get())); - return ChangeKind.REWORK - == changeKindCache.getChangeKind(project, repo, oldCommit, currentCommit); - } - private static T removeOne(final Collection c) { final Iterator i = c.iterator(); final T r = i.next(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseSubmitStrategy.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseSubmitStrategy.java index 52ff29ad7b..43ab01bcb0 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseSubmitStrategy.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseSubmitStrategy.java @@ -295,9 +295,7 @@ public class RebaseSubmitStrategy extends SubmitStrategy { args.mergeTip.getInitialTip(), args.alreadyAccepted, args.canMergeFlag, - args.internalChangeQuery, - args.changeKindCache, - args.repo) + args.internalChangeQuery) .sort(toSort); } catch (IOException e) { throw new IntegrationException("Commit sorting failed", e); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java index caf1473aef..f721978640 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java @@ -31,7 +31,6 @@ import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.PatchSetUtil; import com.google.gerrit.server.account.AccountCache; -import com.google.gerrit.server.change.ChangeKindCache; import com.google.gerrit.server.change.RebaseChangeOp; import com.google.gerrit.server.extensions.events.ChangeMerged; import com.google.gerrit.server.git.CodeReviewCommit; @@ -123,7 +122,6 @@ public abstract class SubmitStrategy { final OnSubmitValidators.Factory onSubmitValidatorsFactory; final TagCache tagCache; final InternalChangeQuery internalChangeQuery; - final ChangeKindCache changeKindCache; final Branch.NameKey destBranch; final CodeReviewRevWalk rw; @@ -167,7 +165,6 @@ public abstract class SubmitStrategy { OnSubmitValidators.Factory onSubmitValidatorsFactory, TagCache tagCache, InternalChangeQuery internalChangeQuery, - ChangeKindCache changeKindCache, @Assisted Branch.NameKey destBranch, @Assisted CommitStatus commitStatus, @Assisted CodeReviewRevWalk rw, @@ -200,7 +197,6 @@ public abstract class SubmitStrategy { this.rebaseFactory = rebaseFactory; this.tagCache = tagCache; this.internalChangeQuery = internalChangeQuery; - this.changeKindCache = changeKindCache; this.serverIdent = serverIdent; this.destBranch = destBranch;