diff --git a/git_cl.py b/git_cl.py index 523dc2c42..a137bfcc7 100755 --- a/git_cl.py +++ b/git_cl.py @@ -1155,7 +1155,7 @@ class Changelist(object): self.lookedup_patchset = False self.patchset = None self.cc = None - self.watchers = () + self.more_cc = [] self._remote = None self._codereview_impl = None @@ -1203,21 +1203,19 @@ class Changelist(object): """ if self.cc is None: base_cc = settings.GetDefaultCCList() - more_cc = ','.join(self.watchers) + more_cc = ','.join(self.more_cc) self.cc = ','.join(filter(None, (base_cc, more_cc))) or '' return self.cc def GetCCListWithoutDefault(self): """Return the users cc'd on this CL excluding default ones.""" if self.cc is None: - self.cc = ','.join(self.watchers) + self.cc = ','.join(self.more_cc) return self.cc - def SetWatchers(self, watchers): - """Sets the list of email addresses that should be cc'd based on the changed - files in this CL. - """ - self.watchers = watchers + def ExtendCC(self, more_cc): + """Extends the list of users to cc on this CL based on the changed files.""" + self.more_cc.extend(more_cc) def GetBranch(self): """Returns the short branch name, e.g. 'master'.""" @@ -1627,7 +1625,7 @@ class Changelist(object): watchlist = watchlists.Watchlists(change.RepositoryRoot()) files = [f.LocalPath() for f in change.AffectedFiles()] if not options.bypass_watchlists: - self.SetWatchers(watchlist.GetWatchersForPaths(files)) + self.ExtendCC(watchlist.GetWatchersForPaths(files)) if not options.bypass_hooks: if options.reviewers or options.tbrs or options.add_owners_to: @@ -1646,6 +1644,7 @@ class Changelist(object): return 1 if not options.reviewers and hook_results.reviewers: options.reviewers = hook_results.reviewers.split(',') + self.ExtendCC(hook_results.more_cc) # TODO(tandrii): Checking local patchset against remote patchset is only # supported for Rietveld. Extend it to Gerrit or remove it completely. diff --git a/presubmit_support.py b/presubmit_support.py index d80985c1f..f71a9b180 100755 --- a/presubmit_support.py +++ b/presubmit_support.py @@ -94,6 +94,7 @@ class PresubmitOutput(object): self.input_stream = input_stream self.output_stream = output_stream self.reviewers = [] + self.more_cc = [] self.written_output = [] self.error_count = 0 @@ -275,6 +276,11 @@ class OutputApi(object): def __init__(self, is_committing): self.is_committing = is_committing + self.more_cc = [] + + def AppendCC(self, cc): + """Appends a user to cc for this change.""" + self.more_cc.append(cc) def PresubmitPromptOrNotify(self, *args, **kwargs): """Warn the user when uploading, but only notify if committing.""" @@ -1271,6 +1277,7 @@ class PresubmitExecuter(object): self.gerrit = gerrit_obj self.verbose = verbose self.dry_run = dry_run + self.more_cc = [] def ExecPresubmitScript(self, script_text, presubmit_path): """Executes a single presubmit script. @@ -1292,6 +1299,7 @@ class PresubmitExecuter(object): input_api = InputApi(self.change, presubmit_path, self.committing, self.rietveld, self.verbose, gerrit_obj=self.gerrit, dry_run=self.dry_run) + output_api = OutputApi(self.committing) context = {} try: exec script_text in context @@ -1306,10 +1314,11 @@ class PresubmitExecuter(object): function_name = 'CheckChangeOnUpload' if function_name in context: try: - context['__args'] = (input_api, OutputApi(self.committing)) + context['__args'] = (input_api, output_api) logging.debug('Running %s in %s', function_name, presubmit_path) result = eval(function_name + '(*__args)', context) logging.debug('Running %s done.', function_name) + self.more_cc = output_api.more_cc finally: map(os.remove, input_api._named_temporary_files) if not (isinstance(result, types.TupleType) or @@ -1402,6 +1411,7 @@ def DoPresubmitChecks(change, presubmit_script = gclient_utils.FileRead(filename, 'rU') results += executer.ExecPresubmitScript(presubmit_script, filename) + output.more_cc.extend(executer.more_cc) errors = [] notifications = [] warnings = [] diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index c9e28f283..7a634dd6c 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -71,6 +71,8 @@ class ChangelistMock(object): class PresubmitMock(object): def __init__(self, *args, **kwargs): self.reviewers = [] + self.more_cc = ['chromium-reviews+test-more-cc@chromium.org'] + @staticmethod def should_continue(): return True @@ -889,16 +891,17 @@ class TestGitCl(TestCase): def _cmd_line(description, args, similarity, find_copies, private, cc): """Returns the upload command line passed to upload.RealMain().""" return [ - 'upload', '--assume_yes', '--server', - 'https://codereview.example.com', + 'upload', '--assume_yes', '--server', 'https://codereview.example.com', '--message', description ] + args + [ - '--cc', ','.join(['joe@example.com'] + cc), - ] + (['--private'] if private else []) + [ - '--git_similarity', similarity or '50' - ] + (['--git_no_find_copies'] if find_copies is False else []) + [ - 'fake_ancestor_sha', 'HEAD' - ] + '--cc', + ','.join( + ['joe@example.com', 'chromium-reviews+test-more-cc@chromium.org'] + + cc), + ] + (['--private'] + if private else []) + ['--git_similarity', similarity or '50'] + ( + ['--git_no_find_copies'] + if find_copies is False else []) + ['fake_ancestor_sha', 'HEAD'] def _run_reviewer_test( self, @@ -1627,9 +1630,10 @@ class TestGitCl(TestCase): ] calls += [ ((['git', 'config', 'rietveld.cc'],), ''), - (('AddReviewers', 'chromium-review.googlesource.com', - 123456 if squash else None, sorted(reviewers), - ['joe@example.com'] + cc, notify), ''), + (('AddReviewers', 'chromium-review.googlesource.com', 123456 + if squash else None, sorted(reviewers), + ['joe@example.com', 'chromium-reviews+test-more-cc@chromium.org'] + + cc, notify), ''), ] if tbr: calls += [ diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index fbafe3a80..775ed7d1e 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -1403,9 +1403,16 @@ class OutputApiUnittest(PresubmitTestsBase): def testMembersChanged(self): self.mox.ReplayAll() members = [ - 'MailTextResult', 'PresubmitError', 'PresubmitNotifyResult', - 'PresubmitPromptWarning', 'PresubmitPromptOrNotify', 'PresubmitResult', - 'is_committing', 'EnsureCQIncludeTrybotsAreAdded', + 'AppendCC', + 'MailTextResult', + 'PresubmitError', + 'PresubmitNotifyResult', + 'PresubmitPromptWarning', + 'PresubmitPromptOrNotify', + 'PresubmitResult', + 'is_committing', + 'more_cc', + 'EnsureCQIncludeTrybotsAreAdded', ] # If this test fails, you should add the relevant test. self.compareMembers(presubmit.OutputApi(False), members) @@ -1424,6 +1431,12 @@ class OutputApiUnittest(PresubmitTestsBase): # TODO(joi) Test MailTextResult once implemented. + def testAppendCC(self): + output_api = presubmit.OutputApi(False) + output_api.AppendCC('chromium-reviews@chromium.org') + self.assertEquals(['chromium-reviews@chromium.org'], output_api.more_cc) + + def testOutputApiHandling(self): self.mox.ReplayAll()