From 85773e527b8f37e1a8d62c70ac8ae4cd0700f7d3 Mon Sep 17 00:00:00 2001 From: John Chen Date: Fri, 23 Oct 2020 18:55:27 +0000 Subject: [PATCH] Revert "Reland "Android: Use locally-build devil deps only when build_with_chromium=true"" This reverts commit f8b7ec28ae8bde48ca760a2162b06c88320e6c79. Reason for revert: Causing all builds to fail on android-pixel2-perf failing Original change's description: > Reland "Android: Use locally-build devil deps only when build_with_chromium=true" > > This reverts commit b47e9c8355d7a2b5b8e424a02ec7b095bd241da7. > > Reason for reland: Updated more .pydeps files. > > Original change's description: > > Revert "Android: Use locally-build devil deps only when build_with_chromium=true" > > > > This reverts commit 49fdeca1c6fe3e06828fe32ea3cf9c34524f4e4a. > > > > Reason for revert: Breaking presubmit due to stale .pydeps file: > > android_webview/tools/run_cts.pydeps > > > > Original change's description: > > > Android: Use locally-build devil deps only when build_with_chromium=true > > > > > > Bug: 1120190 > > > Change-Id: I763e42a7ae69d14eb61caebd220844b756d6d171 > > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2480842 > > > Commit-Queue: Andrew Grieve > > > Reviewed-by: Ben Pastene > > > Cr-Commit-Position: refs/heads/master@{#819489} > > > > TBR=agrieve@chromium.org,bpastene@chromium.org > > > > Change-Id: I960ad355283c66951e5931f61e25421c5a5dfdf7 > > No-Presubmit: true > > No-Tree-Checks: true > > No-Try: true > > Bug: 1120190 > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2490857 > > Reviewed-by: Andrew Grieve > > Commit-Queue: Andrew Grieve > > Cr-Commit-Position: refs/heads/master@{#819668} > > TBR=agrieve # reland > > Bug: 1120190 > Change-Id: I8f3e21182e83a3e88da52331d56a8bef76507939 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2490558 > Commit-Queue: Andrew Grieve > Reviewed-by: Andrew Grieve > Cr-Commit-Position: refs/heads/master@{#819810} TBR=agrieve@chromium.org,bpastene@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 1120190, 1141891 Change-Id: I490f2e8a972b260a81b942e248c0f920af11db46 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2495335 Reviewed-by: John Chen Commit-Queue: John Chen Cr-Commit-Position: refs/heads/master@{#820355} --- BUILD.gn | 4 -- android_webview/test/BUILD.gn | 1 + android_webview/tools/run_cts.pydeps | 1 - build/android/BUILD.gn | 48 +++++--------- build/android/devil_chromium.py | 63 ++++++------------- build/android/devil_chromium.pydeps | 1 - build/config/android/internal_rules.gni | 25 ++++++-- build/config/android/rules.gni | 24 ++++--- .../scripts/monochrome_python_tests.pydeps | 3 +- .../chromedriver/test/run_py_tests.pydeps | 1 - testing/scripts/run_android_wpt.pydeps | 1 - 11 files changed, 71 insertions(+), 101 deletions(-) diff --git a/BUILD.gn b/BUILD.gn index 305075a0f38407..313199ac413eee 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -1253,11 +1253,7 @@ if (!is_ios) { root_build_dir) + ")" ] data = [ - # These tests use //build/android/devil_chromium.py even when !is_android, - # so cannot use the helpers in //build/android (they assert(is_android)). "//build/android/", - "//build/gn_helpers.py", - "//build/config/gclient_args.gni", "//components/crash/content/tools/generate_breakpad_symbols.py", "//third_party/blink/renderer/bindings/scripts/", "//third_party/blink/renderer/build/scripts/", diff --git a/android_webview/test/BUILD.gn b/android_webview/test/BUILD.gn index dca6b0ab825b5a..1061bb15a6433c 100644 --- a/android_webview/test/BUILD.gn +++ b/android_webview/test/BUILD.gn @@ -24,6 +24,7 @@ python_library("webview_cts_tests") { pydeps_file = "//android_webview/tools/run_cts.pydeps" deps = [ "//android_webview:system_webview_apk" ] data_deps = [ + "//build/android:logdog_wrapper_py", "//build/android:test_runner_py", "//testing/buildbot/filters:webview_cts_tests_filters", ] diff --git a/android_webview/tools/run_cts.pydeps b/android_webview/tools/run_cts.pydeps index a2d2a281ff5d6e..d7fb767a279d54 100644 --- a/android_webview/tools/run_cts.pydeps +++ b/android_webview/tools/run_cts.pydeps @@ -13,7 +13,6 @@ //build/android/pylib/local/emulator/proto/avd_pb2.py //build/android/pylib/utils/__init__.py //build/android/pylib/utils/test_filter.py -//build/gn_helpers.py //third_party/catapult/common/py_utils/py_utils/__init__.py //third_party/catapult/common/py_utils/py_utils/cloud_storage.py //third_party/catapult/common/py_utils/py_utils/cloud_storage_global_lock.py diff --git a/build/android/BUILD.gn b/build/android/BUILD.gn index bb313aaf3a2b8b..fc26e9317778b8 100644 --- a/build/android/BUILD.gn +++ b/build/android/BUILD.gn @@ -52,27 +52,9 @@ python_library("devil_chromium_py") { "devil_chromium.json", "//third_party/catapult/third_party/gsutil/", "//third_party/catapult/devil/devil/devil_dependencies.json", - - # Read by gn_helpers.BuildWithChromium() - "//build/config/gclient_args.gni", ] } -# Contains runtime deps for installing apks. -# E.g. from test_runner.py or from apk_operations.py. -group("apk_installer_data") { - # Other //build users let devil library fetch these from Google Storage. - if (build_with_chromium) { - data_deps = [ - "//build/android/pylib/device/commands", - "//tools/android/md5sum", - ] - data = [ - "//third_party/android_build_tools/bundletool/bundletool-all-1.2.0.jar", - ] - } -} - python_library("test_runner_py") { testonly = true pydeps_file = "test_runner.pydeps" @@ -87,17 +69,14 @@ python_library("test_runner_py") { "${android_sdk_root}/platform-tools/adb", "//third_party/requests/", ] - data_deps = [ - ":apk_installer_data", - ":devil_chromium_py", - ":logdog_wrapper_py", - ":stack_tools", - ] - - # Other //build users let devil library fetch these from Google Storage. + data_deps = [ ":devil_chromium_py" ] if (build_with_chromium) { - data_deps += [ "//tools/android/forwarder2" ] - data += [ "//tools/android/avd/proto/" ] + data += [ + "//third_party/android_build_tools/bundletool/bundletool-all-1.2.0.jar", + "//tools/android/avd/proto/", + ] + data_deps += + [ "//third_party/android_platform/development/scripts:stack_py" ] if (is_asan) { data_deps += [ "//tools/android/asan/third_party:asan_device_setup" ] } @@ -125,14 +104,19 @@ python_library("resource_sizes_py") { ] } +python_library("bundle_wrapper_script_py") { + pydeps_file = "gyp/create_bundle_wrapper_script.pydeps" + data = [ + "//third_party/android_build_tools/bundletool/bundletool-all-1.2.0.jar", + ] +} + # Tools necessary for symbolizing tombstones or stack traces that are output to # logcat. # Hidden behind build_with_chromium because some third party repos that use # //build don't pull in //third_party/android_platform. -# TODO(crbug.com/1120190): Move stack script into //build/third_party -# and enable unconditionally. -group("stack_tools") { - if (build_with_chromium) { +if (build_with_chromium) { + group("stack_tools") { data = [ "tombstones.py", "pylib/symbols/", diff --git a/build/android/devil_chromium.py b/build/android/devil_chromium.py index df8f2e8be3aa82..1cd5a87154db5f 100644 --- a/build/android/devil_chromium.py +++ b/build/android/devil_chromium.py @@ -7,21 +7,14 @@ import os import sys -from pylib import constants from pylib.constants import host_paths if host_paths.DEVIL_PATH not in sys.path: - sys.path.insert(1, host_paths.DEVIL_PATH) + sys.path.append(host_paths.DEVIL_PATH) from devil import devil_env from devil.android.ndk import abis -_BUILD_DIR = os.path.join(constants.DIR_SOURCE_ROOT, 'build') -if _BUILD_DIR not in sys.path: - sys.path.insert(1, _BUILD_DIR) - -import gn_helpers - _DEVIL_CONFIG = os.path.abspath( os.path.join(os.path.dirname(__file__), 'devil_chromium.json')) @@ -114,33 +107,6 @@ } -def _UseLocalBuildProducts(output_directory, devil_dynamic_config): - output_directory = os.path.abspath(output_directory) - devil_dynamic_config['dependencies'] = { - dep_name: { - 'file_info': { - '%s_%s' % (dep_config['platform'], dep_config['arch']): { - 'local_paths': [ - os.path.join(output_directory, - *dep_config['path_components']), - ], - } - for dep_config in dep_configs - } - } - for dep_name, dep_configs in _DEVIL_BUILD_PRODUCT_DEPS.iteritems() - } - - -def _BuildWithChromium(): - """Returns value of gclient's |build_with_chromium|.""" - gni_path = os.path.join(_BUILD_DIR, 'config', 'gclient_args.gni') - with open(gni_path) as f: - data = f.read() - args = gn_helpers.FromGNArgs(data) - return args['build_with_chromium'] - - def Initialize(output_directory=None, custom_deps=None, adb_path=None): """Initializes devil with chromium's binaries and third-party libraries. @@ -168,18 +134,26 @@ def Initialize(output_directory=None, custom_deps=None, adb_path=None): adb_path: An optional path to use for the adb binary. If not set, this uses the adb binary provided by the Android SDK. """ - build_with_chromium = _BuildWithChromium() devil_dynamic_config = { 'config_type': 'BaseConfig', 'dependencies': {}, } - if build_with_chromium and output_directory: - # Non-chromium users of chromium's //build directory fetch build products - # from google storage rather than use locally built copies. Chromium uses - # locally-built copies so that changes to the tools can be easily tested. - _UseLocalBuildProducts(output_directory, devil_dynamic_config) - + if output_directory: + output_directory = os.path.abspath(output_directory) + devil_dynamic_config['dependencies'] = { + dep_name: { + 'file_info': { + '%s_%s' % (dep_config['platform'], dep_config['arch']): { + 'local_paths': [ + os.path.join(output_directory, *dep_config['path_components']), + ], + } + for dep_config in dep_configs + } + } + for dep_name, dep_configs in _DEVIL_BUILD_PRODUCT_DEPS.iteritems() + } if custom_deps: devil_dynamic_config['dependencies'].update(custom_deps) if adb_path: @@ -193,6 +167,5 @@ def Initialize(output_directory=None, custom_deps=None, adb_path=None): } }) - config_files = [_DEVIL_CONFIG] if build_with_chromium else None - devil_env.config.Initialize(configs=[devil_dynamic_config], - config_files=config_files) + devil_env.config.Initialize( + configs=[devil_dynamic_config], config_files=[_DEVIL_CONFIG]) diff --git a/build/android/devil_chromium.pydeps b/build/android/devil_chromium.pydeps index 4b9c58dd9ea02f..ea8f0c2f8ab3de 100644 --- a/build/android/devil_chromium.pydeps +++ b/build/android/devil_chromium.pydeps @@ -32,7 +32,6 @@ ../../third_party/catapult/devil/devil/utils/timeout_retry.py ../../third_party/catapult/devil/devil/utils/watchdog_timer.py ../../third_party/catapult/third_party/zipfile/zipfile_2_7_13.py -../gn_helpers.py devil_chromium.py pylib/__init__.py pylib/constants/__init__.py diff --git a/build/config/android/internal_rules.gni b/build/config/android/internal_rules.gni index 33892d018346c5..e3da602843ee65 100644 --- a/build/config/android/internal_rules.gni +++ b/build/config/android/internal_rules.gni @@ -700,7 +700,7 @@ template("test_runner_script") { "deps", "public_deps", ]) - data_deps = [] + data_deps = [ "//tools/android/md5sum" ] if (defined(invoker.data_deps)) { data_deps += invoker.data_deps } @@ -726,6 +726,18 @@ template("test_runner_script") { generate_android_wrapper(target_name) { wrapper_script = "$root_build_dir/bin/run_${_test_name}" + forward_variables_from(invoker, + [ + "data_deps", + "deps", + ]) + if (!defined(deps)) { + deps = [] + } + + if (!defined(data_deps)) { + data_deps = [] + } if (defined(android_test_runner_script)) { executable = android_test_runner_script @@ -737,20 +749,21 @@ template("test_runner_script") { if (defined(invoker.deps)) { deps = invoker.deps } - data_deps = [ "//build/android:test_runner_py" ] + data_deps = [] if (defined(invoker.data_deps)) { - data_deps += invoker.data_deps + data_deps = invoker.data_deps } data = [] - if (defined(invoker.data)) { - data = invoker.data - } executable_args = [ _test_type, "--output-directory", "@WrappedPath(.)", ] + data_deps += [ + "//build/android:logdog_wrapper_py", + "//build/android:test_runner_py", + ] if (_runtime_deps) { deps += [ ":$_runtime_deps_target" ] diff --git a/build/config/android/rules.gni b/build/config/android/rules.gni index 82c5eae8fb345c..869e521ab615b0 100644 --- a/build/config/android/rules.gni +++ b/build/config/android/rules.gni @@ -3305,6 +3305,7 @@ if (enable_java_templates) { _generated_script = "$root_build_dir/bin/${invoker.target_name}" script = "//build/android/gyp/create_apk_operations_script.py" outputs = [ _generated_script ] + data_deps = [ "//tools/android/md5sum" ] args = [ "--script-output-path", rebase_path(_generated_script, root_build_dir), @@ -3338,12 +3339,9 @@ if (enable_java_templates) { _static_library_apk_path, ] } - data = [] - data_deps = [ - "//build/android:apk_installer_data", - "//build/android:stack_tools", - ] - + if (!defined(data)) { + data = [] + } if (_proguard_enabled && !_incremental_apk) { # Required by logcat command. data_deps += [ "//build/android/stacktrace:java_deobfuscate" ] @@ -3410,6 +3408,10 @@ if (enable_java_templates) { data_deps += [ ":${target_name}__lint" ] } + if (_incremental_apk) { + # device/commands is used by the installer script to push files via .zip. + data_deps += [ "//build/android/pylib/device/commands" ] + } if (_uses_static_library) { data_deps += [ invoker.static_library_provider ] } @@ -3777,6 +3779,8 @@ if (enable_java_templates) { # symbolization can be done. ":${target_name}__secondary_abi_shared_library_list", ":${target_name}__shared_library_list", + "//build/android/pylib/device/commands", + "//tools/android/forwarder2", ] if (defined(invoker.data_deps)) { data_deps += invoker.data_deps @@ -3954,6 +3958,10 @@ if (enable_java_templates) { "//base:base_java", "//testing/android/reporter:reporter_java", ] + data_deps += [ "//build/android/pylib/device/commands" ] + if (host_os == "linux") { + data_deps += [ "//tools/android/forwarder2" ] + } } } @@ -4983,8 +4991,8 @@ if (enable_java_templates) { _bundle_path, ] data_deps = [ - "//build/android:apk_installer_data", - "//build/android:stack_tools", + "//build/android:bundle_wrapper_script_py", + "//tools/android/md5sum", ] deps = [ _base_module_build_config_target ] diff --git a/chrome/android/monochrome/scripts/monochrome_python_tests.pydeps b/chrome/android/monochrome/scripts/monochrome_python_tests.pydeps index f0942ec1a554a0..fba475a935d8aa 100644 --- a/chrome/android/monochrome/scripts/monochrome_python_tests.pydeps +++ b/chrome/android/monochrome/scripts/monochrome_python_tests.pydeps @@ -1,10 +1,9 @@ # Generated by running: -# build/print_python_deps.py --output chrome/android/monochrome/scripts/monochrome_python_tests.pydeps --gn-paths chrome/android/monochrome/scripts +# build/print_python_deps.py --gn-paths chrome/android/monochrome/scripts //build/android/devil_chromium.py //build/android/pylib/__init__.py //build/android/pylib/constants/__init__.py //build/android/pylib/constants/host_paths.py -//build/gn_helpers.py //chrome/android/monochrome/scripts/monochrome_android_manifest_test.py //chrome/android/monochrome/scripts/monochrome_apk_checker_test.py //chrome/android/monochrome/scripts/monochrome_dexdump_test.py diff --git a/chrome/test/chromedriver/test/run_py_tests.pydeps b/chrome/test/chromedriver/test/run_py_tests.pydeps index 1780200094370a..97ff8cca4f7003 100644 --- a/chrome/test/chromedriver/test/run_py_tests.pydeps +++ b/chrome/test/chromedriver/test/run_py_tests.pydeps @@ -4,7 +4,6 @@ ../../../../build/android/pylib/__init__.py ../../../../build/android/pylib/constants/__init__.py ../../../../build/android/pylib/constants/host_paths.py -../../../../build/gn_helpers.py ../../../../third_party/catapult/common/py_utils/py_utils/__init__.py ../../../../third_party/catapult/common/py_utils/py_utils/cloud_storage.py ../../../../third_party/catapult/common/py_utils/py_utils/cloud_storage_global_lock.py diff --git a/testing/scripts/run_android_wpt.pydeps b/testing/scripts/run_android_wpt.pydeps index b7e72c4f1cfa8f..06416555afa742 100644 --- a/testing/scripts/run_android_wpt.pydeps +++ b/testing/scripts/run_android_wpt.pydeps @@ -4,7 +4,6 @@ //build/android/pylib/__init__.py //build/android/pylib/constants/__init__.py //build/android/pylib/constants/host_paths.py -//build/gn_helpers.py //testing/scripts/common.py //testing/scripts/run_android_wpt.py //testing/scripts/wpt_common.py