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

Reland "Use .r8dex.jar and .mergeddex.jar rather than .dex.zip"

This reverts commit 01c325ec.

Reason for revert: Repro'ed locally and ensured fixed.
To elaborate:
 * The breakage occurred only in incremental builds, and only when
   building before / after this change.
   * E.g. Clean builds were correct, but the break was due to
     the transition of before this CL to after.
 * The fix I've done here is just to rename the output of the
   pre-dexsplitter r8 output so that the stale artifact does
   not break things.

I think there might be a bug in dexsplitter that contributed
to this breakage as well, but not going to dig into it since
I've got another change underway to remove the use of
dexsplitter.

Original change's description:
> Revert "Use .r8dex.jar and .mergeddex.jar rather than .dex.zip"
>
> This reverts commit 89a7ef17.
>
> Reason for revert: This is breaking WebView bundles on the official builders, see crbug.com/1069724. It looks like bundletool looks specifically for .dex extensions[1], and this change breaks that assumption.
>
> 1. https://github.com/google/bundletool/blob/3586f8630e08733503ddbd1e74e10d4fb50e0d69/src/main/java/com/android/tools/build/bundletool/validation/DexFilesValidator.java#L48
>
> Original change's description:
> > Use .r8dex.jar and .mergeddex.jar rather than .dex.zip
> >
> > This just renames the extensions in order to make build speed summaries
> > distinguish the two.
> >
> > Bug: 1067273
> > Change-Id: I0a1e14cfed3557d29cd6beb0987436cacfaf6b65
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2140908
> > Commit-Queue: Andrew Grieve <agrieve@chromium.org>
> > Commit-Queue: Peter Wen <wnwen@chromium.org>
> > Auto-Submit: Andrew Grieve <agrieve@chromium.org>
> > Reviewed-by: Peter Wen <wnwen@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#757855}
>
> TBR=wnwen@chromium.org,agrieve@chromium.org
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug: 1067273, 1069724
> Change-Id: Ied71fc35aef250bdbf21ee4ec34f5bb2d0eb3423
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2145860
> Reviewed-by: Clark DuVall <cduvall@chromium.org>
> Commit-Queue: Clark DuVall <cduvall@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#758275}

Bug: 1067273, 1069724
Change-Id: I8c505ba68d1c5880a3b39eaec53455e804d48e29
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2149286Reviewed-by: default avatarClark DuVall <cduvall@chromium.org>
Reviewed-by: default avatarPeter Wen <wnwen@chromium.org>
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#759240}
parent 6b778298
...@@ -145,12 +145,12 @@ This step happens only when targets have `supports_android = true`. ...@@ -145,12 +145,12 @@ This step happens only when targets have `supports_android = true`.
This step is skipped when building using [Incremental Install]. This step is skipped when building using [Incremental Install].
When `is_java_debug = true`: When `is_java_debug = true`:
* [d8] merges all library `.dex.jar` files into a final `.dex.zip`. * [d8] merges all library `.dex.jar` files into a final `.mergeddex.jar`.
When `is_java_debug = false`: When `is_java_debug = false`:
* [R8] performs whole-program optimization on all library `lib.java` `.jar` * [R8] performs whole-program optimization on all library `lib.java` `.jar`
files and outputs a final `.dex.zip`. files and outputs a final `.r8dex.jar`.
* For App Bundles, R8 creates a single `.dex.zip` with the code from all * For App Bundles, R8 creates a single `.r8dex.jar` with the code from all
modules. modules.
[Incremental Install]: /build/android/incremental_install/README.md [Incremental Install]: /build/android/incremental_install/README.md
...@@ -160,7 +160,7 @@ When `is_java_debug = false`: ...@@ -160,7 +160,7 @@ When `is_java_debug = false`:
This step happens only when `is_java_debug = false`. This step happens only when `is_java_debug = false`.
* [dexsplitter.py] splits the single `.dex.zip` into per-module `.dex.zip` * [dexsplitter.py] splits the single `*dex.jar` into per-module `*dex.jar`
files. files.
## Test APKs with apk_under_test ## Test APKs with apk_under_test
......
...@@ -258,7 +258,7 @@ def _ZipMultidex(file_dir, dex_files): ...@@ -258,7 +258,7 @@ def _ZipMultidex(file_dir, dex_files):
""" """
ordered_files = [] # List of (archive name, file name) ordered_files = [] # List of (archive name, file name)
for f in dex_files: for f in dex_files:
if f.endswith('classes.dex.zip'): if f.endswith('dex.jar'):
ordered_files.append(('classes.dex', f)) ordered_files.append(('classes.dex', f))
break break
if not ordered_files: if not ordered_files:
......
...@@ -101,15 +101,15 @@ def main(args): ...@@ -101,15 +101,15 @@ def main(args):
if os.path.exists(module_dex_file): if os.path.exists(module_dex_file):
curr_location_to_dest.append((module_dex_file, dest)) curr_location_to_dest.append((module_dex_file, dest))
else: else:
module_dex_file += '.zip' module_dex_file += '.jar'
assert os.path.exists( assert os.path.exists(
module_dex_file), 'Dexsplitter tool output not found.' module_dex_file), 'Dexsplitter tool output not found.'
curr_location_to_dest.append((module_dex_file + '.zip', dest)) curr_location_to_dest.append((module_dex_file + '.jar', dest))
for curr_location, dest in curr_location_to_dest: for curr_location, dest in curr_location_to_dest:
with build_utils.AtomicOutput(dest) as f: with build_utils.AtomicOutput(dest) as f:
if curr_location.endswith('.zip'): if curr_location.endswith('.jar'):
if dest.endswith('.zip'): if dest.endswith('.jar'):
shutil.copy(curr_location, f.name) shutil.copy(curr_location, f.name)
else: else:
with zipfile.ZipFile(curr_location, 'r') as z: with zipfile.ZipFile(curr_location, 'r') as z:
...@@ -119,7 +119,7 @@ def main(args): ...@@ -119,7 +119,7 @@ def main(args):
options.input_dex_zip) options.input_dex_zip)
z.extract(namelist[0], f.name) z.extract(namelist[0], f.name)
else: else:
if dest.endswith('.zip'): if dest.endswith('.jar'):
build_utils.ZipDir( build_utils.ZipDir(
f.name, os.path.abspath(os.path.join(curr_location, os.pardir))) f.name, os.path.abspath(os.path.join(curr_location, os.pardir)))
else: else:
......
...@@ -2349,7 +2349,7 @@ if (enable_java_templates) { ...@@ -2349,7 +2349,7 @@ if (enable_java_templates) {
defined(invoker.static_library_dependent_targets) && _proguard_enabled defined(invoker.static_library_dependent_targets) && _proguard_enabled
if (_is_static_library_provider) { if (_is_static_library_provider) {
_static_library_sync_dex_path = _static_library_sync_dex_path =
"$_out_dir/static_library_synchronized_proguard.classes.dex.zip" "$_out_dir/static_library_synchronized_proguard.r8dex.jar"
_resource_ids_provider_deps = [] _resource_ids_provider_deps = []
foreach(_target, invoker.static_library_dependent_targets) { foreach(_target, invoker.static_library_dependent_targets) {
if (_target.is_resource_ids_provider) { if (_target.is_resource_ids_provider) {
...@@ -2397,7 +2397,11 @@ if (enable_java_templates) { ...@@ -2397,7 +2397,11 @@ if (enable_java_templates) {
if (!_incremental_apk) { if (!_incremental_apk) {
# Bundle modules don't build the dex here, but need to write this path # Bundle modules don't build the dex here, but need to write this path
# to their .build_config file. # to their .build_config file.
_final_dex_path = "$_out_dir/classes.dex.zip" if (_proguard_enabled) {
_final_dex_path = "$_base_path.r8dex.jar"
} else {
_final_dex_path = "$_base_path.mergeddex.jar"
}
} }
_android_manifest = _android_manifest =
...@@ -4593,7 +4597,7 @@ if (enable_java_templates) { ...@@ -4593,7 +4597,7 @@ if (enable_java_templates) {
} }
_unsplit_dex_zip = _unsplit_dex_zip =
"${target_gen_dir}/${_target_name}/${_target_name}__unsplit_dex.zip" "${target_gen_dir}/${_target_name}/${_target_name}__unsplit.r8dex.jar"
write_build_config(_build_config_target) { write_build_config(_build_config_target) {
type = "android_app_bundle" type = "android_app_bundle"
......
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