[GR-33930] [GR-33504] Decouple HostedOptionParser setup from classpat…

…h/modulepath scanning. PullRequest: graal/9748
oldcwj · Sep 28, 2021 · 4ac4799 · 4ac4799
2 parents ed7b9d2 + 71008c5
commit 4ac4799
Show file tree

Hide file tree

Showing 31 changed files with 274 additions and 182 deletions.
diff --git a/compiler/mx.compiler/suite.py b/compiler/mx.compiler/suite.py
@@ -2173,7 +2173,7 @@
           "org.graalvm.compiler.debug                  to jdk.internal.vm.compiler.management,org.graalvm.nativeimage.objectfile",
           "org.graalvm.compiler.hotspot                to jdk.internal.vm.compiler.management",
           "org.graalvm.compiler.nodes.graphbuilderconf to org.graalvm.nativeimage.driver",
-          "org.graalvm.compiler.options                to jdk.internal.vm.compiler.management,org.graalvm.nativeimage.driver,org.graalvm.nativeimage.librarysupport",
+          "org.graalvm.compiler.options                to jdk.internal.vm.compiler.management,org.graalvm.nativeimage.driver,org.graalvm.nativeimage.junitsupport",
           "org.graalvm.compiler.phases.common          to org.graalvm.nativeimage.agent.tracing,org.graalvm.nativeimage.configure",
           "org.graalvm.compiler.phases.common.jmx      to jdk.internal.vm.compiler.management",
           "org.graalvm.compiler.serviceprovider        to jdk.internal.vm.compiler.management,org.graalvm.nativeimage.driver,org.graalvm.nativeimage.agent.jvmtibase,org.graalvm.nativeimage.agent.diagnostics",

diff --git a/...rc/org.graalvm.compiler.options.jdk11/src/org/graalvm/compiler/options/ModuleSupport.java b/...rc/org.graalvm.compiler.options.jdk11/src/org/graalvm/compiler/options/ModuleSupport.java
@@ -28,18 +28,13 @@
 
 public class ModuleSupport {
 
-    public static final boolean USE_NI_JPMS = Boolean.parseBoolean(System.getenv().get("USE_NATIVE_IMAGE_JAVA_PLATFORM_MODULE_SYSTEM"));
-
     static Iterable<OptionDescriptors> getOptionsLoader() {
         /*
          * The Graal module (i.e., jdk.internal.vm.compiler) is loaded by the platform class loader
          * as of JDK 9. Modules that depend on and extend Graal are loaded by the app class loader.
          * As such, we need to start the provider search at the app class loader instead of the
          * platform class loader.
          */
-        if (USE_NI_JPMS) {
-            return ServiceLoader.load(ModuleSupport.class.getModule().getLayer(), OptionDescriptors.class);
-        }
         return ServiceLoader.load(OptionDescriptors.class, ClassLoader.getSystemClassLoader());
     }
 }
