8166846: jdeps fails to generate module info if there is any class in unnamed package
authormchung
Wed, 05 Oct 2016 18:41:43 -0700
changeset 41442 14db641d4a9f
parent 41441 8fb8d9c6c687
child 41443 6398801363f2
8166846: jdeps fails to generate module info if there is any class in unnamed package Reviewed-by: lancea
langtools/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/ClassFileReader.java
langtools/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/JdepsTask.java
langtools/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/resources/jdeps.properties
langtools/test/tools/jdeps/jdkinternals/RemovedJDKInternals.java
langtools/test/tools/jdeps/jdkinternals/ShowReplacement.java
langtools/test/tools/jdeps/lib/JdepsRunner.java
langtools/test/tools/jdeps/lib/JdepsUtil.java
langtools/test/tools/jdeps/modules/GenModuleInfo.java
langtools/test/tools/jdeps/modules/UnnamedPackage.java
--- a/langtools/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/ClassFileReader.java	Wed Oct 05 13:06:21 2016 +0100
+++ b/langtools/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/ClassFileReader.java	Wed Oct 05 18:41:43 2016 -0700
@@ -63,7 +63,7 @@
      * Returns a ClassFileReader instance of a given path.
      */
     public static ClassFileReader newInstance(Path path) throws IOException {
-        return newInstance(path, JarFile.baseVersion());
+        return newInstance(path, null);
     }
 
     /**
--- a/langtools/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/JdepsTask.java	Wed Oct 05 13:06:21 2016 +0100
+++ b/langtools/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/JdepsTask.java	Wed Oct 05 18:41:43 2016 -0700
@@ -483,6 +483,7 @@
             }
             if (options.checkModuleDeps != null && !inputArgs.isEmpty()) {
                 reportError("err.invalid.module.option", inputArgs, "--check");
+                return EXIT_CMDERR;
             }
 
             boolean ok = run();
@@ -676,17 +677,29 @@
     }
 
     private boolean genModuleInfo(JdepsConfiguration config) throws IOException {
+        // check if any JAR file contains unnamed package
+        for (String arg : inputArgs) {
+            Optional<String> classInUnnamedPackage =
+                ClassFileReader.newInstance(Paths.get(arg))
+                    .entries().stream()
+                    .filter(n -> n.endsWith(".class"))
+                    .filter(cn -> toPackageName(cn).isEmpty())
+                    .findFirst();
+
+            if (classInUnnamedPackage.isPresent()) {
+                if (classInUnnamedPackage.get().equals("module-info.class")) {
+                    reportError("err.genmoduleinfo.not.jarfile", arg);
+                } else {
+                    reportError("err.genmoduleinfo.unnamed.package", arg);
+                }
+                return false;
+            }
+        }
+
         ModuleInfoBuilder builder
             = new ModuleInfoBuilder(config, inputArgs, options.genModuleInfo);
         boolean ok = builder.run();
 
-        builder.modules().forEach(module -> {
-            if (module.packages().contains("")) {
-                reportError("ERROR: %s contains unnamed package.  " +
-                    "module-info.java not generated%n", module.getPathName());
-            }
-        });
-
         if (!ok && !options.nowarning) {
             log.println("ERROR: missing dependencies");
             builder.visitMissingDeps(
@@ -703,6 +716,11 @@
         return ok;
     }
 
+    private String toPackageName(String name) {
+        int i = name.lastIndexOf('/');
+        return i > 0 ? name.replace('/', '.').substring(0, i) : "";
+    }
+
     /**
      * Returns a filter used during dependency analysis
      */
--- a/langtools/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/resources/jdeps.properties	Wed Oct 05 13:06:21 2016 +0100
+++ b/langtools/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/resources/jdeps.properties	Wed Oct 05 18:41:43 2016 -0700
@@ -166,7 +166,8 @@
 err.missing.arg=no value given for {0}
 err.invalid.arg.for.option=invalid argument for option: {0}
 err.option.after.class=option must be specified before classes: {0}
-err.genmoduleinfo.not.jarfile={0} not valid for --generate-module-info option (must be non-modular JAR file)
+err.genmoduleinfo.not.jarfile={0} is a modular JAR file that cannot be specified with the --generate-module-info option
+err.genmoduleinfo.unnamed.package={0} contains an unnamed package that is not allowed in a module
 err.profiles.msg=No profile information
 err.exception.message={0}
 err.invalid.path=invalid path: {0}
