Reapply r193525 "Make gcl use git_cl.py code for consistency in the CL description formatting."

depot_tools/gcl.py

 #!/usr/bin/env python
 # Copyright (c) 2012 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.
 """
 
 import json
 import optparse
 import os
 import random
 import re
 import string
 import sys
 import tempfile
 import time
 import urllib2
 
 import breakpad  # pylint: disable=W0611
 
 
 import fix_encoding
 import gclient_utils
+import git_cl
 import presubmit_support
 import rietveld
 from scm import SVN
 import subprocess2
 from third_party import upload
 
 __version__ = '1.2.1'
 
 
 CODEREVIEW_SETTINGS = {
@@ skipping 15 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)
     if home != None:
@@ skipping 192 matching lines @@
     files: a list of 2 tuple containing (status, filename) of changed files,
            with paths being relative to the top repository directory.
     local_root: Local root directory
     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_url, needs_upload):
+    # Defer the description processing to git_cl.ChangeDescription.
+    self._desc = git_cl.ChangeDescription(description)
     self.name = name
     self.issue = int(issue)
     self.patchset = int(patchset)
-    self._description = None
-    self._reviewers = None
-    self._set_description(description)
-    if files is None:
-      files = []
-    self._files = files
+    self._files = files or  []
     self.patch = None
     self._local_root = local_root
     self.needs_upload = needs_upload
     self.rietveld = gclient_utils.UpgradeToHttps(
         rietveld_url or GetCodeReviewSetting('CODE_REVIEW_SERVER'))
     self._rpc_server = None
 
-  def _get_description(self):
-    return self._description
+  @property
+  def description(self):
+    return self._desc.description
 
-  def _set_description(self, description):
-    # TODO(dpranke): Cloned from git_cl.py. These should be shared.
-    if not description:
-      self._description = description
-      return
+  def force_description(self, new_description):
+    self._desc = git_cl.ChangeDescription(new_description)
+    self.needs_upload = True
 
-    parsed_lines = []
-    reviewers_re = re.compile(REVIEWERS_REGEX)
-    reviewers = ''
-    for l in description.splitlines():
-      matched_reviewers = reviewers_re.match(l)
-      if matched_reviewers:
-        reviewers = matched_reviewers.group(1).split(',')
-      parsed_lines.append(l)
-    self._reviewers = reviewers
-    self._description = '\n'.join(parsed_lines)
+  def append_footer(self, line):
+    self._desc.append_footer(line)
 
-  description = property(_get_description, _set_description)
-
-  @property
-  def reviewers(self):
-    return self._reviewers
+  def get_reviewers(self):
+    return self._desc.get_reviewers()
 
   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."""
@@ skipping 54 matching lines @@
     ctype, body = upload.EncodeMultipartFormData(data, [])
     self.SendToRietveld('/%d/close' % self.issue, payload=body,
         content_type=ctype)
 
   def UpdateRietveldDescription(self):
     """Sets the description for an issue on Rietveld."""
     data = [("description", self.description),]
     ctype, body = upload.EncodeMultipartFormData(data, [])
     self.SendToRietveld('/%d/description' % self.issue, payload=body,
         content_type=ctype)
+    self.needs_upload = False
 
   def GetIssueDescription(self):
     """Returns the issue description from Rietveld."""
     return self.SendToRietveld('/%d/description' % self.issue)
 
