Move !is_android to build_engine_artifacts declaration (#54006)
The `//flutter/build/archives:artifacts` target is used to build a zip
archive (artifacts.zip) of host tools such as flutter_tester, the Dart
kernel compiler, the impellerc shader compiler, and other tooling that
is bundled in debug-mode host builds.
This moves the `!is_android` to the definition of
`build_engine_artifacts`. This is required because of the way that we
produce 32-bit arm gen_snapshot for Android on Windows hosts, which
relies on the regular x64 host toolchain due to us having no 32-bit arm
toolchain for Windows. As such, `current_toolchain == host_toolchain` on
that platform.
```
build_engine_artifacts =
flutter_build_engine_artifacts &&
(current_toolchain == host_toolchain ||
(is_linux && !is_chromeos && current_cpu != "arm") || is_mac || is_win)
```
On iOS builds, we don't have this issue since `current_toolchain` will
be one of:
* `//build/toolchain/mac:ios_clang_arm`
* `//build/toolchain/mac:ios_clang_arm_sim`
* `//build/toolchain/mac:ios_clang_x64_sim`
Whereas `host_toolchain` will be one of:
* `//build/toolchain/mac:clang_arm64`
* `//build/toolchain/mac:clang_x64`
This patch also adds documentation to clarify the purpose of this target
and where related artifacts are produced so that future readers don't
need to do a deep dive into our build plumbing to figure this out.
While the target itself is primarily intended for producing host
binaries, one target binary (gen_snapshot) is bundled into the same
archive bundle as the host tools. This should be refactored such that
just like iOS and Android, they are bundled into their own
target-platform-specific archive, and the tool code accordingly updated
to pull these down into the appropriate cache directory.
As a side-note, on macOS we do rely on this archive target for the host
tools, but the bundled gen_snapshot is unused -- instead, one produced
by the //flutter/sky/tools/create_macos_gen_snapshots.py script used.
This should be fixe in a followup patch.
Related: https://github.com/flutter/flutter/issues/38935
Identified while trying to resolve:
Issue: https://github.com/flutter/flutter/issues/101138
Issue: https://github.com/flutter/flutter/issues/69157
## Pre-launch Checklist
- [X] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [X] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [X] I read and followed the [Flutter Style Guide] and the [C++,
Objective-C, Java style guides].
- [X] I listed at least one issue that this PR fixes in the description
above.
- [X] I added new tests to check the change I am making or feature I am
adding, or the PR is [test-exempt]. See [testing the engine] for
instructions on writing and running engine tests.
- [X] I updated/added relevant documentation (doc comments with `///`).
- [X] I signed the [CLA].
- [X] All existing and new tests are passing.
If you need help, consider asking for advice on the #hackers-new channel
on [Discord].
<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[C++, Objective-C, Java style guides]:
https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
[testing the engine]:
https://github.com/flutter/flutter/wiki/Testing-the-engine
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chatdiff --git a/build/archives/BUILD.gn b/build/archives/BUILD.gn
index 809312c..dd35d33 100644
--- a/build/archives/BUILD.gn
+++ b/build/archives/BUILD.gn
@@ -21,7 +21,28 @@
}
}
-if (build_engine_artifacts && !is_android) {
+# This target is used to build a zip archive (artifacts.zip) of host tools such
+# as flutter_tester, the Dart kernel compiler, the impellerc shader compiler,
+# and other tooling that is bundled in debug-mode host builds.
+#
+# For historical reasons, this bundle also includes gen_snapshot (which is a
+# target-platform-specific binary) for desktop platforms, though the
+# un-suffixed gen_snapshot is unused on macOS (gen_snapshot_arm64 and
+# gen_snapshot_x64 are used in its place).
+#
+# Target-specific iOS artifacts.zip are produced in:
+# //flutter/sky/tools/create_full_ios_framework.py
+#
+# Target-specific Android archives are produced in:
+# //flutter/shell/platform/android:gen_snapshot
+#
+# macOS-specific gen_snapshot_arm64 and gen_snapshot_x64 are produced in:
+# //flutter/sky/tools/create_macos_gen_snapshots.py
+#
+# TODO: https://github.com/flutter/flutter/issues/38935
+# Target-specific binaries such as gen_snapshot should be separated into a
+# separate archive target from host binaries such as flutter_tester.
+if (build_engine_artifacts) {
zip_bundle("artifacts") {
deps = [
"$dart_src/runtime/bin:gen_snapshot",
diff --git a/common/config.gni b/common/config.gni
index c587735..4f8f690 100644
--- a/common/config.gni
+++ b/common/config.gni
@@ -162,9 +162,15 @@
# Flutter SDK artifacts should only be built when either doing host builds, or
# for cross-compiled desktop targets.
+#
+# !is_android is necessary because of the way that Android 32-bit arm
+# gen_snapshot is built on Windows using the host toolchain, since we have no
+# 32-bit arm toolchain for Windows. See comments in //flutter/tools/gn for
+# details.
+#
# TODO: We can't build the engine artifacts for arm (32-bit) right now;
-# see https://github.com/flutter/flutter/issues/74322
+# see https://github.com/flutter/flutter/issues/74322.
build_engine_artifacts =
- flutter_build_engine_artifacts &&
+ flutter_build_engine_artifacts && !is_android &&
(current_toolchain == host_toolchain ||
(is_linux && !is_chromeos && current_cpu != "arm") || is_mac || is_win)
