Modify gcl to use suggested_reviewer output from presubmit_support.

gcl.py

 #!/usr/bin/python
 # 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.
 
 """\
 Wrapper script around Rietveld's upload.py that simplifies working with groups
 of files.
 """
 
@@ skipping 17 matching lines @@
     import json
     # Some versions of python2.5 have an incomplete json module.  Check to make
     # sure loads exists.
     # pylint: disable=W0104
     json.loads
   except (ImportError, AttributeError):
     # Import the one included in depot_tools.
     sys.path.append(os.path.join(os.path.dirname(__file__), 'third_party'))
     import simplejson as json  # pylint: disable=F0401
 
-import breakpad  # pylint: disable=W0611
-
 # gcl now depends on gclient.
 from scm import SVN
+
+import breakpad  # pylint: disable=W0611

[M-A Ruel, 2011/03/15 19:11:57]

breakpad _must_ be the first.

 import gclient_utils
+import owners
+import presubmit_support
 
 __version__ = '1.2'
 
 
 CODEREVIEW_SETTINGS = {
   # To make gcl send reviews to a server, check in a file named
   # "codereview.settings" (see |CODEREVIEW_SETTINGS_FILE| below) to your
   # project's base directory and add the following line to codereview.settings:
   # CODE_REVIEW_SERVER: codereview.yourserver.org
 }
@@ skipping 10 matching lines @@
 # Warning message when the change appears to be missing tests.
 MISSING_TEST_MSG = "Change contains new or modified methods, but no new tests!"
 
 # Global cache of files cached in GetCacheDir().
 FILES_CACHE = {}
 
 # Valid extensions for files we want to lint.
 DEFAULT_LINT_REGEX = r"(.*\.cpp|.*\.cc|.*\.h)"
 DEFAULT_LINT_IGNORE_REGEX = r"$^"
 
+REVIEWERS_REGEX = r'\s*R=(.+)'
 
 def CheckHomeForFile(filename):
   """Checks the users home dir for the existence of the given file.  Returns
   the path to the file if it's there, or None if it is not.
   """
   home_vars = ['HOME']
   if sys.platform in ('cygwin', 'win32'):
     home_vars.append('USERPROFILE')
   for home_var in home_vars:
     home = os.getenv(home_var)
