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 679129 911bafc2b6858b1ef0c2b98c3e1f9ed714caa41b
parent 679128 5c8014361629ca5734aaa42afce360bc48411a39
child 679130 831c287a4ae882823ec7411a00b05e88e69ebda2
push id84141
push userbmo:schien@mozilla.com
push dateThu, 12 Oct 2017 11:13:04 +0000
reviewersahal
bugs1403342
milestone58.0a1
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: