From c69d28f8f82f052ba8d6d2725197ba918172cc26 Mon Sep 17 00:00:00 2001 From: Sebastian Hengst Date: Tue, 31 Jan 2017 21:47:45 +0100 Subject: [PATCH] Backed out changeset d6648b8f36ed (bug 1333167) for breaking gecko decision task. r=backout on a CLOSED TREE --- taskcluster/ci/build/kind.yml | 2 - taskcluster/ci/test/kind.yml | 2 - taskcluster/docs/loading.rst | 9 -- taskcluster/taskgraph/generator.py | 11 +- taskcluster/taskgraph/target_tasks.py | 32 +--- .../taskgraph/test/test_target_tasks.py | 5 - .../taskgraph/test/test_try_option_syntax.py | 37 ----- taskcluster/taskgraph/transforms/build.py | 12 -- taskcluster/taskgraph/transforms/tests.py | 19 --- taskcluster/taskgraph/try_option_syntax.py | 146 +++++++----------- 10 files changed, 64 insertions(+), 211 deletions(-) diff --git a/taskcluster/ci/build/kind.yml b/taskcluster/ci/build/kind.yml index 232d09003f59..83d3a73d71c3 100644 --- a/taskcluster/ci/build/kind.yml +++ b/taskcluster/ci/build/kind.yml @@ -15,5 +15,3 @@ jobs-from: - linux.yml - macosx.yml - windows.yml - -parse-commit: taskgraph.try_option_syntax:parse_message diff --git a/taskcluster/ci/test/kind.yml b/taskcluster/ci/test/kind.yml index 1bc3a4366d6c..31348b994d50 100644 --- a/taskcluster/ci/test/kind.yml +++ b/taskcluster/ci/test/kind.yml @@ -6,5 +6,3 @@ kind-dependencies: transforms: - taskgraph.transforms.tests:transforms - taskgraph.transforms.task:transforms - -parse-commit: taskgraph.try_option_syntax:parse_message diff --git a/taskcluster/docs/loading.rst b/taskcluster/docs/loading.rst index 9c92f1182a51..1fa3c50f1ecb 100644 --- a/taskcluster/docs/loading.rst +++ b/taskcluster/docs/loading.rst @@ -29,12 +29,3 @@ on tasks that already exist. You can see a nice example of this behavior in For more information on how all of this works, consult the docstrings and comments in the source code itself. - -Try option syntax ------------------ - -The ``parse-commit`` optional field specified in ``kind.yml`` links to a -function to parse the command line options in the ``--message`` mach parameter. -Currently, the only valid value is ``taskgraph.try_option_syntax:parse_message``. -The parsed arguments are stored in ``config.config['args']``, it corresponds -to the same object returned by ``parse_args`` from ``argparse`` Python module. diff --git a/taskcluster/taskgraph/generator.py b/taskcluster/taskgraph/generator.py index 5748115e2474..7dd7596d93be 100644 --- a/taskcluster/taskgraph/generator.py +++ b/taskcluster/taskgraph/generator.py @@ -6,7 +6,6 @@ from __future__ import absolute_import, print_function, unicode_literals import logging import os import yaml -import copy from . import filter_tasks from .graph import Graph @@ -39,15 +38,7 @@ class Kind(object): def load_tasks(self, parameters, loaded_tasks): impl_class = self._get_impl_class() - config = copy.deepcopy(self.config) - - if 'parse-commit' in self.config: - parse_commit = find_object(config['parse-commit']) - config['args'] = parse_commit(parameters['message']) - else: - config['args'] = None - - return impl_class.load_tasks(self.name, self.path, config, + return impl_class.load_tasks(self.name, self.path, self.config, parameters, loaded_tasks) diff --git a/taskcluster/taskgraph/target_tasks.py b/taskcluster/taskgraph/target_tasks.py index 55a5328b3d95..40401c854674 100644 --- a/taskcluster/taskgraph/target_tasks.py +++ b/taskcluster/taskgraph/target_tasks.py @@ -42,32 +42,12 @@ def target_tasks_try_option_syntax(full_task_graph, parameters): target_tasks_labels = [t.label for t in full_task_graph.tasks.itervalues() if options.task_matches(t.attributes)] - attributes = { - k: getattr(options, k) for k in [ - 'env', - 'no_retry', - 'tag', - ] - } - - for l in target_tasks_labels: - task = full_task_graph[l] - if 'unittest_suite' in task.attributes: - task.attributes['task_duplicates'] = options.trigger_tests - - for l in target_tasks_labels: - task = full_task_graph[l] - # If the developer wants test jobs to be rebuilt N times we add that value here - if options.trigger_tests > 1 and 'unittest_suite' in task.attributes: - task.attributes['task_duplicates'] = options.trigger_tests - task.attributes['profile'] = False - - # If the developer wants test talos jobs to be rebuilt N times we add that value here - if options.talos_trigger_tests > 1 and 'talos_suite' in task.attributes: - task.attributes['task_duplicates'] = options.talos_trigger_tests - task.attributes['profile'] = options.profile - - task.attributes.update(attributes) + # If the developer wants test jobs to be rebuilt N times we add that value here + if int(options.trigger_tests) > 1: + for l in target_tasks_labels: + task = full_task_graph[l] + if 'unittest_suite' in task.attributes: + task.attributes['task_duplicates'] = options.trigger_tests # Add notifications here as well if options.notifications: diff --git a/taskcluster/taskgraph/test/test_target_tasks.py b/taskcluster/taskgraph/test/test_target_tasks.py index b112fb38abfa..035ccefd83f1 100644 --- a/taskcluster/taskgraph/test/test_target_tasks.py +++ b/taskcluster/taskgraph/test/test_target_tasks.py @@ -18,12 +18,7 @@ class FakeTryOptionSyntax(object): def __init__(self, message, task_graph): self.trigger_tests = 0 - self.talos_trigger_tests = 0 self.notifications = None - self.env = [] - self.profile = False - self.tag = None - self.no_retry = False def task_matches(self, attributes): return 'at-at' in attributes diff --git a/taskcluster/taskgraph/test/test_try_option_syntax.py b/taskcluster/taskgraph/test/test_try_option_syntax.py index 7ee985a7863a..59134619f850 100644 --- a/taskcluster/taskgraph/test/test_try_option_syntax.py +++ b/taskcluster/taskgraph/test/test_try_option_syntax.py @@ -56,12 +56,6 @@ class TestTryOptionSyntax(unittest.TestCase): self.assertEqual(tos.unittests, []) self.assertEqual(tos.talos, []) self.assertEqual(tos.platforms, []) - self.assertEqual(tos.trigger_tests, 0) - self.assertEqual(tos.talos_trigger_tests, 0) - self.assertEqual(tos.env, []) - self.assertFalse(tos.profile) - self.assertIsNone(tos.tag) - self.assertFalse(tos.no_retry) def test_message_without_try(self): "Given a non-try message, it should return an empty value" @@ -71,12 +65,6 @@ class TestTryOptionSyntax(unittest.TestCase): self.assertEqual(tos.unittests, []) self.assertEqual(tos.talos, []) self.assertEqual(tos.platforms, []) - self.assertEqual(tos.trigger_tests, 0) - self.assertEqual(tos.talos_trigger_tests, 0) - self.assertEqual(tos.env, []) - self.assertFalse(tos.profile) - self.assertIsNone(tos.tag) - self.assertFalse(tos.no_retry) def test_unknown_args(self): "unknown arguments are ignored" @@ -258,11 +246,6 @@ class TestTryOptionSyntax(unittest.TestCase): tos = TryOptionSyntax('try: --rebuild 10', empty_graph) self.assertEqual(tos.trigger_tests, 10) - def test_talos_trigger_tests(self): - "--rebuild-talos 10 sets talos_trigger_tests" - tos = TryOptionSyntax('try: --rebuild-talos 10', empty_graph) - self.assertEqual(tos.talos_trigger_tests, 10) - def test_interactive(self): "--interactive sets interactive" tos = TryOptionSyntax('try: --interactive', empty_graph) @@ -283,25 +266,5 @@ class TestTryOptionSyntax(unittest.TestCase): tos = TryOptionSyntax('try:', empty_graph) self.assertEqual(tos.notifications, None) - def test_setenv(self): - "--setenv VAR=value adds a environment variables setting to env" - tos = TryOptionSyntax('try: --setenv VAR1=value1 --setenv VAR2=value2', empty_graph) - self.assertEqual(tos.env, ['VAR1=value1', 'VAR2=value2']) - - def test_profile(self): - "--spsProfile sets profile to true" - tos = TryOptionSyntax('try: --spsProfile', empty_graph) - self.assertTrue(tos.profile) - - def test_tag(self): - "--tag TAG sets tag to TAG value" - tos = TryOptionSyntax('try: --tag tagName', empty_graph) - self.assertEqual(tos.tag, 'tagName') - - def test_no_retry(self): - "--no-retry sets no_retry to true" - tos = TryOptionSyntax('try: --no-retry', empty_graph) - self.assertTrue(tos.no_retry) - if __name__ == '__main__': main() diff --git a/taskcluster/taskgraph/transforms/build.py b/taskcluster/taskgraph/transforms/build.py index 9ff9c5dc6838..0cf79003e61b 100644 --- a/taskcluster/taskgraph/transforms/build.py +++ b/taskcluster/taskgraph/transforms/build.py @@ -29,16 +29,4 @@ def set_defaults(config, jobs): job['extra']['chainOfTrust']['inputs']['docker-image'] = { "task-reference": "" } - job['worker'].setdefault('env', {}) - yield job - - -@transforms.add -def set_env(config, jobs): - """Set extra environment variables from try command line.""" - for job in jobs: - env = config.config['args'].env - if env: - job_env = job['worker']['env'] - job_env.update(dict(x.split('=') for x in env)) yield job diff --git a/taskcluster/taskgraph/transforms/tests.py b/taskcluster/taskgraph/transforms/tests.py index c211f5bb57e8..c97975076b29 100644 --- a/taskcluster/taskgraph/transforms/tests.py +++ b/taskcluster/taskgraph/transforms/tests.py @@ -567,25 +567,6 @@ def set_retry_exit_status(config, tests): yield test -@transforms.add -def set_profile(config, tests): - """Set profiling mode for tests.""" - for test in tests: - if config.config['args'].profile and test['suite'] == 'talos': - test['mozharness']['extra-options'].append('--spsProfile') - yield test - - -@transforms.add -def set_tag(config, tests): - """Set test for a specific tag.""" - for test in tests: - tag = config.config['args'].tag - if tag: - test['mozharness']['extra-options'].extend(['--tag', tag]) - yield test - - @transforms.add def remove_linux_pgo_try_talos(config, tests): """linux64-pgo talos tests don't run on try.""" diff --git a/taskcluster/taskgraph/try_option_syntax.py b/taskcluster/taskgraph/try_option_syntax.py index 1d50745fbe2f..3e3f0020171c 100644 --- a/taskcluster/taskgraph/try_option_syntax.py +++ b/taskcluster/taskgraph/try_option_syntax.py @@ -172,73 +172,6 @@ RIDEALONG_BUILDS = { TEST_CHUNK_SUFFIX = re.compile('(.*)-([0-9]+)$') -def escape_whitespace_in_brackets(input_str): - ''' - In tests you may restrict them by platform [] inside of the brackets - whitespace may occur this is typically invalid shell syntax so we escape it - with backslash sequences . - ''' - result = "" - in_brackets = False - for char in input_str: - if char == '[': - in_brackets = True - result += char - continue - - if char == ']': - in_brackets = False - result += char - continue - - if char == ' ' and in_brackets: - result += '\ ' - continue - - result += char - - return result - - -def parse_message(message): - # shlex used to ensure we split correctly when giving values to argparse. - parts = shlex.split(escape_whitespace_in_brackets(message)) - try_idx = None - for idx, part in enumerate(parts): - if part == TRY_DELIMITER: - try_idx = idx - break - - if try_idx is None: - return None - - # Argument parser based on try flag flags - parser = argparse.ArgumentParser() - parser.add_argument('-b', '--build', dest='build_types') - parser.add_argument('-p', '--platform', nargs='?', - dest='platforms', const='all', default='all') - parser.add_argument('-u', '--unittests', nargs='?', - dest='unittests', const='all', default='all') - parser.add_argument('-t', '--talos', nargs='?', dest='talos', const='all', default='none') - parser.add_argument('-i', '--interactive', - dest='interactive', action='store_true', default=False) - parser.add_argument('-e', '--all-emails', - dest='notifications', action='store_const', const='all') - parser.add_argument('-f', '--failure-emails', - dest='notifications', action='store_const', const='failure') - parser.add_argument('-j', '--job', dest='jobs', action='append') - parser.add_argument('--rebuild-talos', dest='talos_trigger_tests', action='store', - type=int, default=1) - parser.add_argument('--setenv', dest='env', action='append') - parser.add_argument('--spsProfile', dest='profile', action='store_true') - parser.add_argument('--tag', dest='tag', action='store', default=None) - parser.add_argument('--no-retry', dest='no_retry', action='store_true') - # In order to run test jobs multiple times - parser.add_argument('--rebuild', dest='trigger_tests', type=int, default=1) - args, _ = parser.parse_known_args(parts[try_idx:]) - return args - - class TryOptionSyntax(object): def __init__(self, message, full_task_graph): @@ -256,11 +189,8 @@ class TryOptionSyntax(object): - trigger_tests: the number of times tests should be triggered (--rebuild) - interactive: true if --interactive - notifications: either None if no notifications or one of 'all' or 'failure' - - talos_trigger_tests: the number of time talos tests should be triggered (--rebuild-talos) - - env: additional environment variables (ENV=value) - - profile: run talos in profile mode - - tag: restrict tests to the specified tag - - no_retry: do not retry failed jobs + + Note that -t is currently completely ignored. The unittests and talos lists contain dictionaries of the form: @@ -278,16 +208,37 @@ class TryOptionSyntax(object): self.trigger_tests = 0 self.interactive = False self.notifications = None - self.talos_trigger_tests = 0 - self.env = [] - self.profile = False - self.tag = None - self.no_retry = False - args = parse_message(message) - if args is None: + # shlex used to ensure we split correctly when giving values to argparse. + parts = shlex.split(self.escape_whitespace_in_brackets(message)) + try_idx = None + for idx, part in enumerate(parts): + if part == TRY_DELIMITER: + try_idx = idx + break + + if try_idx is None: return + # Argument parser based on try flag flags + parser = argparse.ArgumentParser() + parser.add_argument('-b', '--build', dest='build_types') + parser.add_argument('-p', '--platform', nargs='?', + dest='platforms', const='all', default='all') + parser.add_argument('-u', '--unittests', nargs='?', + dest='unittests', const='all', default='all') + parser.add_argument('-t', '--talos', nargs='?', dest='talos', default='none') + parser.add_argument('-i', '--interactive', + dest='interactive', action='store_true', default=False) + parser.add_argument('-e', '--all-emails', + dest='notifications', action='store_const', const='all') + parser.add_argument('-f', '--failure-emails', + dest='notifications', action='store_const', const='failure') + parser.add_argument('-j', '--job', dest='jobs', action='append') + # In order to run test jobs multiple times + parser.add_argument('--rebuild', dest='trigger_tests', type=int, default=1) + args, _ = parser.parse_known_args(parts[try_idx:]) + self.jobs = self.parse_jobs(args.jobs) self.build_types = self.parse_build_types(args.build_types) self.platforms = self.parse_platforms(args.platforms) @@ -297,11 +248,6 @@ class TryOptionSyntax(object): self.trigger_tests = args.trigger_tests self.interactive = args.interactive self.notifications = args.notifications - self.talos_trigger_tests = args.talos_trigger_tests - self.env = args.env - self.profile = args.profile - self.tag = args.tag - self.no_retry = args.no_retry def parse_jobs(self, jobs_arg): if not jobs_arg or jobs_arg == ['all']: @@ -521,6 +467,33 @@ class TryOptionSyntax(object): rv.add(a[len(prefix):]) return sorted(rv) + def escape_whitespace_in_brackets(self, input_str): + ''' + In tests you may restrict them by platform [] inside of the brackets + whitespace may occur this is typically invalid shell syntax so we escape it + with backslash sequences . + ''' + result = "" + in_brackets = False + for char in input_str: + if char == '[': + in_brackets = True + result += char + continue + + if char == ']': + in_brackets = False + result += char + continue + + if char == ' ' and in_brackets: + result += '\ ' + continue + + result += char + + return result + def task_matches(self, attributes): attr = attributes.get @@ -587,9 +560,4 @@ class TryOptionSyntax(object): "trigger_tests: " + str(self.trigger_tests), "interactive: " + str(self.interactive), "notifications: " + str(self.notifications), - "talos_trigger_tests: " + str(self.talos_trigger_tests), - "env: " + str(self.env), - "profile: " + str(self.profile), - "tag: " + str(self.tag), - "no_retry: " + str(self.no_retry), ])