Allow rebasing change edits with conflicts
Bug: Google b/364179875
Release-Notes: Added support for rebasing change edits with conflicts
Change-Id: I8ad5627b5dac19b4608efb377b772478c03f1b43
Signed-off-by: Edwin Kempin <ekempin@google.com>
(cherry picked from commit 34699b66ad4165132c375386225c0279895eb983)
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 6279f02..b9c7acc 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -3634,6 +3634,9 @@
Rebases change edit on top of latest patch set.
+Optionally, input parameters may be specified in the request body as a
+link:#rebase-change-edit-input[RebaseChangeEditInput] entity.
+
If one of the secondary emails associated with the user performing the operation was used as the
committer email in the latest patch set, the same email will be used as the committer email in the
new change edit commit; otherwise, the user's preferred email will be used.
@@ -8482,6 +8485,19 @@
|`end` | Last index.
|===========================
+[[rebase-change-edit-input]]
+=== RebaseChangeEditInput
+The `RebaseChangeEditInput` entity contains information for rebasing a change edit.
+
+[options="header",cols="1,^1,5"]
+|====================================
+|Field Name ||Description
+|`allow_conflicts` |optional, defaults to false|
+If `true`, the rebase also succeeds if there are conflicts. +
+If there are conflicts the file contents of the rebased patch set contain
+git conflict markers to indicate the conflicts.
+|====================================
+
[[rebase-input]]
=== RebaseInput
The `RebaseInput` entity contains information for changing parent when rebasing.
diff --git a/java/com/google/gerrit/extensions/api/changes/ChangeEditApi.java b/java/com/google/gerrit/extensions/api/changes/ChangeEditApi.java
index 2fd8a07..1b937e0 100644
--- a/java/com/google/gerrit/extensions/api/changes/ChangeEditApi.java
+++ b/java/com/google/gerrit/extensions/api/changes/ChangeEditApi.java
@@ -91,6 +91,14 @@
void rebase() throws RestApiException;
/**
+ * Rebases the change edit on top of the latest patch set of this change.
+ *
+ * @param input params for rebasing the change edit
+ * @throws RestApiException if the change edit couldn't be rebased or a change edit wasn't present
+ */
+ void rebase(RebaseChangeEditInput input) throws RestApiException;
+
+ /**
* Publishes the change edit using default settings. See {@link #publish(PublishChangeEditInput)}
* for more details.
*
@@ -238,6 +246,11 @@
}
@Override
+ public void rebase(RebaseChangeEditInput input) throws RestApiException {
+ throw new NotImplementedException();
+ }
+
+ @Override
public void publish() throws RestApiException {
throw new NotImplementedException();
}
diff --git a/java/com/google/gerrit/extensions/api/changes/RebaseChangeEditInput.java b/java/com/google/gerrit/extensions/api/changes/RebaseChangeEditInput.java
new file mode 100644
index 0000000..4eb0ebb
--- /dev/null
+++ b/java/com/google/gerrit/extensions/api/changes/RebaseChangeEditInput.java
@@ -0,0 +1,25 @@
+// Copyright (C) 2024 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.extensions.api.changes;
+
+public class RebaseChangeEditInput {
+ /**
+ * Whether the rebase should succeed if there are conflicts.
+ *
+ * <p>If there are conflicts the file contents of the rebased change contain git conflict markers
+ * to indicate the conflicts.
+ */
+ public boolean allowConflicts;
+}
diff --git a/java/com/google/gerrit/server/api/changes/ChangeEditApiImpl.java b/java/com/google/gerrit/server/api/changes/ChangeEditApiImpl.java
index c76eeeb..5f2ae0f 100644
--- a/java/com/google/gerrit/server/api/changes/ChangeEditApiImpl.java
+++ b/java/com/google/gerrit/server/api/changes/ChangeEditApiImpl.java
@@ -20,6 +20,7 @@
import com.google.gerrit.extensions.api.changes.ChangeEditIdentityType;
import com.google.gerrit.extensions.api.changes.FileContentInput;
import com.google.gerrit.extensions.api.changes.PublishChangeEditInput;
+import com.google.gerrit.extensions.api.changes.RebaseChangeEditInput;
import com.google.gerrit.extensions.client.ChangeEditDetailOption;
import com.google.gerrit.extensions.common.EditInfo;
import com.google.gerrit.extensions.common.Input;
@@ -150,9 +151,14 @@
@Override
public void rebase() throws RestApiException {
+ rebase(new RebaseChangeEditInput());
+ }
+
+ @Override
+ public void rebase(RebaseChangeEditInput input) throws RestApiException {
try {
@SuppressWarnings("unused")
- var unused = rebaseChangeEdit.apply(changeResource, null);
+ var unused = rebaseChangeEdit.apply(changeResource, input);
} catch (Exception e) {
throw asRestApiException("Cannot rebase change edit", e);
}
diff --git a/java/com/google/gerrit/server/edit/ChangeEditModifier.java b/java/com/google/gerrit/server/edit/ChangeEditModifier.java
index 4c856e9..52db66f 100644
--- a/java/com/google/gerrit/server/edit/ChangeEditModifier.java
+++ b/java/com/google/gerrit/server/edit/ChangeEditModifier.java
@@ -26,6 +26,7 @@
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.api.changes.ChangeEditIdentityType;
+import com.google.gerrit.extensions.api.changes.RebaseChangeEditInput;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.MergeConflictException;
@@ -64,12 +65,15 @@
import java.time.Instant;
import java.time.ZoneId;
import java.util.List;
+import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import org.eclipse.jgit.diff.DiffAlgorithm;
import org.eclipse.jgit.diff.DiffAlgorithm.SupportedAlgorithm;
import org.eclipse.jgit.diff.RawText;
import org.eclipse.jgit.diff.RawTextComparator;
+import org.eclipse.jgit.diff.Sequence;
+import org.eclipse.jgit.dircache.DirCache;
import org.eclipse.jgit.dircache.InvalidPathException;
import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.CommitBuilder;
@@ -159,12 +163,13 @@
*
* @param repository the affected Git repository
* @param notes the {@link ChangeNotes} of the change whose change edit should be rebased
+ * @param input the request input
* @throws AuthException if the user isn't authenticated or not allowed to use change edits
* @throws InvalidChangeOperationException if a change edit doesn't exist for the specified
* change, the change edit is already based on the latest patch set, or the change represents
* the root commit
*/
- public void rebaseEdit(Repository repository, ChangeNotes notes)
+ public void rebaseEdit(Repository repository, ChangeNotes notes, RebaseChangeEditInput input)
throws AuthException, InvalidChangeOperationException, IOException,
PermissionBackendException, ResourceConflictException {
assertCanEdit(notes);
@@ -184,14 +189,15 @@
notes.getChangeId(), currentPatchSet.id()));
}
- rebase(notes.getProjectName(), repository, changeEdit, currentPatchSet);
+ rebase(notes.getProjectName(), repository, changeEdit, currentPatchSet, input.allowConflicts);
}
private void rebase(
Project.NameKey project,
Repository repository,
ChangeEdit changeEdit,
- PatchSet currentPatchSet)
+ PatchSet currentPatchSet,
+ boolean allowConflicts)
throws IOException, MergeConflictException, InvalidChangeOperationException {
RevCommit currentEditCommit = changeEdit.getEditCommit();
if (currentEditCommit.getParentCount() == 0) {
@@ -200,7 +206,7 @@
}
RevCommit basePatchSetCommit = NoteDbEdits.lookupCommit(repository, currentPatchSet.commitId());
- ObjectId newTreeId = merge(repository, changeEdit, basePatchSetCommit);
+ ObjectId newTreeId = merge(repository, changeEdit, basePatchSetCommit, allowConflicts);
Instant nowTimestamp = TimeUtil.now();
String commitMessage = currentEditCommit.getFullMessage();
ObjectId newEditCommitId =
@@ -555,30 +561,66 @@
}
private static ObjectId merge(
- Repository repository, ChangeEdit changeEdit, RevCommit basePatchSetCommit)
+ Repository repository,
+ ChangeEdit changeEdit,
+ RevCommit basePatchSetCommit,
+ boolean allowConflicts)
throws IOException, MergeConflictException {
PatchSet basePatchSet = changeEdit.getBasePatchSet();
ObjectId basePatchSetCommitId = basePatchSet.commitId();
ObjectId editCommitId = changeEdit.getEditCommit();
- ThreeWayMerger merger = MergeStrategy.RESOLVE.newMerger(repository, true);
- merger.setBase(basePatchSetCommitId);
- boolean successful = merger.merge(basePatchSetCommit, editCommitId);
+ try (RevWalk revWalk = new RevWalk(repository);
+ ObjectInserter objectInserter = repository.newObjectInserter()) {
+ ThreeWayMerger merger = MergeStrategy.RESOLVE.newMerger(repository, true);
+ merger.setBase(basePatchSetCommitId);
- if (!successful) {
- List<String> conflicts = ImmutableList.of();
- if (merger instanceof ResolveMerger) {
- conflicts = ((ResolveMerger) merger).getUnmergedPaths();
+ DirCache dc = DirCache.newInCore();
+ if (allowConflicts && merger instanceof ResolveMerger) {
+ // The DirCache must be set on ResolveMerger before calling
+ // ResolveMerger#merge(AnyObjectId...) otherwise the entries in DirCache don't get
+ // populated.
+ ((ResolveMerger) merger).setDirCache(dc);
}
- throw new MergeConflictException(
- String.format(
- "Rebasing change edit onto another patchset results in merge conflicts.\n\n"
- + "%s\n\n"
- + "Download the edit patchset and rebase manually to preserve changes.",
- MergeUtil.createConflictMessage(conflicts)));
+ boolean successful = merger.merge(basePatchSetCommit, editCommitId);
+
+ ObjectId newTreeId;
+ if (successful) {
+ newTreeId = merger.getResultTreeId();
+ } else {
+ List<String> conflicts = ImmutableList.of();
+ if (merger instanceof ResolveMerger) {
+ conflicts = ((ResolveMerger) merger).getUnmergedPaths();
+ }
+
+ if (!allowConflicts || !(merger instanceof ResolveMerger)) {
+ throw new MergeConflictException(
+ String.format(
+ "Rebasing change edit onto another patchset results in merge conflicts.\n\n"
+ + "%s\n\n"
+ + "Download the edit patchset and rebase manually to preserve changes.",
+ MergeUtil.createConflictMessage(conflicts)));
+ }
+
+ Map<String, MergeResult<? extends Sequence>> mergeResults =
+ ((ResolveMerger) merger).getMergeResults();
+
+ newTreeId =
+ MergeUtil.mergeWithConflicts(
+ revWalk,
+ objectInserter,
+ dc,
+ "PATCH SET",
+ basePatchSetCommit,
+ "EDIT",
+ revWalk.parseCommit(editCommitId),
+ mergeResults,
+ /* diff3Format= */ false);
+ objectInserter.flush();
+ }
+ return newTreeId;
}
- return merger.getResultTreeId();
}
private static ObjectId mergeTrees(Repository repository, ChangeEdit changeEdit, ObjectId treeId)
diff --git a/java/com/google/gerrit/server/restapi/change/RebaseChangeEdit.java b/java/com/google/gerrit/server/restapi/change/RebaseChangeEdit.java
index 9fb8de8..8c1fc02 100644
--- a/java/com/google/gerrit/server/restapi/change/RebaseChangeEdit.java
+++ b/java/com/google/gerrit/server/restapi/change/RebaseChangeEdit.java
@@ -15,7 +15,7 @@
package com.google.gerrit.server.restapi.change;
import com.google.gerrit.entities.Project;
-import com.google.gerrit.extensions.common.Input;
+import com.google.gerrit.extensions.api.changes.RebaseChangeEditInput;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Response;
@@ -31,7 +31,7 @@
import org.eclipse.jgit.lib.Repository;
@Singleton
-public class RebaseChangeEdit implements RestModifyView<ChangeResource, Input> {
+public class RebaseChangeEdit implements RestModifyView<ChangeResource, RebaseChangeEditInput> {
private final GitRepositoryManager repositoryManager;
private final ChangeEditModifier editModifier;
@@ -42,11 +42,15 @@
}
@Override
- public Response<Object> apply(ChangeResource rsrc, Input in)
+ public Response<Object> apply(ChangeResource rsrc, RebaseChangeEditInput input)
throws AuthException, ResourceConflictException, IOException, PermissionBackendException {
+ if (input == null) {
+ input = new RebaseChangeEditInput();
+ }
+
Project.NameKey project = rsrc.getProject();
try (Repository repository = repositoryManager.openRepository(project)) {
- editModifier.rebaseEdit(repository, rsrc.getNotes());
+ editModifier.rebaseEdit(repository, rsrc.getNotes(), input);
} catch (InvalidChangeOperationException e) {
throw new ResourceConflictException(e.getMessage());
}
diff --git a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
index b55cfee..e1e384a 100644
--- a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
+++ b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
@@ -54,6 +54,7 @@
import com.google.gerrit.extensions.api.changes.FileContentInput;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.PublishChangeEditInput;
+import com.google.gerrit.extensions.api.changes.RebaseChangeEditInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.ReviewerInput;
import com.google.gerrit.extensions.client.ChangeEditDetailOption;
@@ -75,6 +76,7 @@
import com.google.gerrit.extensions.restapi.MergeConflictException;
import com.google.gerrit.extensions.restapi.RawInput;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.git.ObjectIds;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.project.testing.TestLabels;
import com.google.gerrit.server.restapi.change.ChangeEdits;
@@ -106,8 +108,10 @@
private static final String FILE_NAME3 = "foo3";
private static final String FILE_NAME4 = "foo4";
private static final int FILE_MODE = 100644;
- private static final byte[] CONTENT_OLD = "bar".getBytes(UTF_8);
- private static final byte[] CONTENT_NEW = "baz".getBytes(UTF_8);
+ private static final String CONTENT_OLD_STR = "bar";
+ private static final byte[] CONTENT_OLD = CONTENT_OLD_STR.getBytes(UTF_8);
+ private static final String CONTENT_NEW_STR = "baz";
+ private static final byte[] CONTENT_NEW = CONTENT_NEW_STR.getBytes(UTF_8);
private static final String CONTENT_NEW2_STR = "quxÄÜÖßµ";
private static final byte[] CONTENT_NEW2 = CONTENT_NEW2_STR.getBytes(UTF_8);
private static final String CONTENT_BINARY_ENCODED_NEW =
@@ -304,6 +308,48 @@
}
@Test
+ public void rebaseEditWithConflictsAllowed() throws Exception {
+ // Create change where FILE_NAME has OLD_CONTENT
+ String changeId = newChange(admin.newIdent());
+
+ PatchSet previousPatchSet = getCurrentPatchSet(changeId);
+ createEmptyEditFor(changeId);
+ gApi.changes().id(changeId).edit().modifyFile(FILE_NAME, RawInputUtil.create(CONTENT_NEW));
+
+ // add new patch set that touches the same file as the edit
+ addNewPatchSetWithModifiedFile(changeId, FILE_NAME, new String(CONTENT_NEW2, UTF_8));
+ PatchSet currentPatchSet = getCurrentPatchSet(changeId);
+
+ Optional<EditInfo> originalEdit = getEdit(changeId);
+ assertThat(originalEdit).value().baseRevision().isEqualTo(previousPatchSet.commitId().name());
+
+ Timestamp beforeRebase = originalEdit.get().commit.committer.date;
+
+ RebaseChangeEditInput input = new RebaseChangeEditInput();
+ input.allowConflicts = true;
+ gApi.changes().id(changeId).edit().rebase(input);
+
+ ensureSameBytes(
+ getFileContentOfEdit(changeId, FILE_NAME),
+ String.format(
+ "<<<<<<< PATCH SET (%s %s)\n"
+ + "%s\n"
+ + "=======\n"
+ + "%s\n"
+ + ">>>>>>> EDIT (%s %s)\n",
+ ObjectIds.abbreviateName(currentPatchSet.commitId(), 6),
+ gApi.changes().id(changeId).get().subject,
+ CONTENT_NEW2_STR,
+ CONTENT_NEW_STR,
+ ObjectIds.abbreviateName(ObjectId.fromString(originalEdit.get().commit.commit), 6),
+ originalEdit.get().commit.subject)
+ .getBytes(UTF_8));
+ Optional<EditInfo> rebasedEdit = getEdit(changeId);
+ assertThat(rebasedEdit).value().baseRevision().isEqualTo(currentPatchSet.commitId().name());
+ assertThat(rebasedEdit).value().commit().committer().date().isNotEqualTo(beforeRebase);
+ }
+
+ @Test
public void rebaseEditAfterUpdatingPreferredEmail() throws Exception {
String emailOne = "email1@example.com";
Account.Id testUser = accountOperations.newAccount().preferredEmail(emailOne).create();
