Remove unneeded extra_r_text_files from resource management
We migrated into an inheritance based R.java files that means that each R.java file has access to all resources without manually specifying each resource individually. This means the extra_r_text_files option is no longer needed/used since we no longer need to know exactly what is in each R.java file separately. Change-Id: I71ebabda0d998a53a0ae54b8a3fd6cd84311105d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2302933 Commit-Queue: Mohamed Heikal <mheikal@chromium.org> Reviewed-by: Peter Wen <wnwen@chromium.org> Cr-Commit-Position: refs/heads/master@{#789316}
This commit is contained in:
Mohamed Heikal
committed by
Commit Bot
Commit Bot
parent
291c96173f
commit
eac6444a79
build
@ -67,7 +67,6 @@ This will end up generating the following JSON file under
|
||||
"resource_zips/ui/android/ui_strings_grd.resources.zip"
|
||||
],
|
||||
"extra_package_names": [],
|
||||
"extra_r_text_files": []
|
||||
}
|
||||
}
|
||||
```
|
||||
@ -130,7 +129,6 @@ python ../../build/android/gyp/process_resources.py \
|
||||
--aapt-path ../../third_party/android_sdk/public/build-tools/29.0.2/aapt \
|
||||
--dependencies-res-zips=@FileArg\(gen/ui/android/ui_java_resources.build_config:resources:dependency_zips\) \
|
||||
--extra-res-packages=@FileArg\(gen/ui/android/ui_java_resources.build_config:resources:extra_package_names\) \
|
||||
--extra-r-text-files=@FileArg\(gen/ui/android/ui_java_resources.build_config:resources:extra_r_text_files\) \
|
||||
--resource-dirs=\[\"../../ui/android/java/res\"\] \
|
||||
--debuggable \
|
||||
--resource-zip-out resource_zips/ui/android/ui_java_resources.resources.zip \
|
||||
|
||||
@ -1117,9 +1117,9 @@ def _OnStaleMd5(options):
|
||||
logging.debug('Creating R.srcjar')
|
||||
resource_utils.CreateRJavaFiles(
|
||||
build.srcjar_dir, package_for_library, build.r_txt_path,
|
||||
options.extra_res_packages, options.extra_r_text_files,
|
||||
rjava_build_options, options.srcjar_out, custom_root_package_name,
|
||||
grandparent_custom_package_name, options.extra_main_r_text_files)
|
||||
options.extra_res_packages, rjava_build_options, options.srcjar_out,
|
||||
custom_root_package_name, grandparent_custom_package_name,
|
||||
options.extra_main_r_text_files)
|
||||
build_utils.ZipDir(build.srcjar_path, build.srcjar_dir)
|
||||
|
||||
# Sanity check that the created resources have the expected package ID.
|
||||
@ -1151,9 +1151,8 @@ def main(args):
|
||||
if options.only_verify_expectations:
|
||||
return
|
||||
|
||||
depfile_deps = (
|
||||
options.dependencies_res_zips + options.extra_main_r_text_files +
|
||||
options.extra_r_text_files + options.include_resources)
|
||||
depfile_deps = (options.dependencies_res_zips +
|
||||
options.extra_main_r_text_files + options.include_resources)
|
||||
|
||||
possible_input_paths = depfile_deps + [
|
||||
options.aapt2_path,
|
||||
|
||||
@ -26,7 +26,6 @@ def _CreateRJava(resource_zips, package_name, srcjar_out, rtxt_out):
|
||||
package_name,
|
||||
rtxt_out,
|
||||
extra_res_packages=[],
|
||||
extra_r_txt_files=[],
|
||||
rjava_build_options=rjava_build_options,
|
||||
srcjar_out=srcjar_out)
|
||||
build_utils.ZipDir(srcjar_out, build.srcjar_dir)
|
||||
|
||||
@ -195,9 +195,9 @@ def _OnStaleMd5(options):
|
||||
|
||||
# Not passing in custom_root_package_name or parent to keep
|
||||
# file names unique.
|
||||
resource_utils.CreateRJavaFiles(
|
||||
build.srcjar_dir, package, r_txt_path, options.extra_res_packages,
|
||||
options.extra_r_text_files, rjava_build_options, options.srcjar_out)
|
||||
resource_utils.CreateRJavaFiles(build.srcjar_dir, package, r_txt_path,
|
||||
options.extra_res_packages,
|
||||
rjava_build_options, options.srcjar_out)
|
||||
|
||||
build_utils.ZipDir(options.srcjar_out, build.srcjar_dir)
|
||||
|
||||
@ -236,7 +236,6 @@ def main(args):
|
||||
possible_input_paths += options.include_resources
|
||||
input_paths = [x for x in possible_input_paths if x]
|
||||
input_paths.extend(options.dependencies_res_zips)
|
||||
input_paths.extend(options.extra_r_text_files)
|
||||
|
||||
# Resource files aren't explicitly listed in GN. Listing them in the depfile
|
||||
# ensures the target will be marked stale when resource files are removed.
|
||||
|
||||
@ -495,7 +495,6 @@ def CreateRJavaFiles(srcjar_dir,
|
||||
package,
|
||||
main_r_txt_file,
|
||||
extra_res_packages,
|
||||
extra_r_txt_files,
|
||||
rjava_build_options,
|
||||
srcjar_out,
|
||||
custom_root_package_name=None,
|
||||
@ -510,9 +509,6 @@ def CreateRJavaFiles(srcjar_dir,
|
||||
main_r_txt_file: The main R.txt file containing the valid values
|
||||
of _all_ resource IDs.
|
||||
extra_res_packages: A list of extra package names.
|
||||
extra_r_txt_files: A list of extra R.txt files. One per item in
|
||||
|extra_res_packages|. Note that all resource IDs in them will be ignored,
|
||||
|and replaced by the values extracted from |main_r_txt_file|.
|
||||
rjava_build_options: An RJavaBuildOptions instance that controls how
|
||||
exactly the R.java file is generated.
|
||||
srcjar_out: Path of desired output srcjar.
|
||||
@ -527,18 +523,14 @@ def CreateRJavaFiles(srcjar_dir,
|
||||
Raises:
|
||||
Exception if a package name appears several times in |extra_res_packages|
|
||||
"""
|
||||
assert len(extra_res_packages) == len(extra_r_txt_files), \
|
||||
'Need one R.txt file per package'
|
||||
rjava_build_options._MaybeRewriteRTxtPackageIds(main_r_txt_file)
|
||||
|
||||
packages = list(extra_res_packages)
|
||||
r_txt_files = list(extra_r_txt_files)
|
||||
|
||||
if package and package not in packages:
|
||||
# Sometimes, an apk target and a resources target share the same
|
||||
# AndroidManifest.xml and thus |package| will already be in |packages|.
|
||||
packages.append(package)
|
||||
r_txt_files.append(main_r_txt_file)
|
||||
|
||||
# Map of (resource_type, name) -> Entry.
|
||||
# Contains the correct values for resources.
|
||||
@ -580,53 +572,19 @@ def CreateRJavaFiles(srcjar_dir,
|
||||
with open(root_r_java_path, 'w') as f:
|
||||
f.write(root_java_file_contents)
|
||||
|
||||
# Map of package_name->resource_type->entry
|
||||
resources_by_package = (
|
||||
collections.defaultdict(lambda: collections.defaultdict(list)))
|
||||
# Build the R.java files using each package's R.txt file, but replacing
|
||||
# each entry's placeholder value with correct values from all_resources.
|
||||
for package, r_txt_file in zip(packages, r_txt_files):
|
||||
if package in resources_by_package:
|
||||
raise Exception(('Package name "%s" appeared twice. All '
|
||||
'android_resources() targets must use unique package '
|
||||
'names, or no package name at all.') % package)
|
||||
resources_by_type = resources_by_package[package]
|
||||
# The sub-R.txt files have the wrong values at this point. Read them to
|
||||
# figure out which entries belong to them, but use the values from the
|
||||
# main R.txt file.
|
||||
for entry in _ParseTextSymbolsFile(r_txt_file):
|
||||
entry = all_resources.get((entry.resource_type, entry.name))
|
||||
# For most cases missing entry here is an error. It means that some
|
||||
# library claims to have or depend on a resource that isn't included into
|
||||
# the APK. There is one notable exception: Google Play Services (GMS).
|
||||
# GMS is shipped as a bunch of AARs. One of them - basement - contains
|
||||
# R.txt with ids of all resources, but most of the resources are in the
|
||||
# other AARs. However, all other AARs reference their resources via
|
||||
# basement's R.java so the latter must contain all ids that are in its
|
||||
# R.txt. Most targets depend on only a subset of GMS AARs so some
|
||||
# resources are missing, which is okay because the code that references
|
||||
# them is missing too. We can't get an id for a resource that isn't here
|
||||
# so the only solution is to skip the resource entry entirely.
|
||||
#
|
||||
# We can verify that all entries referenced in the code were generated
|
||||
# correctly by running Proguard on the APK: it will report missing
|
||||
# fields.
|
||||
if entry:
|
||||
resources_by_type[entry.resource_type].append(entry)
|
||||
|
||||
for package, resources_by_type in resources_by_package.iteritems():
|
||||
for package in packages:
|
||||
_CreateRJavaSourceFile(srcjar_dir, package, root_r_java_package,
|
||||
resources_by_type, rjava_build_options)
|
||||
rjava_build_options)
|
||||
|
||||
|
||||
def _CreateRJavaSourceFile(srcjar_dir, package, root_r_java_package,
|
||||
resources_by_type, rjava_build_options):
|
||||
rjava_build_options):
|
||||
"""Generates an R.java source file."""
|
||||
package_r_java_dir = os.path.join(srcjar_dir, *package.split('.'))
|
||||
build_utils.MakeDirectory(package_r_java_dir)
|
||||
package_r_java_path = os.path.join(package_r_java_dir, 'R.java')
|
||||
java_file_contents = _RenderRJavaSource(
|
||||
package, root_r_java_package, resources_by_type, rjava_build_options)
|
||||
java_file_contents = _RenderRJavaSource(package, root_r_java_package,
|
||||
rjava_build_options)
|
||||
with open(package_r_java_path, 'w') as f:
|
||||
f.write(java_file_contents)
|
||||
|
||||
@ -646,8 +604,7 @@ def _GetNonSystemIndex(entry):
|
||||
return len(res_ids)
|
||||
|
||||
|
||||
def _RenderRJavaSource(package, root_r_java_package, resources_by_type,
|
||||
rjava_build_options):
|
||||
def _RenderRJavaSource(package, root_r_java_package, rjava_build_options):
|
||||
"""Generates the contents of a R.java file."""
|
||||
template = Template(
|
||||
"""/* AUTO-GENERATED FILE. DO NOT MODIFY. */
|
||||
@ -671,7 +628,6 @@ public final class R {
|
||||
|
||||
return template.render(
|
||||
package=package,
|
||||
resources=resources_by_type,
|
||||
resource_types=sorted(_ALL_RESOURCE_TYPES),
|
||||
root_package=root_r_java_package,
|
||||
has_on_resources_loaded=rjava_build_options.has_on_resources_loaded)
|
||||
@ -958,12 +914,6 @@ def ResourceArgsParser():
|
||||
'--extra-res-packages',
|
||||
help='Additional package names to generate R.java files for.')
|
||||
|
||||
input_opts.add_argument(
|
||||
'--extra-r-text-files',
|
||||
help='For each additional package, the R.txt file should contain a '
|
||||
'list of resources to be included in the R.java file in the format '
|
||||
'generated by aapt.')
|
||||
|
||||
return (parser, input_opts, output_opts)
|
||||
|
||||
|
||||
@ -990,12 +940,6 @@ def HandleCommonOptions(options):
|
||||
else:
|
||||
options.extra_res_packages = []
|
||||
|
||||
if options.extra_r_text_files:
|
||||
options.extra_r_text_files = (
|
||||
build_utils.ParseGnList(options.extra_r_text_files))
|
||||
else:
|
||||
options.extra_r_text_files = []
|
||||
|
||||
|
||||
def ParseAndroidResourceStringsFromXml(xml_data):
|
||||
"""Parse and Android xml resource file and extract strings from it.
|
||||
|
||||
@ -129,10 +129,9 @@ the list of `deps_info['package_name']` entries for all `android_resources`
|
||||
dependencies for the current target. Computed automatically by
|
||||
`write_build_config.py`.
|
||||
|
||||
* `deps_info['extra_r_text_files']`:
|
||||
Always empty for `android_resources` types. Otherwise, the list of
|
||||
`deps_info['r_text']` entries for all `android_resources` dependencies for
|
||||
the current target. Computed automatically.
|
||||
* `deps_info['dependency_r_txt_files']`:
|
||||
Exists only on dist_aar. It is the list of deps_info['r_text_path'] from
|
||||
transitive dependencies. Computed automatically.
|
||||
|
||||
|
||||
# `.build_config` target types description:
|
||||
@ -174,7 +173,7 @@ Java package name that the R class for this target belongs to.
|
||||
Optional. Path to the top-level Android manifest file associated with these
|
||||
resources (if not provided, an empty manifest will be used to generate R.txt).
|
||||
|
||||
* `deps_info['r_text']`:
|
||||
* `deps_info['r_text_path']`:
|
||||
Provide the path to the `R.txt` file that describes the resources wrapped by
|
||||
this target. Normally this file is generated from the content of the resource
|
||||
directories or zip file, but some targets can provide their own `R.txt` file
|
||||
@ -182,8 +181,8 @@ if they want.
|
||||
|
||||
* `deps_info['srcjar_path']`:
|
||||
Path to the `.srcjar` file that contains the auto-generated `R.java` source
|
||||
file corresponding to the content of `deps_info['r_text']`. This is *always*
|
||||
generated from the content of `deps_info['r_text']` by the
|
||||
file corresponding to the content of `deps_info['r_text_path']`. This is
|
||||
*always* generated from the content of `deps_info['r_text_path']` by the
|
||||
`build/android/gyp/process_resources.py` script.
|
||||
|
||||
* `deps_info['static_library_dependent_classpath_configs']`:
|
||||
@ -1390,24 +1389,18 @@ def main(argv):
|
||||
c['resources_zip'] for c in all_resources_deps if c['resources_zip']
|
||||
]
|
||||
extra_package_names = []
|
||||
extra_r_text_files = []
|
||||
|
||||
if options.type != 'android_resources':
|
||||
extra_package_names = [
|
||||
c['package_name'] for c in all_resources_deps if 'package_name' in c
|
||||
]
|
||||
extra_r_text_files = [
|
||||
c['r_text_path'] for c in all_resources_deps if 'r_text_path' in c
|
||||
]
|
||||
# In final types (i.e. apks and modules) that create real R.java files,
|
||||
# they must collect package names and rtxt from java_libraries as well.
|
||||
# they must collect package names from java_libraries as well.
|
||||
# https://crbug.com/1073476
|
||||
if options.type != 'java_library':
|
||||
extra_package_names.extend([
|
||||
c['package_name'] for c in all_library_deps if 'package_name' in c
|
||||
])
|
||||
extra_r_text_files.extend(
|
||||
[c['r_text_path'] for c in all_library_deps if 'r_text_path' in c])
|
||||
|
||||
|
||||
# For feature modules, remove any resources that already exist in the base
|
||||
@ -1421,14 +1414,9 @@ def main(argv):
|
||||
c for c in extra_package_names if c not in
|
||||
base_module_build_config['deps_info']['extra_package_names']
|
||||
]
|
||||
extra_r_text_files = [
|
||||
c for c in extra_r_text_files if c not in
|
||||
base_module_build_config['deps_info']['extra_r_text_files']
|
||||
]
|
||||
|
||||
config['deps_info']['dependency_zips'] = dependency_zips
|
||||
config['deps_info']['extra_package_names'] = extra_package_names
|
||||
config['deps_info']['extra_r_text_files'] = extra_r_text_files
|
||||
if options.type == 'android_apk' and options.tested_apk_config:
|
||||
config['deps_info']['arsc_package_name'] = (
|
||||
tested_apk_config['package_name'])
|
||||
@ -1442,6 +1430,14 @@ def main(argv):
|
||||
options.extra_classpath_jars)
|
||||
deps_info['extra_classpath_jars'] = extra_classpath_jars
|
||||
|
||||
if options.type == 'dist_aar':
|
||||
# dist_aar combines all dependency R.txt files into one for the aar.
|
||||
r_text_files = [
|
||||
c['r_text_path'] for c in all_resources_deps + all_library_deps
|
||||
if 'r_text_path' in c
|
||||
]
|
||||
deps_info['dependency_r_txt_files'] = r_text_files
|
||||
|
||||
if is_java_target:
|
||||
# The classpath used to compile this target when annotation processors are
|
||||
# present.
|
||||
@ -1619,9 +1615,6 @@ def main(argv):
|
||||
# The srcjars containing the generated R.java files are excluded for APK
|
||||
# targets the use static libraries, so we add them here to ensure the
|
||||
# union of resource IDs are available in the static library APK.
|
||||
for r_text in base_config['extra_r_text_files']:
|
||||
if r_text not in extra_r_text_files:
|
||||
extra_r_text_files.append(r_text)
|
||||
for package in base_config['extra_package_names']:
|
||||
if package not in extra_package_names:
|
||||
extra_package_names.append(package)
|
||||
|
||||
@ -2112,7 +2112,6 @@ if (enable_java_templates) {
|
||||
"--include-resources=@FileArg($_rebased_build_config:android:sdk_jars)",
|
||||
"--dependencies-res-zips=@FileArg($_rebased_build_config:deps_info:dependency_zips)",
|
||||
"--extra-res-packages=@FileArg($_rebased_build_config:deps_info:extra_package_names)",
|
||||
"--extra-r-text-files=@FileArg($_rebased_build_config:deps_info:extra_r_text_files)",
|
||||
"--res-sources-path=$_rebased_res_sources_path",
|
||||
"--resource-zip-out",
|
||||
rebase_path(invoker.resources_zip, root_build_dir),
|
||||
@ -2329,7 +2328,6 @@ if (enable_java_templates) {
|
||||
rebase_path(android_sdk_tools_bundle_aapt2, root_build_dir),
|
||||
"--dependencies-res-zips=@FileArg($_rebased_build_config:deps_info:dependency_zips)",
|
||||
"--extra-res-packages=@FileArg($_rebased_build_config:deps_info:extra_package_names)",
|
||||
"--extra-r-text-files=@FileArg($_rebased_build_config:deps_info:extra_r_text_files)",
|
||||
"--extra-main-r-text-files=@FileArg($_rebased_build_config:deps_info:extra_main_r_text_files)",
|
||||
"--min-sdk-version=${invoker.min_sdk_version}",
|
||||
"--target-sdk-version=${invoker.target_sdk_version}",
|
||||
|
||||
@ -1783,7 +1783,7 @@ if (enable_java_templates) {
|
||||
"--output",
|
||||
rebase_path(invoker.output, root_build_dir),
|
||||
"--dependencies-res-zips=@FileArg($_rebased_build_config:deps_info:dependency_zips)",
|
||||
"--r-text-files=@FileArg($_rebased_build_config:deps_info:extra_r_text_files)",
|
||||
"--r-text-files=@FileArg($_rebased_build_config:deps_info:dependency_r_txt_files)",
|
||||
"--proguard-configs=@FileArg($_rebased_build_config:deps_info:proguard_all_configs)",
|
||||
]
|
||||
if (_direct_deps_only) {
|
||||
|
||||
Reference in New Issue
Block a user