From 81363a89a01ab6d63398a9d3412747e5ad1e2556 Mon Sep 17 00:00:00 2001 From: Chris Chua Date: Sat, 23 Mar 2024 09:52:02 +0800 Subject: [PATCH 1/8] check: pin flutter version * Introduce a git submodule to pin the version of flutter SDK under `vendor/flutter`. * Use direnv to add `vendor/flutter/bin` to the current PATH so that subsequent calls to `flutter` on the shell uses the our pinned version of flutter. * Update instructions to use pinned version of flutter. * Pin to 18340ea16c of flutter which matches the current lowerbound version in pubsec.yaml (3.21.0-12.0.pre.26). NOTE: Users can still choose to opt-out and use their own version of flutter. This just provides a recommended and tested version on our CI. Closes #15 --- .envrc | 5 +++ .github/workflows/ci.yml | 20 +++++++--- .gitmodules | 4 ++ .vscode/settings.json | 1 + README.md | 80 +++++++++++++++++++++++++++++++------- analysis_options.yaml | 3 ++ tools/check | 3 +- tools/lib/git.sh | 10 +++++ tools/setup-vendor-flutter | 19 +++++++++ vendor/flutter | 1 + 10 files changed, 125 insertions(+), 21 deletions(-) create mode 100644 .envrc create mode 100644 .gitmodules create mode 100755 tools/setup-vendor-flutter create mode 160000 vendor/flutter diff --git a/.envrc b/.envrc new file mode 100644 index 0000000000..5a6ff78b15 --- /dev/null +++ b/.envrc @@ -0,0 +1,5 @@ +# This file is used by direnv to setup the environment when entering +# to use vendored flutter SDK. + +# Comment out the next line if you want to use your system flutter. +PATH_add vendor/flutter/bin diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f346aa0d82..af35f8eae8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,17 +6,25 @@ jobs: check: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - name: Check out repository + uses: actions/checkout@v3 + # We're not using `with: submodules` here because we can't do a + # depth=1 clone. See below for more details. - - name: Clone Flutter SDK + - name: Checkout flutter submodule # We can't do a depth-1 clone, because we need the most recent tag # so that Flutter knows its version and sees the constraint in our # pubspec is satisfied. It's uncommon for flutter/flutter to go - # more than 100 commits between tags. Fetch 1000 for good measure. + # more than 100 commits between tags. Fetch 1000 for good measure. run: | - git clone --depth=1000 https://github.com/flutter/flutter ~/flutter - TZ=UTC git --git-dir ~/flutter/.git log -1 --format='%h | %ci | %s' --date=iso8601-local - echo ~/flutter/bin >> "$GITHUB_PATH" + git submodule update --init --depth 1000 + + - name: Add vendored flutter to PATH + run: | + echo vendor/flutter/bin >> "$GITHUB_PATH" + + - name: Run setup + run: tools/setup-vendor-flutter - name: Download Flutter SDK artifacts (flutter precache) run: flutter precache --universal diff --git a/.gitmodules b/.gitmodules new file mode 100644 index 0000000000..98627b69dd --- /dev/null +++ b/.gitmodules @@ -0,0 +1,4 @@ +[submodule "vendor/flutter"] + path = vendor/flutter + url = https://github.com/flutter/flutter.git + branch = main diff --git a/.vscode/settings.json b/.vscode/settings.json index 730c9474c9..921dd355e9 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -17,4 +17,5 @@ // This much more focused automatic fix is helpful, though. "files.trimTrailingWhitespace": true, + "dart.flutterSdkPaths": ["vendor/flutter"], } diff --git a/README.md b/README.md index 59e1247129..09dfb7b933 100644 --- a/README.md +++ b/README.md @@ -92,26 +92,75 @@ Two specific points to expand on: ### Setting up -1. Follow the [Flutter installation guide](https://docs.flutter.dev/get-started/install) - for your platform of choice. -2. Switch to the latest version of Flutter by running `flutter channel main` - and `flutter upgrade` (see [Flutter version](#flutter-version) below). -3. Ensure Flutter is correctly configured by running `flutter doctor`. -4. Start the app with `flutter run`, or from your IDE. +1. Setup the pinned version of flutter. + + 1.1. Make sure you have initialized submodule by doing: + + ```sh + # Update and initialize submodule + git submodule update --init + # Run setup-vendor-flutter + tools/setup-vendor-flutter + ``` + + 1.2. Install [direnv][direnv], and set up its hook: + + ```sh + # On Debian or Ubuntu (bash): + sudo apt install direnv && echo 'eval "$(direnv hook bash)"' >>~/.bashrc + + # Or using Homebrew: + brew install direnv + + # Or see: https://direnv.net/docs/installation.html + # Then: https://direnv.net/docs/hook.html + + # After installing and setting up the hook, run this in the zulip-flutter + # directory to allow the .envrc file. + direnv allow + ``` + + 1.3. Follow remaining instructions in [Flutter installation guide](https://docs.flutter.dev/get-started/install) + for your platform of choice. You can skip the Flutter SDK installation as that is now done. + +2. Ensure Flutter is correctly configured by running `flutter doctor`. +3. Start the app with `flutter run`, or from your IDE. + If you're using VSCode, you're set. + If you're using Android Studio, please read the next step. + + 3.1 For Android Studio users only (unless you are using your own + custom version of flutter): + + After you first open, the project, go to: + `Settings -> Languages & Frameworks -> Flutter` + Under `Flutter SDK Path`, enter `/vendor/flutter` (where + `` is the location of the checkout of the repo). ### Flutter version -While in the beta phase, we use the latest Flutter from Flutter's -main branch. Use `flutter channel main` and `flutter upgrade`. +We pin to a particular version of flutter SDK via git submodules in +[vendor/flutter/](vendor/flutter/). +If your local checkout of this repository does not have this submodule +checked out at the same commit, the build may fail. + +However, if you want to manage your own flutter SDK version you can +opt out of this behavior by either of: +- Skip installing direnv or don't do `direnv allow`. +- Comment out the lines in `.envrc`. + +Do note that if you do this, you need to manually make sure that the +flutter SDK version matches or is compatible with the version +indicated by the submodule commit SHA. -We don't pin a specific version, because Flutter itself doesn't offer -a way to do so. So far that hasn't been a problem. When it becomes one, -we'll figure it out; there are several tools for this in the Flutter -community. See [issue #15][]. +Otherwise, the build can fail as we have not tested the current code +with that particular flutter SDK version. -[issue #15]: https://github.com/zulip/zulip-flutter/issues/15 +If you want manage your own flutter SDK version, follow the [Flutter +installation guide](https://docs.flutter.dev/get-started/install) for +your platform of choice. +[direnv]: https://direnv.net/ ### Tests @@ -242,13 +291,16 @@ that's a good prompt to do this. We also do this when there's a new PR merged that we particularly want to take. To update the version bounds: +* First, make sure you're on the `main` channel, run: + `flutter channel main`. * Use `flutter upgrade` to upgrade your local Flutter and Dart. * Update the lower bounds at `environment` in `pubspec.yaml` to the new versions, as seen in `flutter --version`. * Run `flutter pub get`, which will update `pubspec.lock`. * Make a quick check that things work: `tools/check`, and do a quick smoke-test of the app. -* Commit and push the changes in `pubspec.yaml` and `pubspec.lock`. +* Commit and push the changes in the submodule `vendor/flutter`, + `pubspec.yaml` and `pubspec.lock`. ### Upgrading dependencies diff --git a/analysis_options.yaml b/analysis_options.yaml index 2ea8830e38..f578490407 100644 --- a/analysis_options.yaml +++ b/analysis_options.yaml @@ -12,6 +12,9 @@ analyzer: # TODO(pigeon) re-enable lints once clean: https://github.com/flutter/flutter/issues/145633 - lib/host/*.g.dart + # Skip analysis on `vendor/`, which has outside code like the Flutter SDK. + - vendor/** + linter: # For a list of all available lints, with docs, see: # https://dart-lang.github.io/linter/lints/index.html. diff --git a/tools/check b/tools/check index 49df5cdc0a..66f05a2e3e 100755 --- a/tools/check +++ b/tools/check @@ -353,7 +353,7 @@ run_icons() { run_shellcheck() { # Omitted from this check: nothing (nothing known, anyway). - files_check tools/ '!*.'{dart,js,json} \ + files_check tools/ '!*.'{dart,js,json} .envrc \ || return 0 # Shellcheck is fast, <1s; so if we touched any possible targets at all, @@ -362,6 +362,7 @@ run_shellcheck() { targets=( $(git grep -l '#!.*sh\b' -- tools/) $(git ls-files -- tools/'*.sh') + .envrc ) if ! type shellcheck >/dev/null 2>&1; then diff --git a/tools/lib/git.sh b/tools/lib/git.sh index 1fa254e1a4..e4fa101ff6 100644 --- a/tools/lib/git.sh +++ b/tools/lib/git.sh @@ -90,3 +90,13 @@ git_base_commit() { git_changed_files() { git diff --name-only --diff-filter=d "$@" } + +# Note that this assumes ONE submodule. If we ever have more than one, +# we'll need to generalize this. +submodule_is_clean() +{ + # first character of every line status indicates the status of the + # submodule. If there is a space, it means the submodule is clean + # and initiated. + ! git submodule status -- "$@" | grep -q "^[^ ]" +} diff --git a/tools/setup-vendor-flutter b/tools/setup-vendor-flutter new file mode 100755 index 0000000000..dd12cfeda8 --- /dev/null +++ b/tools/setup-vendor-flutter @@ -0,0 +1,19 @@ +#!/usr/bin/env bash +set -euo pipefail + +this_dir=${BASH_SOURCE[0]%/*} + +# shellcheck source=tools/lib/git.sh +. "${this_dir}"/lib/git.sh + + +# One-time setup to let git submodule + flutter SDK know that we're +# using the main channel (main branch). +# We should do a quick check that there's no changes. +if ! submodule_is_clean "vendor/flutter" ; then + echo >&2 "There are changes in 'vendor/flutter' tree or it is not initialized." + echo >&2 "Please run 'git submodule update --init' then run this script again." + exit 1 +fi + +git -C vendor/flutter checkout -B main HEAD diff --git a/vendor/flutter b/vendor/flutter new file mode 160000 index 0000000000..b0198426b5 --- /dev/null +++ b/vendor/flutter @@ -0,0 +1 @@ +Subproject commit b0198426b5ec57572b2ffb94b958bb4528c144e2 From b990e53e3b10654fbf17b92df5e1c23ffd45d4e4 Mon Sep 17 00:00:00 2001 From: Chris Chua Date: Tue, 26 Mar 2024 12:24:43 +0800 Subject: [PATCH 2/8] check: add flutter-main check The `check-flutter-main` job serves 2 purposes: 1. It checks that the code works with the flutter's latest main channel. 2. It checks that our code environment setup works with a typical Flutter SDK installation setup (via ~/flutter). --- .github/workflows/ci-flutter-main.yml | 33 +++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 .github/workflows/ci-flutter-main.yml diff --git a/.github/workflows/ci-flutter-main.yml b/.github/workflows/ci-flutter-main.yml new file mode 100644 index 0000000000..e73c9cc28f --- /dev/null +++ b/.github/workflows/ci-flutter-main.yml @@ -0,0 +1,33 @@ +# The `check-flutter-main` job serves 2 purposes: +# 1. It checks that the code works with the flutter's latest main +# channel. +# 2. It checks that tools/check and our related scripts don't embed +# any hidden assumptions that the Flutter SDK lives at vendor/flutter/. +name: CI + +on: push + +jobs: + check-flutter-main: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + + - name: Clone Flutter SDK + # We can't do a depth-1 clone, because we need the most recent tag + # so that Flutter knows its version and sees the constraint in our + # pubspec is satisfied. It's uncommon for flutter/flutter to go + # more than 100 commits between tags. Fetch 1000 for good measure. + run: | + git clone --depth=1000 https://github.com/flutter/flutter ~/flutter + TZ=UTC git --git-dir ~/flutter/.git log -1 --format='%h | %ci | %s' --date=iso8601-local + echo ~/flutter/bin >> "$GITHUB_PATH" + + - 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 --verbose From 03e305d550cc5e2197a54cbedcf923fc3a305489 Mon Sep 17 00:00:00 2001 From: Chris Chua Date: Sun, 5 May 2024 13:10:00 +0800 Subject: [PATCH 3/8] Update README.md Co-authored-by: Greg Price --- README.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/README.md b/README.md index 09dfb7b933..b8797495e2 100644 --- a/README.md +++ b/README.md @@ -94,12 +94,10 @@ Two specific points to expand on: 1. Setup the pinned version of flutter. - 1.1. Make sure you have initialized submodule by doing: + 1.1. Initialize the pinned Flutter tree: ```sh - # Update and initialize submodule git submodule update --init - # Run setup-vendor-flutter tools/setup-vendor-flutter ``` From 093f5e5b6d88f7a173d78f3226090af06918913d Mon Sep 17 00:00:00 2001 From: Chris Chua Date: Sun, 5 May 2024 13:10:10 +0800 Subject: [PATCH 4/8] Update README.md Co-authored-by: Greg Price --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index b8797495e2..d336a42854 100644 --- a/README.md +++ b/README.md @@ -104,7 +104,7 @@ Two specific points to expand on: 1.2. Install [direnv][direnv], and set up its hook: ```sh - # On Debian or Ubuntu (bash): + # On Debian or Ubuntu (with Bash): sudo apt install direnv && echo 'eval "$(direnv hook bash)"' >>~/.bashrc # Or using Homebrew: From 44f5d2f3cbabcee0bf2ecb3082c6a9234c934ffd Mon Sep 17 00:00:00 2001 From: Chris Chua Date: Sun, 5 May 2024 13:10:25 +0800 Subject: [PATCH 5/8] Update tools/setup-vendor-flutter Co-authored-by: Greg Price --- tools/setup-vendor-flutter | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/setup-vendor-flutter b/tools/setup-vendor-flutter index dd12cfeda8..6f28cccb6d 100755 --- a/tools/setup-vendor-flutter +++ b/tools/setup-vendor-flutter @@ -11,7 +11,7 @@ this_dir=${BASH_SOURCE[0]%/*} # using the main channel (main branch). # We should do a quick check that there's no changes. if ! submodule_is_clean "vendor/flutter" ; then - echo >&2 "There are changes in 'vendor/flutter' tree or it is not initialized." + echo >&2 "There are changes in the 'vendor/flutter' tree or it is not initialized." echo >&2 "Please run 'git submodule update --init' then run this script again." exit 1 fi From 7a97c0284aac113baac558a7a1033a2a29194b6f Mon Sep 17 00:00:00 2001 From: Chris Chua Date: Sun, 5 May 2024 13:10:37 +0800 Subject: [PATCH 6/8] Update README.md Co-authored-by: Greg Price --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index d336a42854..22ac9157d1 100644 --- a/README.md +++ b/README.md @@ -92,7 +92,7 @@ Two specific points to expand on: ### Setting up -1. Setup the pinned version of flutter. +1. Set up the pinned version of Flutter. 1.1. Initialize the pinned Flutter tree: From 60048821d6b6cbe900cd994d05f0b9a603571c0f Mon Sep 17 00:00:00 2001 From: Chris Chua Date: Sun, 5 May 2024 13:47:54 +0800 Subject: [PATCH 7/8] run submmodule update in the script --- README.md | 1 - tools/setup-vendor-flutter | 21 ++++++++++++++++----- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 22ac9157d1..72d3435744 100644 --- a/README.md +++ b/README.md @@ -97,7 +97,6 @@ Two specific points to expand on: 1.1. Initialize the pinned Flutter tree: ```sh - git submodule update --init tools/setup-vendor-flutter ``` diff --git a/tools/setup-vendor-flutter b/tools/setup-vendor-flutter index 6f28cccb6d..71a6ac5762 100755 --- a/tools/setup-vendor-flutter +++ b/tools/setup-vendor-flutter @@ -6,14 +6,25 @@ this_dir=${BASH_SOURCE[0]%/*} # shellcheck source=tools/lib/git.sh . "${this_dir}"/lib/git.sh +divider_line='================================================================' # One-time setup to let git submodule + flutter SDK know that we're # using the main channel (main branch). -# We should do a quick check that there's no changes. +# Otherwise, `flutter doctor` complains about being on a user branch. +# We do a quick check that there's no changes. if ! submodule_is_clean "vendor/flutter" ; then - echo >&2 "There are changes in the 'vendor/flutter' tree or it is not initialized." - echo >&2 "Please run 'git submodule update --init' then run this script again." - exit 1 + echo "Initializing 'vendor/flutter' submodule:" + echo "${divider_line}" + if ! git submodule update --init ; then + echo "${divider_line}" + echo >&2 "Failed to initialize the 'vendor/flutter' submodule, please resolve the issues indicated above then run this script again." + exit 1 + else + echo "${divider_line}" + fi +fi +# Check if current branch is main, if not, switch to main +if [[ $(git -C vendor/flutter branch --show-current) == "main" ]]; then + exit 0 fi - git -C vendor/flutter checkout -B main HEAD From e444099c6bf2612a726baf90edd0bb8693b1da9f Mon Sep 17 00:00:00 2001 From: Chris Chua Date: Sun, 5 May 2024 13:51:32 +0800 Subject: [PATCH 8/8] fix git.sh comment --- tools/lib/git.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tools/lib/git.sh b/tools/lib/git.sh index e4fa101ff6..89dc9f14b1 100644 --- a/tools/lib/git.sh +++ b/tools/lib/git.sh @@ -91,8 +91,7 @@ git_changed_files() { git diff --name-only --diff-filter=d "$@" } -# Note that this assumes ONE submodule. If we ever have more than one, -# we'll need to generalize this. +# Check if provided submodule path is clean. submodule_is_clean() { # first character of every line status indicates the status of the