-
Notifications
You must be signed in to change notification settings - Fork 321
Upgrade Flutter, packages and Android build dependencies #1791
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
Conversation
This upgrade should also include the fix for dart-lang/sdk#61197, which was previously observed by @sm-sayedi, see discussion: https://chat.zulip.org/#narrow/channel/516-mobile-dev-help/topic/New.20Flutter.20upgrade.20degrades.20code.20autocompletion. |
0557a97
to
99d4266
Compare
Thanks, LGTM! Marking for Greg's review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking care of this! Sorry for the delayed review.
These changes all look good — one question below, and a few nits.
pigeon: ^25.3.1 | ||
pigeon: ^26.0.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
Couple of changes for @ProxyApi APIs, which we do not use.
Instead say:
Couple of changes for `@ProxyApi` APIs, which we do not use.
That avoids GitHub interpreting it as an @-mention, which GitHub handles in a spammy way in commit messages:
https://github.com/zulip/zulip-mobile/blob/main/docs/style.md#mentioning-people
https://github.com/flutter/flutter/blob/7920077a5d1e5ef670a987f592bfb6f36dc8a894/docs/contributing/Tree-hygiene.md#using-git
lib/widgets/subscription_list.dart
Outdated
@@ -44,6 +44,9 @@ class _SubscriptionListPageBodyState extends State<SubscriptionListPageBody> wit | |||
}); | |||
} | |||
|
|||
// TODO dart linter incorrectly flags the following regexp string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
// TODO dart linter incorrectly flags the following regexp string | |
// TODO(linter): The linter incorrectly flags the following regexp string |
PODFILE CHECKSUM: 66b7725a92b85e7acc28f0ced0cdacebf18e1997 | ||
PODFILE CHECKSUM: f23347f4ef610d6b07bcd5d9dc9e3ed75fb84721 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: this line doesn't change in the commit that touches ios/Podfile
:
9d1bd9e ios: Bump min iOS version to 15.0, as required by latest Firebase iOS SDK
I think that means this file is inaccurate either before or after that commit.
platform :ios, '14.0' | ||
platform :ios, '15.0' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: let's explain a bit more in the commit message why this upgrade is OK. See the last such update, in a7ed31e.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also went and did a fresh round of those platform-version statistics. So you can link to this message:
#mobile > platform versions @ 💬
firebase_core: ^3.3.0 | ||
firebase_messaging: ^15.0.1 | ||
firebase_core: ^4.0.0 | ||
firebase_messaging: ^16.0.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
deps: Upgrade firebase_core, firebase_messaging to latest
This commit is result of following commands:
That latter line is a lot like how we write summary lines for commits. But here in the commit-message body, it should instead be a normal English sentence:
This commit is the result of the following commands:
More generally: those summary lines are like the headlines of news articles, and the unusual, compressed syntax we use there is basically a form of "headlinese":
https://en.wikipedia.org/wiki/Headline#Headlinese
But then the body of a commit message should be written in prose, as normal English sentences (much like how the body of a news article is written in prose, and not in headlinese).
kotlinVersion=2.1.20 | ||
kotlinVersion=2.2.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deps android: Upgrade Kotlin to 2.2.0, from 2.1.20
Changelog:
https://kotlinlang.org/docs/whatsnew22.html
One item in the "Breaking changes and deprecations" section looks relevant:
- Kotlin 2.2.0 raises the deprecation level of the
kotlinOptions{}
block in Gradle to error. Use thecompilerOptions{}
block instead. For guidance on updating your build scripts, see Migrate fromkotlinOptions{}
tocompilerOptions{}
.
We do have kotlinOptions
blocks. But it looks like that error hasn't appeared in our build. Do you know why?
There's probably also an upstream Flutter issue about this, since kotlinOptions
also appears in the upstream flutter create
templates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like these errors are only shown if the script is in Kotlin DSL script, not Groovy. I migrated build.gradle
to Kotlin DSL for testing and got these errors:
e: file:///Users/rajveer/Projects/zulip-flutter/android/app/build.gradle.kts:86:4: 'val kotlinOptions: KotlinJvmOptions' is deprecated. Please migrate to the compilerOptions DSL. More details are here: https://kotl.in/u1r8ln.
FAILURE: Build failed with an exception.
* Where:
Build file '/Users/rajveer/Projects/zulip-flutter/android/app/build.gradle.kts' line: 28
* What went wrong:
Script compilation errors:
Line 28: kotlinOptions {
^ 'fun BaseAppModuleExtension.kotlinOptions(configure: Action<DeprecatedKotlinJvmOptions>): Unit' is deprecated. Please migrate to the compilerOptions DSL. More details are here: https://kotl.in/u1r8ln.
Line 29: jvmTarget = "1.8"
^ 'var jvmTarget: String' is deprecated. Please migrate to the compilerOptions DSL. More details are here: https://kotl.in/u1r8ln.
Line 86: kotlinOptions.allWarningsAsErrors = name.contains("Release")
^ 'val kotlinOptions: KotlinJvmOptions' is deprecated. Please migrate to the compilerOptions DSL. More details are here: https://kotl.in/u1r8ln.
Line 86: kotlinOptions.allWarningsAsErrors = name.contains("Release")
^ 'var allWarningsAsErrors: Boolean' is deprecated. Please migrate to the compilerOptions DSL. More details are here: https://kotl.in/u1r8ln.
4 errors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahhh, that makes sense, thanks.
I guess migrating our Gradle scripts to Kotlin would be a good thing to do too — it's already happened in the flutter create
templates. But that can be a follow-up PR and a separate issue.
a66b9fa
to
0158bdd
Compare
Thanks for the review @gnprice! Pushed an update, PTAL. |
And update Flutter's supporting libraries to match.
… SDK Firebase iOS SDK version 12.0 bumps minimum supported iOS version to 15.0, see: https://firebase.google.com/support/release-notes/ios#version_1200_-_july_15_2025 firebase/firebase-ios-sdk#14732 So update the min iOS version to 15.0, as we plan to bump our Firebase dependency soon. Also based on the platform-version statistics Greg posted: https://chat.zulip.org/#narrow/channel/48-mobile/topic/platform.20versions/near/2243725 we could bump it to 16.0, but there isn't specific need for that right now, so might as well leave that for later.
This commit is the result of the following commands: flutter pub upgrade --major-versions firebase_messaging firebase_core tools/upgrade pod Changelogs: https://pub.dev/packages/firebase_core/changelog#400 https://pub.dev/packages/firebase_messaging/changelog#1600 Notable changes include bump to Firebase Android BoM (33.16.0 to 34.0.0) and Firebase iOS SDK (11.15.0 to 12.0.0), changelog for those are at: https://firebase.google.com/support/release-notes/android https://firebase.google.com/support/release-notes/ios In those bumps, one notable change for Android SDK is bumping minimum SDK version to 23, but it doesn't affect us, as we already require API 28. Similarly, one notable change on iOS is minimum supported platform version being changed to 15.0 on iOS and 10.15 on macOS.
Changelog: https://pub.dev/packages/pigeon/changelog#2600 Couple of changes for `@ProxyApi` APIs, which we do not use.
Changelog: https://docs.gradle.org/9.0.0/release-notes.html Some highlights: - Configuration Cache improves build performance by caching and reusing the result of the configuration phase. - Embeds the latest stable release of Kotlin 2.2.x runtime and uses Kotlin language version 2.2. - Update to Groovy 4 - Version numbers are expressed as MAJOR.MINOR.PATCH, whereas previous minor releases omitted the patch segment (e.g., 8.5 instead of 8.5.0).
Changelog: https://kotlinlang.org/docs/whatsnew22.html Highlights: * Language features promotion to stable: - Guard conditions in when with a subject. - Non-local break and continue. - Multi-dollar interpolation: improved handling of $ in string literals. * Standard Library; Base64 API and HexFormat API are now Stable. * Otherwise mostly experimental features. There is one breaking change which almost affects us: * In the Gradle DSL, `kotlinOptions` is deprecated in favor of `compilerOptions`. The deprecation actually happened in 2.0.0 as a warning, but is now an error: https://kotlinlang.org/docs/compatibility-guide-22.html#deprecate-kotlinoptions-dsl We're not seeing the error (and haven't seen the warning) because it appears only when the Gradle script is itself written in Kotlin, and ours are still in Groovy. (That's zulip#1803.) So it's no blocker, but it'd be good to switch to `compilerOptions`; filed as zulip#1804.
Release notes: https://developer.android.com/build/releases/past-releases/agp-8-11-0-release-notes#android-gradle-plugin-8.11.0 https://developer.android.com/build/releases/gradle-plugin#android-gradle-plugin-8.12.0 (for 8.12.0, for now) Mostly bug fixes, one notable change is that AGP version 8.12 now requires Android Studio Narwhal Feature Drop (2025.1.2).
Thanks! Merging, with a couple more commit-message edits: 836f1d0 deps: Upgrade firebase_core, firebase_messaging to latest
to fully convert out of headlinese, per #1791 (comment) 277fc97 deps android: Upgrade Kotlin to 2.2.0, from 2.1.20
|
0158bdd
to
c6fcbf6
Compare
Flutter notable commits
flutter/flutter#161460 (comment)
flutter/flutter#161460 (comment)