# HG changeset patch # User mchung # Date 1482188474 28800 # Node ID 74bcf37d15d83d171193be8230f4322df1d6e9e1 # Parent f32797dd88065eb62125150cc199c8ff4b3cabb1 8168836: Minor clean up on warning/error messages on --add-exports and --add-reads Reviewed-by: alanb diff -r f32797dd8806 -r 74bcf37d15d8 jdk/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java --- a/jdk/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java Mon Dec 19 14:52:19 2016 -0800 +++ b/jdk/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java Mon Dec 19 15:01:14 2016 -0800 @@ -306,6 +306,13 @@ fail(name + ": cannot be loaded from application module path"); } } + + // check if module specified in --patch-module is present + for (String mn: patcher.patchedModules()) { + if (!cf.findModule(mn).isPresent()) { + warnUnknownModule(PATCH_MODULE, mn); + } + } } // if needed check that there are no split packages in the set of @@ -481,7 +488,7 @@ String mn = e.getKey(); Optional om = bootLayer.findModule(mn); if (!om.isPresent()) { - warn("Unknown module: " + mn); + warnUnknownModule(ADD_READS, mn); continue; } Module m = om.get(); @@ -495,7 +502,7 @@ if (om.isPresent()) { Modules.addReads(m, om.get()); } else { - warn("Unknown module: " + name); + warnUnknownModule(ADD_READS, name); } } } @@ -527,24 +534,25 @@ Map> map, boolean opens) { + String option = opens ? ADD_OPENS : ADD_EXPORTS; for (Map.Entry> e : map.entrySet()) { // the key is $MODULE/$PACKAGE String key = e.getKey(); String[] s = key.split("/"); if (s.length != 2) - fail("Unable to parse as /: " + key); + fail(unableToParse(option, "/", key)); String mn = s[0]; String pn = s[1]; if (mn.isEmpty() || pn.isEmpty()) - fail("Module and package name must be specified: " + key); + fail(unableToParse(option, "/", key)); // The exporting module is in the boot layer Module m; Optional om = bootLayer.findModule(mn); if (!om.isPresent()) { - warn("Unknown module: " + mn); + warnUnknownModule(option, mn); continue; } @@ -566,7 +574,7 @@ if (om.isPresent()) { other = om.get(); } else { - warn("Unknown module: " + name); + warnUnknownModule(option, name); continue; } } @@ -610,24 +618,30 @@ int pos = value.indexOf('='); if (pos == -1) - fail("Unable to parse as =: " + value); + fail(unableToParse(option(prefix), "=", value)); if (pos == 0) - fail("Missing module name in: " + value); + fail(unableToParse(option(prefix), "=", value)); // key is or / String key = value.substring(0, pos); String rhs = value.substring(pos+1); if (rhs.isEmpty()) - fail("Unable to parse as =: " + value); + fail(unableToParse(option(prefix), "=", value)); // value is (,)* or ()* if (!allowDuplicates && map.containsKey(key)) - fail(key + " specified more than once"); + fail(key + " specified more than once in " + option(prefix)); List values = map.computeIfAbsent(key, k -> new ArrayList<>()); + int ntargets = 0; for (String s : rhs.split(regex)) { - if (s.length() > 0) values.add(s); + if (s.length() > 0) { + values.add(s); + ntargets++; + } } + if (ntargets == 0) + fail("Target must be specified: " + option(prefix) + " " + value); index++; value = getAndRemoveProperty(prefix + index); @@ -689,6 +703,42 @@ System.err.println("WARNING: " + m); } + static void warnUnknownModule(String option, String mn) { + warn("Unknown module: " + mn + " specified in " + option); + } + + static String unableToParse(String option, String text, String value) { + return "Unable to parse " + option + " " + text + ": " + value; + } + + private static final String ADD_MODULES = "--add-modules"; + private static final String ADD_EXPORTS = "--add-exports"; + private static final String ADD_OPENS = "--add-opens"; + private static final String ADD_READS = "--add-reads"; + private static final String PATCH_MODULE = "--patch-module"; + + + /* + * Returns the command-line option name corresponds to the specified + * system property prefix. + */ + static String option(String prefix) { + switch (prefix) { + case "jdk.module.addexports.": + return ADD_EXPORTS; + case "jdk.module.addopens.": + return ADD_OPENS; + case "jdk.module.addreads.": + return ADD_READS; + case "jdk.module.patch.": + return PATCH_MODULE; + case "jdk.module.addmods.": + return ADD_MODULES; + default: + throw new IllegalArgumentException(prefix); + } + } + static class PerfCounters { static PerfCounter systemModulesTime diff -r f32797dd8806 -r 74bcf37d15d8 jdk/src/java.base/share/classes/jdk/internal/module/ModulePatcher.java --- a/jdk/src/java.base/share/classes/jdk/internal/module/ModulePatcher.java Mon Dec 19 14:52:19 2016 -0800 +++ b/jdk/src/java.base/share/classes/jdk/internal/module/ModulePatcher.java Mon Dec 19 15:01:14 2016 -0800 @@ -175,6 +175,12 @@ return map.isEmpty(); } + /* + * Returns the names of the patched modules. + */ + Set patchedModules() { + return map.keySet(); + } /** * A ModuleReader that reads resources from a patched module. diff -r f32797dd8806 -r 74bcf37d15d8 jdk/src/java.base/share/native/libjli/java.c --- a/jdk/src/java.base/share/native/libjli/java.c Mon Dec 19 14:52:19 2016 -0800 +++ b/jdk/src/java.base/share/native/libjli/java.c Mon Dec 19 15:01:14 2016 -0800 @@ -573,6 +573,17 @@ JLI_StrCmp(name, "--patch-module") == 0; } +static jboolean +IsLongFormModuleOption(const char* name) { + return JLI_StrCCmp(name, "--module-path=") == 0 || + JLI_StrCCmp(name, "--upgrade-module-path=") == 0 || + JLI_StrCCmp(name, "--add-modules=") == 0 || + JLI_StrCCmp(name, "--limit-modules=") == 0 || + JLI_StrCCmp(name, "--add-exports=") == 0 || + JLI_StrCCmp(name, "--add-reads=") == 0 || + JLI_StrCCmp(name, "--patch-module=") == 0; +} + /* * Test if the given name has a white space option. */ @@ -1236,7 +1247,7 @@ char *option = NULL; char *value = NULL; int kind = GetOpt(&argc, &argv, &option, &value); - jboolean has_arg = value != NULL; + jboolean has_arg = value != NULL && JLI_StrLen(value) > 0; /* * Option to set main entry point @@ -1285,19 +1296,13 @@ /* * Error missing argument */ - } else if (!has_arg && IsWhiteSpaceOption(arg)) { - if (JLI_StrCmp(arg, "--module-path") == 0 || - JLI_StrCmp(arg, "-p") == 0 || - JLI_StrCmp(arg, "--upgrade-module-path") == 0) { - REPORT_ERROR (has_arg, ARG_ERROR4, arg); - } else if (JLI_StrCmp(arg, "--add-modules") == 0 || - JLI_StrCmp(arg, "--limit-modules") == 0 || - JLI_StrCmp(arg, "--add-exports") == 0 || - JLI_StrCmp(arg, "--add-opens") == 0 || - JLI_StrCmp(arg, "--add-reads") == 0 || - JLI_StrCmp(arg, "--patch-module") == 0) { - REPORT_ERROR (has_arg, ARG_ERROR6, arg); - } + } else if (!has_arg && (JLI_StrCmp(arg, "--module-path") == 0 || + JLI_StrCmp(arg, "-p") == 0 || + JLI_StrCmp(arg, "--upgrade-module-path") == 0)) { + REPORT_ERROR (has_arg, ARG_ERROR4, arg); + + } else if (!has_arg && (IsModuleOption(arg) || IsLongFormModuleOption(arg))) { + REPORT_ERROR (has_arg, ARG_ERROR6, arg); /* * The following cases will cause the argument parsing to stop */ diff -r f32797dd8806 -r 74bcf37d15d8 jdk/test/lib/testlibrary/ModuleSourceBuilder.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/jdk/test/lib/testlibrary/ModuleSourceBuilder.java Mon Dec 19 15:01:14 2016 -0800 @@ -0,0 +1,127 @@ +/* + * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +import java.io.BufferedWriter; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Arrays; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import java.util.stream.Stream; + +import static org.testng.Assert.assertTrue; + +/** + * Utility class for creating test modules. + */ +public class ModuleSourceBuilder { + private static String MODULE_INFO_JAVA = "module-info.java"; + private static Pattern MODULE_PATTERN = + Pattern.compile("module\\s+((?:\\w+\\.)*)"); + private static Pattern PACKAGE_PATTERN = + Pattern.compile("package\\s+(((?:\\w+\\.)*)(?:\\w+))"); + private static Pattern CLASS_PATTERN = + Pattern.compile("(?:public\\s+)?(?:class|enum|interface)\\s+(\\w+)"); + + private final Path dir; + public ModuleSourceBuilder(Path dir) { + this.dir = dir; + } + + /** + * Create java source files of the given module + */ + public void writeJavaFiles(String module, String moduleInfoJava, String... contents) + throws IOException + { + Path msrc = dir.resolve(module); + new JavaSource(moduleInfoJava).write(msrc); + for (String c : contents) { + new JavaSource(c).write(msrc); + } + } + + /** + * Compile the module to the given destination. + */ + public void compile(String module, Path dest, String... options) + throws IOException + { + Path msrc = dir.resolve(module); + Stream args = + Stream.concat(Arrays.stream(options), + Stream.of("--module-source-path", + dir.toString())); + assertTrue(CompilerUtils.compile(msrc, dest, args.toArray(String[]::new)), + "Fail to compile " + module); + } + + static class JavaSource { + final String source; + JavaSource(String source) { + this.source = source; + } + + /** + * Writes the source code to a file in a specified directory. + * @param dir the directory + * @throws IOException if there is a problem writing the file + */ + public void write(Path dir) throws IOException { + Path file = dir.resolve(getJavaFileNameFromSource(source)); + Files.createDirectories(file.getParent()); + try (BufferedWriter out = Files.newBufferedWriter(file)) { + out.write(source.replace("\n", System.lineSeparator())); + } + } + + /** + * Extracts the Java file name from the class declaration. + * This method is intended for simple files and uses regular expressions, + * so comments matching the pattern can make the method fail. + */ + static String getJavaFileNameFromSource(String source) { + String packageName = null; + + Matcher matcher = MODULE_PATTERN.matcher(source); + if (matcher.find()) + return MODULE_INFO_JAVA; + + matcher = PACKAGE_PATTERN.matcher(source); + if (matcher.find()) + packageName = matcher.group(1).replace(".", "/"); + + matcher = CLASS_PATTERN.matcher(source); + if (matcher.find()) { + String className = matcher.group(1) + ".java"; + return (packageName == null) ? className : packageName + "/" + className; + } else if (packageName != null) { + return packageName + "/package-info.java"; + } else { + throw new Error("Could not extract the java class " + + "name from the provided source"); + } + } + } +} diff -r f32797dd8806 -r 74bcf37d15d8 jdk/test/tools/launcher/modules/addexports/AddExportsTestWarningError.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/jdk/test/tools/launcher/modules/addexports/AddExportsTestWarningError.java Mon Dec 19 15:01:14 2016 -0800 @@ -0,0 +1,234 @@ +/* + * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/** + * @test + * @bug 8168836 + * @summary Basic argument validation for --add-exports + * @library /lib/testlibrary + * @modules jdk.compiler + * @build AddExportsTestWarningError CompilerUtils ModuleSourceBuilder + * @build jdk.testlibrary.* + * @run testng AddExportsTestWarningError + */ + +import java.io.BufferedOutputStream; +import java.io.ByteArrayOutputStream; +import java.io.PrintStream; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Arrays; +import java.util.stream.Stream; + +import jdk.testlibrary.OutputAnalyzer; +import static jdk.testlibrary.ProcessTools.*; + +import org.testng.annotations.BeforeTest; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; +import static org.testng.Assert.*; + + +@Test +public class AddExportsTestWarningError { + + private static final Path MODS_DIR = Paths.get("mods"); + private static final Path SRC_DIR = Paths.get("src"); + private static final String M1_MAIN = "m1/p1.C1"; + private static final String M3_MAIN = "m3/p3.C3"; + + @BeforeTest + public void setup() throws Exception { + ModuleSourceBuilder builder = new ModuleSourceBuilder(SRC_DIR); + builder.writeJavaFiles("m1", + "module m1 { }", + "package p1; public class C1 { " + + " public static void main(String... args) {}" + + "}"); + + builder.writeJavaFiles("m2", + "module m2 { requires m1; exports p2; }", + "package p2; public class C2 { private p1.C1 c1; }"); + + builder.writeJavaFiles("m3", + "module m3 { requires m2; }", + "package p3; class C3 { " + + " p1.C1 c; " + + " public static void main(String... args) { new p2.C2(); }" + + "}"); + + builder.compile("m1", MODS_DIR); + builder.compile("m2", MODS_DIR, "--add-exports", "m1/p1=m2"); + builder.compile("m3", MODS_DIR, "--add-exports", "m1/p1=m3"); + } + + + @DataProvider(name = "goodcases") + public Object[][] goodCases() { + return new Object[][]{ + + // empty items + { "m1/p1=,m2,m3", null }, + { "m1/p1=m2,,m3", null }, + { "m1/p1=m2,m3,", null }, + + // duplicates + { "m1/p1=m2,m2,m3,,", null }, + + }; + } + + + @Test(dataProvider = "goodcases") + public void test(String value, String ignore) throws Exception { + testNoWarning(value); + } + + + @DataProvider(name = "illFormedAddExports") + public Object[][] illFormedAddExports() { + return new Object[][]{ + { "m1", "Unable to parse --add-exports =: m1"}, + + // missing source part + { "=m2", "Unable to parse --add-exports =: =m2"}, + { "/=m2", "Unable to parse --add-exports /: /" }, + { "m1=m2", "Unable to parse --add-exports /: m1" }, + { "/p1=m2", "Unable to parse --add-exports /: /p1" }, + { "m1p1=m2", "Unable to parse --add-exports /: m1p1" }, + + // empty list, missing target + { "m1/p1=", "Unable to parse --add-exports =: m1/p1=" }, + { "m1/p1=,,", "Target must be specified: --add-exports m1/p1=,," }, + }; + } + + @Test(dataProvider = "illFormedAddExports") + public void testIllFormedAddExports(String value, String msg) throws Exception { + testError(value, msg); + } + + + @DataProvider(name = "unknownNames") + public Object[][] unknownNames() { + return new Object[][]{ + + // source not found + {"DoesNotExist/p=m1", "WARNING: Unknown module: DoesNotExist specified in --add-exports"}, + {"m1/DoesNotExist=m2", "WARNING: package DoesNotExist not in m1"}, + + // target not found + {"m1/p1=DoesNotExist", "WARNING: Unknown module: DoesNotExist specified in --add-exports"}, + + // bad names + {"m*/p1=m2", "WARNING: Unknown module: m* specified in --add-exports"}, + {"m1/p!=m2", "WARNING: package p! not in m1"}, + {"m1/p1=m!", "WARNING: Unknown module: m! specified in --add-exports"}, + + }; + } + + + @Test(dataProvider = "unknownNames") + public void testUnknownNames(String value, String msg) throws Exception { + testWarning(value, msg); + } + + + @DataProvider(name = "missingArguments") + public Object[][] missingArguments() { + return new Object[][]{ + { new String[] { "--add-exports" }, + "Error: --add-exports requires modules to be specified"}, + + { new String[] { "--add-exports=" }, + "Error: --add-exports= requires modules to be specified" }, + + { new String[] { "--add-exports", "" }, + "Error: --add-exports requires modules to be specified"} + + }; + } + + + @Test(dataProvider = "missingArguments") + public void testMissingArguments(String[] options, String msg) throws Exception { + String[] args = Stream.concat(Arrays.stream(options), + Stream.of("-version")) + .toArray(String[]::new); + int exitValue = executeTestJava(args) + .outputTo(System.out) + .errorTo(System.out) + .shouldContain(msg) + .getExitValue(); + + assertTrue(exitValue != 0); + } + + private void testWarning(String value, String msg) throws Exception { + int exitValue = + executeTestJava("--add-exports", value, + "--module-path", MODS_DIR.toString(), + "-m", M1_MAIN) + .outputTo(System.out) + .errorTo(System.out) + .shouldContain(msg) + .getExitValue(); + + assertTrue(exitValue == 0); + } + + private void testError(String value, String msg) throws Exception { + int exitValue = + executeTestJava("--add-exports", value, + "--module-path", MODS_DIR.toString(), + "-m", M1_MAIN) + .outputTo(System.out) + .errorTo(System.out) + .shouldContain(msg) + .getExitValue(); + + assertTrue(exitValue != 0); + } + + private void testNoWarning(String value) throws Exception { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + PrintStream ps = new PrintStream(new BufferedOutputStream(baos)); + OutputAnalyzer outputAnalyzer = + executeTestJava("--add-exports", value, + "--module-path", MODS_DIR.toString(), + "-m", M3_MAIN) + .outputTo(ps) + .errorTo(ps); + + assertTrue(outputAnalyzer.getExitValue() == 0); + + System.out.println(baos.toString()); + String[] output = baos.toString().split("\\R"); + assertFalse(Arrays.stream(output) + .filter(s -> !s.matches("WARNING: Module name .* may soon be illegal")) + .filter(s -> s.startsWith("WARNING:")) + .findAny().isPresent()); + + } +} diff -r f32797dd8806 -r 74bcf37d15d8 jdk/test/tools/launcher/modules/addreads/AddReadsTestWarningError.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/jdk/test/tools/launcher/modules/addreads/AddReadsTestWarningError.java Mon Dec 19 15:01:14 2016 -0800 @@ -0,0 +1,224 @@ +/* + * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/** + * @test + * @bug 8168836 + * @summary Basic argument validation for --add-reads + * @library /lib/testlibrary + * @modules jdk.compiler + * @build AddReadsTestWarningError CompilerUtils ModuleSourceBuilder + * @build jdk.testlibrary.* + * @run testng AddReadsTestWarningError + */ + +import java.io.BufferedOutputStream; +import java.io.ByteArrayOutputStream; +import java.io.PrintStream; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Arrays; +import java.util.stream.Stream; + +import jdk.testlibrary.OutputAnalyzer; +import static jdk.testlibrary.ProcessTools.*; + +import org.testng.annotations.BeforeTest; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; +import static org.testng.Assert.*; + + +@Test +public class AddReadsTestWarningError { + + private static final Path MODS_DIR = Paths.get("mods"); + private static final Path SRC_DIR = Paths.get("src"); + private static final String M1_MAIN = "m1/p1.C1"; + private static final String M4_MAIN = "m4/p4.C4"; + + @BeforeTest + public void setup() throws Exception { + ModuleSourceBuilder builder = new ModuleSourceBuilder(SRC_DIR); + builder.writeJavaFiles("m1", + "module m1 { requires m4; }", + "package p1; public class C1 { " + + " public static void main(String... args) {" + + " p2.C2 c2 = new p2.C2();" + + " p3.C3 c3 = new p3.C3();" + + " }" + + "}" + ); + + builder.writeJavaFiles("m2", + "module m2 { exports p2; }", + "package p2; public class C2 { }" + ); + + builder.writeJavaFiles("m3", + "module m3 { exports p3; }", + "package p3; public class C3 { }" + ); + + builder.writeJavaFiles("m4", + "module m4 { requires m2; requires m3; }", + "package p4; public class C4 { " + + " public static void main(String... args) {}" + + "}" + ); + + builder.compile("m2", MODS_DIR); + builder.compile("m3", MODS_DIR); + builder.compile("m4", MODS_DIR); + builder.compile("m1", MODS_DIR, "--add-reads", "m1=m2,m3"); + } + + + @DataProvider(name = "goodcases") + public Object[][] goodCases() { + return new Object[][]{ + // empty items + { "m1=,m2,m3", null }, + { "m1=m2,,m3", null }, + { "m1=m2,m3,", null }, + + // duplicates + { "m1=m2,m2,m3,,", null }, + + }; + } + + + @Test(dataProvider = "goodcases") + public void test(String value, String ignore) throws Exception { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + PrintStream ps = new PrintStream(new BufferedOutputStream(baos)); + OutputAnalyzer outputAnalyzer = + executeTestJava("--add-reads", value, + "--module-path", MODS_DIR.toString(), + "-m", M1_MAIN) + .outputTo(ps) + .errorTo(ps); + + assertTrue(outputAnalyzer.getExitValue() == 0); + + System.out.println(baos.toString()); + String[] output = baos.toString().split("\\R"); + assertFalse(Arrays.stream(output) + .filter(s -> !s.matches("WARNING: Module name .* may soon be illegal")) + .filter(s -> s.startsWith("WARNING:")) + .findAny().isPresent()); + } + + + @DataProvider(name = "illFormedAddReads") + public Object[][] illFormedAddReads() { + return new Object[][]{ + { "m1", "Unable to parse --add-reads =: m1" }, + + // missing source part + { "=m2", "Unable to parse --add-reads =: =m2" }, + + // empty list, missing target + { "m1=", "Unable to parse --add-reads =: m1=" }, + + // empty list + { "m1=,,", "Target must be specified: --add-reads m1=,," }, + }; + } + + + @Test(dataProvider = "illFormedAddReads") + public void testIllFormedAddReads(String value, String msg) throws Exception { + int exitValue = + executeTestJava("--add-reads", value, + "--module-path", MODS_DIR.toString(), + "-m", M4_MAIN) + .outputTo(System.out) + .errorTo(System.out) + .shouldContain(msg) + .getExitValue(); + + assertTrue(exitValue != 0); + } + + + @DataProvider(name = "unknownNames") + public Object[][] unknownNames() { + return new Object[][]{ + + // source not found + {"DoesNotExist=m2", "WARNING: Unknown module: DoesNotExist specified in --add-reads"}, + + // target not found + {"m2=DoesNotExist", "WARNING: Unknown module: DoesNotExist specified in --add-reads"}, + + // bad names + {"m*=m2", "WARNING: Unknown module: m* specified in --add-reads"}, + {"m2=m!", "WARNING: Unknown module: m! specified in --add-reads"}, + + }; + } + + @Test(dataProvider = "unknownNames") + public void testUnknownNames(String value, String msg) throws Exception { + int exitValue = + executeTestJava("--add-reads", value, + "--module-path", MODS_DIR.toString(), + "-m", M4_MAIN) + .outputTo(System.out) + .errorTo(System.out) + .shouldContain(msg) + .getExitValue(); + + assertTrue(exitValue == 0); + } + + + @DataProvider(name = "missingArguments") + public Object[][] missingArguments() { + return new Object[][]{ + { new String[] {"--add-reads" }, + "Error: --add-reads requires modules to be specified"}, + + { new String[] { "--add-reads=" }, + "Error: --add-reads= requires modules to be specified"}, + + { new String[] { "--add-reads", "" }, + "Error: --add-reads requires modules to be specified"}, + }; + } + + @Test(dataProvider = "missingArguments") + public void testEmptyArgument(String[] options, String msg) throws Exception { + String[] args = Stream.concat(Arrays.stream(options), Stream.of("-version")) + .toArray(String[]::new); + int exitValue = executeTestJava(args) + .outputTo(System.out) + .errorTo(System.out) + .shouldContain(msg) + .getExitValue(); + + assertTrue(exitValue != 0); + } +} diff -r f32797dd8806 -r 74bcf37d15d8 jdk/test/tools/launcher/modules/patch/basic/PatchTestWarningError.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/jdk/test/tools/launcher/modules/patch/basic/PatchTestWarningError.java Mon Dec 19 15:01:14 2016 -0800 @@ -0,0 +1,220 @@ +/* + * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/** + * @test + * @bug 8168836 + * @summary Basic argument validation for --patch-module + * @library /lib/testlibrary + * @modules jdk.compiler + * @build PatchTestWarningError CompilerUtils JarUtils jdk.testlibrary.* + * @run testng PatchTestWarningError + */ + +import java.io.File; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import static jdk.testlibrary.ProcessTools.*; + +import org.testng.annotations.BeforeTest; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; +import static org.testng.Assert.*; + + +/** + * This test + * See PatchTestWarningError for test description. + */ + +@Test +public class PatchTestWarningError { + + // top-level source directory + private static final String TEST_SRC = System.getProperty("test.src"); + + // source/destination tree for the test module + private static final Path SRC_DIR = Paths.get(TEST_SRC, "src"); + private static final Path MODS_DIR = Paths.get("mods"); + + // source/destination tree for patch tree 1 + private static final Path SRC1_DIR = Paths.get(TEST_SRC, "src1"); + private static final Path PATCHES1_DIR = Paths.get("patches1"); + + // source/destination tree for patch tree 2 + private static final Path SRC2_DIR = Paths.get(TEST_SRC, "src2"); + private static final Path PATCHES2_DIR = Paths.get("patches2"); + + // patch path for java.base + private static final String PATCHES_PATH = + PATCHES1_DIR.resolve("java.base") + File.pathSeparator + + PATCHES2_DIR.resolve("java.base"); + + // the classes overridden or added with --patch-module + private static final String[] CLASSES = { + + // java.base = boot loader + "java.base/java.text.Annotation", // override class + "java.base/java.text.AnnotationBuddy", // add class to package + "java.base/java.lang2.Object", // new package + + }; + + + @BeforeTest + public void setup() throws Exception { + + // javac -d mods/test src/test/** + boolean compiled= CompilerUtils.compile(SRC_DIR.resolve("test"), + MODS_DIR.resolve("test")); + assertTrue(compiled, "classes did not compile"); + + // javac -Xmodule:$MODULE -d patches1/$MODULE patches1/$MODULE/** + Path src = SRC1_DIR.resolve("java.base"); + Path output = PATCHES1_DIR.resolve(src.getFileName()); + Files.createDirectories(output); + String mn = src.getFileName().toString(); + compiled = CompilerUtils.compile(src, output, "-Xmodule:" + mn); + assertTrue(compiled, "classes did not compile"); + + // javac -Xmodule:$MODULE -d patches2/$MODULE patches2/$MODULE/** + src = SRC2_DIR.resolve("java.base"); + output = PATCHES2_DIR.resolve(src.getFileName()); + Files.createDirectories(output); + mn = src.getFileName().toString(); + compiled = CompilerUtils.compile(src, output, "-Xmodule:" + mn); + assertTrue(compiled, "classes did not compile"); + + } + + /** + * Test with --patch-module options patching the same module + */ + public void testDuplicateModule() throws Exception { + int exitValue = + executeTestJava("--patch-module", "java.base=" + PATCHES1_DIR.resolve("java.base"), + "--patch-module", "java.base=" + PATCHES2_DIR.resolve("java.base"), + "--module-path", MODS_DIR.toString(), + "-m", "test/jdk.test.Main") + .outputTo(System.out) + .errorTo(System.out) + // error output by VM + .shouldContain("Cannot specify java.base more than once to --patch-module") + .getExitValue(); + + assertTrue(exitValue != 0); + } + + @DataProvider(name = "emptyItem") + public Object[][] emptyItems() { + String patch1 = PATCHES1_DIR.resolve("java.base").toString(); + String patch2 = PATCHES2_DIR.resolve("java.base").toString(); + String pathSep = File.pathSeparator; + return new Object[][]{ + + { "java.base="+ pathSep + patch1 + pathSep + patch2, null }, + { "java.base="+ patch1 + pathSep + pathSep + patch2, null }, + { "java.base="+ patch1 + pathSep + patch2 + pathSep + pathSep, null }, + }; + } + + /** + * Empty item in a non-empty path list + */ + @Test(dataProvider = "emptyItem") + public void testEmptyItem(String value, String msg) throws Exception { + // the argument to the test is the list of classes overridden or added + String arg = Stream.of(CLASSES).collect(Collectors.joining(",")); + + int exitValue = + executeTestJava("--patch-module", value, + "--add-exports", "java.base/java.lang2=test", + "--module-path", MODS_DIR.toString(), + "-m", "test/jdk.test.Main", arg) + .outputTo(System.out) + .errorTo(System.out) + .getExitValue(); + + assertTrue(exitValue == 0); + } + + /** + * Test bad module name that should emit a warning + */ + public void testBadName() throws Exception { + // the argument to the test is the list of classes overridden or added + String arg = Stream.of(CLASSES).collect(Collectors.joining(",")); + + int exitValue = + executeTestJava("--patch-module", "DoesNotExist=tmp", + "--patch-module", "java.base=" + PATCHES_PATH, + "--add-exports", "java.base/java.lang2=test", + "--module-path", MODS_DIR.toString(), + "-m", "test/jdk.test.Main", arg) + .outputTo(System.out) + .errorTo(System.out) + .shouldContain("WARNING: Unknown module: DoesNotExist specified in --patch-module") + .getExitValue(); + + assertTrue(exitValue == 0); + } + + @DataProvider(name = "badArguments") + public Object[][] badArguments() { + return new Object[][]{ + + // source not found + { "=tmp", "Unable to parse --patch-module =: =tmp" }, + + // target not found: check by VM + { "java.base", "Missing '=' in --patch-module specification" }, + { "foo", "Missing '=' in --patch-module specification" }, + + // target not found + { "java.base=", "Unable to parse --patch-module =: java.base=" }, + { "java.base=" + File.pathSeparator, + "Target must be specified: --patch-module java.base=" + File.pathSeparator } + }; + } + + /** + * Test ill-formed argument to --patch-module + */ + @Test(dataProvider = "badArguments") + public void testBadArgument(String value, String msg) throws Exception { + int exitValue = + executeTestJava("--patch-module", value, + "--module-path", MODS_DIR.toString(), + "-m", "test/jdk.test.Main") + .outputTo(System.out) + .errorTo(System.out) + .shouldContain(msg) + .getExitValue(); + + assertTrue(exitValue != 0); + } +}