Skip to content

Commit

Permalink
Revert of 🎊 Have build_config targets depend only on other build_conf…
Browse files Browse the repository at this point in the history 
…ig targets (patchset chromium#3 id:40001 of https://codereview.chromium.org/2095913003/ )

Reason for revert:
Sheriff: reverting on suspicion of breaking the android build.

It looks like the apk packaging step fails when a required component is not found. This change looked like a likely culprit since it changes the naming conventions, and the problem was file not found.

Link to failed build here:
https://build.chromium.org/p/chromium/builders/Android/builds/58342

Relevant section of build logs here:
FAILED: gen/android_webview/system_webview_apk__create_incremental__package.d gen/android_webview/system_webview_apk/system_webview_apk.apk_intermediates_incremental.unfinished.apk
python ../../build/android/gyp/apkbuilder.py --depfile gen/android_webview/system_webview_apk__create_incremental__package.d --resource-apk=gen/android_webview/system_webview_apk/system_webview_apk.apk_intermediates_incremental.ap_ --output-apk=gen/android_webview/system_webview_apk/system_webview_apk.apk_intermediates_incremental.unfinished.apk --assets=@FileArg\(gen/android_webview/system_webview_apk.build_config:assets\) --uncompressed-assets=@FileArg\(gen/android_webview/system_webview_apk.build_config:uncompressed_assets\) --dex-file=gen/build/android/incremental_install/bootstrap.dex --android-abi=armeabi-v7a --native-lib-placeholders=\[\"libfix.crbug.384638.so\"\]
Traceback (most recent call last):
  File "../../build/android/gyp/apkbuilder.py", line 311, in <module>
    main(sys.argv[1:])
  File "../../build/android/gyp/apkbuilder.py", line 307, in main
    depfile_deps=depfile_deps)
  File "/b/build/slave/Android/build/src/build/android/gyp/util/build_utils.py", line 527, in CallAndWriteDepfileIfStale
    pass_changes=True)
  File "/b/build/slave/Android/build/src/build/android/gyp/util/md5_check.py", line 63, in CallAndRecordIfStale
    new_metadata.AddFile(path, _Md5ForPath(path))
  File "/b/build/slave/Android/build/src/build/android/gyp/util/md5_check.py", line 381, in _Md5ForPath
    _UpdateMd5ForFile(md5, path)
  File "/b/build/slave/Android/build/src/build/android/gyp/util/md5_check.py", line 362, in _UpdateMd5ForFile
    with open(path, 'rb') as infile:
IOError: [Errno 2] No such file or directory: u'snapshot_blob.bin'
[34308/48394] ACTION //android_webview/test:android_webview_apk__create_incremental__package(//build/toolchain/android:arm)
FAILED: gen/android_webview/test/android_webview_apk__create_incremental__package.d gen/android_webview/test/android_webview_apk/android_webview_apk.apk_intermediates_incremental.unfinished.apk
python ../../build/android/gyp/apkbuilder.py --depfile gen/android_webview/test/android_webview_apk__create_incremental__package.d --resource-apk=gen/android_webview/test/android_webview_apk/android_webview_apk.apk_intermediates_incremental.ap_ --output-apk=gen/android_webview/test/android_webview_apk/android_webview_apk.apk_intermediates_incremental.unfinished.apk --assets=@FileArg\(gen/android_webview/test/android_webview_apk.build_config:assets\) --uncompressed-assets=@FileArg\(gen/android_webview/test/android_webview_apk.build_config:uncompressed_assets\) --dex-file=gen/build/android/incremental_install/bootstrap.dex --android-abi=armeabi-v7a --native-lib-placeholders=\[\"libfix.crbug.384638.so\"\]
Traceback (most recent call last):
  File "../../build/android/gyp/apkbuilder.py", line 311, in <module>
    main(sys.argv[1:])
  File "../../build/android/gyp/apkbuilder.py", line 307, in main
    depfile_deps=depfile_deps)
  File "/b/build/slave/Android/build/src/build/android/gyp/util/build_utils.py", line 527, in CallAndWriteDepfileIfStale
    pass_changes=True)
  File "/b/build/slave/Android/build/src/build/android/gyp/util/md5_check.py", line 63, in CallAndRecordIfStale
    new_metadata.AddFile(path, _Md5ForPath(path))
  File "/b/build/slave/Android/build/src/build/android/gyp/util/md5_check.py", line 381, in _Md5ForPath
    _UpdateMd5ForFile(md5, path)
  File "/b/build/slave/Android/build/src/build/android/gyp/util/md5_check.py", line 362, in _UpdateMd5ForFile
    with open(path, 'rb') as infile:
IOError: [Errno 2] No such file or directory: u'snapshot_blob.bin'

