From 456ca7f0c2a5692a517418343f40bedb151d2a75 Mon Sep 17 00:00:00 2001 From: "martiniss@chromium.org" Date: Mon, 23 May 2016 21:33:28 +0000 Subject: [PATCH] tryserver recipe_module: Add get_tags. Lets you get CL tags for a given CL. BUG=591172 Review-Url: https://codereview.chromium.org/1915833003 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@300658 0039d316-1c4b-4281-b951-d872f2087c98 --- git_footers.py | 14 ++++- .../depot_tools/example.expected/basic.json | 42 +++++++++++++ .../depot_tools/example.expected/win.json | 42 +++++++++++++ recipe_modules/depot_tools/example.py | 34 +++++++++++ recipe_modules/git_cl/api.py | 28 +++++++-- .../git_cl/example.expected/basic.json | 36 ++++++++--- recipe_modules/git_cl/example.py | 16 ++--- recipe_modules/tryserver/__init__.py | 1 + recipe_modules/tryserver/api.py | 31 ++++++++++ .../example.expected/basic_tags.json | 59 +++++++++++++++++++ .../example.expected/with_rietveld_patch.json | 26 ++++++++ .../with_rietveld_patch_new.json | 26 ++++++++ recipe_modules/tryserver/example.py | 35 ++++++++++- tests/gclient_test.py | 6 +- tests/git_footers_test.py | 16 ++++- 15 files changed, 381 insertions(+), 31 deletions(-) create mode 100644 recipe_modules/depot_tools/example.expected/basic.json create mode 100644 recipe_modules/depot_tools/example.expected/win.json create mode 100644 recipe_modules/depot_tools/example.py create mode 100644 recipe_modules/tryserver/example.expected/basic_tags.json diff --git a/git_footers.py b/git_footers.py index 4641f7470..c5f69b013 100755 --- a/git_footers.py +++ b/git_footers.py @@ -4,6 +4,7 @@ # found in the LICENSE file. import argparse +import json import re import sys @@ -163,7 +164,8 @@ def main(args): parser = argparse.ArgumentParser( formatter_class=argparse.ArgumentDefaultsHelpFormatter ) - parser.add_argument('ref') + parser.add_argument('ref', nargs='?', help="Git ref to retrieve footers from." + " Omit to parse stdin.") g = parser.add_mutually_exclusive_group() g.add_argument('--key', metavar='KEY', @@ -172,11 +174,16 @@ def main(args): g.add_argument('--position', action='store_true') g.add_argument('--position-ref', action='store_true') g.add_argument('--position-num', action='store_true') + g.add_argument('--json', help="filename to dump JSON serialized headers to.") opts = parser.parse_args(args) - message = git.run('log', '-1', '--format=%B', opts.ref) + if opts.ref: + message = git.run('log', '-1', '--format=%B', opts.ref) + else: + message = '\n'.join(l for l in sys.stdin) + footers = parse_footers(message) if opts.key: @@ -191,6 +198,9 @@ def main(args): pos = get_position(footers) assert pos[1], 'No valid position for commit' print pos[1] + elif opts.json: + with open(opts.json, 'w') as f: + json.dump(footers, f) else: for k in footers.keys(): for v in footers[k]: diff --git a/recipe_modules/depot_tools/example.expected/basic.json b/recipe_modules/depot_tools/example.expected/basic.json new file mode 100644 index 000000000..e8ad01fe6 --- /dev/null +++ b/recipe_modules/depot_tools/example.expected/basic.json @@ -0,0 +1,42 @@ +[ + { + "cmd": [ + "ls", + "RECIPE_PACKAGE_REPO[depot_tools]/download_from_google_storage.py" + ], + "name": "download_from_google_storage" + }, + { + "cmd": [ + "ls", + "RECIPE_PACKAGE_REPO[depot_tools]/cros" + ], + "name": "cros" + }, + { + "cmd": [ + "ls", + "RECIPE_PACKAGE_REPO[depot_tools]/gn.py" + ], + "name": "gn_py_path" + }, + { + "cmd": [ + "ls", + "RECIPE_PACKAGE_REPO[depot_tools]/gsutil.py" + ], + "name": "gsutil_py_path" + }, + { + "cmd": [ + "ls", + "RECIPE_PACKAGE_REPO[depot_tools]/ninja" + ], + "name": "ninja_path" + }, + { + "name": "$result", + "recipe_result": null, + "status_code": 0 + } +] \ No newline at end of file diff --git a/recipe_modules/depot_tools/example.expected/win.json b/recipe_modules/depot_tools/example.expected/win.json new file mode 100644 index 000000000..3c47a3257 --- /dev/null +++ b/recipe_modules/depot_tools/example.expected/win.json @@ -0,0 +1,42 @@ +[ + { + "cmd": [ + "ls", + "RECIPE_PACKAGE_REPO[depot_tools]\\download_from_google_storage.py" + ], + "name": "download_from_google_storage" + }, + { + "cmd": [ + "ls", + "RECIPE_PACKAGE_REPO[depot_tools]\\cros" + ], + "name": "cros" + }, + { + "cmd": [ + "ls", + "RECIPE_PACKAGE_REPO[depot_tools]\\gn.py" + ], + "name": "gn_py_path" + }, + { + "cmd": [ + "ls", + "RECIPE_PACKAGE_REPO[depot_tools]\\gsutil.py" + ], + "name": "gsutil_py_path" + }, + { + "cmd": [ + "ls", + "RECIPE_PACKAGE_REPO[depot_tools]\\ninja.exe" + ], + "name": "ninja_path" + }, + { + "name": "$result", + "recipe_result": null, + "status_code": 0 + } +] \ No newline at end of file diff --git a/recipe_modules/depot_tools/example.py b/recipe_modules/depot_tools/example.py new file mode 100644 index 000000000..e61785f10 --- /dev/null +++ b/recipe_modules/depot_tools/example.py @@ -0,0 +1,34 @@ +# Copyright 2016 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +DEPS = [ + 'depot_tools', + 'recipe_engine/step', + 'recipe_engine/path', + 'recipe_engine/platform', +] + + +def RunSteps(api): + api.step( + 'download_from_google_storage', + ['ls', api.depot_tools.download_from_google_storage_path]) + + api.step('cros', ['ls', api.depot_tools.cros_path]) + + api.step( + 'gn_py_path', ['ls', api.depot_tools.gn_py_path]) + + api.step( + 'gsutil_py_path', ['ls', api.depot_tools.gsutil_py_path]) + + api.step( + 'ninja_path', ['ls', api.depot_tools.ninja_path]) + + + +def GenTests(api): + yield api.test('basic') + + yield api.test('win') + api.platform('win', 32) diff --git a/recipe_modules/git_cl/api.py b/recipe_modules/git_cl/api.py index 0cb1f53e3..8d2ad5c0d 100644 --- a/recipe_modules/git_cl/api.py +++ b/recipe_modules/git_cl/api.py @@ -8,18 +8,34 @@ class GitClApi(recipe_api.RecipeApi): def __call__(self, subcmd, args, name=None, **kwargs): if not name: name = 'git_cl ' + subcmd + + if kwargs.get('suffix'): + name = name + ' (%s)' % kwargs.pop('suffix') + if 'cwd' not in kwargs: kwargs['cwd'] = (self.c and self.c.repo_location) or None return self.m.step( - name, [self.package_repo_resource('git_cl.py')] + args, **kwargs) + name, [self.package_repo_resource('git_cl.py'), subcmd] + args, + **kwargs) + + def get_description(self, patch=None, codereview=None, **kwargs): + args = ['-d'] + if patch or codereview: + assert patch and codereview, "Both patch and codereview must be provided" + args.append('--%s' % codereview) + args.append(patch) + + return self('description', args, stdout=self.m.raw_io.output(), **kwargs) - def get_description(self, **kwargs): - return self('description', ['-d'], stdout=self.m.raw_io.output(), **kwargs) + def set_description(self, description, patch=None, codereview=None, **kwargs): + args = ['-n', '-'] + if patch or codereview: + assert patch and codereview, "Both patch and codereview must be provided" + args.append(patch) + args.append('--%s' % codereview) - def set_description(self, description, **kwargs): return self( - 'description', ['-n', '-'], - stdout=self.m.raw_io.output(), + 'description', args, stdout=self.m.raw_io.output(), stdin=self.m.raw_io.input(data=description), name='git_cl set description', **kwargs) diff --git a/recipe_modules/git_cl/example.expected/basic.json b/recipe_modules/git_cl/example.expected/basic.json index e85b5dec3..43ed4c867 100644 --- a/recipe_modules/git_cl/example.expected/basic.json +++ b/recipe_modules/git_cl/example.expected/basic.json @@ -2,10 +2,25 @@ { "cmd": [ "RECIPE_PACKAGE_REPO[depot_tools]/git_cl.py", - "-d" + "description", + "-d", + "--rietveld", + "https://code.review/123" ], - "cwd": "[TMP_BASE]/fakee_tmp_1", - "name": "git_cl description", + "name": "git_cl description (build)", + "stdout": "/path/to/tmp/" + }, + { + "cmd": [ + "RECIPE_PACKAGE_REPO[depot_tools]/git_cl.py", + "description", + "-n", + "-", + "https://code.review/123", + "--rietveld" + ], + "name": "git_cl set description", + "stdin": "bammmm", "stdout": "/path/to/tmp/" }, { @@ -18,10 +33,11 @@ { "cmd": [ "RECIPE_PACKAGE_REPO[depot_tools]/git_cl.py", + "description", "-d" ], - "cwd": "[TMP_BASE]/fakerepo_tmp_2", - "name": "git_cl description (2)", + "cwd": "[TMP_BASE]/fakerepo_tmp_1", + "name": "git_cl description", "stdout": "/path/to/tmp/" }, { @@ -34,21 +50,23 @@ { "cmd": [ "RECIPE_PACKAGE_REPO[depot_tools]/git_cl.py", + "description", "-n", "-" ], - "cwd": "[TMP_BASE]/fakerepo_tmp_2", - "name": "git_cl set description", + "cwd": "[TMP_BASE]/fakerepo_tmp_1", + "name": "git_cl set description (2)", "stdin": "new description woo", "stdout": "/path/to/tmp/" }, { "cmd": [ "RECIPE_PACKAGE_REPO[depot_tools]/git_cl.py", + "description", "-d" ], - "cwd": "[TMP_BASE]/fakerepo_tmp_2", - "name": "git_cl description (3)", + "cwd": "[TMP_BASE]/fakerepo_tmp_1", + "name": "git_cl description (2)", "stdout": "/path/to/tmp/" }, { diff --git a/recipe_modules/git_cl/example.py b/recipe_modules/git_cl/example.py index cf1f002f0..c76eac954 100644 --- a/recipe_modules/git_cl/example.py +++ b/recipe_modules/git_cl/example.py @@ -14,16 +14,18 @@ DEPS = [ def RunSteps(api): - res = api.git_cl.get_description(cwd=api.path.mkdtemp('fakee')) - # Look ma, no hands! (Can pass in the cwd without configuring git_cl). - api.step('echo', ['echo', res.stdout]) + result = api.git_cl.get_description( + patch='https://code.review/123', codereview='rietveld', suffix='build') + api.git_cl.set_description( + 'bammmm', patch='https://code.review/123', codereview='rietveld') + api.step('echo', ['echo', result.stdout]) api.git_cl.set_config('basic') api.git_cl.c.repo_location = api.path.mkdtemp('fakerepo') api.step('echo', ['echo', api.git_cl.get_description().stdout]) - api.git_cl.set_description("new description woo") + api.git_cl.set_description('new description woo') api.step('echo', ['echo', api.git_cl.get_description().stdout]) @@ -31,11 +33,11 @@ def GenTests(api): yield ( api.test('basic') + api.override_step_data( - 'git_cl description', stdout=api.raw_io.output('hi')) + + 'git_cl description (build)', stdout=api.raw_io.output('hi')) + api.override_step_data( - 'git_cl description (2)', stdout=api.raw_io.output('hey')) + + 'git_cl description', stdout=api.raw_io.output('hey')) + api.override_step_data( - 'git_cl description (3)', stdout=api.raw_io.output( + 'git_cl description (2)', stdout=api.raw_io.output( 'new description woo')) ) diff --git a/recipe_modules/tryserver/__init__.py b/recipe_modules/tryserver/__init__.py index 53957cdc2..76c8fb8c8 100644 --- a/recipe_modules/tryserver/__init__.py +++ b/recipe_modules/tryserver/__init__.py @@ -4,6 +4,7 @@ DEPS = [ 'git', + 'git_cl', 'recipe_engine/json', 'recipe_engine/path', 'recipe_engine/platform', diff --git a/recipe_modules/tryserver/api.py b/recipe_modules/tryserver/api.py index 87d12e491..d1d290d6f 100644 --- a/recipe_modules/tryserver/api.py +++ b/recipe_modules/tryserver/api.py @@ -2,6 +2,7 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. +import collections import contextlib import hashlib @@ -278,3 +279,33 @@ class TryserverApi(recipe_api.RecipeApi): failure_hash.hexdigest() raise + + def get_footers(self, patch_text=None): + """Retrieves footers from the patch description. + + footers are machine readable tags embedded in commit messages. See + git-footers documentation for more information. + """ + if patch_text is None: + codereview = None + if not self.can_apply_issue: #pragma: no cover + raise recipe_api.StepFailure("Cannot get tags from gerrit yet.") + else: + codereview = 'rietveld' + patch = ( + self.m.properties['rietveld'].strip('/') + '/' + + str(self.m.properties['issue'])) + + patch_text = self.m.git_cl.get_description( + patch=patch, codereview=codereview).stdout + + result = self.m.python( + 'parse description', self.package_repo_resource('git_footers.py'), + args=['--json', self.m.json.output()], + stdin=self.m.raw_io.input(data=patch_text)) + return result.json.output + + def get_footer(self, tag, patch_text=None): + """Gets a specific tag from a CL description""" + return self.get_footers(patch_text).get(tag, []) + diff --git a/recipe_modules/tryserver/example.expected/basic_tags.json b/recipe_modules/tryserver/example.expected/basic_tags.json new file mode 100644 index 000000000..9ca4ee365 --- /dev/null +++ b/recipe_modules/tryserver/example.expected/basic_tags.json @@ -0,0 +1,59 @@ +[ + { + "cmd": [ + "python", + "-u", + "RECIPE_PACKAGE_REPO[depot_tools]/git_footers.py", + "--json", + "/path/to/tmp/json" + ], + "name": "parse description", + "stdin": "hihihi\nfoo:bar\nbam:baz", + "~followup_annotations": [ + "@@@STEP_LOG_LINE@json.output@{@@@", + "@@@STEP_LOG_LINE@json.output@ \"Foo\": [@@@", + "@@@STEP_LOG_LINE@json.output@ \"bar\"@@@", + "@@@STEP_LOG_LINE@json.output@ ]@@@", + "@@@STEP_LOG_LINE@json.output@}@@@", + "@@@STEP_LOG_END@json.output@@@" + ] + }, + { + "cmd": [ + "echo", + "OrderedDict([('Foo', ['bar'])])" + ], + "name": "patch_text test" + }, + { + "cmd": [ + "python", + "-u", + "RECIPE_PACKAGE_REPO[depot_tools]/git_footers.py", + "--json", + "/path/to/tmp/json" + ], + "name": "parse description (2)", + "stdin": "hihihi\nfoo:bar\nbam:baz", + "~followup_annotations": [ + "@@@STEP_LOG_LINE@json.output@{@@@", + "@@@STEP_LOG_LINE@json.output@ \"Foo\": [@@@", + "@@@STEP_LOG_LINE@json.output@ \"bar\"@@@", + "@@@STEP_LOG_LINE@json.output@ ]@@@", + "@@@STEP_LOG_LINE@json.output@}@@@", + "@@@STEP_LOG_END@json.output@@@" + ] + }, + { + "cmd": [ + "echo", + "['bar']" + ], + "name": "patch_text test (2)" + }, + { + "name": "$result", + "recipe_result": null, + "status_code": 0 + } +] \ No newline at end of file diff --git a/recipe_modules/tryserver/example.expected/with_rietveld_patch.json b/recipe_modules/tryserver/example.expected/with_rietveld_patch.json index 4c3bb4acb..1d0b488c0 100644 --- a/recipe_modules/tryserver/example.expected/with_rietveld_patch.json +++ b/recipe_modules/tryserver/example.expected/with_rietveld_patch.json @@ -19,6 +19,32 @@ "@@@STEP_LINK@Applied issue 12853011@https://codereview.chromium.org/12853011@@@" ] }, + { + "cmd": [ + "RECIPE_PACKAGE_REPO[depot_tools]/git_cl.py", + "description", + "-d", + "--rietveld", + "https://codereview.chromium.org/12853011" + ], + "name": "git_cl description", + "stdout": "/path/to/tmp/" + }, + { + "cmd": [ + "python", + "-u", + "RECIPE_PACKAGE_REPO[depot_tools]/git_footers.py", + "--json", + "/path/to/tmp/json" + ], + "name": "parse description", + "stdin": "foobar", + "~followup_annotations": [ + "@@@STEP_LOG_LINE@json.output (invalid)@null@@@", + "@@@STEP_LOG_END@json.output (invalid)@@@" + ] + }, { "cmd": [ "git", diff --git a/recipe_modules/tryserver/example.expected/with_rietveld_patch_new.json b/recipe_modules/tryserver/example.expected/with_rietveld_patch_new.json index b990f6cc2..baaf209bd 100644 --- a/recipe_modules/tryserver/example.expected/with_rietveld_patch_new.json +++ b/recipe_modules/tryserver/example.expected/with_rietveld_patch_new.json @@ -19,6 +19,32 @@ "@@@STEP_LINK@Applied issue 12853011@https://codereview.chromium.org/12853011@@@" ] }, + { + "cmd": [ + "RECIPE_PACKAGE_REPO[depot_tools]/git_cl.py", + "description", + "-d", + "--rietveld", + "https://codereview.chromium.org/12853011" + ], + "name": "git_cl description", + "stdout": "/path/to/tmp/" + }, + { + "cmd": [ + "python", + "-u", + "RECIPE_PACKAGE_REPO[depot_tools]/git_footers.py", + "--json", + "/path/to/tmp/json" + ], + "name": "parse description", + "stdin": "foobar", + "~followup_annotations": [ + "@@@STEP_LOG_LINE@json.output (invalid)@null@@@", + "@@@STEP_LOG_END@json.output (invalid)@@@" + ] + }, { "cmd": [ "git", diff --git a/recipe_modules/tryserver/example.py b/recipe_modules/tryserver/example.py index c1bddc461..96b9e0bfa 100644 --- a/recipe_modules/tryserver/example.py +++ b/recipe_modules/tryserver/example.py @@ -3,17 +3,30 @@ # found in the LICENSE file. DEPS = [ + 'recipe_engine/json', + 'recipe_engine/raw_io', 'recipe_engine/path', 'recipe_engine/platform', 'recipe_engine/properties', 'recipe_engine/python', + 'recipe_engine/step', 'tryserver', ] def RunSteps(api): api.path['checkout'] = api.path['slave_build'] + if api.properties.get('patch_text'): + api.step('patch_text test', [ + 'echo', str(api.tryserver.get_footers(api.properties['patch_text']))]) + api.step('patch_text test', [ + 'echo', str(api.tryserver.get_footer( + 'Foo', api.properties['patch_text']))]) + return + api.tryserver.maybe_apply_issue() + if api.tryserver.can_apply_issue: + api.tryserver.get_footers() api.tryserver.get_files_affected_by_patch( api.properties.get('test_patch_root')) @@ -30,6 +43,8 @@ def RunSteps(api): def GenTests(api): + description_step = api.override_step_data( + 'git_cl description', stdout=api.raw_io.output('foobar')) yield (api.test('with_svn_patch') + api.properties(patch_url='svn://checkout.url')) @@ -41,13 +56,27 @@ def GenTests(api): patch_ref='johndoe#123.diff')) yield (api.test('with_rietveld_patch') + - api.properties.tryserver()) + api.properties.tryserver() + + description_step) yield (api.test('with_wrong_patch') + api.platform('win', 32)) - yield (api.test('with_rietveld_patch_new') + - api.properties.tryserver(test_patch_root='sub/project')) + api.properties.tryserver(test_patch_root='sub/project') + + description_step) yield (api.test('with_wrong_patch_new') + api.platform('win', 32) + api.properties(test_patch_root='sub\\project')) + + yield (api.test('basic_tags') + + api.properties( + patch_text='hihihi\nfoo:bar\nbam:baz', + footer='foo' + ) + + api.step_data( + 'parse description', + api.json.output({'Foo': ['bar']})) + + api.step_data( + 'parse description (2)', + api.json.output({'Foo': ['bar']})) + ) diff --git a/tests/gclient_test.py b/tests/gclient_test.py index a7c31e268..65dd7327e 100755 --- a/tests/gclient_test.py +++ b/tests/gclient_test.py @@ -711,14 +711,14 @@ class GclientTest(trial_dir.TestCase): obj.RunOnDeps('None', []) self.assertEquals( [ - 'svn://example.com/foo', 'svn://example.com/bar', - 'svn://example.com/tar', + 'svn://example.com/foo', 'svn://example.com/foo/bar', 'svn://example.com/foo/bar/baz', 'svn://example.com/foo/bar/baz/fizz', + 'svn://example.com/tar', ], - self._get_processed()) + sorted(self._get_processed())) def testRecursedepsOverrideWithRelativePaths(self): """Verifies gclient respects |recursedeps| with relative paths.""" diff --git a/tests/git_footers_test.py b/tests/git_footers_test.py index c959949f9..23ca47363 100755 --- a/tests/git_footers_test.py +++ b/tests/git_footers_test.py @@ -3,14 +3,17 @@ """Tests for git_footers.""" import os +import StringIO import sys import unittest sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +from testing_support.auto_stub import TestCase + import git_footers -class GitFootersTest(unittest.TestCase): +class GitFootersTest(TestCase): _message = """ This is my commit message. There are many like it, but this one is mine. @@ -104,6 +107,17 @@ My commit message is my best friend. It is my life. I must master it. git_footers.add_footer_change_id('header: like footer', 'Ixxx'), 'header: like footer\n\nChange-Id: Ixxx') + def testReadStdin(self): + self.mock(git_footers.sys, 'stdin', StringIO.StringIO( + 'line\r\notherline\r\n\r\n\r\nFoo: baz')) + + stdout = StringIO.StringIO() + self.mock(git_footers.sys, 'stdout', stdout) + + self.assertEqual(git_footers.main([]), 0) + self.assertEqual(stdout.getvalue(), "Foo: baz\n") + + if __name__ == '__main__': unittest.main()