From d9c2ab32164162393f75bec0c336c6692ea5a5ca Mon Sep 17 00:00:00 2001 From: agrieve Date: Wed, 29 Jun 2016 18:19:47 -0700 Subject: [PATCH] Reland of Have build_config targets depend only on other build_config targets Reason for revert: Added missing dep for andriod_assets. Testing clean builds. TBR=brettw BUG=620034 Review-Url: https://codereview.chromium.org/2109593005 Cr-Original-Commit-Position: refs/heads/master@{#403054} Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src Cr-Mirrored-Commit: 3df1aa821cb0298158b788bb37087460a19e52ea --- android/gyp/write_build_config.py | 15 +--- config/android/internal_rules.gni | 133 +++++++++++++++++++++++++----- config/android/rules.gni | 59 ++++++++----- 3 files changed, 157 insertions(+), 50 deletions(-) diff --git a/android/gyp/write_build_config.py b/android/gyp/write_build_config.py index 6845dabdb..69acb2bb8 100755 --- a/android/gyp/write_build_config.py +++ b/android/gyp/write_build_config.py @@ -200,10 +200,8 @@ def main(argv): '--type', help='Type of this target (e.g. android_library).') parser.add_option( - '--possible-deps-configs', - help='List of paths for dependency\'s build_config files. Some ' - 'dependencies may not write build_config files. Missing build_config ' - 'files are handled differently based on the type of this target.') + '--deps-configs', + help='List of paths for dependency\'s build_config files. ') # android_resources options parser.add_option('--srcjar', help='Path to target\'s resources srcjar.') @@ -290,14 +288,7 @@ def main(argv): raise Exception( '--supports-android is required when using --requires-android') - possible_deps_config_paths = build_utils.ParseGypList( - options.possible_deps_configs) - - unknown_deps = [ - c for c in possible_deps_config_paths if not os.path.exists(c)] - - direct_deps_config_paths = [ - c for c in possible_deps_config_paths if not c in unknown_deps] + direct_deps_config_paths = build_utils.ParseGypList(options.deps_configs) direct_deps_config_paths = _FilterUnwantedDepsPaths(direct_deps_config_paths, options.type) diff --git a/config/android/internal_rules.gni b/config/android/internal_rules.gni index f436a0762..4d1d261fe 100644 --- a/config/android/internal_rules.gni +++ b/config/android/internal_rules.gni @@ -7,6 +7,58 @@ import("//build/config/sanitizers/sanitizers.gni") assert(is_android) +# These identify targets that have .build_config files (except for android_apk, +# java_binary, resource_rewriter, since we never need to depend on these). +_java_target_whitelist = [ + "*:*_java", + "*:*_javalib", + "*:*_java_*", # e.g. java_test_support + "*:java", + "*:junit", + "*:junit_*", + "*:*_junit_*", + "*:*javatests", + "*:*_assets", + "*android*:assets", + "*:*_apk_*resources", + "*android*:resources", + "*:*_resources", + "*:*_grd", + "*:*locale_paks", + + # TODO(agrieve): Rename targets below to match above patterns. + "//android_webview/glue:glue", + "//build/android/pylib/device/commands:chromium_commands", + "//build/android/rezip:rezip", + "//chrome/test/android/cast_emulator:cast_emulator", + "//components/cronet/android:cronet_api", + "//components/cronet/android:cronet_javadoc_classpath", + "//components/policy:app_restrictions_resources", + "//device/battery/android:battery_monitor_android", + "//device/vibration/android:vibration_manager_android", + "//mojo/public/java:bindings", + "//mojo/public/java:system", + "//third_party/android_tools:emma_device", + "//third_party/cardboard-java:cardboard-java", + "//third_party/custom_tabs_client:custom_tabs_client_shared_lib", + "//third_party/custom_tabs_client:custom_tabs_support_lib", + "//third_party/errorprone:chromium_errorprone", + "//third_party/haha:haha", + "//third_party/junit:hamcrest", + "//third_party/netty4:netty_all", + "//third_party/netty-tcnative:netty-tcnative", + "//third_party/robolectric:android-all-4.3_r2-robolectric-0", + "//third_party/robolectric:json-20080701", + "//third_party/robolectric:tagsoup-1.2", +] + +# Targets that match the whitelist but are not actually java targets. +_java_target_blacklist = [ + "//chrome:packed_extra_resources", + "//chrome:packed_resources", + "//remoting/android:remoting_android_raw_resources", +] + # Write the target's .build_config file. This is a json file that contains a # dictionary of information about how to build this target (things that # require knowledge about this target's dependencies and cannot be calculated @@ -17,9 +69,34 @@ assert(is_android) # See build/android/gyp/write_build_config.py and # build/android/gyp/util/build_utils.py:ExpandFileArgs template("write_build_config") { + type = invoker.type + + # Don't need to enforce naming scheme for these targets since we never + # consider them in dependency chains. + if (type != "android_apk" && type != "java_binary" && + type != "resource_rewriter") { + set_sources_assignment_filter(_java_target_whitelist) + _parent_invoker = invoker.invoker + _target_label = + get_label_info(":${_parent_invoker.target_name}", "label_no_toolchain") + sources = [ + _target_label, + ] + if (sources != []) { + set_sources_assignment_filter(_java_target_blacklist) + sources = [] + sources = [ + _target_label, + ] + if (sources != []) { + assert(false, "Invalid java target name: $_target_label") + } + } + sources = [] + } + action(target_name) { set_sources_assignment_filter([]) - type = invoker.type build_config = invoker.build_config assert(type == "android_apk" || type == "java_library" || @@ -31,7 +108,6 @@ template("write_build_config") { [ "deps", "testonly", - "visibility", ]) if (!defined(deps)) { deps = [] @@ -41,14 +117,32 @@ template("write_build_config") { depfile = "$target_gen_dir/$target_name.d" inputs = [] - possible_deps_configs = [] - foreach(d, deps) { - dep_gen_dir = get_label_info(d, "target_gen_dir") - dep_name = get_label_info(d, "name") - possible_deps_configs += [ "$dep_gen_dir/$dep_name.build_config" ] + _deps_configs = [] + if (defined(invoker.possible_config_deps)) { + foreach(_possible_dep, invoker.possible_config_deps) { + set_sources_assignment_filter(_java_target_whitelist) + _target_label = get_label_info(_possible_dep, "label_no_toolchain") + sources = [ + _target_label, + ] + if (sources == []) { + set_sources_assignment_filter(_java_target_blacklist) + sources = [] + sources = [ + _target_label, + ] + if (sources != []) { + deps += [ "${_target_label}__build_config" ] + _dep_gen_dir = get_label_info(_possible_dep, "target_gen_dir") + _dep_name = get_label_info(_possible_dep, "name") + _deps_configs += [ "$_dep_gen_dir/$_dep_name.build_config" ] + } + } + sources = [] + } + set_sources_assignment_filter([]) } - rebase_possible_deps_configs = - rebase_path(possible_deps_configs, root_build_dir) + _rebased_deps_configs = rebase_path(_deps_configs, root_build_dir) outputs = [ depfile, @@ -60,7 +154,7 @@ template("write_build_config") { type, "--depfile", rebase_path(depfile, root_build_dir), - "--possible-deps-configs=$rebase_possible_deps_configs", + "--deps-configs=$_rebased_deps_configs", "--build-config", rebase_path(build_config, root_build_dir), ] @@ -130,13 +224,11 @@ template("write_build_config") { if (is_android_assets) { if (defined(invoker.asset_sources)) { - inputs += invoker.asset_sources _rebased_asset_sources = rebase_path(invoker.asset_sources, root_build_dir) args += [ "--asset-sources=$_rebased_asset_sources" ] } if (defined(invoker.asset_renaming_sources)) { - inputs += invoker.asset_renaming_sources _rebased_asset_renaming_sources = rebase_path(invoker.asset_renaming_sources, root_build_dir) args += [ "--asset-renaming-sources=$_rebased_asset_renaming_sources" ] @@ -1601,7 +1693,9 @@ if (enable_java_templates) { requires_android = defined(invoker.requires_android) && invoker.requires_android - deps = _deps + if (defined(invoker.deps)) { + possible_config_deps = _deps + } build_config = _build_config jar_path = _jar_path if (_supports_android) { @@ -2027,12 +2121,14 @@ if (enable_java_templates) { build_config_target_name = "${_template_name}__build_config" write_build_config(build_config_target_name) { - deps = _accumulated_deps if (defined(invoker.is_java_binary) && invoker.is_java_binary) { type = "java_binary" } else { type = "java_library" } + if (defined(invoker.deps)) { + possible_config_deps = invoker.deps + } supports_android = _supports_android requires_android = _requires_android bypass_platform_checks = defined(invoker.bypass_platform_checks) && @@ -2334,11 +2430,10 @@ if (enable_java_templates) { build_config_target_name = "${target_name}__build_config" write_build_config(build_config_target_name) { - forward_variables_from(invoker, - [ - "deps", - "dex_path", - ]) + forward_variables_from(invoker, [ "dex_path" ]) + if (defined(invoker.deps)) { + possible_config_deps = invoker.deps + } type = "deps_dex" build_config = build_config } diff --git a/config/android/rules.gni b/config/android/rules.gni index a0913a526..4b851beeb 100644 --- a/config/android/rules.gni +++ b/config/android/rules.gni @@ -710,6 +710,7 @@ if (enable_java_templates) { # is specified. # android_manifest: AndroidManifest.xml for this target. Defaults to # //build/android/AndroidManifest.xml. + # android_manifest_dep: Target that generates AndroidManifest (if applicable) # custom_package: java package for generated .java files. # v14_skip: If true, don't run v14 resource generator on this. Defaults to # false. (see build/android/gyp/generate_v14_compatible_resources.py) @@ -746,14 +747,23 @@ if (enable_java_templates) { final_target_name = target_name write_build_config(build_config_target_name) { + type = "android_resources" forward_variables_from(invoker, [ "android_manifest", "custom_package", - "deps", "resource_dirs", ]) + if (defined(invoker.deps)) { + possible_config_deps = invoker.deps + } + if (defined(invoker.android_manifest_dep)) { + deps = [ + invoker.android_manifest_dep, + ] + } + # No package means resources override their deps. if (defined(custom_package) || defined(android_manifest)) { r_text = r_text_path @@ -761,15 +771,12 @@ if (enable_java_templates) { assert(defined(invoker.deps), "Must specify deps when custom_package is omitted.") } - visibility = [ ":$process_resources_target_name" ] - type = "android_resources" resources_zip = zip_path srcjar = srcjar_path } process_resources(process_resources_target_name) { - visibility = [ ":$final_target_name" ] forward_variables_from(invoker, [ "app_as_shared_lib", @@ -786,6 +793,9 @@ if (enable_java_templates) { deps = [] } deps += [ ":$build_config_target_name" ] + if (defined(invoker.android_manifest_dep)) { + deps += [ invoker.android_manifest_dep ] + } # Always generate R.onResourcesLoaded() method, it is required for # compiling ResourceRewriter, there is no side effect because the @@ -849,13 +859,15 @@ if (enable_java_templates) { _build_config_target_name = "${target_name}__build_config" write_build_config(_build_config_target_name) { - forward_variables_from(invoker, - [ - "deps", - "disable_compression", - ]) type = "android_assets" build_config = _build_config + + forward_variables_from(invoker, [ "disable_compression" ]) + + if (defined(invoker.deps)) { + possible_config_deps = invoker.deps + } + if (defined(invoker.sources)) { asset_sources = invoker.sources } @@ -878,7 +890,11 @@ if (enable_java_templates) { } group(target_name) { - forward_variables_from(invoker, [ "visibility" ]) + forward_variables_from(invoker, + [ + "deps", + "visibility", + ]) public_deps = [ ":$_build_config_target_name", ] @@ -895,13 +911,18 @@ if (enable_java_templates) { # } template("java_group") { write_build_config("${target_name}__build_config") { - forward_variables_from(invoker, [ "deps" ]) type = "group" build_config = "$target_gen_dir/${invoker.target_name}.build_config" + + if (defined(invoker.deps)) { + possible_config_deps = invoker.deps + } } group(target_name) { - deps = [] forward_variables_from(invoker, "*") + if (!defined(deps)) { + deps = [] + } deps += [ ":${target_name}__build_config" ] } } @@ -929,7 +950,6 @@ if (enable_java_templates) { build_config = base_path + ".build_config" write_build_config("${target_name}__build_config") { - forward_variables_from(invoker, [ "deps" ]) type = "android_resources" } @@ -939,6 +959,7 @@ if (enable_java_templates) { grit_target_name = "${target_name}__grit" grit_output_dir = "$target_gen_dir/$extra_output_path" grit(grit_target_name) { + forward_variables_from(invoker, [ "deps" ]) grit_flags = [ "-E", "ANDROID_JAVA_TAGGED_ONLY=false", @@ -999,7 +1020,6 @@ if (enable_java_templates) { final_target_name = target_name write_build_config(build_config_target_name) { - visibility = [ ":$zip_target_name" ] type = "android_resources" } @@ -1610,18 +1630,19 @@ if (enable_java_templates) { android_manifest = _android_manifest deps = _android_manifest_deps + if (defined(invoker.deps)) { - deps += invoker.deps + possible_config_deps = invoker.deps } if (defined(invoker.alternative_locale_resource_dep)) { - deps += [ invoker.alternative_locale_resource_dep ] + possible_config_deps += [ invoker.alternative_locale_resource_dep ] has_alternative_locale_resource = true } # Added emma to the target's classpath via its .build_config. if (emma_coverage && !_emma_never_instrument) { - deps += [ "//third_party/android_tools:emma_device" ] + possible_config_deps += [ "//third_party/android_tools:emma_device" ] } proguard_enabled = _proguard_enabled @@ -2327,8 +2348,8 @@ if (enable_java_templates) { forward_variables_from(invoker, "*") testonly = true - assert(!defined(invoker.proguard_enabled) || !invoker.proguard_enabled - || invoker.proguard_configs != []) + assert(!defined(invoker.proguard_enabled) || !invoker.proguard_enabled || + invoker.proguard_configs != []) if (!defined(apk_name)) { apk_name = get_label_info(invoker.shared_library, "name")