8160286: jmod hash is creating unlinkable modules

Reviewed-by: alanb, psandoz, chegar

jdk/src/java.base/share/classes/jdk/internal/module/ModuleHashesBuilder.java

@@ -0,0 +1,312 @@
+/*
+ * Copyright (c) 2017, 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.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * 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.
+ */
+
+package jdk.internal.module;
+
+import java.io.PrintStream;
+import java.lang.module.Configuration;
+import java.lang.module.ResolvedModule;
+import java.net.URI;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.ArrayDeque;
+import java.util.Collections;
+import java.util.Deque;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Consumer;
+import java.util.function.Function;
+import java.util.stream.Stream;
+import static java.util.stream.Collectors.*;
+
+/**
+ * A Builder to compute ModuleHashes from a given configuration
+ */
+public class ModuleHashesBuilder {
+    private final Configuration configuration;
+    private final Set<String> hashModuleCandidates;
+
+    /**
+     * Constructs a ModuleHashesBuilder that finds the packaged modules
+     * from the location of ModuleReference found from the given Configuration.
+     *
+     * @param config Configuration for building module hashes
+     * @param modules the candidate modules to be hashed
+     */
+    public ModuleHashesBuilder(Configuration config, Set<String> modules) {
+        this.configuration = config;
+        this.hashModuleCandidates = modules;
+    }
+
+    /**
+     * Returns a map of a module M to ModuleHashes for the modules
+     * that depend upon M directly or indirectly.
+     *
+     * The key for each entry in the returned map is a module M that has
+     * no outgoing edges to any of the candidate modules to be hashed
+     * i.e. M is a leaf node in a connected subgraph containing M and
+     * other candidate modules from the module graph filtering
+     * the outgoing edges from M to non-candidate modules.
+     */
+    public Map<String, ModuleHashes> computeHashes(Set<String> roots) {
+        // build a graph containing the the packaged modules and
+        // its transitive dependences matching --hash-modules
+        Graph.Builder<String> builder = new Graph.Builder<>();
+        Deque<ResolvedModule> deque = new ArrayDeque<>(configuration.modules());
+        Set<ResolvedModule> visited = new HashSet<>();
+        while (!deque.isEmpty()) {
+            ResolvedModule rm = deque.pop();
+            if (!visited.contains(rm)) {
+                visited.add(rm);
+                builder.addNode(rm.name());
+                for (ResolvedModule dm : rm.reads()) {
+                    if (!visited.contains(dm)) {
+                        deque.push(dm);
+                    }
+                    builder.addEdge(rm.name(), dm.name());
+                }
+            }
+        }
+
+        // each node in a transposed graph is a matching packaged module
+        // in which the hash of the modules that depend upon it is recorded
+        Graph<String> transposedGraph = builder.build().transpose();
+
+        // traverse the modules in topological order that will identify
+        // the modules to record the hashes - it is the first matching
+        // module and has not been hashed during the traversal.
+        Set<String> mods = new HashSet<>();
+        Map<String, ModuleHashes> hashes = new HashMap<>();
+        builder.build()
+               .orderedNodes()
+               .filter(mn -> roots.contains(mn) && !mods.contains(mn))
+               .forEach(mn -> {
+                   // Compute hashes of the modules that depend on mn directly and
+                   // indirectly excluding itself.
+                   Set<String> ns = transposedGraph.dfs(mn)
+                       .stream()
+                       .filter(n -> !n.equals(mn) && hashModuleCandidates.contains(n))
+                       .collect(toSet());
+                   mods.add(mn);
+                   mods.addAll(ns);
+
+                   if (!ns.isEmpty()) {
+                       Map<String, Path> moduleToPath = ns.stream()
+                           .collect(toMap(Function.identity(), this::moduleToPath));
+                       hashes.put(mn, ModuleHashes.generate(moduleToPath, "SHA-256"));
+                   }
+               });
+        return hashes;
+    }
+
+    private Path moduleToPath(String name) {
+        ResolvedModule rm = configuration.findModule(name).orElseThrow(
+            () -> new InternalError("Selected module " + name + " not on module path"));
+
+        URI uri = rm.reference().location().get();
+        Path path = Paths.get(uri);
+        String fn = path.getFileName().toString();
+        if (!fn.endsWith(".jar") && !fn.endsWith(".jmod")) {
+            throw new UnsupportedOperationException(path + " is not a modular JAR or jmod file");
+        }
+        return path;
+    }
+
+    /*
+     * Utilty class
+     */
+    static class Graph<T> {
+        private final Set<T> nodes;
+        private final Map<T, Set<T>> edges;
+
+        public Graph(Set<T> nodes, Map<T, Set<T>> edges) {
+            this.nodes = Collections.unmodifiableSet(nodes);
+            this.edges = Collections.unmodifiableMap(edges);
+        }
+
+        public Set<T> nodes() {
+            return nodes;
+        }
+
+        public Map<T, Set<T>> edges() {
+            return edges;
+        }
+
+        public Set<T> adjacentNodes(T u) {
+            return edges.get(u);
+        }
+
+        public boolean contains(T u) {
+            return nodes.contains(u);
+        }
+
+        /**
+         * Returns nodes sorted in topological order.
+         */
+        public Stream<T> orderedNodes() {
+            TopoSorter<T> sorter = new TopoSorter<>(this);
+            return sorter.result.stream();
+        }
+
+        /**
+         * Traverse this graph and performs the given action in topological order
+         */
+        public void ordered(Consumer<T> action) {
+            TopoSorter<T> sorter = new TopoSorter<>(this);
+            sorter.ordered(action);
+        }
+
+        /**
+         * Traverses this graph and performs the given action in reverse topological order
+         */
+        public void reverse(Consumer<T> action) {
+            TopoSorter<T> sorter = new TopoSorter<>(this);
+            sorter.reverse(action);
+        }
+
+        /**
+         * Returns a transposed graph from this graph
+         */
+        public Graph<T> transpose() {
+            Builder<T> builder = new Builder<>();
+            nodes.stream().forEach(builder::addNode);
+            // reverse edges
+            edges.keySet().forEach(u -> {
+                edges.get(u).stream()
+                    .forEach(v -> builder.addEdge(v, u));
+            });
+            return builder.build();
+        }
+
+        /**
+         * Returns all nodes reachable from the given root.
+         */
+        public Set<T> dfs(T root) {
+            return dfs(Set.of(root));
+        }
+
+        /**
+         * Returns all nodes reachable from the given set of roots.
+         */
+        public Set<T> dfs(Set<T> roots) {
+            Deque<T> deque = new LinkedList<>(roots);
+            Set<T> visited = new HashSet<>();
+            while (!deque.isEmpty()) {
+                T u = deque.pop();
+                if (!visited.contains(u)) {
+                    visited.add(u);
+                    if (contains(u)) {
+                        adjacentNodes(u).stream()
+                            .filter(v -> !visited.contains(v))
+                            .forEach(deque::push);
+                    }
+                }
+            }
+            return visited;
+        }
+
+        public void printGraph(PrintStream out) {
+            out.println("graph for " + nodes);
+            nodes.stream()
+                .forEach(u -> adjacentNodes(u).stream()
+                    .forEach(v -> out.format("  %s -> %s%n", u, v)));
+        }
+
+        static class Builder<T> {
+            final Set<T> nodes = new HashSet<>();
+            final Map<T, Set<T>> edges = new HashMap<>();
+
+            public void addNode(T node) {
+                if (nodes.contains(node)) {
+                    return;
+                }
+                nodes.add(node);
+                edges.computeIfAbsent(node, _e -> new HashSet<>());
+            }
+
+            public void addEdge(T u, T v) {
+                addNode(u);
+                addNode(v);
+                edges.get(u).add(v);
+            }
+
+            public Graph<T> build() {
+                return new Graph<T>(nodes, edges);
+            }
+        }
+    }
+
+    /**
+     * Topological sort
+     */
+    private static class TopoSorter<T> {
+        final Deque<T> result = new LinkedList<>();
+        final Deque<T> nodes;
+        final Graph<T> graph;
+
+        TopoSorter(Graph<T> graph) {
+            this.graph = graph;
+            this.nodes = new LinkedList<>(graph.nodes);
+            sort();
+        }
+
+        public void ordered(Consumer<T> action) {
+            result.iterator().forEachRemaining(action);
+        }
+
+        public void reverse(Consumer<T> action) {
+            result.descendingIterator().forEachRemaining(action);
+        }
+
+        private void sort() {
+            Deque<T> visited = new LinkedList<>();
+            Deque<T> done = new LinkedList<>();
+            T node;
+            while ((node = nodes.poll()) != null) {
+                if (!visited.contains(node)) {
+                    visit(node, visited, done);
+                }
+            }
+        }
+
+        private void visit(T node, Deque<T> visited, Deque<T> done) {
+            if (visited.contains(node)) {
+                if (!done.contains(node)) {
+                    throw new IllegalArgumentException("Cyclic detected: " +
+                        node + " " + graph.edges().get(node));
+                }
+                return;
+            }
+            visited.add(node);
+            graph.edges().get(node).stream()
+                .forEach(x -> visit(x, visited, done));
+            done.add(node);
+            result.addLast(node);
+        }
+    }
+}

jdk/src/jdk.jartool/share/classes/sun/tools/jar/GNUStyleOptions.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2017, 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
@@ -34,6 +34,8 @@ import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.regex.Pattern;
 import java.util.regex.PatternSyntaxException;
+
+import jdk.internal.module.ModulePath;
 import jdk.internal.module.ModuleResolution;
 
 /**
@@ -155,8 +157,8 @@ class GNUStyleOptions {
                     for (String dir : dirs) {
                         paths[i++] = Paths.get(dir);
                     }
-                    jartool.moduleFinder = ModuleFinder.compose(jartool.moduleFinder,
+                    jartool.moduleFinder =
-                                                                ModuleFinder.of(paths));
+                        new ModulePath(Runtime.version(), true, paths);
                 }
             },
             new Option(false, OptionType.CREATE_UPDATE, "--do-not-resolve-by-default") {

jdk/src/jdk.jartool/share/classes/sun/tools/jar/Main.java

@@ -47,7 +47,6 @@ import java.nio.file.Paths;
 import java.nio.file.StandardCopyOption;
 import java.util.*;
 import java.util.function.Consumer;
-import java.util.function.Function;
 import java.util.function.Supplier;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
@@ -60,6 +59,7 @@ import java.text.MessageFormat;
 
 import jdk.internal.module.Checks;
 import jdk.internal.module.ModuleHashes;
+import jdk.internal.module.ModuleHashesBuilder;
 import jdk.internal.module.ModuleInfo;
 import jdk.internal.module.ModuleInfoExtender;
 import jdk.internal.module.ModuleResolution;
@@ -68,7 +68,6 @@ import jdk.internal.util.jar.JarIndex;
 import static jdk.internal.util.jar.JarIndex.INDEX_NAME;
 import static java.util.jar.JarFile.MANIFEST_NAME;
 import static java.util.stream.Collectors.joining;
-import static java.util.stream.Collectors.toSet;
 import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;
 
 /**
@@ -1930,8 +1929,7 @@ public class Main {
             if (moduleHashes != null) {
                 extender.hashes(moduleHashes);
             } else {
-                // should it issue warning or silent?
+                warn("warning: no module is recorded in hash in " + mn);
-                System.out.println("warning: no module is recorded in hash in " + mn);
             }
         }
 
@@ -1947,10 +1945,9 @@ public class Main {
      * Compute and record hashes
      */
     private class Hasher {
+        final ModuleHashesBuilder hashesBuilder;
         final ModuleFinder finder;
-        final Map<String, Path> moduleNameToPath;
         final Set<String> modules;
-        final Configuration configuration;
         Hasher(ModuleDescriptor descriptor, String fname) throws IOException {
             // Create a module finder that finds the modular JAR
             // being created/updated
@@ -1980,119 +1977,46 @@ public class Main {
                     }
                 });
 
-            // Determine the modules that matches the modulesToHash pattern
+            // Determine the modules that matches the pattern {@code modulesToHash}
-            this.modules = moduleFinder.findAll().stream()
+            Set<String> roots = finder.findAll().stream()
-                .map(moduleReference -> moduleReference.descriptor().name())
+                .map(ref -> ref.descriptor().name())
                 .filter(mn -> modulesToHash.matcher(mn).find())
                 .collect(Collectors.toSet());
 
