Bug 1403342 - default to `-j none` and do not optimize_target_tasks for try; r=ahal
authorDustin J. Mitchell <dustin@mozilla.com>
Tue, 03 Oct 2017 21:15:15 +0000
changeset 385701 911bafc2b6858b1ef0c2b98c3e1f9ed714caa41b
parent 385700 5c8014361629ca5734aaa42afce360bc48411a39
child 385702 831c287a4ae882823ec7411a00b05e88e69ebda2
push id32664
push userarchaeopteryx@coole-files.de
push dateThu, 12 Oct 2017 09:34:55 +0000
treeherdermozilla-central@a32c32d9631c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersahal
bugs1403342
milestone58.0a1
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
Bug 1403342 - default to `-j none` and do not optimize_target_tasks for try; r=ahal With this in place, all `-j`obs will not run by default on try. This will omit such jobs in most try pushes even if files-changed matches. This is unfortunate, but better than running them unconditionally. Fuzzy selections, and later `just try it` pushes, are the ultimate solution here. With this change, a push with no try syntax or try_task_config.json will schedule no tasks at all. MozReview-Commit-ID: FGjqlDW1FT6
taskcluster/docs/try.rst
taskcluster/taskgraph/decision.py
taskcluster/taskgraph/target_tasks.py
taskcluster/taskgraph/test/test_target_tasks.py
taskcluster/taskgraph/test/test_try_option_syntax.py
taskcluster/taskgraph/try_option_syntax.py
--- a/taskcluster/docs/try.rst
+++ b/taskcluster/docs/try.rst
@@ -8,17 +8,18 @@ various forms, and can sometimes show it
 Access to "push to try" is typically avilable to a much larger group of
 developers than those who can land changes in integration and release branches.
 Specifically, try pushes are allowed for anyone with `SCM Level`_ 1, while
 integration branches are at SCM level 3.
 
 Scheduling a Task on Try
 ------------------------
 
-There are two methods for scheduling a task on try.
+There are three methods for scheduling a task on try: legacy try option syntax,
+try task config, and an empty try.
 
 Try Option Syntax
 :::::::::::::::::
 
 The first, older method is a command line string called ``try syntax`` which is passed
 into the decision task via the commit message. The resulting commit is then
 pushed to the https://hg.mozilla.org/try repository.  An example try syntax
 might look like:
@@ -127,14 +128,22 @@ It looks like this:
         "enabled": 1,
         ...
       },
       "taskId": "<task id>"
     }
 
 See the `existing templates`_ for examples.
 
+Empty Try
+:::::::::
+
+If there is no try syntax or ``try_task_config.json``, the ``try_mode``
+parameter is None and no tasks are selected to run.  The resulting push will
+only have a decision task, but one with an "add jobs" action that can be used
+to add the desired jobs to the try push.
+
 .. _tryselect: https://dxr.mozilla.org/mozilla-central/source/tools/tryselect
 .. _JSON-e: https://taskcluster.github.io/json-e/
 .. _taskgraph module: https://dxr.mozilla.org/mozilla-central/source/taskcluster/taskgraph/templates
 .. _condition statements: https://taskcluster.github.io/json-e/#%60$if%60%20-%20%60then%60%20-%20%60else%60
 .. _existing templates: https://dxr.mozilla.org/mozilla-central/source/taskcluster/taskgraph/templates
 .. _SCM Level: https://www.mozilla.org/en-US/about/governance/policies/commit/access-policy/
--- a/taskcluster/taskgraph/decision.py
+++ b/taskcluster/taskgraph/decision.py
@@ -210,21 +210,17 @@ def get_decision_parameters(options):
 
         if 'try:' in parameters['message']:
             parameters['try_mode'] = 'try_option_syntax'
             args = parse_message(parameters['message'])
             parameters['try_options'] = args
         else:
             parameters['try_options'] = None
 
-        if parameters['try_mode'] == 'try_option_syntax':
-            # Try option syntax is imprecise, so optimize away tasks even if they
-            # are selected by the syntax.
-            parameters['optimize_target_tasks'] = True
-        elif parameters['try_mode'] == 'try_task_config':
+        if parameters['try_mode']:
             # The user has explicitly requested a set of jobs, so run them all
             # regardless of optimization.  Their dependencies can be optimized,
             # though.
             parameters['optimize_target_tasks'] = False
         else:
             # For a try push with no task selection, apply the default optimization
             # process to all of the tasks.
             parameters['optimize_target_tasks'] = True
