Overhaul Goal and @console_rule such that @console_rules actually pro…

…duce an instance of their Goal type, and options are provided by an inner class of Goal.

build-support/bin/release.sh

@@ -155,6 +155,7 @@ function execute_packaged_pants_with_internal_backends() {
     --no-verify-config \
     --pythonpath="['pants-plugins/src/python']" \
     --backend-packages="[\
+        'pants.rules.core',\
         'pants.backend.codegen',\
         'pants.backend.docgen',\
         'pants.backend.graph_info',\

pants-plugins/src/python/internal_backend/rules_for_testing/register.py

@@ -8,7 +8,6 @@
 from pants.engine.console import Console
 from pants.engine.goal import Goal
 from pants.engine.rules import console_rule
-from pants.rules.core.exceptions import GracefulTerminationException
 
 
 class ListAndDieForTesting(Goal):
@@ -21,7 +20,7 @@ class ListAndDieForTesting(Goal):
 def fast_list_and_die_for_testing(console, addresses):
   for address in addresses.dependencies:
     console.print_stdout(address.spec)
-  raise GracefulTerminationException(exit_code=42)
+  yield ListAndDieForTesting(exit_code=42)
 
 
 def rules():

src/python/pants/backend/project_info/rules/source_file_validator.py

@@ -9,14 +9,14 @@
 
 from future.utils import text_type
 
+from pants.base.exiter import PANTS_FAILED_EXIT_CODE, PANTS_SUCCEEDED_EXIT_CODE
 from pants.engine.console import Console
 from pants.engine.fs import Digest, FilesContent
 from pants.engine.goal import Goal
 from pants.engine.legacy.graph import HydratedTarget, HydratedTargets
 from pants.engine.objects import Collection
 from pants.engine.rules import console_rule, optionable_rule, rule
 from pants.engine.selectors import Get
-from pants.rules.core.exceptions import GracefulTerminationException
 from pants.subsystem.subsystem import Subsystem
 from pants.util.memo import memoized_method
 from pants.util.objects import datatype, enum
@@ -210,12 +210,12 @@ def get_applicable_content_pattern_names(self, path):
 
 # TODO: Switch this to `lint` once we figure out a good way for v1 tasks and v2 rules
 # to share goal names.
-@console_rule(Validate, [Console, HydratedTargets, Validate])
-def validate(console, hydrated_targets, validate_goal):
+@console_rule(Validate, [Console, HydratedTargets, Validate.Options])
+def validate(console, hydrated_targets, validate_options):
   per_tgt_rmrs = yield [Get(RegexMatchResults, HydratedTarget, ht) for ht in hydrated_targets]
   regex_match_results = list(itertools.chain(*per_tgt_rmrs))
 
-  detail_level = validate_goal.options.detail_level
+  detail_level = validate_options.values.detail_level
   regex_match_results = sorted(regex_match_results, key=lambda x: x.path)
   num_matched_all = 0
   num_nonmatched_some = 0
@@ -241,7 +241,11 @@ def validate(console, hydrated_targets, validate_goal):
       num_nonmatched_some))
 
   if num_nonmatched_some:
-    raise GracefulTerminationException('Files failed validation.')
+    console.print_stderr('Files failed validation.')
+    exit_code = PANTS_FAILED_EXIT_CODE
+  else:
+    exit_code = PANTS_SUCCEEDED_EXIT_CODE
+  yield Validate(exit_code)
 
 
 @rule(RegexMatchResults, [HydratedTarget, SourceFileValidation])

src/python/pants/backend/project_info/tasks/filedeps.py

@@ -9,15 +9,23 @@
 from pants.backend.jvm.targets.jvm_app import JvmApp
 from pants.backend.jvm.targets.scala_library import ScalaLibrary
 from pants.base.build_environment import get_buildroot
-from pants.rules.core import filedeps as filedeps_rules
 from pants.task.console_task import ConsoleTask
 
 
 class FileDeps(ConsoleTask):
+  """List all source and BUILD files a target transitively depends on.
+
+  Files may be listed with absolute or relative paths and any BUILD files implied in the transitive
+  closure of targets are also included.
+  """
 
   @classmethod
-  def subsystem_dependencies(cls):
-    return super(FileDeps, cls).subsystem_dependencies() + (filedeps_rules.Filedeps,)
+  def register_options(cls, register):
+    super(FileDeps, cls).register_options(register)
+    register('--globs', type=bool,
+             help='Instead of outputting filenames, output globs (ignoring excludes)')
+    register('--absolute', type=bool, default=True,
+             help='If True output with absolute path, else output with path relative to the build root')
 
   def _file_path(self, path):
     return os.path.join(get_buildroot(), path) if self.get_options().absolute else path

src/python/pants/bin/daemon_pants_runner.py

@@ -23,7 +23,6 @@
 from pants.java.nailgun_io import NailgunStreamStdinReader, NailgunStreamWriter
 from pants.java.nailgun_protocol import ChunkType, NailgunProtocol
 from pants.pantsd.process_manager import ProcessManager
