From e8fdedc0b5ef8adcdb1141b9f1d11f835a73071c Mon Sep 17 00:00:00 2001 From: ivan-ristovic Date: Thu, 6 Jun 2024 12:26:07 +0200 Subject: [PATCH] Check if modules contain packages before attempting to open/export --- .../core/NativeImageClassLoaderOptions.java | 1 - .../hosted/NativeImageClassLoaderSupport.java | 73 ++++++++++--------- .../src/com/oracle/svm/util/LogUtils.java | 21 +++++- 3 files changed, 58 insertions(+), 37 deletions(-) diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/NativeImageClassLoaderOptions.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/NativeImageClassLoaderOptions.java index 1ec4debe5dc3..cae1f4c4b0c7 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/NativeImageClassLoaderOptions.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/NativeImageClassLoaderOptions.java @@ -32,7 +32,6 @@ public class NativeImageClassLoaderOptions { public static final String AddExportsAndOpensFormat = "/=(,)*"; - public static final String AddReadsFormat = "=(,)*"; @APIOption(name = "add-exports", extra = true, launcherOption = true, valueSeparator = {APIOption.WHITESPACE_SEPARATOR, '='})// diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageClassLoaderSupport.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageClassLoaderSupport.java index 439b1a5eca3f..05012b614196 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageClassLoaderSupport.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageClassLoaderSupport.java @@ -32,6 +32,7 @@ import java.io.File; import java.io.IOException; import java.lang.module.Configuration; +import java.lang.module.FindException; import java.lang.module.ModuleDescriptor; import java.lang.module.ModuleFinder; import java.lang.module.ModuleReader; @@ -389,21 +390,29 @@ void processClassLoaderOptions() { } processOption(NativeImageClassLoaderOptions.AddExports).forEach(val -> { - if (val.targetModules.isEmpty()) { - Modules.addExportsToAllUnnamed(val.module, val.packageName); - } else { - for (Module targetModule : val.targetModules) { - Modules.addExports(val.module, val.packageName, targetModule); + if (val.module.getPackages().contains(val.packageName)) { + if (val.targetModules.isEmpty()) { + Modules.addExportsToAllUnnamed(val.module, val.packageName); + } else { + for (Module targetModule : val.targetModules) { + Modules.addExports(val.module, val.packageName, targetModule); + } } + } else { + warn("package " + val.packageName + " not in " + val.module.getName()); } }); processOption(NativeImageClassLoaderOptions.AddOpens).forEach(val -> { - if (val.targetModules.isEmpty()) { - Modules.addOpensToAllUnnamed(val.module, val.packageName); - } else { - for (Module targetModule : val.targetModules) { - Modules.addOpens(val.module, val.packageName, targetModule); + if (val.module.getPackages().contains(val.packageName)) { + if (val.targetModules.isEmpty()) { + Modules.addOpensToAllUnnamed(val.module, val.packageName); + } else { + for (Module targetModule : val.targetModules) { + Modules.addOpens(val.module, val.packageName, targetModule); + } } + } else { + warn("package " + val.packageName + " not in " + val.module.getName()); } }); processOption(NativeImageClassLoaderOptions.AddReads).forEach(val -> { @@ -417,6 +426,10 @@ void processClassLoaderOptions() { }); } + private static void warn(String m) { + LogUtils.warning("WARNING", m, true); + } + private static void processListModulesOption(ModuleLayer layer) { Class launcherHelperClass = ReflectionUtil.lookupClass(false, "sun.launcher.LauncherHelper"); Method initOutputMethod = ReflectionUtil.lookupMethod(launcherHelperClass, "initOutput", boolean.class); @@ -499,33 +512,20 @@ private Stream processOption(OptionKey parsedOptions = valuesWithOrigins.flatMap(valWithOrig -> { try { return Stream.of(asAddExportsAndOpensAndReadsFormatValue(specificOption, valWithOrig)); - } catch (UserError.UserException e) { - if (ModuleSupport.modulePathBuild && classpath().isEmpty()) { - throw e; - } else { - /* - * Until we switch to always running the image-builder on module-path we have to - * be tolerant if invalid --add-exports -add-opens or --add-reads options are - * used. GR-30433 - */ - LogUtils.warning(e.getMessage()); - return Stream.empty(); - } + } catch (FindException e) { + /* + * Print a specially-formatted warning to be 100% compatible with the output of + * `java` in this case. + */ + LogUtils.warning("WARNING", e.getMessage(), true); + return Stream.empty(); } }); return parsedOptions; } - private static final class AddExportsAndOpensAndReadsFormatValue { - private final Module module; - private final String packageName; - private final List targetModules; - - private AddExportsAndOpensAndReadsFormatValue(Module module, String packageName, List targetModules) { - this.module = module; - this.packageName = packageName; - this.targetModules = targetModules; - } + private record AddExportsAndOpensAndReadsFormatValue(Module module, String packageName, + List targetModules) { } private AddExportsAndOpensAndReadsFormatValue asAddExportsAndOpensAndReadsFormatValue(OptionKey option, Pair valueOrigin) { @@ -581,7 +581,7 @@ private AddExportsAndOpensAndReadsFormatValue asAddExportsAndOpensAndReadsFormat } Module module = findModule(moduleName).orElseThrow(() -> { - return userErrorAddExportsAndOpensAndReads(option, optionOrigin, optionValue, " Specified module '" + moduleName + "' is unknown."); + throw userWarningModuleNotFound(option, moduleName); }); List targetModules; if (targetModuleNamesList.contains("ALL-UNNAMED")) { @@ -589,7 +589,7 @@ private AddExportsAndOpensAndReadsFormatValue asAddExportsAndOpensAndReadsFormat } else { targetModules = targetModuleNamesList.stream().map(mn -> { return findModule(mn).orElseThrow(() -> { - throw userErrorAddExportsAndOpensAndReads(option, optionOrigin, optionValue, " Specified target-module '" + mn + "' is unknown."); + throw userWarningModuleNotFound(option, mn); }); }).collect(Collectors.toList()); } @@ -601,6 +601,11 @@ private static UserError.UserException userErrorAddExportsAndOpensAndReads(Optio return UserError.abort("Invalid option %s provided by %s.%s", SubstrateOptionsParser.commandArgument(option, value), origin, detailMessage); } + private static FindException userWarningModuleNotFound(OptionKey option, String moduleName) { + String optionName = SubstrateOptionsParser.commandArgument(option, ""); + return new FindException("Unknown module: " + moduleName + " specified to " + optionName); + } + Class loadClassFromModule(Module module, String className) { assert isModuleClassLoader(classLoader, module.getClassLoader()) : "Argument `module` is java.lang.Module from unknown ClassLoader"; return Class.forName(module, className); diff --git a/substratevm/src/com.oracle.svm.util/src/com/oracle/svm/util/LogUtils.java b/substratevm/src/com.oracle.svm.util/src/com/oracle/svm/util/LogUtils.java index 920861273d33..a5d90f3d227b 100644 --- a/substratevm/src/com.oracle.svm.util/src/com/oracle/svm/util/LogUtils.java +++ b/substratevm/src/com.oracle.svm.util/src/com/oracle/svm/util/LogUtils.java @@ -27,13 +27,22 @@ import org.graalvm.nativeimage.Platform; import org.graalvm.nativeimage.Platforms; +import java.io.PrintStream; + // Checkstyle: Allow raw info or warning printing - begin public class LogUtils { /** * Print an info message. */ public static void info(String message) { - System.out.println("Info: " + message); + info("Info", message); + } + + /** + * Print an info message with the given prefix. + */ + public static void info(String prefix, String message) { + System.out.println(prefix + ": " + message); } /** @@ -53,7 +62,15 @@ public static void info(String format, Object... args) { * Print a warning. */ public static void warning(String message) { - System.out.println("Warning: " + message); + warning("Warning", message, false); + } + + /** + * Print a warning message with the given prefix, optionally to stderr. + */ + public static void warning(String prefix, String message, boolean stderr) { + PrintStream out = stderr ? System.err : System.out; + out.println(prefix + ": " + message); } /**