Actually check Rietveld for LGTMs in CheckOwners()

presubmit_canned_checks.py

 # Copyright (c) 2010 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.
 
 """Generic presubmit checks that can be reused by other presubmit checks."""
 
 ### Description checks
 
 def CheckChangeHasTestField(input_api, output_api):
   """Requires that the changelist have a TEST= field."""
@@ skipping 522 matching lines @@
     if result:
       if input_api.is_committing:
         error_type = output_api.PresubmitError
       else:
         error_type = output_api.PresubmitPromptWarning
       return [error_type('Fix pylint errors first.')]
     return []
   finally:
     warnings.filterwarnings('default', category=DeprecationWarning)
 
-
+# TODO(dpranke): Get the host_url from the input_api instead
 def CheckRietveldTryJobExecution(input_api, output_api, host_url, platforms,
                                  owner):
   if not input_api.is_committing:
     return []
   if not input_api.change.issue or not input_api.change.patchset:
     return []
   url = '%s/%d/get_build_results/%d' % (
       host_url, input_api.change.issue, input_api.change.patchset)
   try:
     connection = input_api.urllib2.urlopen(url)
@@ skipping 65 matching lines @@
       out.append('%s has %d build(s) pending' %
                   (builder_name, pending_builds_len))
   if out:
     return [output_api.PresubmitPromptWarning(
         'Build(s) pending. It is suggested to wait that no more than %d '
             'builds are pending.' % max_pendings,
         long_text='\n'.join(out))]
   return []
 
 
-def CheckOwners(input_api, output_api, source_file_filter=None):
+def CheckOwners(input_api, output_api, email_regexp=None,
+    source_file_filter=None):
   affected_files = set([f.LocalPath() for f in
       input_api.change.AffectedFiles(source_file_filter)])
   owners_db = input_api.owners_db
+  if email_regexp:
+    owners_db.email_regexp = input_api.re.compile(email_regexp)

[M-A Ruel, 2011/03/11 21:19:09]

If there is:
a/PRESUBMIT.py
a/b/PRESUBMIT.py

both with a CheckOwners() but different email_regexp, the regexp will be
overwritten with a's version, right?

 
-  if input_api.is_committing:
-    missing_files = owners_db.files_not_covered_by(affected_files,
-                                                input_api.change.approvers)
+  if input_api.is_committing and input_api.is_tbr:
+    return [output_api.PresubmitNotifyResult(
+        '--tbr was specified, skipping OWNERS check')]
+  elif input_api.is_committing:
+    approvers = _Approvers(input_api, owners_db.email_regexp)
+    missing_files = owners_db.files_not_covered_by(affected_files, approvers)
     if missing_files:
-      return [output_api.PresubmitError('Missing owner LGTM for: %s' %
+      return [output_api.PresubmitError('Missing LGTM from an OWNER for: %s' %
               ','.join(missing_files))]
     return []
   elif input_api.change.tags.get('R'):
     return []
 
   suggested_reviewers = owners_db.reviewers_for(affected_files)
   return [output_api.PresubmitAddText('R=%s' % ','.join(suggested_reviewers))]
+
+
+def _Approvers(input_api, email_regexp):
+  if not input_api.change.issue:
+    return []
+
+  path = '/api/%s?messages=true'
+  url = (input_api.host_url + path) % input_api.change.issue
+
+  f = input_api.urllib2.urlopen(url)
+  issue_props = input_api.json.load(f)
+  owner = input_api.re.escape(issue_props['owner'])
+
+  # TODO(dpranke): This mimics the logic in
+  # /tools/commit-queue/verifiers/reviewer_lgtm.py
+  # We should share the code and/or remove the check there where it is
+  # redundant (since the commit queue also enforces the presubmit checks).
+  def match_reviewer(r):
+    return email_regexp.match(r) and not input_api.re.match(owner, r)
+
+  approvers = []
+  for m in issue_props['messages']:
+    if 'lgtm' in m['text'].lower() and match_reviewer(m['sender']):
+      approvers.append(m['sender'])
+  return set(approvers)
+