Skip to content

Update template/test compileSdk, targetSdk, ndk versions #152487

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 27 commits into from
Aug 20, 2024

Conversation

gmackall
Copy link
Member

@gmackall gmackall commented Jul 29, 2024

Updates compileSdk, targetSdk, and ndk versions (former 2 to latest, latter to the version of the ndk we are hosting on CIPD).

Summary of changes:

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jul 29, 2024
@gmackall gmackall marked this pull request as ready for review August 13, 2024 00:03
@gmackall
Copy link
Member Author

Looks like we are hitting https://issuetracker.google.com/issues/342522139. Will need to update all the tests to a newer AGP version.

Unfortunately the gradle/gradle lockfile generation script seems to be broken for the use case of generating the gradle files themselves at this point, so first step will be to fix that.

@github-actions github-actions bot added the d: examples Sample code and demos label Aug 13, 2024
@gmackall gmackall requested a review from a team August 14, 2024 19:50
Copy link
Contributor

@camsim99 camsim99 left a comment

Choose a reason for hiding this comment

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

Looks good! Left some questions I had

@@ -2,4 +2,4 @@ distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-7.6.3-all.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-8.4-all.zip
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you updating to these versions of Gradle and AGP (8.4, 8.1.0) in the tests because of https://docs.gradle.org/current/userguide/compatibility.html#android?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm updating them because of b/342522139 - basically compileSdk 35 seems to require AGP 8.1.0+. The specific choice of Gradle 8.4 was arbitrary though.

Copy link
Member Author

Choose a reason for hiding this comment

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

I also realized that because this changes the FlutterExtension, it would also impact existing apps, so I added a gradle error handler for this case and also added a test for it.

Example of the error message:

Execution failed for task ':app:processReleaseResources'.
> A failure occurred while executing com.android.build.gradle.internal.res.LinkApplicationAndroidResourcesTask$TaskAction
   > Android resource linking failed
     aapt2 E 08-19 15:13:02 78026 5943581 LoadedArsc.cpp:94] RES_TABLE_TYPE_TYPE entry offsets overlap actual entry data.
     aapt2 E 08-19 15:13:02 78026 5943581 ApkAssets.cpp:152] Failed to load resources table in APK '/Users/mackall/Library/Android/sdk/platforms/android-35/android.jar'.
     error: failed to load include path /Users/mackall/Library/Android/sdk/platforms/android-35/android.jar.

And example of the handled version:

┌─ Flutter Fix ──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
│ [!] Using compileSdk 35 requires Android Gradle Plugin (AGP) 8.1.0 or higher.                                                              │
│ Please upgrade to a newer AGP version. The version of AGP that your project uses is likely defined in:                                     │
│ /Users/mackall/development/BugTesting/testFlutterPluginGradlePlugin/foobarfoo/android/settings.gradle,                                     │
│ in the 'plugins' closure.                                                                                                                  │
│ Alternatively, if you have a strong reason to avoid upgrading AGP, you can temporarily lower the compileSdk version in the following file: │
│ /Users/mackall/development/BugTesting/testFlutterPluginGradlePlugin/foobarfoo/android/app/build.gradle                                     │
└────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘

Copy link
Contributor

Choose a reason for hiding this comment

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

This is probably not the right file for most users. This is only the right file since we started using the plugin way of importing FGP.
/Users/mackall/development/BugTesting/testFlutterPluginGradlePlugin/foobarfoo/android/settings.gradle
prior to that it would have been /Users/mackall/development/BugTesting/testFlutterPluginGradlePlugin/foobarfoo/android/build.gradle
See this example from flutter vlc https://github.com/solid-software/flutter_vlc_player/blob/master/flutter_vlc_player/android/build.gradle

Copy link
Member Author

@gmackall gmackall Aug 20, 2024

Choose a reason for hiding this comment

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

Hmm I agree it is the less likely one (hopefully not by much now?), but it is definitely the one we support the most. I can keep telling people both locations for now I suppose, though I feel that three file paths in the message makes it a bit lengthy/gross.