--- a/taskcluster/taskgraph/target_tasks.py
+++ b/taskcluster/taskgraph/target_tasks.py
@@ -105,21 +105,19 @@ def _try_option_syntax(full_task_graph, 
 @_target_task('try_tasks')
 def target_tasks_try(full_task_graph, parameters):
     try_mode = parameters['try_mode']
     if try_mode == 'try_task_config':
         return _try_task_config(full_task_graph, parameters)
     elif try_mode == 'try_option_syntax':
         return _try_option_syntax(full_task_graph, parameters)
     else:
-        # With no try mode, we would like to schedule everything (following
-        # run_on_projects) and let optimization trim it down.  But optimization
-        # isn't yet up to the task, so instead we use try_option_syntax with
-        # an empty message (which basically just schedules `-j`objs)
-        return _try_option_syntax(full_task_graph, parameters)
+        # With no try mode, we schedule nothing, allowing the user to add tasks
+        # later via treeherder.
+        return []
 
 
 @_target_task('default')
 def target_tasks_default(full_task_graph, parameters):
     """Target the tasks which have indicated they should be run on this project
     via the `run_on_projects` attributes."""
     return [l for l, t in full_task_graph.tasks.iteritems()
             if standard_filter(t, parameters)]
--- a/taskcluster/taskgraph/test/test_target_tasks.py
+++ b/taskcluster/taskgraph/test/test_target_tasks.py
@@ -79,26 +79,27 @@ class TestTargetTasks(unittest.TestCase)
     def fake_TryOptionSyntax(self):
         orig_TryOptionSyntax = try_option_syntax.TryOptionSyntax
         try:
             try_option_syntax.TryOptionSyntax = FakeTryOptionSyntax
             yield
         finally:
             try_option_syntax.TryOptionSyntax = orig_TryOptionSyntax
 
-    def test_just_try_it(self):
-        "try_mode = None runs try optoin syntax with no options"
+    def test_empty_try(self):
+        "try_mode = None runs nothing"
         tg = self.make_task_graph()
         method = target_tasks.get_method('try_tasks')
-        with self.fake_TryOptionSyntax():
-            params = {
-                'try_mode': None,
-                'message': '',
-            }
-            self.assertEqual(method(tg, params), ['b'])
+        params = {
+            'try_mode': None,
+            'project': 'try',
+            'message': '',
+        }
+        # only runs the task with run_on_projects: try
+        self.assertEqual(method(tg, params), [])
 
     def test_try_option_syntax(self):
         "try_mode = try_option_syntax uses TryOptionSyntax"
         tg = self.make_task_graph()
         method = target_tasks.get_method('try_tasks')
         with self.fake_TryOptionSyntax():
             params = {
                 'try_mode': 'try_option_syntax',
--- a/taskcluster/taskgraph/test/test_try_option_syntax.py
+++ b/taskcluster/taskgraph/test/test_try_option_syntax.py
@@ -64,17 +64,17 @@ graph_with_jobs = TaskGraph(tasks, Graph
 class TestTryOptionSyntax(unittest.TestCase):
 
     def test_unknown_args(self):
         "unknown arguments are ignored"
         parameters = {'try_options': parse_message('try: --doubledash -z extra')}
         tos = TryOptionSyntax(parameters, graph_with_jobs)
         # equilvant to "try:"..
         self.assertEqual(tos.build_types, [])
-        self.assertEqual(tos.jobs, None)
+        self.assertEqual(tos.jobs, [])
 
     def test_apostrophe_in_message(self):
         "apostrophe does not break parsing"
         parameters = {'try_options': parse_message('Increase spammy log\'s log level. try: -b do')}
         tos = TryOptionSyntax(parameters, graph_with_jobs)
         self.assertEqual(sorted(tos.build_types), ['debug', 'opt'])
 
     def test_b_do(self):
--- a/taskcluster/taskgraph/try_option_syntax.py
+++ b/taskcluster/taskgraph/try_option_syntax.py
@@ -318,17 +318,19 @@ class TryOptionSyntax(object):
         self.talos_trigger_tests = options['talos_trigger_tests']
         self.env = options['env']
         self.profile = options['profile']
         self.tag = options['tag']
         self.no_retry = options['no_retry']
         self.include_nightly = options['include_nightly']
 
     def parse_jobs(self, jobs_arg):
-        if not jobs_arg or jobs_arg == ['all']:
+        if not jobs_arg or jobs_arg == ['none']:
+            return []  # default is `-j none`
+        if jobs_arg == ['all']:
             return None
         expanded = []
         for job in jobs_arg:
             expanded.extend(j.strip() for j in job.split(','))
         return expanded
 
     def parse_build_types(self, build_types_arg, full_task_graph):
         if build_types_arg is None:
@@ -592,33 +594,27 @@ class TryOptionSyntax(object):
                 platform = attr('test_platform', '').split('/')[0]
                 # Platforms can be forced by syntax like "-u xpcshell[Windows 8]"
                 return platform in test['platforms']
             elif run_by_default:
                 return check_run_on_projects()
             else:
                 return False
 
-        job_try_name = attr('job_try_name')
-        if job_try_name:
+        if attr('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:
-                return job_try_name in self.jobs
-            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
+            if self.jobs is not None:
+                return attr('job_try_name') in self.jobs
+
+            # User specified `-j all`
+            if self.platforms is not None and attr('build_platform') not in self.platforms:
+                return False  # honor -p for jobs governed by a platform
+            # "all" means "everything with `try` in run_on_projects"
             return check_run_on_projects()
         elif attr('kind') == 'test':
             return match_test(self.unittests, 'unittest_try_name') \
                  or match_test(self.talos, 'talos_try_name')
         elif attr('kind') in BUILD_KINDS:
             if attr('build_type') not in self.build_types:
                 return False
             elif self.platforms is None: