Skip to content

Commit

Permalink
Android: Add GN arg "desugar_with_d8"
Browse files Browse the repository at this point in the history
This is an initial implementation of desugaring with D8/R8 rather
than Desugar.jar. The flag is default false, so this change should have
no affect without it.

Known issues:
* Does not work with incremental dexing
* Produces larger apks
* Generated classes not mapped to source files by supersize

Tested that chrome_public_apk and monochrome_public_bundle start without
crashing.

TBR=agrieve # Trivial BUILD.gn updates

Bug: 1015559
Change-Id: I3030f6f489c23f208461f4400c1849569bdbd5fe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2127506
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Peter Wen <wnwen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#755006}
  • Loading branch information
agrieve authored and Commit Bot committed Mar 31, 2020
1 parent 754eaff commit c3fdf09
Show file tree
Hide file tree
Showing 10 changed files with 145 additions and 26 deletions.
79 changes: 69 additions & 10 deletions build/android/gyp/dex.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@
import convert_dex_profile


_IGNORE_WARNINGS = (
# A play services library triggers this.
'Type `libcore.io.Memory` was not found', )


def _ParseArgs(args):
args = build_utils.ExpandFileArgs(args)
parser = argparse.ArgumentParser()
Expand Down Expand Up @@ -53,6 +58,15 @@ def _ParseArgs(args):
action='store_true',
help='Allow multiple dex files within output.')
parser.add_argument('--r8-jar-path', required=True, help='Path to R8 jar.')
parser.add_argument('--desugar', action='store_true')
parser.add_argument(
'--bootclasspath',
action='append',
help='GN-list of bootclasspath. Needed for --desugar')
parser.add_argument(
'--classpath',
action='append',
help='GN-list of full classpath. Needed for --desugar')
parser.add_argument(
'--release',
action='store_true',
Expand Down Expand Up @@ -100,9 +114,16 @@ def _ParseArgs(args):
if options.main_dex_list_path and not options.multi_dex:
parser.error('--main-dex-list-path is unused if multidex is not enabled')

if options.desugar and options.classpath is None:
parser.error('--classpath required with use of --desugar')
if options.desugar and options.bootclasspath is None:
parser.error('--bootclasspath required with use of --desugar')

options.class_inputs = build_utils.ParseGnList(options.class_inputs)
options.class_inputs_filearg = build_utils.ParseGnList(
options.class_inputs_filearg)
options.bootclasspath = build_utils.ParseGnList(options.bootclasspath)
options.classpath = build_utils.ParseGnList(options.classpath)
options.dex_inputs = build_utils.ParseGnList(options.dex_inputs)
options.dex_inputs_filearg = build_utils.ParseGnList(
options.dex_inputs_filearg)
Expand All @@ -112,7 +133,27 @@ def _ParseArgs(args):

def _RunD8(dex_cmd, input_paths, output_path):
dex_cmd = dex_cmd + ['--output', output_path] + input_paths
build_utils.CheckOutput(dex_cmd, print_stderr=False)

def stderr_filter(output):
# Filter out warnings caused by our fake main dex list used to enable
# multidex on library targets.
# Warning: Application does not contain `Foo` as referenced in main-dex-list
pattern = r'does not contain `Foo`'
pattern += '|' + '|'.join(re.escape(p) for p in _IGNORE_WARNINGS)
output = build_utils.FilterLines(output, pattern)

# Each warning has a prefix line of tthe file it's from. If we've filtered
# out the warning, then also filter out the file header.
# E.g.:
# Warning in path/to/Foo.class:
# Error message #1 indented here.
# Error message #2 indented here.
output = re.sub(r'^Warning in .*?:\n(?! )', '', output, flags=re.MULTILINE)
return output

# stdout sometimes spams with things like:
# Stripped invalid locals information from 1 method.
build_utils.CheckOutput(dex_cmd, stderr_filter=stderr_filter)


def _EnvWithArtLibPath(binary_path):
Expand Down Expand Up @@ -353,12 +394,16 @@ def _CreateIntermediateDexFiles(changes, options, tmp_dir, dex_cmd):
tmp_extract_dir = os.path.join(tmp_dir, 'tmp_extract_dir')
os.mkdir(tmp_extract_dir)

# Check whether changes were to a non-jar file, requiring full re-dex.
# E.g. r8.jar updated.
rebuild_all = changes.HasStringChanges() or not all(
p.endswith('.jar') for p in changes.IterChangedPaths())
# Do a full rebuild when changes are to classpath or other non-input files.
allowed_changed = set(options.class_inputs)
allowed_changed.update(options.dex_inputs)
strings_changed = changes.HasStringChanges()
non_direct_input_changed = next(
(p for p in changes.IterChangedPaths() if p not in allowed_changed), None)

if rebuild_all:
if strings_changed or non_direct_input_changed:
logging.debug('Full dex required: strings_changed=%s path_changed=%s',
strings_changed, non_direct_input_changed)
changes = None
class_files = _ExtractClassFiles(changes, tmp_extract_dir,
options.class_inputs)
Expand Down Expand Up @@ -411,6 +456,8 @@ def main(args):
input_paths.append(options.main_dex_list_path)
input_paths.append(options.r8_jar_path)

depfile_deps = options.class_inputs_filearg + options.dex_inputs_filearg

output_paths = [options.output]

if options.incremental_dir:
Expand All @@ -425,25 +472,37 @@ def main(args):

dex_cmd = [
build_utils.JAVA_PATH, '-jar', options.r8_jar_path, 'd8',
'--no-desugaring'
]
if options.release:
dex_cmd += ['--release']
if options.min_api:
dex_cmd += ['--min-api', options.min_api]

if options.desugar:
dex_cmd += ['--lib', build_utils.JAVA_HOME]
for path in options.bootclasspath:
dex_cmd += ['--lib', path]
for path in options.classpath:
dex_cmd += ['--classpath', path]
depfile_deps += options.classpath
depfile_deps += options.bootclasspath
input_paths += options.classpath
input_paths += options.bootclasspath
else:
dex_cmd += ['--no-desugaring']

if options.force_enable_assertions:
dex_cmd += ['--force-enable-assertions']

md5_check.CallAndWriteDepfileIfStale(
lambda changes: _OnStaleMd5(changes, options, final_dex_inputs, dex_cmd),
options,
depfile_deps=options.class_inputs_filearg + options.dex_inputs_filearg,
output_paths=output_paths,
input_paths=input_paths,
input_strings=dex_cmd + [bool(options.incremental_dir)],
output_paths=output_paths,
pass_changes=True,
track_subpaths_allowlist=track_subpaths_allowlist)
track_subpaths_allowlist=track_subpaths_allowlist,
depfile_deps=depfile_deps)


