Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

pr: detect clean backports #885

Closed
wants to merge 2 commits into from
Closed

pr: detect clean backports #885

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c33ec6f
pr: detect clean backports
edvbld Oct 12, 2020
49542d6
Fix spelling
edvbld Oct 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 126 additions & 1 deletion bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import java.util.*;
import java.util.logging.Logger;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.*;

class CheckRun {
Expand All @@ -60,6 +61,7 @@ class CheckRun {
private static final String emptyPrBodyMarker = "<!--\nReplace this text with a description of your pull request (also remove the surrounding HTML comment markers).\n" +
"If in doubt, feel free to delete everything in this edit box first, the bot will restore the progress section as needed.\n-->";
private static final String fullNameWarningMarker = "<!-- PullRequestBot full name warning comment -->";
private static final Pattern BACKPORT_PATTERN = Pattern.compile("<!-- backport ([0-9a-z]{40}) -->");
private final Set<String> newLabels;

private Duration expiresIn;
Expand Down Expand Up @@ -271,6 +273,116 @@ private void updateReadyForReview(PullRequestCheckIssueVisitor visitor, List<Str
}
}

private boolean updateClean(Hash hash) {
var result = pr.repository().forge().search(hash);
if (result.isEmpty()) {
throw new IllegalStateException("Backport comment for PR " + pr.id() + " contains bad hash: " + hash.hex());
}

var hasCleanLabel = labels.contains("clean");

var commit = result.get();
var originalPatches = new HashMap<String, Patch>();
for (var patch : commit.parentDiffs().get(0).patches()) {
originalPatches.put(patch.toString(), patch);
}
var prPatches = new HashMap<String, Patch>();
for (var patch : pr.diff().patches()) {
prPatches.put(patch.toString(), patch);
}

if (originalPatches.size() != prPatches.size()) {
if (hasCleanLabel) {
pr.removeLabel("clean");
}
return false;
}

var descriptions = new HashSet<>(originalPatches.keySet());
descriptions.removeAll(prPatches.keySet());
if (!descriptions.isEmpty()) {
if (hasCleanLabel) {
pr.removeLabel("clean");
}
return false;
}

for (var desc : originalPatches.keySet()) {
var original = originalPatches.get(desc).asTextualPatch();
var backport = prPatches.get(desc).asTextualPatch();
if (original.hunks().size() != backport.hunks().size()) {
if (hasCleanLabel) {
pr.removeLabel("clean");
}
return false;
}
if (original.additions() != backport.additions()) {
if (hasCleanLabel) {
pr.removeLabel("clean");
}
return false;
}
if (original.deletions() != backport.deletions()) {
if (hasCleanLabel) {
pr.removeLabel("clean");
}
return false;
}
for (var i = 0; i < original.hunks().size(); i++) {
var originalHunk = original.hunks().get(i);
var backportHunk = backport.hunks().get(i);

if (originalHunk.source().lines().size() != backportHunk.source().lines().size()) {
if (hasCleanLabel) {
pr.removeLabel("clean");
}
return false;
}
var sourceLines = new HashSet<>(originalHunk.source().lines());
sourceLines.removeAll(backportHunk.source().lines());
if (!sourceLines.isEmpty()) {
if (hasCleanLabel) {
pr.removeLabel("clean");
}
return false;
}

if (originalHunk.target().lines().size() != backportHunk.target().lines().size()) {
if (hasCleanLabel) {
pr.removeLabel("clean");
}
return false;
}
var targetLines = new HashSet<>(originalHunk.target().lines());
targetLines.removeAll(backportHunk.target().lines());
if (!targetLines.isEmpty()) {
if (hasCleanLabel) {
pr.removeLabel("clean");
}
return false;
}
}
}

if (!hasCleanLabel) {
pr.addLabel("clean");
}
return true;
}

private Optional<Hash> backportedFrom() {
var botUser = pr.repository().forge().currentUser();
var backportLines = pr.comments()
.stream()
.filter(c -> c.author().equals(botUser))
.flatMap(c -> Stream.of(c.body().split("\n")))
.map(l -> BACKPORT_PATTERN.matcher(l))
.filter(Matcher::find)
.collect(Collectors.toList());
return backportLines.isEmpty()?
Optional.empty() : Optional.of(new Hash(backportLines.get(0).group(1)));
}

private String getRole(String username) {
var project = censusInstance.project();
var version = censusInstance.census().version().format();
Expand Down Expand Up @@ -846,6 +958,8 @@ private void checkStatus() {
List<String> additionalErrors = List.of();
Hash localHash;
try {
// Do not pass eventual original commit even for backports since it will cause
// the reviwer check to be ignored.
edvbld marked this conversation as resolved.
Show resolved Hide resolved
localHash = checkablePullRequest.commit(commitHash, censusInstance.namespace(), censusDomain, null, null);
} catch (CommitFailure e) {
additionalErrors = List.of(e.getMessage());
Expand All @@ -864,6 +978,11 @@ private void checkStatus() {
}
updateCheckBuilder(checkBuilder, visitor, additionalErrors);
updateReadyForReview(visitor, additionalErrors);
var original = backportedFrom();
var isCleanBackport = false;
if (original.isPresent()) {
isCleanBackport = updateClean(original.get());
}

var integrationBlockers = botSpecificIntegrationBlockers();

Expand All @@ -877,12 +996,18 @@ private void checkStatus() {
updateReviewedMessages(comments, allReviews);
}

var amendedHash = checkablePullRequest.amendManualReviewers(localHash, censusInstance.namespace());
var amendedHash = checkablePullRequest.amendManualReviewers(localHash, censusInstance.namespace(), original.orElse(null));
var commit = localRepo.lookup(amendedHash).orElseThrow();
var commitMessage = String.join("\n", commit.message());
var readyForIntegration = visitor.messages().isEmpty() &&
additionalErrors.isEmpty() &&
integrationBlockers.isEmpty();
if (isCleanBackport) {
// Reviews are not needed for clean backports
readyForIntegration = visitor.isReadyForReview() &&
additionalErrors.isEmpty() &&
integrationBlockers.isEmpty();
}

updateMergeReadyComment(readyForIntegration, commitMessage, comments, activeReviews, rebasePossible);
if (readyForIntegration && rebasePossible) {
Expand Down
6 changes: 3 additions & 3 deletions bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckablePullRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,10 @@ Hash commit(Hash finalHead, Namespace namespace, String censusDomain, String spo
return PullRequestUtils.createCommit(pr, localRepo, finalHead, author, committer, commitMessage);
}

Hash amendManualReviewers(Hash commit, Namespace namespace) throws IOException {
Hash amendManualReviewers(Hash commit, Namespace namespace, Hash original) throws IOException {
var activeReviews = filterActiveReviews(pr.reviews());
var originalCommitMessage = commitMessage(activeReviews, namespace, false);
var amendedCommitMessage = commitMessage(activeReviews, namespace, true);
var originalCommitMessage = commitMessage(activeReviews, namespace, false, original);
var amendedCommitMessage = commitMessage(activeReviews, namespace, true, original);

if (originalCommitMessage.equals(amendedCommitMessage)) {
return commit;
Expand Down
2 changes: 1 addition & 1 deletion bots/pr/src/main/java/org/openjdk/skara/bots/pr/IntegrateCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInst

// Rebase and push it!
if (!localHash.equals(pr.targetHash())) {
var amendedHash = checkablePr.amendManualReviewers(localHash, censusInstance.namespace());
var amendedHash = checkablePr.amendManualReviewers(localHash, censusInstance.namespace(), original);
var finalRebaseMessage = rebaseMessage.toString();
if (!finalRebaseMessage.isBlank()) {
reply.println(rebaseMessage.toString());
Expand Down
4 changes: 2 additions & 2 deletions bots/pr/src/main/java/org/openjdk/skara/bots/pr/SponsorCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@

public class SponsorCommand implements CommandHandler {
private final Logger log = Logger.getLogger("org.openjdk.skara.bots.pr");
private static final Pattern BACKPORT_PATTERN = Pattern.compile("<-- backport ([0-9a-z]{40}) -->");
private static final Pattern BACKPORT_PATTERN = Pattern.compile("<!-- backport ([0-9a-z]{40}) -->");

@Override
public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInstance, Path scratchPath, CommandInvocation command, List<Comment> allComments, PrintWriter reply) {
Expand Down Expand Up @@ -137,7 +137,7 @@ public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInst
}

if (!localHash.equals(pr.targetHash())) {
var amendedHash = checkablePr.amendManualReviewers(localHash, censusInstance.namespace());
var amendedHash = checkablePr.amendManualReviewers(localHash, censusInstance.namespace(), original);
var finalRebaseMessage = rebaseMessage.toString();
if (!finalRebaseMessage.isBlank()) {
reply.println(rebaseMessage.toString());
Expand Down
Loading