Infer CL author and reviewer list from local state if the issue has not previously been uploaded.

R=agable@chromium.org
BUG=none

Review URL: https://codereview.chromium.org/195793021

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@259250 0039d316-1c4b-4281-b951-d872f2087c98

git_cl.py

@@ -1694,6 +1694,11 @@ def CMDupload(parser, args):
     cl.SetWatchers(watchlist.GetWatchersForPaths(files))
 
   if not options.bypass_hooks:
+    if options.reviewers:
+      # Set the reviewer list now so that presubmit checks can access it.
+      change_description = ChangeDescription(change.FullDescriptionText())
+      change_description.update_reviewers(options.reviewers)
+      change.SetDescriptionText(change_description.description)
     hook_results = cl.RunHook(committing=False,
                               may_prompt=not options.force,
                               verbose=options.verbose,

presubmit_canned_checks.py

@@ -852,6 +852,16 @@ def _GetRietveldIssueProps(input_api, messages):
         issue=int(issue), messages=messages)
 
 
+def _ReviewersFromChange(change):
+  """Return the reviewers specified in the |change|, if any."""
+  reviewers = set()
+  if change.R:
+    reviewers.update(set([r.strip() for r in change.R.split(',')]))
+  if change.TBR:
+    reviewers.update(set([r.strip() for r in change.TBR.split(',')]))
+  return reviewers
+
+
 def _RietveldOwnerAndReviewers(input_api, email_regexp, approval_needed=False):
   """Return the owner and reviewers of a change, if any.
 
@@ -860,7 +870,10 @@ def _RietveldOwnerAndReviewers(input_api, email_regexp, approval_needed=False):
   """
   issue_props = _GetRietveldIssueProps(input_api, True)
   if not issue_props:
-    return None, set()
+    reviewers = set()
+    if not approval_needed:
+      reviewers = _ReviewersFromChange(input_api.change)
+    return None, reviewers
 
   if not approval_needed:
     return issue_props['owner_email'], set(issue_props['reviewers'])

presubmit_support.py

@@ -794,27 +794,16 @@ class Change(object):
     if files is None:
       files = []
     self._name = name
-    self._full_description = description
     # Convert root into an absolute path.
     self._local_root = os.path.abspath(local_root)
     self.issue = issue
     self.patchset = patchset
     self.author_email = author
 
-    # From the description text, build up a dictionary of key/value pairs
-    # plus the description minus all key/value or "tag" lines.
-    description_without_tags = []
+    self._full_description = ''
     self.tags = {}
-    for line in self._full_description.splitlines():
-      m = self.TAG_LINE_RE.match(line)
-      if m:
-        self.tags[m.group('key')] = m.group('value')
-      else:
-        description_without_tags.append(line)
-
-    # Change back to text and remove whitespace at end.
-    self._description_without_tags = (
-        '\n'.join(description_without_tags).rstrip())
+    self._description_without_tags = ''
+    self.SetDescriptionText(description)
 
     assert all(
         (isinstance(f, (list, tuple)) and len(f) == 2) for f in files), files
@@ -842,6 +831,27 @@ class Change(object):
     """Returns the complete changelist description including tags."""
     return self._full_description
 
+  def SetDescriptionText(self, description):
+    """Sets the full description text (including tags) to |description|.
+    
+    Also updates the list of tags."""
+    self._full_description = description
+
+    # From the description text, build up a dictionary of key/value pairs
+    # plus the description minus all key/value or "tag" lines.
+    description_without_tags = []
+    self.tags = {}
+    for line in self._full_description.splitlines():
+      m = self.TAG_LINE_RE.match(line)
+      if m:
+        self.tags[m.group('key')] = m.group('value')
+      else:
+        description_without_tags.append(line)
+
+    # Change back to text and remove whitespace at end.
+    self._description_without_tags = (
+        '\n'.join(description_without_tags).rstrip())
+
   def RepositoryRoot(self):
     """Returns the repository (checkout) root directory for this change,
     as an absolute path.

tests/presubmit_unittest.py

@@ -1768,7 +1768,7 @@ class ChangeUnittest(PresubmitTestsBase):
         'AbsoluteLocalPaths', 'AffectedFiles', 'AffectedTextFiles',
         'DescriptionText', 'FullDescriptionText', 'LocalPaths', 'Name',
         'RepositoryRoot', 'RightHandSideLines', 'ServerPaths',
-        'TAG_LINE_RE',
+        'SetDescriptionText', 'TAG_LINE_RE',
         'author_email', 'issue', 'patchset', 'scm', 'tags',
     ]
     # If this test fails, you should add the relevant test.
@@ -1791,6 +1791,19 @@ class ChangeUnittest(PresubmitTestsBase):
     self.assertEquals(1, len(change.AffectedFiles(include_dirs=True)))
     self.assertEquals('Y', change.AffectedFiles(include_dirs=True)[0].Action())
 
+  def testSetDescriptionText(self):
+    change = presubmit.Change(
+        '', 'foo\nDRU=ro', self.fake_root_dir, [], 3, 5, '')
+    self.assertEquals('foo', change.DescriptionText())
+    self.assertEquals('foo\nDRU=ro', change.FullDescriptionText())
+    self.assertEquals('ro', change.DRU)
+
+    change.SetDescriptionText('bar\nWHIZ=bang')
+    self.assertEquals('bar', change.DescriptionText())
+    self.assertEquals('bar\nWHIZ=bang', change.FullDescriptionText())
+    self.assertEquals('bang', change.WHIZ)
+    self.assertFalse(change.DRU)
+
 
 def CommHelper(input_api, cmd, ret=None, **kwargs):
   ret = ret or (('', None), 0)
@@ -2574,6 +2587,8 @@ class CannedChecksUnittest(PresubmitTestsBase):
     change = self.mox.CreateMock(presubmit.Change)
     change.issue = issue
     change.author_email = 'john@example.com'
+    change.R = ''
+    change.TBR = ''
     affected_file = self.mox.CreateMock(presubmit.SvnAffectedFile)
     input_api = self.MockInputApi(change, False)
     fake_db = self.mox.CreateMock(owners.Database)
@@ -2585,7 +2600,7 @@ class CannedChecksUnittest(PresubmitTestsBase):
     if not is_committing or (not tbr and issue):
       affected_file.LocalPath().AndReturn('foo/xyz.cc')
       change.AffectedFiles(file_filter=None).AndReturn([affected_file])
-      if not rietveld_response:
+      if issue and not rietveld_response:
         rietveld_response = {
           "owner_email": change.author_email,
           "messages": [
@@ -2599,6 +2614,7 @@ class CannedChecksUnittest(PresubmitTestsBase):
         people = approvers
       else:
         people = reviewers
+        change.R = ','.join(reviewers)
 
       if issue:
         input_api.rietveld.get_issue_properties(
@@ -2710,6 +2726,16 @@ class CannedChecksUnittest(PresubmitTestsBase):
         expected_output='Missing OWNER reviewers for these files:\n'
                         '    foo\n')
 
+  def testCannedCheckOwners_NoIssueLocalReviewers(self):
+    self.AssertOwnersWorks(issue=None,
+        reviewers=set(['jane@example.com']),
+        expected_output="OWNERS check failed: this change has no Rietveld "
+                        "issue number, so we can't check it for approvals.\n")
+    self.AssertOwnersWorks(issue=None,
+        reviewers=set(['jane@example.com']),
+        is_committing=False,
+        expected_output='')
+
   def testCannedCheckOwners_NoLGTM(self):
     self.AssertOwnersWorks(expected_output='Missing LGTM from someone '
                                            'other than john@example.com\n')