diff --git a/.bazelrc b/.bazelrc index f9707cce..38853235 100644 --- a/.bazelrc +++ b/.bazelrc @@ -1,30 +1,40 @@ # BzlMod common --enable_bzlmod=false -# Cache +# Cache setup common --disk_cache=bazel-cache common --remote_cache_compression=true +common --remote_upload_local_results=false # Don't upload results by default startup --digest_function=blake3 -common --experimental_action_cache_store_output_metadata +common --experimental_remote_cache_eviction_retries=10 common --modify_execution_info=JavaDeployJar=+no-remote-cache # Don't cache Java deploy jar which is huge in size +common --remote_cache_compression=true +common --experimental_disk_cache_gc_max_size=4G -# Errors +# Error config common --verbose_failures -# Action Env +# Env config common --incompatible_strict_action_env common --reuse_sandbox_directories common --repo_env=RJE_VERBOSE=true +# JVM External rules +common --@rules_jvm_external//settings:stamp_manifest=False + +# Resources +common --local_resources=cpu=HOST_CPUS*.75 # Android actions start their own threads which can overwhelm the system + # JAVA - START common --experimental_strict_java_deps=off # Turn off strict java deps common --java_runtime_version=remotejdk_17 # Use inbuilt Java 17 for hermeticity common --tool_java_runtime_version=remotejdk_17 common --tool_java_language_version=17 common --java_language_version=17 +## See https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/Locale.html#legacy_language_codes common --jvmopt="-Djava.locale.providers=COMPAT,SPI" # Use Java 8 default locale provider common --jvmopt="--add-exports=java.xml/com.sun.org.apache.xerces.internal.dom=ALL-UNNAMED" -common --experimental_java_classpath=bazel +common --experimental_java_classpath=bazel common --experimental_java_header_input_pruning # JAVA - END @@ -46,37 +56,60 @@ common --output_library_merged_assets=false # Turn off asset merging artifact # Workers common --worker_verbose -common --experimental_worker_multiplex +common --worker_multiplex common --experimental_shrink_worker_pool common --experimental_worker_for_repo_fetching=platform +common --experimental_collect_worker_data_in_profiler +## Android Resource Workers common --experimental_persistent_aar_extractor -common --persistent_multiplex_android_tools common --persistent_android_dex_desugar +common --persistent_android_resource_processor +common --persistent_android_dex_desugar +common --persistent_multiplex_android_dex_desugar +common --persistent_multiplex_android_resource_processor +common --persistent_multiplex_android_tools +# Action Strategies +common --strategy=AARGenerator=worker common --strategy=DatabindingStubs=worker -common --worker_max_instances=Javac=1 -common --worker_max_instances=KotlinCompile=1 -common --worker_max_instances=KotlinKapt=1 +common --modify_execution_info=GenerateDataBindingBaseClasses=+supports-multiplex-workers=1 +## Java Workers +common --strategy=KotlinCompile=worker +common --strategy=Javac=worker +# common --strategy=Turbine=worker +## Worker configuration to avoid CPU thrashing https://github.com/bazelbuild/bazel/issues/8586#issuecomment-500070549 +common --worker_max_instances=Aapt2Optimize=1 common --worker_max_instances=AaptPackage=1 -common --worker_max_instances=AndroidResourceParser=1 +common --worker_max_instances=AndroidAapt2=1 +common --worker_max_instances=AndroidAssetMerger=1 +common --worker_max_instances=AndroidCompiledResourceMerger=1 common --worker_max_instances=AndroidResourceCompiler=1 +common --worker_max_instances=AndroidResourceMerger=1 +common --worker_max_instances=AndroidResourceParser=1 common --worker_max_instances=AndroidResourceValidator=1 common --worker_max_instances=AndroidLintAnalyze=1 common --worker_max_instances=AndroidLint=1 -common --worker_max_instances=RClassGenerator=1 -common --worker_max_instances=AndroidAapt2=1 -common --worker_max_instances=AndroidAssetMerger=1 -common --worker_max_instances=AndroidResourceMerger=1 -common --worker_max_instances=AndroidCompiledResourceMerger=1 -common --worker_max_instances=Aapt2Optimize=1 +common --worker_max_instances=BuildConfigGenerationWorker=1 common --worker_max_instances=DatabindingStubs=1 -common --worker_max_instances=GenerateDataBindingBaseClasses=1 -common --worker_max_instances=DexBuilder=1 +common --worker_max_instances=DatabindingWorker=1 common --worker_max_instances=Desugar=1 +common --worker_max_instances=DexBuilder=1 +common --worker_max_instances=GenerateDataBindingBaseClasses=1 +common --worker_max_instances=Javac=1 +common --worker_max_instances=JdepsMerge=1 +common --worker_max_instances=KotlinCompile=1 +common --worker_max_instances=KotlinKapt=1 +common --worker_max_instances=MergeSourceSets=1 +common --worker_max_instances=ProcessDatabinding=1 +common --worker_max_instances=RClassGenerator=1 +common --worker_max_instances=Turbine=1 -test --test_output=errors # Print test logs for failed tests +# Test config test --build_tests_only +test --test_verbose_timeout_warnings +test --test_output=errors # Print test logs for failed tests +test --test_summary=terse # Print information only about unsuccessful tests that were run run --ui_event_filters=-info,-stdout --noshow_progress diff --git a/.bazelversion b/.bazelversion index 468c41f9..b6167179 100644 --- a/.bazelversion +++ b/.bazelversion @@ -1 +1 @@ -7.2.1 \ No newline at end of file +7.4.0 \ No newline at end of file diff --git a/rules/android/android_binary.bzl b/rules/android/android_binary.bzl index 9ad1f3ae..40d1b864 100644 --- a/rules/android/android_binary.bzl +++ b/rules/android/android_binary.bzl @@ -1,15 +1,15 @@ -load("@grab_bazel_common//tools/build_config:build_config.bzl", _build_config = "build_config") -load("@grab_bazel_common//tools/kotlin:android.bzl", "kt_android_library") load("@grab_bazel_common//rules/android/databinding:databinding.bzl", "DATABINDING_DEPS") load("@grab_bazel_common//rules/android/lint:defs.bzl", "LINT_ENABLED", "lint", "lint_sources", _lint_baseline = "baseline") load("@grab_bazel_common//rules/check/detekt:defs.bzl", "detekt") +load("@grab_bazel_common//tools/build_config:build_config.bzl", _build_config = "build_config") +load("@grab_bazel_common//tools/kotlin:android.bzl", "kt_android_library") load(":resources.bzl", "build_resources") def android_binary( name, debug = True, build_config = {}, - custom_package = {}, + custom_package = "", res_values = {}, enable_data_binding = False, enable_compose = False, @@ -44,13 +44,17 @@ def android_binary( longs = build_config.get("longs", default = {}), strings = build_config.get("strings", default = {}), ) - - resource_files = build_resources( + merged_resources = build_resources( name = name, + is_binary = True, + namespace = attrs.get("manifest_values")["applicationId"], + manifest = attrs.get("manifest", None), resource_files = attrs.get("resource_files", default = []), - resources = attrs.get("resources", default = {}), + resource_sets = attrs.get("resource_sets", default = {}), res_values = res_values, ) + resource_files = merged_resources.res + manifest = merged_resources.manifest # Kotlin compilation with kt_android_library kotlin_target = "lib_" + name @@ -61,10 +65,10 @@ def android_binary( kt_android_library( name = kotlin_target, srcs = attrs.get("srcs", default = []), - assets = attrs.get("assets", default = None), - assets_dir = attrs.get("assets_dir", default = None), + assets = merged_resources.assets, + assets_dir = merged_resources.asset_dir, custom_package = custom_package, - manifest = attrs.get("manifest", default = None), + manifest = manifest, resource_files = resource_files, visibility = attrs.get("visibility", default = None), deps = kotlin_library_deps, @@ -73,6 +77,7 @@ def android_binary( lint_enabled = lint_options.get("enabled", False) and (len(attrs.get("srcs", default = [])) > 0 or len(resource_files) > 0) tags = [] android_binary_deps = [kotlin_target] + if lint_enabled: lint_sources_target = "_" + name + "_lint_sources" lint_baseline = _lint_baseline(lint_options.get("baseline", None)) @@ -80,7 +85,7 @@ def android_binary( name = lint_sources_target, srcs = attrs.get("srcs", default = []), resources = [file for file in resource_files if file.endswith(".xml")], - manifest = attrs.get("manifest"), + manifest = manifest, baseline = lint_baseline, lint_config = lint_options.get("config", None), deps = kotlin_library_deps, @@ -127,7 +132,7 @@ def android_binary( dexopts = attrs.get("dexopts", default = None), incremental_dexing = attrs.get("incremental_dexing", default = None), javacopts = attrs.get("javacopts", default = None), - manifest = attrs.get("manifest"), + manifest = manifest, multidex = attrs.get("multidex", default = None), manifest_values = attrs.get("manifest_values", default = None), resource_configuration_filters = attrs.get("resource_configuration_filters", default = None), diff --git a/rules/android/android_library.bzl b/rules/android/android_library.bzl index 28f5173e..40dc2c58 100644 --- a/rules/android/android_library.bzl +++ b/rules/android/android_library.bzl @@ -1,16 +1,16 @@ -load("@grab_bazel_common//tools/build_config:build_config.bzl", _build_config = "build_config") -load("@grab_bazel_common//tools/kotlin:android.bzl", "kt_android_library") load("@grab_bazel_common//rules/android/databinding:databinding.bzl", "kt_db_android_library") -load(":resources.bzl", "build_resources") load("@grab_bazel_common//rules/android/lint:defs.bzl", "LINT_ENABLED", "lint", "lint_sources") load("@grab_bazel_common//rules/check/detekt:defs.bzl", "detekt") +load("@grab_bazel_common//tools/build_config:build_config.bzl", _build_config = "build_config") +load("@grab_bazel_common//tools/kotlin:android.bzl", "kt_android_library") +load(":resources.bzl", "build_resources") def android_library( name, debug = True, srcs = [], build_config = {}, - custom_package = {}, + custom_package = "", res_values = {}, enable_data_binding = False, enable_compose = False, @@ -46,16 +46,22 @@ def android_library( strings = build_config.get("strings", default = {}), ) - resource_files = build_resources( + merged_resources = build_resources( name = name, + is_binary = False, + namespace = custom_package, + manifest = attrs.get("manifest", None), resource_files = attrs.get("resource_files", default = []), - resources = attrs.get("resources", default = {}), + resource_sets = attrs.get("resource_sets", default = {}), res_values = res_values, ) + resource_files = merged_resources.res + manifest = merged_resources.manifest lint_enabled = lint_options.get("enabled", False) and (len(srcs) > 0 or len(resource_files) > 0) android_library_deps = attrs.get("deps", default = []) + [build_config_target] tags = attrs.get("tags", default = []) + if lint_enabled: lint_sources_target = "_" + name + "_lint_sources" lint_baseline = lint_options.get("baseline", None) @@ -63,7 +69,7 @@ def android_library( name = lint_sources_target, srcs = srcs, resources = [file for file in resource_files if file.endswith(".xml")], - manifest = attrs.get("manifest"), + manifest = manifest, baseline = lint_baseline, lint_config = lint_options.get("config", None), deps = android_library_deps, @@ -115,10 +121,10 @@ def android_library( name = name, srcs = srcs, custom_package = custom_package, - manifest = attrs.get("manifest"), + manifest = manifest, resource_files = resource_files, - assets = attrs.get("assets", default = None), - assets_dir = attrs.get("assets_dir", default = None), + assets = merged_resources.assets, + assets_dir = merged_resources.asset_dir, visibility = attrs.get("visibility", default = None), tags = tags, deps = android_library_deps, diff --git a/rules/android/private/resource_merger.bzl b/rules/android/private/resource_merger.bzl index 9a43bf5a..92f2f41c 100644 --- a/rules/android/private/resource_merger.bzl +++ b/rules/android/private/resource_merger.bzl @@ -1,19 +1,23 @@ +load("@grab_bazel_common//rules/android:utils.bzl", "utils") + """ -Rule to merge android variant specific resource folders and account for overrides. +Rule to merge android variant specific resource folders, assets and manifest and account for overrides. """ -def _to_path(f): - return f.path - def _resource_merger_impl(ctx): outputs = ctx.outputs.merged_resources + merged_manifest = ctx.outputs.merged_manifest # Args for compiler args = ctx.actions.args() args.set_param_file_format("multiline") args.use_param_file("--flagfile=%s", use_always = True) args.add("RESOURCE_MERGER") + args.add("--label", ctx.label) + if ctx.attr.is_binary: + args.add("--is-binary") args.add("--target", ctx.label.package) + args.add("--package-name", ctx.attr.namespace) args.add_joined( "--source-sets", ctx.attr.source_sets, @@ -23,17 +27,18 @@ def _resource_merger_impl(ctx): "--output", outputs, join_with = ",", - map_each = _to_path, + map_each = utils.to_path, ) + args.add("--manifest", merged_manifest) mnemonic = "MergeSourceSets" ctx.actions.run( mnemonic = mnemonic, inputs = depset(ctx.files.resources + ctx.files.manifests), - outputs = outputs, + outputs = ctx.outputs.merged_resources + [merged_manifest], executable = ctx.executable._compiler, arguments = [args], - progress_message = "%s %s" % (mnemonic, ctx.label), + progress_message = "%s %s" % (mnemonic, str(ctx.label).lstrip("@")), execution_requirements = { "supports-workers": "1", "supports-multiplex-workers": "1", @@ -47,9 +52,12 @@ def _resource_merger_impl(ctx): resource_merger = rule( implementation = _resource_merger_impl, attrs = { + "is_binary": attr.bool(mandatory = True, default = False), "source_sets": attr.string_list(), + "namespace": attr.string(mandatory = True), "resources": attr.label_list(allow_files = True, mandatory = True), "manifests": attr.label_list(allow_files = True, mandatory = True), + "merged_manifest": attr.output(mandatory = True), "merged_resources": attr.output_list(mandatory = True), "_compiler": attr.label( default = Label("@grab_bazel_common//tools/aapt_lite:aapt_lite"), diff --git a/rules/android/resources.bzl b/rules/android/resources.bzl index e143bb11..840f87dd 100644 --- a/rules/android/resources.bzl +++ b/rules/android/resources.bzl @@ -1,96 +1,175 @@ -load("@grab_bazel_common//tools/res_value:res_value.bzl", "res_value") load("@grab_bazel_common//rules/android/private:resource_merger.bzl", "resource_merger") +load("@grab_bazel_common//tools/res_value:res_value.bzl", "res_value") def _calculate_output_files(name, all_resources): """ - Resource merger would merge source resource files and write to a merged directory. Bazel needs to know output files in advance, so this - method tries to predict the output files so we can register them as predeclared outputs. + Calculates the output file paths Bazel would create from the given resources + + Resource merger would merge resources and write to a single merged directory. Bazel needs to know output files in advance, so this + method tries to predict the output files so we can register them as predeclared outputs. We can't use `actions.declare_dir` since + `android_binary|library` does not accept a directory as input. Args: - all_resources: All resource files sorted based on priority with higher priority appearing first. + name: Name of the target the does the merging + all_resources: All resource_files/assets sorted based on priority with higher priority appearing first """ - outputs = [] - # Multiple res folders root can contain same file name of resource, prevent creating same outputs by storing normalized resource paths - # eg: `res/values/strings.xml` - normalized_res_paths = {} + # Multiple res folders root can contain same file name of resource, dedup them using a dict + outputs = {} + + # Two different resource buckets can contain different file extensions but same file name. For example icon.png and icon.webp, based on + # what comes first, only one file will be present after merging. Track such cases and remove them from outputs. + output_file_names = {} for file in all_resources: res_name_and_dir = file.split("/")[-2:] # ["values", "values.xml"] etc res_dir = res_name_and_dir[0] res_name = res_name_and_dir[1] + res_name_no_ext = res_name.split(".")[0] # Just the file name "values" + + out_res_dir = "/res" if res_dir != "assets" else "" # Merged assets folder will not have a parent res dir if "values" in res_dir: # Resource merging merges all values files into single values.xml file. - normalized_res_path = "%s/out/res/%s/values.xml" % (name, res_dir) + normalized_res_path = "%s/out%s/%s/values.xml" % (name, out_res_dir, res_dir) + outputs[normalized_res_path] = normalized_res_path else: - normalized_res_path = "%s/out/res/%s/%s" % (name, res_dir, res_name) + normalized_res_path = "%s/out%s/%s/%s" % (name, out_res_dir, res_dir, res_name) + if res_name_no_ext not in output_file_names: + # Dedupe with resource name for any resource that is of type `values`. + outputs[normalized_res_path] = normalized_res_path + output_file_names[res_name_no_ext] = res_name_no_ext + + return list(outputs.values()) - if normalized_res_path not in normalized_res_paths: - normalized_res_paths[normalized_res_path] = normalized_res_path - outputs.append(normalized_res_path) - return outputs +def _res_glob(includes = []): + """ + `glob` wrapper to exclude common problematic files and glob a directory as a whole using /** + + Args: + includes: List of string directory path + """ + return native.glob( + include = [include + "/**" for include in includes], + exclude = ["**/.DS_Store"], + ) + +def _generate_resources(res_value_strings, name): + if res_value_strings: + return res_value(name = name + "_res_value", strings = res_value_strings) + else: + return [] + +def _validate_resource_parameters(resource_sets, resource_files): + if len(resource_files) != 0: + fail("resouce_files is deprecrated, migrate to using resources format. See resources.bzl") def build_resources( name, + is_binary, + namespace, + manifest, resource_files, - resources, + resource_sets, res_values): """ - Calculates and returns resource_files either generated, merged or just the source ones based on parameters given. When `resources` are - declared and it has multiple resource roots then all those roots are merged into single directory and contents of the directory are returned. - Conversely if resource_files are used then sources are returned as is. In both cases, generated resources passed via res_values are - accounted for. + Merge multiple source sets and return a single merged output + + Calculates and returns resource_files, assets and manifest either generated, merged or just the source ones based on parameters given. + When `resource_sets` are declared and it has multiple resource roots then all those roots are merged into single directory and + contents of the directory are returned. + Using `resource_files` is an error and not recommended. + + Args: + name: The name of the resource merger target + is_binary: Whether the target is android_binary + manifest: The default primary manifest. + namespace: The namespace of this target + resource_files: Default bazel expected Android resource_files format (deprecated) + resource_sets: Dict of various resources, manifest and assets keyed by a source set name + For example + "main": { + "res": "src/main/res", + "manifest": "src/main/AndroidManifest.xml", + "assets": "src/main/assets", + } + res_values: Dict of various resources keyed by their type to be generated during build. Uses res_value + + Returns: + - resources: Merged output containing resources, assets, assets_dir and manifest. """ - generated_resources = [] - res_value_strings = res_values.get("strings", default = {}) - if len(res_value_strings) != 0: - generated_resources = res_value( - name = name + "_res_value", - strings = res_value_strings, - ) + generated_resources = _generate_resources(res_values.get("strings", default = {}), name) + _validate_resource_parameters(resource_sets, resource_files) + + if (len(resource_sets) != 0): + # Resources are passed with the new format, merge sources and return the merged result + if (len(resource_sets) == 1): + # Only only source set is provided, hence glob it and return as-is. + resource_set_name = resource_sets.keys()[0] + resource_dict = resource_sets.get(resource_set_name) - if (len(resources) != 0 and len(resource_files) != 0): - fail("Either resources or resource_files should be specified but not both") - - if (len(resources) != 0): - # Resources are passed with the new format - # Merge sources and return the merged result - - if (len(resources) == 1): - resource_dir = resources.keys()[0] - return native.glob( - include = [resource_dir + "/**"], - exclude = ["**/.DS_Store"], - ) + generated_resources - - source_sets = [] # Source sets in the format res_dir::manifest - all_resources = [] - all_manifests = [] - - for resource_dir in resources.keys(): - resource_dict = resources.get(resource_dir) - all_resources.extend( - native.glob( - include = [resource_dir + "/**"], - exclude = ["**/.DS_Store"], - ), + resource_dir = resource_dict.get("res", None) + resources = _res_glob([resource_dir]) if resource_dir else [] + + asset_dir = resource_dict.get("assets", None) + assets = _res_glob([asset_dir]) if asset_dir else [] + has_assets = len(assets) != 0 + + return struct( + res = resources + generated_resources, + assets = assets if has_assets else None, + asset_dir = asset_dir if has_assets else None, + manifest = resource_dict.get("manifest", manifest), ) + else: + source_sets = [] # Source sets args in the res_dir:assets:manifest format + all_resources = [] + all_assets = [] + all_manifests = [] - manifest = resource_dict.get("manifest", "") - if manifest != "": - all_manifests.append(manifest) + for resource_set_name in resource_sets.keys(): + resource_dict = resource_sets.get(resource_set_name) - source_sets.append("%s::%s" % (resource_dir, manifest)) + resource_dir = resource_dict.get("res", "") + if resource_dir != "": + all_resources.extend(_res_glob([resource_dir])) - merge_target_name = name + "_res" - merged_resources = _calculate_output_files(merge_target_name, all_resources) - resource_merger( - name = merge_target_name, - source_sets = source_sets, - resources = all_resources, - manifests = all_manifests, - merged_resources = merged_resources, - ) - return merged_resources + generated_resources + asset_dir = resource_dict.get("assets", "") + if asset_dir != "": + all_assets.extend(_res_glob([asset_dir])) + + manifest = resource_dict.get("manifest", "") + if manifest != "": + all_manifests.append(manifest) + + source_sets.append("%s:%s:%s" % (resource_dir, asset_dir, manifest)) + + merge_target_name = "_" + name + "_res" + merged_resources = _calculate_output_files(merge_target_name, all_resources) + merged_assets = _calculate_output_files(merge_target_name, all_assets) + + # Outputs that would be produced by merger + merged_manifest = "%s/_merged/AndroidManifest.xml" % merge_target_name + asset_dir = "%s/out/assets/" % merge_target_name + resource_merger( + name = merge_target_name, + is_binary = is_binary, + namespace = namespace, + source_sets = source_sets, + resources = all_resources + all_assets, + manifests = all_manifests, + merged_manifest = merged_manifest, + merged_resources = merged_resources + merged_assets, + ) + return struct( + res = merged_resources + generated_resources, + assets = merged_assets if len(merged_assets) != 0 else None, + asset_dir = asset_dir if len(merged_assets) != 0 else None, + manifest = merged_manifest, + ) else: - return resource_files + generated_resources + return struct( + res = resource_files + generated_resources, + assets = None, + asset_dir = None, + manifest = manifest, + ) diff --git a/tests/android/binary/BUILD.bazel b/tests/android/binary/BUILD.bazel index e9972a45..c2bafdba 100644 --- a/tests/android/binary/BUILD.bazel +++ b/tests/android/binary/BUILD.bazel @@ -19,6 +19,7 @@ android_binary( "minSdkVersion": "21", "targetSdkVersion": "31", "applicationId": "com.grab.test", + "orientation": "portrait", }, resource_configuration_filters = [ "en", @@ -33,8 +34,16 @@ android_binary( "ko", "ja", ], - resources = { - "src/main/res": { + resource_sets = { + "flavor": { + "res": "src/flavor/res", + "manifest": "src/flavor/AndroidManifest.xml", + "assets": "src/flavor/assets", + }, + "main": { + "res": "src/main/res", + "manifest": "src/main/AndroidManifest.xml", + "assets": "src/main/assets", }, }, deps = [ diff --git a/tests/android/binary/lint_baseline.xml b/tests/android/binary/lint_baseline.xml index 52efc6b5..2553185f 100755 --- a/tests/android/binary/lint_baseline.xml +++ b/tests/android/binary/lint_baseline.xml @@ -5,7 +5,7 @@ id="LocaleFolder" message="The locale folder "`id`" should be called "`in`" instead; see the `java.util.Locale` documentation"> + file="../../../../../../armeabi-v7a-fastbuild-android-ST-5b74a929aefd/bin/tests/android/binary/_android_binary_sample_res/out/res/values-id"/> + + + + @@ -47,31 +58,31 @@ errorLine1="<manifest xmlns:android="http://schemas.android.com/apk/res/android"" errorLine2=" ~~~~~~~~"> + errorLine1="<string name="app_name">Flavor</string>" + errorLine2=" ~~~~~~~~~~~~~~~"> + file="../../../../../../armeabi-v7a-fastbuild-android-ST-5b74a929aefd/bin/tests/android/binary/_android_binary_sample_res/out/res/values/values.xml" + line="5" + column="9"/> + errorLine1="<string name="app_name1">Test</string>" + errorLine2=" ~~~~~~~~~~~~~~~~"> + file="../../../../../../armeabi-v7a-fastbuild-android-ST-5b74a929aefd/bin/tests/android/binary/_android_binary_sample_res/out/res/values-id/values.xml" + line="5" + column="9"/> @@ -102,41 +113,41 @@ errorLine1="<layout xmlns:android="http://schemas.android.com/apk/res/android">" errorLine2="^"> + message="The resource `R.string.un_used_resource` appears to be unused" + errorLine1=" <string name="un_used_resource">Unused resource</string>" + errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~"> + message="The resource `R.string.app_name1` appears to be unused" + errorLine1="<string name="app_name1">Test</string>" + errorLine2=" ~~~~~~~~~~~~~~~~"> + file="../../../../../../armeabi-v7a-fastbuild-android-ST-5b74a929aefd/bin/tests/android/binary/_android_binary_sample_res/out/res/values-id/values.xml" + line="5" + column="9"/> diff --git a/tests/android/binary/src/flavor/AndroidManifest.xml b/tests/android/binary/src/flavor/AndroidManifest.xml new file mode 100644 index 00000000..69fb2cc7 --- /dev/null +++ b/tests/android/binary/src/flavor/AndroidManifest.xml @@ -0,0 +1,7 @@ + + + + + + \ No newline at end of file diff --git a/tests/android/binary/src/flavor/assets/sample.json b/tests/android/binary/src/flavor/assets/sample.json new file mode 100644 index 00000000..fe0b4ff4 --- /dev/null +++ b/tests/android/binary/src/flavor/assets/sample.json @@ -0,0 +1,3 @@ +{ + "name": "flavor" +} \ No newline at end of file diff --git a/tests/android/binary/src/flavor/res/values/strings.xml b/tests/android/binary/src/flavor/res/values/strings.xml new file mode 100644 index 00000000..86906d34 --- /dev/null +++ b/tests/android/binary/src/flavor/res/values/strings.xml @@ -0,0 +1,3 @@ + + Flavor + \ No newline at end of file diff --git a/tests/android/binary/src/main/AndroidManifest.xml b/tests/android/binary/src/main/AndroidManifest.xml index 0e1a70fd..09ba1c23 100644 --- a/tests/android/binary/src/main/AndroidManifest.xml +++ b/tests/android/binary/src/main/AndroidManifest.xml @@ -4,7 +4,8 @@ + android:exported="true" + android:screenOrientation="${orientation}" /> \ No newline at end of file diff --git a/tests/android/binary/src/main/assets/sample.json b/tests/android/binary/src/main/assets/sample.json new file mode 100644 index 00000000..92eabcba --- /dev/null +++ b/tests/android/binary/src/main/assets/sample.json @@ -0,0 +1,3 @@ +{ + "name": "main" +} diff --git a/tests/android/binary/src/main/res-extra/values/strings.xml b/tests/android/binary/src/main/res-extra/values/strings.xml new file mode 100644 index 00000000..3d8a6c2d --- /dev/null +++ b/tests/android/binary/src/main/res-extra/values/strings.xml @@ -0,0 +1,3 @@ + + Extra + \ No newline at end of file diff --git a/tests/android/library/BUILD.bazel b/tests/android/library/BUILD.bazel index 01aa04cd..4d6f517e 100644 --- a/tests/android/library/BUILD.bazel +++ b/tests/android/library/BUILD.bazel @@ -16,8 +16,10 @@ android_library( "baseline": "lint_baseline.xml", }, manifest = "src/main/AndroidManifest.xml", - resources = { - "src/main/res": { + resource_sets = { + "main": { + "res": "src/main/res", + "manifest": "src/main/AndroidManifest.xml", }, }, visibility = [ diff --git a/tools/aapt_lite/src/main/java/com/google/devtools/build/android/ResourceMerger.java b/tools/aapt_lite/src/main/java/com/google/devtools/build/android/ResourceMerger.java index 1366cead..086f3c8a 100644 --- a/tools/aapt_lite/src/main/java/com/google/devtools/build/android/ResourceMerger.java +++ b/tools/aapt_lite/src/main/java/com/google/devtools/build/android/ResourceMerger.java @@ -1,5 +1,13 @@ package com.google.devtools.build.android; +import static com.android.manifmerger.ManifestMerger2.MergeType.APPLICATION; +import static com.android.manifmerger.ManifestMerger2.MergeType.LIBRARY; +import static com.android.manifmerger.MergingReport.MergedManifestKind.MERGED; + +import com.android.manifmerger.ManifestMerger2; +import com.android.manifmerger.ManifestMerger2.Invoker.Feature; +import com.android.manifmerger.MergingReport; +import com.android.utils.StdLogger; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -12,9 +20,12 @@ import java.nio.file.Paths; import java.util.Collections; import java.util.List; +import java.util.Objects; +import java.util.Optional; import java.util.stream.Collectors; public class ResourceMerger { + private static final StdLogger STD_LOGGER = new StdLogger(StdLogger.Level.WARNING); public static ParsedAndroidData emptyAndroidData() { return ParsedAndroidData.of( @@ -24,7 +35,15 @@ public static ParsedAndroidData emptyAndroidData() { ImmutableMap.of()); } - public static void merge(final List sourceSets, final File outputDir) throws IOException { + public static void merge(final boolean isBinary, + final List sourceSets, + final File outputDir, + final File mergeManifest) throws IOException { + mergeManifests(isBinary, sourceSets, mergeManifest); + mergeResources(sourceSets, outputDir, mergeManifest); + } + + private static void mergeResources(final List sourceSets, final File outputDir, final File manifest) throws IOException { final Path target = Paths.get(outputDir.getAbsolutePath()); Collections.reverse(sourceSets); final ImmutableList deps = ImmutableList.copyOf(sourceSets @@ -32,16 +51,16 @@ public static void merge(final List sourceSets, final File outputDir) .map(sourceSet -> new DependencyAndroidData( /*resourceDirs*/ ImmutableList.copyOf(sourceSet.getResourceDirs()), /*assetDirs*/ ImmutableList.copyOf(sourceSet.getAssetDirs()), - /*manifest*/ sourceSet.getManifest().toPath(), + /*manifest*/ sourceSet.getManifest() != null ? sourceSet.getManifest().toPath() : manifest.toPath(), /*rTxt*/ null, /*symbols*/ null, /*compiledSymbols*/ null )).collect(Collectors.toList())); final ParsedAndroidData androidData = ParsedAndroidData.from(deps); - final AndroidDataMerger merger = AndroidDataMerger.createWithDefaults(); + final AndroidDataMerger androidDataMerger = AndroidDataMerger.createWithDefaults(); - final UnwrittenMergedAndroidData unwrittenMergedAndroidData = merger.doMerge( + final UnwrittenMergedAndroidData unwrittenMergedAndroidData = androidDataMerger.doMerge( /*transitive*/ emptyAndroidData(), /*direct*/ emptyAndroidData(), /*parsedPrimary*/ androidData, @@ -57,4 +76,53 @@ public static void merge(final List sourceSets, final File outputDir) /*executorService*/ MoreExecutors.newDirectExecutorService()) ); } + + /** + * Do manifest merging just like Gradle would do for variant source sets i.e retain the placeholders in the Manifests and just do content + * merging. + * + * @param isBinary Whether the merging is for binary + * @param sourceSets The list of source sets + * @param mergedManifest The result merged manifest file + * @throws IOException Rethrow IO exceptions from manifest merger. + */ + private static void mergeManifests(boolean isBinary, + final List sourceSets, + final File mergedManifest) throws IOException { + // https://cs.android.com/android-studio/platform/tools/base/+/mirror-goog-studio-main:build-system/manifest-merger/src/test/java/com/android/manifmerger/ManifestMerger2SmallTest.java;l=792;drc=549798d9f7af50d4202041071bcb1f604e7229e9 + final AndroidManifestProcessor manifestProcessor = AndroidManifestProcessor.with(STD_LOGGER); + + final List manifests = sourceSets + .parallelStream() + .map(SourceSet::getManifest) + .filter(Objects::nonNull) + .collect(Collectors.toList()); + + Collections.reverse(manifests); + + if (manifests.size() > 1) { + final File mainManifest = manifests.remove(0); + try { + final ManifestMerger2.MergeType mergeType = isBinary ? APPLICATION : LIBRARY; + final MergingReport mergingReport = ManifestMerger2 + .newMerger(mainManifest, STD_LOGGER, mergeType) + .withFeatures(Feature.NO_PLACEHOLDER_REPLACEMENT, Feature.EXTRACT_FQCNS) + .addFlavorAndBuildTypeManifests(manifests.toArray(File[]::new)) + .merge(); + if (mergingReport.getResult().isError()) { + throw new IllegalStateException(mergingReport.getReportString()); + } + manifestProcessor.writeMergedManifest(MERGED, mergingReport, mergedManifest.toPath()); + } catch (ManifestMerger2.MergeFailureException e) { + throw new RuntimeException(e); + } + } else { + final Optional mainManifest = manifests.stream().findFirst(); + if (mainManifest.isPresent()) { + Files.copy(mainManifest.get().toPath(), mergedManifest.toPath()); + } else { + throw new IllegalArgumentException("Missing manifest declaration, check if at least one manifest is declared in any source set"); + } + } + } } diff --git a/tools/aapt_lite/src/main/java/com/google/devtools/build/android/ResourceMergerCommand.kt b/tools/aapt_lite/src/main/java/com/google/devtools/build/android/ResourceMergerCommand.kt index 4010f4ad..6aa5728a 100644 --- a/tools/aapt_lite/src/main/java/com/google/devtools/build/android/ResourceMergerCommand.kt +++ b/tools/aapt_lite/src/main/java/com/google/devtools/build/android/ResourceMergerCommand.kt @@ -1,7 +1,9 @@ package com.google.devtools.build.android import com.github.ajalt.clikt.core.CliktCommand +import com.github.ajalt.clikt.parameters.options.convert import com.github.ajalt.clikt.parameters.options.default +import com.github.ajalt.clikt.parameters.options.flag import com.github.ajalt.clikt.parameters.options.option import com.github.ajalt.clikt.parameters.options.required import com.github.ajalt.clikt.parameters.options.split @@ -11,10 +13,29 @@ import java.io.File class ResourceMergerCommand : CliktCommand() { + @Suppress("unused") + private val label by option( + "-l", + "--label", + help = "The label name that invokes this merger." + ) + + private val binary: Boolean by option( + "-b", + "--is-binary", + help = "Whether the target is for binary" + ).flag(default = false) + private val target by option( "-t", "--target", - help = "The target name" + help = "The target name, this will be used to decode source set paths" + ).required() + + private val packageName by option( + "-pn", + "--package-name", + help = "The target name, this will be used to decode source set paths" ).required() private val sourceSets by option( @@ -23,6 +44,12 @@ class ResourceMergerCommand : CliktCommand() { help = "List of sources sets in the format $SOURCE_SET_FORMAT separated by `,`" ).split(",").default(emptyList()) + private val mergedManifestOutput by option( + "-m", + "--manifest", + help = "The merged manifest output file" + ).convert { File(it) } + private val outputs by option( "-o", "--output", @@ -36,7 +63,12 @@ class ResourceMergerCommand : CliktCommand() { deleteRecursively() parentFile?.mkdirs() } - ResourceMerger.merge(/* sourceSets = */ sourceSets, /* outputDir = */ outputDir) + ResourceMerger.merge( + /* isBinary = */ binary, + /* sourceSets = */ sourceSets, + /* outputDir = */ outputDir, + /* mergeManifest = */ mergedManifestOutput + ) OutputFixer.process(outputDir = outputDir, declaredOutputs = outputs) } } \ No newline at end of file diff --git a/tools/aapt_lite/src/main/java/com/google/devtools/build/android/SourceSet.kt b/tools/aapt_lite/src/main/java/com/google/devtools/build/android/SourceSet.kt index 77e833d6..89aaaf04 100644 --- a/tools/aapt_lite/src/main/java/com/google/devtools/build/android/SourceSet.kt +++ b/tools/aapt_lite/src/main/java/com/google/devtools/build/android/SourceSet.kt @@ -6,23 +6,25 @@ import java.nio.file.Path data class SourceSet( val resourceDirs: List, val assetDirs: List, - val manifest: File + val manifest: File? ) { companion object { const val SOURCE_SET_FORMAT = "resources:assets:manifest" fun from(target: String, inputArg: String): SourceSet { val chunks = inputArg.split(":") require(chunks.size == 3) { "Invalid format, should be $SOURCE_SET_FORMAT" } + val (res, assets, manifest) = chunks - fun String.chunkToPaths() = when { + fun String.toPaths() = when { trim().isEmpty() -> emptyList() else -> listOf(File(target, this).toPath()) } - return SourceSet( - resourceDirs = chunks[0].chunkToPaths(), - assetDirs = chunks[1].chunkToPaths(), - manifest = File(target, chunks[2]) + resourceDirs = res.toPaths(), + assetDirs = assets.toPaths(), + manifest = if (manifest.trim().isNotEmpty() && File(target, manifest).exists()) { + File(target, manifest) + } else null ) } } diff --git a/tools/aapt_lite/src/main/java/com/grab/aapt/AaptLiteCommand.kt b/tools/aapt_lite/src/main/java/com/grab/aapt/AaptLiteCommand.kt index 4a54b373..e9dda470 100644 --- a/tools/aapt_lite/src/main/java/com/grab/aapt/AaptLiteCommand.kt +++ b/tools/aapt_lite/src/main/java/com/grab/aapt/AaptLiteCommand.kt @@ -25,6 +25,7 @@ import com.github.ajalt.clikt.parameters.options.required import com.github.ajalt.clikt.parameters.options.split import com.grab.cli.WorkingDirectory import java.io.File +import java.nio.file.Files class AaptLiteCommand : CliktCommand() { @@ -70,16 +71,23 @@ class AaptLiteCommand : CliktCommand() { help = "The stubs srcjar location where the generated stubs will be written to" ).convert { File(it) }.required() + private val targetName by option( + "-n", + "--name", + ).required() + override fun run() { - val resourcesFiles = resources.map { path -> File(path) } + val resourcesFiles = resources.map(::File) val layoutFiles = resourcesFiles.filter { it.path.contains("/layout") } - val classInfoZip = classInfos.map { File(it) } - val depRTxts = rTxts.map { File(it) } + val classInfoZip = classInfos.map(::File) + val depRTxts = rTxts.map(::File) + + val prefix = targetName.replace("/", "_").replace(":", "_") - WorkingDirectory().use { - val command = DaggerAaptLiteComponent.factory().create( - baseDir = it.dir.toFile(), + WorkingDirectory(dir = Files.createTempDirectory(prefix)).use { workingDirectory -> + val aaptLite: AaptLiteComponent = DaggerAaptLiteComponent.factory().create( + baseDir = workingDirectory.dir.toFile(), packageName = packageName, resourceFiles = resourcesFiles, layoutFiles = layoutFiles, @@ -87,20 +95,14 @@ class AaptLiteCommand : CliktCommand() { rTxts = depRTxts, nonTransitiveRClass = nonTransitiveRClass, ) - val layoutBindings = command.layoutBindingsParser().parse(packageName, layoutFiles) - command.resToRClassGenerator().generate(packageName, resourcesFiles, depRTxts) + val layoutBindings = aaptLite.layoutBindingsParser.parse(packageName, layoutFiles) + aaptLite.resToRClassGenerator.generate(packageName, resourcesFiles, depRTxts) - val rClasses = command.brClassGenerator().generate(packageName, layoutBindings) - command.srcJarPackager.packageSrcJar(inputDir = rClasses, outputFile = rClassSrcJar) + val rClassesDir = aaptLite.brClassGenerator.generate(packageName, layoutBindings) + aaptLite.srcJarPackager.packageSrcJar(inputDir = rClassesDir, outputFile = rClassSrcJar) - val dataBindingClasses = command.bindingClassGenerator().generate( - packageName, - layoutBindings - ) - command.srcJarPackager.packageSrcJar( - inputDir = dataBindingClasses, - outputFile = stubClassJar - ) + val dataBindingClassesDir = aaptLite.bindingClassGenerator.generate(packageName, layoutBindings) + aaptLite.srcJarPackager.packageSrcJar(inputDir = dataBindingClassesDir, outputFile = stubClassJar) } } } \ No newline at end of file diff --git a/tools/aapt_lite/src/main/java/com/grab/aapt/AaptLiteComponent.kt b/tools/aapt_lite/src/main/java/com/grab/aapt/AaptLiteComponent.kt index b31e36e0..3c95aace 100644 --- a/tools/aapt_lite/src/main/java/com/grab/aapt/AaptLiteComponent.kt +++ b/tools/aapt_lite/src/main/java/com/grab/aapt/AaptLiteComponent.kt @@ -52,10 +52,10 @@ import javax.inject.Named ] ) interface AaptLiteComponent { - fun layoutBindingsParser(): LayoutBindingsParser - fun resToRClassGenerator(): ResToRClassGenerator - fun bindingClassGenerator(): BindingClassGenerator - fun brClassGenerator(): BrClassGenerator + val layoutBindingsParser: LayoutBindingsParser + val resToRClassGenerator: ResToRClassGenerator + val bindingClassGenerator: BindingClassGenerator + val brClassGenerator: BrClassGenerator val srcJarPackager: SrcJarPackager @Component.Factory diff --git a/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/binding/generator/BindingClassGenerator.kt b/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/binding/generator/BindingClassGenerator.kt index 282d6fcb..c463cedb 100644 --- a/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/binding/generator/BindingClassGenerator.kt +++ b/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/binding/generator/BindingClassGenerator.kt @@ -130,7 +130,7 @@ constructor( val bindingClassName = ClassName.get(genPackageName, bindingClass) val bindings = layoutBinding.bindings.filterNot { binding -> - invalidBindingTypes.contains(binding.typeName.toString()) + binding.typeName.toString() in invalidBindingTypes } val fields = buildFields(bindings, layoutBinding.bindables) diff --git a/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/binding/parser/LayoutBindingsParser.kt b/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/binding/parser/LayoutBindingsParser.kt index 1cd1fd49..7f96fc44 100644 --- a/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/binding/parser/LayoutBindingsParser.kt +++ b/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/binding/parser/LayoutBindingsParser.kt @@ -78,8 +78,6 @@ constructor( internal const val INCLUDE = "include" } - private val xpp = XmlPullParserFactory.newInstance().newPullParser() - private val File.bindingName get() = name.split(".xml").first().toLayoutBindingName() @@ -89,7 +87,9 @@ constructor( ): List { return layoutFiles.map { layoutFile -> layoutFile.inputStream().buffered().use { stream -> - xpp.setInput(stream, null) + val xpp = XmlPullParserFactory.newInstance().newPullParser().apply { + setInput(stream, null) + } val bindingClassName = layoutFile.bindingName val bindings = mutableSetOf() val bindables = mutableSetOf() @@ -102,12 +102,12 @@ constructor( if (event == START_TAG) { when (val nodeName = xpp.name) { IMPORT -> { - val attributes = xpp.attributesNameValue() - .withDefault { error("Could not parse: $it") } + val attributes = xpp.attributesNameValue().withDefault { error("Could not parse: $it") } val typeFqcn = attributes.getValue(TYPE) val typeName = attributes[ALIAS] ?: typeFqcn.split(".").last() importedTypes[typeName] = ClassName.bestGuess(typeFqcn) } + VARIABLE -> { val attributes = xpp.attributesNameValue() .withDefault { error("Could not parse: $it") } @@ -121,6 +121,7 @@ constructor( ) ) } + else -> { val attributes = xpp.attributesNameValue() .filterKeys { it == ANDROID_ID || it == LAYOUT } @@ -175,8 +176,10 @@ constructor( val klass = this.substring(0, genericStart) .trim() .toTypeName(importedTypes) - return ParameterizedTypeName.get(klass as ClassName, - *typeParamsQualified.toTypedArray()) + return ParameterizedTypeName.get( + klass as ClassName, + *typeParamsQualified.toTypedArray() + ) } } @@ -212,6 +215,7 @@ constructor( layoutMissing = layoutMissing ) } + else -> BindingType.View } } @@ -235,12 +239,14 @@ constructor( layoutMissing = missing parsedType } + else -> when (nodeName) { "ViewStub" -> ClassName.bestGuess("androidx.databinding.ViewStubProxy") // https://android.googlesource.com/platform/frameworks/data-binding/+/refs/tags/studio-4.1.1/compilerCommon/src/main/java/android/databinding/tool/store/ResourceBundle.java#70 "View", "ViewGroup", "TextureView", "SurfaceView" -> { ClassName.get("android.view", nodeName) } + "WebView" -> ClassName.get("android.webkit", nodeName) else -> ClassName.get("android.widget", nodeName) } @@ -257,8 +263,7 @@ constructor( * direct dependencies */ fun parseIncludedLayoutType(layoutName: String): TypeName? { - return localLayoutTypeStore[layoutName] - ?: depLayoutTypeStore[layoutName] + return localLayoutTypeStore[layoutName] ?: depLayoutTypeStore[layoutName] } /** @@ -322,4 +327,5 @@ private val PRIMITIVE_TYPE_NAME_MAP = mapOf( TypeName.LONG.toString() to TypeName.LONG, TypeName.CHAR.toString() to TypeName.CHAR, TypeName.FLOAT.toString() to TypeName.FLOAT, - TypeName.DOUBLE.toString() to TypeName.DOUBLE) + TypeName.DOUBLE.toString() to TypeName.DOUBLE +) diff --git a/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/binding/store/LayoutTypeStore.kt b/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/binding/store/LayoutTypeStore.kt index 12fa6d19..cb88b7a9 100644 --- a/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/binding/store/LayoutTypeStore.kt +++ b/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/binding/store/LayoutTypeStore.kt @@ -16,6 +16,7 @@ package com.grab.aapt.databinding.binding.store +import com.grab.aapt.databinding.common.BASE_DIR import com.grab.aapt.databinding.common.CLASS_INFOS import com.grab.aapt.databinding.common.LAYOUT_FILES import com.grab.aapt.databinding.common.PACKAGE_NAME @@ -26,7 +27,7 @@ import com.squareup.javapoet.TypeName import dagger.Binds import dagger.Module import java.io.File -import java.nio.file.Files +import java.util.zip.ZipEntry import java.util.zip.ZipFile import javax.inject.Inject import javax.inject.Named @@ -107,6 +108,8 @@ constructor( class DependenciesLayoutTypeStore @Inject constructor( + @Named(BASE_DIR) + private val baseDir: File, @Named(CLASS_INFOS) private val classInfoZips: List, private val bindingClassJsonParser: BindingClassJsonParser, ) : LayoutTypeStore { @@ -114,7 +117,7 @@ constructor( /** * The directory where the classInfo.zips will be extracted to */ - var extractionDir: File = Files.createTempDirectory("temp").toFile() + val extractionDir: File = baseDir.toPath().resolve("class_infos").toFile() /** * Stores classInfo.zip extraction result. This is used to avoid doing repeat extractions when @@ -122,8 +125,7 @@ constructor( * Key: name of the classInfo zip file * Values: Binding class json files */ - private val classInfoZipContentCache = mutableMapOf>() - .withDefault { emptyList() } + private val classInfoZipContentCache = mutableMapOf>().withDefault { emptyList() } /** * Extract the classInfo.zip to [extractionDir] and return list of binding class json files in @@ -139,33 +141,35 @@ constructor( return classInfoZipContentCache.getValue(cacheKey) } else { // Perform an extraction and cache the result + val dirName = classInfoZip.parentFile?.name ?: error("$classInfoZip does not exist") + val targetDir = File(extractionDir, dirName).apply { mkdirs() } val jsonFiles = mutableListOf() ZipFile(classInfoZip).use { zip -> - zip.entries().asSequence().forEach { entry -> - zip.getInputStream(entry).buffered().use { input -> - val dir = File( - extractionDir, - classInfoZip.parentFile?.name ?: error("$classInfoZip does not exist") - ) - val extractedFile = File(dir, entry.name).apply { parentFile?.mkdirs() } - when { - entry.isDirectory -> extractedFile.mkdirs() - else -> { - extractedFile - .also { jsonFiles.add(it) } - .outputStream() - .buffered() - .use { output -> input.copyTo(output) } - } - } + zip.entries().asSequence() + .filterNot { it.isDirectory } + .forEach { entry -> + val extractedFile = File(targetDir, entry.name) + extractFile(zip, entry, extractedFile) + jsonFiles.add(extractedFile) } - } } classInfoZipContentCache[cacheKey] = jsonFiles return jsonFiles } } + /** + * Extracts a single file from the ZIP archive. + */ + private fun extractFile(zip: ZipFile, entry: ZipEntry, outputFile: File) { + outputFile.parentFile?.mkdirs() + zip.getInputStream(entry).buffered().use { input -> + outputFile.outputStream().buffered().use { output -> + input.copyTo(output) + } + } + } + /** * Cache already served requests for layout typename */ diff --git a/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/rclass/parser/ResToRParser.kt b/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/rclass/parser/ResToRParser.kt index 83c75e70..e1baa644 100644 --- a/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/rclass/parser/ResToRParser.kt +++ b/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/rclass/parser/ResToRParser.kt @@ -25,7 +25,7 @@ import com.grab.aapt.databinding.util.events import org.xmlpull.v1.XmlPullParser import org.xmlpull.v1.XmlPullParserFactory import java.io.File -import java.util.* +import java.util.Locale import javax.inject.Inject import javax.inject.Named @@ -45,7 +45,9 @@ private const val ANDROID_ID = "android:id" private const val TAG_TYPE = "type" @AaptScope -class DefaultResToRParser @Inject constructor( +class DefaultResToRParser +@Inject +constructor( private val parsers: Map, @param:Named(NON_TRANSITIVE_R) private val nonTransitiveRClass: Boolean ) : ResToRParser { @@ -57,7 +59,6 @@ class DefaultResToRParser @Inject constructor( } private val resources: MutableMap> = mutableMapOf() - private val xpp = XmlPullParserFactory.newInstance().newPullParser() override fun parse( resources: List, @@ -109,19 +110,22 @@ class DefaultResToRParser @Inject constructor( private fun processValues(file: File) { file.inputStream().buffered().use { stream -> - xpp.setInput(stream, null) + val xpp = XmlPullParserFactory.newInstance().newPullParser().apply { + setInput(stream, null) + } xpp.events() .asSequence() .filter { xpp.attributesNameValue().isNotEmpty() } .filter { xpp.attributesNameValue().contains(NAME) } .forEach { _ -> - val name = xpp.attributeName() - val type = enumTypeValue(xpp.name).let { - getTypedItem(it, xpp.attributesNameValue()) ?: it - } - if (type != XmlTypeValues.ITEM) { - // If item is not typed, we do not include it - parseIntoRField(type, name, xpp) + xpp.attributeName()?.let { name -> + val type = enumTypeValue(xpp.name).let { + getTypedItem(it, xpp.attributesNameValue()) ?: it + } + if (type != XmlTypeValues.ITEM) { + // If item is not typed, we do not include it + parseIntoRField(type, name, xpp) + } } } } @@ -144,10 +148,12 @@ class DefaultResToRParser @Inject constructor( private fun collectIDs(file: File) { if (file.extension != "xml") return //process only xml files to get IDs file.inputStream().buffered().use { stream -> - xpp.setInput(stream, null) + val xpp = XmlPullParserFactory.newInstance().newPullParser().apply { + setInput(stream, null) + } xpp.events() .asSequence() - .filter { xpp.attributesNameValue().contains(ANDROID_ID) } + .filter { ANDROID_ID in xpp.attributesNameValue() } .map { xpp.attributesNameValue()[ANDROID_ID] } .filterNotNull() .filter { !it.contains("@android:") } @@ -164,7 +170,7 @@ class DefaultResToRParser @Inject constructor( isArray: Boolean = false ) { resources.getOrPut(type) { mutableSetOf() }.apply { - this.add( + add( RFieldEntry( type, name, @@ -197,12 +203,9 @@ class DefaultResToRParser @Inject constructor( name, type ) + XmlTypeValues.ENUM -> parse(ParserType.ID_PARSER, name, type) - XmlTypeValues.DECLARE_STYLEABLE -> parseParentEntry( - ParserType.STYLEABLE_PARSER, - name, - xpp - ) + XmlTypeValues.DECLARE_STYLEABLE -> parseStyleableEntry(name, xpp) else -> parse(ParserType.DEFAULT_PARSER, name, type) }.apply { addResources(this) @@ -214,13 +217,13 @@ class DefaultResToRParser @Inject constructor( ?: throw NullPointerException("Missing implementation. Parser must not be null.") } - private fun parseParentEntry(type: ParserType, name: String, xpp: XmlPullParser): ParserResult { + private fun parseStyleableEntry(name: String, xpp: XmlPullParser): ParserResult { val children = mutableListOf() xpp.next() // Proceed to the next (child) node once we save parent + xpp.attributeName()?.replace(":", "_")?.let(children::add) while (xpp.name != XmlTypeValues.DECLARE_STYLEABLE.entry) { - // Get next item xpp.next() @@ -229,15 +232,15 @@ class DefaultResToRParser @Inject constructor( continue } - val childName = xpp.attributeName().replace(":", "_") - // Add every child as a dependent for Parent node - children.add(childName) + xpp.attributeName()?.replace(":", "_")?.let { childName -> + // Add every child as a dependent for Parent node + children.add(childName) - // Add every child as a independent attribute as well - parseIntoRField(XmlTypeValues.ATTR, childName, xpp) + // Add every child as a independent attribute as well + parseIntoRField(XmlTypeValues.ATTR, childName, xpp) + } } - - return parsers[type] + return parsers[ParserType.STYLEABLE_PARSER] ?.parse(ParentXmlEntryImpl(name, XmlTypeValues.DECLARE_STYLEABLE, children)) ?: throw NullPointerException("Missing implementation. Parser must not be null.") } diff --git a/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/rclass/parser/xml/DeclareStyleableParser.kt b/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/rclass/parser/xml/DeclareStyleableParser.kt index 4b3e6994..86fb195e 100644 --- a/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/rclass/parser/xml/DeclareStyleableParser.kt +++ b/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/rclass/parser/xml/DeclareStyleableParser.kt @@ -26,7 +26,7 @@ import javax.inject.Inject /** * DeclareStyleableParser is supposed to parse attribute - * and nexted children under nexted R class `styleable` + * and nested children under nested R class `styleable` * * Parent attribute must be an array with size of nested children * e.g. if children size is 2 => public static final int[] StyleableParentName = { 0x00000000,0x00000000 }; diff --git a/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/util/XmlPullParser.kt b/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/util/XmlPullParser.kt index d5cfb54e..7aa07e97 100644 --- a/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/util/XmlPullParser.kt +++ b/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/util/XmlPullParser.kt @@ -47,5 +47,4 @@ fun XmlPullParser.attributesNameValue() = attributes { index -> /** * Return value by using key "name" or the first value if it is null */ -fun XmlPullParser.attributeName() = - attributesNameValue()[NAME] ?: attributesNameValue().values.first() \ No newline at end of file +fun XmlPullParser.attributeName() = attributesNameValue()[NAME] ?: attributesNameValue().values.firstOrNull() \ No newline at end of file diff --git a/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/util/jars/SourceJarCreator.kt b/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/util/jars/SourceJarCreator.kt index f07bc00b..ad718b00 100644 --- a/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/util/jars/SourceJarCreator.kt +++ b/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/util/jars/SourceJarCreator.kt @@ -19,7 +19,7 @@ package com.grab.aapt.databinding.util.jars import java.nio.charset.Charset import java.nio.file.Files import java.nio.file.Path -import java.util.* +import java.util.TreeMap import java.util.jar.JarFile import java.util.jar.JarOutputStream import java.util.regex.Pattern @@ -32,6 +32,7 @@ class SourceJarCreator( path: Path, verbose: Boolean = false ) : JarHelper(path, normalize = true, verbose = verbose) { + companion object { private const val BL = """\p{Blank}*""" private const val COM_BL = """$BL(?:/\*[^\n]*\*/$BL)*""" @@ -132,8 +133,8 @@ class SourceJarCreator( for (entry in jar.entries()) { if (!entry.isDirectory) { if (isJavaSourceLike(entry.name)) { - jar.getInputStream(entry).readBytes().also { - addEntry(entry.name, path, it) + jar.getInputStream(entry).buffered().use { + addEntry(entry.name, path, it.readBytes()) } } } @@ -170,7 +171,7 @@ class SourceJarCreator( addEntry(jarFilename, path, bytes) } } - Files.newOutputStream(jarPath).use { + Files.newOutputStream(jarPath).buffered().use { JarOutputStream(it).use { out -> for ((key, value) in entries) { try { diff --git a/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/util/jars/SrcJarPackager.kt b/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/util/jars/SrcJarPackager.kt index aca42f4a..e37fa0b3 100644 --- a/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/util/jars/SrcJarPackager.kt +++ b/tools/aapt_lite/src/main/java/com/grab/aapt/databinding/util/jars/SrcJarPackager.kt @@ -36,7 +36,7 @@ class DefaultSrcJarPackager @Inject constructor() : SrcJarPackager { outputFile.parentFile?.mkdirs() SourceJarCreator(path = outputFile.toPath(), verbose = verbose).apply { addSources( - File(inputDir.toURI()) + inputDir .walkTopDown() .filter { it.isFile } .map { it.toPath() } diff --git a/tools/aapt_lite/src/test/java/com/google/devtools/build/android/SourceSetTest.kt b/tools/aapt_lite/src/test/java/com/google/devtools/build/android/SourceSetTest.kt index 99e5d935..3538ddf3 100644 --- a/tools/aapt_lite/src/test/java/com/google/devtools/build/android/SourceSetTest.kt +++ b/tools/aapt_lite/src/test/java/com/google/devtools/build/android/SourceSetTest.kt @@ -18,7 +18,7 @@ class SourceSetTest { SourceSet.from("target", "res::manifest").let { sourceSet -> assertTrue("Res is parsed", sourceSet.resourceDirs.all { it.name == "res" }) assertTrue("Empty paths are skipped", sourceSet.assetDirs.isEmpty()) - assertTrue("Manifest is parsed", sourceSet.manifest.name == "manifest") + assertTrue("Manifest is only parsed when it exists on disk", sourceSet.manifest?.name == null) } } } \ No newline at end of file diff --git a/tools/aapt_lite/src/test/java/com/grab/aapt/databinding/binding/store/DependenciesLayoutTypeStoreTest.kt b/tools/aapt_lite/src/test/java/com/grab/aapt/databinding/binding/store/DependenciesLayoutTypeStoreTest.kt index bb24dcab..19dfa042 100644 --- a/tools/aapt_lite/src/test/java/com/grab/aapt/databinding/binding/store/DependenciesLayoutTypeStoreTest.kt +++ b/tools/aapt_lite/src/test/java/com/grab/aapt/databinding/binding/store/DependenciesLayoutTypeStoreTest.kt @@ -23,13 +23,13 @@ class DependenciesLayoutTypeStoreTest : BaseTest() { fun setup() { val classInfoDir = temporaryFolder.newFolder("classInfoDir") dependenciesLayoutTypeStore = DependenciesLayoutTypeStore( + baseDir = temporaryFolder.newFolder("base"), classInfoZips = listOf( createClassInfoZip(classInfoDir, "clock", DEFAULT_JSON_CONTENT), createClassInfoZip(classInfoDir, "clock2", DEFAULT_JSON_CONTENT) ), bindingClassJsonParser = CachingBindingClassJsonParser() ) - dependenciesLayoutTypeStore.extractionDir = temporaryFolder.newFolder("extracted") } /** diff --git a/tools/aapt_lite/src/test/java/com/grab/aapt/databinding/rclass/parser/ResToRStyleableValueParserTest.kt b/tools/aapt_lite/src/test/java/com/grab/aapt/databinding/rclass/parser/ResToRStyleableValueParserTest.kt index 248465fc..81124724 100644 --- a/tools/aapt_lite/src/test/java/com/grab/aapt/databinding/rclass/parser/ResToRStyleableValueParserTest.kt +++ b/tools/aapt_lite/src/test/java/com/grab/aapt/databinding/rclass/parser/ResToRStyleableValueParserTest.kt @@ -49,6 +49,10 @@ class ResToRStyleableValueParserTest : BaseTest() { + + + + """.trimIndent(), path = "/src/res/values/" @@ -59,18 +63,24 @@ class ResToRStyleableValueParserTest : BaseTest() { listTemp, emptyList() ) as MutableMap> - val parentValue = "{ 0,0,0 }" + val parentValue = "{ 0,0,0,0,0,0 }" val exptectedStyleable = setOf( - RFieldEntry(Type.STYLEABLE, "StyleableView", parentValue, isArray = true), RFieldEntry(Type.STYLEABLE, "StyleableView_colorAttr", value), RFieldEntry(Type.STYLEABLE, "StyleableView_sizeAttr", value), RFieldEntry(Type.STYLEABLE, "StyleableView_android_drawableTint", value), + RFieldEntry(Type.STYLEABLE, "StyleableView_useCase", value), + RFieldEntry(Type.STYLEABLE, "StyleableView_rating", value), + RFieldEntry(Type.STYLEABLE, "StyleableView_postTransit", value), + RFieldEntry(Type.STYLEABLE, "StyleableView", parentValue, isArray = true), ) val exptectedAttrs = setOf( RFieldEntry(Type.ATTR, "colorAttr", value), RFieldEntry(Type.ATTR, "sizeAttr", value), RFieldEntry(Type.ATTR, "android_drawableTint", value), + RFieldEntry(Type.ATTR, "useCase", value), + RFieldEntry(Type.ATTR, "rating", value), + RFieldEntry(Type.ATTR, "postTransit", value), ) assertEquals( diff --git a/tools/databinding/databinding.bzl b/tools/databinding/databinding.bzl index bcf4f057..d4da00d8 100644 --- a/tools/databinding/databinding.bzl +++ b/tools/databinding/databinding.bzl @@ -1,7 +1,7 @@ -load("@io_bazel_rules_kotlin//kotlin:kotlin.bzl", "kt_android_library") +load("@grab_bazel_common//rules/android/lint:defs.bzl", "LINT_ENABLED") load("@io_bazel_rules_kotlin//kotlin:jvm.bzl", "kt_jvm_library") +load("@io_bazel_rules_kotlin//kotlin:kotlin.bzl", "kt_android_library") load(":databinding_stubs.bzl", "databinding_stubs") -load("@grab_bazel_common//rules/android/lint:defs.bzl", "LINT_ENABLED") # TODO(arun) Replace with configurable maven targets _DATABINDING_DEPS = [ @@ -36,36 +36,38 @@ def kt_db_android_library( plugins = [], visibility = None, tags = []): - """Configures rules for compiling android module that uses Databinding and Kotlin. - - The macro ensures that Kotlin code referenced in any XMLs are compiled first using kt_jvm_library - and then uses android_library's enable_data_binding to generate required Databinding classes. + """Generates Android library with Kotlin sources and Databinding support. - This helps in breaking circular dependency when we have android_library (databinding enabled) -> kt_jvm_library. - In that case, Databinding classes can't be generated until resources are processed and that - happens only in android_library target. So compiling Koltin classes becomes dependent on - android_library and android_library depends on kt_jvm_library since it needs class files to - process class references in XML. This macro alleviates this problem by processing resources - without `aapt` via a custom compiler and generates stub classes like - R.java, BR.java and *Binding.java. + This macro resolves circular dependencies that occur when using Databinding with Kotlin + sources. It first compiles Kotlin code referenced in XML layouts using kt_jvm_library, + then processes Databinding in android_library. - Then Kotlin code can be safely compiled without errors. In the final stage, the stub classes - are replaced with actual classes by android_library target. + Without this macro, a circular dependency occurs because: + 1. android_library needs Kotlin classes to process XML references + 2. Kotlin classes need generated Databinding classes + 3. Databinding classes can't be generated until android_library processes resources - It also supports @BindingAdapters written in Kotlin. + The macro breaks this cycle by: + 1. Processing resources without aapt to generate stub classes (R.java, BR.java, *Binding.java) + 2. Compiling Kotlin against these stubs + 3. Replacing stubs with actual classes in a final android_library Args: - name: The name of the target. - srcs: Kotlin and Java classes for the target. - custom_package: Custom package for the target. Forwards to 'kt_|android_library'. - manifest: The AndroidManifest.xml file for android library. - assets: Assets for android_library rule - assets_dir: Assets dir for android_library rule - resource_files: The resource files for the target. - deps: The dependencies for the whole target. - plugins: Kotlin compiler plugins for internal Kotlin target - visibility: Visibility of the target. - tags: Tags for both Kotlin and Android resources target. + name: Name of the target. + srcs: List of Kotlin and Java source files. + custom_package: str, optional. Java package for generated R.java and BuildConfig files. + If not specified, derived from the manifest's package attribute. + manifest: Label, optional. Android manifest file. + assets: List of files, optional. Asset files to include in the library. + assets_dir: str, optional. Directory containing the assets. + resource_files: List of Android resource files (layouts, drawables, etc). + Must include any XML files that use data binding expressions. + deps: List of targets, optional. Dependencies required by both Kotlin sources + and Android resources. + plugins: List of Kotlin compiler plugins, optional. Applied only to the Kotlin + compilation phase. + visibility: List of visibility specifications, optional. + tags: List of tags, optional. Applied to both Kotlin and Android targets. """ # Create R.java and stub classes for classes that Android databinding and AAPT would produce diff --git a/tools/databinding/databinding_stubs.bzl b/tools/databinding/databinding_stubs.bzl index 88cd1361..5a583eca 100644 --- a/tools/databinding/databinding_stubs.bzl +++ b/tools/databinding/databinding_stubs.bzl @@ -17,6 +17,7 @@ Args: Outputs: %{name}_r.srcjar: The R and BR classes %{name}_binding.srcjar: All the databinding *Binding classes + %{name}_mapper.srcjar: Stub DatabindingMapperImpl class """ def _to_path(f): @@ -75,6 +76,7 @@ def _databinding_stubs_impl(ctx): args.use_param_file("--flagfile=%s", use_always = True) args.add("AAPT_LITE") + args.add("--name", str(ctx.label).lstrip("@")) args.add("--package", custom_package) args.add_joined( "--resource-files",