[Android] Add lint as a gyp action.

build/android/gyp/lint.py

+#!/usr/bin/env python
+#
+# Copyright (c) 2013 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.
+
+"""Runs Android's lint tool."""
+
+
+import optparse
+import os
+import pipes
+import subprocess
+import sys
+import time
+from xml.dom import minidom
+
+from util import build_utils
+
+
+def _RunLint(lint_path, config_path, processed_config_path, manifest_path,
+             result_path, product_dir, src_root, src_dirs, classes_dir):
+
+  def _RelativizePath(path):
+    """Returns relative path to top-level src dir.
+
+    Args:
+      path: A path relative to cwd.
+    """
+    return os.path.relpath(os.path.abspath(path), os.path.abspath(src_root))
+
+  def _ProcessConfigFile():
+    if not build_utils.IsTimeStale(processed_config_path, [config_path]):
+      return
+
+    with open(config_path, 'rb') as f:
+      content = f.read().replace(
+          'PRODUCT_DIR', _RelativizePath(product_dir))
+
+    with open(processed_config_path, 'wb') as f:
+      f.write(content)
+
+  def _ProcessResultFile():
+    with open(result_path, 'rb') as f:
+      content = f.read().replace(
+          _RelativizePath(product_dir), 'PRODUCT_DIR')
+
+    with open(result_path, 'wb') as f:
+      f.write(content)
+
+  def _ParseAndShowResultFile():
+    dom = minidom.parse(result_path)
+    num_issues = 0

[newt (away), 2013/12/05 23:39:33]

just an idea: instead of counting num_issues manually:

    issues = dom.getElementsByTagName('issue')
    for issue in issues:
      ...
    return len(issues)

[frankf, 2013/12/06 22:17:36]

Done.

+    for issue in dom.getElementsByTagName('issue'):
+      num_issues += 1
+      issue_id = issue.attributes['id'].value
+      severity = issue.attributes['severity'].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 [%s]' % (path, line, severity, message,
+                                       issue_id)
+      else:
+        # Issues in class files don't have a line number.
+        error = '%s %s: %s [%s]' % (path, severity, message, issue_id)
+      print >> sys.stderr, error
+    return num_issues
+
+  _ProcessConfigFile()
+
+  cmd = [
+      lint_path, '-Werror', '--exitcode', '--showall',
+      '--config', _RelativizePath(processed_config_path),
+      '--classpath', _RelativizePath(classes_dir),
+      '--xml', _RelativizePath(result_path),
+  ]
+  for src in src_dirs:
+    cmd.extend(['--sources', _RelativizePath(src)])
+  cmd.append(_RelativizePath(os.path.join(manifest_path, os.pardir)))
+
+  if os.path.exists(result_path):
+    os.remove(result_path)
+
+  cwd = os.path.abspath(src_root)
+  child = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE,

[newt (away), 2013/12/05 23:39:33]

I have a CL out for review to change build_utils.CheckCallDie() to throw an
exception instead of calling sys.exit(). If that lands, you can catch the
exception (build_utils.CalledProcessError) and re-raise it or do other things as
needed:

    try:
      # Use the new "CheckCallDie" aka "CheckOutput"
      build_utils.CheckOutput(cmd, cwd=cwd)
    except build_utils.CalledProcessError as e:
      if not os.path.exists(result_path):
        raise
      else:
        # Print out list of lint errors, then
        return 1

    return 0

[frankf, 2013/12/06 22:17:36]

Nice refactoring. Done. I'll rebase after your CL lands.

+                           cwd=cwd)
+  stdout, stderr = child.communicate()
+  rc = child.returncode
+
+  if rc:
+    # There is a problem with lint usage
+    if not os.path.exists(result_path):
+      # A user should be able to simply copy and paste the command that failed
+      # into their shell.
+      copyable_command = ' '.join(map(pipes.quote, cmd))
+      copyable_command = ('( cd ' + os.path.abspath(cwd) + '; '
+                          + copyable_command + ' )')
+      print >> sys.stderr, 'Command failed:', copyable_command, '\n'
+
+      if stdout:
+        print stdout,
+      if stderr:
+        print >> sys.stderr, stderr,
+    # There are actual lint issues
+    else:
+      num_issues = _ParseAndShowResultFile()
+      _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' %
+             (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
+
+  return rc
+
+
+def main(argv):
+  start = time.time()
+
+  parser = optparse.OptionParser()
+  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-root', help='Path to top-level src dir.')
+  parser.add_option('--src-dirs', help='Directories containing java files.')
+  parser.add_option('--classes-dir', help='Directory containing class files.')
+  parser.add_option('--stamp', help='Path to touch on success.')
+  parser.add_option('--enable', action='store_true',

[newt (away), 2013/12/05 23:39:33]

Huh, where else is this used?

[frankf, 2013/12/06 22:17:36]

See lint_action.gypi

On 2013/12/05 23:39:33, newt wrote:

+                    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', 'src_root',
+                                 'src_dirs', 'classes_dir'])
+
+  src_dirs = build_utils.ParseGypList(options.src_dirs)
+
+  rc = 0
+
+  if options.enable:
+    rc = _RunLint(options.lint_path, options.config_path,
+                  options.processed_config_path,
+                  options.manifest_path, options.result_path,
+                  options.product_dir, options.src_root, src_dirs,
+                  options.classes_dir)
+
+  if options.stamp and not rc:
+    build_utils.Touch(options.stamp)
+
+  # TODO(frankf): Remove this before committing.

[newt (away), 2013/12/05 23:39:33]

don't forget...

[frankf, 2013/12/06 22:17:36]

Removed debugging logs.

+  print 'Lint took %s for %s' % (int(time.time() - start), options.classes_dir)
+
+  return rc
+
+
+if __name__ == '__main__':
+  sys.exit(main(sys.argv))