-from pants.rules.core.exceptions import GracefulTerminationException
 from pants.util.contextutil import hermetic_environment_as, stdio_as
 from pants.util.socket import teardown_socket
 
@@ -92,7 +91,7 @@ def create(cls, sock, args, env, services, scheduler_service):
       with cls.nailgunned_stdio(sock, env, handle_stdin=False):
         options, _, options_bootstrapper = LocalPantsRunner.parse_options(args, env)
         subprocess_dir = options.for_global_scope().pants_subprocessdir
-        graph_helper, target_roots = scheduler_service.prefork(options, options_bootstrapper)
+        graph_helper, target_roots, _ = scheduler_service.prefork(options, options_bootstrapper)
         deferred_exc = None
     except Exception:
       deferred_exc = sys.exc_info()
@@ -332,10 +331,6 @@ def post_fork_child(self):
         runner.run()
       except KeyboardInterrupt:
         self._exiter.exit_and_fail('Interrupted by user.\n')
-      except GracefulTerminationException as e:
-        ExceptionSink.log_exception(
-          'Encountered graceful termination exception {}; exiting'.format(e))
-        self._exiter.exit(e.exit_code)
       except Exception:
         # TODO: We override sys.excepthook above when we call ExceptionSink.set_exiter(). That
         # excepthook catches `SignalHandledNonLocalExit`s from signal handlers, which isn't

src/python/pants/bin/local_pants_runner.py

@@ -23,7 +23,6 @@
 from pants.init.target_roots_calculator import TargetRootsCalculator
 from pants.option.options_bootstrapper import OptionsBootstrapper
 from pants.reporting.reporting import Reporting
-from pants.rules.core.exceptions import GracefulTerminationException
 from pants.util.contextutil import maybe_profiled
 
 
@@ -273,14 +272,11 @@ def _maybe_run_v2(self):
       return PANTS_SUCCEEDED_EXIT_CODE
 
     try:
-      self._graph_session.run_console_rules(
+      return self._graph_session.run_console_rules(
         self._options_bootstrapper,
         goals,
         self._target_roots,
       )
-    except GracefulTerminationException as e:
-      logger.debug('Encountered graceful termination exception {}; exiting'.format(e))
-      return e.exit_code
     except Exception:
       logger.warning('Encountered unhandled exception during rule execution!')
       logger.warning(traceback.format_exc())

src/python/pants/engine/goal.py

@@ -10,14 +10,23 @@
 from pants.option.optionable import Optionable
 from pants.option.scope import ScopeInfo
 from pants.subsystem.subsystem_client_mixin import SubsystemClientMixin
+from pants.util.memo import memoized_classproperty
 from pants.util.meta import AbstractClass, classproperty
+from pants.util.objects import datatype
 
 
-class Goal(SubsystemClientMixin, Optionable, AbstractClass):
-  """A CLI goal whch is implemented by a `@console_rule`.
+class Goal(datatype([('exit_code', int)]), AbstractClass):
+  """The named product of a `@console_rule`.
   
   This abstract class should be subclassed and given a `Goal.name` that it will be referred to by
   when invoked from the command line. The `Goal.name` also acts as the options_scope for the `Goal`.
+
+  Since `@console_rules` always run in order to produce sideeffects (generally: console output), they
+  are not cacheable, and the `Goal` product of a `@console_rule` contains only a exit_code value to
+  indicate whether the rule exited cleanly.
+
+  Options values for a Goal can be retrived by declaring a dependency on the relevant `Goal.Options`
+  class.
   """
 
   # Subclasser-defined. See the class pydoc.
@@ -28,48 +37,76 @@ class Goal(SubsystemClientMixin, Optionable, AbstractClass):
   # that association.
   deprecated_cache_setup_removal_version = None
 
-  @classproperty
-  def options_scope(cls):
-    if not cls.name:
-      # TODO: Would it be unnecessarily magical to have `cls.__name__.lower()` always be the name?
-      raise AssertionError('{} must have a `Goal.name` defined.'.format(cls.__name__))
-    return cls.name
-
   @classmethod
-  def subsystem_dependencies(cls):
-    # NB: `Goal` implements `SubsystemClientMixin` in order to allow v1 `Tasks` to depend on
-    # v2 Goals, and for `Goals` to declare a deprecated dependency on a `CacheSetup` instance for
-    # backwards compatibility purposes. But v2 Goals should _not_ have subsystem dependencies:
-    # instead, the @rules participating (transitively) in a Goal should directly declare
-    # Subsystem deps.
-    if cls.deprecated_cache_setup_removal_version:
-      dep = CacheSetup.scoped(
-          cls,
-          removal_version=cls.deprecated_cache_setup_removal_version,
-          removal_hint='Goal `{}` uses an independent caching implementation, and ignores `{}`.'.format(
-            cls.name,
-            CacheSetup.subscope(cls.name),
-          )
-        )
-      return (dep,)
-    return tuple()
-
-  options_scope_category = ScopeInfo.GOAL
-
-  def __init__(self, scope, scoped_options):
-    # NB: This constructor is shaped to meet the contract of `Optionable(Factory).signature`.
-    super(Goal, self).__init__()
-    self._scope = scope
-    self._scoped_options = scoped_options
-
-  @property
-  def options(self):
-    """Returns the option values for this Goal."""
-    return self._scoped_options
+  def register_options(cls, register):
+    """Register options for the `Goal.Options` of this `Goal`.
+
+    Subclasses may override and call register(*args, **kwargs). Callers can retrieve the resulting
+    options values by declaring a dependency on the `Goal.Options` class.
+    """
+
+  @memoized_classproperty
+  def Options(cls):
+    # NB: The naming of this property is terribly evil. But this construction allows the inner class
+    # to get a reference to the outer class, which avoids implementers needing to subclass the inner
+    # class in order to define their options values, while still allowing for the useful namespacing
+    # of `Goal.Options`.
+    outer_cls = cls
+    class _Options(SubsystemClientMixin, Optionable, _GoalOptions):
+      @classproperty
+      def options_scope(cls):
+        if not outer_cls.name:
+          # TODO: Would it be unnecessarily magical to have `outer_cls.__name__.lower()` always be
+          # the name?
+          raise AssertionError('{} must have a `Goal.name` defined.'.format(outer_cls.__name__))
+        return outer_cls.name
+
+      @classmethod
+      def register_options(cls, register):
+        super(_Options, cls).register_options(register)
+        # Delegate to the outer class.
+        outer_cls.register_options(register)
+
+      @classmethod
+      def subsystem_dependencies(cls):
+        # NB: `Goal.Options` implements `SubsystemClientMixin` in order to allow v1 `Tasks` to
+        # depend on v2 Goals, and for `Goals` to declare a deprecated dependency on a `CacheSetup`
+        # instance for backwards compatibility purposes. But v2 Goals should _not_ have subsystem
+        # dependencies: instead, the @rules participating (transitively) in a Goal should directly
+        # declare their Subsystem deps.
+        if outer_cls.deprecated_cache_setup_removal_version:
+          dep = CacheSetup.scoped(
+              cls,
+              removal_version=outer_cls.deprecated_cache_setup_removal_version,
+              removal_hint='Goal `{}` uses an independent caching implementation, and ignores `{}`.'.format(
+                cls.options_scope,
+                CacheSetup.subscope(cls.options_scope),
+              )
+            )
+          return (dep,)
+        return tuple()
+
+      options_scope_category = ScopeInfo.GOAL
+
+      def __init__(self, scope, scoped_options):
+        # NB: This constructor is shaped to meet the contract of `Optionable(Factory).signature`.
+        super(_Options, self).__init__()
+        self._scope = scope
+        self._scoped_options = scoped_options
+
+      @property
+      def values(self):
+        """Returns the option values for these Goal.Options."""
+        return self._scoped_options
+    return _Options
+
+
+class _GoalOptions(object):
+  """A marker trait for the anonymous inner `Goal.Options` classes for `Goal`s."""
 
 
 class LineOriented(object):
-  """A mixin for Goal that adds options and a context manager for line-oriented output."""
+  """A mixin for Goal that adds Options to support the `line_oriented` context manager."""
 
   @classmethod
   def register_options(cls, register):
@@ -79,24 +116,32 @@ def register_options(cls, register):
     register('--output-file', metavar='<path>',
              help='Write line-oriented output to this file instead.')
 
-  @contextmanager
-  def line_oriented(self, console):
-    """Takes a Console and yields functions for writing to stdout and stderr, respectively."""
 
-    output_file = self.options.output_file
-    sep = self.options.sep.encode('utf-8').decode('unicode_escape')
+@contextmanager
+def line_oriented(line_oriented_options, console):
+  """Given options and a Console, yields functions for writing to stdout and stderr, respectively.
 
-    stdout, stderr = console.stdout, console.stderr
+  The passed options instance will generally be the `Goal.Options` of a `LineOriented` `Goal`.
+  """
+  if not isinstance(line_oriented_options, _GoalOptions):
+    raise AssertionError(
+        'Expected a `Goal.Options` instance for a `LineOriented` `Goal`, got: {}'.format(
+          type(line_oriented_options)))
+
+  output_file = line_oriented_options.values.output_file
+  sep = line_oriented_options.values.sep.encode('utf-8').decode('unicode_escape')
+
+  stdout, stderr = console.stdout, console.stderr
+  if output_file:
+    stdout = open(output_file, 'w')
+
+  try:
+    print_stdout = lambda msg: print(msg, file=stdout, end=sep)
+    print_stderr = lambda msg: print(msg, file=stderr)
+    yield print_stdout, print_stderr
+  finally:
     if output_file:
-      stdout = open(output_file, 'w')
-
-    try:
-      print_stdout = lambda msg: print(msg, file=stdout, end=sep)
-      print_stderr = lambda msg: print(msg, file=stderr)
-      yield print_stdout, print_stderr
-    finally:
-      if output_file:
-        stdout.close()
-      else:
-        stdout.flush()
-      stderr.flush()
+      stdout.close()
+    else:
+      stdout.flush()
+    stderr.flush()