--- a/langtools/test/tools/jdeps/jdkinternals/RemovedJDKInternals.java	Wed Oct 05 13:06:21 2016 +0100
+++ b/langtools/test/tools/jdeps/jdkinternals/RemovedJDKInternals.java	Wed Oct 05 18:41:43 2016 -0700
@@ -26,7 +26,7 @@
  * @bug 8153042
  * @summary Tests JDK internal APIs that have been removed.
  * @library ../lib
- * @build CompilerUtils JdepsUtil ModuleMetaData
+ * @build CompilerUtils JdepsRunner JdepsUtil ModuleMetaData
  * @modules jdk.jdeps/com.sun.tools.jdeps
  * @run testng RemovedJDKInternals
  */
@@ -123,7 +123,8 @@
 
     @Test
     public void checkReplacement() {
-        String[] output = JdepsUtil.jdeps("-jdkinternals", CLASSES_DIR.toString());
+        JdepsRunner jdeps = JdepsRunner.run("-jdkinternals", CLASSES_DIR.toString());
+        String[] output = jdeps.output();
         int i = 0;
         while (!output[i].contains("Suggested Replacement")) {
             i++;
--- a/langtools/test/tools/jdeps/jdkinternals/ShowReplacement.java	Wed Oct 05 13:06:21 2016 +0100
+++ b/langtools/test/tools/jdeps/jdkinternals/ShowReplacement.java	Wed Oct 05 18:41:43 2016 -0700
@@ -70,7 +70,8 @@
     @Test
     public void withReplacement() {
         Path file = Paths.get("q", "WithRepl.class");
-        String[] output = JdepsUtil.jdeps("-jdkinternals", CLASSES_DIR.resolve(file).toString());
+        JdepsRunner jdeps = JdepsRunner.run("-jdkinternals", CLASSES_DIR.resolve(file).toString());
+        String[] output = jdeps.output();
         int i = 0;
         while (!output[i].contains("Suggested Replacement")) {
             i++;
@@ -99,7 +100,8 @@
     @Test
     public void noReplacement() {
         Path file = Paths.get("q", "NoRepl.class");
-        String[] output = JdepsUtil.jdeps("-jdkinternals", CLASSES_DIR.resolve(file).toString());
+        JdepsRunner jdeps = JdepsRunner.run("-jdkinternals", CLASSES_DIR.resolve(file).toString());
+        String[] output = jdeps.output();
         int i = 0;
         // expect no replacement
         while (i < output.length && !output[i].contains("Suggested Replacement")) {
@@ -116,7 +118,8 @@
     @Test
     public void removedPackage() {
         Path file = Paths.get("q", "RemovedPackage.class");
-        String[] output = JdepsUtil.jdeps("--jdk-internals", CLASSES_DIR.resolve(file).toString());
+        JdepsRunner jdeps = JdepsRunner.run("--jdk-internals", CLASSES_DIR.resolve(file).toString());
+        String[] output = jdeps.output();
         int i = 0;
         // expect no replacement
         while (i < output.length && !output[i].contains("Suggested Replacement")) {
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/jdeps/lib/JdepsRunner.java	Wed Oct 05 18:41:43 2016 -0700
@@ -0,0 +1,88 @@
+/*
+ * 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.PrintStream;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * JdepsRunner class to invoke jdeps with the given command line argument
+ */
+public class JdepsRunner {
+    public static JdepsRunner run(String... args) {
+        JdepsRunner jdeps = new JdepsRunner(args);
+        int rc = jdeps.run();
+        jdeps.printStdout(System.err);
+        if (rc != 0)
+           throw new Error("jdeps failed: rc=" + rc);
+        return jdeps;
+    }
+
+    final StringWriter stdout = new StringWriter();
+    final StringWriter stderr = new StringWriter();
+    final String[] args;
+    public JdepsRunner(String... args) {
+        System.err.println("jdeps " + Arrays.stream(args)
+                                            .collect(Collectors.joining(" ")));
+        this.args = args;
+    }
+
+    public JdepsRunner(List<String> args) {
+        this(args.toArray(new String[0]));
+    }
+
+    public int run() {
+        return run(false);
+    }
+
+    public int run(boolean showOutput) {
+        try (PrintWriter pw = new PrintWriter(stdout)) {
+            int rc = com.sun.tools.jdeps.Main.run(args, pw);
+            if (showOutput) {
+                System.err.println(stdout.toString());
+            }
+            return rc;
+        }
+    }
+
+    public boolean outputContains(String s) {
+        return stdout.toString().contains(s);
+    }
+
+    public void printStdout(PrintStream stream) {
+        stream.println(stdout.toString());
+    }
+
+    public void printStderr(PrintStream stream) {
+        stream.println(stderr.toString());
+    }
+
+    public String[] output() {
+        String lineSep = System.getProperty("line.separator");
+        return stdout.toString().split(lineSep);
+    }
+}
--- a/langtools/test/tools/jdeps/lib/JdepsUtil.java	Wed Oct 05 13:06:21 2016 +0100
+++ b/langtools/test/tools/jdeps/lib/JdepsUtil.java	Wed Oct 05 18:41:43 2016 -0700
@@ -39,37 +39,17 @@
 import java.lang.module.ModuleDescriptor;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.util.Arrays;
 import java.util.HashSet;
 import java.util.Set;
 import java.util.jar.JarEntry;
 import java.util.jar.JarOutputStream;
 import java.util.regex.Pattern;
-import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 /**
  * Utilities to run jdeps command
  */
 public final class JdepsUtil {
-    /*
-     * Runs jdeps with the given arguments
-     */
-    public static String[] jdeps(String... args) {
-        String lineSep =     System.getProperty("line.separator");
-        StringWriter sw = new StringWriter();
-        PrintWriter pw = new PrintWriter(sw);
-        System.err.println("jdeps " + Arrays.stream(args).collect(Collectors.joining(" ")));
-        int rc = com.sun.tools.jdeps.Main.run(args, pw);
-        pw.close();
-        String out = sw.toString();
-        if (!out.isEmpty())
-            System.err.println(out);
-        if (rc != 0)
-            throw new Error("jdeps failed: rc=" + rc);
-        return out.split(lineSep);
-    }
-
     public static Command newCommand(String cmd) {
         return new Command(cmd);
     }
--- a/langtools/test/tools/jdeps/modules/GenModuleInfo.java	Wed Oct 05 13:06:21 2016 +0100
+++ b/langtools/test/tools/jdeps/modules/GenModuleInfo.java	Wed Oct 05 18:41:43 2016 -0700
@@ -25,7 +25,7 @@
  * @test
  * @summary Tests jdeps --generate-module-info option
  * @library ../lib
- * @build CompilerUtils JdepsUtil
+ * @build CompilerUtils JdepsUtil JdepsRunner
  * @modules jdk.jdeps/com.sun.tools.jdeps
  * @run testng GenModuleInfo
  */
@@ -97,7 +97,7 @@
         Stream<String> files = Arrays.stream(modules)
                 .map(mn -> LIBS_DIR.resolve(mn + ".jar"))
                 .map(Path::toString);
-        JdepsUtil.jdeps(Stream.concat(Stream.of("-cp"), files).toArray(String[]::new));
+        JdepsRunner.run(Stream.concat(Stream.of("-cp"), files).toArray(String[]::new));
     }
 
     @Test
@@ -106,7 +106,7 @@
                 .map(mn -> LIBS_DIR.resolve(mn + ".jar"))
                 .map(Path::toString);
 
-        JdepsUtil.jdeps(Stream.concat(Stream.of("--generate-module-info", DEST_DIR.toString()),
+        JdepsRunner.run(Stream.concat(Stream.of("--generate-module-info", DEST_DIR.toString()),
                                       files).toArray(String[]::new));
 
         // check file exists
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/jdeps/modules/UnnamedPackage.java	Wed Oct 05 18:41:43 2016 -0700
@@ -0,0 +1,60 @@
+/*
+ * 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 8166846
+ * @summary Tests --generate-module-info on invalid JAR file
+ * @modules jdk.jdeps/com.sun.tools.jdeps
+ * @library ../lib
+ * @build JdepsRunner JdepsUtil
+ * @run main UnnamedPackage
+ */
+
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.stream.Stream;
+
+public class UnnamedPackage {
+    private static final Path TEST_CLASSES = Paths.get(System.getProperty("test.classes"));
+    private static final Path FOO_JAR_FILE = Paths.get("foo.jar");
+
+    public static void main(String... args) throws Exception {
+        // create foo.jar with unnamed package
+        Path name = TEST_CLASSES.resolve("UnnamedPackage.class");
+        JdepsUtil.createJar(FOO_JAR_FILE, TEST_CLASSES, Stream.of(name));
+
+        // run jdeps --generate-module-info
+        JdepsRunner jdeps = new JdepsRunner("--generate-module-info",
+                                            "tmp", FOO_JAR_FILE.toString());
+        // should fail to generate module-info.java
+        int exitValue = jdeps.run();
+        if (exitValue == 0) {
+            throw new RuntimeException("expected non-zero exitValue");
+        }
+        if (!jdeps.outputContains("foo.jar contains an unnamed package")) {
+            jdeps.printStdout(System.out);
+            throw new RuntimeException("expected error message not found");
+        }
+    }
+}