diff --git a/...iler/src/org.graalvm.compiler.options/src/org/graalvm/compiler/options/ModuleSupport.java b/...iler/src/org.graalvm.compiler.options/src/org/graalvm/compiler/options/ModuleSupport.java
@@ -28,12 +28,6 @@
 
 public class ModuleSupport {
 
-    public static final boolean USE_NI_JPMS;
-
-    static {
-        USE_NI_JPMS = false;
-    }
-
     static Iterable<OptionDescriptors> getOptionsLoader() {
         // On JDK 8, Graal and its extensions are loaded by the same class loader.
         return ServiceLoader.load(OptionDescriptors.class, OptionDescriptors.class.getClassLoader());

diff --git a/substratevm/CHANGELOG.md b/substratevm/CHANGELOG.md
@@ -2,4 +2,8 @@
 
 This changelog summarizes major changes to Native Image, developed under the codename **Substrate VM**.
 
+## Version 22.0.0
+* (GR-33930) Decouple HostedOptionParser setup from classpath/modulepath scanning (use ServiceLoader for collecting options).
+* (GR-33504) Implement --add-reads for native-image and fix --add-opens error handling.
+
 ## Version 21.3.0 
diff --git a/substratevm/mx.substratevm/macro-junit.properties b/substratevm/mx.substratevm/macro-junit.properties
@@ -2,6 +2,7 @@
 
 ImageName = svmjunit
 
+ImageModulePath = ${.}/junit-support.jar
 ImageClasspath = ${.}/junit-tool.jar:${.}/junit.jar:${.}/hamcrest.jar
 
 Args = -H:Features=com.oracle.svm.junit.JUnitFeature \

diff --git a/substratevm/mx.substratevm/macro-junitcp.properties b/substratevm/mx.substratevm/macro-junitcp.properties
@@ -0,0 +1,10 @@
+# This file contains support for building a set of junit tests into a native-image
+
+ImageName = svmjunit
+
+ImageClasspath = ${.}/junit-support.jar:${.}/junit-tool.jar:${.}/junit.jar:${.}/hamcrest.jar
+
+Args = -H:Features=com.oracle.svm.junit.JUnitFeature \
+       -H:Class=com.oracle.svm.junit.SVMJUnitRunner \
+       -H:TestFile=${*} \
+       --initialize-at-build-time=org.junit,com.oracle.mxtool.junit.MxJUnitRequest
diff --git a/substratevm/mx.substratevm/mx_substratevm.py b/substratevm/mx.substratevm/mx_substratevm.py
@@ -434,15 +434,15 @@ def native_unittests_task():
         '-H:AdditionalSecurityServiceTypes=com.oracle.svm.test.SecurityServiceTest$JCACompliantNoOpService'
     ]
 
-    native_unittest(['--build-args', _native_unittest_features] + additional_build_args)
+    native_unittest(['--builder-on-modulepath', '--build-args', _native_unittest_features] + additional_build_args)
 
 
 def javac_image_command(javac_path):
     return [join(javac_path, 'javac'), "-proc:none", "-bootclasspath",
             join(mx_compiler.jdk.home, "jre", "lib", "rt.jar")]
 
 
-def _native_junit(native_image, unittest_args, build_args=None, run_args=None, blacklist=None, whitelist=None, preserve_image=False):
+def _native_junit(native_image, unittest_args, build_args=None, run_args=None, blacklist=None, whitelist=None, preserve_image=False, builder_on_modulepath=False):
     build_args = build_args or []
     javaProperties = {}
     for dist in suite.dists:
@@ -469,7 +469,8 @@ def dummy_harness(test_deps, vm_launcher, vm_args):
         with open(unittest_file, 'r') as f:
             mx.log('Building junit image for matching: ' + ' '.join(l.rstrip() for l in f))
         extra_image_args = mx.get_runtime_jvm_args(unittest_deps, jdk=mx_compiler.jdk, exclude_names=['substratevm:LIBRARY_SUPPORT'])
-        unittest_image = native_image(['-ea', '-esa'] + build_args + extra_image_args + ['--macro:junit=' + unittest_file, '-H:Path=' + junit_test_dir])
+        macro_junit = '--macro:junit' + ('' if builder_on_modulepath else 'cp')
+        unittest_image = native_image(['-ea', '-esa'] + build_args + extra_image_args + [macro_junit + '=' + unittest_file, '-H:Path=' + junit_test_dir])
         mx.log('Running: ' + ' '.join(map(pipes.quote, [unittest_image] + run_args)))
         mx.run([unittest_image] + run_args)
     finally:
@@ -492,13 +493,14 @@ def unmask(args):
 
 def _native_unittest(native_image, cmdline_args):
     parser = ArgumentParser(prog='mx native-unittest', description='Run unittests as native image.')
-    all_args = ['--build-args', '--run-args', '--blacklist', '--whitelist', '-p', '--preserve-image']
+    all_args = ['--build-args', '--run-args', '--blacklist', '--whitelist', '-p', '--preserve-image', '--builder-on-modulepath']
     cmdline_args = [_mask(arg, all_args) for arg in cmdline_args]
     parser.add_argument(all_args[0], metavar='ARG', nargs='*', default=[])
     parser.add_argument(all_args[1], metavar='ARG', nargs='*', default=[])
     parser.add_argument('--blacklist', help='run all testcases not specified in <file>', metavar='<file>')
     parser.add_argument('--whitelist', help='run testcases specified in <file> only', metavar='<file>')
     parser.add_argument('-p', '--preserve-image', help='do not delete the generated native image', action='store_true')
+    parser.add_argument('--builder-on-modulepath', help='perform image build with builder on module-path', action='store_true')
     parser.add_argument('unittest_args', metavar='TEST_ARG', nargs='*')
     pargs = parser.parse_args(cmdline_args)
 
@@ -519,7 +521,11 @@ def _native_unittest(native_image, cmdline_args):
             mx.log('warning: could not read blacklist: ' + blacklist)
 
     unittest_args = unmask(pargs.unittest_args) if unmask(pargs.unittest_args) else ['com.oracle.svm.test', 'com.oracle.svm.configure.test']
-    _native_junit(native_image, unittest_args, unmask(pargs.build_args), unmask(pargs.run_args), blacklist, whitelist, pargs.preserve_image)
+    builder_on_modulepath = pargs.builder_on_modulepath
+    if builder_on_modulepath and svm_java8():
+        mx.log('On Java 8, unittests cannot be built with imagebuilder on module-path. Reverting to imagebuilder on classpath.')
+        builder_on_modulepath = False
+    _native_junit(native_image, unittest_args, unmask(pargs.build_args), unmask(pargs.run_args), blacklist, whitelist, pargs.preserve_image, builder_on_modulepath)
 
 
 def js_image_test(binary, bench_location, name, warmup_iterations, iterations, timeout=None, bin_args=None):
@@ -928,11 +934,24 @@ def _native_image_launcher_extra_jvm_args():
     license_files=[],
     third_party_license_files=[],
     dependencies=['SubstrateVM'],
-    jar_distributions=['mx:JUNIT_TOOL', 'mx:JUNIT', 'mx:HAMCREST'],
+    jar_distributions=['substratevm:JUNIT_SUPPORT', 'mx:JUNIT_TOOL', 'mx:JUNIT', 'mx:HAMCREST'],
     support_distributions=['substratevm:NATIVE_IMAGE_JUNIT_SUPPORT'],
     jlink=False,
 ))
 
