From d1294ff4113b4ccb0a858a41a3c97f654aeea5c1 Mon Sep 17 00:00:00 2001
From: Benjy Weinberger <benjyw@gmail.com>
Date: Wed, 2 Dec 2015 19:37:27 +0000
Subject: [PATCH] Move more taks to core_tasks.

Changed a few names, and put each task in its own file,
for uniformity.

Also replaced the first=True argument to run_prep_command's
install with a comment explaining that it must be first in its
register.py, and another comment in extension_loader explaining
that core_tasks must be registered first.

The reason is that first=True was no less fragile - if anything
else registered into the 'test' goal with first=True later on,
it would preempt run_prep_command, with no comment. At least this way
the comments are clear, and we can maybe (??) move towards
deprecating first=True, or at least relying on it less.

Testing Done:
CI passes: https://travis-ci.org/pantsbuild/pants/builds/94466648

Reviewed at https://rbcommons.com/s/twitter/r/3199/
---
 src/python/pants/backend/core/register.py     | 37 ---------------
 src/python/pants/backend/core/tasks/BUILD     | 45 -------------------
 src/python/pants/bin/extension_loader.py      |  2 +
 src/python/pants/core_tasks/BUILD             |  5 +++
 src/python/pants/core_tasks/clean.py          | 16 +++++++
 .../clean.py => core_tasks/invalidate.py}     |  9 +---
 src/python/pants/core_tasks/register.py       | 25 ++++++++++-
 .../pants/core_tasks/reporting_server_kill.py | 34 ++++++++++++++
 .../reporting_server_run.py}                  | 26 ++---------
 .../core/tasks => core_tasks}/roots.py        |  2 +-
 .../tasks => core_tasks}/run_prep_command.py  |  0
 tests/python/pants_test/backend/core/BUILD    | 11 -----
 tests/python/pants_test/core_tasks/BUILD      | 28 ++++++++++++
 .../{tasks => core_tasks}/test_roots.py       |  2 +-
 .../test_run_prep_command.py}                 |  4 +-
 tests/python/pants_test/tasks/BUILD           | 13 ------
 16 files changed, 116 insertions(+), 143 deletions(-)
 create mode 100644 src/python/pants/core_tasks/clean.py
 rename src/python/pants/{backend/core/tasks/clean.py => core_tasks/invalidate.py} (78%)
 create mode 100644 src/python/pants/core_tasks/reporting_server_kill.py
 rename src/python/pants/{backend/core/tasks/reporting_server.py => core_tasks/reporting_server_run.py} (76%)
 rename src/python/pants/{backend/core/tasks => core_tasks}/roots.py (91%)
 rename src/python/pants/{backend/core/tasks => core_tasks}/run_prep_command.py (100%)
 rename tests/python/pants_test/{tasks => core_tasks}/test_roots.py (96%)
 rename tests/python/pants_test/{backend/core/test_prep.py => core_tasks/test_run_prep_command.py} (96%)

diff --git a/src/python/pants/backend/core/register.py b/src/python/pants/backend/core/register.py
index 4c62ce73bf7..622d55db0bb 100644
--- a/src/python/pants/backend/core/register.py
+++ b/src/python/pants/backend/core/register.py
@@ -6,7 +6,6 @@
                         unicode_literals, with_statement)
 
 import os
-import sys
 
 from pants.backend.core.from_target import FromTarget
 from pants.backend.core.targets.dependencies import Dependencies
@@ -15,7 +14,6 @@
 from pants.backend.core.targets.resources import Resources
 from pants.backend.core.tasks.bash_completion import BashCompletionTask
 from pants.backend.core.tasks.builddictionary import BuildBuildDictionary
-from pants.backend.core.tasks.clean import Cleaner, Invalidator
 from pants.backend.core.tasks.cloc import CountLinesOfCode
 from pants.backend.core.tasks.confluence_publish import ConfluencePublish
 from pants.backend.core.tasks.deferred_sources_mapper import DeferredSourcesMapper
@@ -30,9 +28,6 @@
 from pants.backend.core.tasks.minimal_cover import MinimalCover
 from pants.backend.core.tasks.pathdeps import PathDeps
 from pants.backend.core.tasks.paths import Path, Paths
-from pants.backend.core.tasks.reporting_server import KillServer, RunServer
-from pants.backend.core.tasks.roots import ListRoots
-from pants.backend.core.tasks.run_prep_command import RunPrepCommand
 from pants.backend.core.tasks.sorttargets import SortTargets
 from pants.backend.core.tasks.targets_help import TargetsHelp
 from pants.backend.core.wrapped_globs import Globs, RGlobs, ZGlobs
@@ -92,31 +87,6 @@ def register_goals():
 
   task(name='builddict', action=BuildBuildDictionary).install()
 
-  # Cleaning.
-  invalidate = task(name='invalidate', action=Invalidator)
-  invalidate.install().with_description('Invalidate all targets.')
-
-  clean_all = task(name='clean-all', action=Cleaner).install()
-  clean_all.with_description('Clean all build output.')
-  clean_all.install(invalidate, first=True)
-
-  class AsyncCleaner(Cleaner):
-    def execute(self):
-      print('The `clean-all-async` goal is deprecated and currently just forwards to `clean-all`.',
-            file=sys.stderr)
-      print('Please update your usages to `clean-all`.', file=sys.stderr)
-      super(AsyncCleaner, self).execute()
-  clean_all_async = task(name='clean-all-async', action=AsyncCleaner).install().with_description(
-      '[deprecated] Clean all build output in a background process.')
-  clean_all_async.install(invalidate, first=True)
-
-  # Reporting.
-  task(name='server', action=RunServer, serialize=False).install().with_description(
-      'Run the pants reporting server.')
-
-  task(name='killserver', action=KillServer, serialize=False).install().with_description(
-      'Kill the reporting server.')
-
   task(name='markdown', action=MarkdownToHtml).install('markdown').with_description(
       'Generate html from markdown docs.')
 
@@ -148,13 +118,6 @@ def execute(self):
   task(name='sort', action=SortTargets).install().with_description(
       'Topologically sort the targets.')
 
-  task(name='run_prep_command', action=RunPrepCommand).install('test', first=True).with_description(
-      "Run a command before tests")
-
-  # Source tree information.
-  task(name='roots', action=ListRoots).install('roots').with_description(
-    "Print the workspace's source roots and associated target types.")
-
   task(name='cloc', action=CountLinesOfCode).install('cloc').with_description(
     "Print counts of lines of code.")
 
