Add kotlin compatability to build file validation #167143

reidbaker · 2025-04-14T21:42:17Z

Fixes #161443

adds a new gradle task like javaVersion named kgpVersion that prints the version of kgp.
adds gradle_utils.dart method for getting kgp version
- kgp method is moved to utilities and we attempt to use a plugin method before reflection.
adds methods or evaluating KGP + gradle and KGP + AGP compatibility.
- It turns out that we have been using incompatible versions that happen to work with the subset of kotlin we are using.
Uses new kgp methods in flutter analyze --suggestions as part of the existing agp/java/gradle compatibility matrix.
adds new tests for all new functionality
Adds comments to sections of the code I found could use them.
Modifies flutter gallery to use a compatible version of kotlin and update its lockfiles.

Pre-launch Checklist

I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
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.

…hecker

…p version hardcoded

… list kgp information

reidbaker · 2025-04-15T22:30:50Z

packages/flutter_tools/gradle/src/main/kotlin/DependencyVersionChecker.kt

        return 0
    }

+    override fun equals(other: Any?): Boolean = other is Version && compareTo(other) == 0


This was required so that assertEquals would pass on 2 version instances with the same values.

packages/flutter_tools/lib/src/android/gradle_utils.dart

…tests

dev/integration_tests/flutter_gallery/android/settings.gradle

reidbaker · 2025-04-16T16:35:44Z

packages/flutter_tools/gradle/src/main/kotlin/FlutterPluginUtils.kt

+    /**
+     * Returns the version of the Kotlin Gradle plugin.
+     */
+    internal fun getKGPVersion(project: Project): Version? {


Moved from DependencyVersionChecker with the addition of the section around // Partial implementation of getKotlinPluginVersion from the comment above.
This was added because I couldnt get mockk to handle the reflection we were doing and wanted to rely on something other than the param we set in our templates.

Hmm I'm not sure I understand why it needs to move. Can we not have the tests of getKGPVersion live in DependencyVersionCheckerTest, in exactly the same form they have now, and then reference DependencyVersionChecker.getKGPVersion from the task adder below?

If yes, I think it makes more sense to keep all the methods related to getting a version of a dependency in the same place.

This code moved because it went from a method intended for use in only dependency checker to a method intended for use in multiple files. Aka a utils style method.

The test changes reflect that it is now not an implementation detail (there were tests that checked the result of dependency checking but no unit test for getting the version).
The new test for getKgpVersion unit test its behavior (or 2/3s of it).

Getting the java version is the most similar but the code is significantly less complex so it is duplicated instead of all in one place. If there was going to be one place I think a utils class would be the one that would make the most sense.

Hmm I'm not sure I agree that a utils class makes the most sense, the DependencyVersionChecker is intended to be the singular object which directly interacts with dependency versions. I could see renaming it to DependencyVersionHandler or similar, to better align with its purpose, but I still think it belongs more in that class then in a utils class.

Let me start with I am open to a different location for this code.
If you look at the kt doc I added to DependencyVersionChecker where I was trying to document what the class was for I said it was a class that warns or errors on version ranges that flutter does not support.

The only public method is checkDependencyVersions which prior to this pr was aligned. If you feedback is you dont like the explosion of responsibilities in utils I can totally understand that and it is the risk of any class named utils.

To me DependencyVersionChecker should stay a class focused on evaluating and enforcing the versions. If we were to have a new package named common where code that was intended to be shared lived then I would include utils but not DependencyVersionCheker so it seems like expanding that class to have methods that are used by multiple other classes is the wrong scope.

So why did I pick FlutterPluginUtils? In addition to the previously stated reason that FlutterPluginUtils was designed as a place for methods that get used in other code it also had a section titled Methods that interact primarily with the Gradle project which felt like a good fit. I debated moving all getXVersion methods but decided against it until the methods were needed by other classes and because this pr is already pretty large.

What I didnt consider was if a new class was a better fit.
How about I make a new class named VersionFetcher (or any other better name) where the point of that class is that it has methods to get the various versions that we depend on. Then I would refactor the javaVersion task to use VersionFetcher.getJavaVersion() and modify DependencyVersionChecker to get the versions from VersionFetcher.

I have implemented this option and if you agree it is the right solution then let me know and I will add tests for the methods that were previously @VisibleForTesting and had no unit tests.

My primary concern was just that "getting the versions of dependencies we use" was a logical unit of functionality that I wanted to remain encapsulated in the same class/object - VersionFetcher accomplishes this for me, so LGTM w/ tests!

Fantastic, I will add some unit tests for these methods then merge.

reidbaker

Thank you for the review. Most (all?) feedback is addressed.

reidbaker · 2025-04-17T14:18:27Z

packages/flutter_tools/gradle/src/main/kotlin/FlutterPluginUtils.kt

+    /**
+     * Returns the version of the Kotlin Gradle plugin.
+     */
+    internal fun getKGPVersion(project: Project): Version? {


Let me start with I am open to a different location for this code.
If you look at the kt doc I added to DependencyVersionChecker where I was trying to document what the class was for I said it was a class that warns or errors on version ranges that flutter does not support.

The only public method is checkDependencyVersions which prior to this pr was aligned. If you feedback is you dont like the explosion of responsibilities in utils I can totally understand that and it is the risk of any class named utils.

To me DependencyVersionChecker should stay a class focused on evaluating and enforcing the versions. If we were to have a new package named common where code that was intended to be shared lived then I would include utils but not DependencyVersionCheker so it seems like expanding that class to have methods that are used by multiple other classes is the wrong scope.

So why did I pick FlutterPluginUtils? In addition to the previously stated reason that FlutterPluginUtils was designed as a place for methods that get used in other code it also had a section titled Methods that interact primarily with the Gradle project which felt like a good fit. I debated moving all getXVersion methods but decided against it until the methods were needed by other classes and because this pr is already pretty large.

What I didnt consider was if a new class was a better fit.
How about I make a new class named VersionFetcher (or any other better name) where the point of that class is that it has methods to get the various versions that we depend on. Then I would refactor the javaVersion task to use VersionFetcher.getJavaVersion() and modify DependencyVersionChecker to get the versions from VersionFetcher.

I have implemented this option and if you agree it is the right solution then let me know and I will add tests for the methods that were previously @VisibleForTesting and had no unit tests.

reidbaker · 2025-04-17T14:20:58Z

packages/flutter_tools/gradle/src/main/kotlin/FlutterPluginUtils.kt

            description = "Print the current java version used by gradle. see: " +
                "https://docs.gradle.org/current/javadoc/org/gradle/api/JavaVersion.html"
            doLast {
+                // https://docs.gradle.org/current/kotlin-dsl/gradle/org.gradle.api/-java-version/index.html#-1790786897%2FFunctions%2F-1793262594


removed.

FWIW This is the same comment you added in the initial kts version of dependency version checker.

https://github.com/flutter/flutter/pull/143341/files#diff-6511375a022834485d04afa5202102637390a2636c369e4c78c1939bedcc4382

gmackall · 2025-04-17T16:42:47Z

packages/flutter_tools/gradle/src/main/kotlin/VersionFetcher.kt

+import org.gradle.api.Project
+import org.jetbrains.kotlin.gradle.plugin.KotlinAndroidPluginWrapper
+
+internal class VersionFetcher {


nit: I think this can just be an object right? As the whole contents are within the companion object

ash2moon · 2025-04-17T18:03:27Z

packages/flutter_tools/gradle/src/main/kotlin/VersionFetcher.kt

+        }
+        // Fall back to reflection.
+        val versionField =
+            kotlinPlugin?.javaClass?.kotlin?.members?.first {


.first here would throw an error if it does not exist. Can we use .firstOrNull?

…flutter#167143)

flutter/flutter@ecabb1a...3ed38e2 2025-04-18 [email protected] Update `led` docs (flutter/flutter#167380) 2025-04-18 [email protected] Throw an error if --local-engine-host is used without --local-engine (flutter/flutter#166948) 2025-04-18 [email protected] Add kotlin compatability to build file validation (flutter/flutter#167143) 2025-04-18 [email protected] Roll Skia from 844496884aa0 to 6c4595124690 (2 revisions) (flutter/flutter#167400) 2025-04-18 [email protected] Fix CarouselView crashes when initlal viewportDimension is 0.0 (flutter/flutter#167271) 2025-04-18 [email protected] Roll Dart SDK from b1eb743f97f5 to ab60afc99bcb (1 revision) (flutter/flutter#167396) 2025-04-18 [email protected] Add InputDecoration.visualDensity and InputDecorationTheme.visualDensity (flutter/flutter#166834) 2025-04-18 [email protected] Roll Skia from acc910544da7 to 844496884aa0 (1 revision) (flutter/flutter#167389) 2025-04-18 [email protected] Roll Skia from a409d685a711 to acc910544da7 (10 revisions) (flutter/flutter#167388) 2025-04-18 [email protected] Roll Dart SDK from 2bb85834e77e to b1eb743f97f5 (2 revisions) (flutter/flutter#167387) 2025-04-18 [email protected] Roll Fuchsia Linux SDK from m8Aln7fTF_8zy1V9N... to MwYckh5OvwwmIYLx0... (flutter/flutter#167385) 2025-04-17 [email protected] Roll Skia from cc2b57434651 to a409d685a711 (3 revisions) (flutter/flutter#167347) 2025-04-17 [email protected] Reduce app startup latency by initializing the engine on a separate thread (flutter/flutter#166918) 2025-04-17 [email protected] Roll Dart SDK from 992221a362ec to 2bb85834e77e (7 revisions) (flutter/flutter#167361) 2025-04-17 [email protected] Added docstring for FilterInput::GetSnapshot (flutter/flutter#167226) 2025-04-17 [email protected] adds Entity docstrings (flutter/flutter#167228) 2025-04-17 [email protected] fixed impeller golden content scale (flutter/flutter#167308) 2025-04-17 [email protected] Added docstring for FilterContents::RenderFilter (flutter/flutter#167227) 2025-04-17 [email protected] Feat: Add equality to NoDefaultCupertinoThemeData (flutter/flutter#166655) 2025-04-17 [email protected] Fix link to engine docs (flutter/flutter#167346) 2025-04-17 [email protected] [macOS] Enable merged platform and UI thread by default (flutter/flutter#166536) 2025-04-17 [email protected] [Windows] Enable merged platform and UI thread by default (flutter/flutter#163726) 2025-04-17 [email protected] Update `CHANGELOG.md` (flutter/flutter#167225) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…r#9114) flutter/flutter@ecabb1a...3ed38e2 2025-04-18 [email protected] Update `led` docs (flutter/flutter#167380) 2025-04-18 [email protected] Throw an error if --local-engine-host is used without --local-engine (flutter/flutter#166948) 2025-04-18 [email protected] Add kotlin compatability to build file validation (flutter/flutter#167143) 2025-04-18 [email protected] Roll Skia from 844496884aa0 to 6c4595124690 (2 revisions) (flutter/flutter#167400) 2025-04-18 [email protected] Fix CarouselView crashes when initlal viewportDimension is 0.0 (flutter/flutter#167271) 2025-04-18 [email protected] Roll Dart SDK from b1eb743f97f5 to ab60afc99bcb (1 revision) (flutter/flutter#167396) 2025-04-18 [email protected] Add InputDecoration.visualDensity and InputDecorationTheme.visualDensity (flutter/flutter#166834) 2025-04-18 [email protected] Roll Skia from acc910544da7 to 844496884aa0 (1 revision) (flutter/flutter#167389) 2025-04-18 [email protected] Roll Skia from a409d685a711 to acc910544da7 (10 revisions) (flutter/flutter#167388) 2025-04-18 [email protected] Roll Dart SDK from 2bb85834e77e to b1eb743f97f5 (2 revisions) (flutter/flutter#167387) 2025-04-18 [email protected] Roll Fuchsia Linux SDK from m8Aln7fTF_8zy1V9N... to MwYckh5OvwwmIYLx0... (flutter/flutter#167385) 2025-04-17 [email protected] Roll Skia from cc2b57434651 to a409d685a711 (3 revisions) (flutter/flutter#167347) 2025-04-17 [email protected] Reduce app startup latency by initializing the engine on a separate thread (flutter/flutter#166918) 2025-04-17 [email protected] Roll Dart SDK from 992221a362ec to 2bb85834e77e (7 revisions) (flutter/flutter#167361) 2025-04-17 [email protected] Added docstring for FilterInput::GetSnapshot (flutter/flutter#167226) 2025-04-17 [email protected] adds Entity docstrings (flutter/flutter#167228) 2025-04-17 [email protected] fixed impeller golden content scale (flutter/flutter#167308) 2025-04-17 [email protected] Added docstring for FilterContents::RenderFilter (flutter/flutter#167227) 2025-04-17 [email protected] Feat: Add equality to NoDefaultCupertinoThemeData (flutter/flutter#166655) 2025-04-17 [email protected] Fix link to engine docs (flutter/flutter#167346) 2025-04-17 [email protected] [macOS] Enable merged platform and UI thread by default (flutter/flutter#166536) 2025-04-17 [email protected] [Windows] Enable merged platform and UI thread by default (flutter/flutter#163726) 2025-04-17 [email protected] Update `CHANGELOG.md` (flutter/flutter#167225) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…flutter#167143)

Fixes flutter#161443 * adds a new gradle task like `javaVersion` named `kgpVersion` that prints the version of kgp. * adds gradle_utils.dart method for getting kgp version * * kgp method is moved to utilities and we attempt to use a plugin method before reflection. * adds methods or evaluating KGP + gradle and KGP + AGP compatibility. * * It turns out that we have been using incompatible versions that happen to work with the subset of kotlin we are using. * Uses new kgp methods in flutter analyze --suggestions as part of the existing agp/java/gradle compatibility matrix. * adds new tests for all new functionality * Adds comments to sections of the code I found could use them. * Modifies flutter gallery to use a compatible version of kotlin and update its lockfiles. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [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.

…flutter#167143)

reidbaker added 3 commits April 14, 2025 14:59

Document the difference between gradle utils and dependency version c…

cc551c4

…hecker

Kotlin evaluation code and unit tests, not call in project yet and kg…

8255f05

…p version hardcoded

Add project implemenation and make existing tests pass

d9717aa

github-actions bot added platform-android Android applications specifically tool Affects the "flutter" command-line tool. See also t: labels. labels Apr 14, 2025

reidbaker added 13 commits April 15, 2025 10:23

formatting

e107cca

Expand kgp and gradle evaluation

82a7cfb

formatting

d574650

analyze errors

732a7fb

Formatting and unknown kotlin version test

3154483

tests for validating agp and kgp

ef0d045

Add task that prints kgp version

7b4010c

typo

c756f44

add tests for kgp version printing task

52fc78f

text before kgp version

e0dfb4d

make exising project tests pass after using kgpVersion gradle task to…

3441fb8

… list kgp information

add tests for kgp/gradle compatibility

8fc54b0

Add agp/kgp compatibility and testing

ed90b55

reidbaker commented Apr 15, 2025

View reviewed changes

packages/flutter_tools/lib/src/android/gradle_utils.dart Outdated Show resolved Hide resolved

reidbaker added 3 commits April 16, 2025 11:42

Add regex logic to kgp version finder

3950583

add tests for getKgpVersion

2e1fa2c

update kotlin version in flutter gallery, update analyze suggestions …

d927728

…tests

reidbaker commented Apr 16, 2025

View reviewed changes

dev/integration_tests/flutter_gallery/android/settings.gradle Show resolved Hide resolved

reidbaker commented Apr 16, 2025

View reviewed changes

Formatting and comment updates

c422b0d

reidbaker requested review from a team, ash2moon and gmackall April 16, 2025 16:46

update lockfiles

42f269b

Add VersionFetcher class

1d09475

reidbaker commented Apr 17, 2025

View reviewed changes

Change detector test

2a7534e

gmackall reviewed Apr 17, 2025

View reviewed changes

gmackall approved these changes Apr 17, 2025

View reviewed changes

Add tests for getAgpVersion and getGradleVersion

62de3f5

ash2moon reviewed Apr 17, 2025

View reviewed changes

reidbaker and others added 2 commits April 17, 2025 14:32

First or null instead of first

2ab573f

Merge branch 'master' into i_161443_kotlin_agp_compat

96cc0ee

reidbaker added autosubmit Merge PR when tree becomes green via auto submit App and removed will affect goldens Changes to golden files labels Apr 18, 2025

auto-submit bot added this pull request to the merge queue Apr 18, 2025

Merged via the queue into flutter:master with commit f20bc39 Apr 18, 2025
152 checks passed

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Apr 18, 2025

engine-flutter-autoroll mentioned this pull request Apr 18, 2025

Roll Flutter from ecabb1a05212 to f20bc39cc005 (21 revisions) flutter/packages#9113

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 18, 2025

f20bc39cc Add kotlin compatability to build file validation (flutter/…

94ed472

…flutter#167143)

engine-flutter-autoroll mentioned this pull request Apr 18, 2025

Roll Flutter from ecabb1a05212 to 3ed38e24528a (23 revisions) flutter/packages#9114

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 18, 2025

f20bc39cc Add kotlin compatability to build file validation (flutter/…

18a9f1f

…flutter#167143)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2025

f20bc39cc Add kotlin compatability to build file validation (flutter/…

1e2a22b

…flutter#167143)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2025

f20bc39cc Add kotlin compatability to build file validation (flutter/…

09ec946

…flutter#167143)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2025

f20bc39cc Add kotlin compatability to build file validation (flutter/…

02b795f

…flutter#167143)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2025

f20bc39cc Add kotlin compatability to build file validation (flutter/…

e33d043

…flutter#167143)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 16, 2025

f20bc39cc Add kotlin compatability to build file validation (flutter/…

9ae5009

…flutter#167143)

Add kotlin compatability to build file validation #167143

Add kotlin compatability to build file validation #167143

Uh oh!

Conversation

reidbaker commented Apr 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

reidbaker left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

reidbaker commented Apr 14, 2025 •

edited

Loading