From b16181fe6adf18f448058fd4e68ceb4f78f601ab Mon Sep 17 00:00:00 2001 From: "auto-submit[bot]" <98614782+auto-submit[bot]@users.noreply.github.com> Date: Fri, 17 May 2024 23:35:47 +0000 Subject: [PATCH] Reverts "[macOS] Generate universal gen_snapshots (#52885)" (flutter/engine#52913) Reverts: flutter/engine#52885 Initiated by: cbracken Reason for reverting: while this patch worked fine, it pushed the mac build bot over its time limit. Previous builds were just squeaking under the wire but seeing timeouts on the mac_host_engine host_release shard after this commit. e.g. https://ci.chromium.org/ui/p/flutter/builders/prod/Mac%20mac_host_engine/10667/overview Looking at ci.yaml I see the timeout is set to 240, but the timeout on the failing shard is c Original PR Author: cbracken Reviewed By: {jmagman} This change reverts the following previous change: Previously, the `gen_snapshot_arm64` and `gen_snapshot_x64` binaries used by the tool were all built for x64 architecture. As such, developers building apps with Flutter rely on Rosetta translation with every build. This refactors the gen_snapshot build rules on macOS hosts to consistently produce `gen_snapshot_arm64` and `gen_snapshot_x64` binaries with the target architecture of the build but with as universal binaries with both host architectures. ### arm64 host build Prior to this patch we emitted: * gen_snapshot_arm64 (arch: x64, target_arch: simarm64) After this patch, we emit: * artifacts_x64/gen_snapshot_arm64 (arch: x64, target_arch: simarm64) * artifacts_arm64/gen_snapshot_arm64 (arch: arm64, target_arch: arm64) * gen_snapshot_arm64 (universal binary composed of both of the above) ### x64 host build Prior to this patch we emitted: * gen_snapshot_x64 (arch: x64, target_arch: x64) After this patch, we emit: * artifacts_x64/gen_snapshot_x64 (arch: x64, target_arch: x64) * artifacts_arm64/gen_snapshot_x64 (arch: arm64, target_arch: simx64) * gen_snapshot_x64 (universal binary composed of both of the above) Note that host builds on macOS currently default to a host architecture of x64 (can be overridden via `--force-mac-arm64`) regardless of host architecture and thus, the build itself relies on Rosetta translation when invoked on Apple Silicon arm64 hardware. This is to ensure a consistent build in CI regardless of bot architecture. See: https://github.com/flutter/engine/blob/0d2b0cd0edaddbc8b29bc6ba4116e0019ab78b85/tools/gn#L502-L505 Issue: https://github.com/flutter/flutter/issues/101138 Issue: https://github.com/flutter/flutter/issues/69157 Related issue: https://github.com/flutter/flutter/issues/103386 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style --- engine/src/flutter/build/archives/BUILD.gn | 4 +- .../flutter/ci/licenses_golden/excluded_files | 1 - engine/src/flutter/lib/snapshot/BUILD.gn | 54 +++---------------- .../flutter/sky/tools/create_macos_binary.py | 54 ------------------- 4 files changed, 10 insertions(+), 103 deletions(-) delete mode 100755 engine/src/flutter/sky/tools/create_macos_binary.py diff --git a/engine/src/flutter/build/archives/BUILD.gn b/engine/src/flutter/build/archives/BUILD.gn index ae228afeddc..38637ad2239 100644 --- a/engine/src/flutter/build/archives/BUILD.gn +++ b/engine/src/flutter/build/archives/BUILD.gn @@ -287,8 +287,8 @@ if (is_mac) { output = "$full_platform_name$suffix/gen_snapshot.zip" files = [ { - source = "${root_out_dir}/gen_snapshot_${target_cpu}" - destination = "gen_snapshot_${target_cpu}" + source = "$root_out_dir/gen_snapshot_$target_cpu" + destination = "gen_snapshot_$target_cpu" }, ] } diff --git a/engine/src/flutter/ci/licenses_golden/excluded_files b/engine/src/flutter/ci/licenses_golden/excluded_files index 8819e93b023..37cc6dec125 100644 --- a/engine/src/flutter/ci/licenses_golden/excluded_files +++ b/engine/src/flutter/ci/licenses_golden/excluded_files @@ -416,7 +416,6 @@ ../../../flutter/sky/tools/create_embedder_framework.py ../../../flutter/sky/tools/create_full_ios_framework.py ../../../flutter/sky/tools/create_ios_framework.py -../../../flutter/sky/tools/create_macos_binary.py ../../../flutter/sky/tools/create_macos_framework.py ../../../flutter/sky/tools/create_macos_gen_snapshots.py ../../../flutter/sky/tools/create_xcframework.py diff --git a/engine/src/flutter/lib/snapshot/BUILD.gn b/engine/src/flutter/lib/snapshot/BUILD.gn index 075288d1873..99a431769ea 100644 --- a/engine/src/flutter/lib/snapshot/BUILD.gn +++ b/engine/src/flutter/lib/snapshot/BUILD.gn @@ -186,54 +186,16 @@ if (host_os == "mac" && target_os != "mac" && # # This target is used for builds targeting macOS. if (host_os == "mac" && target_os == "mac") { - template("build_mac_gen_snapshot") { - assert(defined(invoker.host_arch)) - host_cpu = invoker.host_arch + copy("create_macos_gen_snapshots") { + # The toolchain-specific output directory. For cross-compiles, this is a + # clang-x64 or clang-arm64 subdirectory of the top-level build directory. + host_output_dir = + get_label_info("$dart_src/runtime/bin:gen_snapshot($host_toolchain)", + "root_out_dir") - build_toolchain = "//build/toolchain/mac:clang_$host_cpu" - if (host_cpu == target_cpu) { - gen_snapshot_target_name = "gen_snapshot_host_targeting_host" - } else { - gen_snapshot_target_name = "gen_snapshot" - } - gen_snapshot_target = - "$dart_src/runtime/bin:$gen_snapshot_target_name($build_toolchain)" - - copy(target_name) { - # The toolchain-specific output directory. For cross-compiles, this is a - # clang-x64 or clang-arm64 subdirectory of the top-level build directory. - output_dir = get_label_info(gen_snapshot_target, "root_out_dir") - - sources = [ "${output_dir}/${gen_snapshot_target_name}" ] - outputs = - [ "${root_out_dir}/artifacts_$host_cpu/gen_snapshot_${target_cpu}" ] - deps = [ gen_snapshot_target ] - } - } - - build_mac_gen_snapshot("create_macos_gen_snapshot_arm64_${target_cpu}") { - host_arch = "arm64" - } - - build_mac_gen_snapshot("create_macos_gen_snapshot_x64_${target_cpu}") { - host_arch = "x64" - } - - action("create_macos_gen_snapshots") { - script = "//flutter/sky/tools/create_macos_binary.py" + sources = [ "${host_output_dir}/gen_snapshot" ] outputs = [ "${root_out_dir}/gen_snapshot_${target_cpu}" ] - args = [ - "--in-arm64", - rebase_path("${root_out_dir}/artifacts_arm64/gen_snapshot_${target_cpu}"), - "--in-x64", - rebase_path("${root_out_dir}/artifacts_x64/gen_snapshot_${target_cpu}"), - "--out", - rebase_path("${root_out_dir}/gen_snapshot_${target_cpu}"), - ] - deps = [ - ":create_macos_gen_snapshot_arm64_${target_cpu}", - ":create_macos_gen_snapshot_x64_${target_cpu}", - ] + deps = [ "$dart_src/runtime/bin:gen_snapshot($host_toolchain)" ] } } diff --git a/engine/src/flutter/sky/tools/create_macos_binary.py b/engine/src/flutter/sky/tools/create_macos_binary.py deleted file mode 100755 index 687b53c6558..00000000000 --- a/engine/src/flutter/sky/tools/create_macos_binary.py +++ /dev/null @@ -1,54 +0,0 @@ -#!/usr/bin/env python3 -# -# Copyright 2013 The Flutter Authors. All rights reserved. -# Use of this source code is governed by a BSD-style license that can be -# found in the LICENSE file. - -import argparse -import os -import subprocess -import sys - - -def canonical_path(path): - """Returns the canonical path for the input path. - If the input path is not absolute, it is treated as relative to the engine - source tree's buildroot directory.""" - if os.path.isabs(path): - return path - buildroot_dir = os.path.abspath(os.path.join(os.path.realpath(__file__), '..', '..', '..', '..')) - return os.path.join(buildroot_dir, path) - - -def assert_file_exists(binary_path, arch): - if not os.path.isfile(binary_path): - print('Cannot find macOS %s binary at %s' % (arch, binary_path)) - sys.exit(1) - - -def create_universal_binary(in_arm64, in_x64, out): - subprocess.check_call(['lipo', in_arm64, in_x64, '-create', '-output', out]) - - -def main(): - parser = argparse.ArgumentParser( - description='Creates a universal binary from input arm64, x64 binaries' - ) - parser.add_argument('--in-arm64', type=str, required=True) - parser.add_argument('--in-x64', type=str, required=True) - parser.add_argument('--out', type=str, required=True) - args = parser.parse_args() - - in_arm64 = canonical_path(args.in_arm64) - in_x64 = canonical_path(args.in_x64) - out = canonical_path(args.out) - - assert_file_exists(in_arm64, 'arm64') - assert_file_exists(in_x64, 'x64') - create_universal_binary(in_arm64, in_x64, out) - - return 0 - - -if __name__ == '__main__': - sys.exit(main())