Check if modules contain packages before attempting to open/export

Author: ivan-ristovic

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 = "<module>/<package>=<target-module>(,<target-module>)*";
-
     public static final String AddReadsFormat = "<module>=<target-module>(,<target-module>)*";
 
     @APIOption(name = "add-exports", extra = true, launcherOption = true, valueSeparator = {APIOption.WHITESPACE_SEPARATOR, '='})//

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<AddExportsAndOpensAndReadsFormatValue> processOption(OptionKey<Ac
         Stream<AddExportsAndOpensAndReadsFormatValue> 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<Module> targetModules;
-
-        private AddExportsAndOpensAndReadsFormatValue(Module module, String packageName, List<Module> targetModules) {
-            this.module = module;
-            this.packageName = packageName;
-            this.targetModules = targetModules;
-        }
+    private record AddExportsAndOpensAndReadsFormatValue(Module module, String packageName,
+                    List<Module> targetModules) {
     }
 
     private AddExportsAndOpensAndReadsFormatValue asAddExportsAndOpensAndReadsFormatValue(OptionKey<?> option, Pair<String, OptionOrigin> valueOrigin) {
@@ -581,15 +581,15 @@ private AddExportsAndOpensAndReadsFormatValue asAddExportsAndOpensAndReadsFormat
         }
 
         Module module = findModule(moduleName).orElseThrow(() -> {
-            return userErrorAddExportsAndOpensAndReads(option, optionOrigin, optionValue, " Specified module '" + moduleName + "' is unknown.");
+            throw userWarningModuleNotFound(option, moduleName);
         });
         List<Module> targetModules;
         if (targetModuleNamesList.contains("ALL-UNNAMED")) {
             targetModules = Collections.emptyList();
         } 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);

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);
     }
 
     /**