Backed out changeset 50cf83b69046 (bug 1333167) for breaking gecko decision task. r=backout
authorSebastian Hengst <archaeopteryx@coole-files.de>
Tue, 31 Jan 2017 18:25:50 +0100
changeset 380806 fdac8d6dee0904239665338d1808b868cdd50436
parent 380805 50cf83b690462913540effa87a27e8b5becd84a3
child 380807 b5acbffc8ebf1472c5e6d6f85acdf52e142b0d31
push id1468
push userasasaki@mozilla.com
push dateMon, 05 Jun 2017 19:31:07 +0000
treeherdermozilla-release@0641fc6ee9d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1333167
milestone54.0a1
backs out50cf83b690462913540effa87a27e8b5becd84a3
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out changeset 50cf83b69046 (bug 1333167) for breaking gecko decision task. r=backout
taskcluster/ci/build/kind.yml
taskcluster/ci/test/kind.yml
taskcluster/docs/loading.rst
taskcluster/taskgraph/generator.py
taskcluster/taskgraph/target_tasks.py
taskcluster/taskgraph/test/test_target_tasks.py
taskcluster/taskgraph/test/test_try_option_syntax.py
taskcluster/taskgraph/transforms/build.py
taskcluster/taskgraph/transforms/tests.py
taskcluster/taskgraph/try_option_syntax.py
--- a/taskcluster/ci/build/kind.yml
+++ b/taskcluster/ci/build/kind.yml
@@ -10,10 +10,8 @@ transforms:
    - taskgraph.transforms.job:transforms
    - taskgraph.transforms.task:transforms
 
 jobs-from:
     - android.yml
     - linux.yml
     - macosx.yml
     - windows.yml
-
-parse-commit: taskgraph.try_option_syntax:parse_message
--- a/taskcluster/ci/test/kind.yml
+++ b/taskcluster/ci/test/kind.yml
@@ -1,10 +1,8 @@
 implementation: taskgraph.task.test:TestTask
 
 kind-dependencies:
     - build
 
 transforms:
    - taskgraph.transforms.tests:transforms
    - taskgraph.transforms.task:transforms
-
-parse-commit: taskgraph.try_option_syntax:parse_message
--- a/taskcluster/docs/loading.rst
+++ b/taskcluster/docs/loading.rst
@@ -24,17 +24,8 @@ This is handled by the ``TransformTask``
 For kinds producing tasks that depend on other tasks -- for example, signing
 tasks depend on build tasks -- ``TransformTask`` has a ``get_inputs`` method
 that can be overridden in subclasses and written to return a set of items based
 on tasks that already exist.  You can see a nice example of this behavior in
 ``taskcluster/taskgraph/task/post_build.py``.
 
 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.