+  def UpdateDescriptionFromIssue(self):
+    """Updates self.description with the issue description from Rietveld."""
+    self._desc = git_cl.ChangeDescription(
+        self.SendToRietveld('/%d/description' % self.issue))
+
   def AddComment(self, comment):
     """Adds a comment for an issue on Rietveld.
     As a side effect, this will email everyone associated with the issue."""
     return self.RpcServer().add_comment(self.issue, comment)
 
   def PrimeLint(self):
     """Do background work on Rietveld to lint the file so that the results are
     ready when the issue is viewed."""
     if self.issue and self.patchset:
       self.SendToRietveld('/lint/issue%s_%s' % (self.issue, self.patchset),
@@ skipping 438 matching lines @@
   args = map(replace_message, args)
   if found_deprecated_arg[0]:
     print >> sys.stderr, (
         '\nWARNING: Use -t or --title to set the title of the patchset.\n'
         'In the near future, -m or --message will send a message instead.\n'
         'See http://goo.gl/JGg0Z for details.\n')
 
   upload_arg = ["upload.py", "-y"]
   upload_arg.append("--server=%s" % change_info.rietveld)
 
-  reviewers = change_info.reviewers or output.reviewers
+  reviewers = change_info.get_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 = None
   try:
     if change_info.issue:
@@ skipping 131 matching lines @@
   # We face a problem with svn here: Let's say change 'bleh' modifies
   # svn:ignore on dir1\. but another unrelated change 'pouet' modifies
   # dir1\foo.cc. When the user `gcl commit bleh`, foo.cc is *also committed*.
   # The only fix is to use --non-recursive but that has its issues too:
   # Let's say if dir1 is deleted, --non-recursive must *not* be used otherwise
   # you'll get "svn: Cannot non-recursively commit a directory deletion of a
   # directory with child nodes". Yay...
   commit_cmd = ["svn", "commit"]
   if change_info.issue:
     # Get the latest description from Rietveld.
-    change_info.description = change_info.GetIssueDescription()
+    change_info.UpdateDescriptionFromIssue()
 
-  commit_message = change_info.description.replace('\r\n', '\n')
+  commit_desc = git_cl.ChangeDescription(change_info.description)
   if change_info.issue:
     server = change_info.rietveld
     if not server.startswith("http://") and not server.startswith("https://"):
       server = "http://" + server
-    commit_message += ('\nReview URL: %s/%d' % (server, change_info.issue))
+    commit_desc.append_footer('Review URL: %s/%d' % (server, change_info.issue))
 
   handle, commit_filename = tempfile.mkstemp(text=True)
-  os.write(handle, commit_message)
+  os.write(handle, commit_desc.description)
   os.close(handle)
   try:
     handle, targets_filename = tempfile.mkstemp(text=True)
     os.write(handle, "\n".join(change_info.GetFileNames()))
     os.close(handle)
     try:
       commit_cmd += ['--file=' + commit_filename]
       commit_cmd += ['--targets=' + targets_filename]
       # Change the current working directory before calling commit.
       output = ''
       try:
         output = RunShell(commit_cmd, True)
       except subprocess2.CalledProcessError, e:
         ErrorExit('Commit failed.\n%s' % e)
     finally:
       os.remove(commit_filename)
   finally:
     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 += '\n'
       if viewvc_url and revision:
-        change_info.description += "\nCommitted: " + viewvc_url + revision
+        change_info.append_footer('Committed: ' + viewvc_url + revision)
       elif revision:
-        change_info.description += "\nCommitted: " + revision
+        change_info.append_footer('Committed: ' + revision)
       change_info.CloseIssue()
       props = change_info.RpcServer().get_issue_properties(
           change_info.issue, False)
       patch_num = len(props['patchsets'])
       comment = "Committed patchset #%d manually as r%s" % (patch_num, revision)
       comment += ' (presubmit successful).' if not bypassed else '.'
       change_info.AddComment(comment)
   return 0
 
 
@@ skipping 74 matching lines @@
     ErrorExit('Running editor failed')
 
   split_result = result.split(separator1, 1)
   if len(split_result) != 2:
     ErrorExit("Don't modify the text starting with ---!\n\n%r" % result)
 
   # Update the CL description if it has changed.
   new_description = split_result[0]
   cl_files_text = split_result[1]
   if new_description != description or override_description:
-    change_info.description = new_description
-    change_info.needs_upload = True
+    change_info.force_description(new_description)
 
   new_cl_files = []
   for line in cl_files_text.splitlines():
     if not len(line):
       continue
     if line.startswith("---"):
       break
     status = line[:7]
     filename = line[7:]
     new_cl_files.append((status, filename))
 
   if (not len(change_info.GetFiles()) and not change_info.issue and
       not len(new_description) and not new_cl_files):
     ErrorExit("Empty changelist not saved")
 
   change_info._files = new_cl_files
   change_info.Save()
   if svn_info.get('URL', '').startswith('http:'):
     Warn("WARNING: Creating CL in a read-only checkout.  You will need to "
          "commit using a commit queue!")
 
   print change_info.name + " changelist saved."
   if change_info.MissingTests():
     Warn("WARNING: " + MISSING_TEST_MSG)
 
   # Update the Rietveld issue.
   if change_info.issue and change_info.NeedsUpload():
     change_info.UpdateRietveldDescription()
-    change_info.needs_upload = False
     change_info.Save()
   return 0
 
 
 @need_change_and_args
 def CMDlint(change_info, args):
   """Runs cpplint.py on all the files in the change list.
 
   Checks all the files in the changelist for possible style violations.
   """
@@ skipping 288 matching lines @@
       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__":
   fix_encoding.fix_encoding()
   sys.exit(main(sys.argv[1:]))