[embedder] Reorganize documentation. - Split Flutter Engine workflow into a separate doc to make it more clear that these steps are not required for most changes. - Rewrite Flutter Engine workflow docs to explain how to test local changes to the Flutter Engine. - Move git workflow into a separate doc because it's noisy. - Document how updating our dependencies works and why it's necessary. Change-Id: I5793acc47bab0792a62e552b65f2f6b8d3442803 Reviewed-on: https://fuchsia-review.googlesource.com/c/flutter-embedder/+/736504 Reviewed-by: Ben Bergkamp <benbergkamp@google.com>
diff --git a/README.md b/README.md index cf71c32..7adc58e 100644 --- a/README.md +++ b/README.md
@@ -94,130 +94,18 @@ instead of rendering. Re-running the app usually fixes it. We need to fix this.** -## Syncing engine artifacts +## Making Flutter Engine changes -Occasionally you will need to update the Flutter Engine artifacts (`embedder.h` and -`libengine_flutter.so`) that are used by the embedder to run Flutter apps. A script -is provided for this workflow. +Running the Flutter app is handled by the Flutter Engine's +[embedder platform code](https://github.com/flutter/engine/tree/main/shell/platform/embedder). +This code is compiled into a shared library called `libflutter_engine.so`, +which we can interact with using the `embedder.h` header. -### Requirements +If you need to make changes to this Flutter Engine code (for example for +testing a new API in `embedder.h` with this embedder) see +[_`making_engine_changes.md`_ for instructions](https://fuchsia.googlesource.com/flutter-embedder/+/refs/heads/main/docs/making_engine_changes.md). +This isn't required for changes to only flutter-embedder.git's source code. -1. You will - [need to get the Flutter Engine source code](https://github.com/flutter/flutter/wiki/Setting-up-the-Engine-development-environment). - **Note that this is not just cloning https://github.com/flutter/engine.** +## Uploading changes for review. -2. Set `$ENGINE_DIR` to the `src` folder of your Flutter Engine checkout location, - for example `~/engine/src`. - -3. You will need to install - [`depot_tools`](https://commondatastorage.googleapis.com/chrome-infra-docs/flat/depot_tools/docs/html/depot_tools_tutorial.html#_setting_up) - and add it to your `PATH`. - - ```sh - export PATH=$HOME/depot_tools:$PATH - ``` - -4. For `sync_engine_artifacts_to_revision.sh`, you will need to `git stash` or `git commit` any local - changes to the Flutter Engine. This is not necesssary for `build_and_copy_engine_artifacts.sh`. - -### Running the script - -You can run the script with a `flutter/engine` Git commit to sync the Engine artifacts to -that revision. - -```sh -$FUCHSIA_EMBEDDER_DIR/scripts/sync_engine_artifacts_to_revision.sh <ENGINE_COMMIT_SHA> -``` - -A common workflow is to sync your Engine commit to the Flutter tool in this repository, -for example when updating the Flutter tool to a new version. To do this: - -```sh -$FUCHSIA_EMBEDDER_DIR/scripts/sync_engine_artifacts_to_revision.sh $(cat $FUCHSIA_EMBEDDER_DIR/third_party/dart-pkg/internal/flutter/flutter/bin/internal/engine.version) -``` - -If you want to test local changes to the Engine, you can run `build_and_copy_engine_artifacts.sh` -instead, which simply builds and copies your Engine into `$FUCHSIA_EMBEDDER_DIR`. - -```sh -$FUCHSIA_EMBEDDER_DIR/scripts/build_and_copy_engine_artifacts.sh -``` - -## Uploading changes for review - -flutter-embedder reviews are handled using Gerrit at https://fuchsia-review.googlesource.com, which uses a single commit per -code review. See -[this guide](https://fuchsia.dev/fuchsia-src/development/source_code/contribute_changes) -for an intro to Gerrit. - -An alternative Gerrit workflow is a branchless workflow where you do all your work -from `origin/main` without creating new branches. We describe such a workflow below, -but it's not mandatory, it's just provided as an example. - -An example branchless workflow for creating a new change is: - -```sh -# Start work from an up-to-date main branch. -git fetch origin -git checkout origin/main -# Modify some files... -... -# Stage and commit your changes. -git add file1.cc file2.h -git commit -m "[embedder] My cool new feature." -# Upload for review. -git push origin HEAD:refs/for/main -``` - -An example branchless workflow to apply feedback during a code review is: - -1. Visit https://fuchsia-review.googlesource.com and find your change. -2. Select `... > Download patch > Checkout` and copy the command. -3. Run the following commands: - - ```sh - # Start from main with your change on top. - <paste the command you copied above> - # Make some changes to some files to apply the feedback... - ... - # Stage and amend your changes to your commit. - git add file1.cc file2.h - git commit --amend - # Upload your updated change for review. - git push origin HEAD:refs/for/main - ``` - -An example branchless workflow to create a chain of changes is: - -```sh -# Start work from an up-to-date main branch. -git fetch origin -git checkout origin/main -# Modify some files... -... -# Stage and commit your changes. -git add file1.cc file2.h -git commit -m "[embedder] My cool new feature 1." -# Upload for review. -git push origin HEAD:refs/for/main -# Do some more work on top of your cool new feature. -git add file3.cc file4.h -git commit -m "[embedder] My cool new feature 2." -# Create a second code review for your second feature. -# The second code review will only show changes for -# your second feature and it will indicate that it is based on -# top of the first feature. -git push origin HEAD:refs/for/main -``` - -A branchless workflow comes with some benefits: - -1. It reduces the amount of local branches you have to track. -2. It reduces the amount of local branches you have to clean up after submission. -3. It reduces the amount of overhead to starting new work (just `git checkout origin/main`). - -However a branchless workflow also prevents you from storing multiple -chains of commits locally. Uploading your changes to https://fuchsia-review.googlesource.com -frequently can help address that problem, as https://fuchsia-review.googlesource.com then -acts as the storage for each of your ongoing changes. This also keeps your changes in a -state where they're easy to share and hand off to other people. +See [_`git_workflow.md`_](https://fuchsia.googlesource.com/flutter-embedder/+/refs/heads/main/docs/git_workflow.md). \ No newline at end of file
diff --git a/WORKSPACE.bazel b/WORKSPACE.bazel index a4da427..c1d4a1f 100644 --- a/WORKSPACE.bazel +++ b/WORKSPACE.bazel
@@ -1,4 +1,3 @@ - # Copyright 2022 The Fuchsia Authors. All rights reserved. # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. @@ -8,18 +7,20 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") http_archive( - name = "bazel_skylib", - urls = [ - "https://mirror.bazel.build/github.com/bazelbuild/bazel-skylib/releases/download/1.2.1/bazel-skylib-1.2.1.tar.gz", - "https://github.com/bazelbuild/bazel-skylib/releases/download/1.2.1/bazel-skylib-1.2.1.tar.gz", - ], - sha256 = "f7be3474d42aae265405a592bb7da8e171919d74c16f082a5457840f06054728", - ) + name = "bazel_skylib", + sha256 = "f7be3474d42aae265405a592bb7da8e171919d74c16f082a5457840f06054728", + urls = [ + "https://mirror.bazel.build/github.com/bazelbuild/bazel-skylib/releases/download/1.2.1/bazel-skylib-1.2.1.tar.gz", + "https://github.com/bazelbuild/bazel-skylib/releases/download/1.2.1/bazel-skylib-1.2.1.tar.gz", + ], +) load("@bazel_skylib//:workspace.bzl", "bazel_skylib_workspace") + bazel_skylib_workspace() load("@bazel_skylib//lib:versions.bzl", "versions") + versions.check(minimum_bazel_version = "5.1.0") local_repository( @@ -36,19 +37,8 @@ rules_fuchsia_deps() -# To update the Fuchsia SDK, run `$FUCHSIA_EMBEDDER_DIR/scripts/update_dependencies.sh`, -# which will update the Flutter Framework and Flutter Engine along with the -# Embedder's Fuchsia SDK to try to keep the Fuchsia SDK version in sync between the -# Embedder and Engine repos. -# -# If you only want to update the Fuchsia SDK version by itself, run -# `$FUCHSIA_EMBEDDER_DIR/scripts/update_fuchsia_sdk.sh`. This may lead to build issues -# from mismatches between the Flutter Engine and Flutter Embedder repos. -# -# After updating the SDK, you should restart any running emulators and package servers -# and update your product bundles (`ffx product-bundle get workstation_eng.qemu-x64`) as -# old images may be incompatible with the updated SDK. To do this automatically, run: -# `$FUCHSIA_EMBEDDER_DIR/scripts/update_dependencies.sh --cleanup` +# To update the Fuchsia SDK, see +# https://fuchsia.googlesource.com/flutter-embedder/+/refs/heads/main/docs/updating_dependencies.md. fuchsia_sdk_repository( name = "fuchsia_sdk", )
diff --git a/docs/git_workflow.md b/docs/git_workflow.md new file mode 100644 index 0000000..47c09ee --- /dev/null +++ b/docs/git_workflow.md
@@ -0,0 +1,80 @@ + # Uploading changes for review + +flutter-embedder reviews are handled using Gerrit at https://fuchsia-review.googlesource.com, which uses a single commit per +code review. See +[this guide](https://fuchsia.dev/fuchsia-src/development/source_code/contribute_changes) +for an intro to Gerrit. + +## Branchless workflow + +An alternative Gerrit workflow is a branchless workflow where you do all your work +from `origin/main` without creating new branches. We describe such a workflow below, +but it's not mandatory, it's just provided as an example. + +An example branchless workflow for creating a new change is: + +```sh +# Start work from an up-to-date main branch. +git fetch origin +git checkout origin/main +# Modify some files... +... +# Stage and commit your changes. +git add file1.cc file2.h +git commit -m "[embedder] My cool new feature." +# Upload for review. +git push origin HEAD:refs/for/main +``` + +An example branchless workflow to apply feedback during a code review is: + +1. Visit https://fuchsia-review.googlesource.com and find your change. +2. Select `... > Download patch > Checkout` and copy the command. +3. Run the following commands: + + ```sh + # Start from main with your change on top. + <paste the command you copied above> + # Make some changes to some files to apply the feedback... + ... + # Stage and amend your changes to your commit. + git add file1.cc file2.h + git commit --amend + # Upload your updated change for review. + git push origin HEAD:refs/for/main + ``` + +An example branchless workflow to create a chain of changes is: + +```sh +# Start work from an up-to-date main branch. +git fetch origin +git checkout origin/main +# Modify some files... +... +# Stage and commit your changes. +git add file1.cc file2.h +git commit -m "[embedder] My cool new feature 1." +# Upload for review. +git push origin HEAD:refs/for/main +# Do some more work on top of your cool new feature. +git add file3.cc file4.h +git commit -m "[embedder] My cool new feature 2." +# Create a second code review for your second feature. +# The second code review will only show changes for +# your second feature and it will indicate that it is based on +# top of the first feature. +git push origin HEAD:refs/for/main +``` + +A branchless workflow comes with some benefits: + +1. It reduces the amount of local branches you have to track. +2. It reduces the amount of local branches you have to clean up after submission. +3. It reduces the amount of overhead to starting new work (just `git checkout origin/main`). + +However a branchless workflow also prevents you from storing multiple +chains of commits locally. Uploading your changes to https://fuchsia-review.googlesource.com +frequently can help address that problem, as https://fuchsia-review.googlesource.com then +acts as the storage for each of your ongoing changes. This also keeps your changes in a +state where they're easy to share and hand off to other people.
diff --git a/docs/making_engine_changes.md b/docs/making_engine_changes.md new file mode 100644 index 0000000..6465bcd --- /dev/null +++ b/docs/making_engine_changes.md
@@ -0,0 +1,107 @@ +# Making Flutter Engine changes + +Running Flutter apps is handled by the Flutter Engine's +[embedder platform](https://github.com/flutter/engine/tree/main/shell/platform/embedder). +This code is compiled into a shared library called `libflutter_engine.so`, +which we interact with using the `embedder.h` header. + +## Updating Flutter Engine + +If you simply want to pull a newer version of Flutter Engine into this repo with no +local changes, see [_`updating_dependencies.md`_](https://fuchsia.googlesource.com/flutter-embedder/+/refs/heads/main/docs/updating_dependencies.md). + +## Testing local Flutter Engine changes + +This section describes a workflow for making local changes to the Flutter Engine +embedder platform and testing those changes in flutter-embedder.git. + +### Requirements + +1. You will + [need to get the Flutter Engine source code](https://github.com/flutter/flutter/wiki/Setting-up-the-Engine-development-environment). + **Note that this is not just cloning https://github.com/flutter/engine.** + +2. Set `$ENGINE_DIR` to the `src` folder of your Flutter Engine checkout location. For example + for `zsh` add this line to your `.zprofile`: + + ```sh + export ENGINE_DIR=$HOME/engine/src + ``` + +3. You will need to install + [`depot_tools`](https://commondatastorage.googleapis.com/chrome-infra-docs/flat/depot_tools/docs/html/depot_tools_tutorial.html#_setting_up) + and add it to your `PATH`. For example for `zsh` add this line to your `.zprofile`: + + ```sh + export PATH=$HOME/depot_tools:$PATH + ``` + +### Workflow + +There are two caveats to building Flutter Engine artifacts for this repository: + +- The Dart SDK used by the Flutter Engine artifacts needs to match the Dart SDK + used by the Flutter Framework to run examples. + +- The Flutter Engine artifacts are currently built with + [several experimental patches that have not been submitted yet](https://fuchsia.googlesource.com/flutter-embedder/+/refs/heads/main/scripts/sync_engine_artifacts_to_revision.sh#72). + +To address these caveats, first: + +1. `git commit` or `git stash` any local changes to your Flutter Engine source code. + + ```sh + git -C $ENGINE_DIR/flutter stash + ``` + +2. Sync your Flutter Engine source code to Flutter Framework's version of Flutter Engine. + This: + + - Ensures that the Dart SDK in your Flutter Engine artifacts is the same Dart SDK + that is used to run examples in this repository. + + - Applies the experimental patches to your Flutter Engine source code. + + ```sh + $FUCHSIA_EMBEDDER_DIR/scripts/sync_engine_artifacts_to_revision.sh $(cat $FUCHSIA_EMBEDDER_DIR/third_party/dart-pkg/internal/flutter/flutter/bin/internal/engine.version) + ``` + +You can then build Flutter Engine artifacts for this repository with your local changes: + +3. Make any local changes you'd like to test to the Flutter Engine code in `$ENGINE_DIR`. + For example if you stashed some changes earlier: + + ```sh + git -C $ENGINE_DIR/flutter stash apply + ``` + +4. Build and copy your Flutter Engine changes over to this repository. + + ```sh + $FUCHSIA_EMBEDDER_DIR/scripts/build_and_copy_engine_artifacts.sh + ``` + + You can re-run this script multiple times to iterate on your Flutter Engine + changes. + +5. [Run an example app](https://fuchsia.googlesource.com/flutter-embedder/+/refs/heads/main/README.md#run-an-example-app) + to test your changes. + +### Landing your local Flutter Engine changes + +Once you've verified that your local Flutter Engine change works, you have two options to land your +Flutter Engine change in flutter-embedder.git: + +1. Submit a pull request to the Flutter Engine and then update the version of Flutter Engine used by + this repository by following + [_`updating_dependencies.md`_](https://fuchsia.googlesource.com/flutter-embedder/+/refs/heads/main/docs/updating_dependencies.md). + +2. Add your commit to the list of experimental unsubmitted patches in + [`sync_engine_artifacts_to_revision.sh`](https://fuchsia.googlesource.com/flutter-embedder/+/refs/heads/main/scripts/sync_engine_artifacts_to_revision.sh#72) + and commit your changes to `//src/embedder/engine` (i.e. commit the updated Flutter Engine artifacts). + This is a useful tool to unblock while we continue to verify the change in flutter-embedder.git + but you will be expected to submit the change in Flutter Engine later as this blocks production. + + - This is useful when making changes to Flutter Engine's `embedder.h` header as these changes cannot be reverted + post-submission due to [embedder.h's ABI compatibility requirements](https://fuchsia.googlesource.com/flutter-embedder/+/refs/heads/main/src/embedder/engine/embedder.h#18). + It is good to be absolutely sure that the change is necessary and correct before submission. \ No newline at end of file
diff --git a/docs/updating_dependencies.md b/docs/updating_dependencies.md new file mode 100644 index 0000000..509f033 --- /dev/null +++ b/docs/updating_dependencies.md
@@ -0,0 +1,42 @@ +# Updating flutter-embedder.git's dependencies + +Running an example in flutter-embedder.git has two sensitive dependencies: + +1. The **Fuchsia SDK**, which is used by flutter-embedder.git, Flutter Engine and + the Dart SDK. + + - The Fuchsia SDK used by flutter-embedder.git must be around the + same version as the Fuchsia SDK used by flutter-embedder.git's Flutter Engine + artifacts, which come from [a separate repository](https://github.com/flutter/engine), + or compile-time errors can occur from failing to link symbols. + +2. The **Dart SDK**, which is used by Flutter Engine and the Flutter Framework. + + - A Flutter app must be built using the same Dart SDK as the Flutter Engine, or a crash will + occur on startup. + +As a result, we update the flutter-embedder.git's dependencies on the Fuchsia SDK, Flutter Framework +and Flutter Engine at the same time to keep: + +1. flutter-embedder.git and Flutter Engine using roughly the same version of the Fuchsia SDK, and + +2. Flutter Engine and Flutter Framework using the exact same version of the Dart SDK. + +This can be done by running the following command: + +```sh +$FUCHSIA_EMBEDDER_DIR/scripts/update_dependencies.sh +``` + +After updating the SDK, you should update your product bundle (`ffx product-bundle get workstation_eng.qemu-x64`) +and restart any running emulators and package servers as old images may be incompatible +with the updated SDK. To do this automatically, run: + +```sh +$FUCHSIA_EMBEDDER_DIR/scripts/update_dependencies.sh --cleanup +``` + +## Future improvements + +`update_dependencies.sh` should ensure that flutter-embedder.git and Flutter Engine +use the exact same version of the Fuchsia SDK instead of roughly the same version. \ No newline at end of file
diff --git a/src/embedder/engine/README.md b/src/embedder/engine/README.md index ef32d32..b141b29 100644 --- a/src/embedder/engine/README.md +++ b/src/embedder/engine/README.md
@@ -2,9 +2,10 @@ This folder contains artifacts from the [Flutter Engine repository](https://github.com/flutter/engine). -## Updating the artifacts +## Making changes to these artifacts -See ["Syncing Engine Artifacts" in the top-level `README.md`](https://fuchsia.googlesource.com/flutter-embedder#syncing-engine-artifacts). +See +["Making Flutter Engine changes"](https://fuchsia.googlesource.com/flutter-embedder/+/refs/heads/main/docs/making_engine_changes.md). ## Code breakdown @@ -13,12 +14,8 @@ These are builds of the Flutter embedder platform for `OS_FUCHSIA` with `--runtime-mode=<runtime_mode>` and `--cpu=<arch>`. These builds contain a few changes that have not been landed into the -Flutter Engine repository: - -1. [Add `-fPIC` to Fuchsia builds](https://github.com/flutter/buildroot/pull/552). -2. [Fix JIT snapshot support for the embedder platform](https://github.com/flutter/flutter/issues/100640). -3. [Hack](https://github.com/flutter/engine/pull/33472) to workaround http://fxbug.dev/75282. -4. [Add support for acquiring software surface to embedder API](https://github.com/flutter/engine/commit/dadf7a7b06b60f36fdce66759bfd74c01d74e27b). +Flutter Engine repository. The changes are cherry-picked into the Flutter Engine build +[here](https://fuchsia.googlesource.com/flutter-embedder/+/refs/heads/main/scripts/sync_engine_artifacts_to_revision.sh#72). These changes should be landed and `libflutter_engine.so` should be pulled from a CIPD bucket instead.