@@ skipping 196 matching lines @@
     rietveld: rietveld server for this change
   """
   # Kept for unit test support. This is for the old format, it's deprecated.
   _SEPARATOR = "\n-----\n"
 
   def __init__(self, name, issue, patchset, description, files, local_root,
                rietveld, needs_upload=False):
     self.name = name
     self.issue = int(issue)
     self.patchset = int(patchset)
-    self.description = description
+    self._description = None
+    self._subject = None
+    self._reviewers = None
+    self._set_description(description)
     if files is None:
       files = []
     self._files = files
     self.patch = None
     self._local_root = local_root
     self.needs_upload = needs_upload
     self.rietveld = rietveld
     if not self.rietveld:
       # Set the default value.
       self.rietveld = GetCodeReviewSetting('CODE_REVIEW_SERVER')
 
+  def _get_description(self):
+    return self._description
+
+  def _set_description(self, description):
+    # TODO(dpranke): Cloned from git_cl.py. These should be shared.
+    if not description:
+      self._description = description
+      return
+
+    parsed_lines = []
+    reviewers_re = re.compile(REVIEWERS_REGEX)
+    reviewers = ''
+    subject = ''
+    for l in description.splitlines():
+      if not subject:
+        subject = l
+      matched_reviewers = reviewers_re.match(l)
+      if matched_reviewers:
+        reviewers = matched_reviewers.group(1)
+      parsed_lines.append(l)
+
+    if subject.find("\r\n") != -1:

[M-A Ruel, 2011/03/15 19:11:57]

http://docs.python.org/library/stdtypes.html#str.splitlines

you can in fact:
assert not ('\r' in subject or '\n' in subject)

[chase, 2011/03/15 20:48:58]

single quotes here and below, please

+      subject = subject[:subject.find("\r\n")]
+    if subject.find("\n") != -1:
+      subject = subject[:subject.find("\n")]
+    if len(subject) > 77:

[M-A Ruel, 2011/03/15 19:11:57]

The limit is in fact 100.

if len(subject) > 100

+      subject = subject + "..."

[M-A Ruel, 2011/03/15 19:11:57]

subject[:97]

+
+    self._subject = subject
+    self._reviewers = reviewers
+    self._description = '\n'.join(parsed_lines)
+
+  description = property(_get_description, _set_description)

[M-A Ruel, 2011/03/15 19:11:57]

Use python 2.2 format instead:
http://docs.python.org/library/functions.html#property

+
+  @property
+  def reviewers(self):
+    return self._reviewers
+
+  @property
+  def subject(self):
+    return self._subject
+
   def NeedsUpload(self):
     return self.needs_upload
 
   def GetFileNames(self):
     """Returns the list of file names included in this change."""
     return [f[1] for f in self._files]
 
   def GetFiles(self):
     """Returns the list of files included in this change with their status."""
     return self._files
@@ skipping 396 matching lines @@
 
   return editor
 
 
 def GenerateDiff(files, root=None):
   return SVN.GenerateDiff(files, root=root)
 
 
 def OptionallyDoPresubmitChecks(change_info, committing, args):
   if FilterFlag(args, "--no_presubmit") or FilterFlag(args, "--force"):
-    return True
+    return presubmit_support.PresubmitOutput()
   return DoPresubmitChecks(change_info, committing, True)
 
 
+def suggest_reviewers(change_info, affected_files):
+  owners_db = owners.Database(change_info.LocalRoot(), fopen=file,
+                              os_path=os.path)
+  return owners_db.reviewers_for(affected_files)
+
+
 def defer_attributes(a, b):
   """Copy attributes from an object (like a function) to another."""
   for x in dir(a):
     if not getattr(b, x, None):
       setattr(b, x, getattr(a, x))
 
 
 def need_change(function):
   """Converts args -> change_info."""
   # pylint: disable=W0612,W0621
@@ skipping 59 matching lines @@
 def CMDupload(change_info, args):
   """Uploads the changelist to the server for review.
 
   This does not submit a try job; use gcl try to submit a try job.
   """
   if '-s' in args or '--server' in args:
     ErrorExit('Don\'t use the -s flag, fix codereview.settings instead')
   if not change_info.GetFiles():
     print "Nothing to upload, changelist is empty."
     return 0
-  if not OptionallyDoPresubmitChecks(change_info, False, args):
+
+  output = OptionallyDoPresubmitChecks(change_info, False, args)
+  if not output.should_continue():
     return 1
   no_watchlists = (FilterFlag(args, "--no_watchlists") or
                    FilterFlag(args, "--no-watchlists"))
 
   # Map --send-mail to --send_mail
   if FilterFlag(args, "--send-mail"):
     args.append("--send_mail")
 
   upload_arg = ["upload.py", "-y"]
   upload_arg.append("--server=%s" % change_info.rietveld)
+
+  reviewers = change_info.reviewers or output.reviewers
+  if (reviewers and
+      not any(arg.startswith('-r') or arg.startswith('--reviewer') for
+              arg in args)):
+    upload_arg.append('--reviewers=%s' % ','.join(reviewers))
+
   upload_arg.extend(args)
 
   desc_file = ""
   if change_info.issue:  # Uploading a new patchset.
     found_message = False
     for arg in args:
       if arg.startswith("--message") or arg.startswith("-m"):
         found_message = True
         break
 
@@ skipping 13 matching lines @@
       watchlist = watchlists.Watchlists(change_info.GetLocalRoot())
       watchers = watchlist.GetWatchersForPaths(change_info.GetFileNames())
 
     cc_list = GetCodeReviewSetting("CC_LIST")
     if not no_watchlists and watchers:
       # Filter out all empty elements and join by ','
       cc_list = ','.join(filter(None, [cc_list] + watchers))
     if cc_list:
       upload_arg.append("--cc=" + cc_list)
     upload_arg.append("--description_file=" + desc_file + "")
-    if change_info.description:
-      subject = change_info.description[:77]
-      if subject.find("\r\n") != -1:
-        subject = subject[:subject.find("\r\n")]
-      if subject.find("\n") != -1:
-        subject = subject[:subject.find("\n")]
-      if len(change_info.description) > 77:
-        subject = subject + "..."
-      upload_arg.append("--message=" + subject)
+    if change_info.subject:
+      upload_arg.append("--message=" + change_info.subject)
 
     if GetCodeReviewSetting("PRIVATE") == "True":
       upload_arg.append("--private")
 
   # Change the current working directory before calling upload.py so that it
   # shows the correct base.
   previous_cwd = os.getcwd()
   os.chdir(change_info.GetLocalRoot())
   # If we have a lot of files with long paths, then we won't be able to fit
   # the command to "svn diff".  Instead, we generate the diff manually for
@@ skipping 114 matching lines @@
   os.remove(targets_filename)
   if output.find("Committed revision") != -1:
     change_info.Delete()
 
     if change_info.issue:
       revision = re.compile(".*?\nCommitted revision (\d+)",
                             re.DOTALL).match(output).group(1)
       viewvc_url = GetCodeReviewSetting("VIEW_VC")
       change_info.description = change_info.description + '\n'
       if viewvc_url:
-        change_info.description += "\nCommitted: " + viewvc_url + revision
+        change_info.description = (change_info.description +
+                                   "\nCommitted: " + viewvc_url + revision)

[chase, 2011/03/15 20:48:58]

can this be reverted?  seems like there's no difference between pre- and post-
except post- doesn't use +=

       change_info.CloseIssue()
   os.chdir(previous_cwd)
   return 0
 
 
 def CMDchange(args):
   """Creates or edits a changelist.
 
   Only scans the current directory and subdirectories."""
   if len(args) == 0:
@@ skipping 39 matching lines @@
       description = change_info.description
 
   other_files = GetFilesNotInCL()
 
   # Edited files (as opposed to files with only changed properties) will have
   # a letter for the first character in the status string.
   file_re = re.compile(r"^[a-z].+\Z", re.IGNORECASE)
   affected_files = [x for x in other_files if file_re.match(x[0])]
   unaffected_files = [x for x in other_files if not file_re.match(x[0])]
 
+  suggested_reviewers = suggest_reviewers(change_info, affected_files)
+  if suggested_reviewers:
+    reviewers_re = re.compile(REVIEWERS_REGEX)
+    if not any(reviewers_re.match(l) for

[M-A Ruel, 2011/03/15 19:11:57]

if not any(
    reviewers_re.match(l) for l in description.splitlines()):

+                l in description.splitlines()):
+      description += '\nR=' + ','.join(suggested_reviewers) + '\n'
+
   separator1 = ("\n---All lines above this line become the description.\n"
                 "---Repository Root: " + change_info.GetLocalRoot() + "\n"
                 "---Paths in this changelist (" + change_info.name + "):\n")
   separator2 = "\n\n---Paths modified but not in any changelist:\n\n"
   text = (description + separator1 + '\n' +
           '\n'.join([f[0] + f[1] for f in change_info.GetFiles()]))
 
   if change_info.Exists():
     text += (separator2 +
             '\n'.join([f[0] + f[1] for f in affected_files]) + '\n')
@@ skipping 97 matching lines @@
     else:
       print "Skipping file %s" % filename
 
   print "Total errors found: %d\n" % cpplint._cpplint_state.error_count
   os.chdir(previous_cwd)
   return 1
 
 
 def DoPresubmitChecks(change_info, committing, may_prompt):
   """Imports presubmit, then calls presubmit.DoPresubmitChecks."""
-  # Need to import here to avoid circular dependency.
-  import presubmit_support
   root_presubmit = GetCachedFile('PRESUBMIT.py', use_root=True)
   change = presubmit_support.SvnChange(change_info.name,
                                        change_info.description,
                                        change_info.GetLocalRoot(),
                                        change_info.GetFiles(),
                                        change_info.issue,
                                        change_info.patchset)
   output = presubmit_support.DoPresubmitChecks(change=change,
                                                committing=committing,
                                                verbose=False,
                                                output_stream=sys.stdout,
                                                input_stream=sys.stdin,
                                                default_presubmit=root_presubmit,
-                                               may_prompt=may_prompt)
+                                               may_prompt=may_prompt,
+                                               tbr=False,
+                                               host_url=change_info.rietveld)
   if not output.should_continue() and may_prompt:
     # TODO(dpranke): move into DoPresubmitChecks(), unify cmd line args.
     print "\nPresubmit errors, can't continue (use --no_presubmit to bypass)"
 
-  # TODO(dpranke): Return the output object and make use of it.
-  return output.should_continue()
+  return output
 
 
 @no_args
 def CMDchanges():
   """Lists all the changelists and their files."""
   for cl in GetCLs():
     change_info = ChangeInfo.Load(cl, GetRepositoryRoot(), True, True)
     print "\n--- Changelist " + change_info.name + ":"
     for filename in change_info.GetFiles():
       print "".join(filename)
@@ skipping 203 matching lines @@
     if e.code != 500:
       raise
     print >> sys.stderr, (
         'AppEngine is misbehaving and returned HTTP %d, again. Keep faith '
         'and retry or visit go/isgaeup.\n%s') % (e.code, str(e))
     return 1
 
 
 if __name__ == "__main__":
   sys.exit(main(sys.argv[1:]))