8211051: jdeps usage of --dot-output doesn't provide valid output for modular jar
authormchung
Wed, 21 Nov 2018 22:33:33 -0800
changeset 52649 e00cf18e2593
parent 52648 12956ca371c2
child 52650 c16b6cc93272
8211051: jdeps usage of --dot-output doesn't provide valid output for modular jar Reviewed-by: sundar
src/jdk.jdeps/share/classes/com/sun/tools/jdeps/Archive.java
src/jdk.jdeps/share/classes/com/sun/tools/jdeps/JdepsWriter.java
src/jdk.jdeps/share/classes/com/sun/tools/jdeps/ModuleDotGraph.java
test/langtools/tools/jdeps/modules/DotFileTest.java
--- a/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/Archive.java	Thu Nov 22 10:25:44 2018 +0530
+++ b/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/Archive.java	Wed Nov 21 22:33:33 2018 -0800
@@ -37,10 +37,13 @@
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.stream.Stream;
 
+import static com.sun.tools.jdeps.Module.trace;
+
 /**
  * Represents the source of the class files.
  */
@@ -132,6 +135,10 @@
         return path != null ? path.toString() : filename;
     }
 
+    public Optional<Path> path() {
+        return Optional.ofNullable(path);
+    }
+
     @Override
     public int hashCode() {
         return Objects.hash(this.filename, this.path);
@@ -152,9 +159,6 @@
         return filename;
     }
 
