From ba3f3ada8f0c7f59e40998265cbd046f2940d6ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20W=C3=B6gerer?= Date: Thu, 20 Jul 2023 16:14:19 +0200 Subject: [PATCH 1/5] Allow builder module path extension via macro options --- .../src/com/oracle/svm/driver/MacroOptionHandler.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/MacroOptionHandler.java b/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/MacroOptionHandler.java index 102ff101f804..24fe590ac660 100644 --- a/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/MacroOptionHandler.java +++ b/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/MacroOptionHandler.java @@ -95,6 +95,8 @@ private void applyEnabled(MacroOption.EnabledOption enabledOption, String argume enabledOption.forEachPropertyValue(config, "ImageBuilderClasspath", entry -> nativeImage.addImageBuilderClasspath(Path.of(entry)), PATH_SEPARATOR_REGEX); + enabledOption.forEachPropertyValue(config, + "ImageBuilderModulePath", entry -> nativeImage.addImageBuilderModulePath(Path.of(entry)), PATH_SEPARATOR_REGEX); boolean explicitImageModulePath = enabledOption.forEachPropertyValue(config, "ImageModulePath", entry -> nativeImage.addImageModulePath(Path.of((entry))), PATH_SEPARATOR_REGEX); boolean explicitImageClasspath = enabledOption.forEachPropertyValue(config, From d9b47fe10760fff8b65b9db43b16229f791ef111 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20W=C3=B6gerer?= Date: Thu, 20 Jul 2023 16:15:54 +0200 Subject: [PATCH 2/5] Only forward --add-modules arguments that refer to modules of the builder VM --- .../svm/driver/DefaultOptionHandler.java | 2 - .../com/oracle/svm/driver/NativeImage.java | 44 ++++++++++++++++++- 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/DefaultOptionHandler.java b/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/DefaultOptionHandler.java index 15a574dffe3e..2bb267dbfc90 100644 --- a/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/DefaultOptionHandler.java +++ b/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/DefaultOptionHandler.java @@ -93,7 +93,6 @@ public boolean consume(ArgumentQueue args) { if (addModulesArgs == null) { NativeImage.showError(headArg + moduleSetModifierOptionErrorMessage); } - nativeImage.addImageBuilderJavaArgs(addModulesOption, addModulesArgs); nativeImage.addAddedModules(addModulesArgs); return true; case limitModulesOption: @@ -189,7 +188,6 @@ public boolean consume(ArgumentQueue args) { if (addModulesArgs.isEmpty()) { NativeImage.showError(headArg + moduleSetModifierOptionErrorMessage); } - nativeImage.addImageBuilderJavaArgs(addModulesOption, addModulesArgs); nativeImage.addAddedModules(addModulesArgs); return true; } diff --git a/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java b/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java index 269b77e7c899..d85ae5e72217 100644 --- a/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java +++ b/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java @@ -1430,6 +1430,15 @@ protected int buildImage(List javaArgs, LinkedHashSet cp, LinkedHa arguments.addAll(strings); } + String javaExecutable = canonicalize(config.getJavaExecutable()).toString(); + if (!addModules.isEmpty()) { + var builderModules = getBuilderObservableModules(javaExecutable, arguments); + var addModulesForBuilderVM = addModules.stream().filter(builderModules::contains).toList(); + if (!addModulesForBuilderVM.isEmpty()) { + arguments.add(DefaultOptionHandler.addModulesOption + "=" + String.join(",", addModulesForBuilderVM)); + } + } + arguments.addAll(config.getGeneratorMainClass()); if (IS_AOT && OS.getCurrent().hasProcFS) { @@ -1455,7 +1464,6 @@ protected int buildImage(List javaArgs, LinkedHashSet cp, LinkedHa /* Construct ProcessBuilder command from final arguments */ List command = new ArrayList<>(); - String javaExecutable = canonicalize(config.getJavaExecutable()).toString(); command.add(javaExecutable); command.add(createVMInvocationArgumentFile(arguments)); command.add(createImageBuilderArgumentFile(finalImageBuilderArgs)); @@ -1520,6 +1528,40 @@ protected int buildImage(List javaArgs, LinkedHashSet cp, LinkedHa } } + private Set getBuilderObservableModules(String javaExecutable, List arguments) { + Process listModulesProcess = null; + Set result = new HashSet<>(); + try { + var pb = new ProcessBuilder(javaExecutable); + pb.command().addAll(arguments); + pb.command().add("--list-modules"); + pb.environment().clear(); + listModulesProcess = pb.start(); + try (var br = new BufferedReader(new InputStreamReader(listModulesProcess.getInputStream()))) { + while (true) { + var line = br.readLine(); + if (line == null) { + break; + } + var moduleNameColumn = StringUtil.split(line, " ", 2)[0]; + var versionStrippedModuleName = StringUtil.split(moduleNameColumn, "@", 2)[0]; + result.add(versionStrippedModuleName); + } + } + int exitStatus = listModulesProcess.waitFor(); + if (exitStatus != 0) { + throw showError("Determining image-builder observable modules failed (Exit status %d).".formatted(exitStatus)); + } + } catch (IOException | InterruptedException e) { + throw showError(e.getMessage()); + } finally { + if (listModulesProcess != null) { + listModulesProcess.destroy(); + } + } + return result; + } + /** * Adds a shutdown hook to kill the image builder process if it's still alive. * From 03cbf9b190635c4d5a3a392ad7fe9ecbc84d9b06 Mon Sep 17 00:00:00 2001 From: Christian Humer Date: Tue, 25 Jul 2023 22:23:41 +0200 Subject: [PATCH 3/5] Implement a filter for the application-module-path and image addmods to ensure boot or builder modules are never used there. --- .../com/oracle/svm/driver/NativeImage.java | 116 ++++++++++++++---- 1 file changed, 94 insertions(+), 22 deletions(-) diff --git a/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java b/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java index d85ae5e72217..b60eef4c7340 100644 --- a/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java +++ b/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java @@ -30,6 +30,7 @@ import java.io.InputStream; import java.io.InputStreamReader; import java.lang.reflect.Method; +import java.net.URI; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.InvalidPathException; @@ -44,6 +45,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; +import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; import java.util.ListIterator; @@ -1178,9 +1180,6 @@ private int completeImageBuild() { return ExitStatus.FALLBACK_IMAGE.getValue(); } - if (!addModules.isEmpty()) { - imageBuilderJavaArgs.add("-D" + ModuleSupport.PROPERTY_IMAGE_EXPLICITLY_ADDED_MODULES + "=" + String.join(",", addModules)); - } if (!limitModules.isEmpty()) { imageBuilderJavaArgs.add("-D" + ModuleSupport.PROPERTY_IMAGE_EXPLICITLY_LIMITED_MODULES + "=" + String.join(",", limitModules)); } @@ -1425,17 +1424,49 @@ protected int buildImage(List javaArgs, LinkedHashSet cp, LinkedHa if (!cp.isEmpty()) { arguments.addAll(Arrays.asList("-cp", cp.stream().map(Path::toString).collect(Collectors.joining(File.pathSeparator)))); } + if (!mp.isEmpty()) { List strings = Arrays.asList("--module-path", mp.stream().map(Path::toString).collect(Collectors.joining(File.pathSeparator))); arguments.addAll(strings); } String javaExecutable = canonicalize(config.getJavaExecutable()).toString(); + + if (useBundle()) { + LogUtils.warning("Native Image Bundles are an experimental feature."); + } + + BiFunction substituteAuxiliaryPath = useBundle() ? bundleSupport::substituteAuxiliaryPath : (a, b) -> a; + Function imageArgsTransformer = rawArg -> apiOptionHandler.transformBuilderArgument(rawArg, substituteAuxiliaryPath); + List finalImageArgs = imageArgs.stream().map(imageArgsTransformer).collect(Collectors.toList()); + Function substituteClassPath = useBundle() ? bundleSupport::substituteClassPath : Function.identity(); + List finalImageClassPath = imagecp.stream().map(substituteClassPath).collect(Collectors.toList()); + Function substituteModulePath = useBundle() ? bundleSupport::substituteModulePath : Function.identity(); + List substitutedModulePath = imagemp.stream().map(substituteModulePath).toList(); + + Map modules = listModulesFromPath(javaExecutable, Stream.concat(mp.stream(), imagemp.stream()).distinct().toList()); if (!addModules.isEmpty()) { - var builderModules = getBuilderObservableModules(javaExecutable, arguments); - var addModulesForBuilderVM = addModules.stream().filter(builderModules::contains).toList(); + List addModulesForBuilderVM = new ArrayList<>(); + List addModulesForImage = new ArrayList<>(); + + for (String module : addModules) { + Path jarPath = modules.get(module); + if (jarPath == null) { + // boot module + addModulesForBuilderVM.add(module); + } else { + // non boot module + addModulesForImage.add(module); + } + } + if (!addModulesForBuilderVM.isEmpty()) { - arguments.add(DefaultOptionHandler.addModulesOption + "=" + String.join(",", addModulesForBuilderVM)); + arguments.add(DefaultOptionHandler.addModulesOption + "=" + String.join(",", + addModulesForBuilderVM)); + } + if (!addModulesForImage.isEmpty()) { + arguments.add("-D" + ModuleSupport.PROPERTY_IMAGE_EXPLICITLY_ADDED_MODULES + "=" + + String.join(",", addModulesForImage)); } } @@ -1449,18 +1480,34 @@ protected int buildImage(List javaArgs, LinkedHashSet cp, LinkedHa arguments.addAll(Arrays.asList(SubstrateOptions.WATCHPID_PREFIX, "" + ProcessProperties.getProcessID())); } - if (useBundle()) { - LogUtils.warning("Native Image Bundles are an experimental feature."); - } + /* + * Workaround for GR-47186: Native image cannot handle modules on the image module path, + * that are also already installed in the JDK as boot module. As a workaround we filter all + * modules from the module-path that are either already installed in the JDK as boot module, + * or were explicitly added to the builder module-path. + * + * First compute all jar paths that may remain on the module-path. + */ + Set retainedModulePaths = modules.entrySet().stream() + .filter((entry) -> entry.getValue() != null && !mp.contains(entry.getValue())) + .map((entry) -> entry.getValue().toAbsolutePath().normalize()) + .collect(Collectors.toSet()); - BiFunction substituteAuxiliaryPath = useBundle() ? bundleSupport::substituteAuxiliaryPath : (a, b) -> a; - Function imageArgsTransformer = rawArg -> apiOptionHandler.transformBuilderArgument(rawArg, substituteAuxiliaryPath); - List finalImageArgs = imageArgs.stream().map(imageArgsTransformer).collect(Collectors.toList()); - Function substituteClassPath = useBundle() ? bundleSupport::substituteClassPath : Function.identity(); - List finalImageClassPath = imagecp.stream().map(substituteClassPath).collect(Collectors.toList()); - Function substituteModulePath = useBundle() ? bundleSupport::substituteModulePath : Function.identity(); - List finalImageModulePath = imagemp.stream().map(substituteModulePath).collect(Collectors.toList()); - List finalImageBuilderArgs = createImageBuilderArgs(finalImageArgs, finalImageClassPath, finalImageModulePath); + /* + * Now we need to filter the substituted module path list for all the modules that may + * remain on the module-path. + * + * This should normally not be necessary, as the retainedModulePaths should already be the + * set of jar files for the image module path. Nevertheless, we use the original definition + * of the module path to preserve the order of the original module path and as a precaution + * to protect against --list-modules returning too many modules. + */ + List finalModulePath = substitutedModulePath.stream() + .map((path) -> path.toAbsolutePath().normalize()) + .filter((path) -> retainedModulePaths.contains(path)) + .toList(); + + List finalImageBuilderArgs = createImageBuilderArgs(finalImageArgs, finalImageClassPath, finalModulePath); /* Construct ProcessBuilder command from final arguments */ List command = new ArrayList<>(); @@ -1528,9 +1575,26 @@ protected int buildImage(List javaArgs, LinkedHashSet cp, LinkedHa } } - private Set getBuilderObservableModules(String javaExecutable, List arguments) { + /** + * Resolves and lists all modules given a module path. + * + * @see #callListModules(String, List) + */ + private static Map listModulesFromPath(String javaExecutable, Collection modulePath) { + return callListModules(javaExecutable, List.of("-p", modulePath.stream().map(Path::toString).collect(Collectors.joining(File.pathSeparator)))); + } + + /** + * Calls java $arguments --list-modules to list all modules and parse the output. + * The output consists of a map with module name as key and {@link Path} to jar file if the + * module is not installed as part of the JDK. If the module is installed as part of the + * jdk/boot-layer then a null path will be returned. + *

+ * This is a much more robust solution then trying to parse the JDK file structure manually. + */ + private static Map callListModules(String javaExecutable, List arguments) { Process listModulesProcess = null; - Set result = new HashSet<>(); + Map result = new LinkedHashMap<>(); try { var pb = new ProcessBuilder(javaExecutable); pb.command().addAll(arguments); @@ -1543,9 +1607,17 @@ private Set getBuilderObservableModules(String javaExecutable, List 1) { + String pathURI = splitString[1]; // url: file://path/to/file + String path = URI.create(pathURI).getPath(); + if (path != null) { + externalPath = Path.of(path); + } + } + result.put(splitModuleNameAndVersion[0], externalPath); } } int exitStatus = listModulesProcess.waitFor(); From f361f03e9aa0a31f5c9384905f818ca687d8d16e Mon Sep 17 00:00:00 2001 From: Christian Humer Date: Wed, 26 Jul 2023 00:14:26 +0200 Subject: [PATCH 4/5] Remove normalization to fix Windows; Handle an empty module-path properly. --- .../src/com/oracle/svm/driver/NativeImage.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java b/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java index b60eef4c7340..b355bac24a2d 100644 --- a/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java +++ b/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java @@ -1490,7 +1490,7 @@ protected int buildImage(List javaArgs, LinkedHashSet cp, LinkedHa */ Set retainedModulePaths = modules.entrySet().stream() .filter((entry) -> entry.getValue() != null && !mp.contains(entry.getValue())) - .map((entry) -> entry.getValue().toAbsolutePath().normalize()) + .map((entry) -> entry.getValue().toAbsolutePath()) .collect(Collectors.toSet()); /* @@ -1503,7 +1503,7 @@ protected int buildImage(List javaArgs, LinkedHashSet cp, LinkedHa * to protect against --list-modules returning too many modules. */ List finalModulePath = substitutedModulePath.stream() - .map((path) -> path.toAbsolutePath().normalize()) + .map((path) -> path.toAbsolutePath()) .filter((path) -> retainedModulePaths.contains(path)) .toList(); @@ -1581,6 +1581,9 @@ protected int buildImage(List javaArgs, LinkedHashSet cp, LinkedHa * @see #callListModules(String, List) */ private static Map listModulesFromPath(String javaExecutable, Collection modulePath) { + if (modulePath.isEmpty()) { + return Map.of(); + } return callListModules(javaExecutable, List.of("-p", modulePath.stream().map(Path::toString).collect(Collectors.joining(File.pathSeparator)))); } @@ -1612,10 +1615,7 @@ private static Map callListModules(String javaExecutable, List 1) { String pathURI = splitString[1]; // url: file://path/to/file - String path = URI.create(pathURI).getPath(); - if (path != null) { - externalPath = Path.of(path); - } + externalPath = Path.of(URI.create(pathURI)); } result.put(splitModuleNameAndVersion[0], externalPath); } From e4e4dc5ac46da6bd7f9f4f9b21d0ee2f539a6fe4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20W=C3=B6gerer?= Date: Wed, 26 Jul 2023 14:21:40 +0200 Subject: [PATCH 5/5] Provide original addModules list to builder --- .../com/oracle/svm/driver/NativeImage.java | 44 +++++++++---------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java b/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java index b355bac24a2d..a45aee72ce37 100644 --- a/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java +++ b/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java @@ -50,6 +50,7 @@ import java.util.List; import java.util.ListIterator; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.Properties; import java.util.Set; @@ -58,6 +59,7 @@ import java.util.function.BiFunction; import java.util.function.Consumer; import java.util.function.Function; +import java.util.function.Predicate; import java.util.jar.Attributes; import java.util.jar.JarFile; import java.util.jar.Manifest; @@ -1442,31 +1444,25 @@ protected int buildImage(List javaArgs, LinkedHashSet cp, LinkedHa Function substituteClassPath = useBundle() ? bundleSupport::substituteClassPath : Function.identity(); List finalImageClassPath = imagecp.stream().map(substituteClassPath).collect(Collectors.toList()); Function substituteModulePath = useBundle() ? bundleSupport::substituteModulePath : Function.identity(); - List substitutedModulePath = imagemp.stream().map(substituteModulePath).toList(); + List substitutedImageModulePath = imagemp.stream().map(substituteModulePath).toList(); Map modules = listModulesFromPath(javaExecutable, Stream.concat(mp.stream(), imagemp.stream()).distinct().toList()); if (!addModules.isEmpty()) { - List addModulesForBuilderVM = new ArrayList<>(); - List addModulesForImage = new ArrayList<>(); + arguments.add("-D" + ModuleSupport.PROPERTY_IMAGE_EXPLICITLY_ADDED_MODULES + "=" + + String.join(",", addModules)); + + List addModulesForBuilderVM = new ArrayList<>(); for (String module : addModules) { Path jarPath = modules.get(module); if (jarPath == null) { // boot module addModulesForBuilderVM.add(module); - } else { - // non boot module - addModulesForImage.add(module); } } if (!addModulesForBuilderVM.isEmpty()) { - arguments.add(DefaultOptionHandler.addModulesOption + "=" + String.join(",", - addModulesForBuilderVM)); - } - if (!addModulesForImage.isEmpty()) { - arguments.add("-D" + ModuleSupport.PROPERTY_IMAGE_EXPLICITLY_ADDED_MODULES + "=" + - String.join(",", addModulesForImage)); + arguments.add(DefaultOptionHandler.addModulesOption + "=" + String.join(",", addModulesForBuilderVM)); } } @@ -1486,28 +1482,27 @@ protected int buildImage(List javaArgs, LinkedHashSet cp, LinkedHa * modules from the module-path that are either already installed in the JDK as boot module, * or were explicitly added to the builder module-path. * - * First compute all jar paths that may remain on the module-path. + * First compute all module-jar paths that are not on the builder module-path. */ - Set retainedModulePaths = modules.entrySet().stream() - .filter((entry) -> entry.getValue() != null && !mp.contains(entry.getValue())) - .map((entry) -> entry.getValue().toAbsolutePath()) + Set nonBuilderModulePaths = modules.values().stream() + .filter(Objects::nonNull) + .filter(Predicate.not(mp::contains)) .collect(Collectors.toSet()); /* * Now we need to filter the substituted module path list for all the modules that may * remain on the module-path. * - * This should normally not be necessary, as the retainedModulePaths should already be the + * This should normally not be necessary, as the nonBuilderModulePaths should already be the * set of jar files for the image module path. Nevertheless, we use the original definition * of the module path to preserve the order of the original module path and as a precaution * to protect against --list-modules returning too many modules. */ - List finalModulePath = substitutedModulePath.stream() - .map((path) -> path.toAbsolutePath()) - .filter((path) -> retainedModulePaths.contains(path)) + List finalImageModulePath = substitutedImageModulePath.stream() + .filter(nonBuilderModulePaths::contains) .toList(); - List finalImageBuilderArgs = createImageBuilderArgs(finalImageArgs, finalImageClassPath, finalModulePath); + List finalImageBuilderArgs = createImageBuilderArgs(finalImageArgs, finalImageClassPath, finalImageModulePath); /* Construct ProcessBuilder command from final arguments */ List command = new ArrayList<>(); @@ -1584,7 +1579,10 @@ private static Map listModulesFromPath(String javaExecutable, Coll if (modulePath.isEmpty()) { return Map.of(); } - return callListModules(javaExecutable, List.of("-p", modulePath.stream().map(Path::toString).collect(Collectors.joining(File.pathSeparator)))); + String modulePathEntries = modulePath.stream() + .map(Path::toString) + .collect(Collectors.joining(File.pathSeparator)); + return callListModules(javaExecutable, List.of("-p", modulePathEntries)); } /** @@ -1615,7 +1613,7 @@ private static Map callListModules(String javaExecutable, List 1) { String pathURI = splitString[1]; // url: file://path/to/file - externalPath = Path.of(URI.create(pathURI)); + externalPath = Path.of(URI.create(pathURI)).toAbsolutePath(); } result.put(splitModuleNameAndVersion[0], externalPath); }