Improve task output caching (wooga/atlas-build-unity#135)

> Note Original PR: wooga/atlas-build-unity#135

Description
===========

The task output caching was practically not working due to the
task setup. This patch makes sure that the `importCodeSigningIdentities`
can be cached and the xcode build tasks run only when the sources change.

I can not setup a proper test for this yet as we would need credentials
and certificates set up as well. I will create those in a later patch.

Changes
=======

* ![IMPROVE] task output caching

integrationTest/groovy/wooga/gradle/build/unity/ios/tasks/ImportCodeSigningIdentitiesIntegrationSpec.groovy

@@ -23,6 +23,7 @@ class ImportCodeSigningIdentitiesIntegrationSpec extends IOSBuildIntegrationSpec
     def setup() {
         buildFile << """
         task ${subjectUnderTestName}(type: ${subjectUnderTestTypeName}) {
+            //inputKeychain = file('${buildKeychain.location}') 
             keychain = file('${buildKeychain.location}')
         }
         """.stripIndent()
@@ -158,43 +159,51 @@ class ImportCodeSigningIdentitiesIntegrationSpec extends IOSBuildIntegrationSpec
         value = wrapValueBasedOnType(rawValue, type, wrapValueFallback)
     }
 
-    @Unroll
-    def "#expectedBehavior task if #message"() {
-        given: "a test certificate"
+    def "is up-to-date when #reason"() {
+        given: "an invalid sighing certificate"
         def passphrase = "123456"
-        def identityName = "codesign test"
-        def cert = SecurityHelper.createTestCodeSigningCertificatePkcs12([commonName: identityName], passphrase)
-
-        if (identityIsInKeychain) {
-            buildKeychain.importFile(cert, [passphrase: passphrase])
-        }
+        def cert = SecurityHelper.createTestCodeSigningCertificatePkcs12([commonName: signingIdentity], passphrase)
+        def futureKeychain = new File(projectDir, "build/keychains/test.keychain")
+        assert !futureKeychain.exists()
 
         buildFile << """
         ${subjectUnderTestName} {
             passphrase = "${passphrase}"
             p12 = file('${cert.path}')
-            signingIdentity = "${identityName}"
+            signingIdentity = "${signingIdentity}"
             ignoreInvalidSigningIdentity = true
-            applicationAccessPaths = ${appAccessPathsValue} 
+            inputKeychain = file('${buildKeychain.location}') 
+            keychain = file('${futureKeychain.path}')
+            password = "123456" 
         }
         """.stripIndent()
 
         when:
-        def result = runTasksSuccessfully(subjectUnderTestName)
+        def result = runTasks(subjectUnderTestName)
+
+        then:
+        result.success
+        !result.wasSkipped(subjectUnderTestName)
+        futureKeychain.exists()
+
+        when:
+        result = runTasks(subjectUnderTestName)
+
+        then:
+        result.success
+        result.wasUpToDate(subjectUnderTestName)
+
+        when:
+        futureKeychain.delete()
+        result = runTasks(subjectUnderTestName)
 
         then:
-        result.wasSkipped(subjectUnderTestName) == wasSkipped
+        result.success
+        !result.wasUpToDate(subjectUnderTestName)
 
         where:
-        appAccessPaths        | identityIsInKeychain | wasSkipped
-        []                    | true                 | true
-        []                    | false                | false
-        ['/usr/bin/codesign'] | true                 | false
-        ['/usr/bin/codesign'] | false                | false
-        expectedBehavior = wasSkipped ? "skips" : "does not skip"
-        appAccessPathsMessage = appAccessPaths.empty ? "no app access paths are configured" : "app access path are configured"
-        message = identityIsInKeychain ? "identity is already in keychain and ${appAccessPathsMessage}" : "identity is not in keychain and ${appAccessPathsMessage}"
-        appAccessPathsValue = wrapValueBasedOnType(appAccessPaths, "List<String>")
+        signingIdentity            | _
+        "test signing: Wooga GmbH" | _
     }
 
     def "skips with NO-SOURCE when p12 file is not set"() {

main/groovy/wooga/gradle/build/unity/ios/IOSBuildPlugin.groovy

@@ -33,6 +33,7 @@ import wooga.gradle.fastlane.FastlanePlugin
 import wooga.gradle.fastlane.FastlanePluginExtension
 import wooga.gradle.fastlane.tasks.PilotUpload
 import wooga.gradle.fastlane.tasks.SighRenew
+import wooga.gradle.macOS.security.SecurityKeychainOutputSpec
 import wooga.gradle.macOS.security.tasks.*
 import wooga.gradle.xcodebuild.XcodeBuildPlugin
 import wooga.gradle.xcodebuild.tasks.ArchiveDebugSymbols
@@ -90,6 +91,16 @@ class IOSBuildPlugin implements Plugin<Project> {
             }
         })
 
+        project.tasks.withType(ImportCodeSigningIdentities.class, new Action<ImportCodeSigningIdentities>() {
+            @Override
+            void execute(ImportCodeSigningIdentities task) {
+                task.baseName.convention("build")
+                task.extension.convention("keychain")
+                task.password.convention(extension.keychainPassword)
+                task.destinationDir.convention(project.layout.buildDirectory.dir("sign/keychains"))
+            }
+        })
+
         project.tasks.withType(SighRenew.class, new Action<SighRenew>() {
             @Override
             void execute(SighRenew task) {
@@ -169,13 +180,22 @@ class IOSBuildPlugin implements Plugin<Project> {
 
     void generateBuildTasks(final String baseName, final Project project, File xcodeProject, IOSBuildPluginExtension extension) {
         def tasks = project.tasks
-        def buildKeychain = tasks.create(maybeBaseName(baseName, "buildKeychain"), SecurityCreateKeychain) {
+
+        def createKeychain = tasks.create(maybeBaseName(baseName, "createKeychain"), SecurityCreateKeychain) {
             it.baseName = maybeBaseName(baseName, "build")
         }
 
+        ImportCodeSigningIdentities buildKeychain = tasks.create(maybeBaseName(baseName, "importCodeSigningIdentities"), ImportCodeSigningIdentities) {
+            it.inputKeychain.set(createKeychain.getKeychain())
+            it.signingIdentities.convention(extension.signingIdentities)
+            it.passphrase.convention(extension.codeSigningIdentityFilePassphrase)
+            it.p12.convention(extension.codeSigningIdentityFile)
+            dependsOn(createKeychain)
+        }
+
         def unlockKeychain = tasks.create(maybeBaseName(baseName, "unlockKeychain"), SecurityUnlockKeychain) {
             it.dependsOn(buildKeychain, buildKeychain)
-            it.password.set(buildKeychain.password)
+            it.password.set(createKeychain.password)
             it.keychain.set(buildKeychain.keychain)
         }
 
@@ -199,14 +219,7 @@ class IOSBuildPlugin implements Plugin<Project> {
             it.keychain(buildKeychain.keychain.map({ it.asFile }))
         }
 
-        def importCodeSigningIdentities = tasks.create(maybeBaseName(baseName, "importCodeSigningIdentities"), ImportCodeSigningIdentities) {
-            it.keychain.set(buildKeychain.getKeychain())
-            it.signingIdentities.convention(extension.signingIdentities)
-            it.passphrase.convention(extension.codeSigningIdentityFilePassphrase)
-            it.p12.convention(extension.codeSigningIdentityFile)
-            dependsOn(buildKeychain, unlockKeychain)
-            finalizedBy(removeKeychain, lockKeychain)
-        }
+        buildKeychain.finalizedBy(removeKeychain, lockKeychain)
 
         def shutdownHook = new Thread({
             System.err.println("shutdown hook called")
@@ -232,7 +245,7 @@ class IOSBuildPlugin implements Plugin<Project> {
         }
 
         def importProvisioningProfiles = tasks.create(maybeBaseName(baseName, "importProvisioningProfiles"), SighRenew) {
-            it.dependsOn addKeychain, importCodeSigningIdentities, unlockKeychain
+            it.dependsOn addKeychain, buildKeychain, unlockKeychain
             it.finalizedBy removeKeychain, lockKeychain
             it.fileName.set("${maybeBaseName(baseName, 'signing')}.mobileprovision".toString())
         }
@@ -242,7 +255,7 @@ class IOSBuildPlugin implements Plugin<Project> {
         }
 
         def xcodeArchive = tasks.create(maybeBaseName(baseName, "xcodeArchive"), XcodeArchive) {
-            it.dependsOn addKeychain, unlockKeychain, podInstall, importProvisioningProfiles
+            it.dependsOn addKeychain, unlockKeychain, podInstall, buildKeychain
             it.projectPath.set(project.provider({
                 def d = project.layout.buildDirectory.get()
                 if (podInstall.workspace.exists()) {

main/groovy/wooga/gradle/build/unity/ios/tasks/ImportCodeSigningIdentities.groovy

@@ -1,9 +1,11 @@
 package wooga.gradle.build.unity.ios.tasks
 
-import com.wooga.gradle.BaseSpec
 import com.wooga.security.FindIdentityResult
 import com.wooga.security.command.FindIdentity
 import com.wooga.security.command.Import
+import com.wooga.security.command.LockKeychain
+import com.wooga.security.command.UnlockKeychain
+import org.apache.commons.io.FileUtils
 import org.gradle.api.DefaultTask
 import org.gradle.api.GradleException
 import org.gradle.api.file.RegularFile
@@ -12,21 +14,23 @@ import org.gradle.api.provider.ListProperty
 import org.gradle.api.provider.Property
 import org.gradle.api.provider.Provider
 import org.gradle.api.tasks.*
+import wooga.gradle.macOS.security.SecurityKeychainOutputSpec
 
-class ImportCodeSigningIdentities extends DefaultTask implements BaseSpec {
+class ImportCodeSigningIdentities extends DefaultTask implements SecurityKeychainOutputSpec {
 
     @InputFiles
     @SkipWhenEmpty
     protected getInputFiles() {
         if (p12.isPresent()) {
-            return project.files(p12, keychain)
+            return project.files(p12, inputKeychain)
         }
         project.files()
     }
 
     private final RegularFileProperty p12 = objects.fileProperty()
 
-    @Internal
+    @InputFile
+    @Optional
     RegularFileProperty getP12() {
         p12
     }
@@ -70,24 +74,35 @@ class ImportCodeSigningIdentities extends DefaultTask implements BaseSpec {
         ignoreInvalidSigningIdentity.set(value)
     }
 
-    private final RegularFileProperty keychain = objects.fileProperty()
+    private final RegularFileProperty inputKeychain = objects.fileProperty()
 
     @InputFile
-    RegularFileProperty getKeychain() {
-        return keychain
+    RegularFileProperty getInputKeychain() {
+        inputKeychain
+    }
+
+    void setInputKeychain(Provider<RegularFile> value) {
+        inputKeychain.set(value)
     }
 
-    void setKeychain(File value) {
-        keychain.set(value)
+    void setInputKeychain(File value) {
+        inputKeychain.set(value)
     }
 
-    void setKeychain(Provider<RegularFile> value) {
-        keychain.set(value)
+    private final Property<String> password = objects.property(String)
+
+    @Input
+    @Optional
+    Property<String> getPassword() {
+        password
     }
 
-    @OutputFile
-    protected Provider<RegularFile> getOutputFile() {
-        keychain
+    void setPassword(Provider<String> value) {
+        password.set(value)
+    }
+
+    void setPassword(String value) {
+        password.set(value)
     }
 
     private final ListProperty<String> signingIdentities = objects.listProperty(String)
@@ -154,23 +169,22 @@ class ImportCodeSigningIdentities extends DefaultTask implements BaseSpec {
             false
         }
 
-        onlyIf {
-            /* We can only skip if the task does not
-            configure the application access path. There is no
-            way to check the current access rights for items in the keychain
-            from the security cli tool. So we must treat the task as unskippable
-            even if the identity might exist.
-            */
-            def ignoreInvalidSigningIdentity = ignoreInvalidSigningIdentity.getOrElse(false)
-            def signingIdentities = signingIdentities.get()
-            if (applicationAccessPaths.get().isEmpty() && !signingIdentities.isEmpty()) {
-                return !signingIdentities.every({ hasSigningIdentity(it, !ignoreInvalidSigningIdentity) })
+        fileName.convention(baseName.map({
+            if (extension.present) {
+                return it + "." + extension.get()
             }
-            true
-        }
+            it
+        }))
+
+        keychain.convention(destinationDir.file(fileName))
+        inputKeychain.convention(keychain)
     }
 
     Boolean hasSigningIdentity(String identity, Boolean validIdentities = true) {
+        if (!keychain.get().asFile.exists()) {
+            return false
+        }
+
         def command = new FindIdentity()
                 .withKeychain(keychain.get().asFile)
                 .withPolicy(FindIdentity.Policy.Codesigning)
@@ -186,35 +200,50 @@ class ImportCodeSigningIdentities extends DefaultTask implements BaseSpec {
 
     @TaskAction
     protected void importCodeSigningIdentity() {
+        def inputKeychain = inputKeychain.get().asFile
         def keychain = keychain.get().asFile
 
-        def importCertificates = new Import(p12.get().asFile, keychain)
-                .withPassphrase(passphrase.get())
-                .withType(Import.Type.Cert)
-                .withFormat(Import.Format.Pkcs12)
-
-        applicationAccessPaths.get().each {
-            importCertificates.allowAccessFrom(it)
+        if (inputKeychain.canonicalPath != keychain.canonicalPath) {
+            FileUtils.copyFile(inputKeychain, keychain)
         }
 
-        importCertificates.execute()
-
-        if (signingIdentities.present) {
-            signingIdentities.get().each { String signingIdentity ->
-                if (!hasSigningIdentity(signingIdentity, false)) {
-                    throw new GradleException("Unable to find code sign identity '${signingIdentity}' in keychain ${keychain.path}")
-                } else {
-                    if (hasSigningIdentity(signingIdentity)) {
-                        logger.info("Signing Identity '${signingIdentity}' successfull imported into keychain ${keychain.path}")
-                    } else if (!ignoreInvalidSigningIdentity.get()) {
-                        throw new GradleException("Unable to find valid code sign identity '${signingIdentity}' in keychain ${keychain.path}")
+        if (p12.isPresent()) {
+            if (password.present) {
+                new UnlockKeychain().withKeychain(keychain).withPassword(password.get()).execute()
+            }
+
+            def importCertificates = new Import(p12.get().asFile, keychain)
+                    .withPassphrase(passphrase.get())
+                    .withType(Import.Type.Cert)
+                    .withFormat(Import.Format.Pkcs12)
+
+            applicationAccessPaths.get().each {
+                importCertificates.allowAccessFrom(it)
+            }
+
+            importCertificates.execute()
+
+            if (password.present) {
+                new LockKeychain().withKeychain(keychain)
+            }
+
+            if (signingIdentities.present) {
+                signingIdentities.get().each { String signingIdentity ->
+                    if (!hasSigningIdentity(signingIdentity, false)) {
+                        throw new GradleException("Unable to find code sign identity '${signingIdentity}' in keychain ${keychain.path}")
                     } else {
-                        logger.warn("Signing Identity '${signingIdentity}' found but invalid")
+                        if (hasSigningIdentity(signingIdentity)) {
+                            logger.info("Signing Identity '${signingIdentity}' successfull imported into keychain ${keychain.path}")
+                        } else if (!ignoreInvalidSigningIdentity.get()) {
+                            throw new GradleException("Unable to find valid code sign identity '${signingIdentity}' in keychain ${keychain.path}")
+                        } else {
+                            logger.warn("Signing Identity '${signingIdentity}' found but invalid")
+                        }
                     }
                 }
+            } else {
+                logger.info("No CodeSigningIdentity specified. Skip verification")
             }
-        } else {
-            logger.info("No CodeSigningIdentity specified. Skip verification")
         }
     }
 }