-    public Path path() {
-        return path;
-    }
 
     public static boolean isSameLocation(Archive archive, Archive other) {
         if (archive.path == null || other.path == null)
@@ -182,6 +186,7 @@
 
     @Override
     public void close() throws IOException {
+        trace("closing %s %n", getPathName());
         if (reader != null)
             reader.close();
     }
--- a/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/JdepsWriter.java	Thu Nov 22 10:25:44 2018 +0530
+++ b/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/JdepsWriter.java	Wed Nov 21 22:33:33 2018 -0800
@@ -32,10 +32,12 @@
 import java.lang.module.ModuleDescriptor.Requires;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.Collection;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Optional;
 
 public abstract class JdepsWriter {
     public static JdepsWriter newDotWriter(Path outputdir, Analyzer.Type type) {
@@ -79,7 +81,10 @@
                 archives.stream()
                         .filter(analyzer::hasDependences)
                         .forEach(archive -> {
-                            Path dotfile = outputDir.resolve(archive.getName() + ".dot");
+                            // use the filename if path is present; otherwise
+                            // use the module name e.g. from jrt file system
+                            Path path = archive.path().orElse(Paths.get(archive.getName()));
+                            Path dotfile = outputDir.resolve(path.getFileName().toString() + ".dot");
                             try (PrintWriter pw = new PrintWriter(Files.newOutputStream(dotfile));
                                  DotFileFormatter formatter = new DotFileFormatter(pw, archive)) {
                                 analyzer.visitDependences(archive, formatter);
@@ -92,6 +97,7 @@
             generateSummaryDotFile(archives, analyzer);
         }
 
+
         private void generateSummaryDotFile(Collection<Archive> archives, Analyzer analyzer)
                 throws IOException
         {
--- a/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/ModuleDotGraph.java	Thu Nov 22 10:25:44 2018 +0530
+++ b/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/ModuleDotGraph.java	Wed Nov 21 22:33:33 2018 -0800
@@ -47,6 +47,7 @@
 import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.Set;
 import java.util.TreeSet;
 import java.util.function.Function;
@@ -57,10 +58,12 @@
  * Generate dot graph for modules
  */
 public class ModuleDotGraph {
+    private final JdepsConfiguration config;
     private final Map<String, Configuration> configurations;
     private final boolean apiOnly;
     public ModuleDotGraph(JdepsConfiguration config, boolean apiOnly) {
-        this(config.rootModules().stream()
+        this(config,
+             config.rootModules().stream()
                    .map(Module::name)
                    .sorted()
                    .collect(toMap(Function.identity(), mn -> config.resolve(Set.of(mn)))),
@@ -68,8 +71,15 @@
     }
 
     public ModuleDotGraph(Map<String, Configuration> configurations, boolean apiOnly) {
+        this(null, configurations, apiOnly);
+    }
+
+    private ModuleDotGraph(JdepsConfiguration config,
+                           Map<String, Configuration> configurations,
+                           boolean apiOnly) {
         this.configurations = configurations;
         this.apiOnly = apiOnly;
+        this.config = config;
     }
 
     /**
@@ -86,12 +96,22 @@
     {
         Files.createDirectories(dir);
         for (String mn : configurations.keySet()) {
-            Path path = dir.resolve(mn + ".dot");
+            Path path = dir.resolve(toDotFileBaseName(mn) + ".dot");
             genDotFile(path, mn, configurations.get(mn), attributes);
         }
         return true;
     }
 
+    private String toDotFileBaseName(String mn) {
+        if (config == null)
+            return mn;
+
+        Optional<Path> path = config.findModule(mn).flatMap(Module::path);
+        if (path.isPresent())
+            return path.get().getFileName().toString();
+        else
+            return mn;
+    }
     /**
      * Generate dotfile of the given path
      */
--- a/test/langtools/tools/jdeps/modules/DotFileTest.java	Thu Nov 22 10:25:44 2018 +0530
+++ b/test/langtools/tools/jdeps/modules/DotFileTest.java	Wed Nov 21 22:33:33 2018 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2018, 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
@@ -28,12 +28,16 @@
  * @modules java.desktop
  *          java.sql
  *          jdk.jdeps/com.sun.tools.jdeps
+ *          jdk.unsupported
+ * @library ../lib
+ * @build CompilerUtils
  * @run testng DotFileTest
  */
 
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.HashSet;
 import java.util.Set;
 
 import java.util.regex.Matcher;
@@ -41,6 +45,7 @@
 import java.util.spi.ToolProvider;
 import java.util.stream.Collectors;
 
+import org.testng.annotations.BeforeTest;
 import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 
@@ -50,9 +55,19 @@
 public class DotFileTest {
     private static final ToolProvider JDEPS = ToolProvider.findFirst("jdeps")
         .orElseThrow(() -> new RuntimeException("jdeps not found"));
+    private static final ToolProvider JAR = ToolProvider.findFirst("jar")
+        .orElseThrow(() -> new RuntimeException("jar not found"));
 
+    private static final String TEST_SRC = System.getProperty("test.src");
     private static final Path DOTS_DIR = Paths.get("dots");
     private static final Path SPEC_DIR = Paths.get("spec");
+    private static final Path MODS = Paths.get("mods");
+
+
+    @BeforeTest
+    public void setup() throws Exception {
+        assertTrue(CompilerUtils.compile(Paths.get(TEST_SRC, "src", "unsafe"), MODS));
+    }
 
     @DataProvider(name = "modules")
     public Object[][] modules() {
@@ -125,6 +140,79 @@
         assertEquals(lines, edges);
     }
 
+    /*
+     * Test if the file name of the dot output file matches the input filename
+     */
+    @Test
+    public void testModularJar() throws Exception {
+        String filename = "org.unsafe-v1.0.jar";
+        assertTrue(JAR.run(System.out, System.out, "cf", filename,
+                           "-C", MODS.toString(), ".") == 0);
+
+        // assertTrue(JDEPS.run(System.out, System.out,
+        //              "--dot-output", DOTS_DIR.toString(), filename) == 0);
+        assertTrue(JDEPS.run(System.out, System.out,
+                             "--dot-output", DOTS_DIR.toString(),
+                             "--module-path", filename,
+                             "-m", "unsafe") == 0);
+
+        Path path = DOTS_DIR.resolve(filename + ".dot");
+        assertTrue(Files.exists(path));
+
+        // package dependences
+        Set<String> expected = Set.of(
+            "org.indirect -> java.lang",
+            "org.indirect -> org.unsafe",
+            "org.safe -> java.io",
+            "org.safe -> java.lang",
+            "org.unsafe -> java.lang",
+            "org.unsafe -> sun.misc"
+        );
+
+        Pattern pattern = Pattern.compile("(.*) -> +([^ ]*) (.*)");
+        Set<String> lines = new HashSet<>();
+        for (String line : Files.readAllLines(path)) {
+            line = line.replace('"', ' ').replace(';', ' ');
+            Matcher pm = pattern.matcher(line);
+            if (pm.find()) {
+                String origin = pm.group(1).trim();
+                String target = pm.group(2).trim();
+                lines.add(origin + " -> " + target);
+            }
+        }
+        assertEquals(lines, expected);
+    }
+
+    /*
+     * Test module summary with -m option
+     */
+    @Test
+    public void testModuleSummary() throws Exception {
+        String filename = "org.unsafe-v2.0.jar";
+        assertTrue(JAR.run(System.out, System.out, "cf", filename,
+                           "-C", MODS.toString(), ".") == 0);
+
+        assertTrue(JDEPS.run(System.out, System.out, "-s",
+                             "--dot-output", DOTS_DIR.toString(),
+                             "--module-path", filename,
+                             "-m", "unsafe") == 0);
+
+        Path path = DOTS_DIR.resolve(filename + ".dot");
+        assertTrue(Files.exists(path));
+
+        // module dependences
+        Set<String> expected = Set.of(
+            "unsafe -> jdk.unsupported",
+            "jdk.unsupported -> java.base"
+        );
+
+        Set<String> lines = Files.readAllLines(path).stream()
+                                 .filter(l -> l.contains(" -> "))
+                                 .map(this::split)
+                                 .collect(Collectors.toSet());
+        assertEquals(lines, expected);
+    }
+
     static Pattern PATTERN = Pattern.compile(" *\"(\\S+)\" -> \"(\\S+)\" .*");
     String split(String line) {
         Matcher pm = PATTERN.matcher(line);