Skip to content

GitLab

T
tangled
Commit c8827ca3 authored by Peter Wen's avatar Peter Wen Committed by Commit Bot
Browse files
Options

Android: Improve type filtering, add __assetres 

Add _java_resource_types, _java_library_types, and _java_leaf_types.
These types, together with "group", define all existing invoker.type
values for all java targets. Since leaf types like android_apk are not
used as dependencies for other java targets, all java targets in deps
trees can be defined as either resources, libraries, or groups.

Naming conventions have also been improved. Now java targets must match
_java_resource_patterns if the target is a resource target. Otherwise,
it must match _java_library_patterns if the target is a library target.
Group targets can match either of those, but should prefer matching
_java_library_patterns since they are treated as library targets in the
java_group template.

Thus, target filtering is now simplified. To filter for resource targets
, simply use _java_resource_patterns. To filter for library targets, use
_java_library_patterns and ignore the ones that also match
_java_resource_patterns. To filter for any java target, use
_java_target_patterns, which is a simple concatenation of the other two
lists of patterns.

The assert in write_build_config has been updated to enforce naming
schemes accordingly. This enables assumptions about library and resource
targets such as __header, __impl, and the newly added __assetres, which
is defined on library targets to be its transitive list of all resource
target deps.

Now compile_resources targets can filter their deps so that they only
depend on transitive resource deps. This allows compile_resources to
not depend on java library targets, and unblocks header generation for
apk targets. Previously an apk's __header target would depend on
compile_resources to finish, while the compile_resources target would
depend on the rest of the java library dependencies (not headers),
completely removing any benefit of header compilation for apk targets.

