From ed6acfd1977f44f86c7654873dd326c67bf179fe Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Mon, 30 Sep 2024 14:39:36 -0700 Subject: [PATCH] Introduce a GN rule that (explicitly) generates a `dart test` wrapper (flutter/engine#55475) Closes https://github.com/flutter/flutter/issues/155769. This is a variant of the approach in https://github.com/flutter/engine/pull/52241, based on feedback from @jakemac53 and @jonahwilliams (who originally sped up `dart test` significantly by using `frontend_server` under the scenes: https://github.com/dart-lang/test/pull/1399), in short: ```gn # tools/engine_tool/BUILD.gn import("//flutter/build/dart/internal/gen_dartcli_call.gni") gen_dartcli_call("tests") { args = [ "test" ] cwd = "//flutter/tools/engine_tool" } ``` This stanza, when built (`ninja -C ../out/host_debug flutter/tools/engine_tool:tests`) generates the following file: ```sh # ../out/host_debug/gen/flutter/tools/engine_tool/tests set -e # Needed because if it is set, cd may print the path it changed to. unset CDPATH # Store the current working directory. prev_cwd=$(pwd) # Set a trap to restore the working directory. trap 'cd "$prev_cwd"' EXIT CD_PATH="/Users/matanl/Developer/engine/src/flutter/tools/engine_tool" if [ -n "$CD_PATH" ]; then cd "$CD_PATH" fi /Users/matanl/Developer/engine/src/flutter/prebuilts/macos-arm64/dart-sdk/bin/dart "test" ``` In turn, when executed (`../out/host_debug/gen/flutter/tools/engine_tool/tests`) it just runs `dart test`: ```sh flutter % ../out/host_debug/gen/flutter/tools/engine_tool/tests Building package executable... Built test:test. 00:00 +0: test/test_command_test.dart: test command executes test 00:00 +3: test/run_command_test.dart: run command invokes flutter run ... 00:00 +117: All tests passed! ``` There is no actual compilation performed by the tool (that is handled implicitly by `dart test`), and as a result neither a `depfile` is needed, nor generating a pre-compiled artifact like a snapshot or AOT elf/assembly. --- This work is incomplete, that is, we'd want to properly tag the executable so `et` can find it, and create a wrapper template (i.e. `dart_test`) that tightens things up a bit, but I wanted to show the work at this intermediate step to get feedback before moving forward. /cc @jonahwilliams, @jtmcdole as well. --- engine/src/flutter/BUILD.gn | 3 + .../build/dart/internal/gen_dartcli_call.gni | 46 ++++++++++++++ .../dart/internal/gen_executable_call.gni | 62 +++++++++++++++++++ .../dart/internal/gen_executable_call.py | 55 ++++++++++++++++ engine/src/flutter/build/dart/test/BUILD.gn | 36 +++++++++++ engine/src/flutter/tools/engine_tool/BUILD.gn | 15 +++++ 6 files changed, 217 insertions(+) create mode 100644 engine/src/flutter/build/dart/internal/gen_dartcli_call.gni create mode 100644 engine/src/flutter/build/dart/internal/gen_executable_call.gni create mode 100644 engine/src/flutter/build/dart/internal/gen_executable_call.py create mode 100644 engine/src/flutter/build/dart/test/BUILD.gn create mode 100644 engine/src/flutter/tools/engine_tool/BUILD.gn diff --git a/engine/src/flutter/BUILD.gn b/engine/src/flutter/BUILD.gn index 9dd5c94ed8..a865021e51 100644 --- a/engine/src/flutter/BUILD.gn +++ b/engine/src/flutter/BUILD.gn @@ -129,8 +129,11 @@ group("flutter") { if (build_engine_artifacts) { public_deps += [ + "//flutter/build/dart/test:gen_dartcli_call", + "//flutter/build/dart/test:gen_executable_call", "//flutter/shell/testing", "//flutter/tools/const_finder", + "//flutter/tools/engine_tool:tests", "//flutter/tools/font_subset", ] } diff --git a/engine/src/flutter/build/dart/internal/gen_dartcli_call.gni b/engine/src/flutter/build/dart/internal/gen_dartcli_call.gni new file mode 100644 index 0000000000..f7ae2ad40d --- /dev/null +++ b/engine/src/flutter/build/dart/internal/gen_dartcli_call.gni @@ -0,0 +1,46 @@ +# 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("//flutter/build/dart/internal/gen_executable_call.gni") +import("//flutter/common/config.gni") + +# Generates an executable that runs `dart` with a command and set of arguments. +# +# Parameters: +# args (optional): +# Arguments to pass to the Dart CLI. +# +# cwd (optional): +# The working directory for the command. +# +# output (optional): +# Overrides the full output path. +# Defaults to $root_out_dir/gen/$target_path/$target_name; for example +# //flutter/foo/bar emits a binary at out/{variant}/gen/flutter/foo/bar. +template("gen_dartcli_call") { + # Build a reference to the Dart CLI (depending on prebuilt or source). + ext = "" + if (is_win) { + ext = ".exe" + } + dart = rebase_path("$host_prebuilt_dart_sdk/bin/dart$ext") + + # Add default arguments to the Dart CLI. + dart_args = [] + if (defined(invoker.args)) { + dart_args += invoker.args + } + dart_args += [ "--suppress-analytics" ] + + # Actually generate the shell script. + gen_executable_call(target_name) { + command = rebase_path(dart) + args = dart_args + forward_variables_from(invoker, + [ + "cwd", + "output", + ]) + } +} diff --git a/engine/src/flutter/build/dart/internal/gen_executable_call.gni b/engine/src/flutter/build/dart/internal/gen_executable_call.gni new file mode 100644 index 0000000000..5a11da611d --- /dev/null +++ b/engine/src/flutter/build/dart/internal/gen_executable_call.gni @@ -0,0 +1,62 @@ +# 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. + +# Generates an executable that runs a command and set of arguments. +# +# Parameters: +# command (required): +# The command to run, which is typically an absolute path to an executable. +# +# args (optional): +# Arguments to pass to the command. +# +# cwd (optional): +# The working directory for the command. +# +# output (optional): +# Overrides the full output path. +# Defaults to $root_out_dir/gen/$target_path/$target_name; for example +# //flutter/foo/bar emits a binary at out/{variant}/gen/flutter/foo/bar. +template("gen_executable_call") { + assert(defined(invoker.command), "Must specify 'command'") + + # The command to run. + command = invoker.command + + # The output path. + output = "$root_gen_dir/$target_name" + if (defined(invoker.output)) { + output = invoker.output + } else { + # Construct the output path. + output = get_label_info(target_name, "target_gen_dir") + } + + # Build the command line arguments. + call_args = [ + "--output", + rebase_path(output), + "--command", + command, + ] + if (defined(invoker.cwd)) { + call_args += [ + "--cwd", + rebase_path(invoker.cwd), + ] + } + if (defined(invoker.args)) { + call_args += [ + "--", + string_join(" ", invoker.args), + ] + } + + # Run build_cmd.py to generate the file and make it executable. + action(target_name) { + script = "//flutter/build/dart/internal/gen_executable_call.py" + outputs = [ output ] + args = call_args + } +} diff --git a/engine/src/flutter/build/dart/internal/gen_executable_call.py b/engine/src/flutter/build/dart/internal/gen_executable_call.py new file mode 100644 index 0000000000..69ce53a1ef --- /dev/null +++ b/engine/src/flutter/build/dart/internal/gen_executable_call.py @@ -0,0 +1,55 @@ +#!/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. + +"""Generates a shell or batch script to run a command.""" + +import argparse +import os +import string + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument('--output', required=True, help='Output file') + parser.add_argument('--command', required=True, help='Command to run') + parser.add_argument('--cwd', required=False, help='Working directory') + parser.add_argument('rest', nargs='*', help='Arguments to pass to the command') + + # Rest of the arguments are passed to the command. + args = parser.parse_args() + + out_path = os.path.dirname(args.output) + if not os.path.exists(out_path): + os.makedirs(out_path) + + script = string.Template( + '''#!/bin/sh + +set -e + +# Set a trap to restore the working directory. +trap "popd > /dev/null" EXIT +pushd "$cwd" > /dev/null + +$command $args +''' + ) + + params = { + 'command': args.command, + 'args': ' '.join(args.rest), + 'cwd': args.cwd if args.cwd else '', + } + + with open(args.output, 'w') as f: + f.write(script.substitute(params)) + + # Make the script executable. + os.chmod(args.output, 0o755) + + +if __name__ == '__main__': + main() diff --git a/engine/src/flutter/build/dart/test/BUILD.gn b/engine/src/flutter/build/dart/test/BUILD.gn new file mode 100644 index 0000000000..c33392a83c --- /dev/null +++ b/engine/src/flutter/build/dart/test/BUILD.gn @@ -0,0 +1,36 @@ +# 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("//flutter/build/dart/internal/gen_dartcli_call.gni") +import("//flutter/build/dart/internal/gen_executable_call.gni") + +# Manual test targets that can be used to iterate on individual rules. + +# ninja -C ../out/host_debug flutter/build/dart/test:gen_executable_call +# ../out/host_debug/gen/flutter/build/dart/test/gen_executable_call +# +# Expected output: "Hello, World!" +gen_executable_call("gen_executable_call") { + command = "echo" + args = [ "Hello, World!" ] +} + +# ninja -C ../out/host_debug flutter/build/dart/test:gen_executable_call_with_cwd +# ../out/host_debug/gen/flutter/build/dart/test/gen_executable_call_with_cwd +# +# Expected output: {varies}/flutter/build/dart/test +gen_executable_call("gen_executable_call_with_cwd") { + command = "echo \$PWD" + cwd = "//flutter/build/dart/test" +} + +# ninja -C ../out/host_debug flutter/build/dart/test:gen_dartcli_call +# ../out/host_debug/gen/flutter/build/dart/test/gen_dartcli_call +# +# Expected output: Dart SDK version: 3.6.0-273.0.dev (dev) +# +# ... or something like ^, obviously it will constantly change as the SDK rolls. +gen_dartcli_call("gen_dartcli_call") { + args = [ "--version" ] +} diff --git a/engine/src/flutter/tools/engine_tool/BUILD.gn b/engine/src/flutter/tools/engine_tool/BUILD.gn new file mode 100644 index 0000000000..9b36d0e669 --- /dev/null +++ b/engine/src/flutter/tools/engine_tool/BUILD.gn @@ -0,0 +1,15 @@ +# 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("//flutter/build/dart/internal/gen_dartcli_call.gni") + +# TODO(matanl): WIP in https://github.com/flutter/flutter/issues/155769. +# +# Example usage: +# ninja -C ../out/host_debug flutter/tools/engine_tool:tests +# ../out/host_debug/gen/flutter/tools/engine_tool/tests +gen_dartcli_call("tests") { + args = [ "test" ] + cwd = "//flutter/tools/engine_tool" +}