-
Notifications
You must be signed in to change notification settings - Fork 470
Support AGP with Kotlin Built-in #4295
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
rename file to match function name Update test.
12c2bb1 to
4dd9e43
Compare
When using AGP 9, KGP is no longer required. AGP will be responsible for configuring Kotlin compilation. DGP needs to support Kotlin BuiltIn by extracting information from AGP instead of KGP. - Update KotlinAdapter and AndroidAdapter to handle AGP with Kotlin built-in. - Capture classpaths of variants from `AndroidComponentsExtension` instead of the Android extension. - Update captured AGP variant info to include classpaths. - Only apply KotlinAdapter and AndroidAdapter once per project (Remove the `exec()` function, only apply the plugin once using `KotlinAdapter.applyTo()`).
Preparation for testing AGP9. - Add min/max version to `@TestsAndroid` and `@TestsAndroidCompose` filter AGP versions. - Add SemVerRange to help with filtering. - Add AGP9 properties.
| && name.startsWith(MAIN_SOURCE_SET_NAME) | ||
|
|
||
| internal fun applyTo(project: Project) { | ||
| project.pluginManager.apply(type = JavaAdapter::class) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor (again, about consistency): we should probably react on java plugins here, and not just apply it uncoditionally. E.g. use project.plugins.withType<JavaBasePlugin>() as with kotlin/android adapters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True - but let's not do it in this PR, since it's not related to AGP support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in #4362
| AndroidLibrary, | ||
| AndroidTest, | ||
| AndroidDynamicFeature, | ||
| AndroidKmpLibrary, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious: do we need it here? I mean, I thought, that with that plugin, we should have enough information coming from KGP, and so we should not apply AndroidAdapter in that case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean AndroidKmpLibrary?
See the test project:
https://github.com/Kotlin/dokka/blob/adam/support-agp-kotlin-builtin/dokka-integration-tests/gradle/projects/it-android-kotlin-mp-builtin/build.gradle.kts
The only AGP plugin is com.android.kotlin.multiplatform.library. If that plugin is present then AndroidAdapter is required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean AndroidKmpLibrary?
Yeah, I mean specifically this one.
Got it, for some reason, I thought that compilationClasspath from the kotlin compilation itself should be enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to extract anything besides androidExt.bootClasspath() in this case in AndroidAdapter?
Don't we get the classpath from Kotlin compilations?
In which cases classpath from Kotlin compilation is not enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In which cases classpath from Kotlin compilation is not enough?
If built-in-kotlin isn't enabled, or it's a non-Kotlin project (although DGP doesn't have tests for this).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oleg and I looked into it more, applying AndroidAdapter when AndroidKmpLibrary is present doesn't appear to be necessary. compilationClasspath already contains android.jar and the classpath from Android's Components.
In fact, for kotlin-built-in & AGP9 AndroidAdapter shouldn't be necessary at all, since all information should be exposed via 'normal' KGP API (KotlinSourceSets and KotlinCompilations).
I'll try and remove AndroidKmpLibrary and report back if there are issues. If there are missing classpath JARs, we should probably report this as an AGP bug.
dokka-integration-tests/gradle/src/test/kotlin/AndroidKotlinMultiplatformBuiltInTest.kt
Outdated
Show resolved
Hide resolved
…are 'publishable' (#4231) DGP should only document 'publishable' (i.e. main, non-test) source sets. DGP needs to extract this information from KGP and AGP. For AGP projects, DPG currently uses `KotlinJvmAndroidCompilation.androidVariant` to determine publishability of a source set. However, [this is deprecated in KGP 2.2.10](https://github.com/JetBrains/kotlin/blob/v2.2.10/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/jvm/KotlinJvmAndroidCompilation.kt#L38-L39). It will eventually be removed. This is because AGP will implement Kotlin compilation internally (AKA Kotlin built-in). This PR uses an alternative AGP utility: `AndroidComponentsExtension`. DGP extracts Components from this extension and stores it in a separate data structure (to avoid Configuration Cache issues with serialising too much data). If a Component has at least one Variant, it is considered 'publishable'. Each Component is associated with a single KotlinCompilation (matched by name), and each KotlinSourceSet is associated with a list of KotlinCompilations. While using `AndroidComponentsExtension` is required for AGP9, it also works for AGP7 and 8. To reduce complexity, this PR uses the same approach for all AGP versions. --- This PR is the first part of supporting AGP 9 in DGP. The next PR is #4295, which will support and test AGP 9 with and without Kotlin built-in.
dokka-runners/dokka-gradle-plugin/src/main/kotlin/adapters/AndroidAdapter.kt
Outdated
Show resolved
Hide resolved
If a project has `kotlin-multiplatform` and `com.android.kotlin.multiplatform.library` then AndroidAdapter shouldn't be necessary. `compilationClasspath` already contains android.jar and the classpath from Android's Components.
Filter out KGP versions, specifically because com.android.kotlin.multiplatform.library requires KGP 2.0.0+
minor tweak to make it easier to see where the comments refer to
# Conflicts: # dokka-integration-tests/gradle/src/main/kotlin/org/jetbrains/dokka/it/gradle/junit/TestedVersionsSource.kt # dokka-runners/dokka-gradle-plugin/src/main/kotlin/adapters/KotlinAdapter.kt # dokka-runners/dokka-gradle-plugin/src/main/kotlin/internal/findExtensionLenient.kt # dokka-runners/dokka-gradle-plugin/src/testFunctional/kotlin/MultiModuleFunctionalTest.kt
|
Converted to a draft because I'm going to split this PR into smaller pieces.
|
Tag tests that use Kotlin JVM plugin. (This will become more relevant when we add a test for AGP9 with Kotlin-built-in.)
Move the `@WithGradleProperties` defaults to the tags that require them. The test classes are simpler. (This will become more relevant when we need to test AGP9 with built-in-kotlin.)
…ort-agp-kotlin-builtin # Conflicts: # dokka-integration-tests/gradle/src/main/kotlin/org/jetbrains/dokka/it/gradle/junit/testTags.kt # dokka-integration-tests/gradle/src/test/kotlin/AndroidComposeIT.kt # dokka-integration-tests/gradle/src/test/kotlin/AndroidProjectIT.kt # dokka-integration-tests/gradle/src/test/kotlin/MultiplatformAndroidJvmProjectIT.kt
# Conflicts: # dokka-integration-tests/gradle/src/main/kotlin/org/jetbrains/dokka/it/gradle/junit/testTags.kt # dokka-integration-tests/gradle/src/test/kotlin/AndroidComposeIT.kt # dokka-integration-tests/gradle/src/test/kotlin/MultiplatformAndroidJvmProjectIT.kt
# Conflicts: # build.gradle.kts # dokka-integration-tests/gradle/projects/it-android-kotlin-mp-builtin/build.gradle.kts # dokka-integration-tests/gradle/projects/it-android-kotlin-mp-builtin/gradle.properties # dokka-integration-tests/gradle/src/test/kotlin/AndroidKotlinMultiplatformBuiltInTest.kt
- Instead of lower-level manual version selection, instead defined whether a test project supports Kotlin built-in - Remove SemVerRange
… is not necessary since 9.0.0-alpha03.
…enerated It could be a bit of a pain to maintain? But it's quick and easy, and if it prevents a bug where a version is accidentally filtered out it'd be worth it. (we could use test coverage support, but this is a quick and easy one)
When using AGP 9, KGP is no longer required. AGP will be responsible for configuring Kotlin compilation. DGP needs to support Kotlin Built-in by extracting information from AGP instead of KGP.
Summary of changes
AndroidComponentsExtensioninstead of the Android extension.exec()function, only apply the plugin once usingKotlinAdapter.applyTo()).it-android-kotlin-jvm-builtin- AGP & kotlin-built-in (no KGP plugin).it-android-kotlin-mp-builtin- AGP & kotlin-multiplatform.Integration tests: support AGP 9 testing and version filtering
@TestsAndroidand@TestsAndroidComposefilter AGP versions.