+mx_sdk_vm.register_graalvm_component(mx_sdk_vm.GraalVMSvmMacro(
+    suite=suite,
+    name='Native Image JUnit with image-builder on classpath',
+    short_name='njucp',
+    dir_name='junitcp',
+    license_files=[],
+    third_party_license_files=[],
+    dependencies=['SubstrateVM'],
+    jar_distributions=['substratevm:JUNIT_SUPPORT', 'mx:JUNIT_TOOL', 'mx:JUNIT', 'mx:HAMCREST'],
+    support_distributions=['substratevm:NATIVE_IMAGE_JUNITCP_SUPPORT'],
+    jlink=False,
+))
+
 jar_distributions = [
     'substratevm:GRAAL_HOTSPOT_LIBRARY',
     'compiler:GRAAL_TRUFFLE_COMPILER_LIBGRAAL',

diff --git a/substratevm/mx.substratevm/suite.py b/substratevm/mx.substratevm/suite.py
@@ -1,6 +1,6 @@
 # pylint: disable=line-too-long
 suite = {
-    "mxversion": "5.308.1",
+    "mxversion": "5.310.0",
     "name": "substratevm",
     "version" : "22.0.0",
     "release" : False,
@@ -645,7 +645,10 @@
 
         "com.oracle.svm.junit": {
             "subDir": "src",
-            "sourceDirs": ["src"],
+            "sourceDirs": [
+                "src",
+                "resources",
+            ],
             "dependencies": [
                 "com.oracle.svm.reflect",
                 "mx:JUNIT_TOOL",
@@ -1097,16 +1100,19 @@
                     "com.oracle.svm.core.snippets", # Uses of com.oracle.svm.core.snippets.KnownIntrinsics
                     "com.oracle.svm.core", # Uses of com.oracle.svm.core.TypeResult
                     "com.oracle.svm.core.util", # Uses of com.oracle.svm.core.util.VMError
+                    "com.oracle.svm.jfr", # Uses of com.oracle.svm.jfr.JfrEnabled
                     "com.oracle.svm.hosted                        to java.base",
                     "com.oracle.svm.hosted.agent                  to java.instrument",
                     "com.oracle.svm.truffle.api                   to org.graalvm.truffle",
-                    "* to jdk.internal.vm.compiler,org.graalvm.nativeimage.driver,org.graalvm.nativeimage.configure,org.graalvm.nativeimage.librarysupport,org.graalvm.nativeimage.llvm,org.graalvm.nativeimage.agent.jvmtibase,org.graalvm.nativeimage.agent.tracing,org.graalvm.nativeimage.agent.diagnostics,com.oracle.svm.svm_enterprise",
+                    "* to jdk.internal.vm.compiler,org.graalvm.nativeimage.driver,org.graalvm.nativeimage.configure,org.graalvm.nativeimage.librarysupport,org.graalvm.nativeimage.junitsupport,org.graalvm.nativeimage.llvm,org.graalvm.nativeimage.agent.jvmtibase,org.graalvm.nativeimage.agent.tracing,org.graalvm.nativeimage.agent.diagnostics,com.oracle.svm.svm_enterprise",
                 ],
                 "opens" : [
+                    "com.oracle.svm.core                          to jdk.internal.vm.compiler",
                     "com.oracle.svm.core.nodes                    to jdk.internal.vm.compiler",
                     "com.oracle.svm.core.graal.nodes              to jdk.internal.vm.compiler",
                     "com.oracle.svm.core.graal.snippets           to jdk.internal.vm.compiler",
                     "com.oracle.svm.hosted.fieldfolding           to jdk.internal.vm.compiler",
+                    "com.oracle.svm.reflect.hosted                to jdk.internal.vm.compiler",
                 ],
                 "requires": [
                     "java.management",
@@ -1121,6 +1127,7 @@
                 ],
                 "uses" : [
                     "org.graalvm.nativeimage.Platform",
+                    "org.graalvm.compiler.options.OptionDescriptors",
                     "com.oracle.truffle.api.TruffleLanguage.Provider",
                     "com.oracle.truffle.api.instrumentation.TruffleInstrument.Provider",
                     "com.oracle.svm.hosted.agent.NativeImageBytecodeInstrumentationAgentExtension",
@@ -1188,39 +1195,50 @@
                     "com.oracle.svm.jvmtiagentbase",
                     "com.oracle.svm.jvmtiagentbase.jvmti",
                 ],
-                "requires" : [
-                    "static com.oracle.mxtool.junit",
-                    "static junit",
-                    "static hamcrest",
-                ],
             },
         },
 
         "LIBRARY_SUPPORT": {
             "subDir": "src",
             "description" : "SubstrateVM basic library-support components",
             "dependencies": [
-                "com.oracle.svm.junit",
                 "com.oracle.svm.polyglot",
                 "com.oracle.svm.thirdparty",
             ],
             "distDependencies": [
                 "sdk:GRAAL_SDK",
                 "SVM",
-                "OBJECTFILE",
+            ],
+            "moduleInfo" : {
+                "name" : "org.graalvm.nativeimage.librarysupport",
+                "exports" : [
+                    "* to org.graalvm.nativeimage.builder",
+                ],
+            },
+        },
+
+        "JUNIT_SUPPORT": {
+            "subDir": "src",
+            "description" : "SubstrateVM suppoprt for building JUnit test into image",
+            "dependencies": [
+                "com.oracle.svm.junit",
+            ],
+            "distDependencies": [
+                "sdk:GRAAL_SDK",
+                "SVM",
                 "compiler:GRAAL",
                 "mx:JUNIT_TOOL",
             ],
             "moduleInfo" : {
-                "name" : "org.graalvm.nativeimage.librarysupport",
+                "name" : "org.graalvm.nativeimage.junitsupport",
+                "exports" : [
+                    "* to org.graalvm.nativeimage.builder",
+                ],
                 "requires" : [
                     "static com.oracle.mxtool.junit",
                     "static junit",
                     "static hamcrest",
                 ],
-                "exports" : [
-                    "* to org.graalvm.nativeimage.builder",
-                ],
             },
         },
 
@@ -1318,9 +1336,6 @@
                 "org.graalvm.nativeimage.builder",
                 "java.management",
                 "jdk.management",
-                "static com.oracle.mxtool.junit",
-                "static junit",
-                "static hamcrest",
               ],
             },
         },
