Bug 1372510 - Exclude 'job' tasks that require a build from the default try selection, r?dustin draft
authorAndrew Halberstadt <ahalberstadt@mozilla.com>
Tue, 13 Jun 2017 11:04:08 -0400
changeset 593484 e36054b3f62f94508d5bfdb64b415fe00c305aa4
parent 592551 f9605772a0c9098ed1bcaa98089b2c944ed69e9b
child 633125 34b181d955488e3d4892b94ea58d8340bf538b5e
push id63712
push userahalberstadt@mozilla.com
push dateTue, 13 Jun 2017 18:23:12 +0000
reviewersdustin
bugs1372510
milestone55.0a1
Bug 1372510 - Exclude 'job' tasks that require a build from the default try selection, r?dustin It's a bit hacky to single out 'build' dependencies, but most tasks have a dependency on a docker image, so we can't blanket skip all 'job' tasks with any dependencies at all. This is far from ideal but is an improvement on the current behaviour of running build dependencies all the time, even if the 'job' task gets optimized away. There is likely a cleverer solution, but that can be follow-up work. MozReview-Commit-ID: 6T68LT5VSrg
taskcluster/taskgraph/target_tasks.py
taskcluster/taskgraph/test/test_target_tasks.py
taskcluster/taskgraph/try_option_syntax.py
--- a/taskcluster/taskgraph/target_tasks.py
+++ b/taskcluster/taskgraph/target_tasks.py
@@ -49,17 +49,17 @@ def standard_filter(task, parameters):
 
 
 @_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)]
+                           if options.task_matches(t)]
 
     attributes = {
         k: getattr(options, k) for k in [
             'env',
             'no_retry',
             'tag',
         ]
     }
--- a/taskcluster/taskgraph/test/test_target_tasks.py
+++ b/taskcluster/taskgraph/test/test_target_tasks.py
@@ -20,18 +20,18 @@ class FakeTryOptionSyntax(object):
         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
+    def task_matches(self, task):
+        return 'at-at' in task.attributes
 
 
 class TestTargetTasks(unittest.TestCase):
 
     def default_matches(self, run_on_projects, project):
         method = target_tasks.get_method('default')
         graph = TaskGraph(tasks={
             'a': Task(kind='build', label='a',
--- a/taskcluster/taskgraph/try_option_syntax.py
+++ b/taskcluster/taskgraph/try_option_syntax.py
@@ -543,18 +543,18 @@ 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 task_matches(self, attributes):
-        attr = attributes.get
+    def task_matches(self, task):
+        attr = task.attributes.get
 
         def check_run_on_projects():
             if attr('nightly') and not self.include_nightly:
                 return False
             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:
@@ -569,30 +569,36 @@ class TryOptionSyntax(object):
                 return True
             # TODO: optimize this search a bit
             for test in try_spec:
                 if attr(attr_name) == test['test']:
                     break
             else:
                 return False
             if 'platforms' in test:
-                platform = attributes.get('test_platform', '').split('/')[0]
+                platform = attr('test_platform', '').split('/')[0]
                 if platform not in test['platforms']:
                     return False
             if 'only_chunks' in test and attr('test_chunk') not in test['only_chunks']:
                 return False
             return True
 
         job_try_name = attr('job_try_name')
         if job_try_name:
             # Beware the subtle distinction between [] and None for self.jobs and self.platforms.
             # They will be [] if there was no try syntax, and None if try syntax was detected but
             # they remained unspecified.
             if self.jobs and job_try_name not in self.jobs:
                 return False
+            elif not self.jobs and 'build' in task.dependencies:
+                # We exclude tasks with build dependencies from the default set of jobs because
+                # they will schedule their builds even if they end up optimized away. This means
+                # to run these tasks on try, they'll need to be explicitly specified by -j until
+                # we find a better solution (see bug 1372510).
+                return False
             elif not self.jobs and attr('build_platform'):
                 if self.platforms is None or attr('build_platform') in self.platforms:
                     return True
                 return False
             return True
         elif attr('kind') == 'test':
             return match_test(self.unittests, 'unittest_try_name') \
                  or match_test(self.talos, 'talos_try_name')