Add presubmit check to verify issue is not closed.

presubmit_canned_checks.py

Index: presubmit_canned_checks.py
diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py
index af88a39b1e8b35099655b3ac1688edfb33c6171e..7abb16e658ea62939581fb194d25592fc5b088a5 100644
--- a/presubmit_canned_checks.py
+++ b/presubmit_canned_checks.py
@@ -742,7 +742,8 @@ 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, source_file_filter=None,
+                issue_props=None):

[M-A Ruel, 2012/11/28 02:43:49]

Can you rebase on ToT and remove this argument now?

[Isaac (away), 2012/11/28 22:28:17]

done.

   if input_api.is_committing:
     if input_api.tbr:
       return [output_api.PresubmitNotifyResult(
@@ -760,8 +761,9 @@ def CheckOwners(input_api, output_api, source_file_filter=None):
       input_api.change.AffectedFiles(file_filter=source_file_filter)])
 
   owners_db = input_api.owners_db
+  issue_props = issue_props or _GetRietveldIssueProps(input_api, True)
   owner_email, reviewers = _RietveldOwnerAndReviewers(
-      input_api,
+      issue_props,
       owners_db.email_regexp,
       approval_needed=input_api.is_committing)
 
@@ -791,17 +793,39 @@ def CheckOwners(input_api, output_api, source_file_filter=None):
   return []
 
 
-def _RietveldOwnerAndReviewers(input_api, email_regexp, approval_needed=False):
+def _GetRietveldIssueProps(input_api, messages):
+  """Gets the issue properties from rietveld."""
+  issue = input_api.change.issue
+  if issue and input_api.rietveld:
+    return input_api.rietveld.get_issue_properties(
+        issue=issue, messages=messages)
+
+
+def CheckIssueNotClosed(issue_props, output_api):
+  """Verify issue is not closed.
+
+  We should not be working with a closed review. CQ and dcommit set this bit,
+  so it is a pretty good indicator of whether an issue has been committed.
+  """
+  if issue_props['closed']:
+    return [output_api.PresubmitError(
+        'Issue %s is closed.  If this issue was already used for a commit,\n'
+        'please reset the issue number associated with this branch with:\n'
+        'git cl issue 0\n' % issue_props['issue']
+        )]
+  return []
+
+
+def _RietveldOwnerAndReviewers(issue_props, email_regexp,
+                               approval_needed=False):
   """Return the owner and reviewers of a change, if any.
 
   If approval_needed is True, only reviewers who have approved the change
   will be returned.
   """
-  if not input_api.change.issue:
+  if not issue_props:
     return None, None
 
-  issue_props = input_api.rietveld.get_issue_properties(
-      int(input_api.change.issue), True)
   if not approval_needed:
     return issue_props['owner_email'], set(issue_props['reviewers'])
 
@@ -935,10 +959,19 @@ def PanProjectChecks(input_api, output_api,
         print "  %s took a long time: %dms" % (snapshot_memory[1], delta_ms)
     snapshot_memory[:] = (dt2, msg)
 
+  issue_props = _GetRietveldIssueProps(input_api, messages=True)
   if owners_check:
     snapshot("checking owners")
     results.extend(input_api.canned_checks.CheckOwners(
-        input_api, output_api, source_file_filter=None))
+        input_api=input_api,
+        output_api=output_api,
+        source_file_filter=None,
+        issue_props=issue_props))
+
+  if issue_props:
+    snapshot("checking review not closed")
+    results.extend(input_api.canned_checks.CheckIssueNotClosed(
+        issue_props, output_api))
 
   snapshot("checking long lines")
   results.extend(input_api.canned_checks.CheckLongLines(