google / ksp

Kotlin Symbol Processing API
https://github.com/google/ksp
Apache License 2.0
2.87k stars 272 forks source link

Order between KAPT and KSP should be deterministic, KAPT outputs should not be inputs to KSP #863

Open runningcode opened 2 years ago

runningcode commented 2 years ago

KAPT and KSP do not have deterministic ordering.

The issue is that the KSP task adds the outputs of KAPT as inputs to KSP. This breaks up-to-date checks and changes the cache key for the second execution of the KSP task.

Example simplified build scenario: Build A: KotlinCompile executes KSP executes KAPT executes

Build B: KotlinCompile up-to-date KSP re-executes KAPT up-to-date

I would expect that in the second build KSP would be up-to-date but it is not because KAPT outputs are added as an input to KSP and were not present in the previous build.

To fix this:

  1. Guarantee an ordering between KSP and KAPT
  2. Do not add KAPT outputs as inputs to KSP

Here is a sample task input comparison. Build B was the second build described in the scenario above and executed the KSP task because of additional KAPT outputs. image

neetopia commented 2 years ago

Can you share your build.gradle.kts? we might need this for more information on how ksp is getting kapt output to its input.

abghiulhaq commented 2 years ago

Can you share your build.gradle.kts? we might need this for more information on how ksp is getting kapt output to its input.

here is the build.gradle.kts that @runningcode mention https://gist.github.com/abghiulhaq/b1148a7da459ae0449432c818f48de5c#file-build-gradle-kts

ting-yuan commented 2 years ago

KAPT's output being fed to KSP is surprising. By design, KAPT and KSP shouldn't observe outputs from each other. Therefore the order shouldn't matter.

One possibility might be the following KSP setting:

ksp {
  allowSourcesFromOtherPlugins = true
}

However, I can't see it in the provided build.gradle.kts. Just in case, is it enabled in other places?

Other than that, can you print the input files of :core:Cache:kspDebugKotlin to get a hint of where it could be from? e.g.,

tasks.withType<KotlinCompile> {
    if ("ksp" in name)
        println("$name: ${inputs.files.toList()}")
}

If none of the above works, can you try to use a debugger to break on SourceTask.source() and see what wired them together?

abghiulhaq commented 2 years ago

we are not enabling this KSP setting in our code

ksp {
  allowSourcesFromOtherPlugins = true
}

here is the input files that causing the :core:Cache:kspDebugKotlin not up-to-date

Screen Shot 2022-02-26 at 21 19 38
ting-yuan commented 2 years ago

Can you share a sample project for reproduction? I still can't see how the outputs of kapt is propagated to ksp from the build.gradle.kts. ksp tasks add inputs from source sets of corresponding kotlin compilation tasks. The core/Cache/build/generated/source/kapt/debug directory doesn't seem to be added in the build.gradle.kts.

donkeytronkilla commented 2 years ago

Is this setting described somewhere?

ksp { allowSourcesFromOtherPlugins = true }

Does this mean that KSP can be configured to observe and execute on the output of Kapt and java annotation processors?

abghiulhaq commented 2 years ago

We have

sourceSets {
                sourceSets["main"].java.srcDirs(
                    "src/main/java",
                    "${project.buildDir}/generated/source/kapt/debug"
                )
}

in our convention plugin id("com.kitabisa.android.module") to fix some issue in jacoco. This is probably the culprit if ksp add input from source sets.

ting-yuan commented 2 years ago

Is this setting described somewhere?

No. We are still trying to figure out whether it is possible (or should we?) to inherit task dependencies from the compilation task. Currently, developers need to manually add the task dependency, otherwise it'd break Gradle configuration cache. Please use it at your discretion :)

ting-yuan commented 2 years ago

"${project.buildDir}/generated/source/kapt/debug"

Yes, looks like this is how kapt and ksp are wired together. If letting ksp process the outputs from kapt is harmless, a workaround (on top of that jacoco workaround) could be ksp_task.dependsOn(kapt_task). Otherwise I don't see what we can do in ksp. Filtering out sources in build might work, but the next bug report would be ksp not picking up sources from build 😢