Skip to content

C
Chromium_Depot_tools
Commit 7d5cc1b4 authored by jam@chromium.org's avatar jam@chromium.org
Browse files
Options

Revert 193525 

I get the following when doing "gcl upload"

Traceback (most recent call last):
  File "f:\src\depot_tools\gcl.py", line 1459, in <module>
    sys.exit(main(sys.argv[1:]))
  File "f:\src\depot_tools\gcl.py", line 1437, in main
    return command(argv[1:])
  File "f:\src\depot_tools\gcl.py", line 1070, in CMDchange
    description = change_info.GetIssueDescription()
AttributeError: 'ChangeInfo' object has no attribute 'GetIssueDescription'
Sending crash report ...
  args: ['f:\\src\\depot_tools\\gcl.py', 'change', 'w4f4']
  cwd: f:\src\chrome3\src
  exception: 'ChangeInfo' object has no attribute 'GetIssueDesc
  host: JABDELMALEK3-W.ad.corp.google.com
  stack:   File "f:\src\depot_tools\gcl.py", line 1459, in
  user: jabdelmalek
  version: 2.6.2 (r262:71600, Apr 21 2009, 15:05:37) [MSC v.1
A stack trace has been sent to the maintainers.



 "Make gcl use git_cl.py code for consistency in th..."

> 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

TBR=maruel@chromium.org
Review URL: https://codereview.chromium.org/14016004

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@193555 0039d316-1c4b-4281-b951-d872f2087c98
parent 8a0afa04
Hide whitespace changes
Inline Side-by-side
Showing with 59 additions and 48 deletions
gcl.py
View file @ 7d5cc1b4
......@@ -24,7 +24,6 @@ import breakpad # pylint: disable=W0611
import fix_encoding
import gclient_utils
import git_cl
import presubmit_support
import rietveld
from scm import SVN
......@@ -60,6 +59,8 @@ 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.
......@@ -272,12 +273,15 @@ 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._files = files or []
self._description = 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
......@@ -285,19 +289,31 @@ class ChangeInfo(object):
rietveld_url or GetCodeReviewSetting('CODE_REVIEW_SERVER'))
self._rpc_server = None
@property
def description(self):
return self._desc.description
def _get_description(self):
return self._description
def force_description(self, new_description):
self._desc = git_cl.ChangeDescription(new_description)
self.needs_upload = True
def _set_description(self, description):
# TODO(dpranke): Cloned from git_cl.py. These should be shared.
if not description:
self._description = description
return
def append_footer(self, line):
self._desc.append_footer(line)
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 get_reviewers(self):
return self._desc.get_reviewers()
description = property(_get_description, _set_description)
@property
def reviewers(self):
return self._reviewers
def NeedsUpload(self):
return self.needs_upload
......@@ -372,12 +388,10 @@ class ChangeInfo(object):
ctype, body = upload.EncodeMultipartFormData(data, [])
self.SendToRietveld('/%d/description' % self.issue, payload=body,
content_type=ctype)
self.needs_upload = False
def UpdateDescriptionFromIssue(self):
"""Updates self.description with the issue description from Rietveld."""
self._desc = git_cl.ChangeDescription(
self.SendToRietveld('/%d/description' % self.issue))
def GetIssueDescription(self):
"""Returns the issue description from Rietveld."""
return self.SendToRietveld('/%d/description' % self.issue)
def AddComment(self, comment):
"""Adds a comment for an issue on Rietveld.
......@@ -837,7 +851,7 @@ def CMDupload(change_info, args):
upload_arg = ["upload.py", "-y"]
upload_arg.append("--server=%s" % change_info.rietveld)
reviewers = change_info.get_reviewers() or output.reviewers
reviewers = change_info.reviewers or output.reviewers
if (reviewers and
not any(arg.startswith('-r') or arg.startswith('--reviewer') for
arg in args)):
......@@ -989,17 +1003,17 @@ def CMDcommit(change_info, args):
commit_cmd = ["svn", "commit"]
if change_info.issue:
# Get the latest description from Rietveld.
change_info.UpdateDescriptionFromIssue()
change_info.description = change_info.GetIssueDescription()
commit_desc = git_cl.ChangeDescription(change_info.description)
commit_message = change_info.description.replace('\r\n', '\n')
if change_info.issue:
server = change_info.rietveld
if not server.startswith("http://") and not server.startswith("https://"):
server = "http://" + server
commit_desc.append_footer('Review URL: %s/%d' % (server, change_info.issue))
commit_message += ('\nReview URL: %s/%d' % (server, change_info.issue))
handle, commit_filename = tempfile.mkstemp(text=True)
os.write(handle, commit_desc.description)
os.write(handle, commit_message)
os.close(handle)
try:
handle, targets_filename = tempfile.mkstemp(text=True)
......@@ -1025,10 +1039,11 @@ 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.append_footer('Committed: ' + viewvc_url + revision)
change_info.description += "\nCommitted: " + viewvc_url + revision
elif revision:
change_info.append_footer('Committed: ' + revision)
change_info.description += "\nCommitted: " + revision
change_info.CloseIssue()
props = change_info.RpcServer().get_issue_properties(
change_info.issue, False)
......@@ -1123,7 +1138,8 @@ def CMDchange(args):
new_description = split_result[0]
cl_files_text = split_result[1]
if new_description != description or override_description:
change_info.force_description(new_description)
change_info.description = new_description
change_info.needs_upload = True
new_cl_files = []
for line in cl_files_text.splitlines():
......@@ -1152,6 +1168,7 @@ 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 @ 7d5cc1b4
......@@ -53,6 +53,7 @@ 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 = []
......@@ -101,13 +102,13 @@ class GclUnittest(GclTestsBase):
'GetCodeReviewSetting', 'GetFilesNotInCL', 'GetInfoDir',
'GetModifiedFiles', 'GetRepositoryRoot', 'ListFiles',
'LoadChangelistInfoForMultiple', 'MISSING_TEST_MSG',
'OptionallyDoPresubmitChecks', 'REPOSITORY_ROOT',
'OptionallyDoPresubmitChecks', 'REPOSITORY_ROOT', 'REVIEWERS_REGEX',
'RunShell', 'RunShellWithReturnCode', 'SVN',
'TryChange', 'UnknownFiles', 'Warn',
'attrs', 'breakpad', 'defer_attributes', 'fix_encoding',
'gclient_utils', 'git_cl', 'json', 'main', 'need_change',
'need_change_and_args', 'no_args', 'optparse', 'os',
'presubmit_support', 'random', 're', 'rietveld',
'gclient_utils', 'json', 'main', 'need_change', 'need_change_and_args',
'no_args', 'optparse', 'os', 'presubmit_support', 'random', 're',
'rietveld',
'string', 'subprocess2', 'sys', 'tempfile', 'time',
'upload', 'urllib2',
]
......@@ -192,15 +193,13 @@ class ChangeInfoUnittest(GclTestsBase):
self.mox.ReplayAll()
members = [
'AddComment', 'CloseIssue', 'Delete', 'Exists', 'GetFiles',
'GetFileNames', 'GetLocalRoot',
'Load',
'GetFileNames', 'GetLocalRoot', 'GetIssueDescription', 'Load',
'MissingTests', 'NeedsUpload', 'PrimeLint', 'RpcServer', 'Save',
'SendToRietveld',
'SEPARATOR',
'UpdateDescriptionFromIssue', 'UpdateRietveldDescription',
'append_footer',
'description', 'force_description', 'get_reviewers', 'issue', 'name',
'needs_upload', 'patch', 'patchset', 'rietveld',
'UpdateRietveldDescription',
'description', 'issue', 'name',
'needs_upload', 'patch', 'patchset', 'reviewers', 'rietveld',
]
# If this test fails, you should add the relevant test.
self.compareMembers(
......@@ -323,7 +322,6 @@ 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',
......@@ -458,13 +456,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')
......@@ -502,7 +500,6 @@ 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')
......@@ -575,8 +572,7 @@ class CMDCommitUnittest(GclTestsBase):
change_info = self.mockLoad()
self.mockPresubmit(change_info, fail=False)
self.mockCommit(
change_info, 'deescription\n\nReview URL: https://my_server/1', '')
change_info.UpdateDescriptionFromIssue()
change_info, 'deescription\nReview URL: https://my_server/1', '')
self.mox.ReplayAll()
retval = gcl.CMDcommit(['naame'])
......@@ -591,17 +587,15 @@ class CMDCommitUnittest(GclTestsBase):
change_info = self.mockLoad()
self.mockPresubmit(change_info, fail=False)
self.mockCommit(
change_info,
'deescription\n\nReview URL: https://my_server/1',
change_info, 'deescription\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)
# This is because append_footer is mocked.
self.assertEquals(change_info.description, 'deescription')
self.assertEquals(change_info.description,
'deescription\n\nCommitted: http://view/12345')
# 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