Copilot commented on code in PR #6860:
URL: https://github.com/apache/incubator-kie/pull/6860#discussion_r3802646786
##########
script/ci/CiComputeBuildScopes.java:
##########
@@ -202,6 +220,102 @@ private static void writeLines(Path out,
Collection<String> lines) throws IOExce
Files.write(out, sorted);
}
+ private static final List<String> EXPECTED_CATEGORIES =
List.of("optaplanner", "kogito-runtimes", "kogito-apps");
+
+ static Map<String, Set<Path>> parseModuleCategories(Path rootPom, Path
cwd) throws IOException {
+ Map<String, Set<Path>> categories = new LinkedHashMap<>();
+ categories.put("drools", new LinkedHashSet<>());
+ for (String cat : EXPECTED_CATEGORIES) {
+ categories.put(cat, new LinkedHashSet<>());
+ }
+
+ String currentCategory = "drools";
+ boolean inModules = false;
+ Set<String> seenBegins = new HashSet<>();
+ Set<String> seenEnds = new HashSet<>();
+
+ Pattern beginPattern =
Pattern.compile("<!--\\s*BEGIN\\s+(\\S+)\\s+modules\\s+\\(auto\\)\\s*-->");
+ Pattern endPattern =
Pattern.compile("<!--\\s*END\\s+(\\S+)\\s+modules\\s+\\(auto\\)\\s*-->");
+ Pattern modulePattern = Pattern.compile("<module>(.+)</module>");
+
+ for (String line : Files.readAllLines(rootPom)) {
+ String trimmed = line.trim();
+
+ if (trimmed.equals("<modules>")) {
+ inModules = true;
+ continue;
+ }
+ if (trimmed.equals("</modules>")) {
+ break;
+ }
+ if (!inModules) continue;
+
+ Matcher beginMatcher = beginPattern.matcher(trimmed);
+ if (beginMatcher.matches()) {
+ currentCategory = beginMatcher.group(1);
+ if (!categories.containsKey(currentCategory)) {
+ System.err.println("ERROR: unknown module category '" +
currentCategory
+ + "' in pom.xml marker. Expected one of: " +
EXPECTED_CATEGORIES);
+ System.exit(1);
+ }
+ seenBegins.add(currentCategory);
+ continue;
+ }
+
+ Matcher endMatcher = endPattern.matcher(trimmed);
+ if (endMatcher.matches()) {
+ seenEnds.add(endMatcher.group(1));
+ currentCategory = "drools";
+ continue;
+ }
+
+ Matcher moduleMatcher = modulePattern.matcher(trimmed);
+ if (moduleMatcher.matches()) {
+ String modulePath = moduleMatcher.group(1);
+
categories.get(currentCategory).add(cwd.resolve(modulePath).toAbsolutePath().normalize());
+ }
+ }
+
+ for (String cat : EXPECTED_CATEGORIES) {
+ if (!seenBegins.contains(cat)) {
+ System.err.println("ERROR: missing '<!-- BEGIN " + cat
+ + " modules (auto) -->' marker in pom.xml");
+ System.exit(1);
+ }
+ if (!seenEnds.contains(cat)) {
+ System.err.println("ERROR: missing '<!-- END " + cat
+ + " modules (auto) -->' marker in pom.xml");
+ System.exit(1);
+ }
+ }
+
+ return categories;
+ }
+
+ private static String categorizeGa(String ga, Map<String, Path> gaToDir,
+ Map<String, Set<Path>> categories) {
+ Path dir = gaToDir.get(ga);
+ if (dir == null) return "drools";
+ dir = dir.toAbsolutePath().normalize();
+ for (var entry : categories.entrySet()) {
+ for (Path catPath : entry.getValue()) {
+ if (dir.equals(catPath) || dir.startsWith(catPath + "/")) {
+ return entry.getKey();
+ }
+ }
+ }
+ return "drools";
+ }
Review Comment:
`dir.startsWith(catPath + "/")` is not path-separator safe (notably on
Windows where separators are `\`), and it also bypasses `Path` segment
semantics by converting to a `String`. Use `dir.startsWith(catPath)` (which
already correctly handles descendants by path segments) and drop the `+ "/"`
logic.
##########
script/ci/CiComputeBuildScopes.java:
##########
@@ -166,13 +167,30 @@ public static void main(String[] args) throws Exception {
writeLines(affectedOut, affected);
writeLines(changedOut, changed);
+ Map<String, Set<Path>> categories =
parseModuleCategories(cwd.resolve("pom.xml"), cwd);
+
+ for (String cat : categories.keySet()) {
+ Set<String> catAffected = affected.stream()
+ .filter(ga -> categorizeGa(ga, gaToDir,
categories).equals(cat))
+ .collect(Collectors.toCollection(LinkedHashSet::new));
+ writeLines(partitionedPath(affectedOut, cat), catAffected);
+ }
+
int total = gaToDir.size();
int ignored = total - affected.size() - upstreamAll.size();
- System.out.println("total=" + total
- + " changed=" + changed.size()
- + " affected=" + affected.size()
- + " upstream=" + upstreamAll.size()
- + " ignored=" + ignored);
+ StringBuilder sb = new StringBuilder();
+ sb.append("total=").append(total)
+ .append(" changed=").append(changed.size())
+ .append(" affected=").append(affected.size())
+ .append(" upstream=").append(upstreamAll.size())
+ .append(" ignored=").append(ignored);
+ for (String cat : categories.keySet()) {
+ long catCount = affected.stream()
+ .filter(ga -> categorizeGa(ga, gaToDir,
categories).equals(cat))
+ .count();
+ sb.append(" affected-").append(cat).append("=").append(catCount);
+ }
Review Comment:
`categorizeGa(...)` is called repeatedly for every category (and again for
summary counts), which scales poorly as `affected` grows. Precompute a map `ga
-> category` once (or group `affected` using `Collectors.groupingBy(...)`) and
reuse it both for writing category files and for the per-category counts.
##########
script/ci/CiComputeBuildScopes.java:
##########
@@ -166,13 +167,30 @@ public static void main(String[] args) throws Exception {
writeLines(affectedOut, affected);
writeLines(changedOut, changed);
+ Map<String, Set<Path>> categories =
parseModuleCategories(cwd.resolve("pom.xml"), cwd);
+
+ for (String cat : categories.keySet()) {
+ Set<String> catAffected = affected.stream()
+ .filter(ga -> categorizeGa(ga, gaToDir,
categories).equals(cat))
+ .collect(Collectors.toCollection(LinkedHashSet::new));
+ writeLines(partitionedPath(affectedOut, cat), catAffected);
+ }
Review Comment:
`categorizeGa(...)` is called repeatedly for every category (and again for
summary counts), which scales poorly as `affected` grows. Precompute a map `ga
-> category` once (or group `affected` using `Collectors.groupingBy(...)`) and
reuse it both for writing category files and for the per-category counts.
##########
script/ci/CiComputeBuildScopes.java:
##########
@@ -202,6 +220,102 @@ private static void writeLines(Path out,
Collection<String> lines) throws IOExce
Files.write(out, sorted);
}
+ private static final List<String> EXPECTED_CATEGORIES =
List.of("optaplanner", "kogito-runtimes", "kogito-apps");
+
+ static Map<String, Set<Path>> parseModuleCategories(Path rootPom, Path
cwd) throws IOException {
+ Map<String, Set<Path>> categories = new LinkedHashMap<>();
+ categories.put("drools", new LinkedHashSet<>());
+ for (String cat : EXPECTED_CATEGORIES) {
+ categories.put(cat, new LinkedHashSet<>());
+ }
+
+ String currentCategory = "drools";
+ boolean inModules = false;
+ Set<String> seenBegins = new HashSet<>();
+ Set<String> seenEnds = new HashSet<>();
+
+ Pattern beginPattern =
Pattern.compile("<!--\\s*BEGIN\\s+(\\S+)\\s+modules\\s+\\(auto\\)\\s*-->");
+ Pattern endPattern =
Pattern.compile("<!--\\s*END\\s+(\\S+)\\s+modules\\s+\\(auto\\)\\s*-->");
+ Pattern modulePattern = Pattern.compile("<module>(.+)</module>");
+
+ for (String line : Files.readAllLines(rootPom)) {
+ String trimmed = line.trim();
+
+ if (trimmed.equals("<modules>")) {
+ inModules = true;
+ continue;
+ }
+ if (trimmed.equals("</modules>")) {
+ break;
+ }
+ if (!inModules) continue;
+
+ Matcher beginMatcher = beginPattern.matcher(trimmed);
+ if (beginMatcher.matches()) {
+ currentCategory = beginMatcher.group(1);
+ if (!categories.containsKey(currentCategory)) {
+ System.err.println("ERROR: unknown module category '" +
currentCategory
+ + "' in pom.xml marker. Expected one of: " +
EXPECTED_CATEGORIES);
Review Comment:
The error message says “Expected one of: [optaplanner, kogito-runtimes,
kogito-apps]” but `drools` is also a valid category (it’s always present in
`categories`). Include `drools` in the “Expected one of” list (or build the
message from `categories.keySet()`) so the guidance matches what the code
actually accepts.
##########
pom.xml:
##########
@@ -152,6 +152,9 @@
</build>
<modules>
+ <!-- DO NOT remove the BEGIN/END comments. They are used by
CiComputeBuildScopes for parallel CI job split
+ When you add a new module, put it in one of the 4 categories (drools,
optaplanner, kogito-runtimes, kogito-apps) -->
Review Comment:
This comment implies the BEGIN/END markers are mandatory for all categories,
but `parseModuleCategories(...)` only enforces markers for `optaplanner`,
`kogito-runtimes`, and `kogito-apps` (not `drools`). Either (a) also enforce
the drools BEGIN/END markers in code for consistency, or (b) adjust the comment
to reflect which categories are strictly validated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