--- a/taskcluster/taskgraph/generator.py
+++ b/taskcluster/taskgraph/generator.py
@@ -1,17 +1,16 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 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
 from .taskgraph import TaskGraph
 from .optimize import optimize_task_graph
 from .util.python_path import find_object
 from .util.verify import (
     verify_docs,
@@ -34,25 +33,17 @@ class Kind(object):
         try:
             impl = self.config['implementation']
         except KeyError:
             raise KeyError("{!r} does not define implementation".format(self.path))
         return find_object(impl)
 
     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)
 
 
 class TaskGraphGenerator(object):
     """
     The central controller for taskgraph.  This handles all phases of graph
     generation.  The task is generated from all of the kinds defined in
     subdirectories of the generator's root directory.
--- a/taskcluster/taskgraph/target_tasks.py
+++ b/taskcluster/taskgraph/target_tasks.py
@@ -37,42 +37,22 @@ def get_method(method):
 @_target_task('try_option_syntax')
 def target_tasks_try_option_syntax(full_task_graph, parameters):
     """Generate a list of target tasks based on try syntax in
     parameters['message'] and, for context, the full task graph."""
     options = try_option_syntax.TryOptionSyntax(parameters['message'], full_task_graph)
     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:
         for task in full_task_graph:
             owner = parameters.get('owner')
             routes = task.task.setdefault('routes', [])
             if options.notifications == 'all':
                 routes.append("notify.email.{}.on-any".format(owner))
--- a/taskcluster/taskgraph/test/test_target_tasks.py
+++ b/taskcluster/taskgraph/test/test_target_tasks.py
@@ -13,22 +13,17 @@ from ..taskgraph import TaskGraph
 from .util import TestTask
 from mozunit import main
 
 
 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
 
 
 class TestTargetTasks(unittest.TestCase):
 
     def default_matches(self, run_on_projects, project):
--- a/taskcluster/taskgraph/test/test_try_option_syntax.py
+++ b/taskcluster/taskgraph/test/test_try_option_syntax.py
@@ -51,37 +51,25 @@ class TestTryOptionSyntax(unittest.TestC
     def test_empty_message(self):
         "Given an empty message, it should return an empty value"
         tos = TryOptionSyntax('', empty_graph)
         self.assertEqual(tos.build_types, [])
         self.assertEqual(tos.jobs, [])
         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"
         tos = TryOptionSyntax('Bug 1234: frobnicte the foo', empty_graph)
         self.assertEqual(tos.build_types, [])
         self.assertEqual(tos.jobs, [])
         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"
         tos = TryOptionSyntax('try: --doubledash -z extra', empty_graph)
         # equilvant to "try:"..
         self.assertEqual(tos.build_types, [])
         self.assertEqual(tos.jobs, None)
 
@@ -253,21 +241,16 @@ class TestTryOptionSyntax(unittest.TestC
 
     # -t shares an implementation with -u, so it's not tested heavily
 
     def test_trigger_tests(self):
         "--rebuild 10 sets trigger_tests"
         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)
         self.assertEqual(tos.interactive, True)
 
     def test_all_email(self):
         "--all-emails sets notifications"
         tos = TryOptionSyntax('try: --all-emails', empty_graph)
@@ -278,30 +261,10 @@ class TestTryOptionSyntax(unittest.TestC
         tos = TryOptionSyntax('try: --failure-emails', empty_graph)
         self.assertEqual(tos.notifications, 'failure')
 
     def test_no_email(self):
         "no email settings don't set notifications"
         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()
--- a/taskcluster/taskgraph/transforms/build.py
+++ b/taskcluster/taskgraph/transforms/build.py
@@ -24,21 +24,9 @@ def set_defaults(config, jobs):
             job['worker'].setdefault('docker-image', {'in-tree': 'desktop-build'})
             job['worker']['chain-of-trust'] = True
             job.setdefault('extra', {})
             job['extra'].setdefault('chainOfTrust', {})
             job['extra']['chainOfTrust'].setdefault('inputs', {})
             job['extra']['chainOfTrust']['inputs']['docker-image'] = {
                 "task-reference": "<docker-image>"
             }
-        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
--- a/taskcluster/taskgraph/transforms/tests.py
+++ b/taskcluster/taskgraph/transforms/tests.py
@@ -563,35 +563,16 @@ def set_retry_exit_status(config, tests)
     """Set the retry exit status to TBPL_RETRY, the value returned by mozharness
        scripts to indicate a transient failure that should be retried."""
     for test in tests:
         test['retry-exit-status'] = 4
         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."""
     def predicate(test):
         return not(
             test['test-platform'] == 'linux64-pgo/opt'
             and test['suite'] == 'talos'
             and config.params['project'] == 'try'
         )
--- a/taskcluster/taskgraph/try_option_syntax.py
+++ b/taskcluster/taskgraph/try_option_syntax.py
@@ -167,83 +167,16 @@ RIDEALONG_BUILDS = {
         'sm-mozjs-sys',
         'sm-msan',
     ],
 }
 
 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):
         """
         Parse a "try syntax" formatted commit message.  This is the old "-b do -p
         win32 -u all" format.  Aliases are applied to map short names to full
         names.
 
@@ -251,21 +184,18 @@ class TryOptionSyntax(object):
 
         - build_types: a list containing zero or more of 'opt' and 'debug'
         - platforms: a list of selected platform names, or None for all
         - unittests: a list of tests, of the form given below, or None for all
         - jobs: a list of requested job names, or None for all
         - 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:
 
         {
             'test': '<suite name>',
             'platforms': [..platform names..], # to limit to only certain platforms
             'only_chunks': set([..chunk numbers..]), # to limit only to certain chunks
         }
@@ -273,40 +203,56 @@ class TryOptionSyntax(object):
         self.jobs = []
         self.build_types = []
         self.platforms = []
         self.unittests = []
         self.talos = []
         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
+
+        # 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
 
-        args = parse_message(message)
-        if args 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)
         self.unittests = self.parse_test_option(
             "unittest_try_name", args.unittests, full_task_graph)
         self.talos = self.parse_test_option("talos_try_name", args.talos, full_task_graph)
         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']:
             return None
         expanded = []
         for job in jobs_arg:
             expanded.extend(j.strip() for j in job.split(','))
         return expanded
@@ -516,16 +462,43 @@ class TryOptionSyntax(object):
     def find_all_attribute_suffixes(self, graph, prefix):
         rv = set()
         for t in graph.tasks.itervalues():
             for a in t.attributes:
                 if a.startswith(prefix):
                     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
 
         def check_run_on_projects():
             return set(['try', 'all']) & set(attr('run_on_projects', []))
 
         def match_test(try_spec, attr_name):
             if attr('build_type') not in self.build_types:
@@ -582,14 +555,9 @@ class TryOptionSyntax(object):
             "build_types: " + ", ".join(self.build_types),
             "platforms: " + none_for_all(self.platforms),
             "unittests: " + none_for_all(self.unittests),
             "talos: " + none_for_all(self.talos),
             "jobs: " + none_for_all(self.jobs),
             "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),
         ])