if __name__ == '__main__':
Expand Down
1 change: 1 addition & 0 deletions build/android/gyp/main_dex_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ def main():
'-jar',
args.r8_path,
'--classfile',
'--no-desugaring',
'--lib',
args.shrinked_android_path,
]
Expand Down
6 changes: 5 additions & 1 deletion build/android/gyp/proguard.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,8 @@ def _ParseOptions():
'--force-enable-assertions',
action='store_true',
help='Forcefully enable javac generated assertion code.')
parser.add_argument(
'--desugar', action='store_true', help='Enable R8 Desugaring')


options = parser.parse_args(args)
Expand Down Expand Up @@ -229,14 +231,16 @@ def _OptimizeWithR8(options,
build_utils.JAVA_PATH,
'-jar',
options.r8_path,
'--no-desugaring',
'--no-data-resources',
'--output',
tmp_output,
'--pg-map-output',
tmp_mapping_path,
]

if not options.desugar:
cmd += ['--no-desugaring']

for lib in libraries:
cmd += ['--lib', lib]

Expand Down
2 changes: 1 addition & 1 deletion build/android/gyp/util/build_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ def FilterLines(output, filter_string):
"""
re_filter = re.compile(filter_string)
return '\n'.join(
line for line in output.splitlines() if not re_filter.search(line))
line for line in output.split('\n') if not re_filter.search(line))


def FilterReflectiveAccessJavaWarnings(output):
Expand Down
3 changes: 3 additions & 0 deletions build/config/android/config.gni
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,9 @@ if (is_android || is_chromeos) {

# For local development, it's nice to not fail builds on warnings.
java_warnings_as_errors = !is_java_debug

# Desugar with D8 rather than Desugar tool.
desugar_with_d8 = false
}

# Path to where selected build variables are written to.
Expand Down
54 changes: 45 additions & 9 deletions build/config/android/internal_rules.gni
Original file line number Diff line number Diff line change
Expand Up @@ -1084,6 +1084,10 @@ if (enable_java_templates) {
args += [ "--force-enable-assertions" ]
}

if (desugar_with_d8) {
args += [ "--desugar" ]
}

if (defined(invoker.args)) {
args += invoker.args
}
Expand Down Expand Up @@ -1170,6 +1174,10 @@ if (enable_java_templates) {
_main_dex_rules = "//build/android/main_dex_classes.flags"
}

if (invoker.enable_desugar || _proguard_enabled) {
_rebased_build_config = rebase_path(invoker.build_config, root_build_dir)
}

if (_proguard_enabled) {
if (_proguarding_with_r8) {
_proguard_output_path = invoker.output
Expand Down Expand Up @@ -1203,7 +1211,6 @@ if (enable_java_templates) {
inputs += invoker.proguard_configs
}

_rebased_build_config = rebase_path(build_config, root_build_dir)
args = [
"--proguard-configs=@FileArg($_rebased_build_config:deps_info:proguard_all_configs)",
"--input-paths=@FileArg($_rebased_build_config:deps_info:java_runtime_classpath)",
Expand Down Expand Up @@ -1368,11 +1375,14 @@ if (enable_java_templates) {
} else if (enable_incremental_d8 &&
!(defined(invoker.disable_incremental) &&
invoker.disable_incremental)) {
# Don't use incremental dexing for ProGuarded inputs as a precaution.
args += [
"--incremental-dir",
rebase_path("$target_out_dir/$target_name", root_build_dir),
]
# TODO(crbug.com/1015559): Fix incremental builds when d8 is desugaring.
if (!desugar_with_d8) {
# Don't use incremental dexing for ProGuarded inputs as a precaution.
args += [
"--incremental-dir",
rebase_path("$target_out_dir/$target_name", root_build_dir),
]
}
}

if (_enable_multidex) {
Expand Down Expand Up @@ -1441,6 +1451,23 @@ if (enable_java_templates) {
]
}

if (invoker.enable_desugar) {
args += [
"--desugar",
"--bootclasspath=@FileArg($_rebased_build_config:android:sdk_jars)",
"--classpath=@FileArg($_rebased_build_config:deps_info:javac_full_interface_classpath)",
]
if (defined(invoker.final_ijar_path)) {
# Need to include the input .interface.jar on the classpath in order to make
# jar_excluded_patterns classes visible to desugar.
args += [
"--classpath",
rebase_path(invoker.final_ijar_path, root_build_dir),
]
inputs += [ invoker.final_ijar_path ]
}
}

inputs += [ _r8_path ]
args += [
"--r8-jar-path",
Expand Down Expand Up @@ -1542,7 +1569,6 @@ if (enable_java_templates) {
# Turned off because of existing code which fails the assertion
_enable_thread_annotations = false

_desugar = defined(invoker.supports_android) && invoker.supports_android
_jacoco_instrument = invoker.jacoco_instrument
_skip_jetify = defined(invoker.skip_jetify) && invoker.skip_jetify

Expand Down Expand Up @@ -1677,7 +1703,7 @@ if (enable_java_templates) {
_previous_output_jar = _java_bytecode_rewriter_output_jar
}

if (_desugar) {
if (invoker.enable_desugar) {
_desugar_target = "${target_name}__desugar"
_desugar_input_jar = _previous_output_jar
_desugar_output_jar = "$target_out_dir/$target_name-desugar.jar"
Expand Down Expand Up @@ -3532,6 +3558,8 @@ if (enable_java_templates) {
}
_public_deps += [ ":$_copy_system_library_target_name" ]
} else {
_enable_desugar = (!defined(invoker.enable_desugar) ||
!invoker.enable_desugar) && _supports_android
_process_prebuilt_target_name = "${target_name}__process_prebuilt"
process_java_prebuilt(_process_prebuilt_target_name) {
forward_variables_from(invoker,
Expand All @@ -3542,8 +3570,8 @@ if (enable_java_templates) {
"jar_included_patterns",
"skip_jetify",
])
enable_desugar = _enable_desugar && !desugar_with_d8
is_prebuilt = _is_prebuilt
supports_android = _supports_android
enable_build_hooks_android = _enable_build_hooks_android
build_config = _build_config
input_jar_path = _unprocessed_jar_path
Expand Down Expand Up @@ -3578,12 +3606,20 @@ if (enable_java_templates) {
dex("${target_name}__dex") {
input_class_jars = [ _final_jar_path ]

enable_desugar = _enable_desugar && desugar_with_d8

# There's no value in per-class dexing prebuilts since they never
# change just one class at a time.
disable_incremental = _is_prebuilt
output = _dex_path
deps = [ ":$_process_prebuilt_target_name" ]

if (enable_desugar) {
build_config = _build_config
final_ijar_path = _final_ijar_path
deps += _java_header_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
Expand Down
6 changes: 6 additions & 0 deletions build/config/android/rules.gni
Original file line number Diff line number Diff line change
Expand Up @@ -1455,6 +1455,9 @@ if (enable_java_templates) {
# bypass_platform_checks: Disables checks about cross-platform (Java/Android)
# dependencies for this target. This will allow depending on an
# android_library target, for example.
# enable_desugar: If false, disables desugaring of lambdas, etc. Use this
# only when you are sure the library does not require desugaring. E.g.
# to hide warnings shown from desugaring.
#
# additional_jar_files: Use to package additional files (Java resources)
# into the output jar. Pass a list of length-2 lists with format:
Expand Down Expand Up @@ -1673,6 +1676,7 @@ if (enable_java_templates) {
deps = _deps
build_config = _build_config
proguard_enabled = true
enable_desugar = desugar_with_d8
forward_variables_from(invoker,
[
"proguard_configs",
Expand Down Expand Up @@ -2917,6 +2921,7 @@ if (enable_java_templates) {
"disable_r8_outlining",
"dexlayout_profile",
])
enable_desugar = desugar_with_d8
min_sdk_version = _min_sdk_version
proguard_enabled = _proguard_enabled
build_config = _build_config
Expand Down Expand Up @@ -4631,6 +4636,7 @@ if (enable_java_templates) {
"min_sdk_version",
])
enable_multidex = _enable_multidex
enable_desugar = desugar_with_d8
proguard_enabled = true
proguard_mapping_path = _proguard_mapping_path
proguard_sourcefile_suffix = "$android_channel-$_version_code"
Expand Down
6 changes: 5 additions & 1 deletion third_party/accessibility_test_framework/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,13 @@
import("//build/config/android/rules.gni")

android_java_prebuilt("accessibility_test_framework_java") {
testonly = true

# Uses wrong version of proto (not protolite).
# Disable these to avoid build warnings.
enable_bytecode_checks = false
testonly = true
enable_desugar = false

jar_path = "lib/accessibility-test-framework.jar"
deps = [
"//third_party/android_deps:android_support_v4_java",
Expand Down
Loading

0 comments on commit c3fdf09

Please sign in to comment.