From c2b563ca5489aaf7579a4b9f389533b7af336187 Mon Sep 17 00:00:00 2001 From: jbudorick Date: Mon, 21 Mar 2016 09:43:40 -0700 Subject: [Android] Run lint using a cache in the output directory. BUG=583661 Review URL: https://codereview.chromium.org/1815563005 Cr-Commit-Position: refs/heads/master@{#382306} --- build/android/BUILD.gn | 36 +++++ build/android/android_lint_cache.gyp | 47 +++++++ build/android/gyp/lint.py | 228 +++++++++++++++++++------------- build/android/gyp/util/build_utils.py | 4 +- build/android/lint_action.gypi | 10 +- build/common.gypi | 2 + build/config/android/internal_rules.gni | 3 + build/java.gypi | 3 +- build/java_apk.gypi | 1 + 9 files changed, 237 insertions(+), 97 deletions(-) create mode 100644 build/android/android_lint_cache.gyp (limited to 'build') diff --git a/build/android/BUILD.gn b/build/android/BUILD.gn index 5428875..01deb14 100644 --- a/build/android/BUILD.gn +++ b/build/android/BUILD.gn @@ -7,6 +7,42 @@ import("//third_party/ijar/ijar.gni") sun_tools_jar_path = "$root_gen_dir/sun_tools_jar/tools.jar" +# Create or update the API versions cache if necessary by running a +# functionally empty lint task. This prevents racy creation of the +# cache while linting java targets in android_lint. +action("prepare_android_lint_cache") { + _cache_dir = "${root_out_dir}/android_lint_cache" + depfile = "${_cache_dir}/prepare_android_lint_cache.d" + _manifest_file = "//build/android/AndroidManifest.xml" + _result_file = "${_cache_dir}/result.xml" + script = "//build/android/gyp/lint.py" + + inputs = [ + "${android_sdk_root}/platform-tools/api/api-versions.xml", + ] + outputs = [ + depfile, + ] + args = [ + "--build-tools-version", + android_sdk_build_tools_version, + "--cache-dir", + rebase_path(_cache_dir, root_build_dir), + "--depfile", + rebase_path(depfile, root_build_dir), + "--lint-path", + "$rebased_android_sdk_root/tools/lint", + "--manifest-path", + rebase_path(_manifest_file, root_build_dir), + "--product-dir", + ".", + "--result-path", + rebase_path(_result_file, root_build_dir), + "--silent", + "--enable", + ] +} + action("find_sun_tools_jar") { script = "//build/android/gyp/find_sun_tools_jar.py" depfile = "$target_gen_dir/$target_name.d" diff --git a/build/android/android_lint_cache.gyp b/build/android/android_lint_cache.gyp new file mode 100644 index 0000000..a8b514e --- /dev/null +++ b/build/android/android_lint_cache.gyp @@ -0,0 +1,47 @@ +# Copyright 2016 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +{ + 'targets': [ + { + # This target runs a functionally empty lint to create or update the + # API versions cache if necessary. This prevents racy creation of the + # cache while linting java targets in lint_action.gypi. + 'target_name': 'android_lint_cache', + 'type': 'none', + 'actions': [ + { + 'action_name': 'prepare_android_lint_cache', + 'message': 'Preparing Android lint cache', + 'variables': { + 'android_lint_cache_stamp': '<(PRODUCT_DIR)/android_lint_cache/android_lint_cache.stamp', + 'android_manifest_path': '<(DEPTH)/build/android/AndroidManifest.xml', + 'cache_file': '<(PRODUCT_DIR)/android_lint_cache/.android/cache/api-versions-6-23.0.1.bin', + 'result_path': '<(PRODUCT_DIR)/android_lint_cache/result.xml', + }, + 'inputs': [ + '<(DEPTH)/build/android/gyp/lint.py', + '<(android_manifest_path)', + ], + 'outputs': [ + '<(cache_file)', + '<(result_path)', + ], + 'action': [ + 'python', '<(DEPTH)/build/android/gyp/lint.py', + '--lint-path', '<(android_sdk_root)/tools/lint', + '--cache-dir', '<(PRODUCT_DIR)/android_lint_cache', + '--build-tools-version', '<(android_sdk_build_tools_version)', + '--manifest-path', '<(android_manifest_path)', + '--product-dir', '<(PRODUCT_DIR)', + '--result-path', '<(result_path)', + '--stamp', '<(android_lint_cache_stamp)', + '--silent', + '--enable' + ], + }, + ], + }, + ], +} diff --git a/build/android/gyp/lint.py b/build/android/gyp/lint.py index bc761ff..2d3c79b 100755 --- a/build/android/gyp/lint.py +++ b/build/android/gyp/lint.py @@ -7,7 +7,7 @@ """Runs Android's lint tool.""" -import optparse +import argparse import os import sys import traceback @@ -22,7 +22,8 @@ _SRC_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), def _OnStaleMd5(changes, lint_path, config_path, processed_config_path, manifest_path, result_path, product_dir, sources, jar_path, - resource_dir=None, can_fail_build=False): + cache_dir, resource_dir=None, can_fail_build=False, + silent=False): def _RelativizePath(path): """Returns relative path to top-level src dir. @@ -33,6 +34,8 @@ def _OnStaleMd5(changes, lint_path, config_path, processed_config_path, return os.path.relpath(os.path.abspath(path), _SRC_ROOT) def _ProcessConfigFile(): + if not config_path or not processed_config_path: + return if not build_utils.IsTimeStale(processed_config_path, [config_path]): return @@ -54,23 +57,24 @@ def _OnStaleMd5(changes, lint_path, config_path, processed_config_path, def _ParseAndShowResultFile(): dom = minidom.parse(result_path) issues = dom.getElementsByTagName('issue') - print >> sys.stderr - for issue in issues: - issue_id = issue.attributes['id'].value - message = issue.attributes['message'].value - location_elem = issue.getElementsByTagName('location')[0] - path = location_elem.attributes['file'].value - line = location_elem.getAttribute('line') - if line: - error = '%s:%s %s: %s [warning]' % (path, line, message, issue_id) - else: - # Issues in class files don't have a line number. - error = '%s %s: %s [warning]' % (path, message, issue_id) - print >> sys.stderr, error.encode('utf-8') - for attr in ['errorLine1', 'errorLine2']: - error_line = issue.getAttribute(attr) - if error_line: - print >> sys.stderr, error_line.encode('utf-8') + if not silent: + print >> sys.stderr + for issue in issues: + issue_id = issue.attributes['id'].value + message = issue.attributes['message'].value + location_elem = issue.getElementsByTagName('location')[0] + path = location_elem.attributes['file'].value + line = location_elem.getAttribute('line') + if line: + error = '%s:%s %s: %s [warning]' % (path, line, message, issue_id) + else: + # Issues in class files don't have a line number. + error = '%s %s: %s [warning]' % (path, message, issue_id) + print >> sys.stderr, error.encode('utf-8') + for attr in ['errorLine1', 'errorLine2']: + error_line = issue.getAttribute(attr) + if error_line: + print >> sys.stderr, error_line.encode('utf-8') return len(issues) # Need to include all sources when a resource_dir is set so that resources are @@ -84,10 +88,12 @@ def _OnStaleMd5(changes, lint_path, config_path, processed_config_path, cmd = [ _RelativizePath(lint_path), '-Werror', '--exitcode', '--showall', - '--config', _RelativizePath(processed_config_path), - '--classpath', _RelativizePath(jar_path), '--xml', _RelativizePath(result_path), ] + if jar_path: + cmd.extend(['--classpath', _RelativizePath(jar_path)]) + if processed_config_path: + cmd.extend(['--config', _RelativizePath(processed_config_path)]) if resource_dir: cmd.extend(['--resources', _RelativizePath(resource_dir)]) @@ -117,13 +123,18 @@ def _OnStaleMd5(changes, lint_path, config_path, processed_config_path, src_dir = NewSourceDir() os.symlink(os.path.abspath(src), PathInDir(src_dir, src)) - cmd.append(_RelativizePath(os.path.join(manifest_path, os.pardir))) + if manifest_path: + cmd.append(_RelativizePath(os.path.join(manifest_path, os.pardir))) if os.path.exists(result_path): os.remove(result_path) + env = {} + if cache_dir: + env['_JAVA_OPTIONS'] = '-Duser.home=%s' % _RelativizePath(cache_dir) + try: - build_utils.CheckOutput(cmd, cwd=_SRC_ROOT) + build_utils.CheckOutput(cmd, cwd=_SRC_ROOT, env=env or None) except build_utils.CalledProcessError: if can_fail_build: traceback.print_exc() @@ -137,92 +148,129 @@ def _OnStaleMd5(changes, lint_path, config_path, processed_config_path, try: num_issues = _ParseAndShowResultFile() except Exception: # pylint: disable=broad-except - print 'Lint created unparseable xml file...' - print 'File contents:' - with open(result_path) as f: - print f.read() + if not silent: + print 'Lint created unparseable xml file...' + print 'File contents:' + with open(result_path) as f: + print f.read() raise _ProcessResultFile() msg = ('\nLint found %d new issues.\n' - ' - For full explanation refer to %s\n' - ' - Wanna suppress these issues?\n' - ' 1. Read comment in %s\n' - ' 2. Run "python %s %s"\n' % + ' - For full explanation refer to %s\n' % (num_issues, - _RelativizePath(result_path), - _RelativizePath(config_path), - _RelativizePath(os.path.join(_SRC_ROOT, 'build', 'android', - 'lint', 'suppress.py')), _RelativizePath(result_path))) - print >> sys.stderr, msg + if config_path: + msg += (' - Wanna suppress these issues?\n' + ' 1. Read comment in %s\n' + ' 2. Run "python %s %s"\n' % + (_RelativizePath(config_path), + _RelativizePath(os.path.join(_SRC_ROOT, 'build', 'android', + 'lint', 'suppress.py')), + _RelativizePath(result_path))) + if not silent: + print >> sys.stderr, msg if can_fail_build: raise Exception('Lint failed.') def main(): - parser = optparse.OptionParser() + parser = argparse.ArgumentParser() build_utils.AddDepfileOption(parser) - parser.add_option('--lint-path', help='Path to lint executable.') - parser.add_option('--config-path', help='Path to lint suppressions file.') - parser.add_option('--processed-config-path', - help='Path to processed lint suppressions file.') - parser.add_option('--manifest-path', help='Path to AndroidManifest.xml') - parser.add_option('--result-path', help='Path to XML lint result file.') - parser.add_option('--product-dir', help='Path to product dir.') - parser.add_option('--src-dirs', help='Directories containing java files.') - parser.add_option('--java-files', help='Paths to java files.') - parser.add_option('--jar-path', help='Jar file containing class files.') - parser.add_option('--resource-dir', help='Path to resource dir.') - parser.add_option('--can-fail-build', action='store_true', - help='If set, script will exit with nonzero exit status' - ' if lint errors are present') - parser.add_option('--stamp', help='Path to touch on success.') - parser.add_option('--enable', action='store_true', - help='Run lint instead of just touching stamp.') - - options, _ = parser.parse_args() - - build_utils.CheckOptions( - options, parser, required=['lint_path', 'config_path', - 'processed_config_path', 'manifest_path', - 'result_path', 'product_dir', - 'jar_path']) - - if options.enable: + + parser.add_argument('--lint-path', required=True, + help='Path to lint executable.') + parser.add_argument('--product-dir', required=True, + help='Path to product dir.') + parser.add_argument('--result-path', required=True, + help='Path to XML lint result file.') + + parser.add_argument('--build-tools-version', + help='Version of the build tools in the Android SDK.') + parser.add_argument('--cache-dir', + help='Path to the directory in which the android cache ' + 'directory tree should be stored.') + parser.add_argument('--can-fail-build', action='store_true', + help='If set, script will exit with nonzero exit status' + ' if lint errors are present') + parser.add_argument('--config-path', + help='Path to lint suppressions file.') + parser.add_argument('--enable', action='store_true', + help='Run lint instead of just touching stamp.') + parser.add_argument('--jar-path', + help='Jar file containing class files.') + parser.add_argument('--java-files', + help='Paths to java files.') + parser.add_argument('--manifest-path', + help='Path to AndroidManifest.xml') + parser.add_argument('--platform-xml-path', + help='Path to api-platforms.xml') + parser.add_argument('--processed-config-path', + help='Path to processed lint suppressions file.') + parser.add_argument('--resource-dir', + help='Path to resource dir.') + parser.add_argument('--silent', action='store_true', + help='If set, script will not log anything.') + parser.add_argument('--src-dirs', + help='Directories containing java files.') + parser.add_argument('--stamp', + help='Path to touch on success.') + + args = parser.parse_args() + + if args.enable: sources = [] - if options.src_dirs: - src_dirs = build_utils.ParseGypList(options.src_dirs) + if args.src_dirs: + src_dirs = build_utils.ParseGypList(args.src_dirs) sources = build_utils.FindInDirectories(src_dirs, '*.java') - elif options.java_files: - sources = build_utils.ParseGypList(options.java_files) - else: - print 'One of --src-dirs or --java-files must be specified.' - return 1 + elif args.java_files: + sources = build_utils.ParseGypList(args.java_files) + + if args.config_path and not args.processed_config_path: + parser.error('--config-path specified without --processed-config-path') + elif args.processed_config_path and not args.config_path: + parser.error('--processed-config-path specified without --config-path') input_paths = [ - options.lint_path, - options.config_path, - options.manifest_path, - options.jar_path, + args.lint_path, ] - input_paths.extend(sources) - if options.resource_dir: - input_paths.extend(build_utils.FindInDirectory(options.resource_dir, '*')) - - input_strings = [ options.processed_config_path ] - output_paths = [ options.result_path ] + if args.config_path: + input_paths.append(args.config_path) + if args.jar_path: + input_paths.append(args.jar_path) + if args.manifest_path: + input_paths.append(args.manifest_path) + if args.platform_xml_path: + input_paths.append(args.platform_xml_path) + if args.resource_dir: + input_paths.extend(build_utils.FindInDirectory(args.resource_dir, '*')) + if sources: + input_paths.extend(sources) + + input_strings = [] + if args.processed_config_path: + input_strings.append(args.processed_config_path) + + output_paths = [ args.result_path ] + if args.cache_dir: + if not args.build_tools_version: + parser.error('--cache-dir specified without --build-tools-version') + output_paths.append(os.path.join( + args.cache_dir, '.android', 'cache', + 'api-versions-6-%s.bin' % args.build_tools_version)) build_utils.CallAndWriteDepfileIfStale( - lambda changes: _OnStaleMd5(changes, options.lint_path, - options.config_path, - options.processed_config_path, - options.manifest_path, options.result_path, - options.product_dir, sources, - options.jar_path, - resource_dir=options.resource_dir, - can_fail_build=options.can_fail_build), - options, + lambda changes: _OnStaleMd5(changes, args.lint_path, + args.config_path, + args.processed_config_path, + args.manifest_path, args.result_path, + args.product_dir, sources, + args.jar_path, + args.cache_dir, + resource_dir=args.resource_dir, + can_fail_build=args.can_fail_build, + silent=args.silent), + args, input_paths=input_paths, input_strings=input_strings, output_paths=output_paths, diff --git a/build/android/gyp/util/build_utils.py b/build/android/gyp/util/build_utils.py index 3fa91aa..5e088b7 100644 --- a/build/android/gyp/util/build_utils.py +++ b/build/android/gyp/util/build_utils.py @@ -152,7 +152,7 @@ class CalledProcessError(Exception): # This can be used in most cases like subprocess.check_output(). The output, # particularly when the command fails, better highlights the command's failure. # If the command fails, raises a build_utils.CalledProcessError. -def CheckOutput(args, cwd=None, +def CheckOutput(args, cwd=None, env=None, print_stdout=False, print_stderr=True, stdout_filter=None, stderr_filter=None, @@ -161,7 +161,7 @@ def CheckOutput(args, cwd=None, cwd = os.getcwd() child = subprocess.Popen(args, - stdout=subprocess.PIPE, stderr=subprocess.PIPE, cwd=cwd) + stdout=subprocess.PIPE, stderr=subprocess.PIPE, cwd=cwd, env=env) stdout, stderr = child.communicate() if stdout_filter is not None: diff --git a/build/android/lint_action.gypi b/build/android/lint_action.gypi index 5369f64..35b7d49 100644 --- a/build/android/lint_action.gypi +++ b/build/android/lint_action.gypi @@ -11,9 +11,9 @@ 'variables': { 'conditions': [ ['chromium_code != 0 and android_lint != 0 and never_lint == 0', { - 'is_enabled': '--enable', + 'additional_args': ['--enable'], }, { - 'is_enabled': '', + 'additional_args': [], }] ], 'android_manifest_path%': '<(DEPTH)/build/android/AndroidManifest.xml', @@ -24,14 +24,16 @@ '<(DEPTH)/build/android/gyp/util/build_utils.py', '<(DEPTH)/build/android/gyp/lint.py', '<(android_manifest_path)', - '<(suppressions_file)', '<(lint_jar_path)', + '<(suppressions_file)', ], 'action': [ 'python', '<(DEPTH)/build/android/gyp/lint.py', '--lint-path=<(android_sdk_root)/tools/lint', '--config-path=<(suppressions_file)', '--processed-config-path=<(config_path)', + '--cache-dir', '<(PRODUCT_DIR)/android_lint_cache', + '--build-tools-version', '<(android_sdk_build_tools_version)', '--manifest-path=<(android_manifest_path)', '--result-path=<(result_path)', '--resource-dir=<(resource_dir)', @@ -40,6 +42,6 @@ '--jar-path=<(lint_jar_path)', '--can-fail-build', '--stamp=<(stamp_path)', - '<(is_enabled)', + '<@(additional_args)', ], } diff --git a/build/common.gypi b/build/common.gypi index 0112b1a..363ac91 100644 --- a/build/common.gypi +++ b/build/common.gypi @@ -1769,6 +1769,7 @@ 'android_ndk_absolute_root%': '<(android_ndk_absolute_root)', 'android_sdk_root%': '<(android_sdk_root)', 'android_sdk_version%': '<(android_sdk_version)', + 'android_sdk_build_tools_version%': '<(android_sdk_build_tools_version)', 'android_libcpp_root': '<(android_ndk_root)/sources/cxx-stl/llvm-libc++', 'android_libcpp_library': '<(android_libcpp_library)', 'android_must_copy_system_libraries': '<(android_must_copy_system_libraries)', @@ -1843,6 +1844,7 @@ 'android_ndk_include': '<(android_ndk_sysroot)/usr/include', 'android_ndk_lib': '<(android_ndk_sysroot)/<(android_ndk_lib_dir)', + 'android_sdk_build_tools_version%': '<(android_sdk_build_tools_version)', 'android_sdk_tools%': '<(android_sdk_tools)', 'android_aapt_path%': '<(android_sdk_tools)/aapt', 'android_sdk%': '<(android_sdk)', diff --git a/build/config/android/internal_rules.gni b/build/config/android/internal_rules.gni index 732c98b..79805d1 100644 --- a/build/config/android/internal_rules.gni +++ b/build/config/android/internal_rules.gni @@ -32,6 +32,7 @@ template("android_lint") { base_path = "$target_gen_dir/$target_name" action(target_name) { + deps = [] forward_variables_from(invoker, [ "deps", @@ -54,6 +55,8 @@ template("android_lint") { result_path, ] + deps += [ "//build/android:prepare_android_lint_cache" ] + rebased_java_files = rebase_path(java_files, root_build_dir) args = [ diff --git a/build/java.gypi b/build/java.gypi index b2996f3..171d5e0 100644 --- a/build/java.gypi +++ b/build/java.gypi @@ -48,7 +48,8 @@ { 'dependencies': [ - '<(DEPTH)/build/android/setup.gyp:build_output_dirs' + '<(DEPTH)/build/android/android_lint_cache.gyp:android_lint_cache', + '<(DEPTH)/build/android/setup.gyp:build_output_dirs', ], 'variables': { 'add_to_dependents_classpaths%': 1, diff --git a/build/java_apk.gypi b/build/java_apk.gypi index 23f5324..f1c5bbf 100644 --- a/build/java_apk.gypi +++ b/build/java_apk.gypi @@ -856,6 +856,7 @@ }], ], 'dependencies': [ + '<(DEPTH)/build/android/android_lint_cache.gyp:android_lint_cache', '<(DEPTH)/tools/android/md5sum/md5sum.gyp:md5sum', ], 'actions': [ -- cgit v1.1