Browse Source

Improved validation and added support to include incubator modules (#135529)

Moritz Mack 2 weeks ago
parent
commit
f23c9fd2e1

+ 9 - 4
libs/entitlement/tools/common/src/main/java/org/elasticsearch/entitlement/tools/Utils.java

@@ -20,6 +20,7 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
 public class Utils {
@@ -42,6 +43,10 @@ public class Utils {
         "jdk.localedata" // noise, change here are not interesting
     );
 
+    public static final Predicate<String> DEFAULT_MODULE_PREDICATE = m -> EXCLUDED_MODULES.contains(m) == false
+        && m.contains(".internal.") == false
+        && m.contains(".incubator.") == false;
+
     private static Map<String, Set<String>> findModuleExports(FileSystem fs) throws IOException {
         var modulesExports = new HashMap<String, Set<String>>();
         try (var stream = Files.walk(fs.getPath("modules"))) {
@@ -69,20 +74,20 @@ public class Utils {
     }
 
     public static void walkJdkModules(JdkModuleConsumer c) throws IOException {
+        walkJdkModules(DEFAULT_MODULE_PREDICATE, c);
+    }
 
+    public static void walkJdkModules(Predicate<String> modulePredicate, JdkModuleConsumer c) throws IOException {
         FileSystem fs = FileSystems.getFileSystem(URI.create("jrt:/"));
 
         var moduleExports = Utils.findModuleExports(fs);
-
         try (var stream = Files.walk(fs.getPath("modules"))) {
             var modules = stream.filter(x -> x.toString().endsWith(".class"))
                 .collect(Collectors.groupingBy(x -> x.subpath(1, 2).toString()));
 
             for (var kv : modules.entrySet()) {
                 var moduleName = kv.getKey();
-                if (Utils.EXCLUDED_MODULES.contains(moduleName) == false
-                    && moduleName.contains(".internal.") == false
-                    && moduleName.contains(".incubator.") == false) {
+                if (modulePredicate.test(moduleName)) {
                     var thisModuleExports = moduleExports.get(moduleName);
                     c.accept(moduleName, kv.getValue(), thisModuleExports);
                 }

+ 4 - 1
libs/entitlement/tools/jdk-api-extractor/README.md

@@ -13,7 +13,10 @@ Usage example:
 diff libs/entitlement/tools/jdk-api-extractor/api-jdk24.tsv libs/entitlement/tools/jdk-api-extractor/api-jdk25.tsv
 ```
 
-To review the diff of deprecations (by means of `@Deprecated`), use `--deprecations-only` as 2nd argument.
+### Optional arguments:
+
+- `--deprecations-only`: reports public deprecations (by means of `@Deprecated`)
+- `--include-incubator`: include incubator modules (e.g. `jdk.incubator.vector`)
 
 ```bash
 ./gradlew :libs:entitlement:tools:jdk-api-extractor:run -Druntime.java=24 --args="deprecations-jdk24.tsv --deprecations-only"

+ 8 - 0
libs/entitlement/tools/jdk-api-extractor/build.gradle

@@ -16,8 +16,16 @@ ext {
   javaMainClass = "org.elasticsearch.entitlement.tools.jdkapi.JdkApiExtractor"
 }
 
+def addIncubatorModules = {
+  file("${buildParams.runtimeJavaHome.get()}/jmods")
+    .listFiles()
+    .findAll { it.name.endsWith('.jmod') && it.name.contains('.incubator.') }
+    .collectMany { ['--add-modules', it.name[0..-6]] }
+}
+
 application {
   mainClass.set(javaMainClass)
+  applicationDefaultJvmArgs = addIncubatorModules()
 }
 
 tasks.named("run").configure {

+ 39 - 5
libs/entitlement/tools/jdk-api-extractor/src/main/java/org/elasticsearch/entitlement/tools/jdkapi/JdkApiExtractor.java

@@ -20,12 +20,16 @@ import java.lang.constant.ClassDesc;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.Map;
 import java.util.Set;
 import java.util.TreeMap;
 import java.util.TreeSet;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import static org.objectweb.asm.Opcodes.ACC_DEPRECATED;
@@ -49,15 +53,24 @@ public class JdkApiExtractor {
         new AccessibleMethod("close", "()V", true, false, true)
     );
 
+    private static String DEPRECATIONS_ONLY = "--deprecations-only";
+    private static String INCLUDE_INCUBATOR = "--include-incubator";
+
+    private static Set<String> OPTIONAL_ARGS = Set.of(DEPRECATIONS_ONLY, INCLUDE_INCUBATOR);
+
     public static void main(String[] args) throws IOException {
         validateArgs(args);
-        boolean deprecationsOnly = args.length == 2 && args[1].equals("--deprecations-only");
+        boolean deprecationsOnly = optionalArgs(args).anyMatch(DEPRECATIONS_ONLY::equals);
 
         Map<String, Set<AccessibleMethod>> accessibleImplementationsByClass = new TreeMap<>();
         Map<String, Set<AccessibleMethod>> accessibleForOverridesByClass = new TreeMap<>();
         Map<String, Set<AccessibleMethod>> deprecationsByClass = new TreeMap<>();
 
-        Utils.walkJdkModules((moduleName, moduleClasses, moduleExports) -> {
+        Predicate<String> modulePredicate = Utils.DEFAULT_MODULE_PREDICATE.or(
+            m -> optionalArgs(args).anyMatch(INCLUDE_INCUBATOR::equals) && m.contains(".incubator.")
+        );
+
+        Utils.walkJdkModules(modulePredicate, (moduleName, moduleClasses, moduleExports) -> {
             var visitor = new AccessibleClassVisitor(
                 moduleExports,
                 accessibleImplementationsByClass,
@@ -87,16 +100,37 @@ public class JdkApiExtractor {
         return relativePath.substring(0, relativePath.length() - ".class".length());
     }
 
+    private static Stream<String> optionalArgs(String[] args) {
+        return Arrays.stream(args).skip(1);
+    }
+
     @SuppressForbidden(reason = "cli tool printing to standard err/out")
     private static void validateArgs(String[] args) {
-        boolean valid = args.length == 1 || (args.length == 2 && "--deprecations-only".equals(args[1]));
-
+        boolean valid = args.length > 0 && optionalArgs(args).allMatch(OPTIONAL_ARGS::contains);
+        if (valid && isWritableOutputPath(args[0]) == false) {
+            valid = false;
+            System.err.println("invalid output path: " + args[0]);
+        }
         if (valid == false) {
-            System.err.println("usage: <output file path> [--deprecations-only]");
+            String optionalArgs = OPTIONAL_ARGS.stream().collect(Collectors.joining("] [", " [", "]"));
+            System.err.println("usage: <output file path>" + optionalArgs);
             System.exit(1);
         }
     }
 
+    private static boolean isWritableOutputPath(String pathStr) {
+        try {
+            Path path = Paths.get(pathStr);
+            if (Files.exists(path) && Files.isRegularFile(path)) {
+                return Files.isWritable(path);
+            }
+            Path parent = path.toAbsolutePath().getParent();
+            return parent != null && Files.isDirectory(parent) && Files.isWritable(parent);
+        } catch (Exception e) {
+            return false;
+        }
+    }
+
     @SuppressForbidden(reason = "cli tool printing to standard err/out")
     private static void writeFile(Path path, Map<String, Set<AccessibleMethod>> methods) throws IOException {
         System.out.println("Writing result for " + Runtime.version() + " to " + path.toAbsolutePath());