Commit 0dce4507 authored by Kevin McNee's avatar Kevin McNee Committed by Commit Bot

Revert "Android build: moved proguarding into dex() for most cases"

This reverts commit 93aa458f.

Reason for revert: Suspect for build failure https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/android-rel/2802

Original change's description:
> Android build: moved proguarding into dex() for most cases
> 
> This is to allow R8 to proguard and dex in one call, since it has some
> optimizations that only work when doing everything in one shot.
> 
> Bug: 872904
> Change-Id: Id2c75dac4d9feecae461a0b1e279056253f1b71b
> Reviewed-on: https://chromium-review.googlesource.com/c/1227142
> Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
> Reviewed-by: agrieve <agrieve@chromium.org>
> Commit-Queue: Sam Maier <smaier@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#597257}

TBR=yfriedman@chromium.org,agrieve@chromium.org,smaier@chromium.org

Change-Id: Idf8f855094fb1bf4c4e1b47326b76166a158c1e8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 872904
Reviewed-on: https://chromium-review.googlesource.com/c/1265559Reviewed-by: default avatarKevin McNee <mcnee@chromium.org>
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597274}
parent 3d31e76e
......@@ -6,7 +6,6 @@
import optparse
import os
import shutil
import sys
import tempfile
......@@ -57,23 +56,15 @@ def _ParseOptions(args):
'included by --proguard-configs, but that should '
'not actually be included.')
parser.add_option('--mapping', help='Path to proguard mapping to apply.')
parser.add_option('--mapping-output',
help='Path for proguard to output mapping file to.')
parser.add_option('--classpath', action='append',
help='Classpath for proguard.')
parser.add_option('--enable-dangerous-optimizations', action='store_true',
help='Enable optimizations which are known to have issues.')
parser.add_option('--main-dex-rules-path', action='append',
help='Paths to main dex rules for multidex'
'- only works with R8.')
parser.add_option('--verbose', '-v', action='store_true',
help='Print all proguard output')
options, _ = parser.parse_args(args)
assert not options.main_dex_rules_path or options.r8_path, \
"R8 must be enabled to pass main dex rules."
classpath = []
for arg in options.classpath:
classpath += build_utils.ParseGnList(arg)
......@@ -88,38 +79,16 @@ def _ParseOptions(args):
options.input_paths = build_utils.ParseGnList(options.input_paths)
if not options.mapping_output:
options.mapping_output = options.output_path + ".mapping"
return options
def _MoveTempDexFile(tmp_dex_dir, dex_path):
"""Move the temp dex file out of |tmp_dex_dir|.
Args:
tmp_dex_dir: Path to temporary directory created with tempfile.mkdtemp().
The directory should have just a single file.
dex_path: Target path to move dex file to.
Raises:
Exception if there are multiple files in |tmp_dex_dir|.
"""
tempfiles = os.listdir(tmp_dex_dir)
if len(tempfiles) > 1:
raise Exception('%d files created, expected 1' % len(tempfiles))
tmp_dex_path = os.path.join(tmp_dex_dir, tempfiles[0])
shutil.move(tmp_dex_path, dex_path)
def _CreateR8Command(options, map_output_path, output_dir):
def _CreateR8Command(options, map_output_path):
# TODO: R8 needs -applymapping equivalent.
cmd = [
'java', '-jar', options.r8_path,
'--no-desugaring',
'--no-data-resources',
'--output', output_dir,
'--classfile',
'--output', options.output_path,
'--pg-map-output', map_output_path,
]
......@@ -132,10 +101,6 @@ def _CreateR8Command(options, map_output_path, output_dir):
for config_file in options.proguard_configs:
cmd += ['--pg-conf', config_file]
if options.main_dex_rules_path:
for main_dex_rule in options.main_dex_rules_path:
cmd += ['--main-dex-rules', main_dex_rule]
cmd += options.input_paths
return cmd
......@@ -149,7 +114,6 @@ def main(args):
proguard.configs(options.proguard_configs)
proguard.config_exclusions(options.proguard_config_exclusions)
proguard.outjar(options.output_path)
proguard.mapping_output(options.mapping_output)
# If a jar is part of input no need to include it as library jar.
classpath = [
......@@ -163,17 +127,12 @@ def main(args):
# TODO(agrieve): Remove proguard usages.
if options.r8_path:
with tempfile.NamedTemporaryFile() as mapping_temp:
if options.output_path.endswith('.dex'):
with build_utils.TempDir() as tmp_dex_dir:
cmd = _CreateR8Command(options, mapping_temp.name, tmp_dex_dir)
build_utils.CheckOutput(cmd)
_MoveTempDexFile(tmp_dex_dir, options.output_path)
else:
cmd = _CreateR8Command(options, mapping_temp.name, options.output_path)
build_utils.CheckOutput(cmd)
# R8 will output mapping file to this temp file
cmd = _CreateR8Command(options, mapping_temp.name)
build_utils.CheckOutput(cmd)
# Copy the mapping file back to where it should be.
map_path = options.mapping_output
map_path = options.output_path + ".mapping"
with build_utils.AtomicOutput(map_path) as mapping:
# Mapping files generated by R8 include comments that may break
# some of our tooling so remove those.
......
......@@ -51,7 +51,6 @@ class ProguardCmdBuilder(object):
self._configs = None
self._config_exclusions = None
self._outjar = None
self._mapping_output = None
self._verbose = False
self._disabled_optimizations = []
......@@ -59,10 +58,6 @@ class ProguardCmdBuilder(object):
assert self._outjar is None
self._outjar = path
def mapping_output(self, path):
assert self._mapping_output is None
self._mapping_output = path
def mapping(self, path):
assert self._mapping is None
assert os.path.exists(path), path
......@@ -129,7 +124,7 @@ class ProguardCmdBuilder(object):
'-outjars', self._outjar,
'-printseeds', self._outjar + '.seeds',
'-printusage', self._outjar + '.usage',
'-printmapping', self._mapping_output,
'-printmapping', self._outjar + '.mapping',
]
if self._verbose:
......@@ -161,7 +156,7 @@ class ProguardCmdBuilder(object):
return [
self._outjar,
self._outjar + '.flags',
self._mapping_output,
self._outjar + '.mapping',
self._outjar + '.seeds',
self._outjar + '.usage',
]
......
This diff is collapsed.
This diff is collapsed.
......@@ -1500,9 +1500,7 @@ template("monochrome_public_apk_or_module_tmpl") {
}
version_name = chrome_version_name
if (experimental_r8_path == "") {
enable_multidex = true
}
enable_multidex = true
}
}
......@@ -1850,9 +1848,7 @@ android_app_bundle("monochrome_public_bundle") {
base_module_target = ":monochrome_public_base_module"
if (!is_java_debug) {
proguard_enabled = true
if (experimental_r8_path == "") {
enable_multidex = true
}
enable_multidex = true
proguard_android_sdk_dep = webview_framework_dep
}
if (modularize_ar) {
......
......@@ -49,16 +49,19 @@ android_library("runtime_library_for_tests_java") {
]
}
proguarded_dist_dex("webapk_runtime_library") {
dist_jar("webapk_runtime_library") {
requires_android = true
deps = [
":runtime_library_for_assets_java",
]
proguard_enabled = true
proguard_configs = [
"runtime_library.proguard.flags",
"//base/android/proguard/chromium_code.flags",
"//base/android/proguard/chromium_apk.flags",
]
output = "$target_gen_dir/$runtime_library_dex_asset_name"
output = "$target_gen_dir/$target_name.jar"
dex_path = "$target_gen_dir/$runtime_library_dex_asset_name"
}
android_assets("runtime_library_assets") {
......
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