fix incremental behavior when the compilation target has IR merging but no KAPT stub generation #1024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
RBusarow
force-pushed
the
rick/dependency-jar-race
branch
2 times, most recently
from
50311ae to
fef828f
Compare
…ut no KAPT stub generation KGP's "classpath snapshot" incremental logic uses a previous build's `.class` files to track references and invalidate files. Assume there are two projects: `:lib1` and `:lib2`. `:lib2` depends on `:lib1`. If a change is made to an existing type in `:lib1` since the last successful build, KGP will inspect the snapshot of `:lib2`'s class files to see if it references anything in `:lib1`. If `:lib2` didn't already reference `:lib1`, then the `:lib2:compileKotlin` task will be treated as `UP_TO_DATE` and will not execute again. Anvil's module and interface merging logic only changes the IR symbols. This means that all merged references are added to the resultant binary, but not to class files. Those changes are invisible to the classpath snapshots. This is why we disable incremental compilation in KAPT stub generation tasks, but it's also a problem in non-KAPT modules that are using other merging annotations. ### The fix If a compilation merges any interfaces or modules, a file is created with their names. The `AnvilPlugin` checks for the existence of this file during task configuration, and disables incremental compilation for that task if it's present. The file won't exist on a fresh build, regardless of whether there will be merging, but that's okay because the build wouldn't be incremental anyway.
RBusarow
force-pushed
the
rick/dependency-jar-race
branch
from
fef828f to
c274d53
Compare
JoelWilcox
approved these changes
Jul 11, 2024
Comment on lines
+90
to
+93
| PluginOption( | ||
| pluginId = anvilCommandLineProcessor.pluginId, | ||
| optionName = "track-source-files", | ||
| optionValue = (trackSourceFiles && mode is Embedded).toString(), |
There was a problem hiding this comment.
Why not keep this in the Embedded block below since this is Embedded only?
There was a problem hiding this comment.
It isn't technically Embedded only -- it's just only true for Embedded mode. It's now accessed for the IrContributionMerger parameters and that technically isn't exclusive to Embedded, so the argument must always be defined.
gradle-plugin/src/gradleTest/java/com/squareup/anvil/plugin/IncrementalTest.kt
Show resolved
Hide resolved
gradle-plugin/src/gradleTest/java/com/squareup/anvil/plugin/IncrementalTest.kt
Show resolved
Hide resolved
| ) { | ||
| stubsTaskProvider.configure { stubsTask -> | ||
| stubsTaskProvider.configure { stubsTask -> | ||
| if (!mergingIsDisabled()) { |
There was a problem hiding this comment.
Is there any downside (or benefit) to moving this check into the config block instead of avoiding the config block entirely?
There was a problem hiding this comment.
When the check happens outside the config block, it reads the variant's settings eagerly, as soon as the compilation is configured. When the check happens inside the stub gen task's config block, it has a better chance of catching any late changes made by a convention plugin or something.
|
|
||
| if (variant.variantFilter.trackSourceFiles && !mergingIsDisabled()) { | ||
| if (task is AbstractKotlinCompile<*>) { | ||
| // The `ir-merges.txt` file indicates that the last compilation resulted in adding module |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
KGP's "classpath snapshot" incremental logic uses a previous build's
.classfiles to track references and invalidate files. Assume there are two projects::lib1and:lib2.:lib2depends on:lib1. If a change is made to an existing type in:lib1since the last successful build, KGP will inspect the snapshot of:lib2's class files to see if it references anything in:lib1. If:lib2didn't already reference:lib1, then the:lib2:compileKotlintask will be treated asUP_TO_DATEand will not execute again.Anvil's module and interface merging logic only changes the IR symbols. This means that all merged references are added to the resultant binary, but not to class files. Those changes are invisible to the classpath snapshots. This is why we disable incremental compilation in KAPT stub generation tasks, but it's also a problem in non-KAPT modules that are using other merging annotations.
The fix
If a compilation merges any interfaces or modules, a file is created with their names. The
AnvilPluginchecks for the existence of this file during task configuration, and disables incremental compilation for that task if it's present. The file won't exist on a fresh build, regardless of whether there will be merging, but that's okay because the build wouldn't be incremental anyway.