diff --git a/src/python/pants/backend/core/tasks/BUILD b/src/python/pants/backend/core/tasks/BUILD
index 2719cf13152..acd5fb49b80 100644
--- a/src/python/pants/backend/core/tasks/BUILD
+++ b/src/python/pants/backend/core/tasks/BUILD
@@ -44,7 +44,6 @@ target(
   dependencies = [
     ':bash_completion',
     ':builddictionary',
-    ':clean',
     ':cloc',
     ':confluence_publish',
     ':console_task',
@@ -62,9 +61,6 @@ target(
     ':pathdeps',
     ':paths',
     ':repl_task_mixin',
-    ':reporting_server',
-    ':roots',
-    ':run_prep_command',
     ':scm_publish',
     ':sorttargets',
     ':targets_help',
@@ -121,17 +117,6 @@ resources(
   sources = globs('templates/builddictionary/*.mustache'),
 )
 
-
-python_library(
-  name = 'clean',
-  sources = ['clean.py'],
-  dependencies = [
-    'src/python/pants/base:build_environment',
-    'src/python/pants/task',
-    'src/python/pants/util:dirutil',
-  ],
-)
-
 python_library(
   name = 'cloc',
   sources = ['cloc.py'],
@@ -295,18 +280,6 @@ python_library(
   ],
 )
 
-python_library(
-  name = 'reporting_server',
-  sources = ['reporting_server.py'],
-  dependencies = [
-    'src/python/pants/base:build_environment',
-    'src/python/pants/base:run_info',
-    'src/python/pants/binaries:binary_util',
-    'src/python/pants/reporting',
-    'src/python/pants/task',
-  ],
-)
-
 python_library(
   name = 'reflect',
   sources = ['reflect.py'],
@@ -324,24 +297,6 @@ python_library(
   ],
 )
 
-python_library(
-  name = 'roots',
-  sources = ['roots.py'],
-  dependencies = [
-    'src/python/pants/task',
-  ],
-)
-
-python_library(
-  name = 'run_prep_command',
-  sources = ['run_prep_command.py'],
-  dependencies = [
-    'src/python/pants/base:exceptions',
-    'src/python/pants/base:workunit',
-    'src/python/pants/task',
-  ],
-)
-
 python_library(
   name = 'scm_publish',
   sources = ['scm_publish.py'],
diff --git a/src/python/pants/bin/extension_loader.py b/src/python/pants/bin/extension_loader.py
index f7ec6ae63e2..3f8dc7bc4dc 100644
--- a/src/python/pants/bin/extension_loader.py
+++ b/src/python/pants/bin/extension_loader.py
@@ -101,6 +101,8 @@ def load_build_configuration_from_source(build_configuration, additional_backend
   :raises: :class:``pants.base.exceptions.BuildConfigurationError`` if there is a problem loading
     the build configuration.
   """
+  # Note: pants.core_tasks must be first in this list, as it registers various stubs
+  # that other tasks can use for scheduling against.
   # TODO: Allow repos to opt in to any backend (but not to core_tasks, which must always
   # be loaded).
   backend_packages = ['pants.core_tasks',
diff --git a/src/python/pants/core_tasks/BUILD b/src/python/pants/core_tasks/BUILD
index 7e8b1aa3b11..f5d48ccc654 100644
--- a/src/python/pants/core_tasks/BUILD
+++ b/src/python/pants/core_tasks/BUILD
@@ -5,6 +5,11 @@ python_library(
   name='core_tasks',
   sources=globs('*.py'),
   dependencies=[
+    'src/python/pants/base:exceptions',
+    'src/python/pants/base:workunit',
+    'src/python/pants/binaries:binary_util',
     'src/python/pants/goal:task_registrar',
+    'src/python/pants/reporting',
     'src/python/pants/task',
+    'src/python/pants/util:dirutil',
   ])
diff --git a/src/python/pants/core_tasks/clean.py b/src/python/pants/core_tasks/clean.py
new file mode 100644
index 00000000000..b84b889d61d
--- /dev/null
+++ b/src/python/pants/core_tasks/clean.py
@@ -0,0 +1,16 @@
+# coding=utf-8
+# Copyright 2014 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
+                        unicode_literals, with_statement)
+
+from pants.task.task import Task
+from pants.util.dirutil import safe_rmtree
+
+
+class Clean(Task):
+  """Delete all build products, creating a clean workspace."""
+
+  def execute(self):
+    safe_rmtree(self.get_options().pants_workdir)
diff --git a/src/python/pants/backend/core/tasks/clean.py b/src/python/pants/core_tasks/invalidate.py
similarity index 78%
rename from src/python/pants/backend/core/tasks/clean.py
rename to src/python/pants/core_tasks/invalidate.py
index 89afde4fe00..b9e50c836bf 100644
--- a/src/python/pants/backend/core/tasks/clean.py
+++ b/src/python/pants/core_tasks/invalidate.py
@@ -11,16 +11,9 @@
 from pants.util.dirutil import safe_rmtree
 
 
-class Invalidator(Task):
+class Invalidate(Task):
   """Invalidate the entire build."""
 
   def execute(self):
     build_invalidator_dir = os.path.join(self.get_options().pants_workdir, 'build_invalidator')
     safe_rmtree(build_invalidator_dir)
-
-
-class Cleaner(Task):
-  """Clean all current build products."""
-
-  def execute(self):
-    safe_rmtree(self.get_options().pants_workdir)
diff --git a/src/python/pants/core_tasks/register.py b/src/python/pants/core_tasks/register.py
index 35849f96c71..28a1ef5414f 100644
--- a/src/python/pants/core_tasks/register.py
+++ b/src/python/pants/core_tasks/register.py
@@ -6,18 +6,39 @@
                         unicode_literals, with_statement)
 
 from pants.core_tasks.changed_target_tasks import CompileChanged, TestChanged
+from pants.core_tasks.clean import Clean
+from pants.core_tasks.invalidate import Invalidate
 from pants.core_tasks.noop import NoopCompile, NoopTest
+from pants.core_tasks.reporting_server_kill import ReportingServerKill
+from pants.core_tasks.reporting_server_run import ReportingServerRun
+from pants.core_tasks.roots import ListRoots
+from pants.core_tasks.run_prep_command import RunPrepCommand
 from pants.core_tasks.what_changed import WhatChanged
 from pants.goal.task_registrar import TaskRegistrar as task
 
 
 def register_goals():
-  task(name='changed', action=WhatChanged).install()
+  # Cleaning.
+  task(name='invalidate', action=Invalidate).install()
+  task(name='clean-all', action=Clean).install()
+
+  # Reporting server.
+  # TODO: The reporting server should be subsumed into pantsd, and not run via a task.
+  task(name='server', action=ReportingServerRun, serialize=False).install()
+  task(name='killserver', action=ReportingServerKill, serialize=False).install()
 
   # Stub for other goals to schedule 'compile'. See noop_exec_task.py for why this is useful.
   task(name='compile', action=NoopCompile).install('compile')
-  task(name='compile-changed', action=CompileChanged).install()
 
+  # Must be the first thing we register under 'test'.
+  task(name='run_prep_command', action=RunPrepCommand).install('test')
   # Stub for other goals to schedule 'test'. See noop_exec_task.py for why this is useful.
   task(name='test', action=NoopTest).install('test')
+
+  # Operations on files that the SCM detects as changed.
+  task(name='changed', action=WhatChanged).install()
+  task(name='compile-changed', action=CompileChanged).install()
   task(name='test-changed', action=TestChanged).install()
+
+  # Workspace information.
+  task(name='roots', action=ListRoots).install()
diff --git a/src/python/pants/core_tasks/reporting_server_kill.py b/src/python/pants/core_tasks/reporting_server_kill.py
new file mode 100644
index 00000000000..105b9734749
--- /dev/null
+++ b/src/python/pants/core_tasks/reporting_server_kill.py
@@ -0,0 +1,34 @@
+# coding=utf-8
+# Copyright 2014 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
+                        unicode_literals, with_statement)
+
+import logging
+
+from pants.reporting.reporting_server import ReportingServerManager
+from pants.task.task import QuietTaskMixin, Task
+
+
+logger = logging.getLogger(__name__)
+
+
+class ReportingServerKill(QuietTaskMixin, Task):
+  """Kill the reporting server."""
+
+  def execute(self):
+    server = ReportingServerManager(self.context, self.get_options())
+
+    if not server.is_alive():
+      logger.info('No server found.')
+      return
+
+    pid = server.pid
+
+    try:
+      logger.info('Killing server with {pid} at http://localhost:{port}'
+                  .format(pid=pid, port=server.socket))
+      server.terminate()
+    except ReportingServerManager.NonResponsiveProcess:
+      logger.info('Failed to kill server with pid {pid}!'.format(pid=pid))
diff --git a/src/python/pants/backend/core/tasks/reporting_server.py b/src/python/pants/core_tasks/reporting_server_run.py
similarity index 76%
rename from src/python/pants/backend/core/tasks/reporting_server.py
rename to src/python/pants/core_tasks/reporting_server_run.py
index 64641967249..1f5e3e923de 100644
--- a/src/python/pants/backend/core/tasks/reporting_server.py
+++ b/src/python/pants/core_tasks/reporting_server_run.py
@@ -15,12 +15,12 @@
 logger = logging.getLogger(__name__)
 
 
-class RunServer(QuietTaskMixin, Task):
-  """Runs the reporting server."""
+class ReportingServerRun(QuietTaskMixin, Task):
+  """Run the reporting server."""
 
   @classmethod
   def register_options(cls, register):
-    super(RunServer, cls).register_options(register)
+    super(ReportingServerRun, cls).register_options(register)
     register('--port', type=int, default=0,
              help='Serve on this port. Leave unset to choose a free port '
                   'automatically (recommended if using pants concurrently in '
@@ -55,23 +55,3 @@ def execute(self):
                   .format(pid=manager.pid, port=manager.socket))
 
     self._maybe_open(manager.socket)
-
-
-class KillServer(QuietTaskMixin, Task):
-  """Kills the reporting server."""
-
-  def execute(self):
-    server = ReportingServerManager(self.context, self.get_options())
-
-    if not server.is_alive():
-      logger.info('No server found.')
-      return
-
-    pid = server.pid
-
-    try:
-      logger.info('Killing server with {pid} at http://localhost:{port}'
-                  .format(pid=pid, port=server.socket))
-      server.terminate()
-    except ReportingServerManager.NonResponsiveProcess:
-      logger.info('Failed to kill server with pid {pid}!'.format(pid=pid))
diff --git a/src/python/pants/backend/core/tasks/roots.py b/src/python/pants/core_tasks/roots.py
similarity index 91%
rename from src/python/pants/backend/core/tasks/roots.py
rename to src/python/pants/core_tasks/roots.py
index 97eaa34556d..ba5207daac4 100644
--- a/src/python/pants/backend/core/tasks/roots.py
+++ b/src/python/pants/core_tasks/roots.py
@@ -9,7 +9,7 @@
 
 
 class ListRoots(ConsoleTask):
-  """List the registered source roots of the repo."""
+  """List the repo's registered source roots."""
 
   def console_output(self, targets):
     for src_root in self.context.source_roots.all_roots():
diff --git a/src/python/pants/backend/core/tasks/run_prep_command.py b/src/python/pants/core_tasks/run_prep_command.py
similarity index 100%
rename from src/python/pants/backend/core/tasks/run_prep_command.py
rename to src/python/pants/core_tasks/run_prep_command.py
diff --git a/tests/python/pants_test/backend/core/BUILD b/tests/python/pants_test/backend/core/BUILD
index 07bdf867dfa..1f496eb8399 100644
--- a/tests/python/pants_test/backend/core/BUILD
+++ b/tests/python/pants_test/backend/core/BUILD
@@ -5,7 +5,6 @@ target(
   name='core',
   dependencies=[
     ':wrapped_globs',
-    ':prep',
     'tests/python/pants_test/backend/core/tasks',
   ],
 )
@@ -18,13 +17,3 @@ python_tests(
     'tests/python/pants_test:base_test',
   ]
 )
-
-python_tests(
-  name = 'prep',
-  sources = ['test_prep.py'],
-  dependencies = [
-    '3rdparty/python:six',
-    'src/python/pants/backend/core:plugin',
-    'tests/python/pants_test/tasks:task_test_base',
-  ]
-)
diff --git a/tests/python/pants_test/core_tasks/BUILD b/tests/python/pants_test/core_tasks/BUILD
index f6ff6318717..ce0bb412ec5 100644
--- a/tests/python/pants_test/core_tasks/BUILD
+++ b/tests/python/pants_test/core_tasks/BUILD
@@ -1,6 +1,34 @@
 # Copyright 2015 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
+
+python_tests(
+  name = 'run_prep_command',
+  sources = ['test_run_prep_command.py'],
+  dependencies = [
+    '3rdparty/python:six',
+    'src/python/pants/backend/core/targets:common',
+    'src/python/pants/base:exceptions',
+    'src/python/pants/build_graph',
+    'src/python/pants/core_tasks',
+    'src/python/pants/util:contextutil',
+    'src/python/pants/util:dirutil',
+    'tests/python/pants_test/tasks:task_test_base',
+  ]
+)
+
+python_tests(
+  name = 'roots',
+  sources = ['test_roots.py'],
+  dependencies = [
+    'src/python/pants/base:build_environment',
+    'src/python/pants/core_tasks',
+    'src/python/pants/source',
+    'tests/python/pants_test/subsystem:subsystem_utils',
+    'tests/python/pants_test/tasks:task_test_base',
+  ],
+)
+
 python_tests(
   name = 'what_changed',
   sources = ['test_what_changed.py'],
diff --git a/tests/python/pants_test/tasks/test_roots.py b/tests/python/pants_test/core_tasks/test_roots.py
similarity index 96%
rename from tests/python/pants_test/tasks/test_roots.py
rename to tests/python/pants_test/core_tasks/test_roots.py
index 7018987a4bd..d162a138140 100644
--- a/tests/python/pants_test/tasks/test_roots.py
+++ b/tests/python/pants_test/core_tasks/test_roots.py
@@ -7,8 +7,8 @@
 
 import os
 
-from pants.backend.core.tasks.roots import ListRoots
 from pants.base.build_environment import get_buildroot
+from pants.core_tasks.roots import ListRoots
 from pants.source.source_root import SourceRootConfig
 from pants_test.subsystem.subsystem_util import subsystem_instance
 from pants_test.tasks.task_test_base import ConsoleTaskTestBase
diff --git a/tests/python/pants_test/backend/core/test_prep.py b/tests/python/pants_test/core_tasks/test_run_prep_command.py
similarity index 96%
rename from tests/python/pants_test/backend/core/test_prep.py
rename to tests/python/pants_test/core_tasks/test_run_prep_command.py
index 55d7fb84b87..ebcbbe10e67 100644
--- a/tests/python/pants_test/backend/core/test_prep.py
+++ b/tests/python/pants_test/core_tasks/test_run_prep_command.py
@@ -10,15 +10,15 @@
 from six.moves import range
 
 from pants.backend.core.targets.prep_command import PrepCommand
-from pants.backend.core.tasks.run_prep_command import RunPrepCommand
 from pants.base.exceptions import TaskError
 from pants.build_graph.build_file_aliases import BuildFileAliases
+from pants.core_tasks.run_prep_command import RunPrepCommand
 from pants.util.contextutil import temporary_dir
 from pants.util.dirutil import touch
 from pants_test.tasks.task_test_base import TaskTestBase
 
 
-class PrepTest(TaskTestBase):
+class RunPrepCommandTest(TaskTestBase):
   @classmethod
   def task_type(cls):
     return RunPrepCommand
diff --git a/tests/python/pants_test/tasks/BUILD b/tests/python/pants_test/tasks/BUILD
index 8720643671f..b03696da31e 100644
--- a/tests/python/pants_test/tasks/BUILD
+++ b/tests/python/pants_test/tasks/BUILD
@@ -24,7 +24,6 @@ target(
     ':listtargets',
     ':minimal_cover',
     ':reflect',
-    ':roots',
     ':sorttargets',
     ':targets_help',
   ],
@@ -148,18 +147,6 @@ python_tests(
   ],
 )
 
-python_tests(
-  name = 'roots',
-  sources = ['test_roots.py'],
-  dependencies = [
-    ':task_test_base',
-    'src/python/pants/backend/core/tasks:roots',
-    'src/python/pants/base:build_environment',
-    'src/python/pants/source',
-    'tests/python/pants_test/subsystem:subsystem_utils',
-  ],
-)
-
 python_tests(
   name = 'sorttargets',
   sources = ['test_sorttargets.py'],