ApplyPatch: Populate hasGitConflicts filed
Release-Notes: skip
Google-Bug: b/271801921
Change-Id: I3801f124dd92fbfbfb9729d3a3dd3f77a9fe8bab
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index ca7ae0c..6e14415 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -7358,9 +7358,10 @@
Only set if this change info is returned in response to a request that
creates a new change or patch set and conflicts are allowed. In
particular this field is only populated if the change info is returned
-by one of the following REST endpoints: link:#create-change[Create
-Change], link:#create-merge-patch-set-for-change[Create Merge Patch Set
-For Change], link:#cherry-pick[Cherry Pick Revision],
+by one of the following REST endpoints:link:#apply-patch[Apply Patch],
+link:#create-change[Create Change],
+link:#create-merge-patch-set-for-change[Create Merge Patch Set For Change],
+link:#cherry-pick[Cherry Pick Revision],
link:rest-api-project.html#cherry-pick-commit[Cherry Pick Commit],
link:#rebase-change[Rebase Change]
|==================================
diff --git a/java/com/google/gerrit/extensions/common/ChangeInfo.java b/java/com/google/gerrit/extensions/common/ChangeInfo.java
index 52127e4..1576e68 100644
--- a/java/com/google/gerrit/extensions/common/ChangeInfo.java
+++ b/java/com/google/gerrit/extensions/common/ChangeInfo.java
@@ -93,6 +93,7 @@
* <p>Only set if this change info is returned in response to a request that creates a new change
* or patch set and conflicts are allowed. In particular this field is only populated if the
* change info is returned by one of the following REST endpoints: {@link
+ * com.google.gerrit.server.restapi.change.ApplyPatch},{@link
* com.google.gerrit.server.restapi.change.CreateChange}, {@link
* com.google.gerrit.server.restapi.change.CreateMergePatchSet}, {@link
* com.google.gerrit.server.restapi.change.CherryPick}, {@link
diff --git a/java/com/google/gerrit/server/restapi/change/ApplyPatch.java b/java/com/google/gerrit/server/restapi/change/ApplyPatch.java
index 6e25e61..8d5247d 100644
--- a/java/com/google/gerrit/server/restapi/change/ApplyPatch.java
+++ b/java/com/google/gerrit/server/restapi/change/ApplyPatch.java
@@ -56,6 +56,7 @@
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.patch.PatchApplier;
+import org.eclipse.jgit.patch.PatchApplier.Result.Error;
import org.eclipse.jgit.revwalk.FooterLine;
import org.eclipse.jgit.revwalk.RevCommit;
@@ -171,6 +172,10 @@
revWalk,
applyResult.getTreeId(),
commitMessage);
+ if (changeInfo.containsGitConflicts == null
+ && applyResult.getErrors().stream().anyMatch(Error::isGitConflict)) {
+ changeInfo.containsGitConflicts = true;
+ }
return Response.ok(changeInfo);
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/ApplyPatchUtil.java b/java/com/google/gerrit/server/restapi/change/ApplyPatchUtil.java
index 1b26703..8b8aaa6 100644
--- a/java/com/google/gerrit/server/restapi/change/ApplyPatchUtil.java
+++ b/java/com/google/gerrit/server/restapi/change/ApplyPatchUtil.java
@@ -41,6 +41,7 @@
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.patch.Patch;
import org.eclipse.jgit.patch.PatchApplier;
+import org.eclipse.jgit.patch.PatchApplier.Result.Error;
import org.eclipse.jgit.revwalk.FooterLine;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevTree;
@@ -126,11 +127,10 @@
boolean appendResultPatch = false;
String decodedOriginalPatch = decodeIfNecessary(patchInput.patch);
if (!errors.isEmpty()) {
- if (Boolean.TRUE.equals(patchInput.allowConflicts)) {
+ if (errors.stream().allMatch(Error::isGitConflict)) {
res.append(
- "\n\nNOTE FOR REVIEWERS - conflicts or errors occurred while applying the patch.\n"
- + "PLEASE REVIEW CAREFULLY.\nOriginal patch:\n");
- appendOriginalPatch = true;
+ "\n\nATTENTION: Conflicts occurred while applying the patch.\n"
+ + "Please resolve conflict markers.");
} else {
res.append(
"\n\nNOTE FOR REVIEWERS - errors occurred while applying the patch."
diff --git a/java/com/google/gerrit/server/restapi/change/CreateChange.java b/java/com/google/gerrit/server/restapi/change/CreateChange.java
index 60ba4b7..746313b 100644
--- a/java/com/google/gerrit/server/restapi/change/CreateChange.java
+++ b/java/com/google/gerrit/server/restapi/change/CreateChange.java
@@ -37,7 +37,6 @@
import com.google.gerrit.exceptions.InvalidMergeStrategyException;
import com.google.gerrit.exceptions.MergeWithConflictsNotSupportedException;
import com.google.gerrit.extensions.api.accounts.AccountInput;
-import com.google.gerrit.extensions.api.changes.ApplyPatchInput;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.client.ListChangesOption;
@@ -115,6 +114,7 @@
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.lib.TreeFormatter;
import org.eclipse.jgit.patch.PatchApplier;
+import org.eclipse.jgit.patch.PatchApplier.Result.Error;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.util.ChangeIdUtil;
@@ -440,6 +440,7 @@
String commitMessage = getCommitMessage(input.subject, me);
CodeReviewCommit c;
+ boolean hasGitConflicts = false;
if (input.merge != null) {
// create a merge commit
c =
@@ -460,9 +461,32 @@
}
} else if (input.patch != null) {
// create a commit with the given patch.
+ if (mergeTip == null) {
+ throw new BadRequestException("Cannot apply patch on top of an empty tree.");
+ }
+ PatchApplier.Result applyResult =
+ ApplyPatchUtil.applyPatch(git, oi, input.patch, mergeTip);
+ ObjectId treeId = applyResult.getTreeId();
+ logger.atFine().log("tree ID after applying patch: %s", treeId.name());
+ String appliedPatchCommitMessage =
+ getCommitMessage(
+ ApplyPatchUtil.buildCommitMessage(
+ input.subject,
+ ImmutableList.of(),
+ input.patch,
+ ApplyPatchUtil.getResultPatch(git, reader, mergeTip, rw.lookupTree(treeId)),
+ applyResult.getErrors()),
+ me);
c =
- createCommitWithPatch(
- git, reader, oi, rw, mergeTip, input.patch, input.subject, author, committer, me);
+ rw.parseCommit(
+ CommitUtil.createCommitWithTree(
+ oi,
+ author,
+ committer,
+ ImmutableList.of(mergeTip),
+ appliedPatchCommitMessage,
+ treeId));
+ hasGitConflicts = applyResult.getErrors().stream().anyMatch(Error::isGitConflict);
} else if (commitTreeSupplier.isPresent()) {
c =
createCommitWithSuppliedTree(
@@ -525,7 +549,8 @@
opts = ImmutableList.of();
}
ChangeInfo changeInfo = jsonFactory.create(opts).format(ins.getChange());
- changeInfo.containsGitConflicts = !c.getFilesWithGitConflicts().isEmpty() ? true : null;
+ changeInfo.containsGitConflicts =
+ (!c.getFilesWithGitConflicts().isEmpty() || hasGitConflicts) ? true : null;
return changeInfo;
} catch (InvalidMergeStrategyException | MergeWithConflictsNotSupportedException e) {
throw new BadRequestException(e.getMessage());
@@ -663,43 +688,6 @@
oi, authorIdent, committerIdent, parents, commitMessage, treeId));
}
- private CodeReviewCommit createCommitWithPatch(
- Repository repo,
- ObjectReader reader,
- ObjectInserter oi,
- CodeReviewRevWalk rw,
- RevCommit mergeTip,
- ApplyPatchInput patch,
- String subject,
- PersonIdent authorIdent,
- PersonIdent committerIdent,
- IdentifiedUser me)
- throws IOException, RestApiException {
- if (mergeTip == null) {
- throw new BadRequestException("Cannot apply patch on top of an empty tree.");
- }
- PatchApplier.Result applyResult = ApplyPatchUtil.applyPatch(repo, oi, patch, mergeTip);
- ObjectId treeId = applyResult.getTreeId();
- logger.atFine().log("tree ID after applying patch: %s", treeId.name());
- String appliedPatchCommitMessage =
- getCommitMessage(
- ApplyPatchUtil.buildCommitMessage(
- subject,
- ImmutableList.of(),
- patch,
- ApplyPatchUtil.getResultPatch(repo, reader, mergeTip, rw.lookupTree(treeId)),
- applyResult.getErrors()),
- me);
- return rw.parseCommit(
- CommitUtil.createCommitWithTree(
- oi,
- authorIdent,
- committerIdent,
- ImmutableList.of(mergeTip),
- appliedPatchCommitMessage,
- treeId));
- }
-
private static CodeReviewCommit createCommitWithSuppliedTree(
Repository repo,
ObjectInserter oi,
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ApplyPatchIT.java b/javatests/com/google/gerrit/acceptance/api/change/ApplyPatchIT.java
index 2245eb7..eff783e 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ApplyPatchIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ApplyPatchIT.java
@@ -389,8 +389,9 @@
+ ">>>>>>> PATCH");
assertThat(gApi.changes().id(result.id).current().commit(false).message)
.contains(
- "NOTE FOR REVIEWERS - conflicts or errors occurred while applying the patch.\n"
- + "PLEASE REVIEW CAREFULLY.");
+ "ATTENTION: Conflicts occurred while applying the patch.\n"
+ + "Please resolve conflict markers.");
+ assertThat(result.containsGitConflicts).isTrue();
}
@Test
