Merge pull request #318 from gradle/gh/jdkImage/ordering

Fix issue with ordering on serialized module descriptors

Author: ghale

src/main/groovy/org/gradle/android/workarounds/JdkImageWorkaround.groovy

@@ -1,5 +1,7 @@
 package org.gradle.android.workarounds
 
+import com.google.common.annotations.VisibleForTesting
+import com.google.common.collect.Lists
 import org.gradle.android.AndroidIssue
 import org.gradle.api.Project
 import org.gradle.api.artifacts.transform.CacheableTransform
@@ -24,6 +26,8 @@ import javax.inject.Inject
 import java.lang.module.ModuleDescriptor
 import java.nio.ByteBuffer
 import java.nio.file.Files
+import java.util.stream.Collectors
+import java.util.stream.Stream
 
 /**
  * Works around cache misses due to the custom Java runtime used when source compatibility is set higher
@@ -40,13 +44,18 @@ class JdkImageWorkaround implements Workaround {
 
     @Override
     void apply(Project project) {
+        // We would prefer not to configure this if a jdkImage is not in use, but the attributes
+        // being ignored are unlikely to ever have a runtime impact.  Doing this outside of task
+        // configuration prevents issues with things that use the tooling api to finalize the
+        // runtime configuration before querying (and instantiating) task configurations.
+        applyRuntimeClasspathNormalization(project)
+
         applyToAllAndroidVariants(project) { variant ->
             variant.javaCompileProvider.configure { JavaCompile task ->
                 def jdkImageInput = getJdkImageInput(task)
                 if (jdkImageInput != null) {
                     setupExtractedJdkImageInputTransform(project, getJvmHome(task))
                     replaceCommandLineProvider(task, jdkImageInput)
-                    applyRuntimeClasspathNormalization(task.project)
                 }
             }
         }
@@ -178,28 +187,76 @@ class JdkImageWorkaround implements Workaround {
 
             // Capture the module descriptor ignoring the version, which is not enforced anyways
             File moduleInfoFile = new File(targetDir, 'java.base/module-info.class')
-            ModuleDescriptor strippedDescriptor = captureModuleDescriptorWithoutVersion(moduleInfoFile)
+            ModuleDescriptor descriptor = captureModuleDescriptorWithoutVersion(moduleInfoFile)
             File descriptorData = new File(targetDir, "module-descriptor.txt")
-            descriptorData.text = strippedDescriptor.toString()
+            descriptorData.text = serializeDescriptor(descriptor)
 
             fileOperations.delete {
                 delete(moduleInfoFile)
             }
         }
 
         private static ModuleDescriptor captureModuleDescriptorWithoutVersion(File moduleFile) {
-            ModuleDescriptor descriptor = ModuleDescriptor.read(ByteBuffer.wrap(Files.readAllBytes(moduleFile.toPath())))
-            ModuleDescriptor.Builder strippedDescriptor = ModuleDescriptor.newModule(descriptor.name())
-            strippedDescriptor.packages(descriptor.packages())
-            if (descriptor.mainClass().present) {
-                strippedDescriptor.mainClass(descriptor.mainClass().get())
+            return ModuleDescriptor.read(ByteBuffer.wrap(Files.readAllBytes(moduleFile.toPath())))
+        }
+
+        @VisibleForTesting
+        static String serializeDescriptor(ModuleDescriptor descriptor) {
+            StringBuilder sb = new StringBuilder()
+
+            if (descriptor.isOpen())
+                sb.append("open ")
+            sb.append("module { name: ").append(descriptor.name())
+            if (!descriptor.requires().isEmpty())
+                sb.append(", ").append(descriptor.requires().sort().collect { serializeRequires(it) })
+            if (!descriptor.uses().isEmpty())
+                sb.append(", uses: ").append(descriptor.uses().sort())
+            if (!descriptor.exports().isEmpty())
+                sb.append(", exports: ").append(descriptor.exports().sort().collect { serializeExports(it) })
+            if (!descriptor.opens().isEmpty())
+                sb.append(", opens: ").append(descriptor.opens().sort().collect { serializeOpens(it) })
+            if (!descriptor.provides().isEmpty()) {
+                sb.append(", provides: ").append(descriptor.provides().sort().collect { serializeProvides(it) })
             }
-            descriptor.exports().each { strippedDescriptor.exports(it) }
-            descriptor.opens().each {strippedDescriptor.opens(it) }
-            descriptor.provides().each { strippedDescriptor.provides(it) }
-            descriptor.requires().each { strippedDescriptor.requires(it) }
-            descriptor.uses()each { strippedDescriptor.uses(it) }
-            return strippedDescriptor.build()
+            sb.append(" }")
+            return sb.toString()
+        }
+
+        private static String serializeRequires(ModuleDescriptor.Requires requires) {
+            String requireString
+            if (! requires.compiledVersion().empty) {
+                requireString = requires.name() + " (@" + requires.compiledVersion() + ")"
+            } else {
+                requireString = requires.name()
+            }
+            return withSerializedMods(requires.modifiers(), requireString)
+        }
+
+        private static String serializeExports(ModuleDescriptor.Exports exports) {
+            String s = withSerializedMods(exports.modifiers(), exports.source())
+            if (exports.targets().isEmpty())
+                return s;
+            else
+                return s + " to " + exports.targets().sort()
+        }
+
+        private static String serializeOpens(ModuleDescriptor.Opens opens) {
+            String s = withSerializedMods(opens.modifiers(), opens.source())
+            if (opens.targets().isEmpty())
+                return s;
+            else
+                return s + " to " + opens.targets().sort()
+        }
+
+        private static String serializeProvides(ModuleDescriptor.Provides provides) {
+            return provides.service() + " with " + Lists.newArrayList(provides.providers()).sort()
+        }
+
+        static <M> String withSerializedMods(Set<M> mods, String what) {
+            return (Stream.concat(mods.stream().map(e -> e.toString()
+                .toLowerCase(Locale.ROOT)).sorted(),
+                Stream.of(what)))
+                .collect(Collectors.joining(" "))
         }
     }
 }

src/test/groovy/org/gradle/android/JdkImageWorkaroundDescriptorTest.groovy

@@ -0,0 +1,63 @@
+package org.gradle.android
+
+import com.google.common.collect.Sets
+import org.gradle.android.workarounds.JdkImageWorkaround
+import spock.lang.Specification
+import spock.lang.Unroll
+
+import java.lang.module.ModuleDescriptor
+
+class JdkImageWorkaroundDescriptorTest extends Specification {
+    def "normalizes module descriptors with different orders in the values"() {
+        given:
+        def serialized1 = JdkImageWorkaround.ExtractJdkImageTransform.serializeDescriptor(descriptor(["org.foo", "org.bar"], ["baz", "fizz"]))
+        def serialized2 = JdkImageWorkaround.ExtractJdkImageTransform.serializeDescriptor(descriptor(["org.bar", "org.foo"], ["fizz", "baz"]))
+
+        expect:
+        serialized1 == serialized2
+    }
+
+    def "descriptors with different values are serialized differently"() {
+        given:
+        def serialized1 = JdkImageWorkaround.ExtractJdkImageTransform.serializeDescriptor(descriptor(["org.foo", "org.bar"], ["baz", "fizz"]))
+        def serialized2 = JdkImageWorkaround.ExtractJdkImageTransform.serializeDescriptor(descriptor(["org.bar", "org.foo", "org.baz"], ["baz", "fizz"]))
+
+        expect:
+        serialized1 != serialized2
+    }
+
+    @Unroll
+    def "all descriptor values are captured (#modifier.trim())"() {
+        ModuleDescriptor descriptor = builder
+            .requires("foo").requires("bar")
+            .uses("org.baz").uses("org.fizz")
+            .exports("org.exports", ["exportsTarget1", "exportsTarget2"] as Set)
+            .provides("org.provides", ["org.provider1", "org.provider2"])
+            .with {
+                modifier == "" ? opens("org.opens", ["opensTarget1", "opensTarget2"] as Set) : it
+            }.build()
+
+        expect:
+        JdkImageWorkaround.ExtractJdkImageTransform.serializeDescriptor(descriptor) == "${modifier}module { name: myModule, " +
+            "[bar, foo, mandated java.base], " +
+            "uses: [org.baz, org.fizz], " +
+            "exports: [org.exports to [exportsTarget1, exportsTarget2]], " +
+            (modifier == "" ? "opens: [org.opens to [opensTarget1, opensTarget2]], " : "") +
+            "provides: [org.provides with [org.provider1, org.provider2]] }"
+
+        where:
+        builder                                    | modifier
+        ModuleDescriptor.newModule("myModule")     | ""
+        ModuleDescriptor.newOpenModule("myModule") | "open "
+    }
+
+    static def descriptor(List<String> packages, List<String> values) {
+        ModuleDescriptor.Builder builder = ModuleDescriptor.newModule("myModule")
+        values.each { builder.requires(it) }
+        packages.each { builder.uses(it) }
+        packages.each { builder.exports("${it}.exports", Sets.newLinkedHashSet(values)) }
+        packages.each { builder.opens("${it}.opens", Sets.newLinkedHashSet(values)) }
+        packages.each { builder.provides("${it}.provides", values.collect { v -> "org.${v}".toString() }) }
+        return builder.build()
+    }
+}