Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 21 additions & 7 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,11 @@ jobs:
- name: Clone Flutter SDK
run: tools/ci/clone-flutter-sdk

- name: Download Flutter SDK artifacts (flutter precache)
run: flutter precache --universal
Comment on lines -21 to -22
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

git grep precache shows a dangling reference to flutter precache in tools/check.


- name: Download our dependencies (flutter pub get)
run: flutter pub get

- name: Run tools/check
run: TERM=dumb tools/check --no-pub --all --output ci --exclude android
run: TERM=dumb tools/check --no-pub --all --output ci --exclude android --exclude ios

android:
name: "Android build and lint"
Expand All @@ -42,11 +39,28 @@ jobs:
- name: Clone Flutter SDK
run: tools/ci/clone-flutter-sdk

- name: Download Flutter SDK artifacts (flutter precache)
run: flutter precache --universal

- name: Download our dependencies (flutter pub get)
run: flutter pub get

- name: Run tools/check
run: TERM=dumb tools/check --all --output ci android

ios:
name: "iOS build and lint"
runs-on: macos-26
steps:
- uses: actions/checkout@v6

- name: Install dependencies
# Modern Bash and coreutils don't come with macOS; get those.
# (See tools/lib/ensure-shell-deps.sh.)
run: brew install coreutils bash

- name: Clone Flutter SDK
run: tools/ci/clone-flutter-sdk

- name: Download our dependencies (flutter pub get)
run: flutter pub get

- name: Run tools/check
run: TERM=dumb tools/check --no-pub --all --output ci ios
4 changes: 4 additions & 0 deletions ios/Flutter/Release.xcconfig
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
#include? "Pods/Target Support Files/Pods-Runner/Pods-Runner.release.xcconfig"
#include "Generated.xcconfig"
#include "Zulip.xcconfig"

// Compile Swift with "-warnings-as-errors" but only in release builds, so that
// it doesn't get in the way of quick local experiments for debugging.
SWIFT_TREAT_WARNINGS_AS_ERRORS = YES
2 changes: 1 addition & 1 deletion ios/NotificationService/NotificationService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class NotificationService: UNNotificationServiceExtension {
bestAttemptContent.title = improvedNotificationContent.title
bestAttemptContent.subtitle = improvedNotificationContent.subtitle
bestAttemptContent.body = improvedNotificationContent.body
bestAttemptContent.userInfo = improvedNotificationContent.userInfo as [AnyHashable : Any]
bestAttemptContent.userInfo = improvedNotificationContent.userInfo as [AnyHashable: Any]
contentHandler(bestAttemptContent)

case .failure(let error): // TODO(log)
Expand Down
3 changes: 2 additions & 1 deletion ios/Runner/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ import UIKit

let controller = window?.rootViewController as! FlutterViewController

IosNativeHostApiSetup.setUp(binaryMessenger: controller.binaryMessenger, api: IosNativeHostApiImpl())
IosNativeHostApiSetup.setUp(
binaryMessenger: controller.binaryMessenger, api: IosNativeHostApiImpl())

// Retrieve the remote notification payload from launch options;
// this will be null if the launch wasn't triggered by a notification.
Expand Down
79 changes: 77 additions & 2 deletions tools/check
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ default_suites=(
analyze test
flutter_version
build_runner l10n drift pigeon icons
android # This takes multiple minutes in CI, so do it last.
android ios # These take multiple minutes in CI, so do them last.
)

extra_suites=(
Expand Down Expand Up @@ -517,6 +517,78 @@ run_android() {
flutter build appbundle
}

run_ios() {
# Omitted from this check:
# pubspec.{yaml,lock} tools/check
files_check ios/ \
|| return 0

# iOS builds require macOS and Xcode.
if [ "$(uname -s)" != "Darwin" ]; then
echo "Skipping ios suite (not on macOS)."
return 0
fi
Comment on lines +526 to +530
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

In non-verbose mode, I think we'd ideally omit the "Skipping ios suite" and also the "Running ios..." line that precedes it. If omitting "Running ios..." seems complicated, I think it would be better to just log both lines, i.e. cut the if_verbose condition here.


local build_opts=(
${opt_no_pub:+--no-pub}
)

check_no_uncommitted_or_untracked ios/'*'.swift \
|| return

if_verbose echo "Checking for style issues in Swift code..."

# Generated code (*.g.swift) is omitted from this check.
# shellcheck disable=SC2207 # filenames in our own tree, assume well-behaved
local swift_targets=(
$(git ls-files -- ios/'*'.swift :!:ios/'*'.g.swift)
)

# --in-place enables `tools/check --fix ios`; see `check_no_changes`.
# TODO: Once we have a dedicated code formatting check,
# move this swift-format check there.
xcrun swift-format format --in-place \
--configuration ios/.swift-format \
"${swift_targets[@]}" \
|| return
Comment on lines +548 to +553
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# TODO: Once we have a dedicated code formatting check,
# move this swift-format check there.
xcrun swift-format format --in-place \
--configuration ios/.swift-format \
"${swift_targets[@]}" \
|| return
# --in-place enables `tools/check --fix ios`; see `check_no_changes`.
# TODO: Once we have a dedicated code formatting check,
# move this swift-format check there.
xcrun swift-format format --in-place \
--configuration ios/.swift-format \
"${swift_targets[@]}" \
|| return


check_no_changes "changes to Swift files (swift-format)" ios/'*'.swift \
|| return

check_no_uncommitted_or_untracked ios/ macos/ \
|| return

if_verbose echo "Checking for automated changes from Flutter tooling..."

flutter build ios "${build_opts[@]}" --config-only \
|| return
Comment on lines +561 to +564
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Similarly here, about failing right after the log "automated changes from Flutter tooling" when the command that might produce the automated changes hasn't run yet.


# Check macos/ too, because desktop platforms are supported for dev:
# https://github.com/zulip/zulip-flutter#desktop-support
# (This could live in its own suite. If doing that, we'd probably
# still want to run it in the same GitHub CI workflow as the iOS build,
# since GitHub's macOS-based runners use a lot of our free quota:
# https://github.com/zulip/zulip-flutter/issues/329#issue-1950704934
# .)
flutter build macos "${build_opts[@]}" --config-only \
|| return

check_no_changes "changes to ios" ios/ \
|| return
Comment on lines +576 to +577
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: can be clearer that concretely the ios/ directory is meant:

Suggested change
check_no_changes "changes to ios" ios/ \
|| return
check_no_changes "changes to ios/" ios/ \
|| return

Comment on lines +561 to +577
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

check: In iOS, check for automated changes from Flutter tooling

Fixes-partly #329, for iOS.

I see. "Partly" is just because the issue also says flutter build macos --config-only, and we don't do that here; right?

Would the following work, for that?

Suggested change
if_verbose echo "Checking for automated changes from Flutter tooling..."
check_no_uncommitted_or_untracked ios/ \
|| return
flutter build ios "${codesign_opt[@]}" --config-only \
|| return
check_no_changes "changes to ios" ios/ \
|| return
if_verbose echo "Checking for automated changes from Flutter tooling..."
check_no_uncommitted_or_untracked ios/ macos/ \
|| return
flutter build ios "${codesign_opt[@]}" --config-only \
|| return
# Check macos/ too, because desktop platforms are supported for dev:
# https://github.com/zulip/zulip-flutter#desktop-support
# (This could live in its own suite. If doing that, we'd probably
# still want to run it in the same GitHub CI workflow as the iOS build,
# since GitHub's macOS-based runners use a lot of our free quota:
# https://github.com/zulip/zulip-flutter/issues/329#issue-1950704934
# .)
flutter build macos "${codesign_opt[@]}" --config-only \
|| return
check_no_changes "changes to ios" ios/ \
|| return
check_no_changes "changes to macos" macos/ \
|| return


check_no_changes "changes to macos" macos/ \
|| return

if_verbose echo "Building for iOS (in release mode)..."

# Skip codesigning in CI.
if [ "${opt_output}" = ci ]; then
build_opts+=( --no-codesign )
fi

flutter build ipa "${build_opts[@]}"
}

run_shellcheck() {
# Omitted from this check: nothing (nothing known, anyway).
files_check tools/ '!*.'{dart,js,json} \
Expand Down Expand Up @@ -568,7 +640,9 @@ print_header() {

# We avoid `flutter --version` because, weirdly, when run in a
# GitHub Actions step it takes about 30 seconds. (The first time;
# it's fast subsequent times.) That's even after `flutter precache`.
# it's fast subsequent times.) That's even when the Flutter tool's
# binary artifacts have already been downloaded (which, in CI,
# happens during the `flutter pub get` step).
describe_git_head "flutter/flutter" "$(flutter_tree)"

dart --version
Expand All @@ -590,6 +664,7 @@ for suite in "${opt_suites[@]}"; do
pigeon) maybe_time "$suite" run_pigeon ;;
icons) maybe_time "$suite" run_icons ;;
android) maybe_time "$suite" run_android ;;
ios) maybe_time "$suite" run_ios ;;
shellcheck) maybe_time "$suite" run_shellcheck ;;
*) echo >&2 "Internal error: unknown suite $suite" ;;
esac || failed+=( "$suite" )
Expand Down