Original issue's description:
> Have build_config targets depend only on other build_config targets
>
> This works by having android target names follow a naming convention.
> If a target name matches, then it is assumed to have a .build_config file.
> gn gen will raise an error if it doesn't. Likewise, if a target name
> does not match the filters, but does create a .build_config file, gn gen
> will fail on an assert.
>
> This change adds a bunch of naming-convention exceptions, which will
> be addressed in subsequent commits.
>
> Finally, why make this change? This allows all .build_config targets to be
> generated in just a few seconds, which will allow fast generation of
> build.gradle files for Android Studio integration.
>
> BUG=620034
>
> Committed: https://crrev.com/1029ca88161324eff7a62b1e2eecc84d5acf0f6c
> Cr-Commit-Position: refs/heads/master@{#402934}

TBR=brettw@chromium.org,agrieve@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=620034

Review-Url: https://codereview.chromium.org/2110943002
Cr-Commit-Position: refs/heads/master@{#402949}
  • Loading branch information
petewil authored and Commit bot committed Jun 29, 2016
1 parent 0af422d commit 7058618
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 153 deletions.
15 changes: 12 additions & 3 deletions build/android/gyp/write_build_config.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,10 @@ def main(argv):
'--type',
help='Type of this target (e.g. android_library).')
parser.add_option(
'--deps-configs',
help='List of paths for dependency\'s build_config files. ')
'--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.')

# android_resources options
parser.add_option('--srcjar', help='Path to target\'s resources srcjar.')
Expand Down Expand Up @@ -288,7 +290,14 @@ def main(argv):
raise Exception(
'--supports-android is required when using --requires-android')

direct_deps_config_paths = build_utils.ParseGypList(options.deps_configs)
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 = _FilterUnwantedDepsPaths(direct_deps_config_paths,
options.type)

Expand Down
133 changes: 19 additions & 114 deletions build/config/android/internal_rules.gni
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,58 +7,6 @@ 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
Expand All @@ -69,34 +17,9 @@ _java_target_blacklist = [
# 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" ||
Expand All @@ -108,6 +31,7 @@ template("write_build_config") {
[
"deps",
"testonly",
"visibility",
])
if (!defined(deps)) {
deps = []
Expand All @@ -117,32 +41,14 @@ template("write_build_config") {
depfile = "$target_gen_dir/$target_name.d"
inputs = []

_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([])
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" ]
}
_rebased_deps_configs = rebase_path(_deps_configs, root_build_dir)
rebase_possible_deps_configs =
rebase_path(possible_deps_configs, root_build_dir)

outputs = [
depfile,
Expand All @@ -154,7 +60,7 @@ template("write_build_config") {
type,
"--depfile",
rebase_path(depfile, root_build_dir),
"--deps-configs=$_rebased_deps_configs",
"--possible-deps-configs=$rebase_possible_deps_configs",
"--build-config",
rebase_path(build_config, root_build_dir),
]
Expand Down Expand Up @@ -224,11 +130,13 @@ 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" ]
Expand Down Expand Up @@ -1693,9 +1601,7 @@ if (enable_java_templates) {
requires_android =
defined(invoker.requires_android) && invoker.requires_android

if (defined(invoker.deps)) {
possible_config_deps = _deps
}
deps = _deps
build_config = _build_config
jar_path = _jar_path
if (_supports_android) {
Expand Down Expand Up @@ -2121,14 +2027,12 @@ 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) &&
Expand Down Expand Up @@ -2430,10 +2334,11 @@ if (enable_java_templates) {
build_config_target_name = "${target_name}__build_config"

write_build_config(build_config_target_name) {
forward_variables_from(invoker, [ "dex_path" ])
if (defined(invoker.deps)) {
possible_config_deps = invoker.deps
}
forward_variables_from(invoker,
[
"deps",
"dex_path",
])
type = "deps_dex"
build_config = build_config
}
Expand Down
49 changes: 16 additions & 33 deletions build/config/android/rules.gni
View file
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,6 @@ 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)
Expand Down Expand Up @@ -747,36 +746,30 @@ 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
} else {
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",
Expand All @@ -793,9 +786,6 @@ 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
Expand Down Expand Up @@ -859,15 +849,13 @@ 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
}
Expand Down Expand Up @@ -907,18 +895,13 @@ 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" ]
}
}
Expand Down Expand Up @@ -946,6 +929,7 @@ 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"
}

Expand All @@ -955,7 +939,6 @@ 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",
Expand Down Expand Up @@ -1016,6 +999,7 @@ if (enable_java_templates) {
final_target_name = target_name

write_build_config(build_config_target_name) {
visibility = [ ":$zip_target_name" ]
type = "android_resources"
}

Expand Down Expand Up @@ -1626,19 +1610,18 @@ if (enable_java_templates) {
android_manifest = _android_manifest

deps = _android_manifest_deps

if (defined(invoker.deps)) {
possible_config_deps = invoker.deps
deps += invoker.deps
}

if (defined(invoker.alternative_locale_resource_dep)) {
possible_config_deps += [ invoker.alternative_locale_resource_dep ]
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) {
possible_config_deps += [ "//third_party/android_tools:emma_device" ]
deps += [ "//third_party/android_tools:emma_device" ]
}

proguard_enabled = _proguard_enabled
Expand Down
4 changes: 3 additions & 1 deletion chromecast/browser/android/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@ jinja_template("cast_shell_manifest") {
# GYP target: n/a
android_resources("cast_shell_android_resources") {
android_manifest = "$root_gen_dir/cast_shell_manifest/AndroidManifest.xml"
android_manifest_dep = ":cast_shell_manifest"
resource_dirs = [ "//chromecast/browser/android/apk/res" ]
deps = [
":cast_shell_manifest",
]
}

# GYP target: chromecast.gyp:cast_shell_java
Expand Down
Loading

0 comments on commit 7058618

Please sign in to comment.