Mercurial > hg > openjdk > jdk9 > langtools

changeset 3854:7428762055ef

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
8172212: jdeps --require and --check should detect the specified module in the image Reviewed-by: psandoz, lancea
author mchung
date Tue, 03 Jan 2017 17:53:34 -0800
parents 9d2379f8dd41
children 6acc773a9fc6
files src/jdk.jdeps/share/classes/com/sun/tools/jdeps/JdepsConfiguration.java src/jdk.jdeps/share/classes/com/sun/tools/jdeps/JdepsTask.java src/jdk.jdeps/share/classes/com/sun/tools/jdeps/ModuleExportsAnalyzer.java src/jdk.jdeps/share/classes/com/sun/tools/jdeps/resources/jdeps.properties test/tools/jdeps/listdeps/ListModuleDeps.java test/tools/jdeps/modules/CheckModuleTest.java test/tools/jdeps/modules/InverseDeps.java test/tools/jdeps/modules/SplitPackage.java
diffstat 8 files changed, 172 insertions(+), 49 deletions(-) [+]
line wrap: on
line diff
--- a/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/JdepsConfiguration.java	Tue Jan 03 16:27:54 2017 -0700
+++ b/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/JdepsConfiguration.java	Tue Jan 03 17:53:34 2017 -0800
@@ -105,18 +105,18 @@
         // build root set for resolution
         Set<String> mods = new HashSet<>(roots);
 
-        // add default modules to the root set
-        // unnamed module
-        if (!initialArchives.isEmpty() || !classpaths.isEmpty() ||
-                roots.isEmpty() || allDefaultModules) {
-            mods.addAll(systemModulePath.defaultSystemRoots());
-        }
-        if (allSystemModules) {
+        // add all system modules to the root set for unnamed module or set explicitly
+        boolean unnamed = !initialArchives.isEmpty() || !classpaths.isEmpty();
+        if (allSystemModules || (unnamed && !allDefaultModules)) {
             systemModulePath.findAll().stream()
                 .map(mref -> mref.descriptor().name())
                 .forEach(mods::add);
         }
 
+        if (allDefaultModules) {
+            mods.addAll(systemModulePath.defaultSystemRoots());
+        }
+
         this.configuration = Configuration.empty()
                 .resolveRequires(finder, ModuleFinder.of(), mods);
 
@@ -502,6 +502,7 @@
         boolean addAllApplicationModules;
         boolean addAllDefaultModules;
         boolean addAllSystemModules;
+        boolean allModules;
         Runtime.Version version;
 
         public Builder() {
@@ -550,8 +551,7 @@
          * Include all system modules and modules found on modulepath
          */
         public Builder allModules() {
-            this.addAllSystemModules = true;
-            this.addAllApplicationModules = true;
+            this.allModules = true;
             return this;
         }
 
@@ -592,19 +592,30 @@
                         .map(mref -> mref.descriptor().name())
                         .forEach(rootModules::add);
             }
-            if (addAllApplicationModules && appModulePath != null) {
+
+            if ((addAllApplicationModules || allModules) && appModulePath != null) {
                 appModulePath.findAll().stream()
                     .map(mref -> mref.descriptor().name())
                     .forEach(rootModules::add);
             }
 
+            // no archive is specified for analysis
+            // add all system modules as root if --add-modules ALL-SYSTEM is specified
+            if (addAllSystemModules && rootModules.isEmpty() &&
+                    initialArchives.isEmpty() && classPaths.isEmpty()) {
+                systemModulePath.findAll()
+                    .stream()
+                    .map(mref -> mref.descriptor().name())
+                    .forEach(rootModules::add);
+            }
+
             return new JdepsConfiguration(systemModulePath,
                                           finder,
                                           rootModules,
                                           classPaths,
                                           initialArchives,
                                           addAllDefaultModules,
-                                          addAllSystemModules,
+                                          allModules,
                                           version);
         }
 
--- a/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/JdepsTask.java	Tue Jan 03 16:27:54 2017 -0700
+++ b/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/JdepsTask.java	Tue Jan 03 17:53:34 2017 -0800
@@ -38,10 +38,10 @@
 import java.nio.file.Paths;
 import java.text.MessageFormat;
 import java.util.*;
+import java.util.function.Function;
+import java.util.function.ToIntFunction;
 import java.util.jar.JarFile;
 import java.util.regex.Pattern;
-import java.util.stream.Collectors;
-import java.util.stream.Stream;
 
 /**
  * Implementation for the jdeps tool for static class dependency analysis.
@@ -314,7 +314,10 @@
         },
         new Option(true, "-m", "--module") {
             void process(JdepsTask task, String opt, String arg) throws BadArgs {
-                task.options.rootModule = arg;
+                if (!task.options.rootModules.isEmpty()) {
+                    throw new BadArgs("err.option.already.specified", opt);
+                }
+                task.options.rootModules.add(arg);
                 task.options.addmods.add(arg);
             }
         },
@@ -350,6 +353,7 @@
         new Option(true, "--require") {
             void process(JdepsTask task, String opt, String arg) {
                 task.options.requires.add(arg);
+                task.options.addmods.add(arg);
             }
         },
         new Option(true, "-f", "-filter") {
@@ -491,11 +495,6 @@
             if (options.help || options.version || options.fullVersion) {
                 return EXIT_OK;
             }
-
-            if (!inputArgs.isEmpty() && options.rootModule != null) {
-                reportError("err.invalid.arg.for.option", "-m");
-            }
-
             if (options.numFilters() > 1) {
                 reportError("err.invalid.filters");
                 return EXIT_CMDERR;
@@ -543,8 +542,8 @@
                                                      e.getKey(),
                                                      e.getValue().toString())));
 
-            // check if any module specified in --require is missing
-            Stream.concat(options.addmods.stream(), options.requires.stream())
+            // check if any module specified in --add-modules, --require, and -m is missing
+            options.addmods.stream()
                 .filter(mn -> !config.isValidToken(mn))
                 .forEach(mn -> config.findModule(mn).orElseThrow(() ->
                     new UncheckedBadArgs(new BadArgs("err.module.not.found", mn))));
@@ -620,6 +619,7 @@
         protected Command(CommandOption option) {
             this.option = option;
         }
+
         /**
          * Returns true if the command-line options are all valid;
          * otherwise, returns false.
@@ -633,6 +633,10 @@
 
         /**
          * Includes all modules on system module path and application module path
+         *
+         * When a named module is analyzed, it will analyze the dependences
+         * only.  The method should be overridden when this command should
+         * analyze all modules instead.
          */
         boolean allModules() {
             return false;
@@ -680,6 +684,10 @@
                     return false;
                 }
             }
+
+            if (!inputArgs.isEmpty() && !options.rootModules.isEmpty()) {
+                reportError("err.invalid.arg.for.option", "-m");
+            }
             if (inputArgs.isEmpty() && !options.hasSourcePath()) {
                 showHelp();
                 return false;
@@ -808,23 +816,46 @@
             log.println();
             if (!options.requires.isEmpty())
                 log.println(getMessage("inverse.transitive.dependencies.on",
-                    options.requires));
+                                       options.requires));
             else
                 log.println(getMessage("inverse.transitive.dependencies.matching",
-                    options.regex != null
-                        ? options.regex.toString()
-                        : "packages " + options.packageNames));
+                                       options.regex != null
+                                           ? options.regex.toString()
+                                           : "packages " + options.packageNames));
 
-            analyzer.inverseDependences().stream()
-                .sorted(Comparator.comparing(this::sortPath))
-                .forEach(path -> log.println(path.stream()
-                    .map(Archive::getName)
-                    .collect(joining(" <- "))));
+            analyzer.inverseDependences()
+                    .stream()
+                    .sorted(comparator())
+                    .map(this::toInversePath)
+                    .forEach(log::println);
             return ok;
         }
 
-        private String sortPath(Deque<Archive> path) {
-            return path.peekFirst().getName();
+        private String toInversePath(Deque<Archive> path) {
+            return path.stream()
+                       .map(Archive::getName)
+                       .collect(joining(" <- "));
+        }
+
+        /*
+         * Returns a comparator for sorting the inversed path, grouped by
+         * the first module name, then the shortest path and then sort by
+         * the module names of each path
+         */
+        private Comparator<Deque<Archive>> comparator() {
+            return Comparator.<Deque<Archive>, String>
+                comparing(deque -> deque.peekFirst().getName())
+                    .thenComparingInt(Deque::size)
+                    .thenComparing(this::toInversePath);
+        }
+
+        /*
+         * Returns true if --require is specified so that all modules are
+         * analyzed to find all modules that depend on the modules specified in the
+         * --require option directly and indirectly
+         */
+        public boolean allModules() {
+            return options.requires.size() > 0;
         }
     }
 
@@ -924,6 +955,9 @@
             return new ModuleAnalyzer(config, log, modules).run();
         }
 
