Skip to content

C
Chromium_Depot_tools
Commit 8a0afa04 authored by maruel@chromium.org's avatar maruel@chromium.org
Browse files
Options

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

This way, git_cl.py's ChangeDescription becomes the canonical CL description
handler.

R=dpranke@chromium.org
BUG=

Review URL: https://chromiumcodereview.appspot.com/13800018

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@193525 0039d316-1c4b-4281-b951-d872f2087c98
parent 78936cbc
Hide whitespace changes
Inline Side-by-side
Showing with 48 additions and 59 deletions
gcl.py
View file @ 8a0afa04
......@@ -24,6 +24,7 @@ import breakpad # pylint: disable=W0611
import fix_encoding
import gclient_utils
import git_cl
import presubmit_support
import rietveld
from scm import SVN
......@@ -59,8 +60,6 @@ FILES_CACHE = {}
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.
......@@ -273,15 +272,12 @@ class ChangeInfo(object):
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
......@@ -289,31 +285,19 @@ class ChangeInfo(object):
rietveld_url or GetCodeReviewSetting('CODE_REVIEW_SERVER'))
self._rpc_server = None
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
@property
def description(self):
return self._desc.description
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 force_description(self, new_description):
self._desc = git_cl.ChangeDescription(new_description)
self.needs_upload = True
description = property(_get_description, _set_description)
def append_footer(self, line):
self._desc.append_footer(line)
@property
def reviewers(self):
return self._reviewers
def get_reviewers(self):
return self._desc.get_reviewers()
def NeedsUpload(self):
return self.needs_upload
......@@ -388,10 +372,12 @@ class ChangeInfo(object):
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.
......@@ -851,7 +837,7 @@ def CMDupload(change_info, args):
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)):
......@@ -1003,17 +989,17 @@ def CMDcommit(change_info, args):
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)
......@@ -1039,11 +1025,10 @@ def CMDcommit(change_info, args):
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)
......@@ -1138,8 +1123,7 @@ def CMDchange(args):
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():
......@@ -1168,7 +1152,6 @@ def CMDchange(args):
# 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
......
tests/gcl_unittest.py
View file @ 8a0afa04
......@@ -53,7 +53,6 @@ class GclTestsBase(SuperMoxTestBase):
change_info.GetLocalRoot = lambda : 'proout'
change_info.patch = None
change_info.rietveld = 'https://my_server'
change_info.reviewers = None
change_info._closed = False
change_info._deleted = False
change_info._comments_added = []
......@@ -102,13 +101,13 @@ class GclUnittest(GclTestsBase):
'GetCodeReviewSetting', 'GetFilesNotInCL', 'GetInfoDir',
'GetModifiedFiles', 'GetRepositoryRoot', 'ListFiles',
'LoadChangelistInfoForMultiple', 'MISSING_TEST_MSG',
'OptionallyDoPresubmitChecks', 'REPOSITORY_ROOT', 'REVIEWERS_REGEX',
'OptionallyDoPresubmitChecks', 'REPOSITORY_ROOT',
'RunShell', 'RunShellWithReturnCode', 'SVN',
'TryChange', 'UnknownFiles', 'Warn',
'attrs', 'breakpad', 'defer_attributes', 'fix_encoding',
'gclient_utils', 'json', 'main', 'need_change', 'need_change_and_args',
'no_args', 'optparse', 'os', 'presubmit_support', 'random', 're',
'rietveld',
'gclient_utils', 'git_cl', 'json', 'main', 'need_change',
'need_change_and_args', 'no_args', 'optparse', 'os',
'presubmit_support', 'random', 're', 'rietveld',
'string', 'subprocess2', 'sys', 'tempfile', 'time',
'upload', 'urllib2',
]
......@@ -193,13 +192,15 @@ class ChangeInfoUnittest(GclTestsBase):
self.mox.ReplayAll()
members = [
'AddComment', 'CloseIssue', 'Delete', 'Exists', 'GetFiles',
'GetFileNames', 'GetLocalRoot', 'GetIssueDescription', 'Load',
'GetFileNames', 'GetLocalRoot',
'Load',
'MissingTests', 'NeedsUpload', 'PrimeLint', 'RpcServer', 'Save',
'SendToRietveld',
'SEPARATOR',
'UpdateRietveldDescription',
'description', 'issue', 'name',
'needs_upload', 'patch', 'patchset', 'reviewers', 'rietveld',
'UpdateDescriptionFromIssue', 'UpdateRietveldDescription',
'append_footer',
'description', 'force_description', 'get_reviewers', 'issue', 'name',
'needs_upload', 'patch', 'patchset', 'rietveld',
]
# If this test fails, you should add the relevant test.
self.compareMembers(
......@@ -322,6 +323,7 @@ class CMDuploadUnittest(GclTestsBase):
gcl.os.getcwd().AndReturn('somewhere')
change_info.GetFiles().AndReturn(change_info.files)
gcl.os.chdir('proout')
change_info.get_reviewers().AndReturn('foo@bar.com')
change_info.GetFileNames().AndReturn(files)
gcl.GenerateDiff(files)
gcl.upload.RealMain(['upload.py', '-y', '--server=https://my_server',
......@@ -456,13 +458,13 @@ class CMDuploadUnittest(GclTestsBase):
change_info.files = [('A', 'aa'), ('M', 'bb')]
change_info.patch = None
change_info.rietveld = 'https://my_server'
change_info.reviewers = ['georges@example.com']
files = [item[1] for item in change_info.files]
output = presubmit_support.PresubmitOutput()
gcl.DoPresubmitChecks(change_info, False, True).AndReturn(output)
#gcl.GetCodeReviewSetting('CODE_REVIEW_SERVER').AndReturn('my_server')
gcl.os.getcwd().AndReturn('somewhere')
change_info.GetFiles().AndReturn(change_info.files)
change_info.get_reviewers().AndReturn(['georges@example.com'])
change_info.GetFileNames().AndReturn(files)
change_info.GetLocalRoot().AndReturn('proout')
gcl.os.chdir('proout')
......@@ -500,6 +502,7 @@ class CMDuploadUnittest(GclTestsBase):
'--reviewers=foo@example.com,bar@example.com',
'--issue=1', '--title= '],
change_info.patch).AndReturn(("1", "2"))
change_info.get_reviewers().AndReturn(['foo@example.com,bar@example.com'])
change_info.Save()
change_info.PrimeLint()
gcl.os.chdir('somewhere')
......@@ -572,7 +575,8 @@ class CMDCommitUnittest(GclTestsBase):
change_info = self.mockLoad()
self.mockPresubmit(change_info, fail=False)
self.mockCommit(
change_info, 'deescription\nReview URL: https://my_server/1', '')
change_info, 'deescription\n\nReview URL: https://my_server/1', '')
change_info.UpdateDescriptionFromIssue()
self.mox.ReplayAll()
retval = gcl.CMDcommit(['naame'])
......@@ -587,15 +591,17 @@ class CMDCommitUnittest(GclTestsBase):
change_info = self.mockLoad()
self.mockPresubmit(change_info, fail=False)
self.mockCommit(
change_info, 'deescription\nReview URL: https://my_server/1',
change_info,
'deescription\n\nReview URL: https://my_server/1',
'\nCommitted revision 12345')
change_info.UpdateDescriptionFromIssue()
change_info.append_footer('Committed: http://view/12345')
self.mox.ReplayAll()
retval = gcl.CMDcommit(['naame'])
self.assertEquals(retval, 0)
self.assertEquals(change_info.description,
'deescription\n\nCommitted: http://view/12345')
# This is because append_footer is mocked.
self.assertEquals(change_info.description, 'deescription')
# pylint: disable=W0212
self.assertTrue(change_info._deleted)
self.assertTrue(change_info._closed)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment