Actually check Rietveld for LGTMs in CheckOwners()

presubmit_canned_checks.py

Index: presubmit_canned_checks.py
diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py
index 29967f54d22b26873ad0ac8f65250f8c3452c25d..77827054ee803e8b10b82d5c6f6804cff701a190 100644
--- a/presubmit_canned_checks.py
+++ b/presubmit_canned_checks.py
@@ -540,7 +540,7 @@ def RunPylint(input_api, output_api, white_list=None, black_list=None):
   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:
@@ -626,16 +626,22 @@ def CheckBuildbotPendingBuilds(input_api, output_api, url, max_pendings,
   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'):
@@ -643,3 +649,29 @@ def CheckOwners(input_api, output_api, source_file_filter=None):
 
   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)
+