diff --git a/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveItem.java b/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveItem.java index 6ad9f074a..bfd8a53d5 100644 --- a/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveItem.java +++ b/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveItem.java @@ -5,7 +5,7 @@ import org.openjdk.skara.issuetracker.Comment; import org.openjdk.skara.vcs.*; -import java.io.IOException; +import java.io.*; import java.net.URI; import java.time.ZonedDateTime; import java.util.*; @@ -38,10 +38,12 @@ private ArchiveItem(ArchiveItem parent, String id, ZonedDateTime created, ZonedD this.footer = footer; } - private static Optional mergeCommit(Repository localRepo, Hash head) { + private static Optional mergeCommit(PullRequest pr, Repository localRepo, Hash head) { try { - return localRepo.lookup(head).filter(Commit::isMerge); - } catch (IOException e) { + var author = new Author("duke", "duke@openjdk.org"); + var hash = PullRequestUtils.createCommit(pr, localRepo, head, author, author, pr.title()); + return localRepo.lookup(hash); + } catch (IOException | CommitFailure e) { return Optional.empty(); } } @@ -51,39 +53,45 @@ static ArchiveItem from(PullRequest pr, Repository localRepo, HostUserToEmailAut WebrevNotification webrevNotification, ZonedDateTime created, ZonedDateTime updated, Hash base, Hash head, String subjectPrefix, String threadPrefix) { return new ArchiveItem(null, "fc", created, updated, pr.author(), Map.of("PR-Head-Hash", head.hex(), - "PR-Base-Hash", base.hex(), - "PR-Thread-Prefix", threadPrefix), + "PR-Base-Hash", base.hex(), + "PR-Thread-Prefix", threadPrefix), () -> subjectPrefix + threadPrefix + (threadPrefix.isEmpty() ? "" : ": ") + pr.title(), () -> "", - () -> ArchiveMessages.composeConversation(pr, localRepo, base, head), + () -> ArchiveMessages.composeConversation(pr), () -> { - var fullWebrev = webrevGenerator.generate(base, head, "00", WebrevDescription.Type.FULL); - if (pr.title().startsWith("Merge")) { - var mergeCommit = mergeCommit(localRepo, head); - if (mergeCommit.isPresent()) { - var mergeWebrevs = new ArrayList(); - mergeWebrevs.add(fullWebrev); - for (int i = 0; i < mergeCommit.get().parentDiffs().size(); ++i) { - var diff = mergeCommit.get().parentDiffs().get(i); - switch (i) { - case 0: - mergeWebrevs.add(webrevGenerator.generate(diff, String.format("00.%d", i), WebrevDescription.Type.MERGE_TARGET, pr.targetRef())); - break; - case 1: - var mergeSource = pr.title().length() > 6 ? pr.title().substring(6) : null; - mergeWebrevs.add(webrevGenerator.generate(diff, String.format("00.%d", i), WebrevDescription.Type.MERGE_SOURCE, mergeSource)); - break; - default: - mergeWebrevs.add(webrevGenerator.generate(diff, String.format("00.%d", i), WebrevDescription.Type.MERGE_SOURCE, null)); - break; - } - } - webrevNotification.notify(0, mergeWebrevs); - return ArchiveMessages.composeMergeConversationFooter(pr, localRepo, mergeWebrevs, base, head); - } + if (PullRequestUtils.isMerge(pr)) { + //TODO: Try to merge in target - generate possible conflict webrev + var mergeCommit = mergeCommit(pr, localRepo, head); + var mergeWebrevs = new ArrayList(); + if (mergeCommit.isPresent()) { + for (int i = 0; i < mergeCommit.get().parentDiffs().size(); ++i) { + var diff = mergeCommit.get().parentDiffs().get(i); + if (diff.patches().size() == 0) { + continue; + } + switch (i) { + case 0: + mergeWebrevs.add(webrevGenerator.generate(diff, String.format("00.%d", i), WebrevDescription.Type.MERGE_TARGET, pr.targetRef())); + break; + case 1: + var mergeSource = pr.title().length() > 6 ? pr.title().substring(6) : null; + mergeWebrevs.add(webrevGenerator.generate(diff, String.format("00.%d", i), WebrevDescription.Type.MERGE_SOURCE, mergeSource)); + break; + default: + mergeWebrevs.add(webrevGenerator.generate(diff, String.format("00.%d", i), WebrevDescription.Type.MERGE_SOURCE, null)); + break; + } + } + if (!mergeWebrevs.isEmpty()) { + webrevNotification.notify(0, mergeWebrevs); + } + } + return ArchiveMessages.composeMergeConversationFooter(pr, localRepo, mergeWebrevs, base, head); + } else { + var fullWebrev = webrevGenerator.generate(base, head, "00", WebrevDescription.Type.FULL); + webrevNotification.notify(0, List.of(fullWebrev)); + return ArchiveMessages.composeConversationFooter(pr, issueTracker, issuePrefix, localRepo, fullWebrev, base, head); } - webrevNotification.notify(0, List.of(fullWebrev)); - return ArchiveMessages.composeConversationFooter(pr, issueTracker, issuePrefix, localRepo, fullWebrev, base, head); }); } diff --git a/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveMessages.java b/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveMessages.java index 06d34adef..b98b65ae4 100644 --- a/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveMessages.java +++ b/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveMessages.java @@ -111,12 +111,21 @@ private static Optional formatCommitMessagesFull(List co } private static Optional formatCommitMessagesBrief(List commits) { + return formatCommitMessagesBrief(commits, 100); + } + + private static Optional formatCommitMessagesBrief(List commits, int maxEntries) { if (commits.size() == 0) { return Optional.empty(); } else { - return Optional.of(commits.stream() - .map(ArchiveMessages::formatCommitBrief) - .collect(Collectors.joining("\n"))); + var commitSummary = commits.stream() + .limit(maxEntries) + .map(ArchiveMessages::formatCommitBrief) + .collect(Collectors.joining("\n")); + if (commits.size() > maxEntries) { + commitSummary += "\n - ...omitting " + (commits.size() - maxEntries) + " further commits."; + } + return Optional.of(commitSummary); } } @@ -151,7 +160,7 @@ private static String fetchCommand(PullRequest pr) { return "git fetch " + repoUrl + " " + pr.fetchRef() + ":pull/" + pr.id(); } - static String composeConversation(PullRequest pr, Repository localRepo, Hash base, Hash head) { + static String composeConversation(PullRequest pr) { var filteredBody = filterComments(pr.body()); if (filteredBody.isEmpty()) { filteredBody = pr.title().strip(); @@ -254,22 +263,20 @@ static String composeConversationFooter(PullRequest pr, URI issueProject, String static String composeMergeConversationFooter(PullRequest pr, Repository localRepo, List webrevs, Hash base, Hash head) { var commits = commits(localRepo, base, head); - var webrevLinks = ""; - if (webrevs.size() > 1) { - webrevLinks = " Webrev: " + webrevs.get(0).uri() + "\n\n" + - "The following webrevs contain only the adjustments done while merging with regards to each parent branch:\n" + + String webrevLinks; + if (webrevs.size() > 0) { + webrevLinks = "The webrev" + (webrevs.size() > 1 ? "s" : "") + " contain" + (webrevs.size() == 1 ? "s" : "") + + " only the adjustments done while merging with regards to each parent branch:\n" + webrevs.stream() - .skip(1) .map(d -> String.format(" - %s: %s", d.shortLabel(), d.uri())) .collect(Collectors.joining("\n")) + "\n\n"; } else { - webrevLinks = " Webrev: " + webrevs.get(0).uri() + "\n\n" + - "The merge commit only contains trivial merges, so no merge-specific webrevs have been generated.\n\n"; + webrevLinks = "The merge commit only contains trivial merges, so no merge-specific webrevs have been generated.\n\n"; } return "Commit messages:\n" + - formatCommitMessagesBrief(commits).orElse("") + "\n\n" + - "Changes: " + pr.changeUrl() + "\n" + + formatCommitMessagesBrief(commits, 10).orElse("") + "\n\n" + webrevLinks + + "Changes: " + pr.changeUrl() + "\n" + " Stats: " + stats(localRepo, base, head) + "\n" + " Patch: " + pr.diffUrl().toString() + "\n" + " Fetch: " + fetchCommand(pr) + "\n\n" + diff --git a/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveWorkItem.java b/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveWorkItem.java index 4415d9288..2a75bbc3c 100644 --- a/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveWorkItem.java +++ b/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveWorkItem.java @@ -292,20 +292,14 @@ public void run(Path scratchPath) { // Materialize the PR's source and target ref var seedPath = bot.seedStorage().orElse(scratchPath.resolve("seeds")); var hostedRepositoryPool = new HostedRepositoryPool(seedPath); - var repository = pr.repository(); - var localRepoPath = scratchPath.resolve("mlbridge-mergebase"); - var localRepo = hostedRepositoryPool.checkout(pr, localRepoPath.resolve(repository.name())); - localRepo.fetch(repository.url(), "+" + pr.targetRef() + ":mlbridge_prinstance", false); - - var targetHash = pr.targetHash(); - var headHash = pr.headHash(); - var baseHash = localRepo.mergeBase(targetHash, headHash); + var localRepoPath = scratchPath.resolve("mlbridge-mergebase").resolve(pr.repository().name()); + var localRepo = PullRequestUtils.materialize(hostedRepositoryPool, pr, localRepoPath); var webrevPath = scratchPath.resolve("mlbridge-webrevs"); var listServer = MailingListServerFactory.createMailmanServer(bot.listArchive(), bot.smtpServer(), bot.sendInterval()); var list = listServer.getList(bot.listAddress().address()); - var archiver = new ReviewArchive(pr, bot.emailAddress(), baseHash, headHash); + var archiver = new ReviewArchive(pr, bot.emailAddress()); // Regular comments for (var comment : comments) { diff --git a/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ReviewArchive.java b/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ReviewArchive.java index 80dc3ed2f..4a37e659c 100644 --- a/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ReviewArchive.java +++ b/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ReviewArchive.java @@ -6,6 +6,7 @@ import org.openjdk.skara.issuetracker.*; import org.openjdk.skara.vcs.*; +import java.io.IOException; import java.net.URI; import java.nio.charset.StandardCharsets; import java.security.*; @@ -18,8 +19,6 @@ class ReviewArchive { private final PullRequest pr; private final EmailAddress sender; - private final Hash base; - private final Hash head; private final List comments = new ArrayList<>(); private final List reviews = new ArrayList<>(); @@ -27,11 +26,9 @@ class ReviewArchive { private final Logger log = Logger.getLogger("org.openjdk.skara.bots.mlbridge"); - ReviewArchive(PullRequest pr, EmailAddress sender, Hash base, Hash head) { + ReviewArchive(PullRequest pr, EmailAddress sender) { this.pr = pr; this.sender = sender; - this.base = base; - this.head = head; } void addComment(Comment comment) { @@ -55,7 +52,7 @@ private Optional findPreviousReplyBy(List generated, H .findAny(); } - private List generateArchiveItems(List sentEmails, Repository localRepo, URI issueTracker, String issuePrefix, HostUserToEmailAuthor hostUserToEmailAuthor, HostUserToUserName hostUserToUserName, HostUserToRole hostUserToRole, WebrevStorage.WebrevGenerator webrevGenerator, WebrevNotification webrevNotification, String subjectPrefix) { + private List generateArchiveItems(List sentEmails, Repository localRepo, URI issueTracker, String issuePrefix, HostUserToEmailAuthor hostUserToEmailAuthor, HostUserToUserName hostUserToUserName, HostUserToRole hostUserToRole, WebrevStorage.WebrevGenerator webrevGenerator, WebrevNotification webrevNotification, String subjectPrefix) throws IOException { var generated = new ArrayList(); Hash lastBase = null; Hash lastHead = null; @@ -96,12 +93,13 @@ private List generateArchiveItems(List sentEmails, Repositor } // Check if we're at a revision not previously reported - if (!base.equals(lastBase) || !head.equals(lastHead)) { + var baseHash = PullRequestUtils.baseHash(pr, localRepo); + if (!baseHash.equals(lastBase) || !pr.headHash().equals(lastHead)) { if (generated.isEmpty()) { - var first = ArchiveItem.from(pr, localRepo, hostUserToEmailAuthor, issueTracker, issuePrefix, webrevGenerator, webrevNotification, pr.createdAt(), pr.updatedAt(), base, head, subjectPrefix, threadPrefix); + var first = ArchiveItem.from(pr, localRepo, hostUserToEmailAuthor, issueTracker, issuePrefix, webrevGenerator, webrevNotification, pr.createdAt(), pr.updatedAt(), baseHash, pr.headHash(), subjectPrefix, threadPrefix); generated.add(first); } else { - var revision = ArchiveItem.from(pr, localRepo, hostUserToEmailAuthor, webrevGenerator, webrevNotification, pr.updatedAt(), pr.updatedAt(), lastBase, lastHead, base, head, ++revisionIndex, generated.get(0), subjectPrefix, threadPrefix); + var revision = ArchiveItem.from(pr, localRepo, hostUserToEmailAuthor, webrevGenerator, webrevNotification, pr.updatedAt(), pr.updatedAt(), lastBase, lastHead, baseHash, pr.headHash(), ++revisionIndex, generated.get(0), subjectPrefix, threadPrefix); generated.add(revision); } } @@ -230,7 +228,7 @@ private String getStableMessageId(EmailAddress uniqueMessageId) { return uniqueMessageId.localPart().split("\\.")[0]; } - List generateNewEmails(List sentEmails, Duration cooldown, Repository localRepo, URI issueTracker, String issuePrefix, WebrevStorage.WebrevGenerator webrevGenerator, WebrevNotification webrevNotification, HostUserToEmailAuthor hostUserToEmailAuthor, HostUserToUserName hostUserToUserName, HostUserToRole hostUserToRole, String subjectPrefix, Consumer retryConsumer) { + List generateNewEmails(List sentEmails, Duration cooldown, Repository localRepo, URI issueTracker, String issuePrefix, WebrevStorage.WebrevGenerator webrevGenerator, WebrevNotification webrevNotification, HostUserToEmailAuthor hostUserToEmailAuthor, HostUserToUserName hostUserToUserName, HostUserToRole hostUserToRole, String subjectPrefix, Consumer retryConsumer) throws IOException { var ret = new ArrayList(); var allItems = generateArchiveItems(sentEmails, localRepo, issueTracker, issuePrefix, hostUserToEmailAuthor, hostUserToUserName, hostUserToRole, webrevGenerator, webrevNotification, subjectPrefix); var sentItemIds = sentItemIds(sentEmails); diff --git a/bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotTests.java b/bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotTests.java index 03c2fd689..2e8b8a4c8 100644 --- a/bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotTests.java +++ b/bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotTests.java @@ -1731,6 +1731,144 @@ void mergeWebrev(TestInfo testInfo) throws IOException { } } + @Test + void mergeWebrevNoConflict(TestInfo testInfo) throws IOException { + try (var credentials = new HostCredentials(testInfo); + var tempFolder = new TemporaryDirectory(); + var archiveFolder = new TemporaryDirectory(); + var listServer = new TestMailmanServer(); + var webrevServer = new TestWebrevServer()) { + var author = credentials.getHostedRepository(); + var archive = credentials.getHostedRepository(); + var commenter = credentials.getHostedRepository(); + var listAddress = EmailAddress.parse(listServer.createList("test")); + var censusBuilder = credentials.getCensusBuilder() + .addAuthor(author.forge().currentUser().id()); + var from = EmailAddress.from("test", "test@test.mail"); + var mlBot = MailingListBridgeBot.newBuilder() + .from(from) + .repo(author) + .archive(archive) + .archiveRef("archive") + .censusRepo(censusBuilder.build()) + .list(listAddress) + .listArchive(listServer.getArchive()) + .smtpServer(listServer.getSMTP()) + .webrevStorageRepository(archive) + .webrevStorageRef("webrev") + .webrevStorageBase(Path.of("test")) + .webrevStorageBaseUri(webrevServer.uri()) + .issueTracker(URIBuilder.base("http://issues.test/browse/").build()) + .build(); + + // Populate the projects repository + var reviewFile = Path.of("reviewfile.txt"); + var localRepo = CheckableRepository.init(tempFolder.path(), author.repositoryType(), reviewFile); + var masterHash = localRepo.resolve("master").orElseThrow(); + localRepo.push(masterHash, author.url(), "master", true); + localRepo.push(masterHash, archive.url(), "archive", true); + localRepo.push(masterHash, archive.url(), "webrev", true); + + // Create a merge + var editOnlyFile = Path.of("editonly.txt"); + Files.writeString(localRepo.root().resolve(editOnlyFile), "Only added in the edit"); + localRepo.add(editOnlyFile); + var editHash = CheckableRepository.appendAndCommit(localRepo, "Edited", "Commit in edit branch"); + localRepo.checkout(masterHash, true); + var masterOnlyFile = Path.of("masteronly.txt"); + Files.writeString(localRepo.root().resolve(masterOnlyFile), "Only added in master"); + localRepo.add(masterOnlyFile); + var updatedMasterHash = localRepo.commit("Only added in master", "duke", "duke@openjdk.java.net"); + localRepo.push(updatedMasterHash, author.url(), "master"); + localRepo.merge(editHash); + var mergeCommit = localRepo.commit("Merged edit", "duke", "duke@openjdk.java.net"); + localRepo.push(mergeCommit, author.url(), "edit", true); + + // Make a merge PR + var pr = credentials.createPullRequest(archive, "master", "edit", "Merge edit"); + pr.setBody("This is now ready"); + + // Run an archive pass + TestBotRunner.runPeriodicItems(mlBot); + listServer.processIncoming(); + + // The archive should contain a merge style webrev + Repository.materialize(archiveFolder.path(), archive.url(), "archive"); + assertTrue(archiveContains(archiveFolder.path(), "so no merge-specific webrevs have been generated")); + + // The PR should not contain a webrev comment + assertEquals(0, pr.comments().size()); + } + } + + @Test + void mergeWebrevNoMerge(TestInfo testInfo) throws IOException { + try (var credentials = new HostCredentials(testInfo); + var tempFolder = new TemporaryDirectory(); + var archiveFolder = new TemporaryDirectory(); + var listServer = new TestMailmanServer(); + var webrevServer = new TestWebrevServer()) { + var author = credentials.getHostedRepository(); + var archive = credentials.getHostedRepository(); + var commenter = credentials.getHostedRepository(); + var listAddress = EmailAddress.parse(listServer.createList("test")); + var censusBuilder = credentials.getCensusBuilder() + .addAuthor(author.forge().currentUser().id()); + var from = EmailAddress.from("test", "test@test.mail"); + var mlBot = MailingListBridgeBot.newBuilder() + .from(from) + .repo(author) + .archive(archive) + .archiveRef("archive") + .censusRepo(censusBuilder.build()) + .list(listAddress) + .listArchive(listServer.getArchive()) + .smtpServer(listServer.getSMTP()) + .webrevStorageRepository(archive) + .webrevStorageRef("webrev") + .webrevStorageBase(Path.of("test")) + .webrevStorageBaseUri(webrevServer.uri()) + .issueTracker(URIBuilder.base("http://issues.test/browse/").build()) + .build(); + + // Populate the projects repository + var reviewFile = Path.of("reviewfile.txt"); + var localRepo = CheckableRepository.init(tempFolder.path(), author.repositoryType(), reviewFile); + var masterHash = localRepo.resolve("master").orElseThrow(); + localRepo.push(masterHash, author.url(), "master", true); + localRepo.push(masterHash, archive.url(), "archive", true); + localRepo.push(masterHash, archive.url(), "webrev", true); + + // Create a merge + var editOnlyFile = Path.of("editonly.txt"); + Files.writeString(localRepo.root().resolve(editOnlyFile), "Only added in the edit"); + localRepo.add(editOnlyFile); + var editHash = CheckableRepository.appendAndCommit(localRepo, "Edited", "Commit in edit branch"); + localRepo.checkout(masterHash, true); + var masterOnlyFile = Path.of("masteronly.txt"); + Files.writeString(localRepo.root().resolve(masterOnlyFile), "Only added in master"); + localRepo.add(masterOnlyFile); + var updatedMasterHash = CheckableRepository.appendAndCommit(localRepo, "Master change", "Commit in master branch"); + localRepo.push(updatedMasterHash, author.url(), "master"); + localRepo.push(editHash, author.url(), "edit", true); + + // Make a merge PR + var pr = credentials.createPullRequest(archive, "master", "edit", "Merge edit"); + pr.setBody("This is now ready"); + + // Run an archive pass + TestBotRunner.runPeriodicItems(mlBot); + listServer.processIncoming(); + + // The archive should not include any merge-specific webrevs + Repository.materialize(archiveFolder.path(), archive.url(), "archive"); + assertTrue(archiveContains(archiveFolder.path(), "so no merge-specific webrevs have been generated")); + + // The PR should not contain a webrev comment + assertEquals(0, pr.comments().size()); + } + } + @Test void skipAddingExistingWebrev(TestInfo testInfo) throws IOException { try (var credentials = new HostCredentials(testInfo); diff --git a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java index 9516624be..7ed0a31bb 100644 --- a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java +++ b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java @@ -39,7 +39,7 @@ class CheckRun { private final CheckWorkItem workItem; private final PullRequest pr; - private final PullRequestInstance prInstance; + private final Repository localRepo; private final List comments; private final List allReviews; private final List activeReviews; @@ -47,6 +47,9 @@ class CheckRun { private final CensusInstance censusInstance; private final boolean ignoreStaleReviews; + private final Hash baseHash; + private final CheckablePullRequest checkablePullRequest; + private final Logger log = Logger.getLogger("org.openjdk.skara.bots.pr"); private final String progressMarker = ""; private final String mergeReadyMarker = ""; @@ -54,12 +57,12 @@ class CheckRun { private final String sourceBranchWarningMarker = ""; private final Set newLabels; - private CheckRun(CheckWorkItem workItem, PullRequest pr, PullRequestInstance prInstance, List comments, + private CheckRun(CheckWorkItem workItem, PullRequest pr, Repository localRepo, List comments, List allReviews, List activeReviews, Set labels, - CensusInstance censusInstance, boolean ignoreStaleReviews) { + CensusInstance censusInstance, boolean ignoreStaleReviews) throws IOException { this.workItem = workItem; this.pr = pr; - this.prInstance = prInstance; + this.localRepo = localRepo; this.comments = comments; this.allReviews = allReviews; this.activeReviews = activeReviews; @@ -67,12 +70,15 @@ private CheckRun(CheckWorkItem workItem, PullRequest pr, PullRequestInstance prI this.newLabels = new HashSet<>(labels); this.censusInstance = censusInstance; this.ignoreStaleReviews = ignoreStaleReviews; + + baseHash = PullRequestUtils.baseHash(pr, localRepo); + checkablePullRequest = new CheckablePullRequest(pr, localRepo, ignoreStaleReviews); } - static void execute(CheckWorkItem workItem, PullRequest pr, PullRequestInstance prInstance, List comments, + static void execute(CheckWorkItem workItem, PullRequest pr, Repository localRepo, List comments, List allReviews, List activeReviews, Set labels, CensusInstance censusInstance, - boolean ignoreStaleReviews) { - var run = new CheckRun(workItem, pr, prInstance, comments, allReviews, activeReviews, labels, censusInstance, ignoreStaleReviews); + boolean ignoreStaleReviews) throws IOException { + var run = new CheckRun(workItem, pr, localRepo, comments, allReviews, activeReviews, labels, censusInstance, ignoreStaleReviews); run.checkStatus(); } @@ -127,9 +133,8 @@ private List botSpecificChecks(Hash finalHash) throws IOException { ret.add(error); } - var baseHash = prInstance.baseHash(); var headHash = pr.headHash(); - var originalCommits = prInstance.localRepo().commitMetadata(baseHash, headHash); + var originalCommits = localRepo.commitMetadata(baseHash, headHash); if (!checkCommitAuthor(originalCommits)) { var error = "For contributors who are not existing OpenJDK Authors, commit attribution will be taken from " + @@ -401,7 +406,7 @@ private String getMergeReadyComment(String commitMessage, List reviews) try { var hasContributingFile = - !prInstance.localRepo().files(prInstance.targetHash(), Path.of("CONTRIBUTING.md")).isEmpty(); + !localRepo.files(pr.targetHash(), Path.of("CONTRIBUTING.md")).isEmpty(); if (hasContributingFile) { message.append(". When the change also fulfills all "); message.append("[project specific requirements](https://github.com/"); @@ -424,7 +429,7 @@ private String getMergeReadyComment(String commitMessage, List reviews) message.append("- To credit additional contributors, use the `/contributor` command.\n"); message.append("- To add additional solved issues, use the `/solves` command.\n"); - var divergingCommits = prInstance.divergingCommits(); + var divergingCommits = checkablePullRequest.divergingCommits(); if (divergingCommits.size() > 0) { message.append("\n"); message.append("Since the source branch of this PR was last updated there "); @@ -443,7 +448,7 @@ private String getMergeReadyComment(String commitMessage, List reviews) message.append(pr.targetRef()); message.append("` into your branch, and then specify the current head hash when integrating, like this: "); message.append("`/integrate "); - message.append(prInstance.targetHash()); + message.append(pr.targetHash()); message.append("`.\n"); } else { message.append("\n"); @@ -453,7 +458,7 @@ private String getMergeReadyComment(String commitMessage, List reviews) message.append("you perform the `/integrate` command, your PR will be automatically rebased. If you would like to avoid "); message.append("potential automatic rebasing, specify the current head hash when integrating, like this: "); message.append("`/integrate "); - message.append(prInstance.targetHash()); + message.append(pr.targetHash()); message.append("`.\n"); } @@ -513,7 +518,7 @@ private void updateMergeReadyComment(boolean isReady, String commitMessage, List } } - private void addSourceBranchWarningComment(List comments) { + private void addSourceBranchWarningComment(List comments) throws IOException { var existing = findComment(comments, sourceBranchWarningMarker); if (existing.isPresent()) { // Only add the comment once per PR @@ -534,7 +539,7 @@ private void addSourceBranchWarningComment(List comments) { "```" + "$ git checkout " + branch + "\n" + "$ git checkout -b NEW-BRANCH-NAME\n" + - "$ git branch -f " + branch + " " + prInstance.baseHash().hex() + "\n" + + "$ git branch -f " + branch + " " + baseHash.hex() + "\n" + "$ git push -f origin " + branch + "\n" + "```\n" + "\n" + @@ -578,7 +583,7 @@ private void checkStatus() { var ignored = new PrintWriter(new StringWriter()); var rebasePossible = true; var commitHash = pr.headHash(); - var mergedHash = prInstance.mergeTarget(ignored); + var mergedHash = checkablePullRequest.mergeTarget(ignored); if (mergedHash.isPresent()) { commitHash = mergedHash.get(); } else { @@ -588,16 +593,16 @@ private void checkStatus() { List additionalErrors = List.of(); Hash localHash; try { - localHash = prInstance.commit(commitHash, censusInstance.namespace(), censusDomain, null); + localHash = checkablePullRequest.commit(commitHash, censusInstance.namespace(), censusDomain, null); } catch (CommitFailure e) { additionalErrors = List.of(e.getMessage()); - localHash = prInstance.baseHash(); + localHash = baseHash; } - PullRequestCheckIssueVisitor visitor = prInstance.createVisitor(localHash, censusInstance); - if (!localHash.equals(prInstance.baseHash())) { + PullRequestCheckIssueVisitor visitor = checkablePullRequest.createVisitor(localHash, censusInstance); + if (!localHash.equals(baseHash)) { // Determine current status - var additionalConfiguration = AdditionalConfiguration.get(prInstance.localRepo(), localHash, pr.repository().forge().currentUser(), comments); - prInstance.executeChecks(localHash, censusInstance, visitor, additionalConfiguration); + var additionalConfiguration = AdditionalConfiguration.get(localRepo, localHash, pr.repository().forge().currentUser(), comments); + checkablePullRequest.executeChecks(localHash, censusInstance, visitor, additionalConfiguration); additionalErrors = botSpecificChecks(localHash); } else { if (additionalErrors.isEmpty()) { @@ -616,7 +621,7 @@ private void checkStatus() { updateReviewedMessages(comments, allReviews); } - var commit = prInstance.localRepo().lookup(localHash).orElseThrow(); + var commit = localRepo.lookup(localHash).orElseThrow(); var commitMessage = String.join("\n", commit.message()); var readyForIntegration = visitor.getMessages().isEmpty() && additionalErrors.isEmpty(); updateMergeReadyComment(readyForIntegration, commitMessage, comments, activeReviews, rebasePossible); diff --git a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckWorkItem.java b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckWorkItem.java index 0777ce1be..75ee49310 100644 --- a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckWorkItem.java +++ b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckWorkItem.java @@ -140,7 +140,7 @@ public void run(Path scratchPath) { var labels = new HashSet<>(pr.labels()); // Filter out the active reviews - var activeReviews = PullRequestInstance.filterActiveReviews(allReviews); + var activeReviews = CheckablePullRequest.filterActiveReviews(allReviews); if (!currentCheckValid(census, comments, activeReviews, labels)) { if (labels.contains("integrated")) { log.info("Skipping check of integrated PR"); @@ -149,11 +149,11 @@ public void run(Path scratchPath) { try { var seedPath = bot.seedStorage().orElse(scratchPath.resolve("seeds")); - var prInstance = new PullRequestInstance(scratchPath.resolve("pr").resolve("check"), - new HostedRepositoryPool(seedPath), - pr, - bot.ignoreStaleReviews()); - CheckRun.execute(this, pr, prInstance, comments, allReviews, activeReviews, labels, census, bot.ignoreStaleReviews()); + var hostedRepositoryPool = new HostedRepositoryPool(seedPath); + var localRepoPath = scratchPath.resolve("pr").resolve("check").resolve(pr.repository().name()); + var localRepo = PullRequestUtils.materialize(hostedRepositoryPool, pr, localRepoPath); + + CheckRun.execute(this, pr, localRepo, comments, allReviews, activeReviews, labels, census, bot.ignoreStaleReviews()); } catch (IOException e) { throw new UncheckedIOException(e); } diff --git a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckablePullRequest.java b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckablePullRequest.java new file mode 100644 index 000000000..0d7f8c729 --- /dev/null +++ b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckablePullRequest.java @@ -0,0 +1,176 @@ +/* + * Copyright (c) 2019, 2020, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ +package org.openjdk.skara.bots.pr; + +import org.openjdk.skara.census.*; +import org.openjdk.skara.forge.*; +import org.openjdk.skara.host.HostUser; +import org.openjdk.skara.jcheck.JCheck; +import org.openjdk.skara.vcs.*; +import org.openjdk.skara.vcs.openjdk.*; + +import java.io.*; +import java.util.*; +import java.util.stream.Collectors; + +public class CheckablePullRequest { + private final PullRequest pr; + private final Repository localRepo; + private final boolean ignoreStaleReviews; + + CheckablePullRequest(PullRequest pr, Repository localRepo, boolean ignoreStaleReviews) { + this.pr = pr; + this.localRepo = localRepo; + this.ignoreStaleReviews = ignoreStaleReviews; + } + + private String commitMessage(List activeReviews, Namespace namespace) throws IOException { + var reviewers = activeReviews.stream() + .filter(review -> !ignoreStaleReviews || review.hash().equals(pr.headHash())) + .filter(review -> review.verdict() == Review.Verdict.APPROVED) + .map(review -> review.reviewer().id()) + .map(namespace::get) + .filter(Objects::nonNull) + .map(Contributor::username) + .collect(Collectors.toList()); + + var comments = pr.comments(); + var currentUser = pr.repository().forge().currentUser(); + var additionalContributors = Contributors.contributors(currentUser, + comments).stream() + .map(email -> Author.fromString(email.toString())) + .collect(Collectors.toList()); + + var additionalIssues = SolvesTracker.currentSolved(currentUser, comments); + var summary = Summary.summary(currentUser, comments); + var issue = Issue.fromString(pr.title()); + var commitMessageBuilder = issue.map(CommitMessage::title).orElseGet(() -> CommitMessage.title(pr.title())); + if (issue.isPresent()) { + commitMessageBuilder.issues(additionalIssues); + } + commitMessageBuilder.contributors(additionalContributors) + .reviewers(reviewers); + summary.ifPresent(commitMessageBuilder::summary); + + return String.join("\n", commitMessageBuilder.format(CommitMessageFormatters.v1)); + } + + /** + * The Review list is in chronological order, the latest one from a particular reviewer is the + * one that is "active". + * @param allReviews + * @return + */ + static List filterActiveReviews(List allReviews) { + var reviewPerUser = new LinkedHashMap(); + for (var review : allReviews) { + reviewPerUser.put(review.reviewer(), review); + } + return new ArrayList<>(reviewPerUser.values()); + } + + Hash commit(Hash finalHead, Namespace namespace, String censusDomain, String sponsorId) throws IOException, CommitFailure { + Author committer; + Author author; + var contributor = namespace.get(pr.author().id()); + + if (contributor == null) { + if (PullRequestUtils.isMerge(pr)) { + throw new CommitFailure("Merges can only be performed by Committers."); + } + + // Use the information contained in the head commit - jcheck has verified that it contains sane values + var headCommit = localRepo.commitMetadata(pr.headHash().hex() + "^.." + pr.headHash().hex()).get(0); + author = headCommit.author(); + } else { + author = new Author(contributor.fullName().orElseThrow(), contributor.username() + "@" + censusDomain); + } + + if (sponsorId != null) { + var sponsorContributor = namespace.get(sponsorId); + committer = new Author(sponsorContributor.fullName().orElseThrow(), sponsorContributor.username() + "@" + censusDomain); + } else { + committer = author; + } + + var activeReviews = filterActiveReviews(pr.reviews()); + var commitMessage = commitMessage(activeReviews, namespace); + return PullRequestUtils.createCommit(pr, localRepo, finalHead, author, committer, commitMessage); + } + + PullRequestCheckIssueVisitor createVisitor(Hash localHash, CensusInstance censusInstance) throws IOException { + var checks = JCheck.checksFor(localRepo, censusInstance.census(), pr.targetHash()); + return new PullRequestCheckIssueVisitor(checks); + } + + void executeChecks(Hash localHash, CensusInstance censusInstance, PullRequestCheckIssueVisitor visitor, List additionalConfiguration) throws Exception { + try (var issues = JCheck.check(localRepo, censusInstance.census(), CommitMessageParsers.v1, localHash, + pr.targetHash(), additionalConfiguration)) { + for (var issue : issues) { + issue.accept(visitor); + } + } + } + + List divergingCommits() { + return divergingCommits(pr.headHash()); + } + + private List divergingCommits(Hash commitHash) { + try { + var updatedBase = localRepo.mergeBase(pr.targetHash(), commitHash); + return localRepo.commitMetadata(updatedBase, pr.targetHash()); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + Optional mergeTarget(PrintWriter reply) { + var divergingCommits = divergingCommits(pr.headHash()); + if (divergingCommits.size() > 0) { + reply.print("The following commits have been pushed to "); + reply.print(pr.targetRef()); + reply.println(" since your change was applied:"); + divergingCommits.forEach(c -> reply.println(" * " + c.hash().hex() + ": " + c.message().get(0))); + + try { + localRepo.checkout(pr.headHash(), true); + localRepo.merge(pr.targetHash()); + var hash = localRepo.commit("Automatic merge with latest target", "duke", "duke@openjdk.org"); + reply.println(); + reply.println("Your commit was automatically rebased without conflicts."); + return Optional.of(hash); + } catch (IOException e) { + reply.println(); + reply.print("It was not possible to rebase your changes automatically. Please merge `"); + reply.print(pr.targetRef()); + reply.println("` into your branch and try again."); + return Optional.empty(); + } + } else { + // No merge needed + return Optional.of(pr.headHash()); + } + } + +} \ No newline at end of file diff --git a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/IntegrateCommand.java b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/IntegrateCommand.java index d02521995..9f27973e6 100644 --- a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/IntegrateCommand.java +++ b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/IntegrateCommand.java @@ -27,8 +27,6 @@ import org.openjdk.skara.vcs.Hash; import java.io.*; -import java.net.URLEncoder; -import java.nio.charset.StandardCharsets; import java.nio.file.Path; import java.util.*; import java.util.logging.Logger; @@ -88,38 +86,36 @@ public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInst // Run a final jcheck to ensure the change has been properly reviewed try { - var sanitizedUrl = URLEncoder.encode(pr.repository().webUrl().toString(), StandardCharsets.UTF_8); - var path = scratchPath.resolve("integrate").resolve(sanitizedUrl); - + var path = scratchPath.resolve("integrate").resolve(pr.repository().name()); var seedPath = bot.seedStorage().orElse(scratchPath.resolve("seeds")); - var prInstance = new PullRequestInstance(path, - new HostedRepositoryPool(seedPath), - pr, - bot.ignoreStaleReviews()); + var hostedRepositoryPool = new HostedRepositoryPool(seedPath); + var localRepo = PullRequestUtils.materialize(hostedRepositoryPool, pr, path); + var checkablePr = new CheckablePullRequest(pr, localRepo, bot.ignoreStaleReviews()); + // Validate the target hash if requested var rebaseMessage = new StringWriter(); if (!args.isBlank()) { var wantedHash = new Hash(args); - if (!prInstance.targetHash().equals(wantedHash)) { + if (!pr.targetHash().equals(wantedHash)) { reply.print("The head of the target branch is no longer at the requested hash " + wantedHash); - reply.println(" - it has moved to " + prInstance.targetHash() + ". Aborting integration."); + reply.println(" - it has moved to " + pr.targetHash() + ". Aborting integration."); return; } }; // Now merge the latest changes from the target var rebaseWriter = new PrintWriter(rebaseMessage); - var rebasedHash = prInstance.mergeTarget(rebaseWriter); + var rebasedHash = checkablePr.mergeTarget(rebaseWriter); if (rebasedHash.isEmpty()) { reply.println(rebaseMessage.toString()); return; } - var localHash = prInstance.commit(rebasedHash.get(), censusInstance.namespace(), censusInstance.configuration().census().domain(), null); + var localHash = checkablePr.commit(rebasedHash.get(), censusInstance.namespace(), censusInstance.configuration().census().domain(), null); - var issues = prInstance.createVisitor(localHash, censusInstance); - var additionalConfiguration = AdditionalConfiguration.get(prInstance.localRepo(), localHash, pr.repository().forge().currentUser(), allComments); - prInstance.executeChecks(localHash, censusInstance, issues, additionalConfiguration); + var issues = checkablePr.createVisitor(localHash, censusInstance); + var additionalConfiguration = AdditionalConfiguration.get(localRepo, localHash, pr.repository().forge().currentUser(), allComments); + checkablePr.executeChecks(localHash, censusInstance, issues, additionalConfiguration); if (!issues.getMessages().isEmpty()) { reply.print("Your merge request cannot be fulfilled at this time, as "); reply.println("your changes failed the final jcheck:"); @@ -144,7 +140,7 @@ public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInst if (!localHash.equals(pr.targetHash())) { reply.println(rebaseMessage.toString()); reply.println("Pushed as commit " + localHash.hex() + "."); - prInstance.localRepo().push(localHash, pr.repository().url(), pr.targetRef()); + localRepo.push(localHash, pr.repository().url(), pr.targetRef()); pr.setState(PullRequest.State.CLOSED); pr.addLabel("integrated"); pr.removeLabel("ready"); diff --git a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/LabelerWorkItem.java b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/LabelerWorkItem.java index 7daf3a2ab..a1e8dcc78 100644 --- a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/LabelerWorkItem.java +++ b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/LabelerWorkItem.java @@ -23,6 +23,7 @@ package org.openjdk.skara.bots.pr; import org.openjdk.skara.forge.*; +import org.openjdk.skara.vcs.Repository; import java.io.*; import java.nio.file.Path; @@ -40,9 +41,9 @@ public String toString() { return "LabelerWorkItem@" + pr.repository().name() + "#" + pr.id(); } - private Set getLabels(PullRequestInstance prInstance) throws IOException { + private Set getLabels(Repository localRepo) throws IOException { var labels = new HashSet(); - var files = prInstance.changedFiles(); + var files = PullRequestUtils.changedFiles(pr, localRepo); for (var file : files) { for (var label : bot.labelPatterns().entrySet()) { for (var pattern : label.getValue()) { @@ -63,12 +64,11 @@ public void run(Path scratchPath) { return; } try { + var path = scratchPath.resolve("pr").resolve("labeler").resolve(pr.repository().name()); var seedPath = bot.seedStorage().orElse(scratchPath.resolve("seeds")); - var prInstance = new PullRequestInstance(scratchPath.resolve("pr").resolve("labeler"), - new HostedRepositoryPool(seedPath), - pr, - bot.ignoreStaleReviews()); - var newLabels = getLabels(prInstance); + var hostedRepositoryPool = new HostedRepositoryPool(seedPath); + var localRepo = PullRequestUtils.materialize(hostedRepositoryPool, pr, path); + var newLabels = getLabels(localRepo); var currentLabels = pr.labels().stream() .filter(key -> bot.labelPatterns().containsKey(key)) .collect(Collectors.toSet()); diff --git a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestInstance.java b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestInstance.java deleted file mode 100644 index c3f4e4427..000000000 --- a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestInstance.java +++ /dev/null @@ -1,370 +0,0 @@ -/* - * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved. - * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. - * - * This code is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License version 2 only, as - * published by the Free Software Foundation. - * - * This code is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * version 2 for more details (a copy is included in the LICENSE file that - * accompanied this code). - * - * You should have received a copy of the GNU General Public License version - * 2 along with this work; if not, write to the Free Software Foundation, - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA - * or visit www.oracle.com if you need additional information or have any - * questions. - */ -package org.openjdk.skara.bots.pr; - -import org.openjdk.skara.census.*; -import org.openjdk.skara.forge.*; -import org.openjdk.skara.host.HostUser; -import org.openjdk.skara.jcheck.JCheck; -import org.openjdk.skara.vcs.*; -import org.openjdk.skara.vcs.openjdk.*; - -import java.io.*; -import java.nio.file.Path; -import java.time.ZonedDateTime; -import java.util.*; -import java.util.regex.Pattern; -import java.util.stream.Collectors; - -class CommitFailure extends Exception { - CommitFailure(String reason) { - super(reason); - } -} - -class PullRequestInstance { - private final PullRequest pr; - private final Repository localRepo; - private final Hash targetHash; - private final Hash headHash; - private final Hash baseHash; - private final boolean ignoreStaleReviews; - - PullRequestInstance(Path localRepoPath, HostedRepositoryPool hostedRepositoryPool, PullRequest pr, boolean ignoreStaleReviews) throws IOException { - this.pr = pr; - this.ignoreStaleReviews = ignoreStaleReviews; - - // Materialize the PR's source and target ref - var repository = pr.repository(); - localRepo = hostedRepositoryPool.checkout(pr, localRepoPath.resolve(repository.name())); - localRepo.fetch(repository.url(), "+" + pr.targetRef() + ":pr_prinstance", false); - - targetHash = pr.targetHash(); - headHash = pr.headHash(); - baseHash = localRepo.mergeBase(targetHash, headHash); - } - - /** - * The Review list is in chronological order, the latest one from a particular reviewer is the - * one that is "active". - * @param allReviews - * @return - */ - static List filterActiveReviews(List allReviews) { - var reviewPerUser = new LinkedHashMap(); - for (var review : allReviews) { - reviewPerUser.put(review.reviewer(), review); - } - return new ArrayList<>(reviewPerUser.values()); - } - - private String commitMessage(List activeReviews, Namespace namespace) throws IOException { - var reviewers = activeReviews.stream() - .filter(review -> !ignoreStaleReviews || review.hash().equals(headHash)) - .filter(review -> review.verdict() == Review.Verdict.APPROVED) - .map(review -> review.reviewer().id()) - .map(namespace::get) - .filter(Objects::nonNull) - .map(Contributor::username) - .collect(Collectors.toList()); - - var comments = pr.comments(); - var additionalContributors = Contributors.contributors(pr.repository().forge().currentUser(), - comments).stream() - .map(email -> Author.fromString(email.toString())) - .collect(Collectors.toList()); - - var additionalIssues = SolvesTracker.currentSolved(pr.repository().forge().currentUser(), comments); - var summary = Summary.summary(pr.repository().forge().currentUser(), comments); - var issue = Issue.fromString(pr.title()); - var commitMessageBuilder = issue.map(CommitMessage::title).orElseGet(() -> CommitMessage.title(pr.title())); - if (issue.isPresent()) { - commitMessageBuilder.issues(additionalIssues); - } - commitMessageBuilder.contributors(additionalContributors) - .reviewers(reviewers); - summary.ifPresent(commitMessageBuilder::summary); - - return String.join("\n", commitMessageBuilder.format(CommitMessageFormatters.v1)); - } - - private Hash commitSquashed(Hash finalHead, List activeReviews, Namespace namespace, String censusDomain, String sponsorId) throws IOException { - Author committer; - Author author; - var contributor = namespace.get(pr.author().id()); - - if (contributor == null) { - // Use the information contained in the head commit - jcheck has verified that it contains sane values - var headCommit = localRepo.commitMetadata(headHash.hex() + "^.." + headHash.hex()).get(0); - author = headCommit.author(); - } else { - author = new Author(contributor.fullName().orElseThrow(), contributor.username() + "@" + censusDomain); - } - - if (sponsorId != null) { - var sponsorContributor = namespace.get(sponsorId); - committer = new Author(sponsorContributor.fullName().orElseThrow(), sponsorContributor.username() + "@" + censusDomain); - } else { - committer = author; - } - - var commitMessage = commitMessage(activeReviews, namespace); - return localRepo.commit(commitMessage, author.name(), author.email(), ZonedDateTime.now(), - committer.name(), committer.email(), ZonedDateTime.now(), List.of(targetHash), localRepo.tree(finalHead)); - } - - private static class MergeSource { - private final String repositoryName; - private final String branchName; - - private MergeSource(String repositoryName, String branchName) { - this.repositoryName = repositoryName; - this.branchName = branchName; - } - } - - private final Pattern mergeSourceFullPattern = Pattern.compile("^Merge ([-/\\w]+):([-\\w]+)$"); - private final Pattern mergeSourceBranchOnlyPattern = Pattern.compile("^Merge ([-\\w]+)$"); - - private Optional mergeSource() { - var repoMatcher = mergeSourceFullPattern.matcher(pr.title()); - if (!repoMatcher.matches()) { - var branchMatcher = mergeSourceBranchOnlyPattern.matcher(pr.title()); - if (!branchMatcher.matches()) { - return Optional.empty(); - } - - // Verify that the branch exists - var isValidBranch = remoteBranches().stream() - .map(Reference::name) - .anyMatch(branch -> branch.equals(branchMatcher.group(1))); - if (!isValidBranch) { - // Assume the name refers to a sibling repository - var repoName = Path.of(pr.repository().name()).resolveSibling(branchMatcher.group(1)).toString(); - return Optional.of(new MergeSource(repoName, "master")); - } - - return Optional.of(new MergeSource(pr.repository().name(), branchMatcher.group(1))); - } - - return Optional.of(new MergeSource(repoMatcher.group(1), repoMatcher.group(2))); - } - - private CommitMetadata findSourceMergeCommit(List commits) throws IOException, CommitFailure { - if (commits.size() < 2) { - throw new CommitFailure("A merge PR must contain at least two commits that are not already present in the target."); - } - - var source = mergeSource(); - if (source.isEmpty()) { - throw new CommitFailure("Could not determine the source for this merge. A Merge PR title must be specified on the format: " + - "Merge `project`:`branch` to allow verification of the merge contents."); - } - - // Fetch the source - Hash sourceHash; - try { - var mergeSourceRepo = pr.repository().forge().repository(source.get().repositoryName).orElseThrow(() -> - new RuntimeException("Could not find repository " + source.get().repositoryName) - ); - try { - sourceHash = localRepo.fetch(mergeSourceRepo.url(), source.get().branchName, false); - } catch (IOException e) { - throw new CommitFailure("Could not fetch branch `" + source.get().branchName + "` from project `" + - source.get().repositoryName + "` - check that they are correct."); - } - } catch (RuntimeException e) { - throw new CommitFailure("Could not find project `" + - source.get().repositoryName + "` - check that it is correct."); - } - - - // Find the first merge commit with a parent that is an ancestor of the source - int mergeCommitIndex = commits.size(); - for (int i = 0; i < commits.size() - 1; ++i) { - if (commits.get(i).isMerge()) { - boolean isSourceMerge = false; - for (int j = 0; j < commits.get(i).parents().size(); ++j) { - if (localRepo.isAncestor(commits.get(i).parents().get(j), sourceHash)) { - isSourceMerge = true; - } - } - if (isSourceMerge) { - mergeCommitIndex = i; - break; - } - } - } - if (mergeCommitIndex >= commits.size() - 1) { - throw new CommitFailure("A merge PR must contain a merge commit as well as at least one other commit from the merge source."); - } - - return commits.get(mergeCommitIndex); - } - - private Hash commitMerge(Hash finalHead, List activeReviews, Namespace namespace, String censusDomain, String sponsorId) throws IOException, CommitFailure { - var commits = localRepo.commitMetadata(baseHash, finalHead); - var mergeCommit = findSourceMergeCommit(commits); - - // Find the parent which is on the target branch - we will replace it with the target hash (if there were no merge conflicts) - Hash firstParent = null; - var finalParents = new ArrayList(); - for (int i = 0; i < mergeCommit.parents().size(); ++i) { - if (localRepo.isAncestor(mergeCommit.parents().get(i), targetHash)) { - if (firstParent == null) { - firstParent = localRepo.mergeBase(targetHash, finalHead); - continue; - } - } - finalParents.add(mergeCommit.parents().get(i)); - } - if (firstParent == null) { - throw new CommitFailure("The merge commit must have a commit on the target branch as one of its parents."); - } - finalParents.add(0, firstParent); - - var contributor = namespace.get(pr.author().id()); - if (contributor == null) { - throw new CommitFailure("Merges can only be performed by Committers."); - } - - var author = new Author(contributor.fullName().orElseThrow(), contributor.username() + "@" + censusDomain); - Author committer; - if (sponsorId != null) { - var sponsorContributor = namespace.get(sponsorId); - committer = new Author(sponsorContributor.fullName().orElseThrow(), sponsorContributor.username() + "@" + censusDomain); - } else { - committer = author; - } - var commitMessage = commitMessage(activeReviews, namespace); - - return localRepo.commit(commitMessage, author.name(), author.email(), ZonedDateTime.now(), - committer.name(), committer.email(), ZonedDateTime.now(), finalParents, localRepo.tree(finalHead)); - } - - private boolean isMergeCommit() { - return pr.title().startsWith("Merge"); - } - - Hash commit(Hash finalHead, Namespace namespace, String censusDomain, String sponsorId) throws IOException, CommitFailure { - var activeReviews = filterActiveReviews(pr.reviews()); - Hash commit; - if (!isMergeCommit()) { - commit = commitSquashed(finalHead, activeReviews, namespace, censusDomain, sponsorId); - } else { - commit = commitMerge(finalHead, activeReviews, namespace, censusDomain, sponsorId); - } - localRepo.checkout(commit, true); - return commit; - } - - List divergingCommits() { - return divergingCommits(headHash); - } - - private List divergingCommits(Hash commitHash) { - try { - var updatedBase = localRepo.mergeBase(targetHash, commitHash); - return localRepo.commitMetadata(updatedBase, targetHash); - } catch (IOException e) { - throw new RuntimeException(e); - } - } - - Optional mergeTarget(PrintWriter reply) { - var divergingCommits = divergingCommits(headHash); - if (divergingCommits.size() > 0) { - reply.print("The following commits have been pushed to "); - reply.print(pr.targetRef()); - reply.println(" since your change was applied:"); - divergingCommits.forEach(c -> reply.println(" * " + c.hash().hex() + ": " + c.message().get(0))); - - try { - localRepo.checkout(headHash, true); - localRepo.merge(targetHash); - var hash = localRepo.commit("Automatic merge with latest target", "duke", "duke@openjdk.org"); - reply.println(); - reply.println("Your commit was automatically rebased without conflicts."); - return Optional.of(hash); - } catch (IOException e) { - reply.println(); - reply.print("It was not possible to rebase your changes automatically. Please merge `"); - reply.print(pr.targetRef()); - reply.println("` into your branch and try again."); - try { - localRepo.checkout(headHash, true); - } catch (IOException e2) { - throw new UncheckedIOException(e2); - } - return Optional.empty(); - } - } else { - // No merge needed - return Optional.of(headHash); - } - } - - Repository localRepo() { - return localRepo; - } - - Hash baseHash() { - return baseHash; - } - - Hash targetHash() { - return targetHash; - } - - Set changedFiles() throws IOException { - var ret = new HashSet(); - var changes = localRepo.diff(baseHash, headHash); - for (var patch : changes.patches()) { - patch.target().path().ifPresent(ret::add); - patch.source().path().ifPresent(ret::add); - } - return ret; - } - - PullRequestCheckIssueVisitor createVisitor(Hash localHash, CensusInstance censusInstance) throws IOException { - var checks = JCheck.checksFor(localRepo(), censusInstance.census(), targetHash); - return new PullRequestCheckIssueVisitor(checks); - } - - void executeChecks(Hash localHash, CensusInstance censusInstance, PullRequestCheckIssueVisitor visitor, List additionalConfiguration) throws Exception { - try (var issues = JCheck.check(localRepo(), censusInstance.census(), CommitMessageParsers.v1, localHash, - targetHash, additionalConfiguration)) { - for (var issue : issues) { - issue.accept(visitor); - } - } - } - - List remoteBranches() { - try { - return localRepo.remoteBranches(pr.repository().url().toString()); - } catch (IOException e) { - return List.of(); - } - } -} diff --git a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/SponsorCommand.java b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/SponsorCommand.java index a9beea11d..63ad2a58c 100644 --- a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/SponsorCommand.java +++ b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/SponsorCommand.java @@ -27,8 +27,6 @@ import org.openjdk.skara.vcs.Hash; import java.io.*; -import java.net.URLEncoder; -import java.nio.charset.StandardCharsets; import java.nio.file.Path; import java.util.*; import java.util.logging.Logger; @@ -73,39 +71,37 @@ public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInst // Execute merge try { - var sanitizedUrl = URLEncoder.encode(pr.repository().webUrl().toString(), StandardCharsets.UTF_8); - var path = scratchPath.resolve("sponsor").resolve(sanitizedUrl); - + var path = scratchPath.resolve("sponsor").resolve(pr.repository().name()); var seedPath = bot.seedStorage().orElse(scratchPath.resolve("seeds")); - var prInstance = new PullRequestInstance(path, - new HostedRepositoryPool(seedPath), - pr, - bot.ignoreStaleReviews()); + var hostedRepositoryPool = new HostedRepositoryPool(seedPath); + var localRepo = PullRequestUtils.materialize(hostedRepositoryPool, pr, path); + var checkablePr = new CheckablePullRequest(pr, localRepo, bot.ignoreStaleReviews()); + // Validate the target hash if requested var rebaseMessage = new StringWriter(); if (!args.isBlank()) { var wantedHash = new Hash(args); - if (!prInstance.targetHash().equals(wantedHash)) { + if (!pr.targetHash().equals(wantedHash)) { reply.print("The head of the target branch is no longer at the requested hash " + wantedHash); - reply.println(" - it has moved to " + prInstance.targetHash() + ". Aborting integration."); + reply.println(" - it has moved to " + pr.targetHash() + ". Aborting integration."); return; } } // Now rebase onto the target hash var rebaseWriter = new PrintWriter(rebaseMessage); - var rebasedHash = prInstance.mergeTarget(rebaseWriter); + var rebasedHash = checkablePr.mergeTarget(rebaseWriter); if (rebasedHash.isEmpty()) { reply.println(rebaseMessage.toString()); return; } - var localHash = prInstance.commit(rebasedHash.get(), censusInstance.namespace(), censusInstance.configuration().census().domain(), + var localHash = checkablePr.commit(rebasedHash.get(), censusInstance.namespace(), censusInstance.configuration().census().domain(), comment.author().id()); - var issues = prInstance.createVisitor(localHash, censusInstance); - var additionalConfiguration = AdditionalConfiguration.get(prInstance.localRepo(), localHash, pr.repository().forge().currentUser(), allComments); - prInstance.executeChecks(localHash, censusInstance, issues, additionalConfiguration); + var issues = checkablePr.createVisitor(localHash, censusInstance); + var additionalConfiguration = AdditionalConfiguration.get(localRepo, localHash, pr.repository().forge().currentUser(), allComments); + checkablePr.executeChecks(localHash, censusInstance, issues, additionalConfiguration); if (!issues.getMessages().isEmpty()) { reply.print("Your merge request cannot be fulfilled at this time, as "); reply.println("your changes failed the final jcheck:"); @@ -118,7 +114,7 @@ public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInst if (!localHash.equals(pr.targetHash())) { reply.println(rebaseMessage.toString()); reply.println("Pushed as commit " + localHash.hex() + "."); - prInstance.localRepo().push(localHash, pr.repository().url(), pr.targetRef()); + localRepo.push(localHash, pr.repository().url(), pr.targetRef()); pr.setState(PullRequest.State.CLOSED); pr.addLabel("integrated"); pr.removeLabel("sponsor"); diff --git a/bots/pr/src/test/java/org/openjdk/skara/bots/pr/MergeTests.java b/bots/pr/src/test/java/org/openjdk/skara/bots/pr/MergeTests.java index acdd78c73..0a76b46bf 100644 --- a/bots/pr/src/test/java/org/openjdk/skara/bots/pr/MergeTests.java +++ b/bots/pr/src/test/java/org/openjdk/skara/bots/pr/MergeTests.java @@ -463,14 +463,21 @@ void invalidMergeCommit(TestInfo testInfo) throws IOException { pr.addComment("/integrate"); TestBotRunner.runPeriodicItems(mergeBot); - // The bot should reply with a failure message - var error = pr.comments().stream() - .filter(comment -> comment.body().contains("did not complete successfully")) - .count(); - assertEquals(1, error, () -> pr.comments().stream().map(Comment::body).collect(Collectors.joining("\n\n"))); + // The bot will create a proper merge commit + var pushed = pr.comments().stream() + .filter(comment -> comment.body().contains("Pushed as commit")) + .count(); + assertEquals(1, pushed, () -> pr.comments().stream().map(Comment::body).collect(Collectors.joining("\n\n"))); - var check = pr.checks(mergeHash).get("jcheck"); - assertEquals("- The merge commit must have a commit on the target branch as one of its parents.", check.summary().orElseThrow()); + // The change should now be present with correct parents on the master branch + var pushedRepoFolder = tempFolder.path().resolve("pushedrepo"); + var pushedRepo = Repository.materialize(pushedRepoFolder, author.url(), "master"); + assertTrue(CheckableRepository.hasBeenEdited(pushedRepo)); + + var head = pushedRepo.commitMetadata("HEAD^!").get(0); + assertEquals(2, head.parents().size()); + assertEquals(masterHash, head.parents().get(0)); + assertEquals(otherHash, head.parents().get(1)); } } @@ -716,7 +723,7 @@ void wrongSourceBranch(TestInfo testInfo) throws IOException { assertEquals(1, error, () -> pr.comments().stream().map(Comment::body).collect(Collectors.joining("\n\n"))); var check = pr.checks(mergeHash).get("jcheck"); - assertEquals("- A merge PR must contain a merge commit as well as at least one other commit from the merge source.", check.summary().orElseThrow()); + assertEquals("- A merge PR must contain at least one commit from the source branch that is not already present in the target.", check.summary().orElseThrow()); } } @@ -848,7 +855,7 @@ void unrelatedHistory(TestInfo testInfo) throws IOException { assertEquals(1, error, () -> pr.comments().stream().map(Comment::body).collect(Collectors.joining("\n\n"))); var check = pr.checks(mergeHash).get("jcheck"); - assertEquals("- The merge commit must have a commit on the target branch as one of its parents.", check.summary().orElseThrow()); + assertEquals("- The target and the source branches do not share common history - cannot merge them.", check.summary().orElseThrow()); } } diff --git a/forge/src/main/java/org/openjdk/skara/forge/CommitFailure.java b/forge/src/main/java/org/openjdk/skara/forge/CommitFailure.java new file mode 100644 index 000000000..b86a3fcb3 --- /dev/null +++ b/forge/src/main/java/org/openjdk/skara/forge/CommitFailure.java @@ -0,0 +1,29 @@ +/* + * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ +package org.openjdk.skara.forge; + +public class CommitFailure extends Exception { + public CommitFailure(String reason) { + super(reason); + } +} diff --git a/forge/src/main/java/org/openjdk/skara/forge/HostedRepositoryPool.java b/forge/src/main/java/org/openjdk/skara/forge/HostedRepositoryPool.java index f542df50a..b200b878c 100644 --- a/forge/src/main/java/org/openjdk/skara/forge/HostedRepositoryPool.java +++ b/forge/src/main/java/org/openjdk/skara/forge/HostedRepositoryPool.java @@ -171,8 +171,4 @@ public Repository checkout(HostedRepository hostedRepository, String ref, Path p } return localRepo; } - - public Repository checkout(PullRequest pr, Path path) throws IOException { - return checkout(pr.repository(), pr.headHash().hex(), path); - } } diff --git a/forge/src/main/java/org/openjdk/skara/forge/PullRequestUtils.java b/forge/src/main/java/org/openjdk/skara/forge/PullRequestUtils.java new file mode 100644 index 000000000..9a6926cb8 --- /dev/null +++ b/forge/src/main/java/org/openjdk/skara/forge/PullRequestUtils.java @@ -0,0 +1,175 @@ +/* + * Copyright (c) 2019, 2020, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ +package org.openjdk.skara.forge; + +import org.openjdk.skara.vcs.*; + +import java.io.*; +import java.nio.file.Path; +import java.time.ZonedDateTime; +import java.util.*; +import java.util.regex.Pattern; +import java.util.stream.Collectors; + +public class PullRequestUtils { + private static Hash commitSquashed(PullRequest pr, Repository localRepo, Hash finalHead, Author author, Author committer, String commitMessage) throws IOException { + return localRepo.commit(commitMessage, author.name(), author.email(), ZonedDateTime.now(), + committer.name(), committer.email(), ZonedDateTime.now(), List.of(pr.targetHash()), localRepo.tree(finalHead)); + } + + private static class MergeSource { + private final String repositoryName; + private final String branchName; + + private MergeSource(String repositoryName, String branchName) { + this.repositoryName = repositoryName; + this.branchName = branchName; + } + } + + private final static Pattern mergeSourceFullPattern = Pattern.compile("^Merge ([-/\\w]+):([-\\w]+)$"); + private final static Pattern mergeSourceBranchOnlyPattern = Pattern.compile("^Merge ([-\\w]+)$"); + + private static Optional mergeSource(PullRequest pr, Repository localRepo) { + var repoMatcher = mergeSourceFullPattern.matcher(pr.title()); + if (!repoMatcher.matches()) { + var branchMatcher = mergeSourceBranchOnlyPattern.matcher(pr.title()); + if (!branchMatcher.matches()) { + return Optional.empty(); + } + + // Verify that the branch exists + var isValidBranch = remoteBranches(pr, localRepo).stream() + .map(Reference::name) + .anyMatch(branch -> branch.equals(branchMatcher.group(1))); + if (!isValidBranch) { + // Assume the name refers to a sibling repository + var repoName = Path.of(pr.repository().name()).resolveSibling(branchMatcher.group(1)).toString(); + return Optional.of(new MergeSource(repoName, "master")); + } + + return Optional.of(new MergeSource(pr.repository().name(), branchMatcher.group(1))); + } + + return Optional.of(new MergeSource(repoMatcher.group(1), repoMatcher.group(2))); + } + + private static Hash findSourceHash(PullRequest pr, Repository localRepo, List commits) throws IOException, CommitFailure { + if (commits.size() < 1) { + throw new CommitFailure("A merge PR must contain at least one commit that is not already present in the target."); + } + + var source = mergeSource(pr, localRepo); + if (source.isEmpty()) { + throw new CommitFailure("Could not determine the source for this merge. A Merge PR title must be specified on the format: " + + "Merge `project`:`branch` to allow verification of the merge contents."); + } + + // Fetch the source + Hash sourceHead; + try { + var mergeSourceRepo = pr.repository().forge().repository(source.get().repositoryName).orElseThrow(() -> + new RuntimeException("Could not find repository " + source.get().repositoryName) + ); + try { + sourceHead = localRepo.fetch(mergeSourceRepo.url(), source.get().branchName, false); + } catch (IOException e) { + throw new CommitFailure("Could not fetch branch `" + source.get().branchName + "` from project `" + + source.get().repositoryName + "` - check that they are correct."); + } + } catch (RuntimeException e) { + throw new CommitFailure("Could not find project `" + + source.get().repositoryName + "` - check that it is correct."); + } + + // Ensure that the source and the target are related + try { + localRepo.mergeBase(pr.targetHash(), sourceHead); + } catch (IOException e) { + throw new CommitFailure("The target and the source branches do not share common history - cannot merge them."); + } + + // Find the most recent commit from the merge source not present in the target + var sourceHash = localRepo.mergeBase(pr.headHash(), sourceHead); + var commitHashes = commits.stream() + .map(CommitMetadata::hash) + .collect(Collectors.toSet()); + if (!commitHashes.contains(sourceHash)) { + throw new CommitFailure("A merge PR must contain at least one commit from the source branch that is not already present in the target."); + } + + return sourceHash; + } + + private static Hash commitMerge(PullRequest pr, Repository localRepo, Hash finalHead, Author author, Author committer, String commitMessage) throws IOException, CommitFailure { + var commits = localRepo.commitMetadata(baseHash(pr, localRepo), finalHead); + var sourceHash = findSourceHash(pr, localRepo, commits); + var parents = List.of(localRepo.mergeBase(pr.targetHash(), finalHead), sourceHash); + + return localRepo.commit(commitMessage, author.name(), author.email(), ZonedDateTime.now(), + committer.name(), committer.email(), ZonedDateTime.now(), parents, localRepo.tree(finalHead)); + } + + public static Repository materialize(HostedRepositoryPool hostedRepositoryPool, PullRequest pr, Path path) throws IOException { + var localRepo = hostedRepositoryPool.checkout(pr.repository(), pr.headHash().hex(), path); + localRepo.fetch(pr.repository().url(), "+" + pr.targetRef() + ":prutils_targetref", false); + return localRepo; + } + + public static boolean isMerge(PullRequest pr) { + return pr.title().startsWith("Merge"); + } + + public static Hash createCommit(PullRequest pr, Repository localRepo, Hash finalHead, Author author, Author committer, String commitMessage) throws IOException, CommitFailure { + Hash commit; + if (!isMerge(pr)) { + commit = commitSquashed(pr, localRepo, finalHead, author, committer, commitMessage); + } else { + commit = commitMerge(pr, localRepo, finalHead, author, committer, commitMessage); + } + localRepo.checkout(commit, true); + return commit; + } + + public static Hash baseHash(PullRequest pr, Repository localRepo) throws IOException { + return localRepo.mergeBase(pr.targetHash(), pr.headHash()); + } + + public static Set changedFiles(PullRequest pr, Repository localRepo) throws IOException { + var ret = new HashSet(); + var changes = localRepo.diff(baseHash(pr, localRepo), pr.headHash()); + for (var patch : changes.patches()) { + patch.target().path().ifPresent(ret::add); + patch.source().path().ifPresent(ret::add); + } + return ret; + } + + private static List remoteBranches(PullRequest pr, Repository localRepo) { + try { + return localRepo.remoteBranches(pr.repository().url().toString()); + } catch (IOException e) { + return List.of(); + } + } +}