Store for revert commits that they do not contain conflicts Revert commits are based on the commit that is being reverted and have the same tree as the parent of the commit that is being reverted. This means revert commits never contain any conflicts. Storing conflict information for revisions has been added in change I15e48ba87. In this case ours/theirs in ConflictsInfo is not set, since the revision was not created by performing a Git merge. Bug: Google b/373350443 Release-Notes: skip Change-Id: I2950ea744f3db52a70c1d3f90372a88940eb4487 Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/java/com/google/gerrit/server/git/CommitUtil.java b/java/com/google/gerrit/server/git/CommitUtil.java index 4a5e1b0..30f2ee8 100644 --- a/java/com/google/gerrit/server/git/CommitUtil.java +++ b/java/com/google/gerrit/server/git/CommitUtil.java
@@ -46,6 +46,7 @@ import com.google.gerrit.server.change.NotifyResolver; import com.google.gerrit.server.change.ValidationOptionsUtil; import com.google.gerrit.server.extensions.events.ChangeReverted; +import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk; import com.google.gerrit.server.mail.EmailFactories; import com.google.gerrit.server.mail.send.ChangeEmail; import com.google.gerrit.server.mail.send.MessageIdGenerator; @@ -173,12 +174,12 @@ try (Repository git = repoManager.openRepository(notes.getProjectName()); ObjectInserter oi = git.newObjectInserter(); ObjectReader reader = oi.newReader(); - RevWalk revWalk = new RevWalk(reader)) { + CodeReviewRevWalk revWalk = CodeReviewCommit.newRevWalk(reader)) { ObjectId generatedChangeId = CommitMessageUtil.generateChangeId(); - ObjectId revCommit = + CodeReviewCommit revertCommit = createRevertCommit(message, notes, user, timestamp, oi, revWalk, generatedChangeId); return createRevertChangeFromCommit( - revCommit, input, notes, user, generatedChangeId, timestamp, oi, revWalk, git); + revertCommit, input, notes, user, generatedChangeId, timestamp, oi, revWalk, git); } catch (RepositoryNotFoundException e) { throw new ResourceNotFoundException(notes.getChangeId().toString(), e); } @@ -192,16 +193,16 @@ * @param notes ChangeNotes of the change being reverted. * @param user Current User performing the revert. * @param ts Timestamp of creation for the commit. - * @return ObjectId that represents the newly created commit. + * @return that newly created revert commit. */ - public ObjectId createRevertCommit( + public CodeReviewCommit createRevertCommit( String message, ChangeNotes notes, CurrentUser user, Instant ts) throws RestApiException, IOException { try (Repository git = repoManager.openRepository(notes.getProjectName()); ObjectInserter oi = git.newObjectInserter(); ObjectReader reader = oi.newReader(); - RevWalk revWalk = new RevWalk(reader)) { + CodeReviewRevWalk revWalk = CodeReviewCommit.newRevWalk(reader)) { return createRevertCommit(message, notes, user, ts, oi, revWalk, null); } catch (RepositoryNotFoundException e) { throw new ResourceNotFoundException(notes.getProjectName().toString(), e); @@ -256,13 +257,13 @@ * @throws ResourceConflictException Can't revert the initial commit. * @throws IOException Thrown in case of I/O errors. */ - private ObjectId createRevertCommit( + private CodeReviewCommit createRevertCommit( String message, ChangeNotes notes, CurrentUser user, Instant ts, ObjectInserter oi, - RevWalk revWalk, + CodeReviewRevWalk revWalk, @Nullable ObjectId generatedChangeId) throws ResourceConflictException, IOException { @@ -293,17 +294,26 @@ message = ChangeIdUtil.insertId(message, generatedChangeId, true); } - return createCommitWithTree( - oi, - authorIdent, - committerIdent, - ImmutableList.of(commitToRevert), - message, - parentToCommitToRevert.getTree()); + CodeReviewCommit revertCommit = + revWalk.parseCommit( + createCommitWithTree( + oi, + authorIdent, + committerIdent, + ImmutableList.of(commitToRevert), + message, + parentToCommitToRevert.getTree())); + + // The revert commit is based on the commit that is being reverted and has the same tree as the + // parent of the commit that is being reverted. This means revert commit never contains any + // conflicts. + revertCommit.setNoConflicts(); + + return revertCommit; } private Change.Id createRevertChangeFromCommit( - ObjectId revertCommitId, + CodeReviewCommit revertCommit, RevertInput input, ChangeNotes notes, CurrentUser user, @@ -313,7 +323,6 @@ RevWalk revWalk, Repository git) throws IOException, RestApiException, UpdateException, ConfigInvalidException { - RevCommit revertCommit = revWalk.parseCommit(revertCommitId); Change.Id changeId = Change.id(seq.nextChangeId()); if (input.getWorkInProgress()) { input.notify = firstNonNull(input.notify, NotifyHandling.NONE); @@ -341,6 +350,7 @@ ins.setReviewersAndCcsIgnoreVisibility(reviewers, ccs); ins.setRevertOf(notes.getChangeId()); ins.setWorkInProgress(input.getWorkInProgress()); + revertCommit.getConflicts().ifPresent(ins::setConflicts); try (BatchUpdate bu = updateFactory.create(notes.getProjectName(), user, ts)) { bu.setRepository(git, revWalk, oi);
diff --git a/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java b/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java index 5191a58..f6bd27a 100644 --- a/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java +++ b/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java
@@ -205,6 +205,11 @@ assertThat(revertChange.messages).hasSize(1); assertThat(revertChange.messages.iterator().next().message).isEqualTo("Uploaded patch set 1."); assertThat(revertChange.revertOf).isEqualTo(gApi.changes().id(r.getChangeId()).get()._number); + + assertThat(revertChange.getCurrentRevision().conflicts).isNotNull(); + assertThat(revertChange.getCurrentRevision().conflicts.containsConflicts).isFalse(); + assertThat(revertChange.getCurrentRevision().conflicts.ours).isNull(); + assertThat(revertChange.getCurrentRevision().conflicts.theirs).isNull(); } @Test @@ -947,6 +952,12 @@ .isEqualTo(result.getChange().change().getChangeId()); assertThat(revertChanges.get(0).get().topic) .startsWith("revert-" + result.getChange().change().getSubmissionId() + "-"); + + assertThat(revertChanges.get(0).get().getCurrentRevision().conflicts).isNotNull(); + assertThat(revertChanges.get(0).get().getCurrentRevision().conflicts.containsConflicts) + .isFalse(); + assertThat(revertChanges.get(0).get().getCurrentRevision().conflicts.ours).isNull(); + assertThat(revertChanges.get(0).get().getCurrentRevision().conflicts.theirs).isNull(); } @Test @@ -1191,6 +1202,16 @@ assertThat(revertChanges).hasSize(2); assertThat(gApi.changes().id(revertChanges.get(0).id()).current().related().changes).hasSize(2); + + // None of the revert changes has conflicts. + for (int i = 0; i < revertChanges.size(); i++) { + // Internally RevertSubmission either uses Revert or Cherry-Pick to do the reverts. + // Depending on which operation is used ours/theirs is set (if Cherry-Pick is used) or unset + // (if Revert is used). Hence we do not validate ours/theirs here. + assertThat(revertChanges.get(i).get().getCurrentRevision().conflicts).isNotNull(); + assertThat(revertChanges.get(i).get().getCurrentRevision().conflicts.containsConflicts) + .isFalse(); + } } @Test