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