@@ -1343,11 +1358,6 @@
                 "exports" : [
                     "com.oracle.svm.agent",
                 ],
-                "requires" : [
-                    "static com.oracle.mxtool.junit",
-                    "static junit",
-                    "static hamcrest",
-                ],
                 "requiresConcealed" : {
                     "jdk.internal.vm.ci" : [
                         "jdk.vm.ci.meta",
@@ -1372,11 +1382,6 @@
                 "exports" : [
                     "com.oracle.svm.diagnosticsagent",
                 ],
-                "requires" : [
-                    "static com.oracle.mxtool.junit",
-                    "static junit",
-                    "static hamcrest",
-                ],
             },
         },
 
@@ -1397,11 +1402,6 @@
                     "* to org.graalvm.nativeimage.agent.tracing",
                     "com.oracle.svm.configure",
                 ],
-                "requires" : [
-                    "static com.oracle.mxtool.junit",
-                    "static junit",
-                    "static hamcrest",
-                ],
             },
         },
 
@@ -1572,6 +1572,14 @@
             },
         },
 
+        "NATIVE_IMAGE_JUNITCP_SUPPORT" : {
+            "native" : True,
+            "description" : "Native-image based junit testing support but with running image-builder on classpath",
+            "layout" : {
+                "native-image.properties" : "file:mx.substratevm/macro-junitcp.properties",
+            },
+        },
+
         "SVM_LLVM" : {
             "subDir" : "src",
             "description" : "LLVM backend for Native Image",

diff --git a/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/APIOptionHandler.java b/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/APIOptionHandler.java
@@ -106,20 +106,15 @@ boolean isDeprecated() {
             groupInfos = support.groupInfos;
             apiOptions = support.options;
         } else {
-            List<Class<? extends OptionDescriptors>> optionDescriptorsList = new ArrayList<>();
-            ServiceLoader<OptionDescriptors> serviceLoader = ServiceLoader.load(OptionDescriptors.class, nativeImage.getClass().getClassLoader());
-            for (OptionDescriptors optionDescriptors : serviceLoader) {
-                optionDescriptorsList.add(optionDescriptors.getClass());
-            }
             groupInfos = new HashMap<>();
-            apiOptions = extractOptions(optionDescriptorsList, groupInfos);
+            apiOptions = extractOptions(ServiceLoader.load(OptionDescriptors.class, nativeImage.getClass().getClassLoader()), groupInfos);
         }
     }
 
