8214230: Classes generated by SystemModulesPlugin.java are not reproducable
authorehelin
Tue, 04 Dec 2018 09:30:10 +0100
changeset 52814 abccada595dd
parent 52813 767678b5e61b
child 52815 10bb941d7fd4
child 57057 9525129eaa3b
8214230: Classes generated by SystemModulesPlugin.java are not reproducable Reviewed-by: alanb, redestad, mchung
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java
test/jdk/tools/jlink/JLinkReproducibleTest.java
--- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java	Tue Dec 04 09:28:11 2018 +0100
+++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java	Tue Dec 04 09:30:10 2018 +0100
@@ -52,6 +52,7 @@
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
+import java.util.TreeMap;
 import java.util.TreeSet;
 import java.util.function.IntSupplier;
 import java.util.function.Supplier;
@@ -925,7 +926,7 @@
             mv.visitTypeInsn(ANEWARRAY, "java/util/Map$Entry");
 
             int index = 0;
-            for (Map.Entry<String, Set<String>> e : map.entrySet()) {
+            for (var e : new TreeMap<>(map).entrySet()) {
                 String name = e.getKey();
                 Set<String> s = e.getValue();
 
@@ -971,7 +972,7 @@
                 pushInt(mv, size);
                 mv.visitTypeInsn(ANEWARRAY, "java/lang/String");
                 int i = 0;
-                for (String element : set) {
+                for (String element : sorted(set)) {
                     mv.visitInsn(DUP);
                     pushInt(mv, i);
                     mv.visitLdcInsn(element);
@@ -985,7 +986,7 @@
                         true);
             } else {
                 StringBuilder sb = new StringBuilder("(");
-                for (String element : set) {
+                for (String element : sorted(set)) {
                     mv.visitLdcInsn(element);
                     sb.append("Ljava/lang/Object;");
                 }
@@ -1146,7 +1147,7 @@
                 pushInt(mv, requires.size());
                 mv.visitTypeInsn(ANEWARRAY, "java/lang/module/ModuleDescriptor$Requires");
                 int arrayIndex = 0;
-                for (Requires require : requires) {
+                for (Requires require : sorted(requires)) {
                     String compiledVersion = null;
                     if (require.compiledVersion().isPresent()) {
                         compiledVersion = require.compiledVersion().get().toString();
@@ -1192,7 +1193,7 @@
                 pushInt(mv, exports.size());
                 mv.visitTypeInsn(ANEWARRAY, "java/lang/module/ModuleDescriptor$Exports");
                 int arrayIndex = 0;
-                for (Exports export : exports) {
+                for (Exports export : sorted(exports)) {
                     mv.visitInsn(DUP);    // arrayref
                     pushInt(mv, arrayIndex++);
                     newExports(export.modifiers(), export.source(), export.targets());
@@ -1245,7 +1246,7 @@
                 pushInt(mv, opens.size());
                 mv.visitTypeInsn(ANEWARRAY, "java/lang/module/ModuleDescriptor$Opens");
                 int arrayIndex = 0;
-                for (Opens open : opens) {
+                for (Opens open : sorted(opens)) {
                     mv.visitInsn(DUP);    // arrayref
                     pushInt(mv, arrayIndex++);
                     newOpens(open.modifiers(), open.source(), open.targets());
@@ -1310,7 +1311,7 @@
                 pushInt(mv, provides.size());
                 mv.visitTypeInsn(ANEWARRAY, "java/lang/module/ModuleDescriptor$Provides");
                 int arrayIndex = 0;
-                for (Provides provide : provides) {
+                for (Provides provide : sorted(provides)) {
                     mv.visitInsn(DUP);    // arrayref
                     pushInt(mv, arrayIndex++);
                     newProvides(provide.service(), provide.providers());
@@ -1420,6 +1421,8 @@
                 // Invoke ModuleHashes.Builder::hashForModule
                 recordedHashes
                     .names()
+                    .stream()
+                    .sorted()
                     .forEach(mn -> hashForModule(mn, recordedHashes.hashFor(mn)));
 
                 // Put ModuleHashes into the hashes array
@@ -1600,7 +1603,7 @@
          * it will reuse defaultVarIndex.  For a Set with multiple references,
          * it will use a new local variable retrieved from the nextLocalVar
          */
-        class SetBuilder<T> {
+        class SetBuilder<T extends Comparable<T>> {
             private final Set<T> elements;
             private final int defaultVarIndex;
             private final IntSupplier nextLocalVar;
@@ -1660,7 +1663,7 @@
                 if (elements.size() <= 10) {
                     // call Set.of(e1, e2, ...)
                     StringBuilder sb = new StringBuilder("(");
-                    for (T t : elements) {
+                    for (T t : sorted(elements)) {
                         sb.append("Ljava/lang/Object;");
                         visitElement(t, mv);
                     }
@@ -1672,7 +1675,7 @@
                     pushInt(mv, elements.size());
                     mv.visitTypeInsn(ANEWARRAY, "java/lang/String");
                     int arrayIndex = 0;
-                    for (T t : elements) {
+                    for (T t : sorted(elements)) {
                         mv.visitInsn(DUP);    // arrayref
                         pushInt(mv, arrayIndex);
                         visitElement(t, mv);  // value
@@ -1690,7 +1693,7 @@
          * Generates bytecode to create one single instance of EnumSet
          * for a given set of modifiers and assign to a local variable slot.
          */
-        class EnumSetBuilder<T> extends SetBuilder<T> {
+        class EnumSetBuilder<T extends Comparable<T>> extends SetBuilder<T> {
 
             private final String className;
 
@@ -1788,7 +1791,7 @@
         mv.visitTypeInsn(ANEWARRAY, "java/lang/String");
 
         int index = 0;
-        for (String moduleName : map.keySet()) {
+        for (String moduleName : sorted(map.keySet())) {
             mv.visitInsn(DUP);                  // arrayref
             pushInt(mv, index);
             mv.visitLdcInsn(moduleName);
@@ -1811,7 +1814,7 @@
         mv.visitTypeInsn(ANEWARRAY, "java/lang/String");
 
         index = 0;
-        for (String className : map.values()) {
+        for (String className : sorted(map.values())) {
             mv.visitInsn(DUP);                  // arrayref
             pushInt(mv, index);
             mv.visitLdcInsn(className.replace('/', '.'));
@@ -1832,6 +1835,19 @@
     }
 
     /**
+     * Returns a sorted copy of a collection.
+     *
+     * This is useful to ensure a deterministic iteration order.
+     *
+     * @return a sorted copy of the given collection.
+     */
+    private static <T extends Comparable<T>> List<T> sorted(Collection<T> c) {
+        var l = new ArrayList<T>(c);
+        Collections.sort(l);
+        return l;
+    }
+
+    /**
      * Pushes an int constant
      */
     private static void pushInt(MethodVisitor mv, int value) {
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/tools/jlink/JLinkReproducibleTest.java	Tue Dec 04 09:30:10 2018 +0100
@@ -0,0 +1,114 @@
+/*
+ * Copyright (c) 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import java.io.File;
+import java.nio.file.*;
+import java.util.*;
+
+import static jdk.test.lib.Asserts.*;
+import jdk.test.lib.JDKToolFinder;
+import jdk.test.lib.process.ProcessTools;
+
+/*
+ * @test
+ * @bug 8214230
+ * @summary Test that jlinks generates reproducible modules files
+ * @library /test/lib
+ * @run driver JLinkReproducibleTest
+ */
+public class JLinkReproducibleTest {
+    private static void run(List<String> cmd) throws Exception {
+        var pb = new ProcessBuilder(cmd.toArray(new String[0]));
+        var res = ProcessTools.executeProcess(pb);
+        res.shouldHaveExitValue(0);
+    }
+
+    private static void jlink(Path image) throws Exception {
+        var cmd = new ArrayList<String>();
+        cmd.add(JDKToolFinder.getJDKTool("jlink"));
+        cmd.addAll(List.of(
+            "--module-path", JMODS_DIR.toString() + File.pathSeparator + CLASS_DIR.toString(),
+            "--add-modules", "main",
+            "--compress=2",
+            "--output", image.toString()
+        ));
+        run(cmd);
+    }
+
+    private static void javac(String... args) throws Exception {
+        var cmd = new ArrayList<String>();
+        cmd.add(JDKToolFinder.getJDKTool("javac"));
+        cmd.addAll(Arrays.asList(args));
+        run(cmd);
+    }
+
+    private static final List<String> MODULE_INFO = List.of(
+        "module main {",
+        "    exports org.test.main;",
+        "}"
+    );
+
+    private static final List<String> MAIN_CLASS = List.of(
+        "package org.test.main;",
+        "public class Main {",
+        "    public static void main(String[] args) {",
+        "        System.out.println(\"Hello, world\");",
+        "    }",
+        "}"
+    );
+
+    private static final Path CLASS_DIR = Path.of("classes");
+    private static final Path JMODS_DIR = Path.of(System.getProperty("java.home"), "jmods");
+
+    public static void main(String[] args) throws Exception {
+        // Write the source code
+        var srcDir = Path.of("main", "org", "test", "main");
+        Files.createDirectories(srcDir.toAbsolutePath());
+
+        var srcFile = srcDir.resolve("Main.java");
+        Files.write(srcFile, MAIN_CLASS);
+
+        var moduleFile = Path.of("main").resolve("module-info.java");
+        Files.write(moduleFile, MODULE_INFO);
+
+        // Compile the source code to class files
+        javac("--module-source-path", ".",
+              "--module", "main",
+              "-d", CLASS_DIR.toString());
+
+        // Link the first image
+        var firstImage = Path.of("image-first");
+        jlink(firstImage);
+        var firstModulesFile = firstImage.resolve("lib")
+                                         .resolve("modules");
+
+        // Link the second image
+        var secondImage = Path.of("image-second");
+        jlink(secondImage);
+        var secondModulesFile = secondImage.resolve("lib")
+                                           .resolve("modules");
+
+        // Ensure module files are identical
+        assertEquals(-1L, Files.mismatch(firstModulesFile, secondModulesFile));
+    }
+}