Update to support AGP 9.1.0 - #282
Conversation
4dab62b to
f0f8493
Compare
|
In Kotlin KMP, how can we pass along the NDK to the cargo plugin when there is no android {} block now? |
The You can still get a reference to ndkDirectory with KotlinMultiplatformAndroidComponentsExtension that cargoPlugin can leverage in KMP modules. That being said the api changed very substantially, and native support is more limited for android in KMP Google recommends:
Happy to discuss the approach taken in this PR and very open to alternatives suggestions |
1617f4e to
a4c73b8
Compare
| action: OnVariantAction, | ||
| ) { | ||
| kotlinMultiplatformLibraryExtension.onVariants { agpVariant -> | ||
| val isReleaseRequest = project.providers.gradleProperty("gobley.android.release") |
There was a problem hiding this comment.
Now that KMP module can no longer use debug/release variants, I'm not sure how else to configure release mode. It looks like you use a CONFIGuRATION env var for XCode builds should we try to do the same here?
There was a problem hiding this comment.
For Xcode builds, it is guaranteed that Xcode initializes specific environment variables, but for Android builds, there's no such thing. Currently, Gobley exposes properties like cargo.jvmVariant and cargo.jvmPublishingVariant, so the users can use their preferred Cargo profiles. I'm thinking of making this part read cargo.jvmVariant or cargo.jvmPublishingVariant, I'm just not sure where there will be any scenario that the user wants to build the dev version for Android while the release version for Desktop Java, or vice versa.
de445b8 to
b90f619
Compare
|
Also I understand this is an outrageously large PR submitted as a single commit, so I can potentially break it up into multiple PRs to make it more reviewable if desired. |
|
Sorry for the late reply. I'll review this by tomorrow. Thanks for waiting and sorry for making you wait again :( |
Oh no worries, this was a huge change the the gradle API |
| action: OnVariantAction, | ||
| ) { | ||
| kotlinMultiplatformLibraryExtension.onVariants { agpVariant -> | ||
| val isReleaseRequest = project.providers.gradleProperty("gobley.android.release") |
There was a problem hiding this comment.
For Xcode builds, it is guaranteed that Xcode initializes specific environment variables, but for Android builds, there's no such thing. Currently, Gobley exposes properties like cargo.jvmVariant and cargo.jvmPublishingVariant, so the users can use their preferred Cargo profiles. I'm thinking of making this part read cargo.jvmVariant or cargo.jvmPublishingVariant, I'm just not sure where there will be any scenario that the user wants to build the dev version for Android while the release version for Desktop Java, or vice versa.
| else -> dependencyProject.path | ||
| } | ||
| } | ||
| get() = path |
There was a problem hiding this comment.
I want to keep the compatibility between Gobley and older versions of Gradle; is this part for fixing compilation error due to missing ProjectDependency.dependencyProject?
|
|
||
| // Ensures getNumAlive() always returns deterministic value | ||
| private val coverallLock = ReentrantLock() | ||
| private val coverallLock = reentrantLock() |
There was a problem hiding this comment.
Does the compiler complain about this?
|
I was going to include this in 0.3.8, but this will cause a lot of changes in binary APIs. Will it be possible to minimize the changes and split the PR so some of the fixes are included in 0.3.8 (which will be the final version of 0.3.x) and leave the other for 0.4.0? For faster onboarding, integration with the AGP application plugin is important. I want to keep the application examples to use the application plugin. I'm planning to re-organize the examples and the plugin directory, do you have any suggestions? |
… AGP 9-compatible onVariants API AGP 9 sets `MergeSourceSetFolders.variant` to null, causing NPE in GobleyKotlinAndroidExtensionDelegate that silently drops JNI lib wiring. Fixes librival_sdk.so missing from debug APK. Port of gobley PR gobley#282: introduce GobleyAndroidCommonExtensionDelegate and GobleyAndroidKotlinMultiplatformExtensionDelegate that use `androidComponents.onVariants { agpVariant -> agpVariant.sources.jniLibs?.addGeneratedSourceDirectory(task) { it.outputDir } }` instead of task graph introspection. Delete GobleyKotlinAndroidExtensionDelegate. Add InjectJniLibsTask with typed outputDir property.
This is great! Could you please publish your fork to Maven as a temporary fix for AGP 9+ cases? It is so needed. |
Changes
This commit updates now deprecated gradle logic around Kotlin Multiplatform and Android library configurations to support AGP 9.1.0, updates the Rust toolchain, and refactors several build/test plugins to use modern Gradle lazy properties.
rust-toolchain.toml1.83.0to1.85.0. I am not much a Rust dev and didn't even have Cargo installed on my machine. So I might be doing something wrong but I was not able to get the project to build using1.83.0, I always received an error about having to do with getrandom not being compatible. Bumping to toolchain version fixed that. If this is unacceptable for this PR I can just drop the change.Build Logic & Plugins
CargoPlugin.ktUniFfiPlugin.ktmainBindingsDirectory,commonBindingsDirectory, etc.) with explicitflatMapresolutions mapped directly from theBuildUniffiBindingsTaskoutput properties (e.g.buildBindings.flatMap { it.commonMainOutputDir }). This satisfies modern Gradle/AGP isolation requirements.addSingleTargetDependencyhelper for resolving properties independently without aggressively initializing Kotlin SourceSets.GobleyAndroidCommonExtensionDelegate.ktGobleyAndroidBaseExtension.ktbecause the BaseExtension has been remove in AGP 9.GobleyAndroidKotlinMultiplatformExtensionDelegate.ktGobleyAndroidExtensionDelegatefor KMP modules with andorid targets.Example App Configuration
audio-cpp-libortodolist-libinstead ofappsince they aren't really app modules any more but that's not necessary to do now.Testing
I ran through every unit test, android integration test, and ran example apps on JVM, Android, iOS.
Issues Fixed
Fixes: #153