Fix Android ap option providers as Kapt task nested inputs

1) Exclude the providers arguments from the kapt inputs, as the values may contain absolute paths, may be output properties etc. The providers should correctly annotate their inputs. 2) Fix the options passed to kapt as 'value=option' (leading to all the options collapse into one with the null key), use 'key=option' instead to make Kapt pass them as '-Aoption'. Issue #KT-23866 Fixed Issue #KT-25027 Fixed
2018-06-21 23:42:51 +03:00
parent a3be68de82
commit dbd72ae53b
5 changed files with 94 additions and 27 deletions
@@ -45,6 +45,9 @@ enum class FilesOptionKind {
    // such as output directories, inputs or classpath options.
 }

+/** Defines a subplugin option that should be excluded from Gradle input/output checks */
+open class InternalSubpluginOption(key: String, value: String) : SubpluginOption(key, value)
+
 interface KotlinGradleSubplugin<in KotlinCompile : AbstractCompile> {
    fun isApplicable(project: Project, task: AbstractCompile): Boolean

@@ -26,17 +26,20 @@ class KotlinAndroid32GradleIT : KotlinAndroid3GradleIT(GradleVersionRequired.AtL
            apply plugin: 'kotlin-kapt'

            class MyNested implements org.gradle.process.CommandLineArgumentProvider {
-                String value = ""

                @InputFile
                File inputFile = null

                @Override
-                Iterable<String> asArguments() { return [value] }
+                @Internal
+                Iterable<String> asArguments() {
+                    // Read the arguments from a file, because changing them in a build script is treated as an
+                    // implementation change by Gradle:
+                    return [new File('args.txt').text]
+                }
            }

            def nested = new MyNested()
-            nested.value = '123'
            nested.inputFile = file("${'$'}projectDir/in.txt")

            android.applicationVariants.all {
@@ -46,6 +49,7 @@ class KotlinAndroid32GradleIT : KotlinAndroid3GradleIT(GradleVersionRequired.AtL
        )

        File(projectDir, "Android/in.txt").appendText("1234")
+        File(projectDir, "args.txt").appendText("1234")

        val kaptTasks = listOf(":Android:kaptFlavor1DebugKotlin")
        val javacTasks = listOf(":Android:compileFlavor1DebugJavaWithJavac")
@@ -65,14 +69,13 @@ class KotlinAndroid32GradleIT : KotlinAndroid3GradleIT(GradleVersionRequired.AtL
            assertTasksUpToDate(javacTasks)
        }

-        gradleBuildScript(subproject = "Android").modify {
-            it.replace("nested.value = '123'", "nested.value = '456'")
-        }
+        // Changing only the annotation provider arguments should not trigger the tasks to run, as the arguments may be outputs,
+        // internals or neither:
+        File(projectDir, "args.txt").appendText("5678")

        build(*buildTasks) {
            assertSuccessful()
-            assertTasksExecuted(kaptTasks)
-            assertTasksUpToDate(javacTasks)
+            assertTasksUpToDate(javacTasks + kaptTasks)
        }
    }
 }
@@ -1,18 +1,18 @@
 package org.jetbrains.kotlin.gradle

 import org.jetbrains.kotlin.gradle.util.isLegacyAndroidGradleVersion
+import org.jetbrains.kotlin.gradle.util.modify
 import org.jetbrains.kotlin.test.KotlinTestUtils
 import org.junit.Test
-import java.io.File

 class Kapt3Android30IT : Kapt3AndroidIT() {
    override val androidGradlePluginVersion: String
-        get() = "3.0.0-beta1"
+        get() = "3.2.0-alpha18"
 }

 open class Kapt3AndroidIT : Kapt3BaseIT() {
    companion object {
-        private val GRADLE_VERSION = GradleVersionRequired.AtLeast("4.1")
+        private val GRADLE_VERSION = GradleVersionRequired.AtLeast("4.6")
    }

    protected open val androidGradlePluginVersion: String
@@ -37,8 +37,17 @@ open class Kapt3AndroidIT : Kapt3BaseIT() {
            assertSuccessful()
            assertKaptSuccessful()
            assertFileExists("app/build/generated/source/kapt/debug/org/example/kotlin/butterknife/SimpleActivity\$\$ViewBinder.java")
-            assertFileExists("app/build/intermediates/classes/debug/org/example/kotlin/butterknife/SimpleActivity\$\$ViewBinder.class")
+
+            val butterknifeJavaClassesDir =
+                if (isLegacyAndroidGradleVersion(androidGradlePluginVersion))
+                    "app/build/intermediates/classes/debug/org/example/kotlin/butterknife/"
+                else
+                    "app/build/intermediates/javac/debug/compileDebugJavaWithJavac/classes/org/example/kotlin/butterknife/"
+
+            assertFileExists(butterknifeJavaClassesDir + "SimpleActivity\$\$ViewBinder.class")
+
            assertFileExists("app/build/tmp/kotlin-classes/debug/org/example/kotlin/butterknife/SimpleAdapter\$ViewHolder.class")
+
            if (isLegacyAndroidGradleVersion(androidGradlePluginVersion)) {
                // we don't copy classes with new AGP
                assertFileExists("app/build/intermediates/classes/debug/org/example/kotlin/butterknife/SimpleAdapter\$ViewHolder.class")
@@ -61,7 +70,15 @@ open class Kapt3AndroidIT : Kapt3BaseIT() {
            assertKaptSuccessful()
            assertFileExists("app/build/generated/source/kapt/debug/com/example/dagger/kotlin/DaggerApplicationComponent.java")
            assertFileExists("app/build/generated/source/kapt/debug/com/example/dagger/kotlin/ui/HomeActivity_MembersInjector.java")
-            assertFileExists("app/build/intermediates/classes/debug/com/example/dagger/kotlin/DaggerApplicationComponent.class")
+
+            val daggerJavaClassesDir =
+                if (isLegacyAndroidGradleVersion(androidGradlePluginVersion))
+                    "app/build/intermediates/classes/debug/com/example/dagger/kotlin/"
+                else
+                    "app/build/intermediates/javac/debug/compileDebugJavaWithJavac/classes/com/example/dagger/kotlin/"
+
+            assertFileExists(daggerJavaClassesDir + "DaggerApplicationComponent.class")
+
            assertFileExists("app/build/tmp/kotlin-classes/debug/com/example/dagger/kotlin/AndroidModule.class")
            if (isLegacyAndroidGradleVersion(androidGradlePluginVersion)) {
                // we don't copy classes with new AGP
@@ -116,11 +133,40 @@ open class Kapt3AndroidIT : Kapt3BaseIT() {
        val project = Project("android-databinding", GRADLE_VERSION, directoryPrefix = "kapt2")
        val options = androidBuildOptions()

+
+        if (!isLegacyAndroidGradleVersion(androidGradlePluginVersion)) {
+            project.setupWorkingDir()
+
+            // With new AGP, there's no need in the Databinding kapt dependency:
+            project.gradleBuildScript("app").modify {
+                it.lines().filterNot {
+                    it.contains("kapt \"com.android.databinding:compiler")
+                }.joinToString("\n")
+            }
+
+            // Workaround for KT-24915
+            project.gradleBuildScript("app").appendText(
+                "\n" + """
+               afterEvaluate {
+                    kaptDebugKotlin.dependsOn dataBindingExportFeaturePackageIdsDebug
+               }
+            """.trimIndent()
+            )
+        }
+
        project.build("assembleDebug", options = options) {
            assertSuccessful()
            assertKaptSuccessful()
            assertFileExists("app/build/generated/source/kapt/debug/com/example/databinding/BR.java")
-            assertFileExists("app/build/generated/source/kapt/debug/com/example/databinding/databinding/ActivityTestBinding.java")
+
+            if (isLegacyAndroidGradleVersion(androidGradlePluginVersion)) {
+                assertFileExists("app/build/generated/source/kapt/debug/com/example/databinding/databinding/ActivityTestBinding.java")
+            } else {
+                assertFileExists("app/build/generated/source/kapt/debug/com/example/databinding/databinding/ActivityTestBindingImpl.java")
+            }
+
+            // KT-23866
+            assertNotContains("The following options were not recognized by any processor")
        }
    }
 }
@@ -21,10 +21,10 @@ import org.gradle.process.CommandLineArgumentProvider
 import org.jetbrains.kotlin.gradle.internal.Kapt3GradleSubplugin.Companion.getKaptGeneratedClassesDir
 import org.jetbrains.kotlin.gradle.internal.Kapt3GradleSubplugin.Companion.getKaptGeneratedKotlinSourcesDir
 import org.jetbrains.kotlin.gradle.internal.Kapt3GradleSubplugin.Companion.getKaptGeneratedSourcesDir
-import org.jetbrains.kotlin.gradle.tasks.isWorkerAPISupported
 import org.jetbrains.kotlin.gradle.plugin.*
 import org.jetbrains.kotlin.gradle.tasks.CompilerPluginOptions
 import org.jetbrains.kotlin.gradle.tasks.KotlinCompile
+import org.jetbrains.kotlin.gradle.tasks.isWorkerAPISupported
 import org.jetbrains.kotlin.gradle.utils.isGradleVersionAtLeast
 import java.io.ByteArrayOutputStream
 import java.io.File
@@ -261,18 +261,26 @@ class Kapt3KotlinGradleSubplugin : KotlinGradleSubplugin<KotlinCompile> {
        val apOptionsFromProviders =
            if (isGradleVersionAtLeast(4, 6))
                kaptVariantData?.annotationProcessorOptionProviders
-                    ?.flatMap { (it as CommandLineArgumentProvider).asArguments().map { "" to it.removePrefix("-A") } }
+                    ?.flatMap { (it as CommandLineArgumentProvider).asArguments() }
                    .orEmpty()
            else
                emptyList()

+        val subluginOptionsFromProvidedApOptions = apOptionsFromProviders.map {
+            // Use the internal subplugin option type to exclude them from Gradle input/output checks, as their providers are already
+            // properly registered as a nested input:
+
+            // Pass options as they are in the key-only form (key = 'a=b'), kapt will deal with them:
+            InternalSubpluginOption(key = it.removePrefix("-A"), value = "")
+        }
+
        val apOptionsPairsList: List<Pair<String, String>> =
            kaptExtension.getAdditionalArguments(project, kaptVariantData?.variantData, androidPlugin).toList() +
-                    androidOptions.toList() +
-                    apOptionsFromProviders
+                    androidOptions.toList()

        return apOptionsPairsList.map { SubpluginOption(it.first, it.second) } +
-                FilesSubpluginOption(KAPT_KOTLIN_GENERATED, listOf(kotlinSourcesOutputDir))
+                FilesSubpluginOption(KAPT_KOTLIN_GENERATED, listOf(kotlinSourcesOutputDir)) +
+                subluginOptionsFromProvidedApOptions
    }

    private fun Kapt3SubpluginContext.buildAndAddOptionsTo(task: Task, container: CompilerPluginOptions, aptMode: String) {
@@ -350,16 +358,16 @@ class Kapt3KotlinGradleSubplugin : KotlinGradleSubplugin<KotlinCompile> {

        kaptTask.kaptClasspathConfigurations = kaptClasspathConfigurations

-        if (kaptTask is KaptWithKotlincTask) {
-            kaptVariantData?.annotationProcessorOptionProviders?.let {
-                kaptTask.annotationProcessorOptionProviders.add(it)
-            }
+        kaptVariantData?.annotationProcessorOptionProviders?.let {
+            kaptTask.annotationProcessorOptionProviders.add(it)
+        }

+        if (kaptTask is KaptWithKotlincTask) {
            buildAndAddOptionsTo(kaptTask, kaptTask.pluginOptions, aptMode = "apt")
        }

        if (kaptTask is KaptWithoutKotlincTask) {
-            project.configurations.create(KAPT_WORKER_DEPENDENCIES_CONFIGURATION_NAME).apply {
+            project.configurations.maybeCreate(KAPT_WORKER_DEPENDENCIES_CONFIGURATION_NAME).apply {
                project.getKotlinPluginVersion()?.let { kotlinPluginVersion ->
                    val kaptDependency = getPluginArtifact().run { "$groupId:$artifactId:$kotlinPluginVersion" }
                    dependencies.add(project.dependencies.create(kaptDependency))
@@ -16,17 +16,22 @@ import org.gradle.api.plugins.InvalidPluginException
 import org.gradle.api.plugins.JavaBasePlugin
 import org.gradle.api.plugins.JavaPlugin
 import org.gradle.api.plugins.JavaPluginConvention
-import org.gradle.api.tasks.*
+import org.gradle.api.tasks.CompileClasspathNormalizer
+import org.gradle.api.tasks.Delete
+import org.gradle.api.tasks.SourceSet
+import org.gradle.api.tasks.SourceSetOutput
 import org.gradle.api.tasks.compile.AbstractCompile
 import org.gradle.api.tasks.compile.JavaCompile
 import org.gradle.jvm.tasks.Jar
 import org.jetbrains.kotlin.cli.common.arguments.CommonCompilerArguments
 import org.jetbrains.kotlin.gradle.dsl.KotlinJvmOptionsImpl
-import org.jetbrains.kotlin.gradle.internal.*
 import org.jetbrains.kotlin.gradle.internal.Kapt3GradleSubplugin.Companion.getKaptGeneratedClassesDir
+import org.jetbrains.kotlin.gradle.internal.Kapt3KotlinGradleSubplugin
+import org.jetbrains.kotlin.gradle.internal.KaptVariantData
+import org.jetbrains.kotlin.gradle.internal.checkAndroidAnnotationProcessorDependencyUsage
+import org.jetbrains.kotlin.gradle.scripting.internal.ScriptingGradleSubplugin
 import org.jetbrains.kotlin.gradle.tasks.*
 import org.jetbrains.kotlin.gradle.utils.*
-import org.jetbrains.kotlin.gradle.scripting.internal.ScriptingGradleSubplugin
 import java.io.File
 import java.net.URL
 import java.util.*
@@ -722,6 +727,8 @@ internal fun Task.registerSubpluginOptionsAsInputs(subpluginId: String, subplugi
        optionsGroup.forEachIndexed { index, option ->
            val indexSuffix = if (optionsGroup.size > 1) ".$index" else ""
            when (option) {
+                is InternalSubpluginOption -> Unit
+
                is CompositeSubpluginOption -> {
                    val subpluginIdWithWrapperKey = "$subpluginId.${optionKey}$indexSuffix"
                    registerSubpluginOptionsAsInputs(subpluginIdWithWrapperKey, option.originalOptions)