Repository navigation
Conversation
…page draft) Phase P0 of the Flutter Gradle Plugin migration to the AGP public API surface (flutter#180137, flutter#166550): the contributor migration record (replacement map, decision records, phase map, revert-window table) and the draft of the user-facing breaking-change page to be published to docs.flutter.dev before the newDsl flip reaches beta. Revert-safe: always.
…erwrite in custom copy snippet
…k/ndk reads to the public DSL Phase P1 of the AGP public-API migration (flutter#180137, flutter#166550): - VersionFetcher no longer calls the internal com.android.build.gradle.internal.utils.getKotlinAndroidPluginVersion. getKGPVersion now relies on the existing public fallback chain (kotlin_version property -> KotlinAndroidPluginWrapper.pluginVersion -> reflection) and documents that null is the expected result when KGP is absent (e.g. under AGP built-in Kotlin). DependencyVersionChecker already treats null as "KGP not required". - AgpCommonExtensionWrapper gains compileSdkPreview. - getCompileSdkFromProject now reads compileSdk/compileSdkPreview from the new DSL via the wrapper and returns a structured CompileSdkVersion instead of parsing the legacy "android-NN" string. The PluginHandler compileSdk mismatch warning uses CompileSdkVersion.isHigherThan, which defines the preview semantics the old string comparison got wrong (resolves the PluginHandler TODO): preview > numeric, distinct preview codenames incomparable (no warning), numeric compared numerically. Warning message text is unchanged. - getConfiguredNdkVersion reads through the wrapper instead of the legacy BaseExtension fallback. - Deletes the setAgpKotlinVersionToNull test helper (existed only to stub the internal AGP call) and updates unit tests accordingly; adds tests for both preview-vs-numeric directions and the codename reset case. Verification note: the FGP unit test suite could not be executed in this sandbox (the network policy blocks dl.google.com, so AGP artifacts do not resolve); run 'gradle test' in packages/flutter_tools/gradle in CI. Revert-safe until P2 lands.
There was a problem hiding this comment.
Code Review
This pull request migrates the Flutter Gradle Plugin's compile SDK and NDK version detection to the public AGP API surface, replacing legacy BaseExtension usage with wrapper accessors and introducing a CompileSdkVersion data class to handle numeric and preview SDK comparisons. It also removes internal Kotlin Gradle Plugin version fetching and adds documentation detailing the migration path. Feedback was provided to refine the isHigherThan comparison logic in CompileSdkVersion to handle cases where both preview codename and API level are set.
| fun isHigherThan(other: CompileSdkVersion): Boolean = | ||
| when { | ||
| previewCodename != null && other.previewCodename != null -> false | ||
| previewCodename != null && other.apiLevel != null -> true | ||
| apiLevel != null && other.apiLevel != null -> apiLevel > other.apiLevel | ||
| else -> false | ||
| } |
There was a problem hiding this comment.
To make isHigherThan more robust and defensive against cases where both previewCodename and apiLevel might be populated (e.g., in mock environments or future AGP versions), we should explicitly check if other is a preview first. Since a preview codename targets an unreleased SDK, any numeric API level (or another preview) cannot be considered higher than it, so we should immediately return false if other.previewCodename is not null.
| fun isHigherThan(other: CompileSdkVersion): Boolean = | |
| when { | |
| previewCodename != null && other.previewCodename != null -> false | |
| previewCodename != null && other.apiLevel != null -> true | |
| apiLevel != null && other.apiLevel != null -> apiLevel > other.apiLevel | |
| else -> false | |
| } | |
| fun isHigherThan(other: CompileSdkVersion): Boolean = | |
| when { | |
| other.previewCodename != null -> false | |
| previewCodename != null && other.apiLevel != null -> true | |
| apiLevel != null && other.apiLevel != null -> apiLevel > other.apiLevel | |
| else -> false | |
| } |
| internal fun getConfiguredNdkVersion(project: Project): String? = | ||
| project.extensions.findByType(ApplicationExtension::class.java)?.ndkVersion | ||
| ?: getLegacyAndroidExtension(project).ndkVersion |
There was a problem hiding this comment.
Why is is safe to not use getLegacyAndroidExtension in this pr? What versions of AGP were supported by getLegacyAndroidExtension?
| @JvmStatic | ||
| @JvmName("getCompileSdkFromProject") | ||
| internal fun getCompileSdkFromProject(project: Project): String = getLegacyAndroidExtension(project).compileSdkVersion!!.substring(8) | ||
| internal fun getCompileSdkFromProject(project: Project): CompileSdkVersion { |
There was a problem hiding this comment.
I like the type safety here. I was thinking that maybe this needs a different name so that when debugging calling code that did not change in this pr it would be more obvious that something about the method changed BUT I decided that if the type change didnt break the code then that reference was likely a passthrough anyway.
There was a problem hiding this comment.
why delete this file?
| } | ||
| return@answers Unit | ||
| } | ||
| setAgpKotlinVersionToNull(mockProject) |
There was a problem hiding this comment.
Why was this change needed?
| every { project.gradle.startParameter.taskNames } returns emptyList() | ||
| every { project.gradle.startParameter.isOffline } returns false | ||
| every { project.extensions.findByType(ApplicationExtension::class.java) } returns null | ||
| val mockAndroidExtension = mockk<ApplicationExtension>() |
There was a problem hiding this comment.
Here and below it looks like we went from explicitly saying the class was null to returning a value. Why?
There was a problem hiding this comment.
if we need to do this regularly can you author a helper method an use that instead of duplicating code?
…90957) This is PR 2 of 11 in the AGP 9.1.0 / public `gradle-api`/ newdls migration stack. This PR extracts some common utilities used in the Flutter Gradle Plugin to internal functions, to be used in upcoming PRs in this stack. It also introduces a typesafe CompileSdkVersion that handles comparisons between api versions and preview versions which are strings. First attempt was here flutter#190949 this pr includes my feedback from that first review. The first attempt did not follow the pattern of having the agent account open the pr because of rebase shenanigans that ended up touching freeze.yml. --- Standard review context for this pr stack This is PR is part of an 11 pr stack to migrate the "newdsl" `gradle-api` specifically in agp 9.1.0. The complete stack has pass pre submits, post submits and customer tests. https://flutter-dashboard.appspot.com/#/build?repo=flutter&branch=experimental/agp-gradle-api All of the code was LLM authored. A mix of manual prompting, automatic prompting, several models and adversarial review. The combined sessions are enough that I cannot include relevant prompts like I have been doing on other prs. If you want to review the pr stack you can find it here. These prs will be abandoned/closed as prs land into flutter/flutter. 1. reidbaker-agent#1 (branch: agp-api-doc) 2. reidbaker-agent#2 (branch: agp-internal-utils) 3. reidbaker-agent#3 (branch: agp-buildmode-deps) 4. reidbaker-agent#4 (branch: agp-plugin-buildtypes) 5. reidbaker-agent#5 (branch: agp-ndk-fallback) 6. reidbaker-agent#6 (branch: agp-assets-onvariants) 7. reidbaker-agent#7 (branch: agp-apk-copy-versioncode) 8. reidbaker-agent#8 (branch: agp-add-to-app) 9. reidbaker-agent#9 (branch: agp-aar-script) 10. reidbaker-agent#10 (branch: agp-newdsl-flip) 11. reidbaker-agent#11 (branch: agp-gradle-api) This work is urgent in the sense that we are worried that android will publish agp 10 with no opt out but not so urgent that we are willing to break flutter users because we didn't review or understand the code because we were in a rush. Breaking changes are expected as part of this work. There are patterns the android team explicitly does not want apps to use and apis that have no equivalent. As part of the effort to ensure this work does not slip into ai slop, prs from this stack will be reviewed by me (@reidbaker) before asking for review. Then we will have 2 android expert reviewers also review the every pr. --- Agent authored description. This is PR 2 of 11 in the AGP 9.1.0 / public `gradle-api` migration stack. This PR extracts some common utilities used in the Flutter Gradle Plugin to internal functions, to be used in upcoming PRs in this stack. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. --------- Co-authored-by: reidbaker-agent <[email protected]> Co-authored-by: Reid Baker <[email protected]>
Standard review context for this pr stack
This is PR is part of an 11 pr stack to migrate the "newdsl"
gradle-apispecifically in agp 9.1.0.The complete stack has pass pre submits, post submits and customer tests. https://flutter-dashboard.appspot.com/#/build?repo=flutter&branch=experimental/agp-gradle-api
All of the code was LLM authored. A mix of manual prompting, automatic prompting, several models and adversarial review. The combined sessions are enough that I cannot include relevant prompts like I have been doing on other prs.
If you want to review the pr stack you can find it here. These prs will be abandoned/closed as prs land into flutter/flutter.
This work is urgent in the sense that we are worried that android will publish agp 10 with no opt out but not so urgent that we are willing to break flutter users because we didn't review or understand the code because we were in a rush.
Breaking changes are expected as part of this work. There are patterns the android team explicitly does not want apps to use and apis that have no equivalent.
As part of the effort to ensure this work does not slip into ai slop, prs from this stack will be reviewed by me (@reidbaker) before asking for review. Then we will have 2 android expert reviewers also review the every pr.
Agent authored description.
This is PR 2 of 11 in the AGP 9.1.0 / public
gradle-apimigration stack.This PR extracts some common utilities used in the Flutter Gradle Plugin to internal functions, to be used in upcoming PRs in this stack.
Pre-launch Checklist
///).