+        /*
+         * Returns true to analyze all modules
+         */
         public boolean allModules() {
             return true;
         }
@@ -957,6 +991,10 @@
                             option);
                 return false;
             }
+
+            if (!inputArgs.isEmpty() && !options.rootModules.isEmpty()) {
+                reportError("err.invalid.arg.for.option", "-m");
+            }
             if (inputArgs.isEmpty() && !options.hasSourcePath()) {
                 showHelp();
                 return false;
@@ -971,11 +1009,6 @@
                                              reduced,
                                              log).run();
         }
-
-        @Override
-        boolean allModules() {
-            return true;
-        }
     }
 
 
@@ -1155,7 +1188,7 @@
         String systemModulePath = System.getProperty("java.home");
         String upgradeModulePath;
         String modulePath;
-        String rootModule;
+        Set<String> rootModules = new HashSet<>();
         Set<String> addmods = new HashSet<>();
         Runtime.Version multiRelease;
 
--- a/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/ModuleExportsAnalyzer.java	Tue Jan 03 16:27:54 2017 -0700
+++ b/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/ModuleExportsAnalyzer.java	Tue Jan 03 17:53:34 2017 -0800
@@ -34,6 +34,7 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import static com.sun.tools.jdeps.Analyzer.NOT_FOUND;
 
@@ -109,9 +110,7 @@
     private void printDependences() {
         // find use of JDK internals
         Map<Module, Set<String>> jdkinternals = new HashMap<>();
-        deps.keySet().stream()
-            .filter(source -> !source.getModule().isNamed())
-            .map(deps::get)
+        dependenceStream()
             .flatMap(map -> map.entrySet().stream())
             .filter(e -> e.getValue().size() > 0)
             .forEach(e -> jdkinternals.computeIfAbsent(e.getKey().getModule(),
@@ -124,11 +123,10 @@
         Module root = new RootModule("root");
         builder.addModule(root);
         // find named module dependences
-        deps.keySet().stream()
-            .filter(source -> !source.getModule().isNamed())
-            .map(deps::get)
+        dependenceStream()
             .flatMap(map -> map.keySet().stream())
-            .filter(m -> m.getModule().isNamed())
+            .filter(m -> m.getModule().isNamed()
+                            && !configuration.rootModules().contains(m))
             .map(Archive::getModule)
             .forEach(m -> builder.addEdge(root, m));
 
@@ -167,6 +165,16 @@
             });
     }
 
+    /*
+     * Returns a stream of dependence map from an Archive to the set of JDK
+     * internal APIs being used.
+     */
+    private Stream<Map<Archive, Set<String>>> dependenceStream() {
+        return deps.keySet().stream()
+                   .filter(source -> !source.getModule().isNamed()
+                            || configuration.rootModules().contains(source))
+                   .map(deps::get);
+    }
 
     private class RootModule extends Module {
         final ModuleDescriptor descriptor;
--- a/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/resources/jdeps.properties	Tue Jan 03 16:27:54 2017 -0700
+++ b/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/resources/jdeps.properties	Tue Jan 03 17:53:34 2017 -0800
@@ -194,6 +194,7 @@
 err.invalid.options={0} cannot be used with {1} option
 err.module.not.found=module not found: {0}
 err.root.module.not.set=root module set empty
+err.option.already.specified={0} option specified more than once.
 err.filter.not.specified=--package (-p), --regex (-e), --require option must be specified
 err.multirelease.option.exists={0} is not a multi-release jar file, but the --multi-release option is set
 err.multirelease.option.notfound={0} is a multi-release jar file, but the --multi-release option is not set
--- a/test/tools/jdeps/listdeps/ListModuleDeps.java	Tue Jan 03 16:27:54 2017 -0700
+++ b/test/tools/jdeps/listdeps/ListModuleDeps.java	Tue Jan 03 17:53:34 2017 -0800
@@ -83,6 +83,28 @@
         ));
     }
 
+    @DataProvider(name = "jdkModules")
+    public Object[][] jdkModules() {
+        return new Object[][]{
+            {"jdk.compiler", new String[]{
+                                "java.base/sun.reflect.annotation",
+                                "java.compiler",
+                             }
+            },
+        };
+    }
+
+    @Test(dataProvider = "jdkModules")
+    public void testJDKModule(String moduleName, String[] expected) {
+        JdepsRunner jdeps = JdepsRunner.run(
+            "--list-deps", "-m", moduleName
+        );
+        String[] output = Arrays.stream(jdeps.output())
+                                .map(s -> s.trim())
+                                .toArray(String[]::new);
+        assertEquals(output, expected);
+    }
+
     @Test(dataProvider = "listdeps")
     public void testListDeps(Path classes, String[] expected) {
         JdepsRunner jdeps = JdepsRunner.run(
--- a/test/tools/jdeps/modules/CheckModuleTest.java	Tue Jan 03 16:27:54 2017 -0700
+++ b/test/tools/jdeps/modules/CheckModuleTest.java	Tue Jan 03 17:53:34 2017 -0800
@@ -57,6 +57,7 @@
     private static final Set<String> modules = Set.of("unsafe", "mIV", "mV", "mVI", "mVII", "mVIII");
 
     private static final String JAVA_BASE = "java.base";
+    private static final String JAVA_COMPILER = "java.compiler";
 
     /**
      * Compiles classes used by the test
@@ -73,6 +74,8 @@
         return new Object[][] {
             { JAVA_BASE, new ModuleMetaData(JAVA_BASE)
             },
+            { JAVA_COMPILER, new ModuleMetaData(JAVA_BASE)
+            },
         };
     };
 
--- a/test/tools/jdeps/modules/InverseDeps.java	Tue Jan 03 16:27:54 2017 -0700
+++ b/test/tools/jdeps/modules/InverseDeps.java	Tue Jan 03 17:53:34 2017 -0800
@@ -26,7 +26,9 @@
  * @summary Tests split packages
  * @library ../lib
  * @build CompilerUtils JdepsUtil
- * @modules jdk.jdeps/com.sun.tools.jdeps
+ * @modules java.logging
+ *          jdk.jdeps/com.sun.tools.jdeps
+ *          jdk.unsupported
  * @run testng InverseDeps
  */
 
@@ -87,6 +89,44 @@
             }
         }
     }
+    @DataProvider(name = "jdkModules")
+    public Object[][] jdkModules() {
+        return new Object[][]{
+            // --require and a subset of dependences
+            { "jdk.compiler", new String[][] {
+                    new String[] {"jdk.compiler", "jdk.jshell"},
+                    new String[] {"jdk.compiler", "jdk.rmic"},
+                    new String[] {"jdk.compiler", "jdk.javadoc", "jdk.rmic"},
+                }
+            },
+            { "java.compiler", new String[][] {
+                    new String[] {"java.compiler", "jdk.jshell"},
+                    new String[] {"java.compiler", "jdk.compiler", "jdk.jshell"},
+                    new String[] {"java.compiler", "jdk.compiler", "jdk.rmic"},
+                    new String[] {"java.compiler", "jdk.compiler", "jdk.javadoc", "jdk.rmic"},
+                    new String[] {"java.compiler", "java.se", "java.se.ee"},
+                }
+            },
+        };
+    }
+
+    @Test(dataProvider = "jdkModules")
+    public void testJDKModule(String moduleName, String[][] expected) throws Exception {
+        // this invokes the jdeps launcher so that all system modules are observable
+        JdepsRunner jdeps = JdepsRunner.run(
+            "--inverse", "--require", moduleName
+        );
+        List<String> output = Arrays.stream(jdeps.output())
+            .map(s -> s.trim())
+            .collect(Collectors.toList());
+
+        // verify the dependences
+        assertTrue(Arrays.stream(expected)
+                         .map(path -> Arrays.stream(path)
+                         .collect(Collectors.joining(" <- ")))
+                         .anyMatch(output::contains));
+    }
+
 
     @DataProvider(name = "testrequires")
     public Object[][] expected1() {
--- a/test/tools/jdeps/modules/SplitPackage.java	Tue Jan 03 16:27:54 2017 -0700
+++ b/test/tools/jdeps/modules/SplitPackage.java	Tue Jan 03 17:53:34 2017 -0800
@@ -63,10 +63,15 @@
 
     @Test
     public void runTest() throws Exception {
+        // split package detected if java.annotation.common is in the root set
+        runTest(JAVA_ANNOTATIONS_COMMON, SPLIT_PKG_NAME);
+        runTest("ALL-SYSTEM", SPLIT_PKG_NAME);
+        // default
+        runTest(null, SPLIT_PKG_NAME);
+
         // Test jdeps classes
-        runTest(null);
-        // Test jdeps --add-modules
-        runTest(JAVA_ANNOTATIONS_COMMON, SPLIT_PKG_NAME);
+        runTest("ALL-DEFAULT");
+
     }
 
     private void runTest(String root, String... splitPackages) throws Exception {