Android: Use jdeps and zipinfo for bytecode checks
An initial version of using jdeps and zipinfo to check for missing deps
in the direct deps list vs the transitive deps list, gated by an
environment variable for ease of switching until this becomes the
default.
Also fixes a bug in CachedFile that relative_to failes to properly find
the correct relative paths when paths aren't resolved to absolute paths.
Bug: 1099522
Change-Id: Iafe764cea776aaf7575941cf071568fdacc04746
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4544586
Reviewed-by: Mohamed Heikal <mheikal@chromium.org>
Commit-Queue: Peter Wen <wnwen@chromium.org>
Auto-Submit: Peter Wen <wnwen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1146210}
diff --git a/build/android/gyp/bytecode_processor.py b/build/android/gyp/bytecode_processor.py
index f6065db..cc0a4c73 100755
--- a/build/android/gyp/bytecode_processor.py
+++ b/build/android/gyp/bytecode_processor.py
@@ -6,14 +6,113 @@
"""Wraps bin/helper/bytecode_processor and expands @FileArgs."""
import argparse
+import collections
+import logging
+import os
+import pathlib
import sys
+from typing import Dict, List
import javac_output_processor
from util import build_utils
+from util import jar_utils
from util import server_utils
import action_helpers # build_utils adds //build to sys.path.
+def _ShouldIgnoreDep(dep_name: str):
+ if 'gen.base_module.R' in dep_name:
+ return True
+ return False
+
+
+def _ParseDepGraph(jar_path: str, output_dir: str):
+ output = jar_utils.run_jdeps(build_output_dir=pathlib.Path(output_dir),
+ filepath=pathlib.Path(jar_path))
+ assert output is not None, f'Unable to parse jdep for {jar_path}'
+ dep_graph = collections.defaultdict(set)
+ # pylint: disable=line-too-long
+ # Example output:
+ # java.javac.jar -> java.base
+ # java.javac.jar -> not found
+ # org.chromium.chrome.browser.tabmodel.AsyncTabParamsManagerFactory -> java.lang.Object java.base
+ # org.chromium.chrome.browser.tabmodel.TabWindowManagerImpl -> org.chromium.base.ApplicationStatus not found
+ # org.chromium.chrome.browser.tabmodel.TabWindowManagerImpl -> org.chromium.base.ApplicationStatus$ActivityStateListener not found
+ # org.chromium.chrome.browser.tabmodel.TabWindowManagerImpl -> org.chromium.chrome.browser.tab.Tab not found
+ # pylint: enable=line-too-long
+ for line in output.splitlines():
+ parsed = line.split()
+ # E.g. java.javac.jar -> java.base
+ if len(parsed) <= 3:
+ continue
+ # E.g. java.javac.jar -> not found
+ if parsed[2] == 'not' and parsed[3] == 'found':
+ continue
+ if parsed[1] != '->':
+ continue
+ dep_from = parsed[0]
+ dep_to = parsed[2]
+ dep_graph[dep_from].add(dep_to)
+ return dep_graph
+
+
+def _EnsureDirectClasspathIsComplete(*, input_jar: str, gn_target: str,
+ output_dir: str,
+ direct_classpath_jars: List[str],
+ full_classpath_jars: List[str],
+ full_classpath_gn_targets: List[str],
+ warnings_as_errors: bool):
+ logging.info('Parsing %d direct classpath jars', len(direct_classpath_jars))
+ direct_classpath_deps = set()
+ for jar in direct_classpath_jars:
+ deps = jar_utils.extract_full_class_names_from_jar(
+ build_output_dir=pathlib.Path(output_dir), jar_path=pathlib.Path(jar))
+ direct_classpath_deps.update(deps)
+
+ logging.info('Parsing %d full classpath jars', len(full_classpath_jars))
+ full_classpath_deps = set()
+ dep_to_target = collections.defaultdict(set)
+ for jar, target in zip(full_classpath_jars, full_classpath_gn_targets):
+ deps = jar_utils.extract_full_class_names_from_jar(
+ build_output_dir=pathlib.Path(output_dir), jar_path=pathlib.Path(jar))
+ full_classpath_deps.update(deps)
+ for dep in deps:
+ dep_to_target[dep].add(target)
+
+ transitive_deps = full_classpath_deps - direct_classpath_deps
+
+ missing_targets: Dict[str, Dict[str, str]] = collections.defaultdict(dict)
+ dep_graph = _ParseDepGraph(input_jar, output_dir)
+ logging.info('Finding missing deps from %d classes', len(dep_graph))
+ # dep_graph.keys() is a list of all the classes in the current input_jar. Skip
+ # all of these to avoid checking dependencies in the same target (e.g. A
+ # depends on B, but both A and B are in input_jar).
+ seen_deps = set(dep_graph.keys())
+ for dep_from, deps_to in dep_graph.items():
+ for dep_to in deps_to - seen_deps:
+ if _ShouldIgnoreDep(dep_to):
+ continue
+ seen_deps.add(dep_to)
+ if dep_to in transitive_deps:
+ missing_target_names = sorted(dep_to_target[dep_to])
+ if len(missing_target_names) == 1:
+ missing_target_key = tuple(missing_target_names)[0]
+ else:
+ missing_target_key = f'One of {", ".join(missing_target_names)}'
+ missing_targets[missing_target_key][dep_to] = dep_from
+
+ if missing_targets:
+ print('=' * 30 + ' Dependency Checks Failed ' + '=' * 30)
+ print(f'Target: {gn_target}')
+ print('Direct classpath is incomplete. To fix, add deps on:')
+ for missing_target_key, data in missing_targets.items():
+ print(f' * {missing_target_key}')
+ for missing_class, used_by in data.items():
+ print(f' ** {missing_class} (needed by {used_by})')
+ if warnings_as_errors:
+ sys.exit(1)
+
+
def _AddSwitch(parser, val):
parser.add_argument(
val, action='store_const', default='--disabled', const=val)
@@ -34,6 +133,7 @@
parser.add_argument('--sdk-classpath-jars')
parser.add_argument('--full-classpath-jars')
parser.add_argument('--full-classpath-gn-targets')
+ parser.add_argument('--chromium-output-dir')
parser.add_argument('--stamp')
parser.add_argument('-v', '--verbose', action='store_true')
parser.add_argument('--missing-classes-allowlist')
@@ -55,37 +155,54 @@
args.direct_classpath_jars)
args.full_classpath_jars = action_helpers.parse_gn_list(
args.full_classpath_jars)
- args.full_classpath_gn_targets = action_helpers.parse_gn_list(
- args.full_classpath_gn_targets)
+ args.full_classpath_gn_targets = [
+ javac_output_processor.ReplaceGmsPackageIfNeeded(t)
+ for t in action_helpers.parse_gn_list(args.full_classpath_gn_targets)
+ ]
args.missing_classes_allowlist = action_helpers.parse_gn_list(
args.missing_classes_allowlist)
verbose = '--verbose' if args.verbose else '--not-verbose'
- cmd = [args.script, args.gn_target, args.input_jar, verbose, args.is_prebuilt]
- cmd += [str(len(args.missing_classes_allowlist))]
- cmd += args.missing_classes_allowlist
- cmd += [str(len(args.sdk_classpath_jars))]
- cmd += args.sdk_classpath_jars
- cmd += [str(len(args.direct_classpath_jars))]
- cmd += args.direct_classpath_jars
- cmd += [str(len(args.full_classpath_jars))]
- cmd += args.full_classpath_jars
- cmd += [str(len(args.full_classpath_gn_targets))]
- cmd += [
- javac_output_processor.ReplaceGmsPackageIfNeeded(t)
- for t in args.full_classpath_gn_targets
- ]
- try:
- build_utils.CheckOutput(cmd,
- print_stdout=True,
- fail_func=None,
- fail_on_output=args.warnings_as_errors)
- except build_utils.CalledProcessError as e:
- # Do not output command line because it is massive and makes the actual
- # error message hard to find.
- sys.stderr.write(e.output)
- sys.exit(1)
+
+ # TODO(https://crbug.com/1099522): Make jdeps the default.
+ if os.environ.get('BYTECODE_PROCESSOR_USE_JDEPS'):
+ logging.info('Processed args for %s, starting direct classpath check.',
+ args.target_name)
+ _EnsureDirectClasspathIsComplete(
+ input_jar=args.input_jar,
+ gn_target=args.gn_target,
+ output_dir=args.chromium_output_dir,
+ direct_classpath_jars=args.direct_classpath_jars,
+ full_classpath_jars=args.full_classpath_jars,
+ full_classpath_gn_targets=args.full_classpath_gn_targets,
+ warnings_as_errors=args.warnings_as_errors,
+ )
+ logging.info('Check completed.')
+ else:
+ cmd = [
+ args.script, args.gn_target, args.input_jar, verbose, args.is_prebuilt
+ ]
+ cmd += [str(len(args.missing_classes_allowlist))]
+ cmd += args.missing_classes_allowlist
+ cmd += [str(len(args.sdk_classpath_jars))]
+ cmd += args.sdk_classpath_jars
+ cmd += [str(len(args.direct_classpath_jars))]
+ cmd += args.direct_classpath_jars
+ cmd += [str(len(args.full_classpath_jars))]
+ cmd += args.full_classpath_jars
+ cmd += [str(len(args.full_classpath_gn_targets))]
+ cmd += args.full_classpath_gn_targets
+ try:
+ build_utils.CheckOutput(cmd,
+ print_stdout=True,
+ fail_func=None,
+ fail_on_output=args.warnings_as_errors)
+ except build_utils.CalledProcessError as e:
+ # Do not output command line because it is massive and makes the actual
+ # error message hard to find.
+ sys.stderr.write(e.output)
+ sys.exit(1)
if args.stamp:
build_utils.Touch(args.stamp)
diff --git a/build/android/gyp/util/jar_utils.py b/build/android/gyp/util/jar_utils.py
index b7a621c..eabae59 100644
--- a/build/android/gyp/util/jar_utils.py
+++ b/build/android/gyp/util/jar_utils.py
@@ -43,6 +43,12 @@
build_output_dir: pathlib.Path
src_dir: pathlib.Path = _SRC_PATH
+ def __post_init__(self):
+ # Ensure that all paths are absolute so that relative_to works correctly.
+ self.jar_path = self.jar_path.resolve()
+ self.build_output_dir = self.build_output_dir.resolve()
+ self.src_dir = self.src_dir.resolve()
+
@functools.cached_property
def cache_path(self):
"""Return a cache path for the jar that is always in the output dir.
@@ -130,18 +136,18 @@
return output
-def extract_full_class_names_from_jar(abs_build_output_dir: pathlib.Path,
- abs_jar_path: pathlib.Path) -> List[str]:
+def extract_full_class_names_from_jar(build_output_dir: pathlib.Path,
+ jar_path: pathlib.Path) -> List[str]:
"""Returns set of fully qualified class names in passed-in jar."""
- cache_file = CacheFile(jar_path=abs_jar_path,
+ cache_file = CacheFile(jar_path=jar_path,
cache_suffix='.class_name_cache',
- build_output_dir=abs_build_output_dir)
+ build_output_dir=build_output_dir)
if cache_file.is_valid():
return cache_file.read().splitlines()
out = set()
- with zipfile.ZipFile(abs_jar_path) as z:
+ with zipfile.ZipFile(jar_path) as z:
for zip_entry_name in z.namelist():
if not zip_entry_name.endswith('.class'):
continue
diff --git a/build/config/android/internal_rules.gni b/build/config/android/internal_rules.gni
index 28fa9fa..d3427ebd 100644
--- a/build/config/android/internal_rules.gni
+++ b/build/config/android/internal_rules.gni
@@ -1899,6 +1899,8 @@
rebase_path(invoker.input_jar, root_build_dir),
"--stamp",
rebase_path(outputs[0], root_build_dir),
+ "--chromium-output-dir",
+ rebase_path(root_build_dir, root_build_dir),
"--direct-classpath-jars=@FileArg($_rebased_build_config:javac:classpath)",
"--full-classpath-jars=@FileArg($_rebased_build_config:deps_info:javac_full_classpath)",
"--full-classpath-gn-targets=@FileArg($_rebased_build_config:deps_info:javac_full_classpath_targets)",
