Upgrade Flutter and packages; hold back firebase_core; record Flutter commit IDs #1117

gnprice · 2024-12-09T06:42:38Z

Fixes #1118.
Fixes #1119.

This upgrades Flutter and some of our other dependencies.

OTOH firebase_core and the other Firebase packages can't immediately be upgraded; I ran into #1116, which I just filed. So hold those back.

And at the start of the branch, I augment tools/upgrade a bit so that it records the actual commit ID in the Flutter tree in a comment in pubspec.yaml, and give tools/check a tiny new suite flutter_version that validates that information.

On several occasions I've wanted the ability to look back and see what Flutter commit was used in a given published (beta) release, in particular. I already make a practice of doing so with the exact minimum version recorded in pubspec.yaml, in order to make it reproducible; but it's somewhat annoying to identify that commit from the version number that normally appears there. So this solves that problem. (… OK, and filed #1118 to describe that.)

Selected commit messages

`f5395d9` tools/upgrade: Record commit ID of Flutter into pubspec.yaml

Fixes #1118, or anyway the main part of it.

Upcoming commits will add a suite in tools/check that validates
this information, in case this line gets updated manually rather
than using this script.

`a940b8a` check: Add suite flutter_version

`72e01d1` check: In flutter_version, verify version bound matches commit

`a29b614` notif: Add vm:entry-point pragma on class, too

Fixes #1119.
[…]

`479c518` deps: Upgrade Flutter to 3.27.0-1.0.pre.744

`b1e7728` deps: Upgrade packages within constraints (tools/upgrade pub)

This required a manual fixup step to update generated files
for the Drift upgrade:
$ tools/check --fix --all-files build_runner drift

gnprice · 2024-12-09T07:15:17Z

CI failed initially but should be fixed now. The fix was adding this commit:
9618f40 check: Fix branch name for clone of Flutter repo in CI

gnprice · 2024-12-09T18:23:49Z

Running with this on my phone since last night, I noticed this morning I hadn't gotten any notifications, even though there were messages that should have caused notifications. Debugging found #1119, and I've added a commit to fix it.

chrisbobbe

Thanks! ~~LGTM with one small question below.~~ Seeing some iOS build issues actually; investingating.

chrisbobbe · 2024-12-09T20:03:16Z

tools/check

+    # Check the commit is an acceptable commit.
+    local commit_count
+    commit_count=$(
+        "${flutter_git[@]}" rev-list --count origin/main.."${flutter_commit}"


I've named the remote "upstream" instead of "origin", so "origin/main" doesn't work for me (here and below). I've named it back to "origin" for testing this new code; should I keep it that way or should the code adapt to handle the "upstream" name?

I'd suggest just switching back to "origin". It'd be possible to make this handle both conventions, but would make the logic more complicated.

I don't think the Flutter install steps do anything to override Git's default here, so most contributors will have "origin".

(Personally I switched back from using "upstream" to using "origin" a little while ago, for Git repos in general, partly just because I find "origin" is quicker for me to reliably type — "upstream" has long runs on the same hand.)

chrisbobbe · 2024-12-09T20:20:06Z

Oh I did notice an auto-generated change from flutter run:

diff --git ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme
index 5e31d3d34..c53e2b314 100644
--- ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme
+++ ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme
@@ -48,6 +48,7 @@
       ignoresPersistentStateOnLaunch = "NO"
       debugDocumentVersioning = "YES"
       debugServiceExtension = "internal"
+      enableGPUValidationMode = "1"
       allowLocationSimulation = "YES">
       <BuildableProductRunnable
          runnableDebuggingMode = "0">

Hmm, seeing iOS build issues actually—investigating.

chrisbobbe · 2024-12-09T20:34:32Z

Hmm, seeing some iOS build issues actually—please hold off merging while I investigate. Here is the output:

Launching lib/main.dart on iPhone 15 Pro in debug mode...
Running pod install...
Running Xcode build...
Xcode build done.                                           30.5s
Failed to build iOS app
Semantic Issue (Xcode): Duplicate interface definition for class 'Sqlite3FlutterLibsPlugin'
/Users/chrisbobbe/dev/zulip-flutter/build/ios/Debug-iphonesimulator/sqlite3_flutter_libs/sqlite3_flutter_libs.framework/Headers/Sqlite3FlutterLibsPlugin.h:2:0

Modules Issue (Xcode): Definition of 'Sqlite3FlutterLibsPlugin' must be imported from module 'sqlite3_flutter_libs.Swift' before it is required
/Users/chrisbobbe/dev/zulip-flutter/ios/Runner/GeneratedPluginRegistrant.m:105:3

Could not build the application for the simulator.
Error launching application on iPhone 15 Pro.

chrisbobbe · 2024-12-09T20:41:51Z

Hmm, the build succeeded just now when I requested it from the Xcode gui. The output above is from flutter run as triggered from the "Run 'main.dart'" button (with a green "play" icon) in Android Studio.

chrisbobbe · 2024-12-09T20:44:49Z

The output above is from flutter run as triggered from the "Run 'main.dart'" button (with a green "play" icon) in Android Studio.

OK, and it succeeds for me now after running flutter clean. 🤷‍♂️ Please merge at will, then.

gnprice · 2024-12-09T20:56:18Z

Thanks for the review! Evidently I had not tried flutter run for iOS. << >>

I reproduce that build failure, and also reproduce that flutter clean resolves it — good find.

Before flutter clean, I tried flutter build ipa, prompted by thinking of #773. That actually succeeded. More data points FTR though I'm not sure they'll be useful:

flutter run --release succeeded too
but OTOH flutter build ipa --debug still succeeded, so wouldn't have been a way to detect the issue.

gnprice · 2024-12-09T20:58:19Z

Merging, after adding this:

Oh I did notice an auto-generated change from flutter run:

There was also a similar change in macos/ after flutter run for that. I'd noticed that change yesterday — I was experimenting with the macOS target to try to resolve #1116 and unblock upgrading Firebase — but had assumed it was an existing discrepancy that we just hadn't noticed because we don't use that target as often.

Fixes zulip#1118, or anyway the main part of it. Upcoming commits will add a suite in tools/check that validates this information, in case this line gets updated manually rather than using this script.

Future runs of `tools/upgrade` will keep this up to date.

Otherwise the clone doesn't know about the branch name `main`, which would confuse the logic in a check we're about to add.

Fixes zulip#1119. With Flutter from main, in release mode, the app would no longer show notifications on Android when it was in the background. This error message appears in logcat: ``` DartVM E ERROR: To access 'package:zulip/notifications/receive.dart::NotificationService' from native code, it must be annotated. ERROR: See https://github.com/dart-lang/sdk/blob/master/runtime/docs/compiler/aot/entry_point_pragma.md ``` It looks like the relevant upstream commit is probably this one: dart-lang/sdk@9b06e26 AFAICT those docs don't actually say that we should need to annotate the class. We already do annotate the static *method* on this class which we ask native code to call, namely `_onBackgroundMessage`. At least the error message is nice and clear, though.

And apply auto-upgrades that appear after `flutter run` for iOS and macOS respectively.

The upgrade to the Dart package firebase_core 3.4.0 (from 3.3.0) leads to the CocoaPods pod Firebase/CoreOnly 11.0.0 (from 10.29.0) and the further pod Firebase/Messaging 11.0.0 (from 10.29.0). That produces the following `pod update` error message when attempting to run `tools/upgrade pub`: ``` + pod update --project-directory=macos/ Update all pods Updating local specs repositories Analyzing dependencies firebase_core: Using Firebase SDK version '11.0.0' defined in 'firebase_core' firebase_messaging: Using Firebase SDK version '11.0.0' defined in 'firebase_core' [!] CocoaPods could not find compatible versions for pod "Firebase/Messaging": In Podfile: firebase_messaging (from `Flutter/ephemeral/.symlinks/plugins/firebase_messaging/macos`) was resolved to 15.1.0, which depends on Firebase/Messaging (~> 11.0.0) Specs satisfying the `Firebase/Messaging (~> 11.0.0)` dependency were found, but they required a higher minimum deployment target. ``` Fundamentally that should be fixable, but with some effort just now I wasn't able to actually increase that minimum deployment target in a way that satisfied CocoaPods so as to resolve this error. So, filed zulip#1116 for that. Meanwhile, let our other upgrades proceed.

This required a manual fixup step to update generated files for the Drift upgrade: $ tools/check --fix --all-files build_runner drift

This has been addressed in Greg's upstream PR: simolus3/drift#3022 which was pulled in by the Drift upgrade in 8b564e4 (zulip#1117). Signed-off-by: Zixuan James Li <[email protected]>

gnprice added the maintainer review PR ready for review by Zulip maintainers label Dec 9, 2024

gnprice force-pushed the pr-deps branch 2 times, most recently from a24c425 to a278119 Compare December 9, 2024 07:13

gnprice force-pushed the pr-deps branch from a278119 to 62d74f5 Compare December 9, 2024 18:21

gnprice changed the title ~~Upgrade Flutter and deps; hold back firebase_core; record Flutter commit IDs~~ Upgrade Flutter and packages; hold back firebase_core; record Flutter commit IDs Dec 9, 2024

chrisbobbe previously approved these changes Dec 9, 2024

View reviewed changes

chrisbobbe self-requested a review December 9, 2024 20:32

gnprice mentioned this pull request Dec 9, 2024

Run iOS build in CI #773

Open

gnprice force-pushed the pr-deps branch from 62d74f5 to 8d97031 Compare December 9, 2024 20:52

gnprice added 11 commits December 9, 2024 12:58

tools [nfc]: Factor out a function to find the Flutter tree

e6932fb

tools/upgrade: Record commit ID of Flutter into pubspec.yaml

7b4312a

Fixes zulip#1118, or anyway the main part of it. Upcoming commits will add a suite in tools/check that validates this information, in case this line gets updated manually rather than using this script.

deps [nfc]: Record current Flutter commit ID

ba50fec

Future runs of `tools/upgrade` will keep this up to date.

check: Fix branch name for clone of Flutter repo in CI

a6e5804

Otherwise the clone doesn't know about the branch name `main`, which would confuse the logic in a check we're about to add.

check: Add suite flutter_version

cdd48d6

check: In flutter_version, verify version bound matches commit

094073b

deps: Upgrade Flutter to 3.27.0-1.0.pre.744

47ba54e

And apply auto-upgrades that appear after `flutter run` for iOS and macOS respectively.

deps: Update CocoaPods pods (tools/upgrade pod)

8c069cd

deps: Upgrade packages within constraints (tools/upgrade pub)

8b564e4

This required a manual fixup step to update generated files for the Drift upgrade: $ tools/check --fix --all-files build_runner drift

gnprice force-pushed the pr-deps branch from 8d97031 to 8b564e4 Compare December 9, 2024 20:59

gnprice merged commit 8b564e4 into zulip:main Dec 9, 2024
1 check passed

gnprice deleted the pr-deps branch December 9, 2024 21:03

gnprice mentioned this pull request Dec 19, 2024

db [nfc]: Remove handwritten copy of copyWithCompanion #1166

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade Flutter and packages; hold back firebase_core; record Flutter commit IDs #1117

Upgrade Flutter and packages; hold back firebase_core; record Flutter commit IDs #1117

gnprice commented Dec 9, 2024 •

edited

Loading

gnprice commented Dec 9, 2024

gnprice commented Dec 9, 2024

chrisbobbe left a comment •

edited

Loading

chrisbobbe Dec 9, 2024

gnprice Dec 9, 2024

gnprice Dec 9, 2024

chrisbobbe commented Dec 9, 2024 •

edited

Loading

chrisbobbe commented Dec 9, 2024

chrisbobbe commented Dec 9, 2024

chrisbobbe commented Dec 9, 2024

gnprice commented Dec 9, 2024

gnprice commented Dec 9, 2024

Upgrade Flutter and packages; hold back firebase_core; record Flutter commit IDs #1117

Upgrade Flutter and packages; hold back firebase_core; record Flutter commit IDs #1117

Conversation

gnprice commented Dec 9, 2024 • edited Loading

Selected commit messages

f5395d9 tools/upgrade: Record commit ID of Flutter into pubspec.yaml

a940b8a check: Add suite flutter_version

72e01d1 check: In flutter_version, verify version bound matches commit

a29b614 notif: Add vm:entry-point pragma on class, too

479c518 deps: Upgrade Flutter to 3.27.0-1.0.pre.744

b1e7728 deps: Upgrade packages within constraints (tools/upgrade pub)

gnprice commented Dec 9, 2024

gnprice commented Dec 9, 2024

chrisbobbe left a comment • edited Loading

Choose a reason for hiding this comment

chrisbobbe Dec 9, 2024

Choose a reason for hiding this comment

gnprice Dec 9, 2024

Choose a reason for hiding this comment

gnprice Dec 9, 2024

Choose a reason for hiding this comment

chrisbobbe commented Dec 9, 2024 • edited Loading

chrisbobbe commented Dec 9, 2024

chrisbobbe commented Dec 9, 2024

chrisbobbe commented Dec 9, 2024

gnprice commented Dec 9, 2024

gnprice commented Dec 9, 2024

gnprice commented Dec 9, 2024 •

edited

Loading

`f5395d9` tools/upgrade: Record commit ID of Flutter into pubspec.yaml

`a940b8a` check: Add suite flutter_version

`72e01d1` check: In flutter_version, verify version bound matches commit

`a29b614` notif: Add vm:entry-point pragma on class, too

`479c518` deps: Upgrade Flutter to 3.27.0-1.0.pre.744

`b1e7728` deps: Upgrade packages within constraints (tools/upgrade pub)

chrisbobbe left a comment •

edited

Loading

chrisbobbe commented Dec 9, 2024 •

edited

Loading