8162401: Support multiple --add-exports and --add-reads with the same module/package
authormchung
Mon, 10 Oct 2016 15:46:14 -0700
changeset 41414 7fd4548e9733
parent 41413 9493d4c6c5c0
child 41415 9c298252e385
8162401: Support multiple --add-exports and --add-reads with the same module/package Reviewed-by: alanb, redestad, sundar
jdk/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java
jdk/test/tools/launcher/modules/addexports/AddExportsTest.java
jdk/test/tools/launcher/modules/addreads/AddReadsTest.java
--- a/jdk/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java	Mon Oct 10 14:26:49 2016 -0700
+++ b/jdk/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java	Mon Oct 10 15:46:14 2016 -0700
@@ -430,22 +430,23 @@
             // the key is $MODULE
             String mn = e.getKey();
             Optional<Module> om = bootLayer.findModule(mn);
-            if (!om.isPresent())
-                fail("Unknown module: " + mn);
+            if (!om.isPresent()) {
+                warn("Unknown module: " + mn);
+                continue;
+            }
             Module m = om.get();
 
             // the value is the set of other modules (by name)
             for (String name : e.getValue()) {
-
-                Module other;
                 if (ALL_UNNAMED.equals(name)) {
                     Modules.addReadsAllUnnamed(m);
                 } else {
                     om = bootLayer.findModule(name);
-                    if (!om.isPresent())
-                        fail("Unknown module: " + name);
-                    other = om.get();
-                    Modules.addReads(m, other);
+                    if (om.isPresent()) {
+                        Modules.addReads(m, om.get());
+                    } else {
+                        warn("Unknown module: " + name);
+                    }
                 }
 
             }
@@ -472,14 +473,24 @@
 
             String mn = s[0];
             String pn = s[1];
+            if (mn.isEmpty() || pn.isEmpty())
+                fail("Module and package name must be specified:" + key);
 
             // The exporting module is in the boot layer
             Module m;
             Optional<Module> om = bootLayer.findModule(mn);
-            if (!om.isPresent())
-                fail("Unknown module: " + mn);
+            if (!om.isPresent()) {
+                warn("Unknown module: " + mn);
+                continue;
+            }
+
             m = om.get();
 
+            if (!m.getDescriptor().packages().contains(pn)) {
+                warn("package " + pn + " not in " + mn);
+                continue;
+            }
+
             // the value is the set of modules to export to (by name)
             for (String name : e.getValue()) {
                 boolean allUnnamed = false;
@@ -491,10 +502,10 @@
                     if (om.isPresent()) {
                         other = om.get();
                     } else {
-                        fail("Unknown module: " + name);
+                        warn("Unknown module: " + name);
+                        continue;
                     }
                 }
-
                 if (allUnnamed) {
                     Modules.addExportsToAllUnnamed(m, pn);
                 } else {
@@ -534,13 +545,7 @@
             if (rhs.isEmpty())
                 fail("Unable to parse: " + value);
 
-
-            // value is <module>(,<module>)*
-            if (map.containsKey(key))
-                 fail(key + " specified more than once");
-
-            Set<String> values = new HashSet<>();
-            map.put(key, values);
+            Set<String> values = map.computeIfAbsent(key, k -> new HashSet<>());
             for (String s : rhs.split(",")) {
                 if (s.length() > 0) values.add(s);
             }
@@ -566,6 +571,10 @@
         throw new RuntimeException(m);
     }
 
+    static void warn(String m) {
+        System.err.println("WARNING: " + m);
+    }
+
     static class PerfCounters {
 
         static PerfCounter systemModulesTime
--- a/jdk/test/tools/launcher/modules/addexports/AddExportsTest.java	Mon Oct 10 14:26:49 2016 -0700
+++ b/jdk/test/tools/launcher/modules/addexports/AddExportsTest.java	Mon Oct 10 15:46:14 2016 -0700
@@ -202,7 +202,7 @@
 
 
     /**
-     * --add-exports can only be specified once
+     * --add-exports allows duplicates
      */
     public void testWithDuplicateOption() throws Exception {
 
@@ -212,10 +212,40 @@
                                "-version")
                 .outputTo(System.out)
                 .errorTo(System.out)
-                .shouldContain("specified more than once")
                 .getExitValue();
 
-        assertTrue(exitValue != 0);
+        assertTrue(exitValue == 0);
+    }
+
+
+    /**
+     * Exercise --add-exports with unknown values.  Warning is emitted.
+     */
+    @Test(dataProvider = "unknownvalues")
+    public void testWithUnknownValue(String value, String ignore) throws Exception {
+
+        //  --add-exports $VALUE -version
+        int exitValue =
+            executeTestJava("--add-exports", value,
+                            "-version")
+                .stderrShouldMatch("WARNING: .*.monkey.*")
+                .outputTo(System.out)
+                .errorTo(System.out)
+                .getExitValue();
+
+        assertTrue(exitValue == 0);
+    }
+
+    @DataProvider(name = "unknownvalues")
+    public Object[][] unknownValues() {
+        return new Object[][]{
+
+            { "java.base/jdk.internal.misc=sun.monkey", null }, // unknown target
+            { "java.monkey/sun.monkey=ALL-UNNAMED",     null }, // unknown module
+            { "java.base/sun.monkey=ALL-UNNAMED",       null }, // unknown package
+            { "java.monkey/sun.monkey=ALL-UNNAMED",     null }, // unknown module/package
+
+        };
     }
 
 
@@ -241,10 +271,6 @@
         return new Object[][]{
 
             { "java.base/jdk.internal.misc",            null }, // missing target
-            { "java.base/jdk.internal.misc=sun.monkey", null }, // unknown target
-            { "java.monkey/sun.monkey=ALL-UNNAMED",     null }, // unknown module
-            { "java.base/sun.monkey=ALL-UNNAMED",       null }, // unknown package
-            { "java.monkey/sun.monkey=ALL-UNNAMED",     null }, // unknown module/package
             { "java.base=ALL-UNNAMED",                  null }, // missing package
             { "java.base/=ALL-UNNAMED",                 null }  // missing package
 
--- a/jdk/test/tools/launcher/modules/addreads/AddReadsTest.java	Mon Oct 10 14:26:49 2016 -0700
+++ b/jdk/test/tools/launcher/modules/addreads/AddReadsTest.java	Mon Oct 10 15:46:14 2016 -0700
@@ -182,28 +182,43 @@
                   "--add-reads", "m1=java.xml",
                   "--add-reads", "m1=junit",
                   "-m", MAIN)
-                  .shouldContain("specified more than once")
                  .getExitValue();
 
-        assertTrue(exitValue != 0);
+        assertTrue(exitValue == 0);
     }
 
 
     /**
-     * Exercise --add-reads with bad values
+     * Exercise --add-reads with missing source
+     */
+    public void testWithMissingSource() throws Exception {
+
+        //  --add-exports $VALUE -version
+        assertTrue(run("--add-reads", "java.base", "-version").getExitValue() != 0);
+    }
+
+
+    /**
+     * Exercise --add-reads with unknown source/target module.
+     * Warning is emitted.
      */
     @Test(dataProvider = "badvalues")
     public void testWithBadValue(String value, String ignore) throws Exception {
 
         //  --add-exports $VALUE -version
-        assertTrue(run("--add-reads", value, "-version").getExitValue() != 0);
+        int exitValue = run("--add-reads", value, "-version")
+                            .stderrShouldMatch("WARNING: Unknown module: .*.monkey")
+                            .outputTo(System.out)
+                            .errorTo(System.out)
+                            .getExitValue();
+
+        assertTrue(exitValue == 0);
     }
 
     @DataProvider(name = "badvalues")
     public Object[][] badValues() {
         return new Object[][]{
 
-            { "java.base",                  null }, // missing source
             { "java.monkey=java.base",      null }, // unknown module
             { "java.base=sun.monkey",       null }, // unknown source