Commit a5be1dc0 authored by Andrew Grieve's avatar Andrew Grieve Committed by Commit Bot

Reland #2: Android: Increase default_min_sdk_version 19 -> 21

This reverts commit b467849d.

Reason for revert:
* Fixed putting apk_under_test classes in test apks
  (for apks with min_sdk_version < 21).

Original change's description:
> Revert "Reland "Android: Increase default_min_sdk_version 19 -> 21""
>
> This reverts commit 93dfa799.
>
> Reason for revert: The CL causes failures on android-cronet-arm64-dbg and Android arm64 Builder (dbg).
> https://ci.chromium.org/p/chromium/builders/ci/android-cronet-arm64-dbg/59030
>
> https://ci.chromium.org/p/chromium/builders/ci/Android%20arm64%20Builder%20%28dbg%29/42461
>
> The error message is "Error: Cannot fit requested classes in a single dex file (# methods: 76373 > 65536). Try supplying a main-dex list."
>

Bug: 1091919
Change-Id: I76a827ff2becbdcf936fd0f94a4318a9aa73c1e9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2240138
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: default avatarPeter Wen <wnwen@chromium.org>
Reviewed-by: default avatarPaul Jensen <pauljensen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#777331}
parent 538ce8d5
...@@ -62,6 +62,9 @@ def _ParseArgs(args): ...@@ -62,6 +62,9 @@ def _ParseArgs(args):
'--multi-dex', '--multi-dex',
action='store_true', action='store_true',
help='Allow multiple dex files within output.') help='Allow multiple dex files within output.')
parser.add_argument('--library',
action='store_true',
help='Allow numerous dex files within output.')
parser.add_argument('--r8-jar-path', required=True, help='Path to R8 jar.') parser.add_argument('--r8-jar-path', required=True, help='Path to R8 jar.')
parser.add_argument('--desugar', action='store_true') parser.add_argument('--desugar', action='store_true')
parser.add_argument( parser.add_argument(
...@@ -334,13 +337,15 @@ def _PerformDexlayout(tmp_dir, tmp_dex_output, options): ...@@ -334,13 +337,15 @@ def _PerformDexlayout(tmp_dir, tmp_dex_output, options):
def _CreateFinalDex(d8_inputs, output, tmp_dir, dex_cmd, options=None): def _CreateFinalDex(d8_inputs, output, tmp_dir, dex_cmd, options=None):
tmp_dex_output = os.path.join(tmp_dir, 'tmp_dex_output.zip') tmp_dex_output = os.path.join(tmp_dir, 'tmp_dex_output.zip')
if (output.endswith('.dex') needs_dexing = not all(f.endswith('.dex') for f in d8_inputs)
or not all(f.endswith('.dex') for f in d8_inputs)): needs_dexmerge = output.endswith('.dex') or not (options and options.library)
if needs_dexing or needs_dexmerge:
if options: if options:
if options.main_dex_list_path: if options.main_dex_list_path:
dex_cmd = dex_cmd + ['--main-dex-list', options.main_dex_list_path] dex_cmd = dex_cmd + ['--main-dex-list', options.main_dex_list_path]
elif options.multi_dex and int(options.min_api or 1) < 21: elif options.library and int(options.min_api or 1) < 21:
# When dexing library targets, it doesn't matter what's in the main dex. # When dexing D8 requires a main dex list pre-21. For library targets,
# it doesn't matter what's in the main dex, so just use a dummy one.
tmp_main_dex_list_path = os.path.join(tmp_dir, 'main_list.txt') tmp_main_dex_list_path = os.path.join(tmp_dir, 'main_list.txt')
with open(tmp_main_dex_list_path, 'w') as f: with open(tmp_main_dex_list_path, 'w') as f:
f.write('Foo.class\n') f.write('Foo.class\n')
......
...@@ -1695,7 +1695,8 @@ def main(argv): ...@@ -1695,7 +1695,8 @@ def main(argv):
# Add all tested classes to the test's classpath to ensure that the test's # Add all tested classes to the test's classpath to ensure that the test's
# java code is a superset of the tested apk's java code # java code is a superset of the tested apk's java code
java_full_classpath.extend( java_full_classpath_extended = list(java_full_classpath)
java_full_classpath_extended.extend(
p for p in tested_apk_config['java_runtime_classpath'] p for p in tested_apk_config['java_runtime_classpath']
if p not in java_full_classpath) if p not in java_full_classpath)
# Include in the classpath classes that are added directly to the apk under # Include in the classpath classes that are added directly to the apk under
...@@ -1716,13 +1717,13 @@ def main(argv): ...@@ -1716,13 +1717,13 @@ def main(argv):
p for p in tested_apk_config['javac_full_classpath'] p for p in tested_apk_config['javac_full_classpath']
if p not in javac_full_classpath) if p not in javac_full_classpath)
# Exclude dex files from the test apk that exist within the apk under test. # Exclude .jar files from the test apk that exist within the apk under test.
# TODO(agrieve): When proguard is enabled, this filtering logic happens
# within proguard.py. Move the logic for the proguard case to here.
tested_apk_library_deps = tested_apk_deps.All('java_library') tested_apk_library_deps = tested_apk_deps.All('java_library')
tested_apk_deps_dex_files = [c['dex_path'] for c in tested_apk_library_deps] tested_apk_dex_files = {c['dex_path'] for c in tested_apk_library_deps}
all_dex_files = [ all_dex_files = [p for p in all_dex_files if p not in tested_apk_dex_files]
p for p in all_dex_files if not p in tested_apk_deps_dex_files tested_apk_jar_files = set(tested_apk_config['java_runtime_classpath'])
java_full_classpath = [
p for p in java_full_classpath if p not in tested_apk_jar_files
] ]
if options.type in ('android_apk', 'dist_aar', 'dist_jar', if options.type in ('android_apk', 'dist_aar', 'dist_jar',
...@@ -1763,6 +1764,9 @@ def main(argv): ...@@ -1763,6 +1764,9 @@ def main(argv):
if options.type in ('android_apk', 'dist_jar', 'java_binary', 'junit_binary', if options.type in ('android_apk', 'dist_jar', 'java_binary', 'junit_binary',
'android_app_bundle_module', 'android_app_bundle'): 'android_app_bundle_module', 'android_app_bundle'):
deps_info['java_runtime_classpath'] = java_full_classpath deps_info['java_runtime_classpath'] = java_full_classpath
if options.tested_apk_config:
deps_info['java_runtime_classpath_extended'] = (
java_full_classpath_extended)
if options.type in ('android_apk', 'dist_jar'): if options.type in ('android_apk', 'dist_jar'):
all_interface_jars = [] all_interface_jars = []
......
...@@ -46,9 +46,8 @@ if (is_android || is_chromeos) { ...@@ -46,9 +46,8 @@ if (is_android || is_chromeos) {
enable_chrome_android_internal = has_chrome_android_internal enable_chrome_android_internal = has_chrome_android_internal
# The default to use for android:minSdkVersion for targets that do # The default to use for android:minSdkVersion for targets that do
# not explicitly set it. This can generally be set higher than the # not explicitly set it.
# default, but not lower. default_min_sdk_version = 21
default_min_sdk_version = 19
# Android API level for 32 bits platforms # Android API level for 32 bits platforms
android32_ndk_api_level = 16 android32_ndk_api_level = 16
......
...@@ -1304,6 +1304,9 @@ if (enable_java_templates) { ...@@ -1304,6 +1304,9 @@ if (enable_java_templates) {
} }
} }
# It's not safe to dex merge with libraries dex'ed at higher api versions.
assert(!_is_dex_merging || _min_sdk_version >= default_min_sdk_version)
# For D8's backported method desugaring to work properly, the dex merge step # For D8's backported method desugaring to work properly, the dex merge step
# must not be set to a higher minSdkVersion than it was for the libraries. # must not be set to a higher minSdkVersion than it was for the libraries.
if (_enable_desugar && _is_dex_merging) { if (_enable_desugar && _is_dex_merging) {
...@@ -1363,9 +1366,13 @@ if (enable_java_templates) { ...@@ -1363,9 +1366,13 @@ if (enable_java_templates) {
args = [ args = [
"--proguard-configs=@FileArg($_rebased_build_config:deps_info:proguard_all_configs)", "--proguard-configs=@FileArg($_rebased_build_config:deps_info:proguard_all_configs)",
"--input-paths=@FileArg($_rebased_build_config:deps_info:java_runtime_classpath)",
"--min-api=$_min_sdk_version", "--min-api=$_min_sdk_version",
] ]
if (defined(invoker.has_apk_under_test) && invoker.has_apk_under_test) {
args += [ "--input-paths=@FileArg($_rebased_build_config:deps_info:java_runtime_classpath_extended)" ]
} else {
args += [ "--input-paths=@FileArg($_rebased_build_config:deps_info:java_runtime_classpath)" ]
}
if (enable_bazel_desugar) { if (enable_bazel_desugar) {
deps += [ "//third_party/bazel/desugar:desugar_runtime_java" ] deps += [ "//third_party/bazel/desugar:desugar_runtime_java" ]
inputs += [ "$root_build_dir/lib.java/third_party/bazel/desugar/Desugar_runtime.jar" ] inputs += [ "$root_build_dir/lib.java/third_party/bazel/desugar/Desugar_runtime.jar" ]
...@@ -1413,13 +1420,14 @@ if (enable_java_templates) { ...@@ -1413,13 +1420,14 @@ if (enable_java_templates) {
} }
} }
} else { # !_proguard_enabled } else { # !_proguard_enabled
_is_library = defined(invoker.is_library) && invoker.is_library
_input_class_jars = [] _input_class_jars = []
if (defined(invoker.input_class_jars)) { if (defined(invoker.input_class_jars)) {
_input_class_jars = invoker.input_class_jars _input_class_jars = invoker.input_class_jars
} }
_deps = invoker.deps _deps = invoker.deps
if (_is_dex_merging && enable_bazel_desugar) { if (!_is_library && enable_bazel_desugar) {
# It would be more efficient to use the pre-dex'ed copy of the runtime, # It would be more efficient to use the pre-dex'ed copy of the runtime,
# but it's easier to add it in this way. # but it's easier to add it in this way.
_deps += [ "//third_party/bazel/desugar:desugar_runtime_java" ] _deps += [ "//third_party/bazel/desugar:desugar_runtime_java" ]
...@@ -1537,11 +1545,10 @@ if (enable_java_templates) { ...@@ -1537,11 +1545,10 @@ if (enable_java_templates) {
deps += [ ":${_main_dex_list_target_name}" ] deps += [ ":${_main_dex_list_target_name}" ]
inputs += [ _main_dex_list_path ] inputs += [ _main_dex_list_path ]
} }
} else if (defined(invoker.enable_library_multidex) &&
invoker.enable_library_multidex) {
args += [ "--multi-dex" ]
} }
if (_is_library) {
args += [ "--library" ]
}
if (defined(invoker.input_dex_filearg)) { if (defined(invoker.input_dex_filearg)) {
inputs += [ invoker.build_config ] inputs += [ invoker.build_config ]
args += [ "--dex-inputs-filearg=${invoker.input_dex_filearg}" ] args += [ "--dex-inputs-filearg=${invoker.input_dex_filearg}" ]
...@@ -1579,7 +1586,7 @@ if (enable_java_templates) { ...@@ -1579,7 +1586,7 @@ if (enable_java_templates) {
# 1) not require recompiles when toggling is_java_debug, # 1) not require recompiles when toggling is_java_debug,
# 2) allow incremental_install=1 to still have local variable # 2) allow incremental_install=1 to still have local variable
# information even when is_java_debug=false. # information even when is_java_debug=false.
if (!is_java_debug && _is_dex_merging) { if (!is_java_debug && !_is_library) {
args += [ "--release" ] args += [ "--release" ]
} }
...@@ -3252,8 +3259,6 @@ if (enable_java_templates) { ...@@ -3252,8 +3259,6 @@ if (enable_java_templates) {
# from the final .jar file. # from the final .jar file.
# jar_included_patterns: Optional list of .class file patterns to include # jar_included_patterns: Optional list of .class file patterns to include
# in the final .jar file. jar_excluded_patterns take precedence over this. # in the final .jar file. jar_excluded_patterns take precedence over this.
# min_sdk_version: Optional. The minimum Android SDK version this target
# supports.
# #
# For 'android_apk' and 'android_app_bundle_module' targets only: # For 'android_apk' and 'android_app_bundle_module' targets only:
# #
...@@ -3789,10 +3794,8 @@ if (enable_java_templates) { ...@@ -3789,10 +3794,8 @@ if (enable_java_templates) {
deps += _classpath_deps + [ ":$_header_target_name" ] deps += _classpath_deps + [ ":$_header_target_name" ]
} }
# For library targets, we do not need a proper main dex list, but do
# need to allow multiple dex files.
enable_multidex = false enable_multidex = false
enable_library_multidex = true is_library = true
} }
_public_deps += [ ":${target_name}__dex" ] _public_deps += [ ":${target_name}__dex" ]
} }
......
...@@ -2430,9 +2430,7 @@ if (enable_java_templates) { ...@@ -2430,9 +2430,7 @@ if (enable_java_templates) {
_final_deps = [] _final_deps = []
_enable_main_dex_list = _enable_main_dex_list = _enable_multidex && _min_sdk_version < 21
_enable_multidex &&
(!defined(invoker.min_sdk_version) || invoker.min_sdk_version < 21)
if (_enable_main_dex_list) { if (_enable_main_dex_list) {
_generated_proguard_main_dex_config = _generated_proguard_main_dex_config =
"$_base_path.resources.main-dex-proguard.txt" "$_base_path.resources.main-dex-proguard.txt"
...@@ -2927,12 +2925,17 @@ if (enable_java_templates) { ...@@ -2927,12 +2925,17 @@ if (enable_java_templates) {
deps += _deps + [ ":$_compile_resources_target" ] deps += _deps + [ ":$_compile_resources_target" ]
proguard_mapping_path = _proguard_mapping_path proguard_mapping_path = _proguard_mapping_path
proguard_sourcefile_suffix = "$android_channel-$_version_code" proguard_sourcefile_suffix = "$android_channel-$_version_code"
} else { has_apk_under_test = defined(invoker.apk_under_test)
} else if (_min_sdk_version >= default_min_sdk_version) {
# Enable dex merging only when min_sdk_version is >= what the library
# .dex files were created with.
input_dex_filearg = input_dex_filearg =
"@FileArg(${_rebased_build_config}:final_dex:all_dex_files)" "@FileArg(${_rebased_build_config}:final_dex:all_dex_files)"
if (_enable_main_dex_list) { if (_enable_main_dex_list) {
main_dex_list_input_classes_filearg = "@FileArg(${_rebased_build_config}:deps_info:java_runtime_classpath)" main_dex_list_input_classes_filearg = "@FileArg(${_rebased_build_config}:deps_info:java_runtime_classpath_extended)"
} }
} else {
input_classes_filearg = "@FileArg($_rebased_build_config:deps_info:java_runtime_classpath)"
} }
if (_is_static_library_provider) { if (_is_static_library_provider) {
......
...@@ -10,7 +10,6 @@ ...@@ -10,7 +10,6 @@
xmlns:tools="http://schemas.android.com/tools" xmlns:tools="http://schemas.android.com/tools"
package="org.chromium.android_browsertests_apk"> package="org.chromium.android_browsertests_apk">
<uses-sdk android:minSdkVersion="19" android:targetSdkVersion="23" />
<uses-feature android:glEsVersion="0x00020000" /> <uses-feature android:glEsVersion="0x00020000" />
<uses-permission android:name="android.permission.ACCESS_COARSE_LOCATION"/> <uses-permission android:name="android.permission.ACCESS_COARSE_LOCATION"/>
......
...@@ -1110,6 +1110,7 @@ if (!is_component_build) { ...@@ -1110,6 +1110,7 @@ if (!is_component_build) {
"cronet_smoketests_missing_native_library_instrumentation_apk") { "cronet_smoketests_missing_native_library_instrumentation_apk") {
apk_name = "MissingNativeLibrarySmokeTestInstrumentation" apk_name = "MissingNativeLibrarySmokeTestInstrumentation"
android_manifest = "test/javatests/AndroidManifest.xml" android_manifest = "test/javatests/AndroidManifest.xml"
min_sdk_version = _cronet_min_sdk_version
sources = cronet_smoketests_native_common_srcs + [ "test/smoketests/src/org/chromium/net/smoke/MissingNativeLibraryTest.java" ] sources = cronet_smoketests_native_common_srcs + [ "test/smoketests/src/org/chromium/net/smoke/MissingNativeLibraryTest.java" ]
deps = [ deps = [
...@@ -1179,6 +1180,7 @@ if (!is_component_build) { ...@@ -1179,6 +1180,7 @@ if (!is_component_build) {
} }
test("cronet_unittests_android") { test("cronet_unittests_android") {
min_sdk_version = _cronet_min_sdk_version
deps = [ deps = [
":cronet_impl_native_base_java", ":cronet_impl_native_base_java",
":cronet_static", ":cronet_static",
...@@ -1204,6 +1206,7 @@ if (!is_component_build) { ...@@ -1204,6 +1206,7 @@ if (!is_component_build) {
} }
test("cronet_tests_android") { test("cronet_tests_android") {
min_sdk_version = _cronet_min_sdk_version
deps = [ deps = [
":cronet_impl_native_base_java", ":cronet_impl_native_base_java",
":cronet_static", ":cronet_static",
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment