From cbaf3f8fbcda443e23eb1d478deeb0eb95ed4e23 Mon Sep 17 00:00:00 2001 From: "zijiehe@" <68449066+zijiehe-google-com@users.noreply.github.com> Date: Tue, 24 Sep 2024 08:44:56 -0700 Subject: [PATCH] [Fuchsia] Remove deprecated and unnecessary parameters from fuchsia*archive (flutter/engine#55324) This is a precondition to use more high-level templates in fuchsia-gn-sdk to replace build rules in flutter. Meanwhile fuchsia-gn-sdk does not use pm anymore and this change is also helpful to get rid of the use of pm. Briefly, it removes all cmx_file parameters and avoids specifying duplicated parameters (binary / target_name / default cml file). Bug: http://b/353729557, http://b/40935282 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style --- .../platform/fuchsia/dart_runner/BUILD.gn | 7 +- .../dart_aot_runner/BUILD.gn | 3 - .../dart_jit_runner/BUILD.gn | 3 - .../shell/platform/fuchsia/flutter/BUILD.gn | 41 ++--- .../tests/integration/embedder/BUILD.gn | 4 - .../tests/integration/mouse-input/BUILD.gn | 4 - .../tests/integration/text-input/BUILD.gn | 3 - .../tests/integration/touch-input/BUILD.gn | 4 - .../flutter/tools/fuchsia/fuchsia_archive.gni | 143 +++++++----------- 9 files changed, 69 insertions(+), 143 deletions(-) diff --git a/engine/src/flutter/shell/platform/fuchsia/dart_runner/BUILD.gn b/engine/src/flutter/shell/platform/fuchsia/dart_runner/BUILD.gn index 4dac8a9d8b0..e36ce927430 100644 --- a/engine/src/flutter/shell/platform/fuchsia/dart_runner/BUILD.gn +++ b/engine/src/flutter/shell/platform/fuchsia/dart_runner/BUILD.gn @@ -178,8 +178,6 @@ template("aot_runner_package") { binary = "dart_aot${product_suffix}_runner" - cml_file = rebase_path("meta/dart_aot${product_suffix}_runner.cml") - libraries = common_libs resources = [] @@ -238,8 +236,6 @@ template("jit_runner_package") { binary = "dart_jit${product_suffix}_runner" - cml_file = rebase_path("meta/dart_jit${product_suffix}_runner.cml") - libraries = common_libs resources = [ @@ -330,8 +326,7 @@ if (enable_unittests) { fuchsia_test_archive("dart_runner_tests") { deps = [ ":dart_test_runner_unittests" ] - - binary = "$target_name" + gen_cml_file = true } # When adding a new dep here, please also ensure the dep is added to diff --git a/engine/src/flutter/shell/platform/fuchsia/dart_runner/tests/startup_integration_test/dart_aot_runner/BUILD.gn b/engine/src/flutter/shell/platform/fuchsia/dart_runner/tests/startup_integration_test/dart_aot_runner/BUILD.gn index 482378574aa..e1df73b8c23 100644 --- a/engine/src/flutter/shell/platform/fuchsia/dart_runner/tests/startup_integration_test/dart_aot_runner/BUILD.gn +++ b/engine/src/flutter/shell/platform/fuchsia/dart_runner/tests/startup_integration_test/dart_aot_runner/BUILD.gn @@ -40,7 +40,4 @@ executable("dart-aot-runner-integration-test-bin") { fuchsia_test_archive("dart-aot-runner-integration-test") { deps = [ ":dart-aot-runner-integration-test-bin" ] - - binary = "$target_name" - cml_file = rebase_path("meta/$target_name.cml") } diff --git a/engine/src/flutter/shell/platform/fuchsia/dart_runner/tests/startup_integration_test/dart_jit_runner/BUILD.gn b/engine/src/flutter/shell/platform/fuchsia/dart_runner/tests/startup_integration_test/dart_jit_runner/BUILD.gn index 83e5a254515..675753ee8ae 100644 --- a/engine/src/flutter/shell/platform/fuchsia/dart_runner/tests/startup_integration_test/dart_jit_runner/BUILD.gn +++ b/engine/src/flutter/shell/platform/fuchsia/dart_runner/tests/startup_integration_test/dart_jit_runner/BUILD.gn @@ -40,7 +40,4 @@ executable("dart-jit-runner-integration-test-bin") { fuchsia_test_archive("dart-jit-runner-integration-test") { deps = [ ":dart-jit-runner-integration-test-bin" ] - - binary = "$target_name" - cml_file = rebase_path("meta/$target_name.cml") } diff --git a/engine/src/flutter/shell/platform/fuchsia/flutter/BUILD.gn b/engine/src/flutter/shell/platform/fuchsia/flutter/BUILD.gn index ad4e3426a1c..bf8d7a868ee 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/BUILD.gn +++ b/engine/src/flutter/shell/platform/fuchsia/flutter/BUILD.gn @@ -320,8 +320,6 @@ template("jit_runner") { binary = "flutter_jit${product_suffix}_runner" - cml_file = rebase_path("meta/flutter_jit${product_suffix}_runner.cml") - resources = [ { path = rebase_path("//flutter/third_party/icu/common/icudtl.dat") @@ -389,8 +387,6 @@ template("aot_runner") { ] } - cml_file = rebase_path("meta/flutter_aot${product_suffix}_runner.cml") - binary = "flutter_aot${product_suffix}_runner" resources = [ @@ -590,9 +586,7 @@ if (enable_unittests) { fuchsia_test_archive("flutter_runner_tests") { deps = [ ":flutter_runner_unittests" ] - - binary = "$target_name" - + gen_cml_file = true resources = [ { path = rebase_path("//flutter/third_party/icu/common/icudtl.dat") @@ -603,9 +597,7 @@ if (enable_unittests) { fuchsia_test_archive("flutter_runner_tzdata_tests") { deps = [ ":flutter_runner_tzdata_unittests" ] - - binary = "$target_name" - + gen_cml_file = true resources = [ { path = rebase_path("//flutter/third_party/icu/common/icudtl.dat") @@ -631,9 +623,7 @@ if (enable_unittests) { fuchsia_test_archive("flutter_runner_tzdata_missing_tests") { deps = [ ":flutter_runner_tzdata_missing_unittests" ] - - binary = "$target_name" - + gen_cml_file = true resources = [ { path = rebase_path("//flutter/third_party/icu/common/icudtl.dat") @@ -644,15 +634,14 @@ if (enable_unittests) { fuchsia_test_archive("fml_tests") { deps = [ "//flutter/fml:fml_unittests" ] - + gen_cml_file = true binary = "fml_unittests" } fuchsia_test_archive("display_list_tests") { deps = [ "//flutter/display_list:display_list_unittests" ] - + gen_cml_file = true binary = "display_list_unittests" - resources = [ { path = rebase_path( @@ -664,9 +653,8 @@ if (enable_unittests) { fuchsia_test_archive("display_list_render_tests") { deps = [ "//flutter/display_list:display_list_rendertests" ] - + gen_cml_file = true binary = "display_list_rendertests" - resources = [ { path = rebase_path( @@ -678,9 +666,8 @@ if (enable_unittests) { fuchsia_test_archive("flow_tests") { deps = [ "//flutter/flow:flow_unittests" ] - + gen_cml_file = true binary = "flow_unittests" - resources = [ { path = rebase_path( @@ -710,7 +697,7 @@ if (enable_unittests) { "//flutter/runtime:runtime_fixtures", "//flutter/runtime:runtime_unittests", ] - + gen_cml_file = true binary = "runtime_unittests" # TODO(gw280): https://github.com/flutter/flutter/issues/50294 @@ -729,7 +716,7 @@ if (enable_unittests) { "//flutter/shell/common:shell_unittests", "//flutter/shell/common:shell_unittests_fixtures", ] - + gen_cml_file = true binary = "shell_unittests" # TODO(gw280): https://github.com/flutter/flutter/issues/50294 @@ -758,13 +745,13 @@ if (enable_unittests) { fuchsia_test_archive("testing_tests") { deps = [ "//flutter/testing:testing_unittests" ] - + gen_cml_file = true binary = "testing_unittests" } fuchsia_test_archive("txt_tests") { deps = [ "//flutter/third_party/txt:txt_unittests" ] - + gen_cml_file = true binary = "txt_unittests" resources = [ @@ -784,7 +771,7 @@ if (enable_unittests) { "//flutter/lib/ui:ui_unittests", "//flutter/lib/ui:ui_unittests_fixtures", ] - + gen_cml_file = true binary = "ui_unittests" # TODO(gw280): https://github.com/flutter/flutter/issues/50294 @@ -826,7 +813,7 @@ if (enable_unittests) { "//flutter/shell/platform/embedder:embedder_unittests", "//flutter/shell/platform/embedder:fixtures", ] - + gen_cml_file = true binary = "embedder_unittests" # TODO(gw280): https://github.com/flutter/flutter/issues/50294 @@ -904,7 +891,7 @@ if (enable_unittests) { fuchsia_test_archive("dart_utils_tests") { deps = [ "//flutter/shell/platform/fuchsia/runtime/dart/utils:dart_utils_unittests" ] - + gen_cml_file = true binary = "dart_utils_unittests" } diff --git a/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/embedder/BUILD.gn b/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/embedder/BUILD.gn index 43bc60e4fb5..4267976a7bb 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/embedder/BUILD.gn +++ b/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/embedder/BUILD.gn @@ -59,8 +59,4 @@ fuchsia_test_archive("flutter-embedder-test") { # TODO(fxbug.dev/106575): Fix this with subpackages. "//flutter/shell/platform/fuchsia/flutter:oot_flutter_jit_runner", ] - - binary = "$target_name" - - cml_file = rebase_path("meta/$target_name.cml") } diff --git a/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/mouse-input/BUILD.gn b/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/mouse-input/BUILD.gn index d976e940ffa..62697f2cc10 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/mouse-input/BUILD.gn +++ b/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/mouse-input/BUILD.gn @@ -54,7 +54,6 @@ executable("mouse-input-test-bin") { } fuchsia_test_archive("mouse-input-test") { - testonly = true deps = [ ":mouse-input-test-bin", @@ -63,7 +62,4 @@ fuchsia_test_archive("mouse-input-test") { # TODO(fxbug.dev/106575): Fix this with subpackages. "//flutter/shell/platform/fuchsia/flutter:oot_flutter_jit_runner", ] - - binary = "$target_name" - cml_file = rebase_path("meta/$target_name.cml") } diff --git a/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/text-input/BUILD.gn b/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/text-input/BUILD.gn index 41346b91046..64d5c21062a 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/text-input/BUILD.gn +++ b/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/text-input/BUILD.gn @@ -59,7 +59,4 @@ fuchsia_test_archive("text-input-test") { # TODO(fxbug.dev/106575): Fix this with subpackages. "//flutter/shell/platform/fuchsia/flutter:oot_flutter_jit_runner", ] - - binary = "$target_name" - cml_file = rebase_path("meta/$target_name.cml") } diff --git a/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/touch-input/BUILD.gn b/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/touch-input/BUILD.gn index 984456cbbb0..51aaae45e4b 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/touch-input/BUILD.gn +++ b/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/touch-input/BUILD.gn @@ -55,7 +55,6 @@ executable("touch-input-test-bin") { } fuchsia_test_archive("touch-input-test") { - testonly = true deps = [ ":touch-input-test-bin", @@ -64,7 +63,4 @@ fuchsia_test_archive("touch-input-test") { # TODO(fxbug.dev/106575): Fix this with subpackages. "//flutter/shell/platform/fuchsia/flutter:oot_flutter_jit_runner", ] - - binary = "$target_name" - cml_file = rebase_path("meta/$target_name.cml") } diff --git a/engine/src/flutter/tools/fuchsia/fuchsia_archive.gni b/engine/src/flutter/tools/fuchsia/fuchsia_archive.gni index 9161dd0ccf2..d18a4c9ec80 100644 --- a/engine/src/flutter/tools/fuchsia/fuchsia_archive.gni +++ b/engine/src/flutter/tools/fuchsia/fuchsia_archive.gni @@ -33,12 +33,6 @@ template("_compile_cml") { # # binary (required): # The ELF binary for the archive's program. -# cmx_file (optional): -# The path to the V1 component manifest (.cmx file) for the archive's component. -# Should include the file extension. -# cml_file (optional): -# The path to the V2 component manifest (.cml file) for the archive's component. -# Should include the file extension. # deps (optional): # The code dependencies for the archive. # inputs (optional): @@ -100,20 +94,6 @@ template("_fuchsia_archive") { pkg_dir_deps = pkg.deps - if (defined(invoker.cmx_file)) { - # cmx files are used for v1 components, only copy it if it is defined - cmx_file = invoker.cmx_file - - cmx_target = "$pkg_target_name.copy_cmx" - - copy("$cmx_target") { - sources = [ "$cmx_file" ] - outputs = [ "$far_base_dir/meta/${pkg_target_name}.cmx" ] - } - - pkg_dir_deps += [ ":$cmx_target" ] - } - write_file("${far_base_dir}/meta/package", { name = pkg.package_name @@ -187,12 +167,8 @@ template("_fuchsia_archive") { # An archive combines an ELF binary and a component manifest to create # a packaged Fuchsia program that can be deployed to a Fuchsia device. # -# binary (required): -# The ELF binary for the archive's program. -# cmx_file (required if cml_file is not set): -# The V1 component manifest for the Fuchsia component. -# cml_file (required if cmx_file is not set): -# The V2 component manifest for the Fuchsia component. +# binary (optional): +# The ELF binary for the archive's program, or target_name if unspecified. # deps (optional): # The code dependencies for the archive. # inputs (optional): @@ -204,33 +180,34 @@ template("_fuchsia_archive") { # testonly (optional): # Set this to true for archives that are only used for tests. template("fuchsia_archive") { - assert(defined(invoker.cmx_file) || defined(invoker.cml_file), - "must specify either a cmx file, cml file or both") + if (!defined(invoker.binary)) { + _binary = target_name + } else { + _binary = invoker.binary + } _deps = [] if (defined(invoker.deps)) { _deps += invoker.deps } - if (defined(invoker.cml_file)) { - _far_base_dir = "$root_out_dir/${target_name}_far" - _cml_file_name = get_path_info(invoker.cml_file, "name") - _compile_cml_target = "${target_name}_${_cml_file_name}_compile_cml" + _cml_file = rebase_path("meta/${_binary}.cml") + _far_base_dir = "$root_out_dir/${target_name}_far" + _cml_file_name = get_path_info(_cml_file, "name") + _compile_cml_target = "${target_name}_${_cml_file_name}_compile_cml" - _compile_cml(_compile_cml_target) { - forward_variables_from(invoker, [ "testonly" ]) + _compile_cml(_compile_cml_target) { + forward_variables_from(invoker, [ "testonly" ]) - manifest = invoker.cml_file - output = "$_far_base_dir/meta/${_cml_file_name}.cm" - } - _deps += [ ":$_compile_cml_target" ] + manifest = _cml_file + output = "$_far_base_dir/meta/${_cml_file_name}.cm" } + _deps += [ ":$_compile_cml_target" ] _fuchsia_archive(target_name) { deps = _deps + binary = _binary forward_variables_from(invoker, [ - "binary", - "cmx_file", "inputs", "libraries", "resources", @@ -242,46 +219,33 @@ template("fuchsia_archive") { # Creates a Fuchsia archive (.far) file containing a generated test root # component and test driver component, using PM from the Fuchsia SDK. # -# binary (required): -# The binary for the test. +# binary (optional): +# The binary for the test, or target_name if unspecified. # deps (required): # Dependencies for the test archive. -# cmx_file (optional): -# A path to the .cmx file for the test archive. -# If not defined, a generated .cml file for the test archive will be used instead. -# cml_file (optional): -# The path to the V2 component manifest (.cml file) for the test archive's component. -# Should include the file extension. -# If not defined, a generated .cml file for the test archive will be used instead. +# gen_cml_file (optional): +# If is defined and true, an interpolate cml file will be generated. # libraries (optional): # Paths to .so libraries that should be dynamically linked to the binary. # resources (optional): # Files that should be placed into the `data/` directory of the archive. template("fuchsia_test_archive") { assert(defined(invoker.deps), "package must define deps") + if (!defined(invoker.binary)) { + _binary = target_name + } else { + _binary = invoker.binary + } _deps = [] if (defined(invoker.deps)) { _deps += invoker.deps } - if (defined(invoker.cml_file)) { - _far_base_dir = "$root_out_dir/${target_name}_far" - _cml_file_name = get_path_info(invoker.cml_file, "name") - _compile_cml_target = "${target_name}_${_cml_file_name}_compile_cml" - - _compile_cml(_compile_cml_target) { - forward_variables_from(invoker, [ "testonly" ]) - - manifest = invoker.cml_file - output = "$_far_base_dir/meta/${_cml_file_name}.cm" - } - _deps += [ ":$_compile_cml_target" ] - } else { - # Interpolate test_suite.cml template with the test suite's name. - test_suite = target_name - interpolate_cml_target = "${test_suite}_interpolate_cml" - generated_cml_file = "$root_out_dir/$test_suite.cml" - action(interpolate_cml_target) { + _generated_cml = defined(invoker.gen_cml_file) && invoker.gen_cml_file + if (_generated_cml) { + _cml_file = "$root_out_dir/${target_name}.cml" + _interpolate_cml_target = "${target_name}_interpolate_cml" + action(_interpolate_cml_target) { testonly = true script = "//flutter/tools/fuchsia/interpolate_test_suite.py" sources = [ "//flutter/testing/fuchsia/meta/test_suite.cml" ] @@ -289,35 +253,36 @@ template("fuchsia_test_archive") { "--input", rebase_path("//flutter/testing/fuchsia/meta/test_suite.cml"), "--test-suite", - test_suite, + target_name, "--output", - rebase_path(generated_cml_file), + rebase_path(_cml_file), ] - outputs = [ generated_cml_file ] + outputs = [ _cml_file ] } - - far_base_dir = "$root_out_dir/${target_name}_far" - - # Compile the resulting interpolated test suite's cml. - compile_test_suite_cml_target = "${test_suite}_test_suite_compile_cml" - _compile_cml(compile_test_suite_cml_target) { - testonly = true - deps = [ ":$interpolate_cml_target" ] - - manifest = generated_cml_file - output = "$far_base_dir/meta/${test_suite}.cm" - } - - _deps += [ ":$compile_test_suite_cml_target" ] + } else { + _cml_file = rebase_path("meta/${_binary}.cml") } + _far_base_dir = "$root_out_dir/${target_name}_far" + _cml_file_name = get_path_info(_cml_file, "name") + _compile_cml_target = "${target_name}_${_cml_file_name}_compile_cml" + + _compile_cml(_compile_cml_target) { + testonly = true + + manifest = _cml_file + output = "$_far_base_dir/meta/${_cml_file_name}.cm" + + if (_generated_cml) { + deps = [ ":$_interpolate_cml_target" ] + } + } + _deps += [ ":$_compile_cml_target" ] + _fuchsia_archive(target_name) { testonly = true - forward_variables_from(invoker, - [ - "binary", - "resources", - ]) + binary = _binary + forward_variables_from(invoker, [ "resources" ]) libraries = common_libs if (defined(invoker.libraries)) {