-    static SortedMap<String, OptionInfo> extractOptions(List<Class<? extends OptionDescriptors>> optionsClasses, Map<String, GroupInfo> groupInfos) {
+    static SortedMap<String, OptionInfo> extractOptions(ServiceLoader<OptionDescriptors> optionDescriptors, Map<String, GroupInfo> groupInfos) {
         EconomicMap<String, OptionDescriptor> hostedOptions = EconomicMap.create();
         EconomicMap<String, OptionDescriptor> runtimeOptions = EconomicMap.create();
-        HostedOptionParser.collectOptions(optionsClasses, hostedOptions, runtimeOptions);
+        HostedOptionParser.collectOptions(optionDescriptors, hostedOptions, runtimeOptions);
         SortedMap<String, OptionInfo> apiOptions = new TreeMap<>();
         Map<Class<? extends APIOptionGroup>, APIOptionGroup> groupInstances = new HashMap<>();
         hostedOptions.getValues().forEach(o -> extractOption(NativeImage.oH, o, apiOptions, groupInfos, groupInstances));
@@ -450,9 +445,9 @@ final class APIOptionFeature implements Feature {
     @Override
     public void duringSetup(DuringSetupAccess access) {
         FeatureImpl.DuringSetupAccessImpl accessImpl = (FeatureImpl.DuringSetupAccessImpl) access;
-        List<Class<? extends OptionDescriptors>> optionClasses = accessImpl.getImageClassLoader().findSubclasses(OptionDescriptors.class, true);
         Map<String, GroupInfo> groupInfos = new HashMap<>();
-        SortedMap<String, APIOptionHandler.OptionInfo> options = APIOptionHandler.extractOptions(optionClasses, groupInfos);
+        ServiceLoader<OptionDescriptors> optionDescriptors = ServiceLoader.load(OptionDescriptors.class, accessImpl.getImageClassLoader().getClassLoader());
+        SortedMap<String, APIOptionHandler.OptionInfo> options = APIOptionHandler.extractOptions(optionDescriptors, groupInfos);
         ImageSingletons.add(APIOptionSupport.class, new APIOptionSupport(groupInfos, options));
     }
 }
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
@@ -91,8 +91,6 @@
 
 public class NativeImage {
 
-    private static final String ENV_VAR_USE_MODULE_SYSTEM = "USE_NATIVE_IMAGE_JAVA_PLATFORM_MODULE_SYSTEM";
-
     private static final String DEFAULT_GENERATOR_CLASS_NAME = NativeImageGeneratorRunner.class.getName();
     private static final String DEFAULT_GENERATOR_MODULE_NAME = ModuleSupport.getModuleName(NativeImageGeneratorRunner.class);
 
@@ -280,7 +278,7 @@ protected BuildConfiguration(List<String> args) {
 
         @SuppressWarnings("deprecation")
         BuildConfiguration(Path rootDir, Path workDir, List<String> args) {
-            modulePathBuild = Boolean.parseBoolean(System.getenv().get(ENV_VAR_USE_MODULE_SYSTEM));
+            modulePathBuild = Boolean.parseBoolean(System.getenv().get(ModuleSupport.ENV_VAR_USE_MODULE_SYSTEM));
             this.args = args;
             this.workDir = workDir != null ? workDir : Paths.get(".").toAbsolutePath().normalize();
             if (rootDir != null) {
@@ -1401,7 +1399,7 @@ protected int buildImage(List<String> javaArgs, LinkedHashSet<Path> bcp, LinkedH
             ProcessBuilder pb = new ProcessBuilder();
             pb.command(command);
             if (config.modulePathBuild) {
-                pb.environment().put(ENV_VAR_USE_MODULE_SYSTEM, Boolean.toString(true));
+                pb.environment().put(ModuleSupport.ENV_VAR_USE_MODULE_SYSTEM, Boolean.toString(true));
             }
             p = pb.inheritIO().start();
             exitStatus = p.waitFor();
@@ -1875,7 +1873,7 @@ static String resolvePropertyValue(String val, String optionArg, Path componentD
              * substitutions of kind ${<argName>} -> <argValue> on resultVal.
              */
             for (String argNameValue : optionArg.split(",")) {
-                String[] splitted = argNameValue.split(":");
+                String[] splitted = argNameValue.split(":", 2);
                 if (splitted.length == 2) {
                     String argName = splitted[0];
                     String argValue = splitted[1];