-            // a map from a module name to Path of the modular JAR
+            // use system module path unless it creates a modular JAR for
-            this.moduleNameToPath = moduleFinder.findAll().stream()
+            // a module that is present in the system image e.g. upgradeable
-                .map(ModuleReference::descriptor)
+            // module
-                .map(ModuleDescriptor::name)
+            ModuleFinder system;
-                .collect(Collectors.toMap(Function.identity(), mn -> moduleToPath(mn)));
+            String name = descriptor.name();
-
+            if (name != null && ModuleFinder.ofSystem().find(name).isPresent()) {
-            Configuration config = null;
+                system = ModuleFinder.of();
-            try {
+            } else {
-                config = Configuration.empty()
+                system = ModuleFinder.ofSystem();
-                    .resolveRequires(ModuleFinder.ofSystem(), finder, modules);
-            } catch (ResolutionException e) {
-                // should it throw an error?  or emit a warning
-                System.out.println("warning: " + e.getMessage());
             }
-            this.configuration = config;
+            // get a resolved module graph
+            Configuration config =
+                Configuration.empty().resolveRequires(system, finder, roots);
+
+            // filter modules resolved from the system module finder
+            this.modules = config.modules().stream()
+                .map(ResolvedModule::name)
+                .filter(mn -> roots.contains(mn) && !system.find(mn).isPresent())
+                .collect(Collectors.toSet());
+
+            this.hashesBuilder = new ModuleHashesBuilder(config, modules);
         }
 
         /**
-         * Compute hashes of the modules that depend upon the specified
+         * Compute hashes of the specified module.
+         *
+         * It records the hashing modules that depend upon the specified
          * module directly or indirectly.
          */
         ModuleHashes computeHashes(String name) {
-            // the transposed graph includes all modules in the resolved graph
+            if (hashesBuilder == null)
-            Map<String, Set<String>> graph = transpose();
-
-            // find the modules that transitively depend upon the specified name
-            Deque<String> deque = new ArrayDeque<>();
-            deque.add(name);
-            Set<String> mods = visitNodes(graph, deque);
-
-            // filter modules matching the pattern specified in --hash-modules,
-            // as well as the modular jar file that is being created / updated
-            Map<String, Path> modulesForHash = mods.stream()
-                .filter(mn -> !mn.equals(name) && modules.contains(mn))
-                .collect(Collectors.toMap(Function.identity(), moduleNameToPath::get));
-
-            if (modulesForHash.isEmpty())
                 return null;
 
-            return ModuleHashes.generate(modulesForHash, "SHA-256");
+            return hashesBuilder.computeHashes(Set.of(name)).get(name);
-        }
-
-        /**
-         * Returns all nodes traversed from the given roots.
-         */
-        private Set<String> visitNodes(Map<String, Set<String>> graph,
-                                       Deque<String> roots) {
-            Set<String> visited = new HashSet<>();
-            while (!roots.isEmpty()) {
-                String mn = roots.pop();
-                if (!visited.contains(mn)) {
-                    visited.add(mn);
-
-                    // the given roots may not be part of the graph
-                    if (graph.containsKey(mn)) {
-                        for (String dm : graph.get(mn)) {
-                            if (!visited.contains(dm))
-                                roots.push(dm);
-                        }
-                    }
-                }
-            }
-            return visited;
-        }
-
-        /**
-         * Returns a transposed graph from the resolved module graph.
-         */
-        private Map<String, Set<String>> transpose() {
-            Map<String, Set<String>> transposedGraph = new HashMap<>();
-            Deque<String> deque = new ArrayDeque<>(modules);
-
-            Set<String> visited = new HashSet<>();
-            while (!deque.isEmpty()) {
-                String mn = deque.pop();
-                if (!visited.contains(mn)) {
-                    visited.add(mn);
-
-                    // add an empty set
-                    transposedGraph.computeIfAbsent(mn, _k -> new HashSet<>());
-
-                    ResolvedModule resolvedModule = configuration.findModule(mn).get();
-                    for (ResolvedModule dm : resolvedModule.reads()) {
-                        String name = dm.name();
-                        if (!visited.contains(name)) {
-                            deque.push(name);
-                        }
-                        // reverse edge
-                        transposedGraph.computeIfAbsent(name, _k -> new HashSet<>())
-                                       .add(mn);
-                    }
-                }
-            }
-            return transposedGraph;
-        }
-
-        private Path moduleToPath(String name) {
-            ModuleReference mref = moduleFinder.find(name).orElseThrow(
-                () -> new InternalError(formatMsg2("error.hash.dep",name , name)));
-
-            URI uri = mref.location().get();
-            Path path = Paths.get(uri);
-            String fn = path.getFileName().toString();
-            if (!fn.endsWith(".jar")) {
-                throw new UnsupportedOperationException(path + " is not a modular JAR");
-            }
-            return path;
         }
     }
 }

jdk/src/jdk.jlink/share/classes/jdk/tools/jmod/JmodTask.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2017, 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
@@ -58,13 +58,10 @@ import java.nio.file.SimpleFileVisitor;
 import java.nio.file.StandardCopyOption;
 import java.nio.file.attribute.BasicFileAttributes;
 import java.text.MessageFormat;
-import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
-import java.util.Deque;
-import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -101,6 +98,7 @@ import jdk.internal.joptsimple.OptionSpec;
 import jdk.internal.joptsimple.ValueConverter;
 import jdk.internal.loader.ResourceHelper;
 import jdk.internal.module.ModuleHashes;
+import jdk.internal.module.ModuleHashesBuilder;
 import jdk.internal.module.ModuleInfo;
 import jdk.internal.module.ModuleInfoExtender;
 import jdk.internal.module.ModulePath;