Do you have any sense of when we would stop providing both links in situations like these? A year from now?

Copy link
Member Author

Choose a reason for hiding this comment

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

Example of how it looks now:

┌─ Flutter Fix ─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
│ [!] Using compileSdk 35 requires Android Gradle Plugin (AGP) 8.1.0 or higher.                                                         │
│  Please upgrade to a newer AGP version. The version of AGP that your project uses is likely defined in:                               │
│ /Users/mackall/development/BugTesting/testFlutterPluginGradlePlugin/foobarfoo/android/settings.gradle,                                │
│ in the 'plugins' closure.                                                                                                             │
│  Alternatively, if your project was created with an older version of the templates, it is likely                                      │
│ in the buildscript.dependencies closure of the top-level build.gradle:                                                                │
│ /Users/mackall/development/BugTesting/testFlutterPluginGradlePlugin/foobarfoo/android/build.gradle.                                   │
│                                                                                                                                       │
│  Finally, if you have a strong reason to avoid upgrading AGP, you can temporarily lower the compileSdk version in the following file: │
│ /Users/mackall/development/BugTesting/testFlutterPluginGradlePlugin/foobarfoo/android/app/build.gradle                                │
└───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘

I added the new link, and also added spaces to the start of each separate item which hopefully makes it more readable?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm I agree it is the less likely one (hopefully not by much now?), but it is definitely the one we support the most. I can keep telling people both locations for now I suppose, though I feel that three file paths in the message makes it a bit lengthy/gross.

Do you have any sense of when we would stop providing both links in situations like these? A year from now?

We would stop telling people about both when we have removed the ability to import FGP as a script. Maybe a year from now. Most people have not migrated, my gut say 90+ percent of people. I could do a pub example check if we wanted.

@gmackall gmackall requested a review from camsim99 August 19, 2024 22:19
@gmackall gmackall added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 20, 2024
@auto-submit auto-submit bot merged commit ef9cd32 into flutter:master Aug 20, 2024
140 checks passed
@gmackall gmackall added the revert Autorevert PR (with "Reason for revert:" comment) label Aug 20, 2024
Copy link
Contributor

auto-submit bot commented Aug 20, 2024

A reason for requesting a revert of flutter/flutter/152487 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.

@auto-submit auto-submit bot removed the revert Autorevert PR (with "Reason for revert:" comment) label Aug 20, 2024
@gmackall
Copy link
Member Author

Reason for revert: I forgot that I need to override the compileSdkVersion in the AGP 8.0 instance of this test

