-
Notifications
You must be signed in to change notification settings - Fork 6k
Build rules for scenario_app #27302
Build rules for scenario_app #27302
Changes from all commits
578460b
7e1d933
71f998d
3354f7c
c91a053
097d1d3
c19070a
1d5e61f
c38c2ee
06178f2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,10 +2,171 @@ | |
| # Use of this source code is governed by a BSD-style license that can be | ||
| # found in the LICENSE file. | ||
|
|
||
| # This file has rules for making Dart packages and Dart-based Mojo applications. | ||
| # The entrypoint is the dart_pkg rule. | ||
| # This file has rules for making Dart packages and snapshots. | ||
|
|
||
| import("//build/compiled_action.gni") | ||
| import("//build/module_args/dart.gni") | ||
| import("//flutter/common/config.gni") | ||
| import("//third_party/dart/build/dart/dart_action.gni") | ||
|
|
||
| # Creates a dart kernel (dill) file suitable for use with gen_snapshot, as well | ||
| # as the app-jit, aot-elf, or aot-assembly snapshot for Android or iOS. | ||
| # | ||
| # Invoker must supply dart_main and package_config. Invoker may optionally | ||
| # supply aot as a boolean and product as a boolean. | ||
| template("dart_snapshot") { | ||
| assert(!is_fuchsia) | ||
| assert(defined(invoker.main_dart), "main_dart is a required parameter.") | ||
| assert(defined(invoker.package_config), | ||
| "package_config is a required parameter.") | ||
|
|
||
| kernel_target = "_${target_name}_kernel" | ||
| snapshot_target = "_${target_name}_snapshot" | ||
| is_aot = | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Perhaps add @richkadel: FYI, you may run into such assumptions while working with engine GN rules for Dart. We can patch them later though. The issue is that the assumption of AOT being tied to a specific runtime mode does not hold on Fuchsia.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fuchsia has its own template(s) for this. Perhaps we could add it as an arg and default it to this, and let fuchsia specify the arg differently?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For now I'm just moving this to
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Errr... wrong comment thread - I moved it from |
||
| flutter_runtime_mode == "profile" || flutter_runtime_mode == "release" | ||
|
|
||
| kernel_output = "$target_gen_dir/kernel_blob.bin" | ||
|
|
||
| prebuilt_dart_action(kernel_target) { | ||
| script = "//flutter/flutter_frontend_server/bin/starter.dart" | ||
|
|
||
| main_dart = rebase_path(invoker.main_dart) | ||
| package_config = rebase_path(invoker.package_config) | ||
| flutter_patched_sdk = rebase_path("$root_out_dir/flutter_patched_sdk") | ||
|
|
||
| deps = [ "//flutter/lib/snapshot:strong_platform" ] | ||
|
|
||
| inputs = [ | ||
| main_dart, | ||
| package_config, | ||
| ] | ||
|
|
||
| outputs = [ kernel_output ] | ||
|
|
||
| depfile = "$kernel_output.d" | ||
| abs_depfile = rebase_path(depfile) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note: Not that it matters here, but using absolute paths trips up compiler proxies. If the script supports relative depfile paths, I'd recommend just using relative paths from the get go.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was cargo-culted from another template - it seems to make dart happy, which doesn't support compiler proxies so... I guess it's ok? |
||
| rebased_output = rebase_path(kernel_output, root_build_dir) | ||
| vm_args = [ | ||
| "--depfile=$abs_depfile", | ||
| "--depfile_output_filename=$rebased_output", | ||
| "--disable-dart-dev", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this flag need to come first?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know - it's how the VM gets invoked today though and it doesn't complain?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just asked around. It did have to come first at some point in the past, but it can go anywhere now. |
||
| ] | ||
|
|
||
| args = [ | ||
| "--packages=" + rebase_path(package_config), | ||
| "--target=flutter", | ||
| "--sdk-root=" + flutter_patched_sdk, | ||
| "--output-dill=" + rebase_path(kernel_output), | ||
| ] | ||
|
|
||
| if (is_aot) { | ||
| args += [ | ||
| "--aot", | ||
| "--tfa", | ||
| ] | ||
| } else { | ||
| # --no-link-platform is only valid when --aot isn't specified | ||
| args += [ "--no-link-platform" ] | ||
| } | ||
|
|
||
| if (defined(invoker.product) && invoker.product) { | ||
| # Setting this flag in a non-product release build for AOT (a "profile" | ||
| # build) causes the vm service isolate code to be tree-shaken from an app. | ||
| # See the pragma on the entrypoint here: | ||
| # | ||
| # https://github.com/dart-lang/sdk/blob/master/sdk/lib/_internal/vm/bin/vmservice_io.dart#L240 | ||
| # | ||
| # Also, this define excludes debugging and profiling code from Flutter. | ||
| args += [ "-Ddart.vm.product=true" ] | ||
| } else { | ||
| if (!is_debug) { | ||
| # The following define excludes debugging code from Flutter. | ||
| args += [ "-Ddart.vm.profile=true" ] | ||
| } | ||
| } | ||
|
|
||
| args += [ rebase_path(main_dart) ] | ||
| } | ||
|
|
||
| compiled_action(snapshot_target) { | ||
| if (target_cpu == "x86" && host_os == "linux") { | ||
| # By default Dart will create a 32-bit gen_snapshot host binary if the target | ||
| # platform is 32-bit. Override this to create a 64-bit gen_snapshot for x86 | ||
| # targets because some host platforms may not support 32-bit binaries. | ||
| tool = "//third_party/dart/runtime/bin:gen_snapshot_host_targeting_host" | ||
| toolchain = "//build/toolchain/$host_os:clang_x64" | ||
| } else { | ||
| tool = "//third_party/dart/runtime/bin:gen_snapshot" | ||
| } | ||
|
|
||
| inputs = [ kernel_output ] | ||
| deps = [ ":$kernel_target" ] | ||
| outputs = [] | ||
|
|
||
| args = [ "--lazy_async_stacks" ] | ||
|
|
||
| if (is_debug && flutter_runtime_mode != "profile" && | ||
| flutter_runtime_mode != "release" && | ||
| flutter_runtime_mode != "jit_release") { | ||
| args += [ "--enable_asserts" ] | ||
| } | ||
|
|
||
| if (is_aot) { | ||
| args += [ "--deterministic" ] | ||
| if (is_ios) { | ||
| snapshot_assembly = "$target_gen_dir/ios/snapshot_assembly.S" | ||
| outputs += [ snapshot_assembly ] | ||
| args += [ | ||
| "--snapshot_kind=app-aot-assembly", | ||
| "--assembly=" + rebase_path(snapshot_assembly), | ||
| ] | ||
| } else if (is_android) { | ||
| libapp = "$target_gen_dir/android/libs/$android_app_abi/libapp.so" | ||
| outputs += [ libapp ] | ||
| args += [ | ||
| "--snapshot_kind=app-aot-elf", | ||
| "--elf=" + rebase_path(libapp), | ||
| ] | ||
| } else { | ||
| assert(false) | ||
| } | ||
| } else { | ||
| deps += [ "//flutter/lib/snapshot:generate_snapshot_bin" ] | ||
| vm_snapshot_data = | ||
| "$root_gen_dir/flutter/lib/snapshot/vm_isolate_snapshot.bin" | ||
| snapshot_data = "$root_gen_dir/flutter/lib/snapshot/isolate_snapshot.bin" | ||
| isolate_snapshot_data = "$target_gen_dir/isolate_snapshot_data" | ||
| isolate_snapshot_instructions = "$target_gen_dir/isolate_snapshot_instr" | ||
|
|
||
| inputs += [ | ||
| vm_snapshot_data, | ||
| snapshot_data, | ||
| ] | ||
|
|
||
| outputs += [ | ||
| isolate_snapshot_data, | ||
| isolate_snapshot_instructions, | ||
| ] | ||
| args += [ | ||
| "--snapshot_kind=app-jit", | ||
| "--load_vm_snapshot_data=" + rebase_path(vm_snapshot_data), | ||
| "--load_isolate_snapshot_data=" + rebase_path(snapshot_data), | ||
| "--isolate_snapshot_data=" + rebase_path(isolate_snapshot_data), | ||
| "--isolate_snapshot_instructions=" + | ||
| rebase_path(isolate_snapshot_instructions), | ||
| ] | ||
| } | ||
|
|
||
| args += [ rebase_path(kernel_output) ] | ||
| } | ||
|
|
||
| group(target_name) { | ||
| public_deps = [ | ||
| ":$kernel_target", | ||
| ":$snapshot_target", | ||
| ] | ||
| } | ||
| } | ||
|
|
||
| template("dart_pkg_helper") { | ||
| assert(defined(invoker.package_name)) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| # 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/rules.gni") | ||
| import("//flutter/testing/scenario_app/runtime_mode.gni") | ||
|
|
||
| dart_snapshot("scenario_app_snapshot") { | ||
| main_dart = "lib/main.dart" | ||
| package_config = ".dart_tool/package_config.json" | ||
| } | ||
|
|
||
| if (!is_aot) { | ||
| if (is_android) { | ||
| _flutter_assets_dir = "$root_out_dir/scenario_app/app/assets/flutter_assets" | ||
| } else if (is_ios) { | ||
| _flutter_assets_dir = | ||
| "$root_out_dir/scenario_app/Scenarios/App.framework/flutter_assets" | ||
| } else { | ||
| assert(false) | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggest adding a final
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
|
|
||
| copy("copy_jit_assets") { | ||
| visibility = [ ":*" ] | ||
| sources = [ | ||
| "$target_gen_dir/isolate_snapshot_data", | ||
| "$target_gen_dir/isolate_snapshot_instr", | ||
| "$target_gen_dir/kernel_blob.bin", | ||
| ] | ||
| outputs = [ "$_flutter_assets_dir/{{source_file_part}}" ] | ||
| deps = [ ":scenario_app_snapshot" ] | ||
| } | ||
| } | ||
|
|
||
| group("scenario_app") { | ||
| deps = [ ":scenario_app_snapshot" ] | ||
|
|
||
| if (!is_aot) { | ||
| deps += [ ":copy_jit_assets" ] | ||
| } | ||
|
|
||
| if (is_android) { | ||
| deps += [ "android" ] | ||
| } | ||
|
|
||
| if (is_ios) { | ||
| deps += [ "ios" ] | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| # 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/testing/scenario_app/runtime_mode.gni") | ||
|
|
||
| action("android") { | ||
| script = "run_gradle.py" | ||
|
|
||
| inputs = [ "$root_out_dir/flutter.jar" ] | ||
| sources = [ | ||
| "app/build.gradle", | ||
| "app/src/androidTest/java/dev/flutter/TestRunner.java", | ||
| "app/src/androidTest/java/dev/flutter/scenarios/EngineLaunchE2ETest.java", | ||
| "app/src/androidTest/java/dev/flutter/scenarios/ExampleInstrumentedTest.java", | ||
| "app/src/androidTest/java/dev/flutter/scenariosui/MemoryLeakTests.java", | ||
| "app/src/androidTest/java/dev/flutter/scenariosui/PlatformTextureUiTests.java", | ||
| "app/src/androidTest/java/dev/flutter/scenariosui/PlatformViewUiTests.java", | ||
| "app/src/androidTest/java/dev/flutter/scenariosui/ScreenshotUtil.java", | ||
| "app/src/androidTest/java/dev/flutter/scenariosui/SpawnEngineTests.java", | ||
| "app/src/main/AndroidManifest.xml", | ||
| "app/src/main/java/dev/flutter/scenarios/SpawnedEngineActivity.java", | ||
| "app/src/main/java/dev/flutter/scenarios/StrictModeFlutterActivity.java", | ||
| "app/src/main/java/dev/flutter/scenarios/TestActivity.java", | ||
| "app/src/main/java/dev/flutter/scenarios/TestableFlutterActivity.java", | ||
| "app/src/main/java/dev/flutter/scenarios/TextPlatformView.java", | ||
| "app/src/main/java/dev/flutter/scenarios/TextPlatformViewActivity.java", | ||
| "app/src/main/java/dev/flutter/scenarios/TextPlatformViewFactory.java", | ||
| "app/src/main/java/io/flutter/plugins/GeneratedPluginRegistrant.java", | ||
| "build.gradle", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what do you think about globbing under
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. GN does not like globs.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (gradle will still build the files anyway, but GN won't be aware of changes to unmentioned ones dirtying the build) |
||
| ] | ||
| outputs = [ "$root_out_dir/scenario_app/app/outputs/apk/debug/app-debug.apk" ] | ||
|
|
||
| args = [ | ||
| "assembleDebug", | ||
| "--no-daemon", | ||
| "-Pflutter_jar=" + rebase_path("$root_out_dir/flutter.jar"), | ||
| "-Pout_dir=" + rebase_path("$root_out_dir/scenario_app"), | ||
| ] | ||
|
|
||
| if (is_aot) { | ||
| args += [ "-Plibapp=" + rebase_path("$target_gen_dir/libs") ] | ||
| inputs += [ "$target_gen_dir/libs/$android_app_abi/libapp.so" ] | ||
| } | ||
|
|
||
| deps = [ | ||
| "//flutter/shell/platform/android:android_jar", | ||
| "//flutter/testing/scenario_app:scenario_app_snapshot", | ||
| ] | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| #!/usr/bin/env python | ||
| # 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. | ||
|
|
||
| """ | ||
| Invokes gradlew for building the scenario_app from GN/Ninja. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. GN knows how to run compiler binaries with some help, and it knows how to run python scripts. This is actually really hacky - we shouldn't be invoking gradle from GN, but refactoring that will take more work and we're no worse off than we were before invoking gradle from a shell script. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does it know how to run a bash script? If that's the case, then gradlew directly should work, no?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That doesn't work on Windows. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It could use gradlew.bat on Windows
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Which is all this python script is doing |
||
| """ | ||
|
|
||
| import os | ||
| import sys | ||
| import subprocess | ||
|
|
||
| def main(): | ||
| BAT = '.bat' if sys.platform.startswith(('cygwin', 'win')) else '' | ||
| android_dir = os.path.abspath(os.path.dirname(__file__)) | ||
| gradle_bin = os.path.join('.', 'gradlew%s' % BAT) | ||
| result = subprocess.check_output( | ||
| args=[gradle_bin] + sys.argv[1:], | ||
| cwd=android_dir, | ||
| ) | ||
| return 0 | ||
|
|
||
|
|
||
| if __name__ == '__main__': | ||
| sys.exit(main()) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an unfortunate repetition of the rule (in
//flutter/testing/testing.gni) used by the gtest harness for Dart fixtures. Can we dry this up (in a later patch if you wish)? If anything,//build/dart/rules.gniis the right spot for such rules.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this will lead to template name collision if both
gnis are included from a.GNdefinition.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are now several places where we have rules like this - I'd like to clean them up separately to avoid potential issues that only get caught in post submit :\
One thing I can see is that the testing rules lack special handling for iOS, which will need to be addressed.