@@ -286,7 +284,27 @@ public class JmodTask {
     }
 
     private boolean hashModules() {
-        return new Hasher(options.moduleFinder).run();
+        if (options.dryrun) {
+            out.println("Dry run:");
+        }
+
+        Hasher hasher = new Hasher(options.moduleFinder);
+        hasher.computeHashes().forEach((mn, hashes) -> {
+            if (options.dryrun) {
+                out.format("%s%n", mn);
+                hashes.names().stream()
+                    .sorted()
+                    .forEach(name -> out.format("  hashes %s %s %s%n",
+                        name, hashes.algorithm(), toHex(hashes.hashFor(name))));
+            } else {
+                try {
+                    hasher.updateModuleInfo(mn, hashes);
+                } catch (IOException ex) {
+                    throw new UncheckedIOException(ex);
+                }
+            }
+        });
+        return true;
     }
 
     private boolean describe() throws IOException {
@@ -377,7 +395,7 @@ public class JmodTask {
         // create jmod with temporary name to avoid it being examined
         // when scanning the module path
         Path target = options.jmodFile;
-        Path tempTarget = target.resolveSibling(target.getFileName() + ".tmp");
+        Path tempTarget = Files.createTempFile(target.getFileName().toString(), ".tmp");
         try {
             try (JmodOutputStream jos = JmodOutputStream.newOutputStream(tempTarget)) {
                 jmod.write(jos);
@@ -411,7 +429,6 @@ public class JmodTask {
         final String osArch = options.osArch;
         final String osVersion = options.osVersion;
         final List<PathMatcher> excludes = options.excludes;
-        final Hasher hasher = hasher();
         final ModuleResolution moduleResolution = options.moduleResolution;
 
         JmodFileWriter() { }
@@ -514,8 +531,17 @@ public class JmodTask {
                 if (moduleVersion != null)
                     extender.version(moduleVersion);
 
-                if (hasher != null) {
+                // --hash-modules
-                    ModuleHashes moduleHashes = hasher.computeHashes(descriptor.name());
+                if (options.modulesToHash != null) {
+                    // To compute hashes, it creates a Configuration to resolve
+                    // a module graph.  The post-resolution check requires
+                    // the packages in ModuleDescriptor be available for validation.
+                    ModuleDescriptor md;
+                    try (InputStream is = miSupplier.get()) {
+                        md = ModuleDescriptor.read(is, () -> packages);
+                    }
+
+                    ModuleHashes moduleHashes = computeHashes(md);
                     if (moduleHashes != null) {
                         extender.hashes(moduleHashes);
                     } else {
@@ -557,50 +583,34 @@ public class JmodTask {
          * The jmod file is being created and does not exist in the
          * given modulepath.
          */
-        private Hasher hasher() {
+        private ModuleHashes computeHashes(ModuleDescriptor descriptor) {
-            if (options.modulesToHash == null)
+            String mn = descriptor.name();
-                return null;
+            URI uri = options.jmodFile.toUri();
-
+            ModuleReference mref = new ModuleReference(descriptor, uri) {
-            try {
+                @Override
-                Supplier<InputStream> miSupplier = newModuleInfoSupplier();
+                public ModuleReader open() {
-                if (miSupplier == null) {
+                    throw new UnsupportedOperationException("opening " + mn);
-                    throw new IOException(MODULE_INFO + " not found");
                 }
+            };
 
-                ModuleDescriptor descriptor;
+            // compose a module finder with the module path and also
-                try (InputStream in = miSupplier.get()) {
+            // a module finder that can find the jmod file being created
-                    descriptor = ModuleDescriptor.read(in);
+            ModuleFinder finder = ModuleFinder.compose(options.moduleFinder,
-                }
+                new ModuleFinder() {
-
-                URI uri = options.jmodFile.toUri();
-                ModuleReference mref = new ModuleReference(descriptor, uri) {
                     @Override
-                    public ModuleReader open() {
+                    public Optional<ModuleReference> find(String name) {
-                        throw new UnsupportedOperationException();
+                        if (descriptor.name().equals(name))
+                            return Optional.of(mref);
+                        else return Optional.empty();
                     }
-                };
 
-                // compose a module finder with the module path and also
+                    @Override
-                // a module finder that can find the jmod file being created
+                    public Set<ModuleReference> findAll() {
-                ModuleFinder finder = ModuleFinder.compose(options.moduleFinder,
+                        return Collections.singleton(mref);
-                    new ModuleFinder() {
+                    }
-                        @Override
+                });
-                        public Optional<ModuleReference> find(String name) {
-                            if (descriptor.name().equals(name))
-                                return Optional.of(mref);
-                            else return Optional.empty();
-                        }
 
-                        @Override
+            return new Hasher(mn, finder).computeHashes().get(mn);
-                        public Set<ModuleReference> findAll() {
-                            return Collections.singleton(mref);
-                        }
-                    });
-
-                return new Hasher(finder);
-            } catch (IOException e) {
-                throw new UncheckedIOException(e);
-            }
         }
 
         /**
@@ -789,192 +799,93 @@ public class JmodTask {
      * Compute and record hashes
      */
     private class Hasher {
-        final ModuleFinder moduleFinder;
-        final Map<String, Path> moduleNameToPath;
-        final Set<String> modules;
         final Configuration configuration;
-        final boolean dryrun = options.dryrun;
+        final ModuleHashesBuilder hashesBuilder;
+        final Set<String> modules;
+        final String moduleName;  // a specific module to record hashes, if set
+
+        /**
+         * This constructor is for jmod hash command.
+         *
+         * This Hasher will determine which modules to record hashes, i.e.
+         * the module in a subgraph of modules to be hashed and that
+         * has no outgoing edges.  It will record in each of these modules,
+         * say `M`, with the the hashes of modules that depend upon M
+         * directly or indirectly matching the specified --hash-modules pattern.
+         */
         Hasher(ModuleFinder finder) {
-            this.moduleFinder = finder;
+            this(null, finder);
+        }
+
+        /**
+         * Constructs a Hasher to compute hashes.
+         *
+         * If a module name `M` is specified, it will compute the hashes of
+         * modules that depend upon M directly or indirectly matching the
+         * specified --hash-modules pattern and record in the ModuleHashes
+         * attribute in M's module-info.class.
+         *
+         * @param name    name of the module to record hashes
+         * @param finder  module finder for the specified --module-path
+         */
+        Hasher(String name, ModuleFinder finder) {
             // Determine the modules that matches the pattern {@code modulesToHash}
-            this.modules = moduleFinder.findAll().stream()
+            Set<String> roots = finder.findAll().stream()
                 .map(mref -> mref.descriptor().name())
                 .filter(mn -> options.modulesToHash.matcher(mn).find())
                 .collect(Collectors.toSet());
 
-            // a map from a module name to Path of the packaged module
+            // use system module path unless it creates a JMOD file for
-            this.moduleNameToPath = moduleFinder.findAll().stream()
+            // a module that is present in the system image e.g. upgradeable
-                .map(mref -> mref.descriptor().name())
+            // module
-                .collect(Collectors.toMap(Function.identity(), mn -> moduleToPath(mn)));
+            ModuleFinder system;
-
+            if (name != null && ModuleFinder.ofSystem().find(name).isPresent()) {
+                system = ModuleFinder.of();
+            } else {
+                system = ModuleFinder.ofSystem();
+            }
             // get a resolved module graph
             Configuration config = null;
             try {
-                config = Configuration.empty()
+                config = Configuration.empty().resolveRequires(system, finder, roots);
-                    .resolveRequires(ModuleFinder.ofSystem(), moduleFinder, modules);
             } catch (ResolutionException e) {
-                warning("warn.module.resolution.fail", e.getMessage());
+                throw new CommandException("err.module.resolution.fail", e.getMessage());
             }
+
+            this.moduleName = name;
             this.configuration = config;
+
+            // filter modules resolved from the system module finder
+            this.modules = config.modules().stream()
+                .map(ResolvedModule::name)
+                .filter(mn -> roots.contains(mn) && !system.find(mn).isPresent())
+                .collect(Collectors.toSet());
+
+            this.hashesBuilder = new ModuleHashesBuilder(config, modules);
         }
 
         /**
-         * This method is for jmod hash command.
+         * Returns a map of a module M to record hashes of the modules
+         * that depend upon M directly or indirectly.
          *
-         * Identify the base modules in the module graph, i.e. no outgoing edge
+         * For jmod hash command, the returned map contains one entry
-         * to any of the modules to be hashed.
+         * for each module M that has no outgoing edges to any of the
+         * modules matching the specified --hash-modules pattern.
          *
-         * For each base module M, compute the hashes of all modules that depend
+         * Each entry represents a leaf node in a connected subgraph containing
-         * upon M directly or indirectly.  Then update M's module-info.class
+         * M and other candidate modules from the module graph where M's outgoing
-         * to record the hashes.
+         * edges to any module other than the ones matching the specified
+         * --hash-modules pattern are excluded.
          */
-        boolean run() {
+        Map<String, ModuleHashes> computeHashes() {
-            if (configuration == null)
+            if (hashesBuilder == null)
-                return false;
-
-            // transposed graph containing the the packaged modules and
-            // its transitive dependences matching --hash-modules
-            Map<String, Set<String>> graph = new HashMap<>();
-            for (String root : modules) {
-                Deque<String> deque = new ArrayDeque<>();
-                deque.add(root);
-                Set<String> visited = new HashSet<>();
-                while (!deque.isEmpty()) {
-                    String mn = deque.pop();
-                    if (!visited.contains(mn)) {
-                        visited.add(mn);
-
-                        if (modules.contains(mn))
-                            graph.computeIfAbsent(mn, _k -> new HashSet<>());
-
-                        ResolvedModule resolvedModule = configuration.findModule(mn).get();
-                        for (ResolvedModule dm : resolvedModule.reads()) {
-                            String name = dm.name();
-                            if (!visited.contains(name)) {
-                                deque.push(name);
-                            }
-
-                            // reverse edge
-                            if (modules.contains(name) && modules.contains(mn)) {
-                                graph.computeIfAbsent(name, _k -> new HashSet<>()).add(mn);
-                            }
-                        }
-                    }
-                }
-            }
-
-            if (dryrun)
-                out.println("Dry run:");
-
-            // each node in a transposed graph is a matching packaged module
-            // in which the hash of the modules that depend upon it is recorded
-            graph.entrySet().stream()
-                .filter(e -> !e.getValue().isEmpty())
-                .forEach(e -> {
-                    String mn = e.getKey();
-                    Map<String, Path> modulesForHash = e.getValue().stream()
-                            .collect(Collectors.toMap(Function.identity(),
-                                                      moduleNameToPath::get));
-                    ModuleHashes hashes = ModuleHashes.generate(modulesForHash, "SHA-256");
-                    if (dryrun) {
-                        out.format("%s%n", mn);
-                        hashes.names().stream()
-                              .sorted()
-                              .forEach(name -> out.format("  hashes %s %s %s%n",
-                                  name, hashes.algorithm(), hashes.hashFor(name)));
-                    } else {
-                        try {
-                            updateModuleInfo(mn, hashes);
-                        } catch (IOException ex) {
-                            throw new UncheckedIOException(ex);
-                        }
-                    }
-                });
-            return true;
-        }
-
-        /**
-         * Compute hashes of the specified module.
-         *
-         * It records the hashing modules that depend upon the specified
-         * module directly or indirectly.
-         */
-        ModuleHashes computeHashes(String name) {
-            if (configuration == null)
                 return null;
 
-            // the transposed graph includes all modules in the resolved graph
+            if (moduleName != null) {
-            Map<String, Set<String>> graph = transpose();
+                return hashesBuilder.computeHashes(Set.of(moduleName));
-
+            } else {
-            // find the modules that transitively depend upon the specified name
+                return hashesBuilder.computeHashes(modules);
-            Deque<String> deque = new ArrayDeque<>();
-            deque.add(name);
-            Set<String> mods = visitNodes(graph, deque);
-
-            // filter modules matching the pattern specified --hash-modules
-            // as well as itself as the jmod file is being generated
-            Map<String, Path> modulesForHash = mods.stream()
-                .filter(mn -> !mn.equals(name) && modules.contains(mn))
-                .collect(Collectors.toMap(Function.identity(), moduleNameToPath::get));
-
-            if (modulesForHash.isEmpty())
-                return null;
-
-           return ModuleHashes.generate(modulesForHash, "SHA-256");
-        }
-
-        /**
-         * Returns all nodes traversed from the given roots.
-         */
-        private Set<String> visitNodes(Map<String, Set<String>> graph,
-                                       Deque<String> roots) {
-            Set<String> visited = new HashSet<>();
-            while (!roots.isEmpty()) {
-                String mn = roots.pop();
-                if (!visited.contains(mn)) {
-                    visited.add(mn);
-                    // the given roots may not be part of the graph
-                    if (graph.containsKey(mn)) {
-                        for (String dm : graph.get(mn)) {
-                            if (!visited.contains(dm)) {
-                                roots.push(dm);
-                            }
-                        }
-                    }
-                }
             }
-            return visited;
-        }
-
-        /**
-         * Returns a transposed graph from the resolved module graph.
-         */
-        private Map<String, Set<String>> transpose() {
-            Map<String, Set<String>> transposedGraph = new HashMap<>();
-            Deque<String> deque = new ArrayDeque<>(modules);
-
-            Set<String> visited = new HashSet<>();
-            while (!deque.isEmpty()) {
-                String mn = deque.pop();
-                if (!visited.contains(mn)) {
-                    visited.add(mn);
-
-                    transposedGraph.computeIfAbsent(mn, _k -> new HashSet<>());
-
-                    ResolvedModule resolvedModule = configuration.findModule(mn).get();
-                    for (ResolvedModule dm : resolvedModule.reads()) {
-                        String name = dm.name();
-                        if (!visited.contains(name)) {
-                            deque.push(name);
-                        }
-
-                        // reverse edge
-                        transposedGraph.computeIfAbsent(name, _k -> new HashSet<>())
-                                .add(mn);
-                    }
-                }
-            }
-            return transposedGraph;
         }
 
         /**
@@ -993,11 +904,11 @@ public class JmodTask {
             extender.write(out);
         }
 
-        private void updateModuleInfo(String name, ModuleHashes moduleHashes)
+        void updateModuleInfo(String name, ModuleHashes moduleHashes)
             throws IOException
         {
-            Path target = moduleNameToPath.get(name);
+            Path target = moduleToPath(name);
-            Path tempTarget = target.resolveSibling(target.getFileName() + ".tmp");
+            Path tempTarget = Files.createTempFile(target.getFileName().toString(), ".tmp");
             try {
                 if (target.getFileName().toString().endsWith(".jmod")) {
                     updateJmodFile(target, tempTarget, moduleHashes);
@@ -1075,10 +986,10 @@ public class JmodTask {
         }
 
         private Path moduleToPath(String name) {
-            ModuleReference mref = moduleFinder.find(name).orElseThrow(
+            ResolvedModule rm = configuration.findModule(name).orElseThrow(
                 () -> new InternalError("Selected module " + name + " not on module path"));
 
-            URI uri = mref.location().get();
+            URI uri = rm.reference().location().get();
             Path path = Paths.get(uri);
             String fn = path.getFileName().toString();
             if (!fn.endsWith(".jar") && !fn.endsWith(".jmod")) {

jdk/src/jdk.jlink/share/classes/jdk/tools/jmod/resources/jmod.properties

@@ -108,9 +108,9 @@ err.internal.error=internal error: {0} {1} {2}
 err.invalid.dryrun.option=--dry-run can only be used with hash mode
 err.module.descriptor.not.found=Module descriptor not found
 err.missing.export.or.open.packages=Packages that are exported or open in {0} are not present: {1}
+err.module.resolution.fail=Resolution failed: {0}
 warn.invalid.arg=Invalid classname or pathname not exist: {0}
 warn.no.module.hashes=No hashes recorded: no module specified for hashing depends on {0}
-warn.module.resolution.fail=No hashes recorded: {0}
 warn.ignore.entry=ignoring entry {0}, in section {1}
 warn.ignore.duplicate.entry=ignoring duplicate entry {0}, in section {1}
 

jdk/test/tools/jmod/JmodTest.java

@@ -579,37 +579,25 @@ public class JmodTest {
             });
     }
 
-    @Test
-    public void testTmpFileAlreadyExists() throws IOException {
-        // Implementation detail: jmod tool creates <jmod-file>.tmp
-        // Ensure that there are no problems if existing
-
-        Path jmod = MODS_DIR.resolve("testTmpFileAlreadyExists.jmod");
-        Path tmp = MODS_DIR.resolve("testTmpFileAlreadyExists.jmod.tmp");
-        FileUtils.deleteFileIfExistsWithRetry(jmod);
-        FileUtils.deleteFileIfExistsWithRetry(tmp);
-        Files.createFile(tmp);
-        String cp = EXPLODED_DIR.resolve("foo").resolve("classes").toString();
-
-        jmod("create",
-             "--class-path", cp,
-             jmod.toString())
-            .assertSuccess()
-            .resultChecker(r ->
-                assertTrue(Files.notExists(tmp), "Unexpected tmp file:" + tmp)
-            );
-    }
-
     @Test
     public void testTmpFileRemoved() throws IOException {
         // Implementation detail: jmod tool creates <jmod-file>.tmp
         // Ensure that it is removed in the event of a failure.
         // The failure in this case is a class in the unnamed package.
 
-        Path jmod = MODS_DIR.resolve("testTmpFileRemoved.jmod");
+        String filename = "testTmpFileRemoved.jmod";
-        Path tmp = MODS_DIR.resolve("testTmpFileRemoved.jmod.tmp");
+        Path jmod = MODS_DIR.resolve(filename);
+
+        // clean up files
         FileUtils.deleteFileIfExistsWithRetry(jmod);
-        FileUtils.deleteFileIfExistsWithRetry(tmp);
+        findTmpFiles(filename).forEach(tmp -> {
+            try {
+                FileUtils.deleteFileIfExistsWithRetry(tmp);
+            } catch (IOException e) {
+                throw new UncheckedIOException(e);
+            }
+        });
+
         String cp = EXPLODED_DIR.resolve("foo").resolve("classes") + File.pathSeparator +
                     EXPLODED_DIR.resolve("foo").resolve("classes")
                                 .resolve("jdk").resolve("test").resolve("foo").toString();
@@ -620,10 +608,22 @@ public class JmodTest {
              .assertFailure()
              .resultChecker(r -> {
                  assertContains(r.output, "unnamed package");
-                 assertTrue(Files.notExists(tmp), "Unexpected tmp file:" + tmp);
+                 Set<Path> tmpfiles = findTmpFiles(filename).collect(toSet());
+                 assertTrue(tmpfiles.isEmpty(), "Unexpected tmp file:" + tmpfiles);
              });
     }
 
+    private Stream<Path> findTmpFiles(String prefix) {
+        try {
+            Path tmpdir = Paths.get(System.getProperty("java.io.tmpdir"));
+            return Files.find(tmpdir, 1, (p, attrs) ->
+                p.getFileName().toString().startsWith(prefix)
+                    && p.getFileName().toString().endsWith(".tmp"));
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);
+        }
+    }
+
     // ---
 
     static boolean compileModule(String name, Path dest) throws IOException {

jdk/test/tools/jmod/hashes/HashesTest.java

@@ -1,5 +1,5 @@
 /**
- * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2017, 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
@@ -23,19 +23,22 @@
 
 /*
  * @test
+ * @bug 8160286
  * @summary Test the recording and checking of module hashes
- * @author Andrei Eremeev
  * @library /lib/testlibrary
  * @modules java.base/jdk.internal.misc
  *          java.base/jdk.internal.module
- *          jdk.jlink
  *          jdk.compiler
- * @build CompilerUtils
+ *          jdk.jartool
+ *          jdk.jlink
+ * @build CompilerUtils ModuleInfoMaker
  * @run testng HashesTest
  */
 
+import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
+import java.io.UncheckedIOException;
 import java.lang.module.ModuleDescriptor;
 import java.lang.module.ModuleFinder;
 import java.lang.module.ModuleReader;
@@ -53,109 +56,311 @@ import java.util.List;
 import java.util.Set;
 import java.util.spi.ToolProvider;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import jdk.internal.module.ModuleInfo;
 import jdk.internal.module.ModuleHashes;
 import jdk.internal.module.ModulePath;
 
-import org.testng.annotations.BeforeTest;
 import org.testng.annotations.Test;
 
 import static org.testng.Assert.*;
+import static java.lang.module.ModuleDescriptor.Requires.Modifier.*;
 
 public class HashesTest {
     static final ToolProvider JMOD_TOOL = ToolProvider.findFirst("jmod")
         .orElseThrow(() ->
             new RuntimeException("jmod tool not found")
         );
+    static final ToolProvider JAR_TOOL = ToolProvider.findFirst("jar")
+        .orElseThrow(() ->
+            new RuntimeException("jar tool not found")
+        );
 
-    private final Path testSrc = Paths.get(System.getProperty("test.src"));
+    private final Path mods;
-    private final Path modSrc = testSrc.resolve("src");
+    private final Path srcDir;
-    private final Path mods = Paths.get("mods");
+    private final Path lib;
-    private final Path jmods = Paths.get("jmods");
+    private final ModuleInfoMaker builder;
-    private final String[] modules = new String[] { "m1", "m2", "m3"};
+    HashesTest(Path dest) throws IOException {
-
+        if (Files.exists(dest)) {
-    @BeforeTest
+            deleteDirectory(dest);
-    private void setup() throws Exception {
-        if (Files.exists(jmods)) {
-            deleteDirectory(jmods);
         }
-        Files.createDirectories(jmods);
+        this.mods = dest.resolve("mods");
+        this.srcDir = dest.resolve("src");
+        this.lib = dest.resolve("lib");
+        this.builder = new ModuleInfoMaker(srcDir);
 
-        // build m2, m3 required by m1
+        Files.createDirectories(lib);
-        compileModule("m2", modSrc);
+        Files.createDirectories(mods);
-        jmod("m2");
-
-        compileModule("m3", modSrc);
-        jmod("m3");
-
-        // build m1
-        compileModule("m1", modSrc);
-        // no hash is recorded since m1 has outgoing edges
-        jmod("m1", "--module-path", jmods.toString(), "--hash-modules", ".*");
-
-        // compile org.bar and org.foo
-        compileModule("org.bar", modSrc);
-        compileModule("org.foo", modSrc);
     }
 
     @Test
-    public void test() throws Exception {
+    public static void test() throws IOException {
-        for (String mn : modules) {
+        Path dest = Paths.get("test");
-            assertTrue(hashes(mn) == null);
+        HashesTest ht = new HashesTest(dest);
-        }
+
+        // create modules for test cases
+        ht.makeModule("m2");
+        ht.makeModule("m3");
+        ht.makeModule("m1", "m2", "m3");
+
+        ht.makeModule("org.bar", TRANSITIVE, "m1");
+        ht.makeModule("org.foo", TRANSITIVE, "org.bar");
+
+        // create JMOD for m1, m2, m3
+        ht.makeJmod("m2");
+        ht.makeJmod("m3");
+
+        // no hash is recorded since m1 has outgoing edges
+        ht.jmodHashModules("m1", ".*");
+
+        // no hash is recorded in m1, m2, m3
+        assertTrue(ht.hashes("m1") == null);
+        assertTrue(ht.hashes("m2") == null);
+        assertTrue(ht.hashes("m3") == null);
 
         // hash m1 in m2
-        jmod("m2", "--module-path", jmods.toString(), "--hash-modules", "m1");
+        ht.jmodHashModules("m2",  "m1");
-        checkHashes(hashes("m2"), "m1");
+        ht.checkHashes("m2", "m1");
 
         // hash m1 in m2
-        jmod("m2", "--module-path", jmods.toString(), "--hash-modules", ".*");
+        ht.jmodHashModules("m2",  ".*");
-        checkHashes(hashes("m2"), "m1");
+        ht.checkHashes("m2", "m1");
 
         // create m2.jmod with no hash
-        jmod("m2");
+        ht.makeJmod("m2");
         // run jmod hash command to hash m1 in m2 and m3
-        runJmod(Arrays.asList("hash", "--module-path", jmods.toString(),
+        runJmod(List.of("hash", "--module-path", ht.lib.toString(),
-                "--hash-modules", ".*"));
+                        "--hash-modules", ".*"));
-        checkHashes(hashes("m2"), "m1");
+        ht.checkHashes("m2", "m1");
-        checkHashes(hashes("m3"), "m1");
+        ht.checkHashes("m3", "m1");
 
-        jmod("org.bar");
+        // check transitive requires
-        jmod("org.foo");
+        ht.makeJmod("org.bar");
+        ht.makeJmod("org.foo");
 
-        jmod("org.bar", "--module-path", jmods.toString(), "--hash-modules", "org.*");
+        ht.jmodHashModules("org.bar", "org.*");
-        checkHashes(hashes("org.bar"), "org.foo");
+        ht.checkHashes("org.bar", "org.foo");
 
-        jmod("m3", "--module-path", jmods.toString(), "--hash-modules", ".*");
+        ht.jmodHashModules( "m3", ".*");
-        checkHashes(hashes("m3"), "org.foo", "org.bar", "m1");
+        ht.checkHashes("m3", "org.foo", "org.bar", "m1");
     }
 
-    private void checkHashes(ModuleHashes hashes, String... hashModules) {
+    @Test
+    public static void multiBaseModules() throws IOException {
+        Path dest = Paths.get("test2");
+        HashesTest ht = new HashesTest(dest);
+
+        /*
+         * y2 -----------> y1
+         *    |______
+         *    |      |
+         *    V      V
+         *    z3 -> z2
+         *    |      |
+         *    |      V
+         *    |---> z1
+         */
+
+        ht.makeModule("z1");
+        ht.makeModule("z2", "z1");
+        ht.makeModule("z3", "z1", "z2");
+
+        ht.makeModule("y1");
+        ht.makeModule("y2", "y1", "z2", "z3");
+
+        Set<String> ys = Set.of("y1", "y2");
+        Set<String> zs = Set.of("z1", "z2", "z3");
+
+        // create JMOD files
+        Stream.concat(ys.stream(), zs.stream()).forEach(ht::makeJmod);
+
+        // run jmod hash command
+        runJmod(List.of("hash", "--module-path", ht.lib.toString(),
+                        "--hash-modules", ".*"));
+
+        /*
+         * z1 and y1 are the modules with hashes recorded.
+         */
+        ht.checkHashes("y1", "y2");
+        ht.checkHashes("z1", "z2", "z3", "y2");
+        Stream.concat(ys.stream(), zs.stream())
+              .filter(mn -> !mn.equals("y1") && !mn.equals("z1"))
+              .forEach(mn -> assertTrue(ht.hashes(mn) == null));
+    }
+
+    @Test
+    public static void mixJmodAndJarFile() throws IOException {
+        Path dest = Paths.get("test3");
+        HashesTest ht = new HashesTest(dest);
+
+        /*
+         * j3 -----------> j2
+         *    |______
+         *    |      |
+         *    V      V
+         *    m3 -> m2
+         *    |      |
+         *    |      V
+         *    |---> m1 -> j1 -> jdk.jlink
+         */
+
+        ht.makeModule("j1");
+        ht.makeModule("j2");
+        ht.makeModule("m1", "j1");
+        ht.makeModule("m2", "m1");
+        ht.makeModule("m3", "m1", "m2");
+
+        ht.makeModule("j3", "j2", "m2", "m3");
+
+        Set<String> jars = Set.of("j1", "j2", "j3");
+        Set<String> jmods = Set.of("m1", "m2", "m3");
+
+        // create JMOD and JAR files
+        jars.forEach(ht::makeJar);
+        jmods.forEach(ht::makeJmod);
+
+        // run jmod hash command
+        runJmod(List.of("hash", "--module-path", ht.lib.toString(),
+                        "--hash-modules", "^j.*|^m.*"));
+
+        /*
+         * j1 and j2 are the modules with hashes recorded.
+         */
+        ht.checkHashes("j2", "j3");
+        ht.checkHashes("j1", "m1", "m2", "m3", "j3");
+        Stream.concat(jars.stream(), jmods.stream())
+              .filter(mn -> !mn.equals("j1") && !mn.equals("j2"))
+              .forEach(mn -> assertTrue(ht.hashes(mn) == null));
+    }
+
+    @Test
+    public static void upgradeableModule() throws IOException {
+        Path mpath = Paths.get(System.getProperty("java.home"), "jmods");
+        if (!Files.exists(mpath)) {
+            return;
+        }
+
+        Path dest = Paths.get("test4");
+        HashesTest ht = new HashesTest(dest);
+        ht.makeModule("m1");
+        ht.makeModule("java.xml.bind", "m1");
+        ht.makeModule("java.xml.ws", "java.xml.bind");
+        ht.makeModule("m2", "java.xml.ws");
+
+        ht.makeJmod("m1");
+        ht.makeJmod("m2");
+        ht.makeJmod("java.xml.ws");
+        ht.makeJmod("java.xml.bind",
+                    "--module-path",
+                    ht.lib.toString() + File.pathSeparator + mpath,
+                    "--hash-modules", "^java.xml.*|^m.*");
+
+        ht.checkHashes("java.xml.bind", "java.xml.ws", "m2");
+    }
+
+    @Test
+    public static void testImageJmods() throws IOException {
+        Path mpath = Paths.get(System.getProperty("java.home"), "jmods");
+        if (!Files.exists(mpath)) {
+            return;
+        }
+
+        Path dest = Paths.get("test5");
+        HashesTest ht = new HashesTest(dest);
+        ht.makeModule("m1", "jdk.compiler", "jdk.attach");
+        ht.makeModule("m2", "m1");
+        ht.makeModule("m3", "java.compiler");
+
+        ht.makeJmod("m1");
+        ht.makeJmod("m2");
+
+        runJmod(List.of("hash",
+                        "--module-path",
+                        mpath.toString() + File.pathSeparator + ht.lib.toString(),
+                        "--hash-modules", ".*"));
+
+        validateImageJmodsTest(ht, mpath);
+    }
+
+    @Test
+    public static void testImageJmods1() throws IOException {
+        Path mpath = Paths.get(System.getProperty("java.home"), "jmods");
+        if (!Files.exists(mpath)) {
+            return;
+        }
+
+        Path dest = Paths.get("test6");
+        HashesTest ht = new HashesTest(dest);
+        ht.makeModule("m1", "jdk.compiler", "jdk.attach");
+        ht.makeModule("m2", "m1");
+        ht.makeModule("m3", "java.compiler");
+
+        ht.makeJar("m2");
+        ht.makeJar("m1",
+                    "--module-path",
+                    mpath.toString() + File.pathSeparator + ht.lib.toString(),
+                    "--hash-modules", ".*");
+        validateImageJmodsTest(ht, mpath);
+    }
+
+    private static void validateImageJmodsTest(HashesTest ht, Path mpath)
+        throws IOException
+    {
+        // hash is recorded in m1 and not any other packaged modules on module path
+        ht.checkHashes("m1", "m2");
+        assertTrue(ht.hashes("m2") == null);
+
+        // should not override any JDK packaged modules
+        ModuleFinder finder = new ModulePath(Runtime.version(),
+                                             true,
+                                             mpath);
+        assertTrue(ht.hashes(finder,"jdk.compiler") == null);
+        assertTrue(ht.hashes(finder,"jdk.attach") == null);
+    }
+
+    private void checkHashes(String mn, String... hashModules) throws IOException {
+        ModuleHashes hashes = hashes(mn);
         assertTrue(hashes.names().equals(Set.of(hashModules)));
     }
 
-    private ModuleHashes hashes(String name) throws Exception {
+    private ModuleHashes hashes(String name) {
         ModuleFinder finder = new ModulePath(Runtime.version(),
                                              true,
-                                             jmods.resolve(name + ".jmod"));
+                                             lib);
+        return hashes(finder, name);
+    }
+
+    private ModuleHashes hashes(ModuleFinder finder, String name) {
         ModuleReference mref = finder.find(name).orElseThrow(RuntimeException::new);
-        ModuleReader reader = mref.open();
+        try {
-        try (InputStream in = reader.open("module-info.class").get()) {
+            ModuleReader reader = mref.open();
-            ModuleHashes hashes = ModuleInfo.read(in, null).recordedHashes();
+            try (InputStream in = reader.open("module-info.class").get()) {
-            System.out.format("hashes in module %s %s%n", name,
+                ModuleHashes hashes = ModuleInfo.read(in, null).recordedHashes();
+                System.out.format("hashes in module %s %s%n", name,
                     (hashes != null) ? "present" : "absent");
-            if (hashes != null) {
+                if (hashes != null) {
-                hashes.names().stream()
+                    hashes.names().stream().sorted().forEach(n ->
-                    .sorted()
+                        System.out.format("  %s %s%n", n, toHex(hashes.hashFor(n)))
-                    .forEach(n -> System.out.format("  %s %s%n", n, hashes.hashFor(n)));
+                    );
+                }
+                return hashes;
+            } finally {
+                reader.close();
             }
-            return hashes;
+        } catch (IOException e) {
-        } finally {
+            throw new UncheckedIOException(e);
-            reader.close();
         }
     }
 
+    private String toHex(byte[] ba) {
+        StringBuilder sb = new StringBuilder(ba.length);
+        for (byte b: ba) {
+            sb.append(String.format("%02x", b & 0xff));
+        }
+        return sb.toString();
+    }
+
     private void deleteDirectory(Path dir) throws IOException {
         Files.walkFileTree(dir, new SimpleFileVisitor<Path>() {
             @Override
@@ -176,31 +381,94 @@ public class HashesTest {
         });
     }
 
+
+    private void makeModule(String mn, String... deps) throws IOException {
+        makeModule(mn, null, deps);
+    }
+
+    private void makeModule(String mn, ModuleDescriptor.Requires.Modifier mod,  String... deps)
+        throws IOException
+    {
+        if (mod != null && mod != TRANSITIVE && mod != STATIC) {
+            throw new IllegalArgumentException(mod.toString());
+        }
+
+        StringBuilder sb = new StringBuilder();
+        sb.append("module " + mn + " {").append("\n");
+        Arrays.stream(deps).forEach(req -> {
+            sb.append("    requires ");
+            if (mod != null) {
+                sb.append(mod.toString().toLowerCase()).append(" ");
+            }
+            sb.append(req + ";\n");
+        });
+        sb.append("}\n");
+        builder.writeJavaFiles(mn, sb.toString());
+
+        compileModule(mn, srcDir);
+    }
+
     private void compileModule(String moduleName, Path src) throws IOException {
         Path msrc = src.resolve(moduleName);
         assertTrue(CompilerUtils.compile(msrc, mods, "--module-source-path", src.toString()));
     }
 
-    private void jmod(String moduleName, String... options) throws IOException {
+    private void jmodHashModules(String moduleName, String hashModulesPattern) {
+        makeJmod(moduleName, "--module-path", lib.toString(),
+                 "--hash-modules", hashModulesPattern);
+    }
+
+    private void makeJmod(String moduleName, String... options) {
         Path mclasses = mods.resolve(moduleName);
-        Path outfile = jmods.resolve(moduleName + ".jmod");
+        Path outfile = lib.resolve(moduleName + ".jmod");
         List<String> args = new ArrayList<>();
         args.add("create");
         Collections.addAll(args, options);
         Collections.addAll(args, "--class-path", mclasses.toString(),
                            outfile.toString());
 
-        if (Files.exists(outfile))
+        if (Files.exists(outfile)) {
-            Files.delete(outfile);
+            try {
-
+                Files.delete(outfile);
+            } catch (IOException e) {
+                throw new UncheckedIOException(e);
+            }
+        }
         runJmod(args);
     }
 
-    private void runJmod(List<String> args) {
+    private static void runJmod(List<String> args) {
         int rc = JMOD_TOOL.run(System.out, System.out, args.toArray(new String[args.size()]));
-        System.out.println("jmod options: " + args.stream().collect(Collectors.joining(" ")));
+        System.out.println("jmod " + args.stream().collect(Collectors.joining(" ")));
         if (rc != 0) {
-            throw new AssertionError("Jmod failed: rc = " + rc);
+            throw new AssertionError("jmod failed: rc = " + rc);
+        }
+    }
+
+    private void makeJar(String moduleName, String... options) {
+        Path mclasses = mods.resolve(moduleName);
+        Path outfile = lib.resolve(moduleName + ".jar");
+        List<String> args = new ArrayList<>();
+        Stream.concat(Stream.of("--create",
+                                "--file=" + outfile.toString()),
+                      Arrays.stream(options))
+              .forEach(args::add);
+        args.add("-C");
+        args.add(mclasses.toString());
+        args.add(".");
+
+        if (Files.exists(outfile)) {
+            try {
+                Files.delete(outfile);
+            } catch (IOException e) {
+                throw new UncheckedIOException(e);
+            }
+        }
+
+        int rc = JAR_TOOL.run(System.out, System.out, args.toArray(new String[args.size()]));
+        System.out.println("jar " + args.stream().collect(Collectors.joining(" ")));
+        if (rc != 0) {
+            throw new AssertionError("jar failed: rc = " + rc);
         }
     }
 }

jdk/test/tools/jmod/hashes/src/m1/module-info.java

@@ -1,27 +0,0 @@
-/*
- * Copyright (c) 2015, 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.
- */
-
-module m1 {
-    requires m2;
-    requires m3;
-}

jdk/test/tools/jmod/hashes/src/m1/org/m1/Main.java

@@ -1,34 +0,0 @@
-/*
- * Copyright (c) 2015, 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.
- */
-
-package org.m1;
-
-import org.m2.Util;
-import org.m3.Name;
-
-public class Main {
-    public static void main(String[] args) {
-        System.out.println(Util.timeOfDay());
-        System.out.println(Name.name());
-    }
-}

jdk/test/tools/jmod/hashes/src/m2/module-info.java

@@ -1,26 +0,0 @@
-/*
- * Copyright (c) 2015, 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.
- */
-
-module m2 {
-    exports org.m2;
-}

jdk/test/tools/jmod/hashes/src/m2/org/m2/Util.java

@@ -1,32 +0,0 @@
-/*
- * Copyright (c) 2015, 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.
- */
-
-package org.m2;
-
-public class Util {
-    private Util() { }
-
-    public static String timeOfDay() {
-        return "Time for lunch";
-    }
-}

jdk/test/tools/jmod/hashes/src/m3/module-info.java

@@ -1,26 +0,0 @@
-/*
- * Copyright (c) 2015, 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.
- */
-
-module m3 {
-    exports org.m3;
-}

jdk/test/tools/jmod/hashes/src/m3/org/m3/Name.java

@@ -1,32 +0,0 @@
-/*
- * Copyright (c) 2015, 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.
- */
-
-package org.m3;
-
-public class Name {
-    private Name() { }
-
-    public static String name() {
-        return "m3";
-    }
-}

jdk/test/tools/jmod/hashes/src/org.bar/module-info.java

@@ -1,26 +0,0 @@
-/*
- * 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.
- */
-
-module org.bar {
-    requires transitive m1;
-}

jdk/test/tools/jmod/hashes/src/org.foo/module-info.java

@@ -1,26 +0,0 @@
-/*
- * 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.
- */
-
-module org.foo {
-    requires transitive org.bar;
-}