@gmackall gmackall added the revert Autorevert PR (with "Reason for revert:" comment) label Aug 20, 2024
auto-submit bot pushed a commit that referenced this pull request Aug 20, 2024
@auto-submit auto-submit bot removed the revert Autorevert PR (with "Reason for revert:" comment) label Aug 20, 2024
auto-submit bot added a commit that referenced this pull request Aug 20, 2024
…ns (#152487)" (#153793)

Reverts: #152487
Initiated by: gmackall
Reason for reverting: I forgot that I need to override the compileSdkVersion in the AGP 8.0 [instance of this test](https://github.com/flutter/flutter/blob/ef9cd32f5a2c0e27300fb9f4662d11da976087d7/dev/devicelab/bin/tasks/android_java17_dependency_smoke_tests.dart#L19)
Original PR Author: gmackall

Reviewed By: {reidbaker}

This change reverts the following previous change:
Updates `compileSdk`, `targetSdk`, and `ndk` versions (former 2 to latest, latter to the version of the ndk we are hosting on CIPD).

Summary of changes:
- Updates mentioned template values
- `compileSdk` 35 requires AGP 8.0+, so updated to 8.1 in many places.
- This also necessitated Gradle upgrades in most places
- This also necessitated moving the `package` xml attribute to the AGP `namespace` field in a couple places (test + template).
- Some tests use the output of `flutter create` but then use intentionally lower AGP versions. [I downgraded the `compileSdk` in these tests.](fee34fd)
- [Stopped lockfile generation](82324a2) script from hitting the `hello_world` example because it uses `.kts` gradle files.
- One test needed [some Gradle options we had already added to templates](6aa187b).
gmackall pushed a commit to gmackall/flutter that referenced this pull request Aug 20, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 21, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 21, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 21, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 21, 2024
auto-submit bot pushed a commit that referenced this pull request Aug 21, 2024
…s" (#153795)

Relands #152487.

The difference is in the two new commits:
-1354c6d
-931788a, short version is that:
- I forgot that I need to override the compileSdkVersion in the AGP 8.0 [instance of this test](https://github.com/flutter/flutter/blob/ef9cd32f5a2c0e27300fb9f4662d11da976087d7/dev/devicelab/bin/tasks/android_java17_dependency_smoke_tests.dart#L19)
- A postsubmit integration test needed new lockfiles + the package attribute -> AGP namespace change.

These were the only two postsubmit failures: [dashboard](https://flutter-dashboard.appspot.com/#/build).
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 21, 2024
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Aug 21, 2024
flutter/flutter@e7da16d...b8f89f7

2024-08-21 [email protected] docs: Update doc for scale behaviour of floating label (flutter/flutter#151835)
2024-08-21 [email protected] Disable Dropdown menu search during keyboard navigation (flutter/flutter#152378)
2024-08-21 [email protected] Add a `bin/flutter-dev` script, for running the `flutter` command-line tool from source (flutter/flutter#153599)
2024-08-21 [email protected] Roll Packages from 4d2d2e3 to 4e5d47e (6 revisions) (flutter/flutter#153848)
2024-08-21 [email protected] Remove unnecessary breaks in default clauses of switch statements (flutter/flutter#153843)
2024-08-21 [email protected] Create Postmortem-Platform-View-android-14-regression (flutter/flutter#149201)
2024-08-21 [email protected] Roll pub packages (flutter/flutter#153838)
2024-08-21 [email protected] Roll pub packages (flutter/flutter#153833)
2024-08-21 [email protected] Add tests for `SingleChildScrollView` examples (flutter/flutter#153548)
2024-08-21 [email protected] Roll Flutter Engine from b1220aa0ebf2 to b94e0097035d (3 revisions) (flutter/flutter#153817)
2024-08-21 [email protected] Roll Flutter Engine from aa1422391cf6 to b1220aa0ebf2 (1 revision) (flutter/flutter#153810)
2024-08-21 [email protected] Roll Flutter Engine from a6508d6557dc to aa1422391cf6 (2 revisions) (flutter/flutter#153808)
2024-08-21 [email protected] Roll Flutter Engine from 5cbf96d0a80f to a6508d6557dc (1 revision) (flutter/flutter#153806)
2024-08-21 [email protected] Roll Flutter Engine from 3d18f65c378a to 5cbf96d0a80f (3 revisions) (flutter/flutter#153804)
2024-08-21 [email protected] `_InteractiveViewerState` code cleanup (flutter/flutter#153645)
2024-08-20 [email protected] Roll Flutter Engine from b80c831e03f7 to 3d18f65c378a (1 revision) (flutter/flutter#153797)
2024-08-20 [email protected] Fix leaky test. (flutter/flutter#153798)
2024-08-20 [email protected] Fix leaky tests. (flutter/flutter#153786)
2024-08-20 [email protected] Roll Flutter Engine from 663176175b4c to b80c831e03f7 (1 revision) (flutter/flutter#153789)
2024-08-20 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Update template/test `compileSdk`, `targetSdk`, `ndk` versions (#152487)" (flutter/flutter#153793)
2024-08-20 [email protected] Update template/test `compileSdk`, `targetSdk`, `ndk` versions (flutter/flutter#152487)
2024-08-20 [email protected] add autofocus to fix a11y issue with dialog (flutter/flutter#152637)
2024-08-20 [email protected] [Release] Update bots to expect new entitlements (flutter/flutter#153787)
2024-08-20 [email protected] Roll Flutter Engine from d6bc4dc6e59d to 663176175b4c (1 revision) (flutter/flutter#153785)
2024-08-20 [email protected] Roll pub packages (flutter/flutter#153740)
2024-08-20 [email protected] Implement tap to scroll to item in CupertinoPicker (flutter/flutter#153386)
2024-08-20 [email protected] Fix leaky test. (flutter/flutter#153780)
2024-08-20 [email protected] Roll Flutter Engine from e10b07598091 to d6bc4dc6e59d (2 revisions) (flutter/flutter#153781)

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],[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
Buchimi pushed a commit to Buchimi/flutter that referenced this pull request Sep 2, 2024
…er#152487)

Updates `compileSdk`, `targetSdk`, and `ndk` versions (former 2 to latest, latter to the version of the ndk we are hosting on CIPD).

Summary of changes:
- Updates mentioned template values
- `compileSdk` 35 requires AGP 8.0+, so updated to 8.1 in many places.
- This also necessitated Gradle upgrades in most places
- This also necessitated moving the `package` xml attribute to the AGP `namespace` field in a couple places (test + template).
- Some tests use the output of `flutter create` but then use intentionally lower AGP versions. [I downgraded the `compileSdk` in these tests.](flutter@fee34fd)
- [Stopped lockfile generation](flutter@82324a2) script from hitting the `hello_world` example because it uses `.kts` gradle files.
- One test needed [some Gradle options we had already added to templates](flutter@6aa187b).
Buchimi pushed a commit to Buchimi/flutter that referenced this pull request Sep 2, 2024
…ns (flutter#152487)" (flutter#153793)

Reverts: flutter#152487
Initiated by: gmackall
Reason for reverting: I forgot that I need to override the compileSdkVersion in the AGP 8.0 [instance of this test](https://github.com/flutter/flutter/blob/ef9cd32f5a2c0e27300fb9f4662d11da976087d7/dev/devicelab/bin/tasks/android_java17_dependency_smoke_tests.dart#L19)
Original PR Author: gmackall

Reviewed By: {reidbaker}

This change reverts the following previous change:
Updates `compileSdk`, `targetSdk`, and `ndk` versions (former 2 to latest, latter to the version of the ndk we are hosting on CIPD).

Summary of changes:
- Updates mentioned template values
- `compileSdk` 35 requires AGP 8.0+, so updated to 8.1 in many places.
- This also necessitated Gradle upgrades in most places
- This also necessitated moving the `package` xml attribute to the AGP `namespace` field in a couple places (test + template).
- Some tests use the output of `flutter create` but then use intentionally lower AGP versions. [I downgraded the `compileSdk` in these tests.](flutter@fee34fd)
- [Stopped lockfile generation](flutter@82324a2) script from hitting the `hello_world` example because it uses `.kts` gradle files.
- One test needed [some Gradle options we had already added to templates](flutter@6aa187b).
Buchimi pushed a commit to Buchimi/flutter that referenced this pull request Sep 2, 2024
…s" (flutter#153795)

Relands flutter#152487.

The difference is in the two new commits:
-flutter@1354c6d
-flutter@931788a, short version is that:
- I forgot that I need to override the compileSdkVersion in the AGP 8.0 [instance of this test](https://github.com/flutter/flutter/blob/ef9cd32f5a2c0e27300fb9f4662d11da976087d7/dev/devicelab/bin/tasks/android_java17_dependency_smoke_tests.dart#L19)
- A postsubmit integration test needed new lockfiles + the package attribute -> AGP namespace change.

These were the only two postsubmit failures: [dashboard](https://flutter-dashboard.appspot.com/#/build).
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 11, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: tests "flutter test", flutter_test, or one of our tests autosubmit Merge PR when tree becomes green via auto submit App d: examples Sample code and demos f: integration_test The flutter/packages/integration_test plugin framework flutter/packages/flutter repository. See also f: labels. tool Affects the "flutter" command-line tool. See also t: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants