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 e3a02942f..52ec498df 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 @@ -44,7 +44,7 @@ public String toString() { private Set getLabels(Repository localRepo) throws IOException { var files = PullRequestUtils.changedFiles(pr, localRepo); - return bot.labelConfiguration().fromChanges(files); + return bot.labelConfiguration().label(files); } @Override diff --git a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestBotBuilder.java b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestBotBuilder.java index d019c643e..e08b32a3e 100644 --- a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestBotBuilder.java +++ b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestBotBuilder.java @@ -23,6 +23,7 @@ package org.openjdk.skara.bots.pr; import org.openjdk.skara.forge.HostedRepository; +import org.openjdk.skara.forge.LabelConfiguration; import org.openjdk.skara.issuetracker.IssueProject; import java.nio.file.Path; @@ -33,7 +34,7 @@ public class PullRequestBotBuilder { private HostedRepository repo; private HostedRepository censusRepo; private String censusRef = "master"; - private LabelConfiguration labelConfiguration = LabelConfiguration.newBuilder().build(); + private LabelConfiguration labelConfiguration = LabelConfiguration.builder().build(); private Map externalCommands = Map.of(); private Map blockingCheckLabels = Map.of(); private Set readyLabels = Set.of(); diff --git a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestBotFactory.java b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestBotFactory.java index 37a3ea43d..5f966882c 100644 --- a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestBotFactory.java +++ b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestBotFactory.java @@ -23,6 +23,7 @@ package org.openjdk.skara.bots.pr; import org.openjdk.skara.bot.*; +import org.openjdk.skara.forge.LabelConfiguration; import org.openjdk.skara.json.*; import java.util.*; @@ -64,31 +65,8 @@ public List create(BotConfiguration configuration) { var labelConfigurations = new HashMap(); for (var labelGroup : specific.get("labels").fields()) { - var labelConfiguration = LabelConfiguration.newBuilder(); - if (labelGroup.value().contains("matchers")) { - var matchers = labelGroup.value().get("matchers").fields().stream() - .collect(Collectors.toMap(JSONObject.Field::name, - field -> field.value().stream() - .map(JSONValue::asString) - .map(Pattern::compile) - .collect(Collectors.toList()))); - matchers.forEach(labelConfiguration::addMatchers); - } - if (labelGroup.value().contains("groups")) { - var groups = labelGroup.value().get("groups").fields().stream() - .collect(Collectors.toMap(JSONObject.Field::name, - field -> field.value().stream() - .map(JSONValue::asString) - .collect(Collectors.toList()))); - groups.forEach(labelConfiguration::addGroup); - } - if (labelGroup.value().contains("extra")) { - var extra = labelGroup.value().get("extra").stream() - .map(JSONValue::asString) - .collect(Collectors.toList()); - extra.forEach(labelConfiguration::addExtra); - } - labelConfigurations.put(labelGroup.name(), labelConfiguration.build()); + labelConfigurations.put(labelGroup.name(), + LabelConfiguration.fromJSON(labelGroup.value())); } for (var repo : specific.get("repositories").fields()) { diff --git a/bots/pr/src/test/java/org/openjdk/skara/bots/pr/LabelTests.java b/bots/pr/src/test/java/org/openjdk/skara/bots/pr/LabelTests.java index 95011e668..d1959a011 100644 --- a/bots/pr/src/test/java/org/openjdk/skara/bots/pr/LabelTests.java +++ b/bots/pr/src/test/java/org/openjdk/skara/bots/pr/LabelTests.java @@ -22,6 +22,7 @@ */ package org.openjdk.skara.bots.pr; +import org.openjdk.skara.forge.LabelConfiguration; import org.junit.jupiter.api.*; import org.openjdk.skara.test.*; @@ -44,7 +45,7 @@ void simple(TestInfo testInfo) throws IOException { var censusBuilder = credentials.getCensusBuilder() .addReviewer(integrator.forge().currentUser().id()) .addCommitter(author.forge().currentUser().id()); - var labelConfiguration = LabelConfiguration.newBuilder() + var labelConfiguration = LabelConfiguration.builder() .addMatchers("1", List.of(Pattern.compile("cpp$"))) .addMatchers("2", List.of(Pattern.compile("hpp$"))) .addGroup("group", List.of("1", "2")) @@ -134,7 +135,7 @@ void adjustAutoApplied(TestInfo testInfo) throws IOException { var censusBuilder = credentials.getCensusBuilder() .addReviewer(integrator.forge().currentUser().id()) .addCommitter(author.forge().currentUser().id()); - var labelConfiguration = LabelConfiguration.newBuilder() + var labelConfiguration = LabelConfiguration.builder() .addMatchers("1", List.of(Pattern.compile("cpp$"))) .addMatchers("2", List.of(Pattern.compile("hpp$"))) .addGroup("group", List.of("1", "2")) @@ -202,7 +203,7 @@ void overrideAutoApplied(TestInfo testInfo) throws IOException { var censusBuilder = credentials.getCensusBuilder() .addReviewer(integrator.forge().currentUser().id()) .addCommitter(author.forge().currentUser().id()); - var labelConfiguration = LabelConfiguration.newBuilder() + var labelConfiguration = LabelConfiguration.builder() .addMatchers("1", List.of(Pattern.compile("cpp$"))) .addMatchers("2", List.of(Pattern.compile("hpp$"))) .addGroup("group", List.of("1", "2")) @@ -264,7 +265,7 @@ void commandAuthor(TestInfo testInfo) throws IOException { .addCommitter(committer.forge().currentUser().id()) .addAuthor(author.forge().currentUser().id()) .addAuthor(other.forge().currentUser().id()); - var labelConfiguration = LabelConfiguration.newBuilder() + var labelConfiguration = LabelConfiguration.builder() .addMatchers("1", List.of(Pattern.compile("cpp$"))) .addMatchers("2", List.of(Pattern.compile("hpp$"))) .addGroup("group", List.of("1", "2")) diff --git a/bots/pr/src/test/java/org/openjdk/skara/bots/pr/LabelerTests.java b/bots/pr/src/test/java/org/openjdk/skara/bots/pr/LabelerTests.java index 968f7a1e5..9bcc10b00 100644 --- a/bots/pr/src/test/java/org/openjdk/skara/bots/pr/LabelerTests.java +++ b/bots/pr/src/test/java/org/openjdk/skara/bots/pr/LabelerTests.java @@ -23,6 +23,7 @@ package org.openjdk.skara.bots.pr; import org.openjdk.skara.test.*; +import org.openjdk.skara.forge.LabelConfiguration; import org.junit.jupiter.api.*; @@ -41,7 +42,7 @@ void simple(TestInfo testInfo) throws IOException { var author = credentials.getHostedRepository(); var reviewer = credentials.getHostedRepository(); - var labelConfiguration = LabelConfiguration.newBuilder() + var labelConfiguration = LabelConfiguration.builder() .addMatchers("test1", List.of(Pattern.compile("a.txt"))) .addMatchers("test2", List.of(Pattern.compile("b.txt"))) .build(); diff --git a/cli/src/main/java/org/openjdk/skara/cli/pr/GitPrCreate.java b/cli/src/main/java/org/openjdk/skara/cli/pr/GitPrCreate.java index 03b5c8c2a..5ac8e911e 100644 --- a/cli/src/main/java/org/openjdk/skara/cli/pr/GitPrCreate.java +++ b/cli/src/main/java/org/openjdk/skara/cli/pr/GitPrCreate.java @@ -26,17 +26,18 @@ import org.openjdk.skara.cli.GitPublish; import org.openjdk.skara.cli.GitJCheck; import org.openjdk.skara.vcs.Branch; +import org.openjdk.skara.vcs.ReadOnlyRepository; import org.openjdk.skara.vcs.openjdk.CommitMessageParsers; +import org.openjdk.skara.forge.Forge; +import org.openjdk.skara.forge.LabelConfiguration; +import org.openjdk.skara.json.JSON; import static org.openjdk.skara.cli.pr.Utils.*; import java.io.IOException; import java.nio.file.Files; import java.nio.file.StandardOpenOption; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; -import java.util.Optional; +import java.util.*; import java.util.stream.Collectors; public class GitPrCreate { @@ -56,6 +57,11 @@ public class GitPrCreate { .describe("NAME") .helptext("Name of target branch, defaults to 'master'") .optional(), + Option.shortcut("") + .fullname("cc") + .describe("MAILING LISTS") + .helptext("Mailing lists to CC for inital RFR e-mail") + .optional(), Switch.shortcut("") .fullname("ignore-workspace") .helptext("Ignore local changes in worktree and staging area when creating pull request") @@ -97,6 +103,33 @@ public class GitPrCreate { .optional() ); + + private static LabelConfiguration labelConfiguration(Forge forge, String project) throws IOException { + var group = project.split("/")[0]; + var skaraRemoteRepo = forge.repository(group + "/skara").orElseThrow(() -> + new IOException("error: could not resolve Skara repository") + ); + var rules = skaraRemoteRepo.fileContents("config/mailinglist/rules/jdk.json", "master"); + var json = JSON.parse(rules); + return LabelConfiguration.fromJSON(json); + } + + private static Set suggestedLabels(ReadOnlyRepository repo, Forge forge, String project, String targetRef, String headRef) throws IOException { + var config = labelConfiguration(forge, project); + var baseHash = repo.resolve(targetRef).orElseThrow(() -> + new IOException("error: cannot resolve " + targetRef) + ); + var headHash = repo.resolve(headRef).orElseThrow(() -> + new IOException("error: cannot resolve " + headRef) + ); + var status = repo.status(baseHash, headHash); + var files = status.stream() + .filter(e -> !e.status().isDeleted()) + .map(e -> e.target().path().get()) + .collect(Collectors.toSet()); + return config.label(files); + } + public static void main(String[] args) throws IOException, InterruptedException { var parser = new ArgumentParser("git-pr", flags, inputs); var arguments = parse(parser, args); @@ -269,6 +302,84 @@ public static void main(String[] args) throws IOException, InterruptedException } } + var mailingLists = new ArrayList(); + var parentProject = projectName(parentRepo.url()); + var isTargetingJDKRepo = parentProject.endsWith("jdk"); + var cc = getOption("cc", "create", arguments); + var isCCManual = cc != null && !cc.equals("auto"); + if (!isTargetingJDKRepo && isCCManual) { + System.out.println("error: you cannot manually CC additional mailing lists for " + parentProject); + System.exit(1); + } + if (isTargetingJDKRepo) { + if (isCCManual) { + var config = labelConfiguration(host, parentProject); + var lists = cc.split(","); + for (var input : lists) { + var label = input; + if (label.endsWith("@openjdk.java.net")) { + label = input.split("@")[0]; + } + if (label.endsWith("-dev")) { + label = label.replace("-dev", ""); + } + if (!config.isAllowed(label) && !config.isAllowed(label + "-dev")) { + System.out.println("error: the mailing list \"" + label + + "-dev@openjdk.java.net\" is not applicable, aborting."); + System.exit(1); + } + } + System.out.println("You have chosen the following mailing lists to be CC:d for the \"RFR\" e-mail:"); + for (var input : lists) { + String list = null; + if (input.endsWith("@openjdk.java.net")) { + list = input; + } else if (input.endsWith("-dev")) { + list = input + "@openjdk.java.net"; + } else { + list = input + "-dev@openjdk.java.net"; + } + System.out.println("- " + list); + mailingLists.add(list); + } + } else { + var suggested = suggestedLabels(repo, host, parentProject, targetBranch, headRef); + System.out.println("The following mailing lists will be CC:d for the \"RFR\" e-mail:"); + for (var label : suggested) { + String list = null; + if (label.endsWith("-dev")) { + list = label + "@openjdk.java.net"; + } else { + list = label + "-dev@openjdk.java.net"; + } + if (cc == null) { + System.out.println("- " + list); + } + mailingLists.add(list); + } + } + if (cc == null || !cc.equals("auto")) { + System.out.println(""); + System.out.print("Do you want to proceed with this mailing list selection? [Y/n]: "); + var scanner = new Scanner(System.in); + var answer = scanner.nextLine().toLowerCase(); + while (!(answer.equals("y") || answer.equals("n") || answer.isEmpty())) { + System.out.print("Please answer with 'y', 'n' or empty for the default choice: "); + answer = scanner.nextLine().toLowerCase(); + } + if (!(answer.isEmpty() || answer.equals("y"))) { + System.out.println(""); + System.out.println("error: user not satisfied with mailing list selection, aborting."); + if (cc == null) { + System.out.println(" To specify mailing lists manually, use the --cc option."); + } else if (cc.equals("auto")) { + System.out.println(" You have set --cc=auto, you can use --cc to specify mailing lists manually"); + } + System.exit(1); + } + } + } + var project = jbsProjectFromJcheckConf(repo, targetBranch); var issue = getIssue(currentBranch, project); var file = Files.createTempFile("PULL_REQUEST_", ".md"); @@ -327,6 +438,13 @@ public static void main(String[] args) throws IOException, InterruptedException } appendPaddedHTMLComment(file, "Repository: " + parentRepo.webUrl()); appendPaddedHTMLComment(file, "Branch: " + targetBranch); + if (!mailingLists.isEmpty()) { + appendPaddedHTMLComment(file, ""); + appendPaddedHTMLComment(file, "The following mailing lists will be CC:d for the \"RFR\" e-mail:"); + for (var list : mailingLists) { + appendPaddedHTMLComment(file, "- " + list); + } + } var success = spawnEditor(repo, file); if (!success) { @@ -355,6 +473,13 @@ public static void main(String[] args) throws IOException, InterruptedException System.exit(1); } + if (isCCManual && !mailingLists.isEmpty()) { + var arg = mailingLists.stream() + .map(l -> l.split("@")[0].replace("-dev", "")) + .collect(Collectors.joining(",")); + body.add("/cc " + arg); + } + var isDraft = getSwitch("draft", "create", arguments); if (upstream.isEmpty() && shouldPublish) { GitPublish.main(new String[] { "--quiet", remote }); diff --git a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/LabelConfiguration.java b/forge/src/main/java/org/openjdk/skara/forge/LabelConfiguration.java similarity index 59% rename from bots/pr/src/main/java/org/openjdk/skara/bots/pr/LabelConfiguration.java rename to forge/src/main/java/org/openjdk/skara/forge/LabelConfiguration.java index 09779f5be..a266fe7ef 100644 --- a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/LabelConfiguration.java +++ b/forge/src/main/java/org/openjdk/skara/forge/LabelConfiguration.java @@ -20,11 +20,15 @@ * or visit www.oracle.com if you need additional information or have any * questions. */ -package org.openjdk.skara.bots.pr; +package org.openjdk.skara.forge; + +import org.openjdk.skara.json.JSONValue; +import org.openjdk.skara.json.JSONObject; import java.nio.file.Path; import java.util.*; import java.util.regex.Pattern; +import java.util.stream.Collectors; public class LabelConfiguration { private final Map> matchers; @@ -44,22 +48,22 @@ private LabelConfiguration(Map> matchers, Map> matchers = new HashMap<>(); private final Map> groups = new HashMap<>(); private final Set extra = new HashSet<>(); - public LabelConfigurationBuilder addMatchers(String label, List matchers) { + public Builder addMatchers(String label, List matchers) { this.matchers.put(label, matchers); return this; } - public LabelConfigurationBuilder addGroup(String label, List members) { + public Builder addGroup(String label, List members) { groups.put(label, members); return this; } - public LabelConfigurationBuilder addExtra(String label) { + public Builder addExtra(String label) { extra.add(label); return this; } @@ -69,11 +73,43 @@ public LabelConfiguration build() { } } - static LabelConfigurationBuilder newBuilder() { - return new LabelConfigurationBuilder(); + public static Builder builder() { + return new Builder(); + } + + public static LabelConfiguration fromJSON(JSONValue json) { + var builder = builder(); + if (json.contains("matchers")) { + var fields = json.get("matchers").fields(); + var matchers = fields.stream() + .collect(Collectors.toMap(JSONObject.Field::name, + field -> field.value() + .stream() + .map(JSONValue::asString) + .map(Pattern::compile) + .collect(Collectors.toList()))); + matchers.forEach(builder::addMatchers); + } + if (json.contains("groups")) { + var fields = json.get("groups").fields(); + var groups = fields.stream() + .collect(Collectors.toMap(JSONObject.Field::name, + field -> field.value() + .stream() + .map(JSONValue::asString) + .collect(Collectors.toList()))); + groups.forEach(builder::addGroup); + } + if (json.contains("extra")) { + var extra = json.get("extra").stream() + .map(JSONValue::asString) + .collect(Collectors.toList()); + extra.forEach(builder::addExtra); + } + return builder.build(); } - public Set fromChanges(Set changes) { + public Set label(Set changes) { var labels = new HashSet(); for (var file : changes) { for (var label : matchers.entrySet()) { @@ -108,4 +144,8 @@ public Set fromChanges(Set changes) { public Set allowed() { return allowed; } + + public boolean isAllowed(String s) { + return allowed.contains(s); + } } diff --git a/bots/pr/src/test/java/org/openjdk/skara/bots/pr/LabelConfigurationTests.java b/forge/src/test/java/org/openjdk/skara/forge/LabelConfigurationTests.java similarity index 74% rename from bots/pr/src/test/java/org/openjdk/skara/bots/pr/LabelConfigurationTests.java rename to forge/src/test/java/org/openjdk/skara/forge/LabelConfigurationTests.java index f3a9dffeb..a93ed017b 100644 --- a/bots/pr/src/test/java/org/openjdk/skara/bots/pr/LabelConfigurationTests.java +++ b/forge/src/test/java/org/openjdk/skara/forge/LabelConfigurationTests.java @@ -20,7 +20,7 @@ * or visit www.oracle.com if you need additional information or have any * questions. */ -package org.openjdk.skara.bots.pr; +package org.openjdk.skara.forge; import org.junit.jupiter.api.Test; @@ -33,21 +33,21 @@ public class LabelConfigurationTests { @Test void simple() { - var config = LabelConfiguration.newBuilder() + var config = LabelConfiguration.builder() .addMatchers("1", List.of(Pattern.compile("cpp$"))) .addMatchers("2", List.of(Pattern.compile("hpp$"))) .build(); assertEquals(Set.of("1", "2"), config.allowed()); - assertEquals(Set.of("1"), config.fromChanges(Set.of(Path.of("a.cpp")))); - assertEquals(Set.of("2"), config.fromChanges(Set.of(Path.of("a.hpp")))); - assertEquals(Set.of("1", "2"), config.fromChanges(Set.of(Path.of("a.cpp"), Path.of("a.hpp")))); + assertEquals(Set.of("1"), config.label(Set.of(Path.of("a.cpp")))); + assertEquals(Set.of("2"), config.label(Set.of(Path.of("a.hpp")))); + assertEquals(Set.of("1", "2"), config.label(Set.of(Path.of("a.cpp"), Path.of("a.hpp")))); } @Test void group() { - var config = LabelConfiguration.newBuilder() + var config = LabelConfiguration.builder() .addMatchers("1", List.of(Pattern.compile("cpp$"))) .addMatchers("2", List.of(Pattern.compile("hpp$"))) .addGroup("both", List.of("1", "2")) @@ -55,8 +55,8 @@ void group() { assertEquals(Set.of("1", "2", "both"), config.allowed()); - assertEquals(Set.of("1"), config.fromChanges(Set.of(Path.of("a.cpp")))); - assertEquals(Set.of("2"), config.fromChanges(Set.of(Path.of("a.hpp")))); - assertEquals(Set.of("both"), config.fromChanges(Set.of(Path.of("a.cpp"), Path.of("a.hpp")))); + assertEquals(Set.of("1"), config.label(Set.of(Path.of("a.cpp")))); + assertEquals(Set.of("2"), config.label(Set.of(Path.of("a.hpp")))); + assertEquals(Set.of("both"), config.label(Set.of(Path.of("a.cpp"), Path.of("a.hpp")))); } }