diff --git a/gcl.py b/gcl.py index 9b3d213d70..f2876518aa 100755 --- a/gcl.py +++ b/gcl.py @@ -248,14 +248,30 @@ def WriteFile(filename, contents): class ChangeInfo(object): """Holds information about a changelist. - issue: the Rietveld issue number, of "" if it hasn't been uploaded yet. + name: change name. + issue: the Rietveld issue number or 0 if it hasn't been uploaded yet. + patchset: the Rietveld latest patchset number or 0. description: the description. files: a list of 2 tuple containing (status, filename) of changed files, with paths being relative to the top repository directory. """ - def __init__(self, name="", issue="", description="", files=None): + + _SEPARATOR = "\n-----\n" + # The info files have the following format: + # issue_id, patchset\n (, patchset is optional) + # _SEPARATOR\n + # filepath1\n + # filepath2\n + # . + # . + # filepathn\n + # _SEPARATOR\n + # description + + def __init__(self, name, issue, patchset, description, files): self.name = name - self.issue = issue + self.issue = int(issue) + self.patchset = int(patchset) self.description = description if files is None: files = [] @@ -276,9 +292,10 @@ class ChangeInfo(object): def Save(self): """Writes the changelist information to disk.""" - data = SEPARATOR.join([self.issue, - "\n".join([f[0] + f[1] for f in self.files]), - self.description]) + data = ChangeInfo._SEPARATOR.join([ + "%d, %d" % (self.issue, self.patchset), + "\n".join([f[0] + f[1] for f in self.files]), + self.description]) WriteFile(GetChangelistInfoFile(self.name), data) def Delete(self): @@ -378,18 +395,57 @@ class ChangeInfo(object): return False + @staticmethod + def Load(changename, fail_on_not_found=True, update_status=False): + """Gets information about a changelist. + + Args: + fail_on_not_found: if True, this function will quit the program if the + changelist doesn't exist. + update_status: if True, the svn status will be updated for all the files + and unchanged files will be removed. -SEPARATOR = "\n-----\n" -# The info files have the following format: -# issue_id\n -# SEPARATOR\n -# filepath1\n -# filepath2\n -# . -# . -# filepathn\n -# SEPARATOR\n -# description + Returns: a ChangeInfo object. + """ + info_file = GetChangelistInfoFile(changename) + if not os.path.exists(info_file): + if fail_on_not_found: + ErrorExit("Changelist " + changename + " not found.") + return ChangeInfo(changename) + split_data = ReadFile(info_file).split(ChangeInfo._SEPARATOR, 2) + if len(split_data) != 3: + ErrorExit("Changelist file %s is corrupt" % info_file) + items = split_data[0].split(',') + issue = 0 + patchset = 0 + if items[0]: + issue = int(items[0]) + if len(items) > 1: + patchset = int(items[1]) + files = [] + for line in split_data[1].splitlines(): + status = line[:7] + file = line[7:] + files.append((status, file)) + description = split_data[2] + save = False + if update_status: + for file in files: + filename = os.path.join(GetRepositoryRoot(), file[1]) + status_result = gclient.CaptureSVNStatus(filename) + if not status_result or not status_result[0][0]: + # File has been reverted. + save = True + files.remove(file) + continue + status = status_result[0][0] + if status != file[0]: + save = True + files[files.index(file)] = (status, file[1]) + change_info = ChangeInfo(changename, issue, patchset, description, files) + if save: + change_info.Save() + return change_info def GetChangelistInfoFile(changename): @@ -406,63 +462,13 @@ def LoadChangelistInfoForMultiple(changenames, fail_on_not_found=True, This is mainly usefull to concatenate many changes into one for a 'gcl try'. """ changes = changenames.split(',') - aggregate_change_info = ChangeInfo(name=changenames) + aggregate_change_info = ChangeInfo(changenames, 0, 0, '', None) for change in changes: - aggregate_change_info.files += LoadChangelistInfo(change, - fail_on_not_found, - update_status).files + aggregate_change_info.files += ChangeInfo.Load(change, fail_on_not_found, + update_status).files return aggregate_change_info -def LoadChangelistInfo(changename, fail_on_not_found=True, - update_status=False): - """Gets information about a changelist. - - Args: - fail_on_not_found: if True, this function will quit the program if the - changelist doesn't exist. - update_status: if True, the svn status will be updated for all the files - and unchanged files will be removed. - - Returns: a ChangeInfo object. - """ - info_file = GetChangelistInfoFile(changename) - if not os.path.exists(info_file): - if fail_on_not_found: - ErrorExit("Changelist " + changename + " not found.") - return ChangeInfo(changename) - data = ReadFile(info_file) - split_data = data.split(SEPARATOR, 2) - if len(split_data) != 3: - os.remove(info_file) - ErrorExit("Changelist file %s was corrupt and deleted" % info_file) - issue = split_data[0] - files = [] - for line in split_data[1].splitlines(): - status = line[:7] - file = line[7:] - files.append((status, file)) - description = split_data[2] - save = False - if update_status: - for file in files: - filename = os.path.join(GetRepositoryRoot(), file[1]) - status_result = gclient.CaptureSVNStatus(filename) - if not status_result or not status_result[0][0]: - # File has been reverted. - save = True - files.remove(file) - continue - status = status_result[0][0] - if status != file[0]: - save = True - files[files.index(file)] = (status, file[1]) - change_info = ChangeInfo(changename, issue, description, files) - if save: - change_info.Save() - return change_info - - def GetCLs(): """Returns a list of all the changelists in this repository.""" cls = os.listdir(GetChangesDir()) @@ -501,7 +507,7 @@ def GetModifiedFiles(): # Get a list of all files in changelists. files_in_cl = {} for cl in GetCLs(): - change_info = LoadChangelistInfo(cl) + change_info = ChangeInfo.Load(cl) for status, filename in change_info.files: files_in_cl[filename] = change_info.name @@ -559,7 +565,7 @@ def SendToRietveld(request_path, payload=None, def GetIssueDescription(issue): """Returns the issue description from Rietveld.""" - return SendToRietveld("/" + issue + "/description") + return SendToRietveld("/%d/description" % issue) def Opened(): @@ -570,7 +576,7 @@ def Opened(): for cl_name in cl_keys: if cl_name: note = "" - if len(LoadChangelistInfo(cl_name).files) != len(files[cl_name]): + if len(ChangeInfo.Load(cl_name).files) != len(files[cl_name]): note = " (Note: this changelist contains files outside this directory)" print "\n--- Changelist " + cl_name + note + ":" for file in files[cl_name]: @@ -761,7 +767,7 @@ def UploadCL(change_info, args): if not found_message: upload_arg.append("--message=''") - upload_arg.append("--issue=" + change_info.issue) + upload_arg.append("--issue=%d" % change_info.issue) else: # First time we upload. handle, desc_file = tempfile.mkstemp(text=True) os.write(handle, change_info.description) @@ -792,8 +798,9 @@ def UploadCL(change_info, args): if change_info.patch is None: change_info.patch = GenerateDiff(change_info.FileList()) issue, patchset = upload.RealMain(upload_arg, change_info.patch) - if issue and issue != change_info.issue: - change_info.issue = issue + if issue and patchset: + change_info.issue = int(issue) + change_info.patchset = int(patchset) change_info.Save() if desc_file: @@ -807,14 +814,10 @@ def UploadCL(change_info, args): if not no_try: try_on_upload = GetCodeReviewSetting('TRY_ON_UPLOAD') if try_on_upload and try_on_upload.lower() == 'true': - # Use the local diff. - args = [ - "--issue", change_info.issue, - "--patchset", patchset, - ] + trychange_args = [] if clobber: - args.append('--clobber') - TryChange(change_info, args, swallow_exception=True) + trychange_args.append('--clobber') + TryChange(change_info, trychange_args, swallow_exception=True) os.chdir(previous_cwd) @@ -843,6 +846,10 @@ def TryChange(change_info, args, swallow_exception): if change_info: trychange_args = ['--name', change_info.name] + if change_info.issue: + trychange_args.extend(["--issue", str(change_info.issue)]) + if change_info.patchset: + trychange_args.extend(["--patchset", str(change_info.patchset)]) trychange_args.extend(args) trychange.TryChange(trychange_args, file_list=change_info.FileList(), @@ -881,7 +888,7 @@ def Commit(change_info, args): commit_message = change_info.description.replace('\r\n', '\n') if change_info.issue: - commit_message += ('\nReview URL: http://%s/%s' % + commit_message += ('\nReview URL: http://%s/%d' % (GetCodeReviewSetting("CODE_REVIEW_SERVER"), change_info.issue)) @@ -1041,7 +1048,7 @@ def DoPresubmitChecks(change_info, committing): def Changes(): """Print all the changelists and their files.""" for cl in GetCLs(): - change_info = LoadChangelistInfo(cl, True, True) + change_info = ChangeInfo.Load(cl, True, True) print "\n--- Changelist " + change_info.name + ":" for file in change_info.files: print "".join(file) @@ -1115,7 +1122,7 @@ def main(argv=None): if command == "try" and changename.find(',') != -1: change_info = LoadChangelistInfoForMultiple(changename, True, True) else: - change_info = LoadChangelistInfo(changename, fail_on_not_found, True) + change_info = ChangeInfo.Load(changename, fail_on_not_found, True) if command == "change": if (len(argv) == 4): diff --git a/presubmit_support.py b/presubmit_support.py index 928929e35f..574e4e61c2 100755 --- a/presubmit_support.py +++ b/presubmit_support.py @@ -443,6 +443,8 @@ class GclChange(object): self._name = change_info.name self._full_description = change_info.description self._repository_root = repository_root + self.issue = change_info.issue + self.patchset = change_info.patchset # From the description text, build up a dictionary of key/value pairs # plus the description minus all key/value or "tag" lines. diff --git a/tests/gcl_unittest.py b/tests/gcl_unittest.py index dd499e4cc7..2e9b3bea0e 100755 --- a/tests/gcl_unittest.py +++ b/tests/gcl_unittest.py @@ -48,10 +48,10 @@ class GclUnittest(GclTestsBase): 'GetFilesNotInCL', 'GetInfoDir', 'GetIssueDescription', 'GetModifiedFiles', 'GetRepositoryRoot', 'GetSVNFileProperty', 'Help', 'IGNORE_PATHS', 'IsSVNMoved', - 'Lint', 'LoadChangelistInfo', 'LoadChangelistInfoForMultiple', + 'Lint', 'LoadChangelistInfoForMultiple', 'MISSING_TEST_MSG', 'Opened', 'PresubmitCL', 'ReadFile', 'REPOSITORY_ROOT', 'RunShell', - 'RunShellWithReturnCode', 'SEPARATOR', 'SendToRietveld', 'TryChange', + 'RunShellWithReturnCode', 'SendToRietveld', 'TryChange', 'UnknownFiles', 'UploadCL', 'Warn', 'WriteFile', 'gclient', 'getpass', 'main', 'os', 'random', 're', 'shutil', 'string', 'subprocess', 'sys', 'tempfile', @@ -95,25 +95,71 @@ class GclUnittest(GclTestsBase): class ChangeInfoUnittest(GclTestsBase): + def setUp(self): + GclTestsBase.setUp(self) + self.mox.StubOutWithMock(gcl, 'GetChangelistInfoFile') + self.mox.StubOutWithMock(gcl.os.path, 'exists') + self.mox.StubOutWithMock(gcl, 'ReadFile') + self.mox.StubOutWithMock(gcl, 'WriteFile') + def testChangeInfoMembers(self): members = [ - 'CloseIssue', 'Delete', 'FileList', 'MissingTests', 'Save', + 'CloseIssue', 'Delete', 'FileList', 'Load', 'MissingTests', 'Save', 'UpdateRietveldDescription', 'description', 'files', 'issue', 'name', - 'patch' + 'patch', 'patchset', ] # If this test fails, you should add the relevant test. - self.compareMembers(gcl.ChangeInfo(), members) + self.compareMembers(gcl.ChangeInfo('', 0, 0, '', None), members) def testChangeInfoBase(self): files = [('M', 'foo'), ('A', 'bar')] - o = gcl.ChangeInfo('name2', 'issue2', 'description2', files) + self.mox.ReplayAll() + o = gcl.ChangeInfo('name2', '42', '53', 'description2', files) self.assertEquals(o.name, 'name2') - self.assertEquals(o.issue, 'issue2') + self.assertEquals(o.issue, 42) + self.assertEquals(o.patchset, 53) self.assertEquals(o.description, 'description2') self.assertEquals(o.files, files) self.assertEquals(o.patch, None) self.assertEquals(o.FileList(), ['foo', 'bar']) + def testLoadWithIssue(self): + description = ["This is some description.", "force an extra separator."] + gcl.GetChangelistInfoFile('bleh').AndReturn('bleeeh') + gcl.os.path.exists('bleeeh').AndReturn(True) + gcl.ReadFile('bleeeh').AndReturn( + gcl.ChangeInfo._SEPARATOR.join(["42,53", "G b.cc"] + description)) + self.mox.ReplayAll() + + change_info = gcl.ChangeInfo.Load('bleh', True, False) + self.assertEquals(change_info.name, 'bleh') + self.assertEquals(change_info.issue, 42) + self.assertEquals(change_info.patchset, 53) + self.assertEquals(change_info.description, + gcl.ChangeInfo._SEPARATOR.join(description)) + self.assertEquals(change_info.files, [('G ', 'b.cc')]) + + def testLoadEmpty(self): + gcl.GetChangelistInfoFile('bleh').AndReturn('bleeeh') + gcl.os.path.exists('bleeeh').AndReturn(True) + gcl.ReadFile('bleeeh').AndReturn( + gcl.ChangeInfo._SEPARATOR.join(["", "", ""])) + self.mox.ReplayAll() + + change_info = gcl.ChangeInfo.Load('bleh', True, False) + self.assertEquals(change_info.name, 'bleh') + self.assertEquals(change_info.issue, 0) + self.assertEquals(change_info.patchset, 0) + self.assertEquals(change_info.description, "") + self.assertEquals(change_info.files, []) + + def testSaveEmpty(self): + gcl.GetChangelistInfoFile('').AndReturn('foo') + gcl.WriteFile('foo', gcl.ChangeInfo._SEPARATOR.join(['0, 0', '', ''])) + self.mox.ReplayAll() + change_info = gcl.ChangeInfo('', 0, 0, '', None) + change_info.Save() + class UploadCLUnittest(GclTestsBase): def setUp(self): @@ -132,9 +178,9 @@ class UploadCLUnittest(GclTestsBase): self.mox.StubOutWithMock(gcl, 'TryChange') def testNew(self): - change_info = gcl.ChangeInfo('naame', 'iissue', 'deescription', + change_info = gcl.ChangeInfo('naame', 1, 0, 'deescription', ['aa', 'bb']) - change_info.Save = self.mox.CreateMockAnything() + self.mox.StubOutWithMock(change_info, 'Save') args = ['--foo=bar'] change_info.Save() gcl.DoPresubmitChecks(change_info, committing=False).AndReturn(True) @@ -143,19 +189,17 @@ class UploadCLUnittest(GclTestsBase): gcl.os.chdir(gcl.GetRepositoryRoot().AndReturn(None)) gcl.GenerateDiff(change_info.FileList()) gcl.upload.RealMain(['upload.py', '-y', '--server=my_server', '--foo=bar', - "--message=''", '--issue=iissue'], change_info.patch).AndReturn(("1", - "2")) + "--message=''", '--issue=1'], change_info.patch).AndReturn(("1", + "2")) gcl.SendToRietveld("/lint/issue%s_%s" % ('1', '2'), timeout=0.5) gcl.GetCodeReviewSetting('TRY_ON_UPLOAD').AndReturn('True') - gcl.TryChange(change_info, - ['--issue', '1', '--patchset', '2'], - swallow_exception=True) + gcl.TryChange(change_info, [], swallow_exception=True) gcl.os.chdir('somewhere') self.mox.ReplayAll() gcl.UploadCL(change_info, args) def testServerOverride(self): - change_info = gcl.ChangeInfo('naame', '', 'deescription', + change_info = gcl.ChangeInfo('naame', 0, 0, 'deescription', ['aa', 'bb']) change_info.Save = self.mox.CreateMockAnything() args = ['--server=a'] @@ -179,7 +223,7 @@ class UploadCLUnittest(GclTestsBase): gcl.UploadCL(change_info, args) def testNoTry(self): - change_info = gcl.ChangeInfo('naame', '', 'deescription', + change_info = gcl.ChangeInfo('naame', 0, 0, 'deescription', ['aa', 'bb']) change_info.Save = self.mox.CreateMockAnything() args = ['--no-try'] @@ -203,7 +247,7 @@ class UploadCLUnittest(GclTestsBase): gcl.UploadCL(change_info, args) def testNormal(self): - change_info = gcl.ChangeInfo('naame', '', 'deescription', + change_info = gcl.ChangeInfo('naame', 0, 0, 'deescription', ['aa', 'bb']) self.mox.StubOutWithMock(change_info, 'Save') args = [] @@ -223,12 +267,13 @@ class UploadCLUnittest(GclTestsBase): gcl.os.remove('descfile') gcl.SendToRietveld("/lint/issue%s_%s" % ('1', '2'), timeout=0.5) gcl.GetCodeReviewSetting('TRY_ON_UPLOAD').AndReturn('True') - gcl.TryChange(change_info, - ['--issue', '1', '--patchset', '2'], - swallow_exception=True) + gcl.TryChange(change_info, [], swallow_exception=True) gcl.os.chdir('somewhere') self.mox.ReplayAll() + gcl.UploadCL(change_info, args) + self.assertEquals(change_info.issue, 1) + self.assertEquals(change_info.patchset, 2) if __name__ == '__main__': diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 00ea0f6128..be3cd74c19 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -197,7 +197,7 @@ class PresubmitUnittest(PresubmitTestsBase): presubmit.gcl.ReadFile(notfound).AndReturn('look!\nthere?') self.mox.ReplayAll() - ci = presubmit.gcl.ChangeInfo(name='mychange', + ci = presubmit.gcl.ChangeInfo(name='mychange', issue=0, patchset=0, description='\n'.join(description_lines), files=files) change = presubmit.GclChange(ci) @@ -265,7 +265,7 @@ class PresubmitUnittest(PresubmitTestsBase): fake_presubmit = presubmit.os.path.join(self.fake_root_dir, 'PRESUBMIT.py') self.mox.ReplayAll() - ci = presubmit.gcl.ChangeInfo(name='mychange', + ci = presubmit.gcl.ChangeInfo(name='mychange', issue=0, patchset=0, description='\n'.join(description_lines), files=files) @@ -334,7 +334,7 @@ class PresubmitUnittest(PresubmitTestsBase): 'rU').AndReturn(self.presubmit_text) self.mox.ReplayAll() - ci = presubmit.gcl.ChangeInfo(name='mychange', + ci = presubmit.gcl.ChangeInfo(name='mychange', issue=0, patchset=0, description='\n'.join(description_lines), files=files) @@ -364,7 +364,7 @@ class PresubmitUnittest(PresubmitTestsBase): ).AndReturn(self.presubmit_text) self.mox.ReplayAll() - ci = presubmit.gcl.ChangeInfo(name='mychange', + ci = presubmit.gcl.ChangeInfo(name='mychange', issue=0, patchset=0, description='\n'.join(description_lines), files=files) @@ -403,7 +403,7 @@ class PresubmitUnittest(PresubmitTestsBase): ).AndReturn(self.presubmit_text) self.mox.ReplayAll() - ci = presubmit.gcl.ChangeInfo(name='mychange', + ci = presubmit.gcl.ChangeInfo(name='mychange', issue=0, patchset=0, description='\n'.join(description_lines), files=files) output = StringIO.StringIO() @@ -435,7 +435,7 @@ def CheckChangeOnCommit(input_api, output_api): 'PRESUBMIT.py')).AndReturn(False) self.mox.ReplayAll() - ci = presubmit.gcl.ChangeInfo(name='mychange', + ci = presubmit.gcl.ChangeInfo(name='mychange', issue=0, patchset=0, description='\n'.join(description_lines), files=files) @@ -459,9 +459,8 @@ def CheckChangeOnCommit(input_api, output_api): presubmit.os.path.isdir(presubmit.os.path.join('isdir', 'blat.cc') ).AndReturn(False) self.mox.ReplayAll() - ci = presubmit.gcl.ChangeInfo(name='mychange', - description='foo', - files=files) + ci = presubmit.gcl.ChangeInfo(name='mychange', issue=0, patchset=0, + description='foo', files=files) change = presubmit.GclChange(ci) affected_files = change.AffectedFiles(include_dirs=False) @@ -494,7 +493,10 @@ def CheckChangeOnCommit(input_api, output_api): change = presubmit.gcl.ChangeInfo( name='foo', - description="Blah Blah\n\nSTORY=http://tracker.com/42\nBUG=boo\n") + issue=0, + patchset=0, + description="Blah Blah\n\nSTORY=http://tracker.com/42\nBUG=boo\n", + files=None) output = StringIO.StringIO() input = StringIO.StringIO('y\n') self.failUnless(presubmit.DoPresubmitChecks(change, False, True, output, @@ -587,7 +589,7 @@ class InputApiUnittest(PresubmitTestsBase): presubmit.gcl.ReadFile(blat).AndReturn('whatever\ncookie') self.mox.ReplayAll() - ci = presubmit.gcl.ChangeInfo(name='mychange', + ci = presubmit.gcl.ChangeInfo(name='mychange', issue=0, patchset=0, description='\n'.join(description_lines), files=files) change = presubmit.GclChange(ci) @@ -622,7 +624,8 @@ class InputApiUnittest(PresubmitTestsBase): ] self.mox.ReplayAll() - ci = presubmit.gcl.ChangeInfo(name='mychange', description='', files=files) + ci = presubmit.gcl.ChangeInfo(name='mychange', issue=0, patchset=0, + description='', files=files) # It doesn't make sense on non-Windows platform. This is somewhat hacky, # but it is needed since we can't just use os.path.join('c:', 'temp'). change = presubmit.GclChange(ci, self.fake_root_dir) @@ -654,7 +657,8 @@ class InputApiUnittest(PresubmitTestsBase): stacklevel=2) self.mox.ReplayAll() change = presubmit.GclChange( - presubmit.gcl.ChangeInfo(name='mychange', description='Bleh\n')) + presubmit.gcl.ChangeInfo(name='mychange', issue=0, patchset=0, + description='Bleh\n', files=None)) api = presubmit.InputApi(change, 'foo/PRESUBMIT.py') api.AffectedTextFiles(include_deletes=False) @@ -792,6 +796,20 @@ class AffectedFileUnittest(PresubmitTestsBase): self.failUnless(list[0] == output[0]) +class GclChangeUnittest(PresubmitTestsBase): + def testMembersChanged(self): + self.mox.ReplayAll() + members = [ + 'AbsoluteLocalPaths', 'AffectedFiles', 'AffectedTextFiles', 'Change', + 'DescriptionText', 'FullDescriptionText', 'LocalPaths', + 'RepositoryRoot', 'RightHandSideLines', 'ServerPaths', + 'issue', 'patchset', 'tags', + ] + # If this test fails, you should add the relevant test. + ci = presubmit.gcl.ChangeInfo('', 0, 0, '', None) + self.compareMembers(presubmit.GclChange(ci, self.fake_root_dir), members) + + class CannedChecksUnittest(PresubmitTestsBase): """Tests presubmit_canned_checks.py.""" @@ -808,7 +826,7 @@ class CannedChecksUnittest(PresubmitTestsBase): return input_api def MakeBasicChange(self, name, description): - ci = presubmit.gcl.ChangeInfo(name=name, description=description) + ci = presubmit.gcl.ChangeInfo(name, 0, 0, description, None) return presubmit.GclChange(ci, self.fake_root_dir) def testMembersChanged(self):