From 5626a92eec27dc6f8488cc632e1e0030d9a3dff8 Mon Sep 17 00:00:00 2001 From: "rmistry@google.com" Date: Thu, 26 Feb 2015 14:03:30 +0000 Subject: [PATCH] Add ability to specify and run post upload hooks. Motivation: There are a few things we want to do in Skia infrastructure that is not possible to do without this functionality. Eg1: If there is a change made to Skia's markdown then automatically include a 'DOCS_PREVIEW_URL=https://skia.org?cl=1234' in the CL's description. Eg2: Automatically add 'NOTRY=true' for changes impacting things that do not need trybot runs. Eg3: Include CL specific links to skia's perf (https://perf.skia.org/) and correctness (https://gold.skia.org/) servers. BUG=chromium:462208 Review URL: https://codereview.chromium.org/949273002 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@294242 0039d316-1c4b-4281-b951-d872f2087c98 --- git_cl.py | 17 +++++++++ presubmit_support.py | 74 +++++++++++++++++++++++++++++++++++++ tests/git_cl_test.py | 5 +++ tests/presubmit_unittest.py | 3 +- 4 files changed, 98 insertions(+), 1 deletion(-) diff --git a/git_cl.py b/git_cl.py index 728bae7abc..9ac9470e22 100755 --- a/git_cl.py +++ b/git_cl.py @@ -432,6 +432,11 @@ class Settings(object): def GetBugPrefix(self): return self._GetRietveldConfig('bug-prefix', error_ok=True) + def GetRunPostUploadHook(self): + run_post_upload_hook = self._GetRietveldConfig( + 'run-post-upload-hook', error_ok=True) + return run_post_upload_hook == "True" + def GetDefaultCCList(self): return self._GetRietveldConfig('cc', error_ok=True) @@ -1029,6 +1034,8 @@ def GetCodereviewSettingsInteractively(): 'tree-status-url', False) SetProperty(settings.GetViewVCUrl(), 'ViewVC URL', 'viewvc-url', True) SetProperty(settings.GetBugPrefix(), 'Bug Prefix', 'bug-prefix', False) + SetProperty(settings.GetRunPostUploadHook(), 'Run Post Upload Hook', + 'run-post-upload-hook', False) # TODO: configure a default branch to diff against, rather than this # svn-based hackery. @@ -1211,6 +1218,8 @@ def LoadCodereviewSettingsFromFile(fileobj): SetProperty('cpplint-ignore-regex', 'LINT_IGNORE_REGEX', unset_error_ok=True) SetProperty('project', 'PROJECT', unset_error_ok=True) SetProperty('pending-ref-prefix', 'PENDING_REF_PREFIX', unset_error_ok=True) + SetProperty('run-post-upload-hook', 'RUN_POST_UPLOAD_HOOK', + unset_error_ok=True) if 'GERRIT_HOST' in keyvals: RunGit(['config', 'gerrit.host', keyvals['GERRIT_HOST']]) @@ -2036,6 +2045,14 @@ def CMDupload(parser, args): if not ret: git_set_branch_value('last-upload-hash', RunGit(['rev-parse', 'HEAD']).strip()) + # Run post upload hooks, if specified. + if settings.GetRunPostUploadHook(): + presubmit_support.DoPostUploadExecuter( + change, + cl, + settings.GetRoot(), + options.verbose, + sys.stdout) return ret diff --git a/presubmit_support.py b/presubmit_support.py index b386524068..db677cfd04 100755 --- a/presubmit_support.py +++ b/presubmit_support.py @@ -1143,6 +1143,37 @@ class GetTryMastersExecuter(object): return get_preferred_try_masters(project, change) +class GetPostUploadExecuter(object): + @staticmethod + def ExecPresubmitScript(script_text, presubmit_path, cl, change): + """Executes PostUploadHook() from a single presubmit script. + + Args: + script_text: The text of the presubmit script. + presubmit_path: Project script to run. + cl: The Changelist object. + change: The Change object. + + Return: + A list of results objects. + """ + context = {} + try: + exec script_text in context + except Exception, e: + raise PresubmitFailure('"%s" had an exception.\n%s' + % (presubmit_path, e)) + + function_name = 'PostUploadHook' + if function_name not in context: + return {} + post_upload_hook = context[function_name] + if not len(inspect.getargspec(post_upload_hook)[0]) == 3: + raise PresubmitFailure( + 'Expected function "PostUploadHook" to take three arguments.') + return post_upload_hook(cl, change, OutputApi(False)) + + def DoGetTrySlaves(change, changed_files, repository_root, @@ -1263,6 +1294,49 @@ def DoGetTryMasters(change, return results +def DoPostUploadExecuter(change, + cl, + repository_root, + verbose, + output_stream): + """Execute the post upload hook. + + Args: + change: The Change object. + cl: The Changelist object. + repository_root: The repository root. + verbose: Prints debug info. + output_stream: A stream to write debug output to. + """ + presubmit_files = ListRelevantPresubmitFiles( + change.LocalPaths(), repository_root) + if not presubmit_files and verbose: + output_stream.write("Warning, no PRESUBMIT.py found.\n") + results = [] + executer = GetPostUploadExecuter() + # The root presubmit file should be executed after the ones in subdirectories. + # i.e. the specific post upload hooks should run before the general ones. + # Thus, reverse the order provided by ListRelevantPresubmitFiles. + presubmit_files.reverse() + + for filename in presubmit_files: + filename = os.path.abspath(filename) + if verbose: + output_stream.write("Running %s\n" % filename) + # Accept CRLF presubmit script. + presubmit_script = gclient_utils.FileRead(filename, 'rU') + results.extend(executer.ExecPresubmitScript( + presubmit_script, filename, cl, change)) + output_stream.write('\n') + if results: + output_stream.write('** Post Upload Hook Messages **\n') + for result in results: + result.handle(output_stream) + output_stream.write('\n') + + return results + + class PresubmitExecuter(object): def __init__(self, change, committing, rietveld_obj, verbose): """ diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 94cd662637..ed29824002 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -221,6 +221,7 @@ class TestGitCl(TestCase): ((['git', 'symbolic-ref', 'HEAD'],), 'hash'), ((['git', 'config', 'branch.hash.last-upload-hash', 'hash'],), ''), + ((['git', 'config', 'rietveld.run-post-upload-hook'],), ''), ] @staticmethod @@ -731,6 +732,8 @@ class TestGitCl(TestCase): 'rietveld.project'],), ''), ((['git', 'config', '--unset-all', 'rietveld.pending-ref-prefix'],), ''), + ((['git', 'config', '--unset-all', + 'rietveld.run-post-upload-hook'],), ''), ((['git', 'config', 'gerrit.host', 'gerrit.chromium.org'],), ''), # DownloadHooks(False) @@ -758,6 +761,8 @@ class TestGitCl(TestCase): # DownloadHooks(True) ((['git', 'config', 'rietveld.bug-prefix'],), ''), (('Bug Prefix:',), ''), + ((['git', 'config', 'rietveld.run-post-upload-hook'],), ''), + (('Run Post Upload Hook:',), ''), ((commit_msg_path, os.X_OK,), True), ] git_cl.main(['config']) diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 4be5daffeb..a55b736f39 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -165,7 +165,8 @@ class PresubmitUnittest(PresubmitTestsBase): def testMembersChanged(self): self.mox.ReplayAll() members = [ - 'AffectedFile', 'Change', 'DoGetTrySlaves', 'DoPresubmitChecks', + 'AffectedFile', 'Change', 'DoGetTrySlaves', + 'DoPostUploadExecuter', 'DoPresubmitChecks', 'GetPostUploadExecuter', 'GetTrySlavesExecuter', 'GitAffectedFile', 'CallCommand', 'CommandData', 'GitChange', 'InputApi', 'ListRelevantPresubmitFiles', 'Main', 'NonexistantCannedCheckFilter', 'OutputApi', 'ParseFiles',