From 1bfd312855d5c728c8829050c83a9e11b0dea08b Mon Sep 17 00:00:00 2001 From: Patrick Doyle Date: Thu, 22 May 2025 10:49:44 -0400 Subject: [PATCH 1/6] Use walkFileTree for extractModuleNameFromDirectory --- .../plugin/GenerateTestBuildInfoTask.java | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/build-tools/src/main/java/org/elasticsearch/gradle/plugin/GenerateTestBuildInfoTask.java b/build-tools/src/main/java/org/elasticsearch/gradle/plugin/GenerateTestBuildInfoTask.java index c0f9eda18b21a..dc8dd68b65099 100644 --- a/build-tools/src/main/java/org/elasticsearch/gradle/plugin/GenerateTestBuildInfoTask.java +++ b/build-tools/src/main/java/org/elasticsearch/gradle/plugin/GenerateTestBuildInfoTask.java @@ -316,20 +316,24 @@ private String extractClassNameFromDirectory(File dir) throws IOException { * if it exists or the preset one derived from the jar task */ private String extractModuleNameFromDirectory(File dir) throws IOException { - List files = new ArrayList<>(List.of(dir)); - while (files.isEmpty() == false) { - File find = files.removeFirst(); - if (find.exists()) { - if (find.getName().equals("module-info.class")) { - try (InputStream inputStream = new FileInputStream(find)) { - return extractModuleNameFromModuleInfo(inputStream); + var visitor = new SimpleFileVisitor() { + private String result = getModuleName().getOrNull(); + + @Override + public @NotNull FileVisitResult visitFile(@NotNull Path candidate, @NotNull BasicFileAttributes attrs) throws IOException { + String name = candidate.getFileName().toString(); // Just the part after the last dir separator + if (name.equals("module-info.class")) { + try (InputStream inputStream = new FileInputStream(candidate.toFile())) { + result = extractModuleNameFromModuleInfo(inputStream); + return TERMINATE; } - } else if (find.isDirectory()) { - files.addAll(Arrays.asList(find.listFiles())); + } else { + return CONTINUE; } } - } - return getModuleName().getOrNull(); + }; + Files.walkFileTree(dir.toPath(), visitor); + return visitor.result; } /** From bd25fd537836ff922fabb7f128b3824a38be1921 Mon Sep 17 00:00:00 2001 From: Patrick Doyle Date: Thu, 22 May 2025 11:36:21 -0400 Subject: [PATCH 2/6] More tests in TestBuildInfoPluginFuncTest --- .../test/TestBuildInfoPluginFuncTest.groovy | 63 ++++++++++++++++++- 1 file changed, 62 insertions(+), 1 deletion(-) diff --git a/build-tools/src/integTest/groovy/org/elasticsearch/gradle/test/TestBuildInfoPluginFuncTest.groovy b/build-tools/src/integTest/groovy/org/elasticsearch/gradle/test/TestBuildInfoPluginFuncTest.groovy index a12e6aba7b6fd..e94041753d4a2 100644 --- a/build-tools/src/integTest/groovy/org/elasticsearch/gradle/test/TestBuildInfoPluginFuncTest.groovy +++ b/build-tools/src/integTest/groovy/org/elasticsearch/gradle/test/TestBuildInfoPluginFuncTest.groovy @@ -5,8 +5,10 @@ import com.fasterxml.jackson.databind.ObjectMapper import org.elasticsearch.gradle.fixtures.AbstractGradleFuncTest import org.gradle.testkit.runner.TaskOutcome +import java.nio.file.Path + class TestBuildInfoPluginFuncTest extends AbstractGradleFuncTest { - def "works"() { + def "basic functionality"() { given: file("src/main/java/com/example/Example.java") << """ package com.example; @@ -60,4 +62,63 @@ class TestBuildInfoPluginFuncTest extends AbstractGradleFuncTest { ) new ObjectMapper().readValue(output, Map.class) == expectedOutput } + + def "dependencies"() { + buildFile << """ + import org.elasticsearch.gradle.plugin.GenerateTestBuildInfoTask; + + plugins { + id 'java' + id 'elasticsearch.test-build-info' + } + + repositories { + mavenCentral() + } + + dependencies { + // We pin to specific versions here because they are known to have the properties we want to test. + // We're not actually running this code. + implementation "org.ow2.asm:asm:9.7.1" // has module-info.class + implementation "junit:junit:4.13" // has Automatic-Module-Name, and brings in hamcrest which does not + } + + tasks.withType(GenerateTestBuildInfoTask.class) { + componentName = 'example-component' + outputFile = new File('build/generated-build-info/plugin-test-build-info.json') + } + """ + + when: + def result = gradleRunner('generateTestBuildInfo').build() + def task = result.task(":generateTestBuildInfo") + + + then: + task.outcome == TaskOutcome.SUCCESS + + def output = file("build/generated-build-info/plugin-test-build-info.json") + output.exists() == true + + def locationFromModuleInfo = Map.of( + "module", "org.objectweb.asm", + "representative_class", Path.of('org', 'objectweb', 'asm', 'AnnotationVisitor.class').toString() + ) + def locationFromManifest = Map.of( + "module", "junit", + "representative_class", Path.of('junit', 'textui', 'TestRunner.class').toString() + ) + def locationFromJarFileName = Map.of( + "module", "hamcrest.core", + "representative_class", Path.of('org', 'hamcrest', 'BaseDescription.class').toString() + ) + def expectedOutput = Map.of( + "component", "example-component", + "locations", List.of(locationFromModuleInfo, locationFromManifest, locationFromJarFileName) + ) + + def value = new ObjectMapper().readValue(output, Map.class) + expectedOutput.forEach((k,v) -> value.get(k) == v) + value == expectedOutput + } } From 29ad38a97d4254f57f71fed165a7b64bff89491d Mon Sep 17 00:00:00 2001 From: Patrick Doyle Date: Thu, 22 May 2025 11:43:12 -0400 Subject: [PATCH 3/6] Remove stray line from debugging --- .../elasticsearch/gradle/test/TestBuildInfoPluginFuncTest.groovy | 1 - 1 file changed, 1 deletion(-) diff --git a/build-tools/src/integTest/groovy/org/elasticsearch/gradle/test/TestBuildInfoPluginFuncTest.groovy b/build-tools/src/integTest/groovy/org/elasticsearch/gradle/test/TestBuildInfoPluginFuncTest.groovy index e94041753d4a2..d837bde443239 100644 --- a/build-tools/src/integTest/groovy/org/elasticsearch/gradle/test/TestBuildInfoPluginFuncTest.groovy +++ b/build-tools/src/integTest/groovy/org/elasticsearch/gradle/test/TestBuildInfoPluginFuncTest.groovy @@ -118,7 +118,6 @@ class TestBuildInfoPluginFuncTest extends AbstractGradleFuncTest { ) def value = new ObjectMapper().readValue(output, Map.class) - expectedOutput.forEach((k,v) -> value.get(k) == v) value == expectedOutput } } From b8e068ef6f328b171e7b535f147c7f1eb208d898 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Thu, 22 May 2025 15:52:33 +0000 Subject: [PATCH 4/6] [CI] Auto commit changes from spotless --- .../elasticsearch/gradle/plugin/GenerateTestBuildInfoTask.java | 1 - 1 file changed, 1 deletion(-) diff --git a/build-tools/src/main/java/org/elasticsearch/gradle/plugin/GenerateTestBuildInfoTask.java b/build-tools/src/main/java/org/elasticsearch/gradle/plugin/GenerateTestBuildInfoTask.java index dc8dd68b65099..3793e82d99f9f 100644 --- a/build-tools/src/main/java/org/elasticsearch/gradle/plugin/GenerateTestBuildInfoTask.java +++ b/build-tools/src/main/java/org/elasticsearch/gradle/plugin/GenerateTestBuildInfoTask.java @@ -41,7 +41,6 @@ import java.nio.file.attribute.BasicFileAttributes; import java.security.CodeSource; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import java.util.jar.JarEntry; import java.util.jar.JarFile; From 73bc8fdaa24a4cfd5b2880a7909cbb319dd93c8a Mon Sep 17 00:00:00 2001 From: Patrick Doyle Date: Fri, 23 May 2025 10:10:09 -0400 Subject: [PATCH 5/6] Eliminate List.reversed() call --- .../gradle/plugin/GenerateTestBuildInfoTask.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/build-tools/src/main/java/org/elasticsearch/gradle/plugin/GenerateTestBuildInfoTask.java b/build-tools/src/main/java/org/elasticsearch/gradle/plugin/GenerateTestBuildInfoTask.java index 3793e82d99f9f..b028f05f50a84 100644 --- a/build-tools/src/main/java/org/elasticsearch/gradle/plugin/GenerateTestBuildInfoTask.java +++ b/build-tools/src/main/java/org/elasticsearch/gradle/plugin/GenerateTestBuildInfoTask.java @@ -41,6 +41,7 @@ import java.nio.file.attribute.BasicFileAttributes; import java.security.CodeSource; import java.util.ArrayList; +import java.util.Comparator; import java.util.List; import java.util.jar.JarEntry; import java.util.jar.JarFile; @@ -208,6 +209,7 @@ private String extractModuleNameFromJar(File file, JarFile jarFile) throws IOExc * @return a {@link StringBuilder} with the {@code META-INF/versions/} if it exists; otherwise null */ private static StringBuilder versionDirectoryIfExists(JarFile jarFile) { + Comparator numericOrder = Integer::compareTo; List versions = jarFile.stream() .filter(je -> je.getName().startsWith(META_INF_VERSIONS_PREFIX) && je.getName().endsWith("/module-info.class")) .map( @@ -215,10 +217,8 @@ private static StringBuilder versionDirectoryIfExists(JarFile jarFile) { je.getName().substring(META_INF_VERSIONS_PREFIX.length(), je.getName().length() - META_INF_VERSIONS_PREFIX.length()) ) ) + .sorted(numericOrder.reversed()) .toList(); - versions = new ArrayList<>(versions); - versions.sort(Integer::compareTo); - versions = versions.reversed(); int major = Runtime.version().feature(); StringBuilder path = new StringBuilder(META_INF_VERSIONS_PREFIX); for (int version : versions) { From 7473692629d5aa277b0e0ba2ac8acb3e58172fbd Mon Sep 17 00:00:00 2001 From: Patrick Doyle Date: Fri, 23 May 2025 14:02:53 -0400 Subject: [PATCH 6/6] Standardize on forward slashes for representative_class. Turns out even on Windows, jar entries use forward slashes. Let's be consistent in all cases. --- .../gradle/test/TestBuildInfoPluginFuncTest.groovy | 10 ++++------ .../gradle/plugin/GenerateTestBuildInfoTask.java | 5 ++++- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/build-tools/src/integTest/groovy/org/elasticsearch/gradle/test/TestBuildInfoPluginFuncTest.groovy b/build-tools/src/integTest/groovy/org/elasticsearch/gradle/test/TestBuildInfoPluginFuncTest.groovy index fc209ea451a5e..a652deb726d1d 100644 --- a/build-tools/src/integTest/groovy/org/elasticsearch/gradle/test/TestBuildInfoPluginFuncTest.groovy +++ b/build-tools/src/integTest/groovy/org/elasticsearch/gradle/test/TestBuildInfoPluginFuncTest.groovy @@ -5,8 +5,6 @@ import com.fasterxml.jackson.databind.ObjectMapper import org.elasticsearch.gradle.fixtures.AbstractGradleFuncTest import org.gradle.testkit.runner.TaskOutcome -import java.nio.file.Path - class TestBuildInfoPluginFuncTest extends AbstractGradleFuncTest { def "basic functionality"() { given: @@ -54,7 +52,7 @@ class TestBuildInfoPluginFuncTest extends AbstractGradleFuncTest { def location = Map.of( "module", "com.example", - "representative_class", Path.of("com", "example", "Example.class").toString() + "representative_class", "com/example/Example.class" ) def expectedOutput = Map.of( "component", "example-component", @@ -102,15 +100,15 @@ class TestBuildInfoPluginFuncTest extends AbstractGradleFuncTest { def locationFromModuleInfo = Map.of( "module", "org.objectweb.asm", - "representative_class", Path.of('org', 'objectweb', 'asm', 'AnnotationVisitor.class').toString() + "representative_class", 'org/objectweb/asm/AnnotationVisitor.class' ) def locationFromManifest = Map.of( "module", "junit", - "representative_class", Path.of('junit', 'textui', 'TestRunner.class').toString() + "representative_class", 'junit/textui/TestRunner.class' ) def locationFromJarFileName = Map.of( "module", "hamcrest.core", - "representative_class", Path.of('org', 'hamcrest', 'BaseDescription.class').toString() + "representative_class", 'org/hamcrest/BaseDescription.class' ) def expectedOutput = Map.of( "component", "example-component", diff --git a/build-tools/src/main/java/org/elasticsearch/gradle/plugin/GenerateTestBuildInfoTask.java b/build-tools/src/main/java/org/elasticsearch/gradle/plugin/GenerateTestBuildInfoTask.java index b028f05f50a84..f015942eb7152 100644 --- a/build-tools/src/main/java/org/elasticsearch/gradle/plugin/GenerateTestBuildInfoTask.java +++ b/build-tools/src/main/java/org/elasticsearch/gradle/plugin/GenerateTestBuildInfoTask.java @@ -299,7 +299,10 @@ private String extractClassNameFromDirectory(File dir) throws IOException { public @NotNull FileVisitResult visitFile(@NotNull Path candidate, @NotNull BasicFileAttributes attrs) { String name = candidate.getFileName().toString(); // Just the part after the last dir separator if (name.endsWith(".class") && (name.equals("module-info.class") || name.contains("$")) == false) { - result = candidate.toAbsolutePath().toString().substring(dir.getAbsolutePath().length() + 1); + result = candidate.toAbsolutePath() + .toString() + .substring(dir.getAbsolutePath().length() + 1) + .replace(File.separatorChar, '/'); return TERMINATE; } else { return CONTINUE;