Bug: 1090812
Change-Id: Ic3c254db9f4d1af2182059119718f15b40957ce9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2220585
Commit-Queue: Peter Wen <wnwen@chromium.org>
Reviewed-by: default avatarAndrew Grieve <agrieve@chromium.org>
Auto-Submit: Peter Wen <wnwen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#775081}
parent b81b79ac
Hide whitespace changes
Inline Side-by-side
Showing with 110 additions and 58 deletions
build/config/android/internal_rules.gni
View file @ c8827ca3
......@@ -16,37 +16,65 @@ import("//build/util/generate_wrapper.gni")
import("//build_overrides/build.gni")
assert(is_android)
# These filters identify most targets that eventually use the java_library_impl
# template.
_java_lib_patterns = [
# The following _java_*_types variables capture all the existing target types.
# If a new type is introduced, please add it to one of these categories,
# preferring the more specific resource/library types.
_java_resource_types = [
"android_assets",
"android_resources",
]
_java_library_types = [
"java_library",
"system_java_library",
"android_app_bundle_module",
]
# These are leaf java target types. They cannot be passed as deps to other
# targets. Thus their naming schemes are not enforced.
_java_leaf_types = [
"android_apk",
"android_app_bundle",
"dist_aar",
"dist_jar",
"java_annotation_processor",
"java_binary",
"junit_binary",
]
# All _java_resource_types targets must conform to these patterns.
_java_resource_patterns = [
"*:*_assets",
"*android*:assets",
"*:*_apk_*resources",
"*android*:resources",
"*:*_resources",
"*:*_grd",
"*:*locale_paks",
"*:*_java_strings",
"*:*strings_java",
]
# All _java_library_types targets must conform to these patterns. This includes
# all non-leaf targets that use java_library_impl.
_java_library_patterns = [
"*:*_java",
"*:*_javalib",
"*:*_java_*", # e.g. java_test_support
"*:*_java_*", # e.g. chrome_java_test_support
"*:java",
"*:junit",
"*:junit_*",
"*:*_junit_*",
"*:*javatests",
"*:*_bundle_module",
# TODO(agrieve): Rename targets below to match above patterns.
"*android_webview/glue*:glue",
"//android_webview/glue/glue:glue",
"//android_webview/glue:glue",
]
# These identify targets that have .build_config files (except for android_apk,
# java_binary, android_app_bundle since we never need to depend on these).
_java_target_patterns = _java_lib_patterns + [
"*:*_assets",
"*android*:assets",
"*:*_apk_*resources",
"*android*:resources",
"*:*_resources",
"*:*_grd",
"*:*locale_paks",
"*_bundle_module",
]
# These targets match _java_lib_patterns but do not use java_library_impl.
_java_lib_exceptions = _java_target_patterns - _java_lib_patterns
# These identify all non-leaf targets that have .build_config files.
_java_target_patterns = _java_library_patterns + _java_resource_patterns
_r8_path = "//third_party/r8/lib/r8.jar"
_desugar_jdk_libs_json = "//third_party/r8/desugar_jdk_libs.json"
......@@ -118,14 +146,24 @@ template("write_build_config") {
_target_label =
get_label_info(":${_parent_invoker.target_name}", "label_no_toolchain")
# 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 != "dist_jar" &&
_type != "java_annotation_processor" && _type != "dist_aar" &&
_type != "android_app_bundle") {
# Ensure targets match naming patterns so that __assetres, __header, __impl
# targets work properly. Those generated targets allow for effective deps
# filtering.
if (filter_exclude([ _type ], _java_resource_types) == []) {
if (filter_exclude([ _target_label ], _java_resource_patterns) != []) {
assert(false, "Invalid java resource target name: $_target_label")
}
} else if (filter_exclude([ _type ], _java_library_types) == []) {
if (filter_exclude([ _target_label ], _java_library_patterns) != [] ||
filter_exclude([ _target_label ], _java_resource_patterns) == []) {
assert(false, "Invalid java library target name: $_target_label")
}
} else if (_type == "group") {
if (filter_exclude([ _target_label ], _java_target_patterns) != []) {
assert(false, "Invalid java target name: $_target_label")
}
} else if (filter_exclude([ _type ], _java_leaf_types) != []) {
assert(false, "This java type needs a category: $_type")
}
if (defined(invoker.public_target_label)) {
......@@ -2171,7 +2209,20 @@ if (enable_java_templates) {
"visibility",
])
_deps = invoker.deps
_deps = [ invoker.build_config_dep ]
if (defined(invoker.android_manifest_dep)) {
_deps += [ invoker.android_manifest_dep ]
}
foreach(_dep, invoker.deps) {
_target_label = get_label_info(_dep, "label_no_toolchain")
if (filter_exclude([ _target_label ], _java_library_patterns) == [] &&
filter_exclude([ _target_label ], _java_resource_patterns) != []) {
# Depend on the java libraries' transitive __assetres target instead.
_deps += [ "${_target_label}__assetres" ]
} else {
_deps += [ _dep ]
}
}
if (defined(invoker.arsc_output)) {
_arsc_output = invoker.arsc_output
......@@ -3025,9 +3076,10 @@ if (enable_java_templates) {
deps = []
foreach(_dep, invoker.deps) {
_target_label = get_label_info(_dep, "label_no_toolchain")
if (filter_exclude([ _target_label ], _java_lib_patterns) == [] &&
filter_exclude([ _target_label ], _java_lib_exceptions) != []) {
# This is a java dep, so replace it.
if (filter_exclude([ _target_label ], _java_library_patterns) == [] &&
filter_exclude([ _target_label ], _java_resource_patterns) !=
[]) {
# This is a java library dep, so replace it.
deps += [ "${_target_label}__${_group_name}" ]
} else {
# Transitive java group targets should also include direct deps.
......@@ -3326,32 +3378,29 @@ if (enable_java_templates) {
}
}
_public_deps = []
_non_java_deps = []
_java_header_deps = []
_java_impl_deps = []
_java_full_deps = []
_analysis_public_deps = []
_sdk_deps = []
_non_java_deps = []
if (defined(invoker.deps)) {
foreach(_dep, invoker.deps) {
_target_label = get_label_info(_dep, "label_no_toolchain")
if (filter_exclude([ _target_label ], _java_lib_patterns) == [] &&
filter_exclude([ _target_label ], _java_lib_exceptions) != []) {
if (filter_exclude([ _target_label ], _java_library_patterns) == [] &&
filter_exclude([ _target_label ], _java_resource_patterns) != []) {
# This is a java dep, so replace it with its header.
_java_header_deps += [ "${_target_label}__header" ]
_java_impl_deps += [ "${_target_label}__impl" ]
_java_full_deps += [ _dep ]
} else {
_non_java_deps += [ _dep ]
}
}
}
_extra_java_deps = []
# TODO(crbug.com/1078484): Don't use desugared .jar files for java binaries.
if (_is_java_binary && enable_bazel_desugar &&
(!defined(invoker.enable_desugar) || invoker.enable_desugar)) {
_non_java_deps += [ "//third_party/bazel/desugar:desugar_runtime_java" ]
_extra_java_deps += [ "//third_party/bazel/desugar:desugar_runtime_java" ]
}
if (_is_prebuilt || _has_sources) {
......@@ -3364,14 +3413,12 @@ if (enable_java_templates) {
!invoker.jacoco_never_instrument && _jacoco_instrument
}
if (_jacoco_instrument) {
_non_java_deps += [ "//third_party/jacoco:jacocoagent_java" ]
_extra_java_deps += [ "//third_party/jacoco:jacocoagent_java" ]
}
}
# Don't need to depend on the apk-under-test to be packaged.
if (defined(invoker.apk_under_test)) {
# No need to add to _java_full_deps since that is only used for
# write_build_config.
_java_header_deps += [ "${invoker.apk_under_test}__java__header" ]
_java_impl_deps += [ "${invoker.apk_under_test}__java__impl" ]
}
......@@ -3387,12 +3434,12 @@ if (enable_java_templates) {
}
# This is an android_system_java_prebuilt target, so no headers.
_sdk_deps = [ _sdk_java_dep ]
_extra_java_deps += [ _sdk_java_dep ]
}
_classpath_deps = _java_header_deps + _non_java_deps + _sdk_deps +
_classpath_deps = _java_header_deps + _non_java_deps + _extra_java_deps +
[ ":$_build_config_target_name" ]
_full_classpath_deps = _java_impl_deps + _non_java_deps + _sdk_deps +
_full_classpath_deps = _java_impl_deps + _non_java_deps + _extra_java_deps +
[ ":$_build_config_target_name" ]
# Often needed, but too hard to figure out when ahead of time.
......@@ -3470,7 +3517,10 @@ if (enable_java_templates) {
enable_jetify = defined(invoker.enable_jetify) && invoker.enable_jetify
is_prebuilt = _is_prebuilt
jetified_jar_path = _jetified_jar_path
possible_config_deps = _java_full_deps + _non_java_deps + _sdk_deps
possible_config_deps = _extra_java_deps
if (defined(invoker.deps)) {
possible_config_deps += invoker.deps
}
if (defined(apk_under_test)) {
possible_config_deps += [ apk_under_test ]
}
......@@ -3513,6 +3563,7 @@ if (enable_java_templates) {
_header_target_name = "${target_name}__header"
}
_analysis_public_deps = []
if (_has_sources) {
if (defined(invoker.enable_errorprone)) {
_enable_errorprone = invoker.enable_errorprone
......@@ -3600,6 +3651,7 @@ if (enable_java_templates) {
}
} # _has_sources
_public_deps = []
if (_is_prebuilt || _has_sources) {
if (_is_prebuilt) {
generate_interface_jar(_header_target_name) {
......@@ -3750,6 +3802,11 @@ if (enable_java_templates) {
public_deps = _public_deps
}
java_lib_group("${target_name}__assetres") {
forward_variables_from(invoker, [ "deps" ])
group_name = "assetres"
}
group(target_name) {
forward_variables_from(invoker,
[
......
build/config/android/rules.gni
View file @ c8827ca3
......@@ -1123,12 +1123,9 @@ if (enable_java_templates) {
# }
# }
template("java_group") {
forward_variables_from(invoker,
[
"testonly",
"input_jars_paths",
])
forward_variables_from(invoker, [ "testonly" ])
write_build_config("$target_name$build_config_target_suffix") {
forward_variables_from(invoker, [ "input_jars_paths" ])
type = "group"
build_config = "$target_gen_dir/${invoker.target_name}.build_config"
supports_android = true
......@@ -1140,6 +1137,7 @@ if (enable_java_templates) {
[
"header",
"impl",
"assetres",
]) {
java_lib_group("${target_name}__${_group_name}") {
forward_variables_from(invoker, [ "deps" ])
......@@ -1360,7 +1358,8 @@ if (enable_java_templates) {
_compile_resources_target = "${target_name}__compile_resources"
compile_resources(_compile_resources_target) {
forward_variables_from(invoker, [ "android_manifest" ])
deps = _deps + [ ":$_build_config_target_name" ]
deps = _deps
build_config_dep = ":$_build_config_target_name"
build_config = _build_config
if (defined(_package_name)) {
rename_manifest_package = _package_name
......@@ -2491,6 +2490,7 @@ if (enable_java_templates) {
short_resource_paths = _short_resource_paths
strip_resource_names = _strip_resource_names
android_manifest = _android_manifest
android_manifest_dep = ":$_merge_manifest_target"
version_code = _version_code
version_name = _version_name
min_sdk_version = _min_sdk_version
......@@ -2498,8 +2498,6 @@ if (enable_java_templates) {
if (defined(invoker.verify_manifest) && invoker.verify_manifest &&
!is_java_debug) {
verify_manifest_target_name = _template_name
build_config_dep = ":$_build_config_target"
android_manifest_dep = ":$_merge_manifest_target"
forward_variables_from(invoker,
[ "expected_manifest_base_expectation" ])
}
......@@ -2523,11 +2521,8 @@ if (enable_java_templates) {
}
build_config = _build_config
deps = _deps + [
":$_merge_manifest_target",
":$_build_config_target",
_android_sdk_dep,
]
build_config_dep = ":$_build_config_target"
deps = _deps + [ _android_sdk_dep ]
# The static library uses the R.txt files generated by the
# static_library_dependent_targets when generating the final R.java file.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment