Updated Most flutter/flutter Defaults to 37 - #193307
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
There was a problem hiding this comment.
Code Review
This pull request updates the Android SDK dependency version to 37v2 in the CI configuration and increments the default compileSdkVersion to 37 in the FlutterExtension class. There are no review comments, and I have no feedback to provide.
FlutterExtension.compileSdkVersion is now 37, but gradle_utils.dart still templated 36 into the module host app, module gradle and plugin templates. Under AGP 9, the :flutter library (compiled at 37) requires consumers to compile against 37 or higher, so 'flutter build apk' in add-to-app modules failed checkDebugAarMetadata. Keep the two values in sync as the Android README requires.
These host apps consume the :flutter module library, which now compiles against flutter.compileSdkVersion (37). AGP 9's AAR metadata check rejects consumers compiled against a lower API, failing module_custom_host_app_name_test, module_host_with_custom_build_test, build_android_host_app_with_module_aar and the multiple_flutters benchmarks. deferred_components_test/component1 is bumped for consistency with its base app; it has no AAR dependencies and was not failing.
Replace the AGP 8.2.1 / Gradle 8.4 case with AGP 8.11.1 / Gradle 8.14,
matching errorAGPVersion and errorGradleVersion in
DependencyVersionChecker.kt. AGP 8.2.1 cannot compile against API 37
("Failed to find Platform SDK with path: platforms;android-37"), which
Flutter AARs now require of their consumers, and it is already below
Flutter's supported minimum. Gradle 8.14 is in CI's gradle_dists cache.
Baklava is an API 36 preview, which is now below the default compileSdk of 37 used by integration_test and generated plugins, so AGP 9 rejects the build. Switch the test to CinnamonBun (API 37 preview), defined once in a previewCodename constant, and point the Linux android_preview_tool_integration_tests target at the android_sdk version:cinnamonbun CIPD package. The two are cross-referenced so they stay in sync.
…t-to-37 # Conflicts: # dev/devicelab/bin/tasks/build_android_host_app_with_module_aar.dart
flutter#193467 moved the pre-AGP 8.3 module AAR case (AGP 8.2.1 / Gradle 8.4) into the android_java17_build_android_host_app_with_module_aar target. AGP 8.2.1 cannot compile against API 37, which Flutter AARs now require of their consumers, so move that case to AGP 8.11.1 / Gradle 8.14 to match errorAGPVersion and errorGradleVersion. Gradle 8.14 still can't run on Java 25, so the Java 17 pin stays.
…t-to-37 # Conflicts: # .ci.yaml
Raised the minimum AGP version to 9.1.1 and the warn version to 9.3.1. Raised the minimum to AGP 9.1.1 so developers are all on AGP 9+ and it guarantees support for `compileSdk` 37, which we set [here](flutter#193307). ## 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 in-code documentation (doc comments with `///`). - [x] If this PR introduces a new feature or capability, I created and linked a website documentation issue or PR in [flutter/website] (or verified none is needed). - [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. - [ ] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [flutter/website]: https://github.com/flutter/website [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
|
A reason for requesting a revert of flutter/flutter/193307 could not be found or the reason was not properly formatted. Begin a comment with 'Reason for revert:' to tell the bot why this issue is being reverted. |
|
These are the failing chromium logs:
where all failed due to:
Starting AGP 9, a module's compileSdk must be >= the |
|
reason for revert: broke the tree |
|
Successfully created revert PR: #193891 |
Reverts: [Updated Most flutter/flutter Defaults to 37](flutter#193307) Initiated by: @jesswrd Reason for reverting: broke the tree Original PR Author: @jesswrd Reviewed By: @mboetger The original PR description is provided below: In this step, we generally update the compileSdk version to 37, the ndkVersion to a compatible version, and any test targets to API 37. We did not bump ndkVersion because the current version is already compatible here. There was also only one outstanding test target that needed to be bumped to API 37. Partially Addresses flutter#189518 ## 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 in-code documentation (doc comments with `///`). - [x] If this PR introduces a new feature or capability, I created and linked a website documentation issue or PR in [flutter/website] (or verified none is needed). - [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. - [ ] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [flutter/website]: https://github.com/flutter/website [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
In this step, we generally update the compileSdk version to 37, the ndkVersion to a compatible version, and any test targets to API 37.
We did not bump ndkVersion because the current version is already compatible here. There was also only one outstanding test target that needed to be bumped to API 37.
Partially Addresses #189518
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.