Skip to content

[AGP 9.1.0 Migration] Extract internal utilities - #190949

Closed
reidbaker wants to merge 10 commits into
flutter:masterfrom
reidbaker:agp-internal-utils
Closed

reidbaker wants to merge 10 commits into
flutter:masterfrom
reidbaker:agp-internal-utils

Conversation

@reidbaker

@reidbaker reidbaker commented Aug 11, 2026 •

Copy link
Copy Markdown
Contributor

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. Add AGP public-API migration docs (contributor doc + breaking-change page draft) reidbaker-agent/flutter#1 (branch: agp-api-doc)
  2. Remove AGP-internal imports from VersionFetcher and migrate compileSdk/ndk reads to the public DSL reidbaker-agent/flutter#2 (branch: agp-internal-utils)
  3. Add public buildModeFor overloads and wire Flutter dependencies through the new DSL reidbaker-agent/flutter#3 (branch: agp-buildmode-deps)
  4. Replace the plugin build-type copy with initWith on the public DSL reidbaker-agent/flutter#4 (branch: agp-plugin-buildtypes)
  5. Delete getLegacyAndroidExtension and migrate the NDK fallback to the public DSL reidbaker-agent/flutter#5 (branch: agp-ndk-fallback)
  6. Register the Flutter compile task lazily from the variant API for app projects reidbaker-agent/flutter#6 (branch: agp-assets-onvariants)
  7. Migrate the flutter-apk copy and per-ABI versionCode to the variant API reidbaker-agent/flutter#7 (branch: agp-apk-copy-versioncode)
  8. Unify add-to-app module wiring on the variant API and drop host-project cross-wiring reidbaker-agent/flutter#8 (branch: agp-add-to-app)
  9. Move aar_init_script off legacy AGP APIs reidbaker-agent/flutter#9 (branch: agp-aar-script)
  10. Flip the newDsl default: drop the opt-out from templates and migrate it away reidbaker-agent/flutter#10 (branch: agp-newdsl-flip)
  11. Compile the Flutter Gradle Plugin against the gradle-api artifact only reidbaker-agent/flutter#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

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • I followed the [breaking change policy] and added [Data Driven Fixes] where supported.
  • All existing and new tests are passing.

…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.
…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.
@reidbaker
reidbaker requested a review from a team as a code owner August 11, 2026 19:30
@reidbaker
reidbaker requested review from jesswrd and removed request for a team August 11, 2026 19:30
@flutter-dashboard flutter-dashboard Bot added the CICD Run CI/CD label Aug 11, 2026
@github-actions github-actions Bot added platform-android Android applications specifically tool Affects the "flutter" command-line tool. See also t: labels. d: docs/ flutter/flutter/docs, for contributors labels Aug 11, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +147 to +153
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
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
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
}

@reidbaker
reidbaker marked this pull request as draft August 11, 2026 20:11
@reidbaker
reidbaker requested review from gmackall and mboetger August 11, 2026 20:12
Comment on lines -534 to -536
internal fun getConfiguredNdkVersion(project: Project): String? =
project.extensions.findByType(ApplicationExtension::class.java)?.ndkVersion
?: getLegacyAndroidExtension(project).ndkVersion

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why delete this file?

}
return@answers Unit
}
setAgpKotlinVersionToNull(mockProject)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and below it looks like we went from explicitly saying the class was null to returning a value. Why?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we need to do this regularly can you author a helper method an use that instead of duplicating code?

@reidbaker reidbaker closed this Aug 11, 2026
okorohelijah pushed a commit to okorohelijah/flutter that referenced this pull request Aug 17, 2026
…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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CICD Run CI/CD d: docs/ flutter/flutter/docs, for contributors platform-android Android applications specifically tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant