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.

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

pr: detect clean backports #885

Closed
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -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 {
@@ -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;
@@ -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();
@@ -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 reviewer check to be ignored.
localHash = checkablePullRequest.commit(commitHash, censusInstance.namespace(), censusDomain, null, null);
} catch (CommitFailure e) {
additionalErrors = List.of(e.getMessage());
@@ -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();

@@ -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) {
@@ -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;
@@ -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());
@@ -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) {
@@ -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());