diff --git a/build/android/docs/coverage.md b/build/android/docs/coverage.md index 6ea548aed7888..35cd80988a131 100644 --- a/build/android/docs/coverage.md +++ b/build/android/docs/coverage.md @@ -17,7 +17,7 @@ class files and runtime **.exec** files. Then we need to process them using the ```gn target_os = "android" - jacoco_coverage = true + use_jacoco_coverage = true ``` Now when building, pre-instrumented files will be created in the build directory. diff --git a/build/android/gyp/jacoco_instr.py b/build/android/gyp/jacoco_instr.py index 4deea4395597e..2f927d1116fa4 100755 --- a/build/android/gyp/jacoco_instr.py +++ b/build/android/gyp/jacoco_instr.py @@ -6,7 +6,7 @@ """Instruments classes and jar files. -This script corresponds to the 'jacoco_instr' action in the java build process. +This script corresponds to the 'jacoco_instr' action in the Java build process. Depending on whether jacoco_instrument is set, the 'jacoco_instr' action will call the instrument command which accepts a jar and instruments it using jacococli.jar. @@ -21,6 +21,7 @@ import os import shutil import sys import tempfile +import zipfile from util import build_utils @@ -53,6 +54,9 @@ def _AddArguments(parser): help='File containing newline-separated .java paths') parser.add_argument( '--jacococli-jar', required=True, help='Path to jacococli.jar.') + parser.add_argument( + '--files-to-instrument', + help='Path to a file containing which source files are affected.') def _GetSourceDirsFromSourceFiles(source_files): @@ -101,8 +105,100 @@ def _CreateSourcesJsonFile(source_dirs, input_path, sources_json_file, json.dump(data, f) +def _GetAffectedClasses(jar_file, source_files): + """Gets affected classes by affected source files to a jar. + + Args: + jar_file: The jar file to get all members. + source_files: The list of affected source files. + + Returns: + A tuple of affected classes and unaffected members. + """ + with zipfile.ZipFile(jar_file) as f: + members = f.namelist() + + affected_classes = [] + unaffected_members = [] + + for member in members: + if not member.endswith('.class'): + unaffected_members.append(member) + continue + + is_affected = False + index = member.find('$') + if index == -1: + index = member.find('.class') + for source_file in source_files: + if source_file.endswith(member[:index] + '.java'): + affected_classes.append(member) + is_affected = True + break + if not is_affected: + unaffected_members.append(member) + + return affected_classes, unaffected_members + + +def _InstrumentWholeJar(instrument_cmd, input_path, output_path, temp_dir): + """Instruments input jar to output_path. + + Args: + instrument_cmd: JaCoCo instrument command. + input_path: The input path to non-instrumented jar. + output_path: The output path to instrumented jar. + temp_dir: The temporary directory. + """ + instrument_cmd.extend([input_path, '--dest', temp_dir]) + + build_utils.CheckOutput(instrument_cmd) + + jars = os.listdir(temp_dir) + if len(jars) != 1: + raise Exception('Error: multiple output files: %s' % jars) + + # Delete output_path first to avoid modifying input_path in the case where + # input_path is a hardlink to output_path. http://crbug.com/571642 + if os.path.exists(output_path): + os.unlink(output_path) + shutil.move(os.path.join(temp_dir, jars[0]), output_path) + + +def _InstrumentClassFiles(instrument_cmd, input_path, output_path, temp_dir, + affected_source_files): + """Instruments affected class files from input jar. + + Args: + instrument_cmd: JaCoCo instrument command. + input_path: The input path to non-instrumented jar. + output_path: The output path to instrumented jar. + temp_dir: The temporary directory. + affected_source_files: The affected source file paths to input jar. + """ + affected_classes, unaffected_members = _GetAffectedClasses( + input_path, affected_source_files) + + # Extract affected class files. + with zipfile.ZipFile(input_path) as f: + f.extractall(temp_dir, affected_classes) + + instrumented_dir = os.path.join(temp_dir, 'instrumented') + + # Instrument extracted class files. + instrument_cmd.extend([temp_dir, '--dest', instrumented_dir]) + build_utils.CheckOutput(instrument_cmd) + + # Extract unaffected members to instrumented_dir. + with zipfile.ZipFile(input_path) as f: + f.extractall(instrumented_dir, unaffected_members) + + # Zip all files to output_path + build_utils.ZipDir(output_path, instrumented_dir) + + def _RunInstrumentCommand(parser): - """Instruments jar files using Jacoco. + """Instruments class or Jar files using JaCoCo. Args: parser: ArgumentParser object. @@ -112,33 +208,30 @@ def _RunInstrumentCommand(parser): """ args = parser.parse_args() - temp_dir = tempfile.mkdtemp() - try: - cmd = [ - 'java', '-jar', args.jacococli_jar, 'instrument', args.input_path, - '--dest', temp_dir - ] - - build_utils.CheckOutput(cmd) - - jars = os.listdir(temp_dir) - if len(jars) != 1: - print('Error: multiple output files in: %s' % (temp_dir)) - return 1 - - # Delete output_path first to avoid modifying input_path in the case where - # input_path is a hardlink to output_path. http://crbug.com/571642 - if os.path.exists(args.output_path): - os.unlink(args.output_path) - shutil.move(os.path.join(temp_dir, jars[0]), args.output_path) - finally: - shutil.rmtree(temp_dir) - source_files = [] if args.java_sources_file: source_files.extend(build_utils.ReadSourcesList(args.java_sources_file)) - source_dirs = _GetSourceDirsFromSourceFiles(source_files) + with build_utils.TempDir() as temp_dir: + instrument_cmd = ['java', '-jar', args.jacococli_jar, 'instrument'] + + if not args.files_to_instrument: + _InstrumentWholeJar(instrument_cmd, args.input_path, args.output_path, + temp_dir) + else: + affected_files = build_utils.ReadSourcesList(args.files_to_instrument) + source_set = set(source_files) + affected_source_files = [f for f in affected_files if f in source_set] + + # Copy input_path to output_path and return if no source file affected. + if not affected_source_files: + shutil.copyfile(args.input_path, args.output_path) + return 0 + else: + _InstrumentClassFiles(instrument_cmd, args.input_path, args.output_path, + temp_dir, affected_source_files) + + source_dirs = _GetSourceDirsFromSourceFiles(source_files) # TODO(GYP): In GN, we are passed the list of sources, detecting source # directories, then walking them to re-establish the list of sources. # This can obviously be simplified! diff --git a/build/config/android/config.gni b/build/config/android/config.gni index c1bfc87466935..6a606d6205f5b 100644 --- a/build/config/android/config.gni +++ b/build/config/android/config.gni @@ -182,10 +182,6 @@ if (is_android || is_chromeos) { # Set to false to disable the Errorprone compiler use_errorprone_java_compiler = true - # Enables Jacoco Java code coverage. Instruments classes during build to - # produce .exec files during runtime - jacoco_coverage = false - # Disables process isolation when building _incremental targets. # Required for Android M+ due to SELinux policies (stronger sandboxing). disable_incremental_isolated_processes = false diff --git a/build/config/android/internal_rules.gni b/build/config/android/internal_rules.gni index d294f207b7256..64399ca4fadfd 100644 --- a/build/config/android/internal_rules.gni +++ b/build/config/android/internal_rules.gni @@ -6,6 +6,7 @@ # Some projects (e.g. V8) do not have non-build directories DEPS'ed in. import("//build/config/android/config.gni") import("//build/config/android/copy_ex.gni") +import("//build/config/coverage/coverage.gni") import("//build/config/dcheck_always_on.gni") import("//build/config/python.gni") import("//build/config/sanitizers/sanitizers.gni") @@ -697,7 +698,7 @@ template("test_runner_script") { if (defined(invoker.proguard_enabled) && invoker.proguard_enabled) { executable_args += [ "--enable-java-deobfuscation" ] } - if (jacoco_coverage) { + if (use_jacoco_coverage) { # Set a default coverage output directory (can be overridden by user # passing the same flag). _rebased_coverage_dir = @@ -729,7 +730,7 @@ template("test_runner_script") { "--robolectric-runtime-deps-dir", "@WrappedPath(${_rebased_robolectric_runtime_deps_dir})", ] - if (jacoco_coverage) { + if (use_jacoco_coverage) { # Set a default coverage output directory (can be overridden by user # passing the same flag). _rebased_coverage_dir = @@ -1092,7 +1093,7 @@ if (enable_java_templates) { } args += [ "--classpath=@FileArg($_rebased_build_config:deps_info:java_runtime_classpath)" ] - if (jacoco_coverage) { + if (use_jacoco_coverage) { args += [ "--classpath", rebase_path("//third_party/jacoco/lib/jacocoagent.jar", @@ -1434,6 +1435,8 @@ if (enable_java_templates) { rebase_path(invoker.java_sources_file, root_build_dir), "--jacococli-jar", rebase_path(_jacococli_jar, root_build_dir), + "--files-to-instrument", + rebase_path(coverage_instrumentation_input_file, root_build_dir), ] } } @@ -3084,7 +3087,7 @@ if (enable_java_templates) { # Chromium-specific unless it is in a 'chromium' sub-directory). # jacoco_never_instrument: Optional. If provided, whether to forbid # instrumentation with the Jacoco coverage processor. If not provided, - # this is controlled by the global jacoco_coverage build arg variable + # this is controlled by the global use_jacoco_coverage build arg variable # and only used for non-test Chromium code. # include_android_sdk: Optional. Whether or not the android SDK dep # should be added to deps. Defaults to true for non-system libraries @@ -3304,7 +3307,7 @@ if (enable_java_templates) { if (defined(_final_jar_path)) { _jacoco_instrument = - jacoco_coverage && _chromium_code && _java_files != [] && + use_jacoco_coverage && _chromium_code && _java_files != [] && !_is_java_binary && !_is_annotation_processor && (!defined(invoker.testonly) || !invoker.testonly) if (defined(invoker.jacoco_never_instrument)) { diff --git a/build/config/coverage/coverage.gni b/build/config/coverage/coverage.gni index fe3af770f11eb..9586d8d980b39 100644 --- a/build/config/coverage/coverage.gni +++ b/build/config/coverage/coverage.gni @@ -5,15 +5,20 @@ import("//build/toolchain/toolchain.gni") # There are two ways to enable code coverage instrumentation: -# 1. When |use_clang_coverage| is true and |coverage_instrumentation_input_file| -# is empty, all source files are instrumented. -# 2. When |use_clang_coverage| is true and |coverage_instrumentation_input_file| -# is NOT empty and points to a text file on the file system, ONLY source -# files specified in the input file are instrumented. +# 1. When |use_clang_coverage| or |use_jacoco_coverage| is true and +# |coverage_instrumentation_input_file| is empty, all source files or +# Java class files are instrumented. +# 2. When |use_clang_coverage| or |use_jacoco_coverage| is true and +# |coverage_instrumentation_input_file| is NOT empty and points to +# a text file on the file system, ONLY source files specified in the +# input file or Java class files related to source files are instrumented. declare_args() { # Enable Clang's Source-based Code Coverage. use_clang_coverage = false + # Enables JaCoCo Java code coverage. + use_jacoco_coverage = false + # The path to the coverage instrumentation input file should be a source root # absolute path (e.g. //out/Release/coverage_instrumentation_input.txt), and # the file consists of multiple lines where each line represents a path to a diff --git a/tools/mb/mb.py b/tools/mb/mb.py index e612679ff4d24..6d2784a0c58ac 100755 --- a/tools/mb/mb.py +++ b/tools/mb/mb.py @@ -399,34 +399,32 @@ class MetaBuildWrapper(object): return self._RunLocallyIsolated(self.args.path, self.args.target) def CmdZip(self): - ret = self.CmdIsolate() - if ret: - return ret + ret = self.CmdIsolate() + if ret: + return ret - zip_dir = None - try: - zip_dir = self.TempDir() - remap_cmd = [ - self.executable, - self.PathJoin(self.chromium_src_dir, 'tools', 'swarming_client', - 'isolate.py'), - 'remap', - '--collapse_symlinks', - '-s', self.PathJoin(self.args.path, self.args.target + '.isolated'), - '-o', zip_dir - ] - self.Run(remap_cmd) + zip_dir = None + try: + zip_dir = self.TempDir() + remap_cmd = [ + self.executable, + self.PathJoin(self.chromium_src_dir, 'tools', 'swarming_client', + 'isolate.py'), 'remap', '--collapse_symlinks', '-s', + self.PathJoin(self.args.path, self.args.target + '.isolated'), '-o', + zip_dir + ] + self.Run(remap_cmd) - zip_path = self.args.zip_path - with zipfile.ZipFile(zip_path, 'w', zipfile.ZIP_DEFLATED, - allowZip64=True) as fp: - for root, _, files in os.walk(zip_dir): - for filename in files: - path = self.PathJoin(root, filename) - fp.write(path, self.RelPath(path, zip_dir)) - finally: - if zip_dir: - self.RemoveDirectory(zip_dir) + zip_path = self.args.zip_path + with zipfile.ZipFile( + zip_path, 'w', zipfile.ZIP_DEFLATED, allowZip64=True) as fp: + for root, _, files in os.walk(zip_dir): + for filename in files: + path = self.PathJoin(root, filename) + fp.write(path, self.RelPath(path, zip_dir)) + finally: + if zip_dir: + self.RemoveDirectory(zip_dir) @staticmethod def _AddBaseSoftware(cmd): @@ -806,8 +804,8 @@ class MetaBuildWrapper(object): vals['cros_passthrough'] = mixin_vals['cros_passthrough'] if 'args_file' in mixin_vals: if vals['args_file']: - raise MBErr('args_file specified multiple times in mixins ' - 'for %s on %s' % (self.args.builder, self.args.master)) + raise MBErr('args_file specified multiple times in mixins ' + 'for %s on %s' % (self.args.builder, self.args.master)) vals['args_file'] = mixin_vals['args_file'] if 'gn_args' in mixin_vals: if vals['gn_args']: @@ -888,10 +886,10 @@ class MetaBuildWrapper(object): ret, output, _ = self.Run(self.GNCmd('ls', build_dir), force_verbose=False) if ret: - # If `gn ls` failed, we should exit early rather than trying to - # generate isolates. - self.Print('GN ls failed: %d' % ret) - return ret + # If `gn ls` failed, we should exit early rather than trying to + # generate isolates. + self.Print('GN ls failed: %d' % ret) + return ret # Create a reverse map from isolate label to isolate dict. isolate_map = self.ReadIsolateMap() @@ -1307,7 +1305,7 @@ class MetaBuildWrapper(object): msan = 'is_msan=true' in vals['gn_args'] tsan = 'is_tsan=true' in vals['gn_args'] cfi_diag = 'use_cfi_diag=true' in vals['gn_args'] - java_coverage = 'jacoco_coverage=true' in vals['gn_args'] + java_coverage = 'use_jacoco_coverage=true' in vals['gn_args'] test_type = isolate_map[target]['type'] use_python3 = isolate_map[target].get('use_python3', False) diff --git a/tools/mb/mb_config.pyl b/tools/mb/mb_config.pyl index 8a6040f31a5eb..c6d27e7bf757e 100644 --- a/tools/mb/mb_config.pyl +++ b/tools/mb/mb_config.pyl @@ -2260,7 +2260,7 @@ }, 'java_coverage': { - 'gn_args': 'jacoco_coverage=true', + 'gn_args': 'use_jacoco_coverage=true', }, 'jumbo': { @@ -2350,6 +2350,8 @@ # https://cs.chromium.org/chromium/build/scripts/slave/recipe_modules/code_coverage/api.py # and # https://cs.chromium.org/chromium/src/docs/clang_code_coverage_wrapper.md + # For Java, see: + # https://cs.chromium.org/chromium/src/build/android/gyp/jacoco_instr.py 'partial_code_coverage_instrumentation': { 'gn_args': 'coverage_instrumentation_input_file="//.code-coverage/files_to_instrument.txt"' },